Skip to content

Commit

Permalink
pkp#9911 changes ofter the code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@bozana
bozana committed Aug 15, 2024
1 parent 3af0d32 commit ffcb8c3
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 41 deletions.
22 changes: 12 additions & 10 deletions classes/cliTool/traits/ConvertLogFile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ public function convert(string $fileName): void
$context = $this->contextsByPath[$foundContextPath];
$newEntry['contextId'] = $context->getId();

$this->setAssoc($assocType, $page, $op, $args, $newEntry);
// temporarily set the canonicalUrlPage that is needed in the child class
$newEntry['canonicalUrlPage'] = $page;
$this->setAssoc($assocType, $args, $newEntry);
if (!array_key_exists('assocType', $newEntry)) {
if (!$this->isApacheAccessLogFile()) {
fwrite(STDERR, "The URL {$entryData['url']} in the line number {$lineNumber} was not considered." . PHP_EOL);
Expand All @@ -200,10 +202,10 @@ public function convert(string $fileName): void
$canonicalUrl = $entryData['url']; // if this is not the apache log file i.e. it is the internal log file, the URLs are already canonical
if ($this->isApacheAccessLogFile()) {
$canonicalUrl = $this->getCanonicalUrl($foundContextPath, $newEntry['canonicalUrlPage'], $newEntry['canonicalUrlOp'], $newEntry['canonicalUrlArgs'] ?? null);
// unset elements that are temporarily used and should not be logged
unset($newEntry['canonicalUrlPage'], $newEntry['canonicalUrlOp'], $newEntry['canonicalUrlArgs']);
}
$newEntry['canonicalUrl'] = $canonicalUrl;
// unset elements that are temporarily used and should not be logged
unset($newEntry['canonicalUrlPage'], $newEntry['canonicalUrlOp'], $newEntry['canonicalUrlArgs']);
} else {
continue;
}
Expand Down Expand Up @@ -609,19 +611,19 @@ protected function getCanonicalUrl(string $contextPath, string $canonicalUrlPage
/**
* Set assoc type and IDs from the passed page, operation and arguments.
*/
protected function setAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setAssoc(int $assocType, array $args, array &$newEntry): void
{
$application = Application::get();
$applicationName = $application->getName();
switch ($applicationName) {
case 'ojs2':
$this->setOJSAssoc($assocType, $page, $op, $args, $newEntry);
$this->setOJSAssoc($assocType, $args, $newEntry);
break;
case 'omp':
$this->setOMPAssoc($assocType, $page, $op, $args, $newEntry);
$this->setOMPAssoc($assocType, $args, $newEntry);
break;
case 'ops':
$this->setOPSAssoc($assocType, $page, $op, $args, $newEntry);
$this->setOPSAssoc($assocType, $args, $newEntry);
break;
default:
throw new Exception('Unrecognized application name!');
Expand All @@ -632,7 +634,7 @@ protected function setAssoc(int $assocType, string $page, string $op, array $arg
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OJS.
*/
protected function setOJSAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOJSAssoc(int $assocType, array $args, array &$newEntry): void
{
switch ($assocType) {
case Application::getContextAssocType():
Expand Down Expand Up @@ -875,7 +877,7 @@ protected function setOJSAssoc(int $assocType, string $page, string $op, array $
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OMP.
*/
protected function setOMPAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOMPAssoc(int $assocType, array $args, array &$newEntry): void
{
switch ($assocType) {
case Application::getContextAssocType():
Expand Down Expand Up @@ -1028,7 +1030,7 @@ protected function setOMPAssoc(int $assocType, string $page, string $op, array $
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OPS.
*/
protected function setOPSAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOPSAssoc(int $assocType, array $args, array &$newEntry): void
{
switch ($assocType) {
case Application::getContextAssocType():
Expand Down
58 changes: 27 additions & 31 deletions tools/convertApacheAccessLogFile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public function splitFileByDay(string $filePath): array

// Get all days between the first and the last date, including the last date
$firstDate->setTime(0, 0, 0);
$firstDate->setTime(0, 0, 1);
$lastDate->setTime(0, 0, 1);
$period = new DatePeriod(
$firstDate,
new DateInterval('P1D'),
Expand Down Expand Up @@ -410,34 +410,19 @@ protected function getExpectedPageAndOp(): array
return $pageAndOp;
}

/**
* Set assoc type and IDs from the passed page, operation and arguments.
*/
protected function setAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
{
$application = Application::get();
$applicationName = $application->getName();
switch ($applicationName) {
case 'ojs2':
$this->setOJSAssoc($assocType, $page, $op, $args, $newEntry);
break;
case 'omp':
$this->setOMPAssoc($assocType, $page, $op, $args, $newEntry);
break;
case 'ops':
$this->setOPSAssoc($assocType, $page, $op, $args, $newEntry);
break;
default:
throw new Exception('Unrecognized application name!');
}
}

/**
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OJS.
*/
protected function setOJSAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOJSAssoc(int $assocType, array $args, array &$newEntry): void
{
$newEntry['submissionId'] = null;
$newEntry['representationId'] = null;
$newEntry['submissionFileId'] = null;
$newEntry['fileType'] = null;
$newEntry['issueId'] = null;
$newEntry['issueGalleyId'] = null;

switch ($assocType) {
case Application::getContextAssocType():
// $newEntry['contextId'] has already been set
Expand Down Expand Up @@ -657,7 +642,7 @@ protected function setOJSAssoc(int $assocType, string $page, string $op, array $
}

// is this a full text or supp file
$genreDao = DAORegistry::getDAO('GenreDAO');
$genreDao = DAORegistry::getDAO('GenreDAO'); /** @var GenreDAO $genreDao */
$genre = $genreDao->getById($submissionFile->getData('genreId'));
if ($genre->getCategory() != Genre::GENRE_CATEGORY_DOCUMENT || $genre->getSupplementary() || $genre->getDependent()) {
$newEntry['assocType'] = Application::ASSOC_TYPE_SUBMISSION_FILE_COUNTER_OTHER;
Expand Down Expand Up @@ -797,14 +782,20 @@ protected function setOJSAssoc(int $assocType, string $page, string $op, array $
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OMP.
*/
protected function setOMPAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOMPAssoc(int $assocType, array $args, array &$newEntry): void
{
$newEntry['submissionId'] = null;
$newEntry['representationId'] = null;
$newEntry['submissionFileId'] = null;
$newEntry['fileType'] = null;
$newEntry['chapterId'] = null;
$newEntry['seriesId'] = null;

switch ($assocType) {
case Application::getContextAssocType():
// $newEntry['contextId'] has already been set
$newEntry['assocType'] = $assocType;
$newEntry['canonicalUrlPage'] = $page;
$newEntry['canonicalUrlOp'] = $page == 'catalog' ? 'index' : '';
$newEntry['canonicalUrlOp'] = $newEntry['canonicalUrlPage'] == 'catalog' ? 'index' : '';
break;

case Application::ASSOC_TYPE_SUBMISSION:
Expand Down Expand Up @@ -979,7 +970,7 @@ protected function setOMPAssoc(int $assocType, string $page, string $op, array $
}

// is this a full text or supp file
$genreDao = DAORegistry::getDAO('GenreDAO');
$genreDao = DAORegistry::getDAO('GenreDAO'); /** @var GenreDAO $genreDao */
$genre = $genreDao->getById($submissionFile->getData('genreId'));
if ($genre->getCategory() != Genre::GENRE_CATEGORY_DOCUMENT || $genre->getSupplementary() || $genre->getDependent()) {
$newEntry['assocType'] = Application::ASSOC_TYPE_SUBMISSION_FILE_COUNTER_OTHER;
Expand Down Expand Up @@ -1022,8 +1013,13 @@ protected function setOMPAssoc(int $assocType, string $page, string $op, array $
* Set assoc type and IDs from the passed page, operation and
* arguments specific to OPS.
*/
protected function setOPSAssoc(int $assocType, string $page, string $op, array $args, array &$newEntry): void
protected function setOPSAssoc(int $assocType, array $args, array &$newEntry): void
{
$newEntry['submissionId'] = null;
$newEntry['representationId'] = null;
$newEntry['submissionFileId'] = null;
$newEntry['fileType'] = null;

switch ($assocType) {
case Application::getContextAssocType():
// $newEntry['contextId'] has already been set
Expand Down Expand Up @@ -1170,7 +1166,7 @@ protected function setOPSAssoc(int $assocType, string $page, string $op, array $
}

// is this a full text or supp file
$genreDao = DAORegistry::getDAO('GenreDAO');
$genreDao = DAORegistry::getDAO('GenreDAO'); /** @var GenreDao $genreDao */
$genre = $genreDao->getById($submissionFile->getData('genreId'));
if ($genre->getCategory() != Genre::GENRE_CATEGORY_DOCUMENT || $genre->getSupplementary() || $genre->getDependent()) {
$newEntry['assocType'] = Application::ASSOC_TYPE_SUBMISSION_FILE_COUNTER_OTHER;
Expand Down

0 comments on commit ffcb8c3

Please sign in to comment.