Skip to content

Commit

Permalink
Fix error: Direct use of $GLOBALS Superglobal detected
Browse files Browse the repository at this point in the history
  • Loading branch information
beatrycze-volk committed May 28, 2024
1 parent 7a799a5 commit 34b3f99
Show file tree
Hide file tree
Showing 13 changed files with 54 additions and 35 deletions.
2 changes: 1 addition & 1 deletion Classes/Command/BaseCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ protected function getParentDocumentUidForSaving(Document $document): int

if ($doc !== null && !empty($doc->parentHref)) {
// find document object by record_id of parent
$parent = AbstractDocument::getInstance($doc->parentHref, ['storagePid' => $this->storagePid]);
$parent = AbstractDocument::getInstance($doc->parentHref, 0, ['storagePid' => $this->storagePid]);

if ($parent->recordId) {
$parentDocument = $this->documentRepository->findOneByRecordId($parent->recordId);
Expand Down
2 changes: 1 addition & 1 deletion Classes/Command/HarvestCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$docLocation = $baseLocation . http_build_query($params);
// ...index the document...
$document = null;
$doc = AbstractDocument::getInstance($docLocation, ['storagePid' => $this->storagePid], true);
$doc = AbstractDocument::getInstance($docLocation, 0, ['storagePid' => $this->storagePid], true);

if ($doc === null) {
$io->warning('WARNING: Document "' . $docLocation . '" could not be loaded. Skip to next document.');
Expand Down
4 changes: 2 additions & 2 deletions Classes/Command/IndexCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$io->error('ERROR: Document with UID "' . $input->getOption('doc') . '" could not be found on PID ' . $this->storagePid . ' .');
exit(1);
} else {
$doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $this->storagePid], true);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $this->storagePid], true);
}

} else if (GeneralUtility::isValidUrl($input->getOption('doc'))) {
$doc = AbstractDocument::getInstance($input->getOption('doc'), ['storagePid' => $this->storagePid], true);
$doc = AbstractDocument::getInstance($input->getOption('doc'), 0, ['storagePid' => $this->storagePid], true);

$document = $this->getDocumentFromUrl($doc, $input->getOption('doc'));
}
Expand Down
2 changes: 1 addition & 1 deletion Classes/Command/ReindexCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

foreach ($documents as $id => $document) {
$doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $this->storagePid], true);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $this->storagePid], true);

