From cabf2661404f21c601e0a9878ffef614daa3ef44 Mon Sep 17 00:00:00 2001 From: Bozana Bokan Date: Tue, 13 Aug 2024 16:45:52 +0200 Subject: [PATCH] pkp/pkp-lib#9911 changes ofter the code review --- classes/cliTool/traits/ConvertLogFile.php | 22 +++++++------ tools/convertApacheAccessLogFile.php | 39 +++++------------------ 2 files changed, 20 insertions(+), 41 deletions(-) diff --git a/classes/cliTool/traits/ConvertLogFile.php b/classes/cliTool/traits/ConvertLogFile.php index 84a21fcc450..3c80e61a514 100644 --- a/classes/cliTool/traits/ConvertLogFile.php +++ b/classes/cliTool/traits/ConvertLogFile.php @@ -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); @@ -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; } @@ -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!'); @@ -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(): @@ -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(): @@ -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(): diff --git a/tools/convertApacheAccessLogFile.php b/tools/convertApacheAccessLogFile.php index 83982ad9547..a0ba2ff5f00 100644 --- a/tools/convertApacheAccessLogFile.php +++ b/tools/convertApacheAccessLogFile.php @@ -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'), @@ -410,33 +410,11 @@ 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 { switch ($assocType) { case Application::getContextAssocType(): @@ -657,7 +635,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; @@ -797,14 +775,13 @@ 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(): // $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: @@ -979,7 +956,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; @@ -1022,7 +999,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(): @@ -1170,7 +1147,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;