if ($doc === null) {
$io->warning('WARNING: Document "' . $document->getLocation() . '" could not be loaded. Skip to next document.');
Expand Down
21 changes: 15 additions & 6 deletions Classes/Common/AbstractDocument.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ abstract class AbstractDocument
*/
protected int $cPid = 0;

/**
* @access protected
* @var int This holds the page ID for the requests
*/
protected int $pageId = 0;

/**
* @access public
* @static
Expand Down Expand Up @@ -544,12 +550,13 @@ abstract public function getAllFiles(): array;
* @static
*
* @param string $location The URL of XML file or the IRI of the IIIF resource
* @param int $pageId
* @param array $settings
* @param bool $forceReload Force reloading the document instead of returning the cached instance
*
* @return AbstractDocument|null Instance of this class, either MetsDocument or IiifManifest
*/
public static function &getInstance(string $location, array $settings = [], bool $forceReload = false)
public static function &getInstance(string $location, int $pageId = 0, array $settings = [], bool $forceReload = false)
{
// Create new instance depending on format (METS or IIIF) ...
$documentFormat = null;
Expand Down Expand Up @@ -600,11 +607,11 @@ public static function &getInstance(string $location, array $settings = [], bool
// Sanitize input.
$pid = max((int) $settings['storagePid'], 0);
if ($documentFormat == 'METS') {
$instance = new MetsDocument($pid, $location, $xml, $settings);
$instance = new MetsDocument($pid, $location, $pageId, $xml, $settings);
} elseif ($documentFormat == 'IIIF') {
// TODO: Parameter $preloadedDocument of class Kitodo\Dlf\Common\IiifManifest constructor expects SimpleXMLElement|Ubl\Iiif\Presentation\Common\Model\Resources\IiifResourceInterface, Ubl\Iiif\Presentation\Common\Model\AbstractIiifEntity|null given.
// @phpstan-ignore-next-line
$instance = new IiifManifest($pid, $location, $iiif);
$instance = new IiifManifest($pid, $location, $pageId, $iiif);
}

if ($instance) {
Expand Down Expand Up @@ -1164,7 +1171,7 @@ protected function magicGetRootId(): int
if ($this->parentId) {
// TODO: Parameter $location of static method AbstractDocument::getInstance() expects string, int<min, -1>|int<1, max> given.
// @phpstan-ignore-next-line
$parent = self::getInstance($this->parentId, ['storagePid' => $this->pid]);
$parent = self::getInstance($this->parentId, $this->pageId, ['storagePid' => $this->pid]);
$this->rootId = $parent->rootId;
}
$this->rootIdLoaded = true;
Expand Down Expand Up @@ -1212,14 +1219,16 @@ protected function _setCPid(int $value): void
*
* @param int $pid If > 0, then only document with this PID gets loaded
* @param string $location The location URL of the XML file to parse
* @param int $pageId
* @param \SimpleXMLElement|IiifResourceInterface $preloadedDocument Either null or the \SimpleXMLElement
* or IiifResourceInterface that has been loaded to determine the basic document format.
*
* @return void
*/
protected function __construct(int $pid, string $location, $preloadedDocument, array $settings = [])
protected function __construct(int $pid, string $location, int $pageId, $preloadedDocument, array $settings = [])
{
$this->pid = $pid;
$this->pageId = $pageId;
$this->setPreloadedDocument($preloadedDocument);
$this->init($location, $settings);
$this->establishRecordId($pid);
Expand Down Expand Up @@ -1415,7 +1424,7 @@ public function toArray($uriBuilder, array $config = [])
// Configure @action URL for form.
$file['url'] = $uriBuilder
->reset()
->setTargetPageUid($GLOBALS['TSFE']->id)
->setTargetPageUid($this->pageId)
->setCreateAbsoluteUri($forceAbsoluteUrl)
->setArguments(
[
Expand Down
2 changes: 1 addition & 1 deletion Classes/Common/Indexer.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public static function add(Document $document, DocumentRepository $documentRepos
$parent = $documentRepository->findByUid($parentId);
if ($parent) {
// get XML document of parent
$doc = AbstractDocument::getInstance($parent->getLocation(), ['storagePid' => $parent->getPid()], true);
$doc = AbstractDocument::getInstance($parent->getLocation(), 0, ['storagePid' => $parent->getPid()], true);
if ($doc !== null) {
$parent->setCurrentDocument($doc);
$success = self::add($parent, $documentRepository);
Expand Down
36 changes: 23 additions & 13 deletions Classes/Controller/AbstractController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Kitodo\Dlf\Common\Helper;
use Kitodo\Dlf\Domain\Model\Document;
use Kitodo\Dlf\Domain\Repository\DocumentRepository;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
Expand Down Expand Up @@ -121,15 +122,17 @@ protected function loadDocument(int $documentId = 0): void
$documentId = $this->requestData['id'];
}

$pageId = $this->getRequest()->getAttribute('routing')->getPageId();

// Try to get document format from database
if (!empty($documentId)) {

$doc = null;

if (MathUtility::canBeInterpretedAsInteger($documentId)) {
$doc = $this->getDocumentByUid($documentId);
$doc = $this->getDocumentByUid($documentId, $pageId);
} elseif (GeneralUtility::isValidUrl($documentId)) {
$doc = $this->getDocumentByUrl($documentId);
$doc = $this->getDocumentByUrl($documentId, $pageId);
}

if ($this->document !== null && $doc !== null) {
Expand All @@ -141,7 +144,7 @@ protected function loadDocument(int $documentId = 0): void
$this->document = $this->documentRepository->findOneByRecordId($this->requestData['recordId']);

if ($this->document !== null) {
$doc = AbstractDocument::getInstance($this->document->getLocation(), $this->settings, true);
$doc = AbstractDocument::getInstance($this->document->getLocation(), $pageId, $this->settings, true);
if ($doc !== null) {
$this->document->setCurrentDocument($doc);
} else {
Expand All @@ -165,7 +168,7 @@ protected function loadDocument(int $documentId = 0): void
protected function configureProxyUrl(string &$url): void
{
$this->uriBuilder->reset()
->setTargetPageUid($GLOBALS['TSFE']->id)
->setTargetPageUid($this->getRequest()->getAttribute('routing')->getPageId())
->setCreateAbsoluteUri(!empty($this->settings['forceAbsoluteUrl']))
->setArguments(
[
Expand Down Expand Up @@ -434,22 +437,28 @@ protected function buildSimplePagination(PaginationInterface $pagination, Pagina
];
}

protected function getRequest(): ServerRequestInterface
{
return $GLOBALS['TYPO3_REQUEST'];

Check failure on line 442 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L442

Direct use of $GLOBALS Superglobal detected.

Check failure on line 442 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L442

Direct use of $GLOBALS Superglobal detected.
}

/**
* Get document from repository by uid.
*
* @access private
*
* @param int $documentId The document's UID
* @param int $pageId
*
* @return AbstractDocument
*/
private function getDocumentByUid(int $documentId)
private function getDocumentByUid(int $documentId, int $pageId)
{
$doc = null;
// find document from repository by uid
$this->document = $this->documentRepository->findOneByIdAndSettings($documentId);

if ($this->document) {
$doc = AbstractDocument::getInstance($this->document->getLocation(), $this->settings, true);
$doc = AbstractDocument::getInstance($this->document->getLocation(), $pageId, $this->settings, true);
} else {
$this->logger->error('Invalid UID "' . $documentId . '" or PID "' . $this->settings['storagePid'] . '" for document loading');
}
Expand All @@ -463,12 +472,13 @@ private function getDocumentByUid(int $documentId)
* @access private
*
* @param string $documentId The document's URL
* @param int $pageId
*
* @return AbstractDocument
*/
private function getDocumentByUrl(string $documentId)
private function getDocumentByUrl(string $documentId, int $pageId)
{
$doc = AbstractDocument::getInstance($documentId, $this->settings, true);
$doc = AbstractDocument::getInstance($documentId, $pageId, $this->settings, true);

if ($doc !== null) {
if ($doc->recordId) {
Expand All @@ -484,13 +494,13 @@ private function getDocumentByUrl(string $documentId)

// Make sure configuration PID is set when applicable
if ($doc->cPid == 0) {
$doc->cPid = max((int) $this->settings['storagePid'], 0);
$doc->cPid = max(intval($this->settings['storagePid']), 0);

Check warning on line 497 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L497

The use of function intval() is discouraged; use (int) construction instead.
}

$this->document->setLocation($documentId);
} else {
$this->logger->error('Invalid location given "' . $documentId . '" for document loading');
}
} else {

Check notice on line 501 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L501

Closing brace indented incorrectly; expected 8 spaces, found 16

Check notice on line 501 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L501

Line indented incorrectly; expected 8 spaces, found 16
$this->logger->error('Invalid location given "' . $documentId . '" for document loading');
}

Check notice on line 503 in Classes/Controller/AbstractController.php

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Classes/Controller/AbstractController.php#L503

Line indented incorrectly; expected 4 spaces, found 5

return $doc;
}
Expand Down
2 changes: 1 addition & 1 deletion Classes/Controller/DocumentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ protected function getUrlTemplate()

$make = function ($page, $double, $pageGrid) {
$result = $this->uriBuilder->reset()
->setTargetPageUid($this->configurationManager->getContentObject()->data['pid'])
->setTargetPageUid($this->getRequest()->getAttribute('routing')->getPageId())
->setCreateAbsoluteUri(!empty($this->settings['forceAbsoluteUrl']) ? true : false)
->setArguments(
[
Expand Down
4 changes: 2 additions & 2 deletions Classes/Domain/Repository/DocumentRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public function findOneByParameters($parameters)

} else if (isset($parameters['location']) && GeneralUtility::isValidUrl($parameters['location'])) {

$doc = AbstractDocument::getInstance($parameters['location'], [], true);
$doc = AbstractDocument::getInstance($parameters['location'], 0, [], true);

if ($doc->recordId) {
$document = $this->findOneByRecordId($doc->recordId);
Expand All @@ -91,7 +91,7 @@ public function findOneByParameters($parameters)
}

if ($document !== null && $doc === null) {
$doc = AbstractDocument::getInstance($document->getLocation(), [], true);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, [], true);
}

if ($doc !== null) {
Expand Down
6 changes: 3 additions & 3 deletions Classes/ExpressionLanguage/DocumentTypeFunctionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,13 @@ protected function loadDocument(array $requestData, int $pid): void
// find document from repository by uid
$this->document = $this->documentRepository->findOneByIdAndSettings((int) $requestData['id'], ['storagePid' => $pid]);
if ($this->document) {
$doc = AbstractDocument::getInstance($this->document->getLocation(), ['storagePid' => $pid], true);
$doc = AbstractDocument::getInstance($this->document->getLocation(), 0, ['storagePid' => $pid], true);
} else {
$this->logger->error('Invalid UID "' . $requestData['id'] . '" or PID "' . $pid . '" for document loading');
}
} else if (GeneralUtility::isValidUrl($requestData['id'])) {

$doc = AbstractDocument::getInstance($requestData['id'], ['storagePid' => $pid], true);
$doc = AbstractDocument::getInstance($requestData['id'], 0, ['storagePid' => $pid], true);

if ($doc !== null) {
if ($doc->recordId) {
Expand All @@ -212,7 +212,7 @@ protected function loadDocument(array $requestData, int $pid): void
$this->document = $this->documentRepository->findOneByRecordId($requestData['recordId']);

if ($this->document !== null) {
$doc = AbstractDocument::getInstance($this->document->getLocation(), ['storagePid' => $pid], true);
$doc = AbstractDocument::getInstance($this->document->getLocation(), 0, ['storagePid' => $pid], true);
if ($doc !== null) {
$this->document->setCurrentDocument($doc);
} else {
Expand Down
4 changes: 2 additions & 2 deletions Classes/Hooks/DataHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public function processDatamap_afterDatabaseOperations(string $status, string $t
} else {
// Reindex document.
$document = $this->getDocumentRepository()->findByUid((int) $id);
$doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $document->getPid()], true);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $document->getPid()], true);
if ($document !== null && $doc !== null) {
$document->setCurrentDocument($doc);
Indexer::add($document, $this->getDocumentRepository());
Expand Down Expand Up @@ -340,7 +340,7 @@ public function processCmdmap_postProcess(string $command, string $table, $id):
case 'undelete':
// Reindex document.
$document = $this->getDocumentRepository()->findByUid((int) $id);
$doc = AbstractDocument::getInstance($document->getLocation(), ['storagePid' => $document->getPid()], true);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, ['storagePid' => $document->getPid()], true);
if ($document !== null && $doc !== null) {
$document->setCurrentDocument($doc);
Indexer::add($document, $this->getDocumentRepository());
Expand Down
2 changes: 1 addition & 1 deletion Tests/Functional/Common/MetsDocumentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function setUp(): void
protected function doc(string $file)
{
$url = 'http://web:8001/Tests/Fixtures/MetsDocument/' . $file;
$doc = AbstractDocument::getInstance($url, ['useExternalApisForMetadata' => 0]);
$doc = AbstractDocument::getInstance($url, 0, ['useExternalApisForMetadata' => 0]);
self::assertNotNull($doc);
return $doc;
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/Functional/Common/SolrIndexingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function canIndexAndSearchDocument()
$document->setSolrcore($core->model->getUid());
$this->persistenceManager->persistAll();

$doc = AbstractDocument::getInstance($document->getLocation(), ['useExternalApisForMetadata' => 0]);
$doc = AbstractDocument::getInstance($document->getLocation(), 0, ['useExternalApisForMetadata' => 0]);
$document->setCurrentDocument($doc);

$indexingSuccessful = Indexer::add($document, $this->documentRepository);
Expand Down

0 comments on commit 34b3f99

Please sign in to comment.