From 136e8d1e02d6320db2959e55692d410a2a1b3662 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 24 Oct 2022 10:14:47 +0200 Subject: [PATCH 01/46] #143 Move Opus\Job namespace to opus4-job --- composer.json | 2 +- library/Opus/Job/Runner.php | 204 ----------------- library/Opus/Job/Worker/AbstractWorker.php | 68 ------ .../Opus/Job/Worker/InvalidJobException.php | 48 ---- library/Opus/Job/Worker/MailNotification.php | 211 ------------------ library/Opus/Job/Worker/WorkerInterface.php | 54 ----- tests/Opus/Job/RunnerTest.php | 110 --------- .../Opus/Job/Worker/MailNotificationTest.php | 107 --------- 8 files changed, 1 insertion(+), 803 deletions(-) delete mode 100644 library/Opus/Job/Runner.php delete mode 100644 library/Opus/Job/Worker/AbstractWorker.php delete mode 100644 library/Opus/Job/Worker/InvalidJobException.php delete mode 100644 library/Opus/Job/Worker/MailNotification.php delete mode 100644 library/Opus/Job/Worker/WorkerInterface.php delete mode 100644 tests/Opus/Job/RunnerTest.php delete mode 100644 tests/Opus/Job/Worker/MailNotificationTest.php diff --git a/composer.json b/composer.json index 15bd3184..719450ea 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "ext-json": "*", "ext-libxml": "*", "zendframework/zendframework1": "1.12.*", - "opus4-repo/opus4-common": "dev-master", + "opus4-repo/opus4-common": "4.7.2.x-dev", "opus4-repo/opus4-doi": "dev-master" }, "minimum-stability": "dev", diff --git a/library/Opus/Job/Runner.php b/library/Opus/Job/Runner.php deleted file mode 100644 index e02766b3..00000000 --- a/library/Opus/Job/Runner.php +++ /dev/null @@ -1,204 +0,0 @@ -workers[$worker->getActivationLabel()] = $worker; - } - - /** - * Set the current logger instance. - * - * @param Zend_Log $logger Logger. - * @return $this Fluent interface. - */ - public function setLogger($logger) - { - $this->logger = $logger; - return $this; - } - - /** - * Set the worker delay time in seconds. - * - * @param int $seconds Pause in seconds before the next worker runs. - */ - public function setDelay($seconds) - { - $this->delay = (int) $seconds; - if (null !== $this->logger) { - $this->logger->info('Set worker delay to ' . $seconds . 's'); - } - } - - /** - * Set a limit to number of executing jobs at a run. - * - * @param null|int $limit Limit for jobs. - */ - public function setLimit($limit = null) - { - if ((null !== $limit) && (true === is_int($limit))) { - $this->logger->info('Set job limit to ' . $limit . ' jobs / run.'); - $this->limit = $limit; - } - } - - /** - * Run scheduling of jobs. All jobs currently in the queue get - * processed and any new jobs get created in the jobs table. - */ - public function run() - { - $jobs = Job::getByLabels(array_keys($this->workers), $this->limit, Job::STATE_UNDEFINED); - - if (null !== $this->logger) { - $this->logger->info('Found ' . count($jobs) . ' job(s)'); - } - - $runJobs = 0; - foreach ($jobs as $job) { - if (true === $this->consume($job)) { - $runJobs++; - } else { - if (null !== $this->logger) { - $this->logger->warn('Job with ID ' . $job->getId() . ' failed.'); - } - } - } - if (null !== $this->logger) { - $this->logger->info('Processed ' . $runJobs . ' job(s).'); - } - } - - /** - * Execute a job and remove it from the jobs table on success. - * - * @param JobInterface $job Job description model. - * @return bool Returns true if a job is consumend false if not - */ - protected function consume($job) - { - $label = $job->getLabel(); - - if ($job->getState() !== null) { - return false; - } - - if (array_key_exists($label, $this->workers)) { - $worker = $this->workers[$label]; - - if (null !== $this->logger) { - $this->logger->info('Processing ' . $label); - } - - $job->setState(Job::STATE_PROCESSING); - $job->store(); - - try { - $worker->setLogger($this->logger); - $worker->work($job); - $job->delete(); - sleep($this->delay); - } catch (Exception $ex) { - if (null !== $this->logger) { - $msg = get_class($worker) . ': ' . $ex->getMessage(); - $this->logger->err($msg); - } - $job->setErrors(json_encode([ - 'exception' => get_class($ex), - 'message' => $ex->getMessage(), - 'trace' => $ex->getTraceAsString(), - ])); - $job->setState(Job::STATE_FAILED); - $job->store(); - return false; - } - return true; - } - return false; - } -} diff --git a/library/Opus/Job/Worker/AbstractWorker.php b/library/Opus/Job/Worker/AbstractWorker.php deleted file mode 100644 index ce75c207..00000000 --- a/library/Opus/Job/Worker/AbstractWorker.php +++ /dev/null @@ -1,68 +0,0 @@ - - * @copyright Copyright (c) 2008-2019, OPUS 4 development team - * @license http://www.gnu.org/licenses/gpl.html General Public License - */ - -namespace Opus\Job\Worker; - -use InvalidArgumentException; -use Zend_Log; -use Zend_Log_Writer_Null; - -/** - * Abstract base class for Opus job worker classes. - */ -abstract class AbstractWorker implements WorkerInterface -{ - /** - * Hold current logger instance. - * - * @var Zend_Log - */ - protected $logger; - - /** - * Set logging facility. - * - * @param Zend_Log $logger Logger instance. - */ - public function setLogger($logger) - { - if (null === $logger) { - $this->logger = new Zend_Log(new Zend_Log_Writer_Null()); - } elseif ($logger instanceof Zend_Log) { - $this->logger = $logger; - } else { - throw new InvalidArgumentException('Zend_Log instance expected.'); - } - } -} diff --git a/library/Opus/Job/Worker/InvalidJobException.php b/library/Opus/Job/Worker/InvalidJobException.php deleted file mode 100644 index 855a28ed..00000000 --- a/library/Opus/Job/Worker/InvalidJobException.php +++ /dev/null @@ -1,48 +0,0 @@ - - */ - -namespace Opus\Job\Worker; - -use Exception; - -/** - * Exception type for Opus\Model when trying to access invalid id. - * - * @category Framework - * @package Opus\Model - */ -class InvalidJobException extends Exception -{ -} diff --git a/library/Opus/Job/Worker/MailNotification.php b/library/Opus/Job/Worker/MailNotification.php deleted file mode 100644 index eb4c414c..00000000 --- a/library/Opus/Job/Worker/MailNotification.php +++ /dev/null @@ -1,211 +0,0 @@ -setLogger($logger); - $this->config = Config::get(); - $this->lookupRecipients = $lookupRecipients; - } - - /** - * Return message label that is used to trigger worker process. - * - * @return string Message label. - */ - public function getActivationLabel() - { - return self::LABEL; - } - - /** - * Perfom work. - * - * @param JobInterface $job Job description and attached data. - * @return array Array of Jobs to be newly created. - */ - public function work($job) - { - $data = $job->getData(true); - $message = $data['message']; - $subject = $data['subject']; - $users = $data['users']; - - $from = $this->_getFrom(); - $fromName = $this->_getFromName(); - $replyTo = $this->_getReplyTo(); - $replyToName = $this->_getReplyToName(); - $returnPath = $this->_getReturnPath(); - - if ($users !== null && ! is_array($users)) { - $users = [$users]; - } - - $recipient = []; - if ($this->lookupRecipients) { - $this->logger->debug(self::class . ': Resolving mail addresses for users = {"' . implode('", "', $users) . '"}'); - $recipient = $this->getRecipients($users); - } else { - $recipient = $users; - } -// if (empty($recipient)) { -// $this->_logger->info(__CLASS__ . ': No recipients avaiable. Mail canceled.'); -// return true; -// } - - $mailSendMail = new SendMail(); - - $this->logger->info(self::class . ': Sending notification email...'); - $this->logger->debug(self::class . ': sender: ' . $from); - $mailSendMail->sendMail($from, $fromName, $subject, $message, $recipient, $replyTo, $replyToName, $returnPath); - - return true; - } - - /** - * Returns the 'from' address for notification. - * - * @return string - */ - protected function _getFrom() - { - if (isset($this->config->mail->opus->address)) { - return $this->config->mail->opus->address; - } - return 'not configured'; - } - - /** - * Returns the 'from name' for notification. - * - * @return string - */ - protected function _getFromName() - { - if (isset($this->config->mail->opus->name)) { - return $this->config->mail->opus->name; - } - return 'not configured'; - } - - protected function _getReplyTo() - { - if (isset($this->config->mail->opus->replyTo)) { - return $this->config->mail->opus->replyTo; - } - - return null; - } - - protected function _getReplyToName() - { - if (isset($this->config->mail->opus->replyToName)) { - return $this->config->mail->opus->replyToName; - } - - return null; - } - - protected function _getReturnPath() - { - if (isset($this->config->mail->opus->returnPath)) { - return $this->config->mail->opus->returnPath; - } - - return null; - } - - public function getRecipients($users = null) - { - if (! is_array($users)) { - $users = [$users]; - } - - $allRecipients = []; - foreach ($users as $user) { - try { - $account = Account::fetchAccountByLogin($user); - } catch (SecurityException $ex) { - $account = null; - } - - if ($account === null) { - $this->logger->warn(self::class . ": User '$user' does not exist... skipping mail."); - continue; - } - - $mail = $account->getEmail(); - if ($mail === null || trim($mail) === '') { - $this->logger->warn(self::class . ": No mail address for user '$user'... skipping mail."); - continue; - } - - $allRecipients[] = [ - 'name' => $account->getFirstName() . ' ' . $account->getLastName(), - 'address' => $mail, - ]; - } - - return $allRecipients; - } -} diff --git a/library/Opus/Job/Worker/WorkerInterface.php b/library/Opus/Job/Worker/WorkerInterface.php deleted file mode 100644 index 2756362d..00000000 --- a/library/Opus/Job/Worker/WorkerInterface.php +++ /dev/null @@ -1,54 +0,0 @@ -assertNotNull($runner, 'Simple initializing of Opus\Job\Runner failed.'); - } - - public function testRunIndexWorkerWithInvalidJob() - { - $this->markTestSkipped('Search related and needs to be moved to opus-search'); - - $document = new Document(); - $document->setServerState('published'); - $documentId = $document->store(); - - $job = new Job(); - $job->setLabel('opus-index-document'); - $job->setData([ - 'documentId' => $documentId, - 'task' => 'get-me-a-coffee', - ]); - $jobId = $job->store(); - - $indexWorker = new Job\Worker\IndexOpusDocument(); - - $runner = new Runner(); - $runner->registerWorker($indexWorker); - $runner->run(); - - $job = new Job($jobId); - $this->assertEquals(Job::STATE_FAILED, $job->getState()); - $error = $job->getErrors(); - $this->assertNotEquals('', $error, 'Expected error message from job.'); -// $job->delete(); - } - - public function testRunIndexWorkerWithValidJob() - { - $this->markTestSkipped('Search related and needs to be moved to opus-search'); - - $document = new Document(); - $document->setServerState('published'); - $documentId = $document->store(); - - $job = new Job(); - $job->setLabel('opus-index-document'); - $job->setData([ - 'documentId' => $documentId, - 'task' => 'index', - ]); - $jobId = $job->store(); - - $indexWorker = new Job\Worker\IndexOpusDocument(); - - $runner = new Runner(); - $runner->registerWorker($indexWorker); - $runner->run(); - $this->expectException(NotFoundException::class); - $job = new Job($jobId); - if ($job instanceof Job) { - $job->delete(); - } - } -} diff --git a/tests/Opus/Job/Worker/MailNotificationTest.php b/tests/Opus/Job/Worker/MailNotificationTest.php deleted file mode 100644 index d59fb85e..00000000 --- a/tests/Opus/Job/Worker/MailNotificationTest.php +++ /dev/null @@ -1,107 +0,0 @@ -clearTables(false, ['accounts']); - - $account = Account::new(); - $account->setLogin('admin') - ->setPassword('foobar-' . rand()) - ->store(); - - $account = Account::new(); - $account->setLogin('hasmail') - ->setPassword('foobar-' . rand()) - ->setEmail('has@mail.de') - ->store(); - } - - /** - * Tests getting recipients (from empty list) - */ - public function testGetRecipientsForEmptyList() - { - $mail = new MailNotification(); - $recipients = $mail->getRecipients(); - $this->assertNotNull($recipients); - $this->assertEquals(0, count($recipients)); - } - - /** - * Tests getting recipients (from invalid list) - */ - public function testGetRecipientsForInvalidUser() - { - $mail = new MailNotification(); - $users = ['doesnotexist']; - $recipients = $mail->getRecipients($users); - $this->assertNotNull($recipients); - $this->assertEquals(0, count($recipients)); - } - - /** - * Tests getting recipients (from existing user, without mail) - */ - public function testGetRecipientsForUserWithoutMail() - { - $mail = new MailNotification(); - $users = ['admin']; - $recipients = $mail->getRecipients($users); - $this->assertNotNull($recipients); - $this->assertEquals(0, count($recipients)); - } - - /** - * Tests getting recipients (from existing user, without mail) - */ - public function testGetRecipientsForUserWithMail() - { - $mail = new MailNotification(); - $users = ['hasmail']; - $recipients = $mail->getRecipients($users); - $this->assertNotNull($recipients); - $this->assertEquals(1, count($recipients)); - } -} From 872c72f8221c6bbd5f7373d7a1be687057586892 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 24 Oct 2022 16:45:04 +0200 Subject: [PATCH 02/46] #146 Moved SecurityException to Common Also moved RealmInterface --- library/Opus/Account.php | 2 +- library/Opus/Model/Filter.php | 2 +- library/Opus/Security/AuthAdapter.php | 46 ++++----- library/Opus/Security/AuthAdapter/Ldap.php | 42 ++++---- library/Opus/Security/IRealm.php | 108 -------------------- library/Opus/Security/Realm.php | 88 +++++++--------- library/Opus/Security/SecurityException.php | 48 --------- tests/Opus/AccountTest.php | 9 +- tests/Opus/DocumentFinderTest.php | 2 +- tests/Opus/Security/AccountTest.php | 6 +- tests/Opus/Security/AuthAdapterTest.php | 4 - tests/Opus/Security/RealmTest.php | 14 +-- 12 files changed, 87 insertions(+), 284 deletions(-) delete mode 100644 library/Opus/Security/IRealm.php delete mode 100644 library/Opus/Security/SecurityException.php diff --git a/library/Opus/Account.php b/library/Opus/Account.php index ab613913..086b385e 100644 --- a/library/Opus/Account.php +++ b/library/Opus/Account.php @@ -37,11 +37,11 @@ use Opus\Common\AccountRepositoryInterface; use Opus\Common\Log; use Opus\Common\Model\ModelException; +use Opus\Common\Security\SecurityException; use Opus\Common\UserRoleInterface; use Opus\Db\TableGateway; use Opus\Model\AbstractDb; use Opus\Model\Field; -use Opus\Security\SecurityException; use Zend_Db_Table_Abstract; use Zend_Db_Table_Row; use Zend_Validate; diff --git a/library/Opus/Model/Filter.php b/library/Opus/Model/Filter.php index b50b643d..c8862fbe 100644 --- a/library/Opus/Model/Filter.php +++ b/library/Opus/Model/Filter.php @@ -35,9 +35,9 @@ use InvalidArgumentException; use Opus\Common\Model\ModelException; use Opus\Common\Model\ModelInterface; +use Opus\Common\Security\SecurityException; use Opus\Model\Xml\StrategyInterface; use Opus\Model\Xml\Version1; -use Opus\Security\SecurityException; use function array_diff; use function array_intersect; diff --git a/library/Opus/Security/AuthAdapter.php b/library/Opus/Security/AuthAdapter.php index 9142da3d..0e76122d 100644 --- a/library/Opus/Security/AuthAdapter.php +++ b/library/Opus/Security/AuthAdapter.php @@ -25,18 +25,16 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2020, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus\Security - * @author Ralf Claussnitzer - * @author Thoralf Klein */ namespace Opus\Security; -use Opus\Account; +use Opus\Common\Account; +use Opus\Common\AccountInterface; +use Opus\Common\Log; +use Opus\Common\Security\SecurityException; use Zend_Auth_Adapter_Exception; use Zend_Auth_Adapter_Interface; use Zend_Auth_Result; @@ -45,8 +43,6 @@ /** * A simple authentication adapter using the Opus\Account mechanism. - * - * phpcs:disable */ class AuthAdapter implements Zend_Auth_Adapter_Interface { @@ -55,21 +51,21 @@ class AuthAdapter implements Zend_Auth_Adapter_Interface * * @var string */ - protected $_login; + protected $login; /** * Holds the password. * * @var string */ - protected $_password; + protected $password; /** * Holds an actual Opus\Account implementation. * - * @var Account + * @var AccountInterface */ - protected $_account; + protected $account; /** * Set the credential values for authentication. @@ -81,7 +77,7 @@ class AuthAdapter implements Zend_Auth_Adapter_Interface */ public function setCredentials($login, $password) { - if ((is_string($login) === false) or (is_string($password) === false)) { + if ((is_string($login) === false) || (is_string($password) === false)) { throw new Zend_Auth_Adapter_Exception('Credentials are not strings.'); } if (empty($login) === true) { @@ -90,8 +86,8 @@ public function setCredentials($login, $password) if (empty($password) === true) { throw new Zend_Auth_Adapter_Exception('No password given.'); } - $this->_login = $login; - $this->_password = $password; + $this->login = $login; + $this->password = $password; return $this; } @@ -105,36 +101,36 @@ public function authenticate() { // Try to get the account information try { - $account = new Account(null, null, $this->_login); + $account = Account::fetchAccountByLogin($this->login); } catch (SecurityException $ex) { return new Zend_Auth_Result( Zend_Auth_Result::FAILURE_IDENTITY_NOT_FOUND, - $this->_login, + $this->login, ['auth_error_invalid_credentials'] ); } // Check if password is correcct, but for old hashes. Neede for // migrating md5-hashed passwords to SHA1-hashes. - if ($account->isPasswordCorrectOldHash($this->_password) === true) { - Log::get()->warn('Migrating old password-hash for user: ' . $this->_login); - $account->setPassword($this->_password)->store(); - $account = new Account(null, null, $this->_login); + if ($account->isPasswordCorrectOldHash($this->password) === true) { + Log::get()->warn('Migrating old password-hash for user: ' . $this->login); + $account->setPassword($this->password)->store(); + $account = Account::fetchAccountByLogin($this->login); } // Check the password - $pass = $account->isPasswordCorrect($this->_password); + $pass = $account->isPasswordCorrect($this->password); if ($pass === true) { return new Zend_Auth_Result( Zend_Auth_Result::SUCCESS, - $this->_login, + $this->login, ['auth_login_success'] ); } return new Zend_Auth_Result( Zend_Auth_Result::FAILURE_CREDENTIAL_INVALID, - $this->_login, + $this->login, ['auth_error_invalid_credentials'] ); } diff --git a/library/Opus/Security/AuthAdapter/Ldap.php b/library/Opus/Security/AuthAdapter/Ldap.php index 8b072a1d..c47082f5 100644 --- a/library/Opus/Security/AuthAdapter/Ldap.php +++ b/library/Opus/Security/AuthAdapter/Ldap.php @@ -27,16 +27,12 @@ * * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus\Security - * @author Oliver Marahrens */ namespace Opus\Security\AuthAdapter; use Exception; -use Opus\Account; +use Opus\Common\Account; use Opus\Security\AuthAdapter; use Zend_Auth; use Zend_Auth_Adapter_Exception; @@ -57,8 +53,6 @@ /** * A simple authentication adapter for LDAP using the Opus\Account mechanism. - * - * phpcs:disable */ class Ldap extends AuthAdapter { @@ -72,8 +66,8 @@ public function authenticate() { $config = new Zend_Config_Ini('../application/configs/config.ini', 'production'); - $log_path = $config->ldap->log_path; - $admins = explode(',', $config->ldap->admin_accounts); + $logPath = $config->ldap->log_path; + $admins = explode(',', $config->ldap->admin_accounts); $options = $config->ldap->toArray(); @@ -84,7 +78,7 @@ public function authenticate() // first check local DB with parent class $result = parent::authenticate(); $user = new Zend_Session_Namespace('loggedin'); - $user->usernumber = $this->_login; + $user->usernumber = $this->login; } catch (Exception $e) { throw $e; } @@ -92,16 +86,16 @@ public function authenticate() try { $auth = Zend_Auth::getInstance(); - $adapter = new Zend_Auth_Adapter_Ldap($options, $this->_login, $this->_password); + $adapter = new Zend_Auth_Adapter_Ldap($options, $this->login, $this->password); $result = $auth->authenticate($adapter); // log the result if a log path has been defined in config.ini - if ($log_path) { + if ($logPath) { $messages = $result->getMessages(); $logger = new Zend_Log(); - $logger->addWriter(new Zend_Log_Writer_Stream($log_path)); + $logger->addWriter(new Zend_Log_Writer_Stream($logPath)); $filter = new Zend_Log_Filter_Priority(Zend_Log::DEBUG); $logger->addFilter($filter); @@ -116,14 +110,14 @@ public function authenticate() // if authentication was successfull and user is not already in OPUS DB // register user as publisher to OPUS database try { - $account = new Account(null, null, $this->_login); + $account = Account::fetchAccountByLogin($this->login); } catch (Exception $ex) { if ($result->isValid() === true) { $user = new Zend_Session_Namespace('loggedin'); - $user->usernumber = $this->_login; + $user->usernumber = $this->login; $account = new Account(); - $account->setLogin($this->_login); - $account->setPassword($this->_password); + $account->setLogin($this->login); + $account->setPassword($this->password); $account->store(); $roles = Opus_Role::getAll(); // look for the publisher role in OPUS DB @@ -159,7 +153,7 @@ public function authenticate() $adminRole->addPrivilege($adminprivilege); $adminRole->store(); } - if (in_array($this->_login, $admins) === true) { + if (in_array($this->login, $admins) === true) { $account->addRole($adminRole); } else { $account->addRole($accessRole); @@ -178,7 +172,7 @@ public function authenticate() /** * gets userdata from LDAP * - * @return array data of currently logged in user + * @return array|false data of currently logged in user */ public static function getUserdata() { @@ -193,7 +187,7 @@ public static function getUserdata() $config = new Zend_Config_Ini('../application/configs/config.ini', 'production'); - $log_path = $config->ldap->log_path; + $logPath = $config->ldap->log_path; $multiOptions = $config->ldap->toArray(); $mappingSettings = $config->ldapmappings->toArray(); @@ -205,7 +199,7 @@ public static function getUserdata() foreach ($multiOptions as $name => $options) { $mappingFirstName = $mappingSettings[$name]['firstName']; $mappingLastName = $mappingSettings[$name]['lastName']; - $mappingEMail = $mappingSettings[$name]['EMail']; + $mappingEmail = $mappingSettings[$name]['EMail']; $permanentId = $mappingSettings[$name]['personId']; $ldap->setOptions($options); @@ -227,10 +221,10 @@ public static function getUserdata() } else { $return['lastName'] = $searchresult[$mappingLastName]; } - if (is_array($searchresult[$mappingEMail]) === true) { - $return['email'] = $searchresult[$mappingEMail][0]; + if (is_array($searchresult[$mappingEmail]) === true) { + $return['email'] = $searchresult[$mappingEmail][0]; } else { - $return['email'] = $searchresult[$mappingEMail]; + $return['email'] = $searchresult[$mappingEmail]; } if (is_array($searchresult[$permanentId]) === true) { $return['personId'] = $searchresult[$permanentId][0]; diff --git a/library/Opus/Security/IRealm.php b/library/Opus/Security/IRealm.php deleted file mode 100644 index 89a5efb9..00000000 --- a/library/Opus/Security/IRealm.php +++ /dev/null @@ -1,108 +0,0 @@ - - */ - -namespace Opus\Security; - -/** - * Interface for classes providing security implementation. - * - * phpcs:disable - */ -interface IRealm -{ - /** - * Set the current username. - * - * @param string username username to be set. - * @throws SecurityException Thrown if the supplied identity could not be found. - * @return Realm Fluent interface. - */ - public function setUser($username); - - /** - * Set the current ip address. - * - * @param string ipaddress ip address to be set. - * @throws SecurityException Thrown if the supplied ip address is not a valid ip address. - * @return Realm Fluent interface. - */ - public function setIp($ipaddress); - - /** - * Checks, if the logged user is allowed to access (document_id). - * - * @param null|string $document_id ID of the document to check - * @return bool Returns true only if access is granted. - */ - public function checkDocument($document_id = null); - - /** - * Checks, if the logged user is allowed to access (file_id). - * - * @param null|string $file_id ID of the file to check - * @return bool Returns true only if access is granted. - */ - public function checkFile($file_id = null); - - /** - * Checks, if the logged user is allowed to access (module_name). - * - * @param null|string $module_name Name of the module to check - * @return bool Returns true only if access is granted. - */ - public function checkModule($module_name = null); - - /** - * Returns the roles of the current user. - * - * @return array of strings - Names of roles for current user - */ - public function getRoles(); - - /** - * Checks if a privilege is granted for actual context (usersession, - * ip address). - * - * If administrator is one of the current roles true will be returned - * ingoring everything else. - * - * @deprecated - */ - public function check( - $privilege, - $documentServerState = null, - $fileId = null - ); -} diff --git a/library/Opus/Security/Realm.php b/library/Opus/Security/Realm.php index fc49234c..8aee7dbd 100644 --- a/library/Opus/Security/Realm.php +++ b/library/Opus/Security/Realm.php @@ -25,21 +25,16 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2011, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus\Model - * @author Pascal-Nicolas Becker - * @author Thoralf Klein - * @author Felix Ostrowski (ostrowski@hbz-nrw.de) - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) */ namespace Opus\Security; use Opus\Common\Config; use Opus\Common\Log; +use Opus\Common\Security\RealmInterface; +use Opus\Common\Security\SecurityException; use Opus\Db\Accounts; use Opus\Db\TableGateway; use Opus\Db\UserRoles; @@ -60,10 +55,8 @@ * like the current User, IP address, and method to check rights. * * TODO NAMESPACE rename class? - * - * phpcs:disable */ -class Realm implements IRealm +class Realm implements RealmInterface { /** * The current user roles (merged userRoles and ipaddressRoles). @@ -92,7 +85,7 @@ class Realm implements IRealm /** * Set the current username. * - * @param string username username to be set. + * @param string $username username to be set. * @throws SecurityException Thrown if the supplied identity could not be found. * @return $this Fluent interface. */ @@ -110,7 +103,7 @@ public function setUser($username) /** * Set the current ip address. * - * @param string ipaddress ip address to be set. + * @param string $ipaddress ip address to be set. * @throws SecurityException Thrown if the supplied ip address is not a valid ip address. * @return $this Fluent interface. */ @@ -156,7 +149,7 @@ private function setRoles() /** * Get the roles that are assigned to the specified username. * - * @param string username username to be set. + * @param string $username username to be set. * @throws SecurityException Thrown if the supplied identity could not be found. * @return array Array of assigned roles or an empty array. */ @@ -191,7 +184,7 @@ private static function getUsernameRoles($username) /** * Map an IP address to Roles. * - * @param string ipaddress ip address to be set. + * @param string $ipaddress ip address to be set. * @throws SecurityException Thrown if the supplied ip is not valid. * @return array Array of assigned roles or an empty array. */ @@ -221,15 +214,13 @@ private static function getIpaddressRoles($ipaddress) * Returns all module resources to which the current user and ip address * has access. * - * @param $username name of the account to get resources for. - * Defaults to currently logged in user - * @param $ipaddress IP address to get resources for. - * Defaults to current remote address if available. - * @throws SecurityException Thrown if the supplied ip is not valid or - * user can not be determined - * @return array array of module resource names + * @param string|null $username name of the account to get resources for. + * Defaults to currently logged in user + * @param string|null $ipaddress IP address to get resources for. + * Defaults to current remote address if available. + * @return array Module resource names + * @throws SecurityException Thrown if the supplied ip is not valid or user can not be determined. */ - public static function getAllowedModuleResources($username = null, $ipaddress = null) { $resources = []; @@ -263,7 +254,7 @@ public static function getAllowedModuleResources($username = null, $ipaddress = /** * checks if the string provided is a valid ip address * - * @param string ipaddress ip address to validate. + * @param string $ipaddress ip address to validate. * @return bool Returns true if validation succeeded */ private static function validateIpAddress($ipaddress) @@ -278,16 +269,16 @@ private static function validateIpAddress($ipaddress) /** * Checks, if the logged user is allowed to access (document_id). * - * @param null|string $document_id ID of the document to check + * @param null|string $documentId ID of the document to check * @return bool Returns true only if access is granted. */ - public function checkDocument($document_id = null) + public function checkDocument($documentId = null) { if ($this->skipSecurityChecks()) { return true; } - if (empty($document_id)) { + if (empty($documentId)) { return false; } @@ -297,7 +288,7 @@ public function checkDocument($document_id = null) ->from(['ad' => 'access_documents'], ['document_id']) ->join(['r' => 'user_roles'], 'ad.role_id = r.id', '') ->where('r.name IN (?)', $this->roles) - ->where('ad.document_id = ?', $document_id) + ->where('ad.document_id = ?', $documentId) ); return 1 <= count($results) ? true : false; } @@ -305,16 +296,16 @@ public function checkDocument($document_id = null) /** * Checks, if the logged user is allowed to access (file_id). * - * @param null|string $file_id ID of the file to check + * @param null|string $fileId ID of the file to check * @return bool Returns true only if access is granted. */ - public function checkFile($file_id = null) + public function checkFile($fileId = null) { if ($this->skipSecurityChecks()) { return true; } - if (empty($file_id)) { + if (empty($fileId)) { return false; } @@ -324,7 +315,7 @@ public function checkFile($file_id = null) ->from(['af' => 'access_files'], ['file_id']) ->join(['r' => 'user_roles'], 'af.role_id = r.id', '') ->where('r.name IN (?)', $this->roles) - ->where('af.file_id = ?', $file_id) + ->where('af.file_id = ?', $fileId) ); return 1 <= count($results) ? true : false; } @@ -332,16 +323,16 @@ public function checkFile($file_id = null) /** * Checks, if the logged user is allowed to access (module_name). * - * @param null|string $module_name Name of the module to check + * @param null|string $moduleName Name of the module to check * @return bool Returns true only if access is granted. */ - public function checkModule($module_name = null) + public function checkModule($moduleName = null) { if ($this->skipSecurityChecks()) { return true; } - if (empty($module_name)) { + if (empty($moduleName)) { return false; } @@ -351,7 +342,7 @@ public function checkModule($module_name = null) ->from(['am' => 'access_modules'], ['module_name']) ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') ->where('r.name IN (?)', $this->roles) - ->where('am.module_name = ?', $module_name) + ->where('am.module_name = ?', $moduleName) ); return 1 <= count($results) ? true : false; } @@ -359,10 +350,11 @@ public function checkModule($module_name = null) /** * Checks if a user has access to a module. * - * @param $module_name Name of module - * @param $user Name of user + * @param string $moduleName Name of module + * @param string $user Name of user + * @return bool */ - public static function checkModuleForUser($module_name, $user) + public static function checkModuleForUser($moduleName, $user) { $roles = self::getUsernameRoles($user); @@ -372,7 +364,7 @@ public static function checkModuleForUser($module_name, $user) ->from(['am' => 'access_modules'], ['module_name']) ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') ->where('r.name IN (?)', $roles) - ->where('am.module_name = ?', $module_name) + ->where('am.module_name = ?', $moduleName) ); return 1 <= count($results) ? true : false; } @@ -412,6 +404,11 @@ public function getRoles() * If administrator is one of the current roles true will be returned ingoring everything else. * * @deprecated + * + * @param string $privilege + * @param string|null $documentServerState + * @param int|null $fileId + * @return bool */ public function check($privilege, $documentServerState = null, $fileId = null) { @@ -423,14 +420,14 @@ public function check($privilege, $documentServerState = null, $fileId = null) /** * Holds instance. * - * @var Realm. + * @var RealmInterface */ private static $instance; /** * Delivers the singleton instance. * - * @return Realm + * @return RealmInterface */ final public static function getInstance() { @@ -454,11 +451,4 @@ final private function __construct() final private function __clone() { } - - /** - * Singleton classes should not be put to sleep! - */ - final private function __sleep() - { - } } diff --git a/library/Opus/Security/SecurityException.php b/library/Opus/Security/SecurityException.php deleted file mode 100644 index 7eebf4a9..00000000 --- a/library/Opus/Security/SecurityException.php +++ /dev/null @@ -1,48 +0,0 @@ - - * @copyright Copyright (c) 2008-2020, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License */ namespace OpusTest; use Opus\Account; -use Opus\Security\SecurityException; +use Opus\Common\Security\SecurityException; use Opus\UserRole; use OpusTest\TestAsset\TestCase; diff --git a/tests/Opus/DocumentFinderTest.php b/tests/Opus/DocumentFinderTest.php index 1e301829..bf0e3dc4 100644 --- a/tests/Opus/DocumentFinderTest.php +++ b/tests/Opus/DocumentFinderTest.php @@ -36,13 +36,13 @@ use Opus\CollectionRole; use Opus\Common\Date; use Opus\Common\Model\ModelException; +use Opus\Common\Security\SecurityException; use Opus\Document; use Opus\DocumentFinder; use Opus\DocumentFinder\DefaultDocumentFinder; use Opus\File; use Opus\Licence; use Opus\Person; -use Opus\Security\SecurityException; use Opus\Title; use OpusTest\TestAsset\TestCase; diff --git a/tests/Opus/Security/AccountTest.php b/tests/Opus/Security/AccountTest.php index afe026a4..81000f0b 100644 --- a/tests/Opus/Security/AccountTest.php +++ b/tests/Opus/Security/AccountTest.php @@ -32,19 +32,15 @@ namespace OpusTest\Security; use Opus\Common\Account; +use Opus\Common\Security\SecurityException; use Opus\Db\Accounts; use Opus\Db\TableGateway; -use Opus\Security\SecurityException; use OpusTest\TestAsset\TestCase; use function sha1; /** * Test case for Opus\Account. - * - * @category Tests - * @package Opus\Security - * @group AccountTest */ class AccountTest extends TestCase { diff --git a/tests/Opus/Security/AuthAdapterTest.php b/tests/Opus/Security/AuthAdapterTest.php index c1e4b18a..6cdb88a9 100644 --- a/tests/Opus/Security/AuthAdapterTest.php +++ b/tests/Opus/Security/AuthAdapterTest.php @@ -38,10 +38,6 @@ /** * Test case for Opus\Security\AuthAdapter class. - * - * @category Tests - * @package Opus\Security - * @group AuthAdapterTest */ class AuthAdapterTest extends TestCase { diff --git a/tests/Opus/Security/RealmTest.php b/tests/Opus/Security/RealmTest.php index f57911d0..9d70ce52 100644 --- a/tests/Opus/Security/RealmTest.php +++ b/tests/Opus/Security/RealmTest.php @@ -25,19 +25,14 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2020, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Security - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) - * @author Thoralf Klein - * @author Jens Schwidder */ namespace OpusTest\Security; use Opus\Common\Config; +use Opus\Common\Security\SecurityException; use Opus\Db\AccessDocuments; use Opus\Db\AccessFiles; use Opus\Db\AccessModules; @@ -50,7 +45,6 @@ use Opus\Db\TableGateway; use Opus\Db\UserRoles; use Opus\Security\Realm; -use Opus\Security\SecurityException; use OpusTest\TestAsset\TestCase; use Zend_Config; @@ -60,10 +54,6 @@ /** * Test for Opus\Security\Realm. - * - * @package Opus\Security - * @category Tests - * @group RealmTest */ class RealmTest extends TestCase { From 7a41dc71b4c0c3f447cfdc3d8afe6a09cd84c537 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 26 Oct 2022 08:17:38 +0200 Subject: [PATCH 03/46] #146 Moved Security classes to opus4-security Fixed coding style --- library/Opus/Security/AuthAdapter.php | 137 ------- library/Opus/Security/AuthAdapter/Ldap.php | 246 ----------- library/Opus/Security/Realm.php | 454 --------------------- library/Opus/Security/RealmStorage.php | 239 +++++++++++ library/Opus/Statistic/LocalCounter.php | 87 ++-- tests/Opus/RequireTest.php | 8 +- tests/Opus/Security/AuthAdapterTest.php | 117 ------ tests/Opus/Security/RealmTest.php | 6 +- tests/test.ini | 1 + 9 files changed, 301 insertions(+), 994 deletions(-) delete mode 100644 library/Opus/Security/AuthAdapter.php delete mode 100644 library/Opus/Security/AuthAdapter/Ldap.php delete mode 100644 library/Opus/Security/Realm.php create mode 100644 library/Opus/Security/RealmStorage.php delete mode 100644 tests/Opus/Security/AuthAdapterTest.php diff --git a/library/Opus/Security/AuthAdapter.php b/library/Opus/Security/AuthAdapter.php deleted file mode 100644 index 0e76122d..00000000 --- a/library/Opus/Security/AuthAdapter.php +++ /dev/null @@ -1,137 +0,0 @@ -login = $login; - $this->password = $password; - return $this; - } - - /** - * Performs an authentication attempt - * - * @throws Zend_Auth_Adapter_Exception If authentication cannot be performed. - * @return Zend_Auth_Result - */ - public function authenticate() - { - // Try to get the account information - try { - $account = Account::fetchAccountByLogin($this->login); - } catch (SecurityException $ex) { - return new Zend_Auth_Result( - Zend_Auth_Result::FAILURE_IDENTITY_NOT_FOUND, - $this->login, - ['auth_error_invalid_credentials'] - ); - } - - // Check if password is correcct, but for old hashes. Neede for - // migrating md5-hashed passwords to SHA1-hashes. - if ($account->isPasswordCorrectOldHash($this->password) === true) { - Log::get()->warn('Migrating old password-hash for user: ' . $this->login); - $account->setPassword($this->password)->store(); - $account = Account::fetchAccountByLogin($this->login); - } - - // Check the password - $pass = $account->isPasswordCorrect($this->password); - if ($pass === true) { - return new Zend_Auth_Result( - Zend_Auth_Result::SUCCESS, - $this->login, - ['auth_login_success'] - ); - } - - return new Zend_Auth_Result( - Zend_Auth_Result::FAILURE_CREDENTIAL_INVALID, - $this->login, - ['auth_error_invalid_credentials'] - ); - } -} diff --git a/library/Opus/Security/AuthAdapter/Ldap.php b/library/Opus/Security/AuthAdapter/Ldap.php deleted file mode 100644 index c47082f5..00000000 --- a/library/Opus/Security/AuthAdapter/Ldap.php +++ /dev/null @@ -1,246 +0,0 @@ -ldap->log_path; - $admins = explode(',', $config->ldap->admin_accounts); - - $options = $config->ldap->toArray(); - - unset($options['log_path']); - unset($options['admin_accounts']); - - try { - // first check local DB with parent class - $result = parent::authenticate(); - $user = new Zend_Session_Namespace('loggedin'); - $user->usernumber = $this->login; - } catch (Exception $e) { - throw $e; - } - if ($result->isValid() !== true) { - try { - $auth = Zend_Auth::getInstance(); - - $adapter = new Zend_Auth_Adapter_Ldap($options, $this->login, $this->password); - - $result = $auth->authenticate($adapter); - - // log the result if a log path has been defined in config.ini - if ($logPath) { - $messages = $result->getMessages(); - - $logger = new Zend_Log(); - $logger->addWriter(new Zend_Log_Writer_Stream($logPath)); - $filter = new Zend_Log_Filter_Priority(Zend_Log::DEBUG); - $logger->addFilter($filter); - - foreach ($messages as $i => $message) { - if ($i-- > 1) { // $messages[2] and up are log messages - $message = str_replace("\n", "\n ", $message); - $logger->log("Ldap: $i: $message", Zend_Log::DEBUG); - } - } - } - - // if authentication was successfull and user is not already in OPUS DB - // register user as publisher to OPUS database - try { - $account = Account::fetchAccountByLogin($this->login); - } catch (Exception $ex) { - if ($result->isValid() === true) { - $user = new Zend_Session_Namespace('loggedin'); - $user->usernumber = $this->login; - $account = new Account(); - $account->setLogin($this->login); - $account->setPassword($this->password); - $account->store(); - $roles = Opus_Role::getAll(); - // look for the publisher role in OPUS DB - foreach ($roles as $role) { - if ($role->getDisplayName() === 'publisher') { - $publisherId = $role->getId(); - } - if ($role->getDisplayName() === 'administrator') { - $adminId = $role->getId(); - } - } - if ($publisherId > 0) { - $accessRole = new Opus_Role($publisherId); - } else { - // if there is no publisher role in DB, create it - $accessRole = new Opus_Role(); - $accessRole->setName('publisher'); - // the publisher role needs publish access! - $privilege = new Opus_Privilege(); - $privilege->setPrivilege('publish'); - $accessRole->addPrivilege($privilege); - $accessRole->store(); - } - if ($adminId > 0) { - $adminRole = new Opus_Role($adminId); - } else { - // if there is no publisher role in DB, create it - $adminRole = new Opus_Role(); - $adminRole->setName('administrator'); - // the publisher role needs publish access! - $adminprivilege = new Opus_Privilege(); - $adminprivilege->setPrivilege('administrate'); - $adminRole->addPrivilege($adminprivilege); - $adminRole->store(); - } - if (in_array($this->login, $admins) === true) { - $account->addRole($adminRole); - } else { - $account->addRole($accessRole); - } - $account->store(); - } - } - } catch (Zend_Auth_Adapter_Exception $e) { - throw $e; - } - } - - return $result; - } - - /** - * gets userdata from LDAP - * - * @return array|false data of currently logged in user - */ - public static function getUserdata() - { - // get usernumber from session - // if session has not been defined return false - $user = new Zend_Session_Namespace('loggedin'); - if (isset($user->usernumber) === false) { - return false; - } - - $return = []; - - $config = new Zend_Config_Ini('../application/configs/config.ini', 'production'); - - $logPath = $config->ldap->log_path; - $multiOptions = $config->ldap->toArray(); - $mappingSettings = $config->ldapmappings->toArray(); - - unset($multiOptions['log_path']); - unset($multiOptions['admin_accounts']); - - $ldap = new Zend_Ldap(); - - foreach ($multiOptions as $name => $options) { - $mappingFirstName = $mappingSettings[$name]['firstName']; - $mappingLastName = $mappingSettings[$name]['lastName']; - $mappingEmail = $mappingSettings[$name]['EMail']; - $permanentId = $mappingSettings[$name]['personId']; - - $ldap->setOptions($options); - try { - $ldap->bind(); - - $ldapsearch = $ldap->search('(uid=' . $user->usernumber . ')', 'dc=tub,dc=tu-harburg,dc=de', Zend_Ldap::SEARCH_SCOPE_ONE); - - if ($ldapsearch->count() > 0) { - $searchresult = $ldapsearch->getFirst(); - - if (is_array($searchresult[$mappingFirstName]) === true) { - $return['firstName'] = $searchresult[$mappingFirstName][0]; - } else { - $return['firstName'] = $searchresult[$mappingFirstName]; - } - if (is_array($searchresult[$mappingLastName]) === true) { - $return['lastName'] = $searchresult[$mappingLastName][0]; - } else { - $return['lastName'] = $searchresult[$mappingLastName]; - } - if (is_array($searchresult[$mappingEmail]) === true) { - $return['email'] = $searchresult[$mappingEmail][0]; - } else { - $return['email'] = $searchresult[$mappingEmail]; - } - if (is_array($searchresult[$permanentId]) === true) { - $return['personId'] = $searchresult[$permanentId][0]; - } else { - $return['personId'] = $searchresult[$permanentId]; - } - return $return; - } - } catch (Zend_Ldap_Exception $zle) { - echo ' ' . $zle->getMessage() . "\n"; - if ($zle->getCode() === Zend_Ldap_Exception::LDAP_X_DOMAIN_MISMATCH) { - continue; - } - } - } - - return $return; - } -} diff --git a/library/Opus/Security/Realm.php b/library/Opus/Security/Realm.php deleted file mode 100644 index 8aee7dbd..00000000 --- a/library/Opus/Security/Realm.php +++ /dev/null @@ -1,454 +0,0 @@ -userRoles = []; - $this->setRoles(); - - $this->userRoles = self::getUsernameRoles($username); - $this->setRoles(); - return $this; - } - - /** - * Set the current ip address. - * - * @param string $ipaddress ip address to be set. - * @throws SecurityException Thrown if the supplied ip address is not a valid ip address. - * @return $this Fluent interface. - */ - public function setIp($ipaddress) - { - $this->clientIp = null; - - // reset "old" credentials - $this->ipaddressRoles = []; - $this->setRoles(); - - $this->ipaddressRoles = self::getIpaddressRoles($ipaddress); - $this->setRoles(); - - $this->clientIp = $ipaddress; - - return $this; - } - - /** - * @return string|null - */ - public function getIp() - { - return $this->clientIp; - } - - /** - * Set internal roles from current username/ipaddress. - * Adds the default role "guest", if not done by username/ipaddress. - * - * @return $this Fluent interface. - */ - private function setRoles() - { - $this->roles = array_merge($this->userRoles, $this->ipaddressRoles); - $this->roles[] = 'guest'; - - $this->roles = array_unique($this->roles); - return $this; - } - - /** - * Get the roles that are assigned to the specified username. - * - * @param string $username username to be set. - * @throws SecurityException Thrown if the supplied identity could not be found. - * @return array Array of assigned roles or an empty array. - */ - private static function getUsernameRoles($username) - { - if ($username === null || true === empty($username)) { - return []; - } - - $accounts = TableGateway::getInstance(Accounts::class); - $account = $accounts->fetchRow($accounts->select()->where('login = ?', $username)); - if (null === $account) { - $logger = Log::get(); - $message = "An user with the given name: $username could not be found."; - if ($logger !== null) { - $logger->err($message); - } - throw new SecurityException($message); - } - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - return $db->fetchCol( - $db->select() - ->from(['r' => 'user_roles'], ['r.name']) - ->join(['l' => 'link_accounts_roles'], 'l.role_id = r.id', '') - ->join(['a' => 'accounts'], 'l.account_id = a.id', '') - ->where('login = ?', $username) - ->distinct() - ); - } - - /** - * Map an IP address to Roles. - * - * @param string $ipaddress ip address to be set. - * @throws SecurityException Thrown if the supplied ip is not valid. - * @return array Array of assigned roles or an empty array. - */ - private static function getIpaddressRoles($ipaddress) - { - if ($ipaddress === null || true === empty($ipaddress)) { - return []; - } - - if (! self::validateIpAddress($ipaddress)) { - throw new SecurityException('Your IP address could not be validated.'); - } - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - return $db->fetchCol( - $db->select() - ->from(['r' => 'user_roles'], ['r.name']) - ->join(['l' => 'link_ipranges_roles'], 'l.role_id = r.id', '') - ->join(['i' => 'ipranges'], 'l.iprange_id = i.id', '') - ->where('i.startingip <= ?', sprintf("%u", ip2long($ipaddress))) - ->where('i.endingip >= ?', sprintf("%u", ip2long($ipaddress))) - ->distinct() - ); - } - - /** - * Returns all module resources to which the current user and ip address - * has access. - * - * @param string|null $username name of the account to get resources for. - * Defaults to currently logged in user - * @param string|null $ipaddress IP address to get resources for. - * Defaults to current remote address if available. - * @return array Module resource names - * @throws SecurityException Thrown if the supplied ip is not valid or user can not be determined. - */ - public static function getAllowedModuleResources($username = null, $ipaddress = null) - { - $resources = []; - if ($ipaddress !== null && ! self::validateIpAddress($ipaddress)) { - throw new SecurityException('Your IP address could not be validated.'); - } - - if (empty($ipaddress) && empty($username)) { - throw new SecurityException('username and / or IP address must be provided.'); - } else { - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - $select = $db->select(); - $select->from(['am' => 'access_modules'], ['am.module_name']) - ->joinLeft(['r' => 'user_roles'], 'r.id = am.role_id') - ->distinct(); - if ($username !== null) { - $select->joinLeft(['la' => 'link_accounts_roles'], 'la.role_id = r.id', '') - ->joinLeft(['a' => 'accounts'], 'la.account_id = a.id', '') - ->where('login = ?', $username); - } - if ($ipaddress !== null) { - $select->joinLeft(['li' => 'link_ipranges_roles'], 'li.role_id = r.id', '') - ->joinLeft(['i' => 'ipranges'], 'li.iprange_id = i.id', ''); - $select->orWhere('i.startingip <= ? AND i.endingip >= ?', sprintf("%u", ip2long($ipaddress)), sprintf("%u", ip2long($ipaddress))); - } - $resources = $db->fetchCol($select); - } - return $resources; - } - - /** - * checks if the string provided is a valid ip address - * - * @param string $ipaddress ip address to validate. - * @return bool Returns true if validation succeeded - */ - private static function validateIpAddress($ipaddress) - { - $regex = '/^(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\.' - . '(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\.' - . '(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\.' - . '(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])$/'; - return preg_match($regex, $ipaddress) === 1; - } - - /** - * Checks, if the logged user is allowed to access (document_id). - * - * @param null|string $documentId ID of the document to check - * @return bool Returns true only if access is granted. - */ - public function checkDocument($documentId = null) - { - if ($this->skipSecurityChecks()) { - return true; - } - - if (empty($documentId)) { - return false; - } - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - $results = $db->fetchAll( - $db->select() - ->from(['ad' => 'access_documents'], ['document_id']) - ->join(['r' => 'user_roles'], 'ad.role_id = r.id', '') - ->where('r.name IN (?)', $this->roles) - ->where('ad.document_id = ?', $documentId) - ); - return 1 <= count($results) ? true : false; - } - - /** - * Checks, if the logged user is allowed to access (file_id). - * - * @param null|string $fileId ID of the file to check - * @return bool Returns true only if access is granted. - */ - public function checkFile($fileId = null) - { - if ($this->skipSecurityChecks()) { - return true; - } - - if (empty($fileId)) { - return false; - } - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - $results = $db->fetchAll( - $db->select() - ->from(['af' => 'access_files'], ['file_id']) - ->join(['r' => 'user_roles'], 'af.role_id = r.id', '') - ->where('r.name IN (?)', $this->roles) - ->where('af.file_id = ?', $fileId) - ); - return 1 <= count($results) ? true : false; - } - - /** - * Checks, if the logged user is allowed to access (module_name). - * - * @param null|string $moduleName Name of the module to check - * @return bool Returns true only if access is granted. - */ - public function checkModule($moduleName = null) - { - if ($this->skipSecurityChecks()) { - return true; - } - - if (empty($moduleName)) { - return false; - } - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - $results = $db->fetchAll( - $db->select() - ->from(['am' => 'access_modules'], ['module_name']) - ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') - ->where('r.name IN (?)', $this->roles) - ->where('am.module_name = ?', $moduleName) - ); - return 1 <= count($results) ? true : false; - } - - /** - * Checks if a user has access to a module. - * - * @param string $moduleName Name of module - * @param string $user Name of user - * @return bool - */ - public static function checkModuleForUser($moduleName, $user) - { - $roles = self::getUsernameRoles($user); - - $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); - $results = $db->fetchAll( - $db->select() - ->from(['am' => 'access_modules'], ['module_name']) - ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') - ->where('r.name IN (?)', $roles) - ->where('am.module_name = ?', $moduleName) - ); - return 1 <= count($results) ? true : false; - } - - /** - * Check if user with administrator-role or security is disabled. - * - * @return bool - */ - public function skipSecurityChecks() - { - // Check if security is switched off - $conf = Config::get(); - if (isset($conf->security) && (! filter_var($conf->security, FILTER_VALIDATE_BOOLEAN))) { - return true; - } - - if (true === in_array('administrator', $this->roles)) { - return true; - } - - return false; - } - - /** - * Returns the names of the roles for current user and ip address range. - * - * @return array of strings - Names of roles - */ - public function getRoles() - { - return $this->roles; - } - - /** - * Checks if a privilege is granted for actual context (usersession, ip address). - * If administrator is one of the current roles true will be returned ingoring everything else. - * - * @deprecated - * - * @param string $privilege - * @param string|null $documentServerState - * @param int|null $fileId - * @return bool - */ - public function check($privilege, $documentServerState = null, $fileId = null) - { - return $this->skipSecurityChecks(); - } - - /* Singleton code below */ - - /** - * Holds instance. - * - * @var RealmInterface - */ - private static $instance; - - /** - * Delivers the singleton instance. - * - * @return RealmInterface - */ - final public static function getInstance() - { - if (null === self::$instance) { - $class = static::class; - self::$instance = new $class(); - } - return self::$instance; - } - - /** - * Disallow construction. - */ - final private function __construct() - { - } - - /** - * Singleton classes cannot be cloned! - */ - final private function __clone() - { - } -} diff --git a/library/Opus/Security/RealmStorage.php b/library/Opus/Security/RealmStorage.php new file mode 100644 index 00000000..bce565e4 --- /dev/null +++ b/library/Opus/Security/RealmStorage.php @@ -0,0 +1,239 @@ +fetchRow($accounts->select()->where('login = ?', $username)); + if (null === $account) { + $logger = Log::get(); + $message = "An user with the given name: $username could not be found."; + if ($logger !== null) { + $logger->err($message); + } + throw new SecurityException($message); + } + + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + return $db->fetchCol( + $db->select() + ->from(['r' => 'user_roles'], ['r.name']) + ->join(['l' => 'link_accounts_roles'], 'l.role_id = r.id', '') + ->join(['a' => 'accounts'], 'l.account_id = a.id', '') + ->where('login = ?', $username) + ->distinct() + ); + } + + /** + * Map an IP address to Roles. + * + * @param string $ipaddress ip address to be set. + * @throws SecurityException Thrown if the supplied ip is not valid. + * @return array Array of assigned roles or an empty array. + */ + public function getIpaddressRoles($ipaddress) + { + if ($ipaddress === null || true === empty($ipaddress)) { + return []; + } + + if (! Realm::validateIpAddress($ipaddress)) { + throw new SecurityException('Your IP address could not be validated.'); + } + + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + return $db->fetchCol( + $db->select() + ->from(['r' => 'user_roles'], ['r.name']) + ->join(['l' => 'link_ipranges_roles'], 'l.role_id = r.id', '') + ->join(['i' => 'ipranges'], 'l.iprange_id = i.id', '') + ->where('i.startingip <= ?', sprintf("%u", ip2long($ipaddress))) + ->where('i.endingip >= ?', sprintf("%u", ip2long($ipaddress))) + ->distinct() + ); + } + + /** + * Returns all module resources to which the current user and ip address + * has access. + * + * @param string|null $username name of the account to get resources for. + * Defaults to currently logged in user + * @param string|null $ipaddress IP address to get resources for. + * Defaults to current remote address if available. + * @return array Module resource names + * @throws SecurityException Thrown if the supplied ip is not valid or user can not be determined. + */ + public function getAllowedModuleResources($username = null, $ipaddress = null) + { + // TODO verify parameters? The code for that is now in Opus\Common\Security\Realm + + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + $select = $db->select(); + $select->from(['am' => 'access_modules'], ['am.module_name']) + ->joinLeft(['r' => 'user_roles'], 'r.id = am.role_id') + ->distinct(); + + if ($username !== null) { + $select->joinLeft(['la' => 'link_accounts_roles'], 'la.role_id = r.id', '') + ->joinLeft(['a' => 'accounts'], 'la.account_id = a.id', '') + ->where('login = ?', $username); + } + + if ($ipaddress !== null) { + $select->joinLeft(['li' => 'link_ipranges_roles'], 'li.role_id = r.id', '') + ->joinLeft(['i' => 'ipranges'], 'li.iprange_id = i.id', ''); + $select->orWhere( + 'i.startingip <= ? AND i.endingip >= ?', + sprintf("%u", ip2long($ipaddress)), + sprintf("%u", ip2long($ipaddress)) + ); + } + + return $db->fetchCol($select); + } + + /** + * Checks, if the logged user is allowed to access (document_id). + * + * @param string $documentId ID of the document to check + * @param string[] $roles + * @return bool Returns true only if access is granted. + */ + public function checkDocument($documentId, $roles) + { + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + $results = $db->fetchAll( + $db->select() + ->from(['ad' => 'access_documents'], ['document_id']) + ->join(['r' => 'user_roles'], 'ad.role_id = r.id', '') + ->where('r.name IN (?)', $roles) + ->where('ad.document_id = ?', $documentId) + ); + return 1 <= count($results) ? true : false; + } + + /** + * Checks, if the logged user is allowed to access (file_id). + * + * @param string $fileId ID of the file to check + * @param string[] $roles + * @return bool Returns true only if access is granted. + */ + public function checkFile($fileId, $roles) + { + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + $results = $db->fetchAll( + $db->select() + ->from(['af' => 'access_files'], ['file_id']) + ->join(['r' => 'user_roles'], 'af.role_id = r.id', '') + ->where('r.name IN (?)', $roles) + ->where('af.file_id = ?', $fileId) + ); + return 1 <= count($results) ? true : false; + } + + /** + * Checks, if the logged user is allowed to access (module_name). + * + * @param string $moduleName Name of the module to check + * @param string[] $roles + * @return bool Returns true only if access is granted. + */ + public function checkModule($moduleName, $roles) + { + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + $results = $db->fetchAll( + $db->select() + ->from(['am' => 'access_modules'], ['module_name']) + ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') + ->where('r.name IN (?)', $roles) + ->where('am.module_name = ?', $moduleName) + ); + return 1 <= count($results) ? true : false; + } + + /** + * Checks if a user has access to a module. + * + * @param string $moduleName Name of module + * @param string $user Name of user + * @return bool + */ + public function checkModuleForUser($moduleName, $user) + { + $roles = $this->getUsernameRoles($user); + + $db = TableGateway::getInstance(UserRoles::class)->getAdapter(); + $results = $db->fetchAll( + $db->select() + ->from(['am' => 'access_modules'], ['module_name']) + ->join(['r' => 'user_roles'], 'am.role_id = r.id', '') + ->where('r.name IN (?)', $roles) + ->where('am.module_name = ?', $moduleName) + ); + return 1 <= count($results) ? true : false; + } +} diff --git a/library/Opus/Statistic/LocalCounter.php b/library/Opus/Statistic/LocalCounter.php index 2bb5b620..c9488ea2 100644 --- a/library/Opus/Statistic/LocalCounter.php +++ b/library/Opus/Statistic/LocalCounter.php @@ -26,13 +26,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2009-2019, OPUS 4 development team + * @copyright Copyright (c) 2009, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus\Statistic - * @author Tobias Leidinger - * @author Jens Schwidder */ namespace Opus\Statistic; @@ -41,9 +36,9 @@ use Exception; use Opus\Common\Config; use Opus\Common\Model\ModelException; +use Opus\Common\Security\Realm; use Opus\Db\DocumentStatistics; use Opus\Db\TableGateway; -use Opus\Security\Realm; use function array_key_exists; use function date; @@ -60,15 +55,13 @@ /** * Controller for Opus Applications. - * - * phpcs:disable */ class LocalCounter { /** * Holds instance of the class * - * @var LocalCounter + * @var self */ private static $localCounter; @@ -130,7 +123,7 @@ private function __construct() } /** - * @return LocalCounter + * @return self */ public static function getInstance() { @@ -143,8 +136,8 @@ public static function getInstance() /** * check whether user agent contains one of the spiders from the counter list * - * @param $userAgent $_SERVER['user_agent'] string - * @return bool is spieder? + * @param string $userAgent $_SERVER['user_agent'] string + * @return bool is spider? */ private function checkSpider($userAgent) { @@ -158,27 +151,39 @@ private function checkSpider($userAgent) return false; } + /** + * @param int $redirectStatus + * @return bool + */ private function isRedirectStatusOk($redirectStatus) { return $redirectStatus === 200 || $redirectStatus === 304; } + /** + * @param int $documentId + */ public function countFrontdoor($documentId) { $this->count($documentId, -1, 'frontdoor'); } + /** + * @param int $documentId + * @param int $fileId + */ public function countFiles($documentId, $fileId) { $this->count($documentId, $fileId, 'files'); } /** - * @param $documentId - * @param $fileId + * @param int $documentId + * @param int $fileId + * @param string $type * @param string|null $ip TODO not used - * @param $userAgent - * @param $redirectStatus + * @param string|null $userAgent + * @param int|null $redirectStatus * @return int new counter value for given doc_id - month -year triple or FALSE if double click or spider */ public function count($documentId, $fileId, $type, $ip = null, $userAgent = null, $redirectStatus = null) @@ -187,7 +192,7 @@ public function count($documentId, $fileId, $type, $ip = null, $userAgent = null return 0; } - if ($type != 'frontdoor' && $type != 'files') { + if ($type !== 'frontdoor' && $type !== 'files') { //print('type not defined'); return 0; } @@ -276,17 +281,17 @@ public function count($documentId, $fileId, $type, $ip = null, $userAgent = null /** * log click to temp file and return whether it was a double click or not * - * @param $documentId id of documents table - * @param $fileId id of document_files table - * @param $time + * @param int $documentId id of documents table + * @param int $fileId id of document_files table + * @param int $time * @return bool is it a double click */ public function logClick($documentId, $fileId, $time) { - $ip = ''; + $ip = ''; $clientIp = Realm::getInstance()->getIp(); if ($clientIp !== null) { - $ip = $clientIp; + $ip = $clientIp; } $tempDir = Config::getInstance()->getTempPath(); @@ -312,7 +317,7 @@ public function logClick($documentId, $fileId, $time) //if global file access timestamp too old, the whole log file can be removed $xmlTime = $dom->getElementsByTagName("time")->item(0); - if ($xmlTime != null && ($time - $xmlTime->nodeValue) > max($this->doubleClickIntervalHtml, $this->doubleClickIntervalPdf)) { + if ($xmlTime !== null && ($time - $xmlTime->nodeValue) > max($this->doubleClickIntervalHtml, $this->doubleClickIntervalPdf)) { $xmlAccess = $dom->getElementsByTagName("access")->item(0); $dom->removeChild($xmlAccess); $xmlAccess = $dom->createElement('access'); @@ -320,7 +325,7 @@ public function logClick($documentId, $fileId, $time) } $xmlTime = $xmlAccess->getElementsByTagName('time')->item(0); - if ($xmlTime != null) { + if ($xmlTime !== null) { $xmlAccess->removeChild($xmlTime); } $xmlTime = $dom->createElement('time', $time); @@ -341,22 +346,24 @@ public function logClick($documentId, $fileId, $time) //get file id, create if not exists $xmlFileId = $xmlIp->getElementsByTagName('file' . $fileId)->item(0); - if ($xmlFileId == null) { + if ($xmlFileId === null) { $xmlFileId = $dom->createElement('file' . $fileId); $xmlIp->appendChild($xmlFileId); } //read last Access for this file id - $fileIdTime = $xmlFileId->getAttribute('lastAccess'); + $fileIdTime = ( int )$xmlFileId->getAttribute('lastAccess'); $doubleClick = false; - if ($fileIdTime == null || $time - $fileIdTime > max($this->doubleClickIntervalHtml, $this->doubleClickIntervalPdf)) { + if ($fileIdTime === null || $time - $fileIdTime > max($this->doubleClickIntervalHtml, $this->doubleClickIntervalPdf)) { /*no lastAccess set (new entry for this id) or lastAccess too far away -> create entry with actual time -> return no double click*/ + // TODO should there be code here? + $doubleClick = false; } elseif ((($time - $fileIdTime) <= $this->doubleClickIntervalHtml) && (($filetype === 'html') || ($fileId === -1))) { //html file double click $doubleClick = true; - } elseif ((($time - $fileIdTime) <= $this->doubleClickIntervalPdf) && ($filetype === 'pdf') && ($fileId != -1)) { + } elseif ((($time - $fileIdTime) <= $this->doubleClickIntervalPdf) && ($filetype === 'pdf') && ($fileId !== -1)) { //pdf file double click $doubleClick = true; } @@ -371,6 +378,12 @@ public function logClick($documentId, $fileId, $time) return $doubleClick; } + /** + * @param int $documentId + * @param string $datatype + * @param null|string $year + * @return array + */ public function readMonths($documentId, $datatype = 'files', $year = null) { if ($year === null) { @@ -378,7 +391,7 @@ public function readMonths($documentId, $datatype = 'files', $year = null) $year = date('Y', time()); } - if ($datatype != 'files' && $datatype != 'frontdoor') { + if ($datatype !== 'files' && $datatype !== 'frontdoor') { $datatype = 'files'; } $ods = TableGateway::getInstance(DocumentStatistics::class); @@ -401,9 +414,14 @@ public function readMonths($documentId, $datatype = 'files', $year = null) return $result; } + /** + * @param int $documentId + * @param string $datatype + * @return array|int[] + */ public function readYears($documentId, $datatype = 'files') { - if ($datatype != 'files' && $datatype != 'frontdoor') { + if ($datatype !== 'files' && $datatype !== 'frontdoor') { $datatype = 'files'; } $ods = TableGateway::getInstance(DocumentStatistics::class); @@ -432,9 +450,14 @@ public function readYears($documentId, $datatype = 'files') return $result; } + /** + * @param int $documentId + * @param string $datatype + * @return int|string + */ public function readTotal($documentId, $datatype = 'files') { - if ($datatype != 'files' && $datatype != 'frontdoor') { + if ($datatype !== 'files' && $datatype !== 'frontdoor') { $datatype = 'files'; } $ods = TableGateway::getInstance(DocumentStatistics::class); diff --git a/tests/Opus/RequireTest.php b/tests/Opus/RequireTest.php index e49abad5..d6918c1b 100644 --- a/tests/Opus/RequireTest.php +++ b/tests/Opus/RequireTest.php @@ -38,7 +38,7 @@ use Opus\Doi\DataCiteXmlGenerationException; use Opus\Identifier\Urn; use Opus\Model\Field; -use Opus\Security\Realm; +use Opus\Security\RealmStorage; use Opus\Statistic\LocalCounter; use Opus\Translate\DatabaseAdapter; use OpusDb_Mysqlutf8; @@ -49,10 +49,6 @@ /** * Test cases to load all class files. - * - * @package Opus - * @category Tests - * @group RequireTest */ class RequireTest extends TestCase { @@ -108,7 +104,7 @@ public function instantiateTestProvider() Base::class, LocalCounter::class, Urn::class, - Realm::class, + RealmStorage::class, Field::class, OpusStorageFile::class, DatabaseAdapter::class, diff --git a/tests/Opus/Security/AuthAdapterTest.php b/tests/Opus/Security/AuthAdapterTest.php deleted file mode 100644 index 6cdb88a9..00000000 --- a/tests/Opus/Security/AuthAdapterTest.php +++ /dev/null @@ -1,117 +0,0 @@ -clearTables(false, ['accounts']); - - $bob = Account::new(); - $bob->setLogin('bob')->setPassword('secret')->store(); - - $this->authAdapter = new AuthAdapter(); - } - - /** - * Test if a successful authentication can be performed. - */ - public function testSettingEmptyCredentialsThrowsException() - { - $this->expectException('Zend_Auth_Adapter_Exception'); - $this->authAdapter->setCredentials('', null); - } - - /** - * Test if a successful authentication can be performed. - */ - public function testSuccessfulAuthentication() - { - $this->authAdapter->setCredentials('bob', 'secret'); - $result = $this->authAdapter->authenticate(); - $this->assertNotNull($result, 'Authentication result should not be null.'); - $this->assertInstanceOf('Zend_Auth_Result', $result, 'Authentication result should be of type\Zend_Auth_Result.'); - $this->assertEquals($result->getCode(), Zend_Auth_Result::SUCCESS, 'Authentication should be successful.'); - } - - /** - * Test if given invalid credentials failes. - * - * @param string $login Login credentials. - * @param string $password Password credentials. - * @param int $code Expected\Zend_Auth_Result code. - * @dataProvider invalidCredentialsDataProvider - */ - public function testFailingAuthentication($login, $password, $code) - { - $this->authAdapter->setCredentials($login, $password); - $result = $this->authAdapter->authenticate(); - $this->assertNotNull($result, 'Authentication result should not be null.'); - $this->assertInstanceOf('Zend_Auth_Result', $result, 'Authentication result should be of type\Zend_Auth_Result.'); - $this->assertEquals($result->getCode(), $code, 'Authentication should not be successful.'); - } -} diff --git a/tests/Opus/Security/RealmTest.php b/tests/Opus/Security/RealmTest.php index 9d70ce52..036bf2e1 100644 --- a/tests/Opus/Security/RealmTest.php +++ b/tests/Opus/Security/RealmTest.php @@ -32,6 +32,8 @@ namespace OpusTest\Security; use Opus\Common\Config; +use Opus\Common\Security\Realm; +use Opus\Common\Security\RealmInterface; use Opus\Common\Security\SecurityException; use Opus\Db\AccessDocuments; use Opus\Db\AccessFiles; @@ -44,7 +46,6 @@ use Opus\Db\LinkIprangesRoles; use Opus\Db\TableGateway; use Opus\Db\UserRoles; -use Opus\Security\Realm; use OpusTest\TestAsset\TestCase; use Zend_Config; @@ -183,7 +184,8 @@ public function testGetInstance() { $realm = Realm::getInstance(); $this->assertNotNull($realm, 'Expected instance'); - $this->assertInstanceOf(Realm::class, $realm, 'Expected object of type Opus\Security\Realm.'); + $this->assertInstanceOf(RealmInterface::class, $realm, 'Expected object of type Opus\Security\Realm.'); + $this->assertSame($realm, Realm::getInstance()); } /** diff --git a/tests/test.ini b/tests/test.ini index 68bbee53..5e698308 100644 --- a/tests/test.ini +++ b/tests/test.ini @@ -43,6 +43,7 @@ reviewer.global[] = 'hasmail' reviewer.collections.ddc.510[] = 'admin' reviewer.collections.ddc.510[] = 'foobar' +securityStorageClass = 'Opus\Security\RealmStorage' securityPolicy.files.defaultAccessRole = 'guest' modelFactory = "Opus\ModelFactory" From ed7a253b7e523eb2843981ef41f2a2184c5aba2c Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 26 Oct 2022 08:32:41 +0200 Subject: [PATCH 04/46] #146 Fixed coding style --- library/Opus/Statistic/LocalCounter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Opus/Statistic/LocalCounter.php b/library/Opus/Statistic/LocalCounter.php index c9488ea2..e16c372e 100644 --- a/library/Opus/Statistic/LocalCounter.php +++ b/library/Opus/Statistic/LocalCounter.php @@ -352,7 +352,7 @@ public function logClick($documentId, $fileId, $time) } //read last Access for this file id - $fileIdTime = ( int )$xmlFileId->getAttribute('lastAccess'); + $fileIdTime = (int) $xmlFileId->getAttribute('lastAccess'); $doubleClick = false; if ($fileIdTime === null || $time - $fileIdTime > max($this->doubleClickIntervalHtml, $this->doubleClickIntervalPdf)) { From 7478bf28b8a999f4961423a6862cba4374d52cf7 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 1 Nov 2022 10:45:58 +0100 Subject: [PATCH 05/46] coding style --- bin/install-composer.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bin/install-composer.sh b/bin/install-composer.sh index 3dd488a3..2065ff54 100755 --- a/bin/install-composer.sh +++ b/bin/install-composer.sh @@ -11,9 +11,7 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. # -# @author Thomas Urban -# @author Jens Schwidder -# @copyright Copyright (c) 2010-2016, OPUS 4 development team +# @copyright Copyright (c) 2010, OPUS 4 development team # @license http://www.gnu.org/licenses/gpl.html General Public License # @@ -29,7 +27,6 @@ set -e -SCRIPT_NAME="$(basename "$0")" SCRIPT_NAME_FULL="`readlink -f "$0"`" SCRIPT_PATH="`dirname "$SCRIPT_NAME_FULL"`" From d4dc846d9a1e0bfd70f5f3734d68a0d4c3367e49 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 1 Nov 2022 10:50:38 +0100 Subject: [PATCH 06/46] #243 interactive mode and cleanup --- bin/prepare-database.sh | 56 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/bin/prepare-database.sh b/bin/prepare-database.sh index dfa5785c..20d6905f 100755 --- a/bin/prepare-database.sh +++ b/bin/prepare-database.sh @@ -36,9 +36,28 @@ # IMPORTANT: This script is also used by other OPUS 4 packages that require # a database for testing. # +# TODO dropping users prevents this script from being used to setup multiple databases with the same users (testing) +# For testing on GitHub dropping users doesn't matter because we start with a fresh database. +# TODO create new database without creating new users +# +# TODO interactive mode +# TODO MySQL root password entered interactively by default, so it does not appear in logs +# TODO replace underlines in option names (dashes seem more common and are easier) +# +# TODO variable for MySQL client +# TODO variables for host and port +# TODO variable for MySQL "root" username +# +# TODO merge with setup.sh (get rid of setup.sh or remove redundancy) +# TODO creating database.ini file with config ? (replacing/removing config.ini.template) +# config.ini OR database.ini should just be database options - Rest goes into test.ini and application.ini +# TODO creating schema +# +# TODO make this script available in other projects in vendor/bin (setup in composer.json) +# # Define variables and their default values -root_pwd='root' +# root_pwd='root' admin_name='opus4admin' admin_pwd='opusadminpwd' user_name='opus4' @@ -75,6 +94,8 @@ if [ $# -gt 0 ]; then fi fi +interactive_enabled=0 + # Parse any other command line options while [ $# -gt 0 ]; do if [[ $1 == "--"* ]]; then # only deal with long options @@ -87,9 +108,40 @@ while [ $# -gt 0 ]; do # Process next option shift + else + if [[ $1 == "--interactive" ]]; then + interactive_enabled=1 + fi + fi + else + if [[ $1 == "-i" ]]; then + interactive_enabled=1 fi fi shift done -export MYSQL_PWD=$root_pwd && mysql --default-character-set=utf8 -h 'localhost' -P '3306' -u 'root' -v -e "CREATE DATABASE IF NOT EXISTS $database_name DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; DROP USER IF EXISTS '$admin_name'@'localhost'; CREATE USER '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$admin_pwd'; GRANT ALL PRIVILEGES ON $database_name.* TO '$admin_name'@'localhost'; DROP USER IF EXISTS '$user_name'@'localhost'; CREATE USER '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; GRANT SELECT,INSERT,UPDATE,DELETE ON $database_name.* TO '$user_name'@'localhost'; FLUSH PRIVILEGES;" +if [[ -z $root_pwd ]]; then + if [ $interactive_enabled != 1 ]; then + root_pwd='root' + else + # Querying MySQL root password + [[ -z $root_pwd ]] && read -p "MySQL root user password: " -s root_pwd + fi +fi + +# TODO remove following SQL permanently +# DROP USER IF EXISTS '$admin_name'@'localhost'; +# DROP USER IF EXISTS '$user_name'@'localhost'; + +sql=$(cat <<-ENDSTRING + CREATE DATABASE IF NOT EXISTS $database_name DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; + CREATE USER IF NOT EXITS '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '${admin_pwd}'; + GRANT ALL PRIVILEGES ON $database_name.* TO '$admin_name'@'localhost'; + CREATE IF NOT EXISTS USER '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; + GRANT SELECT,INSERT,UPDATE,DELETE ON $database_name.* TO '$user_name'@'localhost'; + FLUSH PRIVILEGES; +ENDSTRING +) + +export MYSQL_PWD=$root_pwd && mysql --default-character-set=utf8 -h 'localhost' -P '3306' -u 'root' -v -e "$sql" From 825fd6d46169e14955bf1dd9028c94d13784ef1e Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 1 Nov 2022 10:54:58 +0100 Subject: [PATCH 07/46] #243 Fixed typo in SQL --- bin/prepare-database.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/prepare-database.sh b/bin/prepare-database.sh index 20d6905f..42a604b2 100755 --- a/bin/prepare-database.sh +++ b/bin/prepare-database.sh @@ -136,7 +136,7 @@ fi sql=$(cat <<-ENDSTRING CREATE DATABASE IF NOT EXISTS $database_name DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; - CREATE USER IF NOT EXITS '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '${admin_pwd}'; + CREATE USER IF NOT EXISTS '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '${admin_pwd}'; GRANT ALL PRIVILEGES ON $database_name.* TO '$admin_name'@'localhost'; CREATE IF NOT EXISTS USER '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; GRANT SELECT,INSERT,UPDATE,DELETE ON $database_name.* TO '$user_name'@'localhost'; From 6b5fdbbc7805817671ed9f6c991aaa9fb88019a2 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 1 Nov 2022 11:00:22 +0100 Subject: [PATCH 08/46] #243 Fixed 2nd typo in SQL --- bin/prepare-database.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/prepare-database.sh b/bin/prepare-database.sh index 42a604b2..8aca6788 100755 --- a/bin/prepare-database.sh +++ b/bin/prepare-database.sh @@ -138,7 +138,7 @@ sql=$(cat <<-ENDSTRING CREATE DATABASE IF NOT EXISTS $database_name DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; CREATE USER IF NOT EXISTS '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '${admin_pwd}'; GRANT ALL PRIVILEGES ON $database_name.* TO '$admin_name'@'localhost'; - CREATE IF NOT EXISTS USER '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; + CREATE USER IF NOT EXISTS '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; GRANT SELECT,INSERT,UPDATE,DELETE ON $database_name.* TO '$user_name'@'localhost'; FLUSH PRIVILEGES; ENDSTRING From 5fa709f86cab560f7932a7f68e09f612b32a85b4 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 8 Nov 2022 16:12:39 +0100 Subject: [PATCH 09/46] #319 Added options; Install script to vendor/bin --- bin/prepare-database.sh | 72 ++++++++++++++++++++--------------------- composer.json | 3 ++ 2 files changed, 38 insertions(+), 37 deletions(-) diff --git a/bin/prepare-database.sh b/bin/prepare-database.sh index 8aca6788..f15c4377 100755 --- a/bin/prepare-database.sh +++ b/bin/prepare-database.sh @@ -36,17 +36,7 @@ # IMPORTANT: This script is also used by other OPUS 4 packages that require # a database for testing. # -# TODO dropping users prevents this script from being used to setup multiple databases with the same users (testing) -# For testing on GitHub dropping users doesn't matter because we start with a fresh database. -# TODO create new database without creating new users -# -# TODO interactive mode -# TODO MySQL root password entered interactively by default, so it does not appear in logs -# TODO replace underlines in option names (dashes seem more common and are easier) -# -# TODO variable for MySQL client -# TODO variables for host and port -# TODO variable for MySQL "root" username +# TODO make interactive mode default (requires updating usage across all packages) # # TODO merge with setup.sh (get rid of setup.sh or remove redundancy) # TODO creating database.ini file with config ? (replacing/removing config.ini.template) @@ -57,12 +47,16 @@ # # Define variables and their default values -# root_pwd='root' -admin_name='opus4admin' -admin_pwd='opusadminpwd' -user_name='opus4' -user_pwd='opususerpwd' -database_name='opusdb' +sqluser='root' +sqlpwd='root' +admin='opus4admin' +adminpwd='opusadminpwd' +user='opus4' +userpwd='opususerpwd' +dbname='opusdb' +host='localhost' +port='3306' +mysql='mysql' # Print command line help to stderr display_help() { @@ -70,18 +64,26 @@ display_help() { echo echo "Options (default values given in parentheses):" echo - echo " --root_pwd Root password ($root_pwd)" - echo " --admin_name Admin name ($admin_name)" - echo " --admin_pwd Admin password ($admin_pwd)" - echo " --user_name User name ($user_name)" - echo " --user_pwd User password ($user_pwd)" - echo " --database_name Database name ($database_name)" + echo " --help (-h) Print out help" + echo " --interactive (-i) Ask for SQL root password interactively" + echo + echo " --sqluser SQL root user ($sqluser)" + echo " --sqlpwd SQL root password ($sqlpwd)" + echo " --admin Admin name ($admin)" + echo " --adminpwd Admin password ($adminpwd)" + echo " --user User name ($user)" + echo " --userpwd User password ($userpwd)" + echo " --dbname Database name ($dbname)" + echo " --host MySQL host ($host)" + echo " --port MySQL port ($port)" + echo " --mysql MySQL client ($mysql)" echo echo "Examples:" echo echo " $0" echo " $0 --help" - echo " $0 --admin_pwd ADMINPWD --user_pwd USERPWD" + echo " $0 --adminpwd ADMINPWD --userpwd USERPWD" + echo " $0 -i --dbname opusdbtest" echo exit 1 } @@ -121,27 +123,23 @@ while [ $# -gt 0 ]; do shift done -if [[ -z $root_pwd ]]; then +if [[ -z $sqlpwd ]]; then if [ $interactive_enabled != 1 ]; then - root_pwd='root' + sqlpwd='root' else # Querying MySQL root password - [[ -z $root_pwd ]] && read -p "MySQL root user password: " -s root_pwd + [[ -z $sqlpwd ]] && read -p "MySQL root user password: " -s sqlpwd fi fi -# TODO remove following SQL permanently -# DROP USER IF EXISTS '$admin_name'@'localhost'; -# DROP USER IF EXISTS '$user_name'@'localhost'; - sql=$(cat <<-ENDSTRING - CREATE DATABASE IF NOT EXISTS $database_name DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; - CREATE USER IF NOT EXISTS '$admin_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '${admin_pwd}'; - GRANT ALL PRIVILEGES ON $database_name.* TO '$admin_name'@'localhost'; - CREATE USER IF NOT EXISTS '$user_name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$user_pwd'; - GRANT SELECT,INSERT,UPDATE,DELETE ON $database_name.* TO '$user_name'@'localhost'; + CREATE DATABASE IF NOT EXISTS $dbname DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; + CREATE USER IF NOT EXISTS '$admin'@'localhost' IDENTIFIED WITH mysql_native_password BY '${adminpwd}'; + GRANT ALL PRIVILEGES ON $dbname.* TO '$admin'@'localhost'; + CREATE USER IF NOT EXISTS '$user'@'localhost' IDENTIFIED WITH mysql_native_password BY '$userpwd'; + GRANT SELECT,INSERT,UPDATE,DELETE ON $dbname.* TO '$user'@'localhost'; FLUSH PRIVILEGES; ENDSTRING ) -export MYSQL_PWD=$root_pwd && mysql --default-character-set=utf8 -h 'localhost' -P '3306' -u 'root' -v -e "$sql" +export MYSQL_PWD=$sqlpwd && mysql --default-character-set=utf8 -h $host -P $port -u $sqluser -v -e "$sql" diff --git a/composer.json b/composer.json index 719450ea..507b0cc6 100644 --- a/composer.json +++ b/composer.json @@ -45,6 +45,9 @@ "archive": { "exclude": ["/tests", "/nbproject"] }, + "bin": [ + "bin/prepare-database.sh" + ], "scripts": { "analysis": [ "Composer\\Config::disableProcessTimeout", From 810aa5d0c0273d06325bb55596904e7c196fca62 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 8 Nov 2022 16:18:06 +0100 Subject: [PATCH 10/46] #319 Updated GitHub Actions config --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index bd36b876..bc796dc4 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -32,7 +32,7 @@ jobs: run: sudo systemctl start mysql.service - name: Prepare database - run: bash bin/prepare-database.sh --admin_pwd root --user_pwd root + run: bash bin/prepare-database.sh --adminpwd root --userpwd root - name: Setup OPUS 4 run: ant prepare-workspace prepare-config create-database lint -DdbUserPassword=root -DdbAdminPassword=root From 9d21b7d6efb521c2827e167e2bfa0bbf88350f57 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 8 Nov 2022 16:36:38 +0100 Subject: [PATCH 11/46] #319 Ask for MySQL root password by default --- .github/workflows/php.yml | 2 +- bin/prepare-database.sh | 28 +++++----------------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index bc796dc4..44849e14 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -32,7 +32,7 @@ jobs: run: sudo systemctl start mysql.service - name: Prepare database - run: bash bin/prepare-database.sh --adminpwd root --userpwd root + run: bash bin/prepare-database.sh --sqlpwd root --adminpwd root --userpwd root - name: Setup OPUS 4 run: ant prepare-workspace prepare-config create-database lint -DdbUserPassword=root -DdbAdminPassword=root diff --git a/bin/prepare-database.sh b/bin/prepare-database.sh index f15c4377..443eca12 100755 --- a/bin/prepare-database.sh +++ b/bin/prepare-database.sh @@ -36,8 +36,6 @@ # IMPORTANT: This script is also used by other OPUS 4 packages that require # a database for testing. # -# TODO make interactive mode default (requires updating usage across all packages) -# # TODO merge with setup.sh (get rid of setup.sh or remove redundancy) # TODO creating database.ini file with config ? (replacing/removing config.ini.template) # config.ini OR database.ini should just be database options - Rest goes into test.ini and application.ini @@ -48,7 +46,6 @@ # Define variables and their default values sqluser='root' -sqlpwd='root' admin='opus4admin' adminpwd='opusadminpwd' user='opus4' @@ -62,13 +59,14 @@ mysql='mysql' display_help() { echo "Usage: $0 [OPTIONS]" >&2 echo + echo "The script will ask for the SQL root password interactively, unless it is specified as option." + echo echo "Options (default values given in parentheses):" echo echo " --help (-h) Print out help" - echo " --interactive (-i) Ask for SQL root password interactively" echo echo " --sqluser SQL root user ($sqluser)" - echo " --sqlpwd SQL root password ($sqlpwd)" + echo " --sqlpwd SQL root password" echo " --admin Admin name ($admin)" echo " --adminpwd Admin password ($adminpwd)" echo " --user User name ($user)" @@ -96,8 +94,6 @@ if [ $# -gt 0 ]; then fi fi -interactive_enabled=0 - # Parse any other command line options while [ $# -gt 0 ]; do if [[ $1 == "--"* ]]; then # only deal with long options @@ -110,27 +106,13 @@ while [ $# -gt 0 ]; do # Process next option shift - else - if [[ $1 == "--interactive" ]]; then - interactive_enabled=1 - fi - fi - else - if [[ $1 == "-i" ]]; then - interactive_enabled=1 fi fi shift done -if [[ -z $sqlpwd ]]; then - if [ $interactive_enabled != 1 ]; then - sqlpwd='root' - else - # Querying MySQL root password - [[ -z $sqlpwd ]] && read -p "MySQL root user password: " -s sqlpwd - fi -fi +# Querying MySQL root password +[[ -z $sqlpwd ]] && read -p "MySQL root user password: " -s sqlpwd sql=$(cat <<-ENDSTRING CREATE DATABASE IF NOT EXISTS $dbname DEFAULT CHARACTER SET = UTF8 DEFAULT COLLATE = UTF8_GENERAL_CI; From 0e481c842b0dcda33f2cf3c62c57ed395853b293 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 8 Nov 2022 17:35:13 +0100 Subject: [PATCH 12/46] #319 Rename db prep script --- bin/{prepare-database.sh => opus4db} | 0 composer.json | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename bin/{prepare-database.sh => opus4db} (100%) diff --git a/bin/prepare-database.sh b/bin/opus4db similarity index 100% rename from bin/prepare-database.sh rename to bin/opus4db diff --git a/composer.json b/composer.json index 507b0cc6..e2b2ccfd 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "exclude": ["/tests", "/nbproject"] }, "bin": [ - "bin/prepare-database.sh" + "bin/opus4db" ], "scripts": { "analysis": [ From 349badbafc3f4fd5271b7b5354270b9e3dd6a69f Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 8 Nov 2022 17:35:59 +0100 Subject: [PATCH 13/46] #319 Update GitHub Actions --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 44849e14..d8ecf459 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -32,7 +32,7 @@ jobs: run: sudo systemctl start mysql.service - name: Prepare database - run: bash bin/prepare-database.sh --sqlpwd root --adminpwd root --userpwd root + run: bash bin/opus4db --sqlpwd root --adminpwd root --userpwd root - name: Setup OPUS 4 run: ant prepare-workspace prepare-config create-database lint -DdbUserPassword=root -DdbAdminPassword=root From 41a5eb205bcfe3b137543771edbce0e72f128e10 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 23 Nov 2022 21:04:47 +0100 Subject: [PATCH 14/46] #320 Preparations for PHP 8 Fixing typing and coding style --- .github/workflows/php.yml | 19 +++++++++++++----- .gitignore | 1 + composer.json | 18 ++++++----------- library/Opus/Account.php | 4 ++-- library/Opus/Collection.php | 2 +- library/Opus/Db/TableGateway.php | 4 ++-- library/Opus/Db/Util/DatabaseHelper.php | 3 ++- library/Opus/Db2/Database.php | 5 +---- .../DocumentFinder/DefaultDocumentFinder.php | 1 + library/Opus/Identifier/Urn.php | 6 +++--- library/Opus/Model/AbstractDb.php | 1 - library/Opus/Model/Field.php | 6 +++--- library/Opus/Model/Properties.php | 4 ++-- library/Opus/Model/Xml.php | 2 +- library/Opus/Model/Xml/AbstractVersion.php | 2 +- library/Opus/Model/Xml/Version1.php | 18 +++++++++-------- library/Opus/Person.php | 4 ++-- library/Opus/Statistic/LocalCounter.php | 1 + library/Opus/Update/Plugin/DatabaseSchema.php | 7 ++----- phpcs.xml | 20 +++---------------- phpunit.xml | 10 ++-------- tests/Opus/AccountTest.php | 2 +- .../Collection/Plugin/DeleteSubTreeTest.php | 2 +- tests/Opus/CollectionNodeTest.php | 16 ++++++--------- .../CollectionRole/Plugin/DeleteTreeTest.php | 8 ++------ tests/Opus/CollectionRoleTest.php | 18 ++++++++--------- tests/Opus/CollectionTest.php | 10 +++++++--- tests/Opus/DateTest.php | 4 +--- tests/Opus/Db/Adapter/Pdo/Mysqlutf8Test.php | 7 ++++--- tests/Opus/Db/InstanciateGatewayTest.php | 11 +++------- tests/Opus/DnbInstituteTest.php | 4 ++-- .../Document/Plugin/IdentifierDoiTest.php | 4 ++-- .../Document/Plugin/IdentifierUrnTest.php | 9 ++------- .../Document/Plugin/SequenceNumberTest.php | 2 +- tests/Opus/Document/Plugin/XmlCacheTest.php | 2 +- tests/Opus/DocumentFinderTest.php | 6 +----- tests/Opus/DocumentRepositoryTest.php | 2 +- tests/Opus/DocumentTest.php | 4 ++-- tests/Opus/Doi/DataCiteXmlGeneratorTest.php | 13 ++++++++---- tests/Opus/Doi/DoiMailNotificationTest.php | 3 ++- tests/Opus/Doi/DoiManagerDataCiteTest.php | 2 +- tests/Opus/Doi/DoiManagerTest.php | 2 +- tests/Opus/Doi/UserRecipientProviderTest.php | 2 +- tests/Opus/EnrichmentKeyTest.php | 2 +- tests/Opus/EnrichmentTest.php | 2 +- tests/Opus/File/Plugin/DefaultAccessTest.php | 2 +- tests/Opus/FileTest.php | 13 +++++------- tests/Opus/HashValuesTest.php | 10 +++------- tests/Opus/Identifier/UrnTest.php | 4 ++-- tests/Opus/IdentifierTest.php | 2 +- tests/Opus/IprangeTest.php | 2 +- tests/Opus/JobTest.php | 2 +- tests/Opus/LanguageTest.php | 2 +- tests/Opus/LicenceTest.php | 6 +----- tests/Opus/Model/AbstractDbTest.php | 4 ++-- .../Dependent/AbstractDependentModelTest.php | 13 +++++++----- .../Dependent/Link/AbstractLinkModelTest.php | 15 +++----------- .../Dependent/Link/AbstractModelMock.php | 8 +------- tests/Opus/Model/FieldTest.php | 4 ++-- tests/Opus/Model/FilterTest.php | 15 +++----------- tests/Opus/Model/Mock/AbstractDbMock.php | 1 + tests/Opus/Model/Mock/ModelDependentMock.php | 4 ++++ .../Opus/Model/Plugin/AbstractPluginMock.php | 6 ++---- .../Plugin/InvalidateDocumentCacheTest.php | 11 +++++----- tests/Opus/Model/PropertiesTest.php | 3 ++- tests/Opus/Model/Xml/CacheTest.php | 5 +---- tests/Opus/Model/Xml/Version1Test.php | 10 +++------- tests/Opus/Model/Xml/Version2Test.php | 4 ++-- tests/Opus/ModelFactoryTest.php | 2 +- tests/Opus/PermissionTest.php | 2 +- tests/Opus/PersonTest.php | 6 +----- tests/Opus/RequireTest.php | 4 ++-- tests/Opus/Security/AccountTest.php | 5 +++-- tests/Opus/Security/RealmTest.php | 4 ++-- tests/Opus/SeriesTest.php | 4 ++-- tests/Opus/Statistic/LocalCounterTest.php | 16 +++------------ tests/Opus/SubjectTest.php | 2 +- .../Opus/TestAsset/AbstractSimpleTestCase.php | 5 +++-- .../Opus/TestAsset/DocumentBasedTestCase.php | 4 +++- tests/Opus/TestAsset/FileMock.php | 8 +++----- tests/Opus/TestAsset/LoggerMock.php | 7 ++----- tests/Opus/TestAsset/TestCase.php | 2 +- tests/Opus/Translate/DaoTest.php | 10 ++++------ tests/Opus/Translate/DatabaseAdapterTest.php | 14 ++++++------- .../Plugin/AbstractUpdatePluginTest.php | 8 +++----- .../Update/Plugin/DatabaseCharsetTest.php | 7 ++----- tests/Opus/UserRoleTest.php | 2 +- 87 files changed, 222 insertions(+), 311 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index d8ecf459..5a1a131b 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -1,4 +1,4 @@ -name: PHP Composer +name: Testing on: push: @@ -12,15 +12,24 @@ on: jobs: build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 + + strategy: + matrix: + php-versions: ['7.1', '7.4', '8.1'] + + name: PHP ${{ matrix.php-versions }} Test steps: - uses: actions/checkout@v3 - - name: Setup PHP 7.1 + - name: Setup PHP ${{ matrix.php-versions }} uses: shivammathur/setup-php@v2 with: - php-version: '7.1' + php-version: ${{ matrix.php-versions }} + + - name: Check PHP Version + run: php -v - name: Update Ubuntu packages run: sudo apt-get update @@ -37,7 +46,7 @@ jobs: - name: Setup OPUS 4 run: ant prepare-workspace prepare-config create-database lint -DdbUserPassword=root -DdbAdminPassword=root - - name: Test + - name: Tests run: php composer.phar test - name: Coding-Style diff --git a/.gitignore b/.gitignore index 6e305863..3cc37921 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ vendor tests/build tests/config.ini tests/config.ini.backup +.phpunit.result.cache # IntelliJ IDEA files .idea diff --git a/composer.json b/composer.json index e2b2ccfd..06013187 100644 --- a/composer.json +++ b/composer.json @@ -14,9 +14,9 @@ "ext-fileinfo": "*", "ext-json": "*", "ext-libxml": "*", - "zendframework/zendframework1": "1.12.*", - "opus4-repo/opus4-common": "4.7.2.x-dev", - "opus4-repo/opus4-doi": "dev-master" + "shardj/zf1-future": "1.21.*", + "opus4-repo/opus4-common": "dev-4.7.2-zf1f", + "opus4-repo/opus4-doi": "dev-4.7.2-zf1f" }, "minimum-stability": "dev", "prefer-stable": true, @@ -31,15 +31,9 @@ } }, "require-dev": { - "phpunit/phpunit": "6.*", - "behat/behat": "3.3.*", - "phpunit/php-invoker": "~1.1", - "phpunit/phpunit-selenium": "4.*", - "phpmd/phpmd" : "2.*", - "sebastian/phpcpd": "*", - "mayflower/php-codebrowser": "*", - "squizlabs/php_codesniffer": "*", - "laminas/laminas-coding-standard": "<2.3", + "phpunit/phpunit": "<9", + "phpmd/phpmd" : "*", + "opus4-repo/codesniffer": "dev-laminas", "phpmetrics/phpmetrics": "*" }, "archive": { diff --git a/library/Opus/Account.php b/library/Opus/Account.php index 086b385e..22f0ed87 100644 --- a/library/Opus/Account.php +++ b/library/Opus/Account.php @@ -343,9 +343,9 @@ public function getDisplayName() */ public function getFullName() { - $name = $this->getFirstName(); + $name = $this->getFirstName() ?? ''; - $lastName = $this->getLastName(); + $lastName = $this->getLastName() ?? ''; if (strlen($name) > 0 && strlen($lastName) > 0) { $name .= ' '; diff --git a/library/Opus/Collection.php b/library/Opus/Collection.php index 02398f1b..abbb8842 100644 --- a/library/Opus/Collection.php +++ b/library/Opus/Collection.php @@ -472,7 +472,7 @@ public function getDisplayName($context = 'browsing', $role = null) $display = $role->getDisplayName(); }*/ - return trim($display); + return trim($display ?? ''); } public function getDisplayNameForBrowsingContext($role = null) diff --git a/library/Opus/Db/TableGateway.php b/library/Opus/Db/TableGateway.php index 5193d5bb..06ef5505 100644 --- a/library/Opus/Db/TableGateway.php +++ b/library/Opus/Db/TableGateway.php @@ -171,14 +171,14 @@ public function deleteWhereArray($data) /** * Singleton classes cannot be cloned! */ - final private function __clone() + private function __clone() { } /** * Singleton classes should not be put to sleep! */ - final private function __sleep() + public function __sleep() { } } diff --git a/library/Opus/Db/Util/DatabaseHelper.php b/library/Opus/Db/Util/DatabaseHelper.php index e8805ab9..ad4a7cf4 100644 --- a/library/Opus/Db/Util/DatabaseHelper.php +++ b/library/Opus/Db/Util/DatabaseHelper.php @@ -25,7 +25,7 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2021, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License */ @@ -42,6 +42,7 @@ */ class DatabaseHelper { + /** @var string[] */ private $tables; public function resetDatabase() diff --git a/library/Opus/Db2/Database.php b/library/Opus/Db2/Database.php index 84808c9c..767f66b9 100644 --- a/library/Opus/Db2/Database.php +++ b/library/Opus/Db2/Database.php @@ -27,10 +27,6 @@ * * @copyright Copyright (c) 2021, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus\Db2 - * @author Jens Schwidder */ namespace Opus\Db2; @@ -47,6 +43,7 @@ */ class Database { + /** @var Connection */ private static $conn; /** diff --git a/library/Opus/DocumentFinder/DefaultDocumentFinder.php b/library/Opus/DocumentFinder/DefaultDocumentFinder.php index 56942c84..49a33d28 100644 --- a/library/Opus/DocumentFinder/DefaultDocumentFinder.php +++ b/library/Opus/DocumentFinder/DefaultDocumentFinder.php @@ -46,6 +46,7 @@ */ class DefaultDocumentFinder implements DocumentFinderInterface { + /** @var DocumentFinder */ private $finder; public function __construct() diff --git a/library/Opus/Identifier/Urn.php b/library/Opus/Identifier/Urn.php index 8d6c7786..f6627484 100644 --- a/library/Opus/Identifier/Urn.php +++ b/library/Opus/Identifier/Urn.php @@ -99,14 +99,14 @@ class Urn public function __construct($nid, $nss) { $nidRegex = '/^[a-zA-Z0-9][a-zA-z0-9\-]+$/'; - if (preg_match($nidRegex, $nid) !== 0) { + if ($nid !== null && preg_match($nidRegex, $nid) !== 0) { $this->nid = $nid; } else { throw new InvalidArgumentException('Used invalid namespace identifier. See RFC 2141.'); } $nssRegex = '/^[a-zA-z0-9\(\)\+,\-\.:=@;\$_!\*\'%\/\?#]+$/'; - if (preg_match($nssRegex, $nss) !== 0) { + if ($nss !== null && preg_match($nssRegex, $nss) !== 0) { $this->nss = $nss; } else { throw new InvalidArgumentException('Used invalid namespace specific string. See RFC 2141.'); @@ -191,7 +191,7 @@ public function getCheckDigit($documentId) // identify last digit, which is the check digit // TODO: (Thoralf) Not supported by every PHP // $check_digit = ($quotient{mb_strlen($quotient)-1}); - return $quotient{strlen($quotient) - 1}; + return $quotient[strlen($quotient) - 1]; } } diff --git a/library/Opus/Model/AbstractDb.php b/library/Opus/Model/AbstractDb.php index 7fe31f57..c379091a 100644 --- a/library/Opus/Model/AbstractDb.php +++ b/library/Opus/Model/AbstractDb.php @@ -33,7 +33,6 @@ use Exception; use InvalidArgumentException; -use Opus\Common\Log; use Opus\Common\Model\ModelException; use Opus\Common\Model\ModelInterface; use Opus\Common\Model\NotFoundException; diff --git a/library/Opus/Model/Field.php b/library/Opus/Model/Field.php index 999e9e40..3b496f79 100644 --- a/library/Opus/Model/Field.php +++ b/library/Opus/Model/Field.php @@ -553,7 +553,7 @@ private function _tryCastValuesToModel(array $values) } catch (Exception $ex) { throw new ModelException( "Failed to cast value '$value' to class '{$this->valueModelClass}'. (Field {$this->name})", - null, + 0, $ex ); } @@ -651,8 +651,8 @@ public function addValue($value) // Check multiplicity constraint if (is_int($this->multiplicity) === true) { if ( - (count($value) > $this->multiplicity) - or ((count($value) + count($this->value)) > $this->multiplicity) + (1 > $this->multiplicity) + || (1 + count($this->value)) > $this->multiplicity ) { throw new InvalidArgumentException( 'Field ' . $this->name . ' cannot hold more then ' . $this->multiplicity . ' values.' diff --git a/library/Opus/Model/Properties.php b/library/Opus/Model/Properties.php index 52782163..9d64be48 100644 --- a/library/Opus/Model/Properties.php +++ b/library/Opus/Model/Properties.php @@ -280,7 +280,7 @@ public function getProperties($model, $type = null) { $adapter = $this->getAdapter(); - if ($type !== null && (is_int($model) || ctype_digit($model))) { + if ($type !== null && (is_int($model) || (is_string($model) && ctype_digit($model)))) { $modelTypeId = $this->getModelTypeId($type); $modelId = $model; } else { @@ -344,7 +344,7 @@ public function removeProperties($model, $type = null) { $adapter = $this->getAdapter(); - if ($type !== null && (is_int($model) || ctype_digit($model))) { + if ($type !== null && (is_int($model) || (is_string($model) && ctype_digit($model)))) { $modelTypeId = $this->getModelTypeId($type); $modelId = $model; } else { diff --git a/library/Opus/Model/Xml.php b/library/Opus/Model/Xml.php index 6a7e146c..b2db7f94 100644 --- a/library/Opus/Model/Xml.php +++ b/library/Opus/Model/Xml.php @@ -292,7 +292,7 @@ private function getDomDocumentFromXmlCache() $logger = Log::get(); if (null === $this->cache) { - $logger->debug(__METHOD__ . ' skipping cache for ' . get_class($model)); + $logger->debug(__METHOD__ . ' skipping cache for ' . ($model !== null ? get_class($model) : 'NULL')); return null; } diff --git a/library/Opus/Model/Xml/AbstractVersion.php b/library/Opus/Model/Xml/AbstractVersion.php index abe01f1d..f327817f 100644 --- a/library/Opus/Model/Xml/AbstractVersion.php +++ b/library/Opus/Model/Xml/AbstractVersion.php @@ -384,7 +384,7 @@ public function getFieldValues($field) $fieldValues = $fieldValues->getName(); } - return trim($fieldValues); + return trim($fieldValues ?? ''); } /** diff --git a/library/Opus/Model/Xml/Version1.php b/library/Opus/Model/Xml/Version1.php index 2428226d..5618cdf2 100644 --- a/library/Opus/Model/Xml/Version1.php +++ b/library/Opus/Model/Xml/Version1.php @@ -97,7 +97,7 @@ protected function createFieldElement($dom, $fieldName, $value) $modelId = $value->getId(); } // Ignore compound keys. - if (false === is_array($modelId)) { + if (false === is_array($modelId) && $modelId !== null) { $childNode->setAttribute('Id', $modelId); } } @@ -119,13 +119,15 @@ protected function createModelNode($dom, $model) $childNode = $dom->createElement($modelClass); - if ($model instanceof PersistableInterface) { - $childNode->setAttribute('Id', $model->getId()); - } elseif ( - $model instanceof Filter && - $model->getModel() instanceof PersistableInterface - ) { - $childNode->setAttribute('Id', $model->getId()); + if ($model->getId() !== null) { + if ($model instanceof PersistableInterface) { + $childNode->setAttribute('Id', $model->getId()); + } elseif ( + $model instanceof Filter && + $model->getModel() instanceof PersistableInterface + ) { + $childNode->setAttribute('Id', $model->getId()); + } } return $childNode; diff --git a/library/Opus/Person.php b/library/Opus/Person.php index b9b9e306..7d2759b9 100644 --- a/library/Opus/Person.php +++ b/library/Opus/Person.php @@ -782,7 +782,7 @@ public function matches($criteria) foreach ($criteria as $fieldName => $critValue) { $value = $this->getField($fieldName)->getValue(); - if (is_string($value)) { + if (is_string($value) && $critValue !== null) { if (stristr($value, $critValue) === false) { return false; } @@ -821,7 +821,7 @@ protected static function addWherePerson($select, $person) $person = array_merge($defaults, $person); foreach ($person as $column => $value) { - if (strlen(trim($value)) > 0) { + if (strlen(trim($value ?? '')) > 0) { $select->where("trim(p.$column) = ?", trim($value)); } else { $select->where("p.$column IS NULL"); diff --git a/library/Opus/Statistic/LocalCounter.php b/library/Opus/Statistic/LocalCounter.php index e16c372e..7481dbf4 100644 --- a/library/Opus/Statistic/LocalCounter.php +++ b/library/Opus/Statistic/LocalCounter.php @@ -79,6 +79,7 @@ class LocalCounter */ private $doubleClickIntervalHtml = 10; + /** @var string[] */ private $spiderList = [ 'Alexandria prototype project', 'Arachmo', diff --git a/library/Opus/Update/Plugin/DatabaseSchema.php b/library/Opus/Update/Plugin/DatabaseSchema.php index 6e2fdc1b..fc28c239 100644 --- a/library/Opus/Update/Plugin/DatabaseSchema.php +++ b/library/Opus/Update/Plugin/DatabaseSchema.php @@ -25,12 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2018, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus - * @author Jens Schwidder */ namespace Opus\Update\Plugin; @@ -45,6 +41,7 @@ */ class DatabaseSchema extends AbstractUpdatePlugin { + /** @var string */ private $targetVersion; /** diff --git a/phpcs.xml b/phpcs.xml index 9f8f9981..4f91b54d 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -1,23 +1,9 @@ - + + + library tests - - - - - - - - - - - - - - - - diff --git a/phpunit.xml b/phpunit.xml index 53252048..1f8c170a 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -8,24 +8,18 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" stopOnFailure="false" - processIsolation="false" - syntaxCheck="true"> + processIsolation="false"> ./tests - + - - - . - - ./library/Opus diff --git a/tests/Opus/AccountTest.php b/tests/Opus/AccountTest.php index 823e08be..85756fa2 100644 --- a/tests/Opus/AccountTest.php +++ b/tests/Opus/AccountTest.php @@ -41,7 +41,7 @@ */ class AccountTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Collection/Plugin/DeleteSubTreeTest.php b/tests/Opus/Collection/Plugin/DeleteSubTreeTest.php index f629b3c3..9e450fe7 100644 --- a/tests/Opus/Collection/Plugin/DeleteSubTreeTest.php +++ b/tests/Opus/Collection/Plugin/DeleteSubTreeTest.php @@ -43,7 +43,7 @@ class DeleteSubTreeTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/CollectionNodeTest.php b/tests/Opus/CollectionNodeTest.php index cf362107..bde70008 100644 --- a/tests/Opus/CollectionNodeTest.php +++ b/tests/Opus/CollectionNodeTest.php @@ -27,10 +27,6 @@ * * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Collection - * @author Thoralf Klein */ namespace OpusTest; @@ -41,22 +37,22 @@ /** * Test cases for class Opus\CollectionNode. - * - * @category Tests - * @package Opus\Collection - * @group CollectionTests */ class CollectionNodeTest extends TestCase { /** @var Opus\CollectionRole */ protected $roleFixture; - protected $roleName = ""; + + /** @var string */ + protected $roleName = ""; + + /** @var string */ protected $roleOaiName = ""; /** @var Opus\CollectionNode */ protected $fixture; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/CollectionRole/Plugin/DeleteTreeTest.php b/tests/Opus/CollectionRole/Plugin/DeleteTreeTest.php index 33786351..f38956df 100644 --- a/tests/Opus/CollectionRole/Plugin/DeleteTreeTest.php +++ b/tests/Opus/CollectionRole/Plugin/DeleteTreeTest.php @@ -25,12 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2013, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\CollectionRole - * @author Edouard Simon (edouard.simon@zib.de) */ namespace OpusTest\CollectionRole\Plugin; @@ -46,7 +42,7 @@ class DeleteTreeTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/CollectionRoleTest.php b/tests/Opus/CollectionRoleTest.php index 5eb096d4..ebb4ea4e 100644 --- a/tests/Opus/CollectionRoleTest.php +++ b/tests/Opus/CollectionRoleTest.php @@ -78,7 +78,7 @@ public static function createRandomObject() /** * Sets up the fixture. Method is called before each test. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -155,7 +155,7 @@ public function testStore() 'CollectionRole isNewRecord check failed on new record.' ); - $roleId = $this->object->store(); + $roleId = ( int )$this->object->store(); $this->assertNotNull( $roleId, 'CollectionRole roleId return value check on stored record.' @@ -709,7 +709,7 @@ public function testPosition() // Check if setPosition works properly. $numRoles = count($collectionRoleRepository->fetchAll()); - $checkPositions = [1, $numRoles, round((1 + $numRoles) / 2), 1]; + $checkPositions = [1, $numRoles, intval(round((1 + $numRoles) / 2)), 1]; foreach ($checkPositions as $position) { $this->object->setPosition($position); @@ -718,7 +718,7 @@ public function testPosition() // Reload object, otherwise the result will be trivial. $role = new CollectionRole($this->object->getId()); $this->assertTrue( - $role->getPosition() === "$position", + $role->getPosition() === $position, 'CollectionRole position check failed.' ); } @@ -757,12 +757,12 @@ public function testRoleData() $this->assertTrue($role->getDisplayBrowsing() === 'Number, Name', 'CollectionRole display_browsing check failed.'); $this->assertTrue($role->getDisplayFrontdoor() === 'Name', 'CollectionRole display_frontdoor check failed.'); - $this->assertTrue($role->getVisible() === '1', 'CollectionRole visible check failed.'); - $this->assertTrue($role->getVisibleBrowsingStart() === '1', 'CollectionRole visible_browsing_start check failed.'); - $this->assertTrue($role->getVisibleFrontdoor() === '0', 'CollectionRole visible_frontdoor check failed.'); - $this->assertTrue($role->getVisibleOai() === '1', 'CollectionRole visible_oai check failed.'); + $this->assertTrue($role->getVisible() === 1, 'CollectionRole visible check failed.'); + $this->assertTrue($role->getVisibleBrowsingStart() === 1, 'CollectionRole visible_browsing_start check failed.'); + $this->assertTrue($role->getVisibleFrontdoor() === 0, 'CollectionRole visible_frontdoor check failed.'); + $this->assertTrue($role->getVisibleOai() === 1, 'CollectionRole visible_oai check failed.'); - $this->assertTrue($role->getPosition() === '1', 'CollectionRole position check failed.'); + $this->assertTrue($role->getPosition() === 1, 'CollectionRole position check failed.'); } /** diff --git a/tests/Opus/CollectionTest.php b/tests/Opus/CollectionTest.php index bd92a784..26be0a72 100644 --- a/tests/Opus/CollectionTest.php +++ b/tests/Opus/CollectionTest.php @@ -60,7 +60,11 @@ class CollectionTest extends TestCase { /** @var CollectionRoleInterface */ protected $roleFixture; - protected $roleName = ""; + + /** @var string */ + protected $roleName = ""; + + /** @var string */ protected $roleOaiName = ""; /** @var CollectionInterface */ @@ -69,7 +73,7 @@ class CollectionTest extends TestCase /** * SetUp method. Inherits database cleanup from parent. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -90,7 +94,7 @@ public function setUp() $this->roleFixture->store(); } - protected function tearDown() + protected function tearDown(): void { if (is_object($this->roleFixture)) { $this->roleFixture->delete(); diff --git a/tests/Opus/DateTest.php b/tests/Opus/DateTest.php index ed3dd4c0..f53c92ef 100644 --- a/tests/Opus/DateTest.php +++ b/tests/Opus/DateTest.php @@ -46,12 +46,10 @@ */ class DateTest extends TestCase { - protected $localeBackup; - /** * Prepare german locale setup. */ - public function setUp() + public function setUp(): void { parent::setUp(); Zend_Locale::setDefault('de'); diff --git a/tests/Opus/Db/Adapter/Pdo/Mysqlutf8Test.php b/tests/Opus/Db/Adapter/Pdo/Mysqlutf8Test.php index 567b3c97..e9903ef9 100644 --- a/tests/Opus/Db/Adapter/Pdo/Mysqlutf8Test.php +++ b/tests/Opus/Db/Adapter/Pdo/Mysqlutf8Test.php @@ -41,6 +41,7 @@ use Opus\Common\Config; use OpusTest\TestAsset\TestCase; use Zend_Db; +use Zend_Db_Adapter_Abstract; use Zend_Db_Table; /** @@ -52,12 +53,12 @@ */ class Mysqlutf8Test extends TestCase { - /** @var\Zend_Db_Adapter_Abstract */ + /** @var Zend_Db_Adapter_Abstract */ protected $dbaBackup; /** Ensure a clean database table. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -78,7 +79,7 @@ public function setUp() /** * Tear down database changed. */ - public function tearDown() + public function tearDown(): void { // Close connection for clean transaction state. $dba = Zend_Db_Table::getDefaultAdapter(); diff --git a/tests/Opus/Db/InstanciateGatewayTest.php b/tests/Opus/Db/InstanciateGatewayTest.php index ecf69327..8137a4b2 100644 --- a/tests/Opus/Db/InstanciateGatewayTest.php +++ b/tests/Opus/Db/InstanciateGatewayTest.php @@ -25,13 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2020, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Db - * @author Ralf Claussnitzer - * @author Thoralf Klein */ namespace OpusTest\Db; @@ -82,11 +77,11 @@ class InstanciateGatewayTest extends TestCase /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/DnbInstituteTest.php b/tests/Opus/DnbInstituteTest.php index dff15e59..576bb631 100644 --- a/tests/Opus/DnbInstituteTest.php +++ b/tests/Opus/DnbInstituteTest.php @@ -1,6 +1,6 @@ */ namespace OpusTest\Document\Plugin; @@ -50,7 +45,7 @@ class IdentifierUrnTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Document/Plugin/SequenceNumberTest.php b/tests/Opus/Document/Plugin/SequenceNumberTest.php index e27f9c16..66af7c98 100644 --- a/tests/Opus/Document/Plugin/SequenceNumberTest.php +++ b/tests/Opus/Document/Plugin/SequenceNumberTest.php @@ -43,7 +43,7 @@ class SequenceNumberTest extends TestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); Config::get()->merge(new Zend_Config([ diff --git a/tests/Opus/Document/Plugin/XmlCacheTest.php b/tests/Opus/Document/Plugin/XmlCacheTest.php index 869ead40..504da711 100644 --- a/tests/Opus/Document/Plugin/XmlCacheTest.php +++ b/tests/Opus/Document/Plugin/XmlCacheTest.php @@ -59,7 +59,7 @@ class XmlCacheTest extends TestCase */ private $cacheTable; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/DocumentFinderTest.php b/tests/Opus/DocumentFinderTest.php index bf0e3dc4..23efd084 100644 --- a/tests/Opus/DocumentFinderTest.php +++ b/tests/Opus/DocumentFinderTest.php @@ -55,14 +55,10 @@ /** * Test cases for class Opus\DocumentFinder. - * - * @package Opus - * @category Tests - * @group DocumentTest */ class DocumentFinderTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/DocumentRepositoryTest.php b/tests/Opus/DocumentRepositoryTest.php index 76048af4..ba13bf47 100644 --- a/tests/Opus/DocumentRepositoryTest.php +++ b/tests/Opus/DocumentRepositoryTest.php @@ -41,7 +41,7 @@ class DocumentRepositoryTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/DocumentTest.php b/tests/Opus/DocumentTest.php index 0aee4a56..abe19a2f 100644 --- a/tests/Opus/DocumentTest.php +++ b/tests/Opus/DocumentTest.php @@ -109,7 +109,7 @@ class DocumentTest extends TestCase /** * Set up test fixture. */ - public function setUp() + public function setUp(): void { // Set up a mock language list. $list = ['de' => 'Test_Deutsch', 'en' => 'Test_Englisch', 'fr' => 'Test_Französisch']; @@ -120,7 +120,7 @@ public function setUp() $this->clearTables(false); } - public function tearDown() + public function tearDown(): void { $document = new Document(); $document->setDefaultPlugins(null); diff --git a/tests/Opus/Doi/DataCiteXmlGeneratorTest.php b/tests/Opus/Doi/DataCiteXmlGeneratorTest.php index b17a19ba..989608aa 100644 --- a/tests/Opus/Doi/DataCiteXmlGeneratorTest.php +++ b/tests/Opus/Doi/DataCiteXmlGeneratorTest.php @@ -66,11 +66,16 @@ class DataCiteXmlGeneratorTest extends TestCase { - protected $srcPath = ''; + /** @var string */ + protected $srcPath = ''; + + /** @var string */ protected $destPath = ''; - protected $path = ''; - public function setUp() + /** @var string */ + protected $path = ''; + + public function setUp(): void { parent::setUp(); @@ -124,7 +129,7 @@ public function setUp() ])); } - public function tearDown() + public function tearDown(): void { FileUtil::deleteDirectory($this->path); diff --git a/tests/Opus/Doi/DoiMailNotificationTest.php b/tests/Opus/Doi/DoiMailNotificationTest.php index f24f6e6e..248c6449 100644 --- a/tests/Opus/Doi/DoiMailNotificationTest.php +++ b/tests/Opus/Doi/DoiMailNotificationTest.php @@ -46,9 +46,10 @@ */ class DoiMailNotificationTest extends TestCase { + /** @var DoiMailNotification */ private $doiMailNotification; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Doi/DoiManagerDataCiteTest.php b/tests/Opus/Doi/DoiManagerDataCiteTest.php index b5e41dde..a9a23a04 100644 --- a/tests/Opus/Doi/DoiManagerDataCiteTest.php +++ b/tests/Opus/Doi/DoiManagerDataCiteTest.php @@ -45,7 +45,7 @@ class DoiManagerDataCiteTest extends TestCase /** * TODO determine 'skipping' based on configuration (environment) */ - public function setUp() + public function setUp(): void { parent::setUp(); $this->markTestSkipped( diff --git a/tests/Opus/Doi/DoiManagerTest.php b/tests/Opus/Doi/DoiManagerTest.php index b025d411..d04f05ea 100644 --- a/tests/Opus/Doi/DoiManagerTest.php +++ b/tests/Opus/Doi/DoiManagerTest.php @@ -56,7 +56,7 @@ class DoiManagerTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Doi/UserRecipientProviderTest.php b/tests/Opus/Doi/UserRecipientProviderTest.php index 51c35a64..6f00d1e4 100644 --- a/tests/Opus/Doi/UserRecipientProviderTest.php +++ b/tests/Opus/Doi/UserRecipientProviderTest.php @@ -38,7 +38,7 @@ class UserRecipientProviderTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/EnrichmentKeyTest.php b/tests/Opus/EnrichmentKeyTest.php index b78b7baa..280e3b70 100644 --- a/tests/Opus/EnrichmentKeyTest.php +++ b/tests/Opus/EnrichmentKeyTest.php @@ -53,7 +53,7 @@ class EnrichmentKeyTest extends TestCase /** @var EnrichmentKeyInterface */ private $referencedEnrichmentKey; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/EnrichmentTest.php b/tests/Opus/EnrichmentTest.php index 37d9ff6a..b61b6aff 100644 --- a/tests/Opus/EnrichmentTest.php +++ b/tests/Opus/EnrichmentTest.php @@ -61,7 +61,7 @@ class EnrichmentTest extends TestCase /** @var EnrichmentKeyInterface */ private $anotherenrichmentkey; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/File/Plugin/DefaultAccessTest.php b/tests/Opus/File/Plugin/DefaultAccessTest.php index b171d4b0..46445ed1 100644 --- a/tests/Opus/File/Plugin/DefaultAccessTest.php +++ b/tests/Opus/File/Plugin/DefaultAccessTest.php @@ -47,7 +47,7 @@ */ class DefaultAccessTest extends TestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); diff --git a/tests/Opus/FileTest.php b/tests/Opus/FileTest.php index a20d69d0..803c8b06 100644 --- a/tests/Opus/FileTest.php +++ b/tests/Opus/FileTest.php @@ -27,11 +27,6 @@ * * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) - * @author Thoralf Klein */ namespace OpusTest; @@ -77,15 +72,17 @@ */ class FileTest extends TestCase { + /** @var string */ protected $srcPath = ''; + /** @var string */ protected $destPath = ''; /** * Clear test tables and establish directories * for filesystem tests in /tmp. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -112,7 +109,7 @@ public function setUp() * * Roll back global configuration changes. */ - public function tearDown() + public function tearDown(): void { FileUtil::deleteDirectory($this->srcPath); FileUtil::deleteDirectory($this->destPath); @@ -584,7 +581,7 @@ public function testInvalidHashAlgorithmAfterStore() $file = $doc->getFile(0); $doc->store(); - $this->expectException('Exception'); // TODO broken for PHPunit 3.6 + $this->expectException('ValueError'); // TODO broken for PHPunit 3.6 $file->getRealHash('md23'); } diff --git a/tests/Opus/HashValuesTest.php b/tests/Opus/HashValuesTest.php index c61092e5..36d1f818 100644 --- a/tests/Opus/HashValuesTest.php +++ b/tests/Opus/HashValuesTest.php @@ -26,12 +26,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2010-2020, OPUS 4 development team + * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Framework - * @package Opus - * @author Thoralf Klein */ namespace OpusTest; @@ -53,7 +49,7 @@ class HashValuesTest extends TestCase * * @access protected */ - protected function setUp() + protected function setUp(): void { $this->object = new HashValues(); } @@ -64,7 +60,7 @@ protected function setUp() * * @access protected */ - protected function tearDown() + protected function tearDown(): void { } diff --git a/tests/Opus/Identifier/UrnTest.php b/tests/Opus/Identifier/UrnTest.php index 9f49ec88..df201bd7 100644 --- a/tests/Opus/Identifier/UrnTest.php +++ b/tests/Opus/Identifier/UrnTest.php @@ -53,11 +53,11 @@ class UrnTest extends TestCase /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/IdentifierTest.php b/tests/Opus/IdentifierTest.php index fd719836..d37b50c7 100644 --- a/tests/Opus/IdentifierTest.php +++ b/tests/Opus/IdentifierTest.php @@ -50,7 +50,7 @@ class IdentifierTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/IprangeTest.php b/tests/Opus/IprangeTest.php index f29ce6ba..1314e1b0 100644 --- a/tests/Opus/IprangeTest.php +++ b/tests/Opus/IprangeTest.php @@ -43,7 +43,7 @@ class IprangeTest extends TestCase */ protected $object; - protected function setUp() + protected function setUp(): void { parent::setUp(); $this->clearTables(false, ['ipranges']); diff --git a/tests/Opus/JobTest.php b/tests/Opus/JobTest.php index 7b7585ac..cd6eec62 100644 --- a/tests/Opus/JobTest.php +++ b/tests/Opus/JobTest.php @@ -38,7 +38,7 @@ */ class JobTest extends TestCase { - public function tearDown() + public function tearDown(): void { Job::deleteAll(); parent::tearDown(); diff --git a/tests/Opus/LanguageTest.php b/tests/Opus/LanguageTest.php index 6784351e..f3285524 100644 --- a/tests/Opus/LanguageTest.php +++ b/tests/Opus/LanguageTest.php @@ -44,7 +44,7 @@ class LanguageTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/LicenceTest.php b/tests/Opus/LicenceTest.php index d7f78ea2..3c03f4cb 100644 --- a/tests/Opus/LicenceTest.php +++ b/tests/Opus/LicenceTest.php @@ -48,14 +48,10 @@ /** * Test cases for class Opus\Licence. - * - * @package Opus - * @category Tests - * @group LicenceTest */ class LicenceTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Model/AbstractDbTest.php b/tests/Opus/Model/AbstractDbTest.php index f949f592..9ad66b81 100644 --- a/tests/Opus/Model/AbstractDbTest.php +++ b/tests/Opus/Model/AbstractDbTest.php @@ -88,7 +88,7 @@ public function abstractDataSetDataProvider() /** * Prepare the Database. */ - public function setUp() + public function setUp(): void { $dba = Zend_Db_Table::getDefaultAdapter(); $dba->query('DROP TABLE IF EXISTS testtable'); @@ -112,7 +112,7 @@ public function setUp() /** * Remove temporary table. */ - public function tearDown() + public function tearDown(): void { $dba = Zend_Db_Table::getDefaultAdapter(); $dba->query('DROP TABLE IF EXISTS testtable'); diff --git a/tests/Opus/Model/Dependent/AbstractDependentModelTest.php b/tests/Opus/Model/Dependent/AbstractDependentModelTest.php index 68a9eda6..acd6775c 100644 --- a/tests/Opus/Model/Dependent/AbstractDependentModelTest.php +++ b/tests/Opus/Model/Dependent/AbstractDependentModelTest.php @@ -37,7 +37,10 @@ use Opus\Model\Dependent\AbstractDependentModel; use Opus\Model\Plugin\InvalidateDocumentCache; use OpusTest\TestAsset\TestCase; +use Zend_Db_Adapter; +use Zend_Db_Table; use Zend_Db_Table_Abstract; +use Zend_Db_Table_Row; use function class_exists; @@ -60,28 +63,28 @@ class AbstractDependentModelTest extends TestCase /** * \Zend_Db_Table mockup. * - * @var\Zend_Db_Table + * @var Zend_Db_Table */ private $mockTableGateway; /** * \Zend_Db_Table_Row mockup * - * @var\Zend_Db_Table_Row + * @var Zend_Db_Table_Row */ private $mockTableRow; /** * \Zend_Db_Adapter mockup * - * @var\Zend_Db_Adapter + * @var Zend_Db_Adapter */ private $mockAdapter; /** * Set up test instance and mock environment. */ - public function setUp() + public function setUp(): void { if (false === class_exists('Opus_Model_Dependent_AbstractTest_MockTableGateway', false)) { eval(' @@ -168,7 +171,7 @@ public function info($key = null) { /** * Overwrite parent methods. */ - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/Model/Dependent/Link/AbstractLinkModelTest.php b/tests/Opus/Model/Dependent/Link/AbstractLinkModelTest.php index bd621267..e279430d 100644 --- a/tests/Opus/Model/Dependent/Link/AbstractLinkModelTest.php +++ b/tests/Opus/Model/Dependent/Link/AbstractLinkModelTest.php @@ -25,13 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2019, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Model - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) - * @author Thoralf Klein */ namespace OpusTest\Model\Dependent\Link; @@ -48,21 +43,17 @@ /** * Test cases for Opus\Model\Dependent\Link\AbstractLinkModel - * - * @category Tests - * @package Opus\Model - * @group DependentLinkAbstractTest */ class AbstractLinkModelTest extends TestCase { /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/Model/Dependent/Link/AbstractModelMock.php b/tests/Opus/Model/Dependent/Link/AbstractModelMock.php index 9aa1a6a6..314b3819 100644 --- a/tests/Opus/Model/Dependent/Link/AbstractModelMock.php +++ b/tests/Opus/Model/Dependent/Link/AbstractModelMock.php @@ -27,10 +27,6 @@ * * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Model - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) */ namespace OpusTest\Model\Dependent\Link; @@ -39,12 +35,10 @@ /** * Mock model. - * - * @category Tests - * @package Opus\Model */ class AbstractModelMock extends AbstractModel { + /** @var bool */ private $mockValid = true; /** diff --git a/tests/Opus/Model/FieldTest.php b/tests/Opus/Model/FieldTest.php index d4721ce8..6e269862 100644 --- a/tests/Opus/Model/FieldTest.php +++ b/tests/Opus/Model/FieldTest.php @@ -58,11 +58,11 @@ class FieldTest extends TestCase /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/Model/FilterTest.php b/tests/Opus/Model/FilterTest.php index 5014a651..d490cf66 100644 --- a/tests/Opus/Model/FilterTest.php +++ b/tests/Opus/Model/FilterTest.php @@ -25,13 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2010, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Model - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) - * @author Thoralf Klein */ namespace OpusTest\Model; @@ -48,10 +43,6 @@ /** * Test cases for class Opus\Model\Filter. - * - * @package Opus\Model - * @category Tests - * @group FilterTest */ class FilterTest extends TestCase { @@ -69,7 +60,7 @@ class FilterTest extends TestCase */ protected $filter; - public function setUp() + public function setUp(): void { // TODO NAMESPACE is this good code? does it work? @@ -98,7 +89,7 @@ protected function init() { /** * Overwrite parent methods. */ - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/Model/Mock/AbstractDbMock.php b/tests/Opus/Model/Mock/AbstractDbMock.php index 5ea16c53..12bfd19d 100644 --- a/tests/Opus/Model/Mock/AbstractDbMock.php +++ b/tests/Opus/Model/Mock/AbstractDbMock.php @@ -46,6 +46,7 @@ */ class AbstractDbMock extends AbstractDb { + /** @var bool */ public $postStoreHasBeenCalled = false; /** diff --git a/tests/Opus/Model/Mock/ModelDependentMock.php b/tests/Opus/Model/Mock/ModelDependentMock.php index d610c7dd..4c4b1a84 100644 --- a/tests/Opus/Model/Mock/ModelDependentMock.php +++ b/tests/Opus/Model/Mock/ModelDependentMock.php @@ -45,12 +45,16 @@ */ class ModelDependentMock extends AbstractDependentModel { + /** @var bool */ public $deleteHasBeenCalled = false; + /** @var bool */ public $doDeleteHasBeenCalled = false; + /** @var bool */ public $setParentIdHasBeenCalled = false; + /** @var int */ public $id; public function __construct() diff --git a/tests/Opus/Model/Plugin/AbstractPluginMock.php b/tests/Opus/Model/Plugin/AbstractPluginMock.php index 8e414460..abdaa3ed 100644 --- a/tests/Opus/Model/Plugin/AbstractPluginMock.php +++ b/tests/Opus/Model/Plugin/AbstractPluginMock.php @@ -25,7 +25,7 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2022, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License */ @@ -36,9 +36,7 @@ class AbstractPluginMock extends AbstractPlugin { - /** - * Array containing names of methods that have been called - */ + /** @var string[] Array containing names of methods that have been called */ public $calledHooks = []; /** diff --git a/tests/Opus/Model/Plugin/InvalidateDocumentCacheTest.php b/tests/Opus/Model/Plugin/InvalidateDocumentCacheTest.php index 27810323..aaf3f655 100644 --- a/tests/Opus/Model/Plugin/InvalidateDocumentCacheTest.php +++ b/tests/Opus/Model/Plugin/InvalidateDocumentCacheTest.php @@ -36,6 +36,7 @@ namespace OpusTest\Model\Plugin; use DOMXPath; +use Opus\Collection; use Opus\CollectionRole; use Opus\Common\Model\ModelException; use Opus\Common\Patent; @@ -54,18 +55,16 @@ /** * Plugin creating and deleting xml cache entries. - * - * @uses Opus\Model\Plugin\AbstractPlugin - * - * @category Framework - * @package Opus\Document\Plugin */ class InvalidateDocumentCacheTest extends TestCase { + /** @var Collection */ protected $collection; + + /** @var CollectionRole */ protected $collectionRole; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Model/PropertiesTest.php b/tests/Opus/Model/PropertiesTest.php index e876cb69..919e8d10 100644 --- a/tests/Opus/Model/PropertiesTest.php +++ b/tests/Opus/Model/PropertiesTest.php @@ -49,9 +49,10 @@ */ class PropertiesTest extends TestCase { + /** @var Properties */ protected $properties; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Model/Xml/CacheTest.php b/tests/Opus/Model/Xml/CacheTest.php index 4fa1da1f..1610867d 100644 --- a/tests/Opus/Model/Xml/CacheTest.php +++ b/tests/Opus/Model/Xml/CacheTest.php @@ -44,9 +44,6 @@ /** * Search Hit model class. - * - * @category Framework - * @package Opus\Model\Xml */ class CacheTest extends TestCase { @@ -64,7 +61,7 @@ class CacheTest extends TestCase */ private $maxEntries = 5; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Model/Xml/Version1Test.php b/tests/Opus/Model/Xml/Version1Test.php index 1b855586..b5b5f18c 100644 --- a/tests/Opus/Model/Xml/Version1Test.php +++ b/tests/Opus/Model/Xml/Version1Test.php @@ -55,21 +55,17 @@ /** * Test creation XML (version1) from models and creation of models by valid XML respectivly. - * - * @category Tests - * @package Opus\Model - * @group XmlVersion1Test */ class Version1Test extends TestCase { /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } @@ -268,7 +264,7 @@ public function xmlModelDataProvider() public function testCreateFromXml($xml, $model, $msg) { $xmlHelper = new Xml(); - if ($xml instanceof DomDocument) { + if ($xml instanceof DOMDocument) { $xmlHelper->setDomDocument($xml); } else { $xmlHelper->setXml($xml); diff --git a/tests/Opus/Model/Xml/Version2Test.php b/tests/Opus/Model/Xml/Version2Test.php index 4f59772c..fa61c2b4 100644 --- a/tests/Opus/Model/Xml/Version2Test.php +++ b/tests/Opus/Model/Xml/Version2Test.php @@ -53,11 +53,11 @@ class Version2Test extends TestCase /** * Overwrite parent methods. */ - public function setUp() + public function setUp(): void { } - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/ModelFactoryTest.php b/tests/Opus/ModelFactoryTest.php index 651fcb6e..4e66a467 100644 --- a/tests/Opus/ModelFactoryTest.php +++ b/tests/Opus/ModelFactoryTest.php @@ -42,7 +42,7 @@ class ModelFactoryTest extends TestCase { - public function tearDown() + public function tearDown(): void { $this->clearTables(false, ['documents']); diff --git a/tests/Opus/PermissionTest.php b/tests/Opus/PermissionTest.php index 05a9277d..8ac545f7 100644 --- a/tests/Opus/PermissionTest.php +++ b/tests/Opus/PermissionTest.php @@ -39,7 +39,7 @@ class PermissionTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/PersonTest.php b/tests/Opus/PersonTest.php index b3e57fcd..98b98afb 100644 --- a/tests/Opus/PersonTest.php +++ b/tests/Opus/PersonTest.php @@ -57,10 +57,6 @@ /** * Test cases for class Opus\Person. - * - * @package Opus - * @category Tests - * @group PersonTest */ class PersonTest extends TestCase { @@ -81,7 +77,7 @@ class PersonTest extends TestCase /** * Set up test data documents and persons. */ - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/RequireTest.php b/tests/Opus/RequireTest.php index d6918c1b..4df8b00b 100644 --- a/tests/Opus/RequireTest.php +++ b/tests/Opus/RequireTest.php @@ -56,14 +56,14 @@ class RequireTest extends TestCase * Overwrite standard setUp method, no database connection needed. Will * create a file listing of class files instead. */ - public function setUp() + public function setUp(): void { } /** * Overwrite standard tearDown method, no cleanup needed. */ - public function tearDown() + public function tearDown(): void { } diff --git a/tests/Opus/Security/AccountTest.php b/tests/Opus/Security/AccountTest.php index 81000f0b..a8518a4d 100644 --- a/tests/Opus/Security/AccountTest.php +++ b/tests/Opus/Security/AccountTest.php @@ -36,6 +36,7 @@ use Opus\Db\Accounts; use Opus\Db\TableGateway; use OpusTest\TestAsset\TestCase; +use Zend_Db_Table; use function sha1; @@ -47,14 +48,14 @@ class AccountTest extends TestCase /** * Table adapter to accounts table. * - * @var\Zend_Db_Table + * @var Zend_Db_Table */ protected $accounts; /** * Set up table adapter. */ - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Security/RealmTest.php b/tests/Opus/Security/RealmTest.php index 036bf2e1..eb8238b2 100644 --- a/tests/Opus/Security/RealmTest.php +++ b/tests/Opus/Security/RealmTest.php @@ -58,7 +58,7 @@ */ class RealmTest extends TestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -77,7 +77,7 @@ protected function setUp() ]); } - protected function tearDown() + protected function tearDown(): void { parent::tearDown(); } diff --git a/tests/Opus/SeriesTest.php b/tests/Opus/SeriesTest.php index f21c936b..5ea9b911 100644 --- a/tests/Opus/SeriesTest.php +++ b/tests/Opus/SeriesTest.php @@ -48,7 +48,7 @@ class SeriesTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); @@ -389,7 +389,7 @@ public function testAssignVisibleStatus() $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getVisible() === '1'); + $this->assertTrue($s->getVisible() === 1); $s = Series::get($s->getId()); $s->setVisible('0'); diff --git a/tests/Opus/Statistic/LocalCounterTest.php b/tests/Opus/Statistic/LocalCounterTest.php index 2038c053..579f7c91 100644 --- a/tests/Opus/Statistic/LocalCounterTest.php +++ b/tests/Opus/Statistic/LocalCounterTest.php @@ -25,14 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2008-2020, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Statistic - * @author Ralf Claußnitzer (ralf.claussnitzer@slub-dresden.de) - * @author Thoralf Klein - * @author Jens Schwidder */ namespace OpusTest\Statistic; @@ -50,10 +44,6 @@ /** * Test for Opus\Statistic\LocalCounter. - * - * @package Opus\Statistic - * @category Tests - * @group LocalCounterTest */ class LocalCounterTest extends TestCase { @@ -68,7 +58,7 @@ class LocalCounterTest extends TestCase * Provide clean documents and statistics table and remove temporary files. * Create document for counting. */ - public function setUp() + public function setUp(): void { parent::setUp(); @@ -90,7 +80,7 @@ public function setUp() /** * Clean up tables, remove temporary files. */ - public function tearDown() + public function tearDown(): void { parent::tearDown(); diff --git a/tests/Opus/SubjectTest.php b/tests/Opus/SubjectTest.php index 27230e51..79316f78 100644 --- a/tests/Opus/SubjectTest.php +++ b/tests/Opus/SubjectTest.php @@ -38,7 +38,7 @@ class SubjectTest extends TestCase { - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/TestAsset/AbstractSimpleTestCase.php b/tests/Opus/TestAsset/AbstractSimpleTestCase.php index 5ee49c32..e2982f4b 100644 --- a/tests/Opus/TestAsset/AbstractSimpleTestCase.php +++ b/tests/Opus/TestAsset/AbstractSimpleTestCase.php @@ -47,6 +47,7 @@ */ abstract class AbstractSimpleTestCase extends TestCase { + /** @var Zend_Config */ private $configBackup; // TODO get rid of this two constants - filter_var() can handle it @@ -102,7 +103,7 @@ protected function dropDeprecatedConfiguration() ); } - protected function setUp() + protected function setUp(): void { parent::setUp(); @@ -115,7 +116,7 @@ protected function setUp() } } - protected function tearDown() + protected function tearDown(): void { Config::set($this->configBackup); diff --git a/tests/Opus/TestAsset/DocumentBasedTestCase.php b/tests/Opus/TestAsset/DocumentBasedTestCase.php index 7bdf6ee8..7eb9aa5c 100644 --- a/tests/Opus/TestAsset/DocumentBasedTestCase.php +++ b/tests/Opus/TestAsset/DocumentBasedTestCase.php @@ -59,8 +59,10 @@ class DocumentBasedTestCase extends TestCase { + /** @var array */ private $created = []; + /** @var array[] */ protected static $documentPropertySets = [ 'article' => [ 'Type' => 'article', @@ -306,7 +308,7 @@ public function addFileToDocument(Document $document, $filename, $label, $visibl /** * Manages to delete all documents created in a test run. */ - public function tearDown() + public function tearDown(): void { parent::tearDown(); diff --git a/tests/Opus/TestAsset/FileMock.php b/tests/Opus/TestAsset/FileMock.php index 717fc987..84aae54e 100644 --- a/tests/Opus/TestAsset/FileMock.php +++ b/tests/Opus/TestAsset/FileMock.php @@ -25,12 +25,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2010-2019, OPUS 4 development team + * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\File - * @author Thoralf Klein */ namespace OpusTest\TestAsset; @@ -39,8 +35,10 @@ class FileMock extends File { + /** @var bool */ private $newRecord; + /** @var int */ private $fileId; /** diff --git a/tests/Opus/TestAsset/LoggerMock.php b/tests/Opus/TestAsset/LoggerMock.php index 1a1b4614..13a31003 100644 --- a/tests/Opus/TestAsset/LoggerMock.php +++ b/tests/Opus/TestAsset/LoggerMock.php @@ -25,18 +25,15 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2010-2019, OPUS 4 development team + * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\File - * @author Thoralf Klein */ namespace OpusTest\TestAsset; class LoggerMock { + /** @var string[] */ private $messages = []; /** diff --git a/tests/Opus/TestAsset/TestCase.php b/tests/Opus/TestAsset/TestCase.php index 0262fbe9..4dade7e6 100644 --- a/tests/Opus/TestAsset/TestCase.php +++ b/tests/Opus/TestAsset/TestCase.php @@ -135,7 +135,7 @@ protected function clearFiles($directory = null) /** * Standard setUp method for clearing database. */ - protected function setUp() + protected function setUp(): void { parent::setUp(); } diff --git a/tests/Opus/Translate/DaoTest.php b/tests/Opus/Translate/DaoTest.php index 9ab82b34..09204beb 100644 --- a/tests/Opus/Translate/DaoTest.php +++ b/tests/Opus/Translate/DaoTest.php @@ -1,6 +1,6 @@ * @copyright Copyright (c) 2018, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License */ @@ -44,16 +41,17 @@ */ class DaoTest extends TestCase { + /** @var Dao */ private $translations; - public function setUp() + public function setUp(): void { parent::setUp(); $this->translations = new Dao(); } - public function tearDown() + public function tearDown(): void { $this->translations->removeAll(); diff --git a/tests/Opus/Translate/DatabaseAdapterTest.php b/tests/Opus/Translate/DatabaseAdapterTest.php index 36ccdf36..d3ca253c 100644 --- a/tests/Opus/Translate/DatabaseAdapterTest.php +++ b/tests/Opus/Translate/DatabaseAdapterTest.php @@ -1,6 +1,6 @@ - * @copyright Copyright (c) 2018-2021, OPUS 4 development team + * @copyright Copyright (c) 2018, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License */ @@ -37,22 +34,25 @@ use Opus\Translate\Dao; use Opus\Translate\DatabaseAdapter; use OpusTest\TestAsset\TestCase; +use Zend_Cache_Core; use Zend_Translate; class DatabaseAdapterTest extends TestCase { + /** @var Zend_Cache_Core */ private $cache; + /** @var Dao */ private $translations; - public function setUp() + public function setUp(): void { parent::setUp(); $this->cache = Zend_Translate::getCache(); $this->translations = new Dao(); } - public function tearDown() + public function tearDown(): void { Zend_Translate::setCache($this->cache); parent::tearDown(); diff --git a/tests/Opus/Update/Plugin/AbstractUpdatePluginTest.php b/tests/Opus/Update/Plugin/AbstractUpdatePluginTest.php index 762e9226..8bf83c08 100644 --- a/tests/Opus/Update/Plugin/AbstractUpdatePluginTest.php +++ b/tests/Opus/Update/Plugin/AbstractUpdatePluginTest.php @@ -27,24 +27,22 @@ * * @copyright Copyright (c) 2017, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Update - * @author Jens Schwidder */ namespace OpusTest\Update\Plugin; use Opus\Update\Plugin\AbstractUpdatePlugin; use OpusTest\TestAsset\TestCase; +use PHPUnit\Framework\MockObject\MockObject; use const PHP_EOL; class AbstractUpdatePluginTest extends TestCase { + /** @var MockObject */ private $stub; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/Update/Plugin/DatabaseCharsetTest.php b/tests/Opus/Update/Plugin/DatabaseCharsetTest.php index b44277fe..da32cfda 100644 --- a/tests/Opus/Update/Plugin/DatabaseCharsetTest.php +++ b/tests/Opus/Update/Plugin/DatabaseCharsetTest.php @@ -27,10 +27,6 @@ * * @copyright Copyright (c) 2018, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus\Update\Plugin - * @author Jens Schwidder */ namespace OpusTest\Update\Plugin; @@ -40,9 +36,10 @@ class DatabaseCharsetTest extends TestCase { + /** @var DatabaseCharset */ private $plugin; - public function setUp() + public function setUp(): void { parent::setUp(); diff --git a/tests/Opus/UserRoleTest.php b/tests/Opus/UserRoleTest.php index 670a8c12..a460cecf 100644 --- a/tests/Opus/UserRoleTest.php +++ b/tests/Opus/UserRoleTest.php @@ -47,7 +47,7 @@ */ class UserRoleTest extends TestCase { - protected function setUp() + protected function setUp(): void { parent::setUp(); From 42d768a585da8a6c1a557b653c100201e7b94fca Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 17:11:09 +0100 Subject: [PATCH 15/46] #320 Test fixes for PHP 8 --- library/Opus/Document.php | 2 +- library/Opus/Model/DatabaseTrait.php | 3 ++- library/Opus/Model/Field.php | 8 +++++--- library/Opus/Model/Xml/AbstractVersion.php | 2 +- library/Opus/Model/Xml/Version1.php | 20 +++++++++++-------- library/Opus/Series.php | 2 +- tests/Opus/DatabaseTest.php | 7 +++---- .../Dependent/Link/DocumentSeriesTest.php | 16 +++++++-------- tests/Opus/Model/FieldTest.php | 2 +- tests/Opus/Model/Xml/Version1Test.php | 4 +--- tests/Opus/SeriesTest.php | 6 +++--- 11 files changed, 38 insertions(+), 34 deletions(-) diff --git a/library/Opus/Document.php b/library/Opus/Document.php index 201c7f91..397a5d80 100644 --- a/library/Opus/Document.php +++ b/library/Opus/Document.php @@ -1061,7 +1061,7 @@ public function hasFulltext() $files = $this->getFile(); $files = array_filter($files, function ($file) { - return $file->getVisibleInFrontdoor() === '1'; + return $file->getVisibleInFrontdoor() === 1; }); return count($files) > 0; diff --git a/library/Opus/Model/DatabaseTrait.php b/library/Opus/Model/DatabaseTrait.php index c080352d..0fd7a48d 100644 --- a/library/Opus/Model/DatabaseTrait.php +++ b/library/Opus/Model/DatabaseTrait.php @@ -123,7 +123,8 @@ protected function initDatabase($id = null, $tableGatewayModel = null) // This is needed, because find takes as many parameters as // primary keys. It *does* *not* accept arrays with all primary // key columns. - $rowset = call_user_func_array([&$tableGatewayModel, 'find'], $idTupel); + // removing keys from $idTupel because in PHP 8 they have to match parameter names + $rowset = call_user_func_array([&$tableGatewayModel, 'find'], array_values($idTupel)); if (false === $rowset->count() > 0) { throw new NotFoundException( diff --git a/library/Opus/Model/Field.php b/library/Opus/Model/Field.php index 3b496f79..d64ded74 100644 --- a/library/Opus/Model/Field.php +++ b/library/Opus/Model/Field.php @@ -600,7 +600,7 @@ public function getValue($index = null) */ private function _wrapValueInArrayIfRequired($value) { - if (is_array($value) or ! $this->hasMultipleValues()) { + if (is_array($value) || ! $this->hasMultipleValues()) { return $value; } @@ -650,9 +650,11 @@ public function addValue($value) // Check multiplicity constraint if (is_int($this->multiplicity) === true) { + $valueCount = is_array($value) ? count($value) : 1; + if ( - (1 > $this->multiplicity) - || (1 + count($this->value)) > $this->multiplicity + ($valueCount > $this->multiplicity) + || ($valueCount + count($this->value)) > $this->multiplicity ) { throw new InvalidArgumentException( 'Field ' . $this->name . ' cannot hold more then ' . $this->multiplicity . ' values.' diff --git a/library/Opus/Model/Xml/AbstractVersion.php b/library/Opus/Model/Xml/AbstractVersion.php index f327817f..1cc8b442 100644 --- a/library/Opus/Model/Xml/AbstractVersion.php +++ b/library/Opus/Model/Xml/AbstractVersion.php @@ -213,7 +213,7 @@ public function getDomDocument() $this->config->dom = new DOMDocument('1.0', 'UTF-8'); $root = $this->config->dom->createElement('Opus'); - $root->setAttribute('version', $this->getVersion()); + $root->setAttribute('version', $this->version); // TODO use $this->getVersion() $this->config->dom->appendChild($root); $root->setAttribute('xmlns:xlink', 'http://www.w3.org/1999/xlink'); diff --git a/library/Opus/Model/Xml/Version1.php b/library/Opus/Model/Xml/Version1.php index 5618cdf2..6dbb48d5 100644 --- a/library/Opus/Model/Xml/Version1.php +++ b/library/Opus/Model/Xml/Version1.php @@ -119,14 +119,18 @@ protected function createModelNode($dom, $model) $childNode = $dom->createElement($modelClass); - if ($model->getId() !== null) { - if ($model instanceof PersistableInterface) { - $childNode->setAttribute('Id', $model->getId()); - } elseif ( - $model instanceof Filter && - $model->getModel() instanceof PersistableInterface - ) { - $childNode->setAttribute('Id', $model->getId()); + if ($model instanceof PersistableInterface) { + $modelId = $model->getId(); + if ($modelId !== null) { + $childNode->setAttribute('Id', $modelId); + } + } elseif ( + $model instanceof Filter && + $model->getModel() instanceof PersistableInterface + ) { + $modelId = $model->getId(); + if ($modelId !== null) { + $childNode->setAttribute('Id', $modelId); } } diff --git a/library/Opus/Series.php b/library/Opus/Series.php index ace3dd10..b7df1be9 100644 --- a/library/Opus/Series.php +++ b/library/Opus/Series.php @@ -230,7 +230,7 @@ public function isNumberAvailable($number) . 'WHERE series_id = ? AND number = ?', [$this->getId(), $number] ); - return $count === '0'; + return $count === 0; } /** diff --git a/tests/Opus/DatabaseTest.php b/tests/Opus/DatabaseTest.php index 55f8ee60..49b6e900 100644 --- a/tests/Opus/DatabaseTest.php +++ b/tests/Opus/DatabaseTest.php @@ -159,9 +159,6 @@ public function testPdoExecErrorReportingFirstStatement() $pdo->exec($sql); } - /** - * Error in second statement does not produce exception. - */ public function testPdoExecErrorReportingSecondStatement() { $database = new Database(); @@ -170,7 +167,9 @@ public function testPdoExecErrorReportingSecondStatement() $sql = 'TRUNCATE TABLE `schema_version`; INSERT INTO `schema_ver` (`version`) VALUES (\'5.0\');'; - $statement = $pdo->exec($sql); + // Error in second statement produces exception (at least with PHP 8) + $this->expectException(PDOException::class); + $pdo->exec($sql); $this->assertEquals('00000', $pdo->errorCode()); diff --git a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php index 53656650..1a092d4d 100644 --- a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php +++ b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php @@ -55,7 +55,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '0'); + $this->assertTrue($series[0]->getDocSortOrder() === 0); $d = new Document(); $d->addSeries($s)->setNumber('II.'); @@ -63,7 +63,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '1'); + $this->assertTrue($series[0]->getDocSortOrder() === 1); $d = new Document(); $d->addSeries($s)->setNumber('IV.')->setDocSortOrder(4); @@ -71,7 +71,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '4'); + $this->assertTrue($series[0]->getDocSortOrder() === 4); $d = new Document(); $d->addSeries($s)->setNumber('V.'); @@ -79,7 +79,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '5'); + $this->assertTrue($series[0]->getDocSortOrder() === 5); $d = new Document(); $d->addSeries($s)->setNumber('III.')->setDocSortOrder(3); @@ -87,22 +87,22 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '3'); + $this->assertTrue($series[0]->getDocSortOrder() === 3); $series[0]->setDocSortOrder(10); $d->store(); $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === '10'); + $this->assertTrue($series[0]->getDocSortOrder() === 10); $series[0]->setDocSortOrder(null); $d->store(); $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertFalse($series[0]->getDocSortOrder() === '11'); - $this->assertTrue($series[0]->getDocSortOrder() === '6'); + $this->assertFalse($series[0]->getDocSortOrder() === 11); + $this->assertTrue($series[0]->getDocSortOrder() === 6); } public function testToArray() diff --git a/tests/Opus/Model/FieldTest.php b/tests/Opus/Model/FieldTest.php index 6e269862..1e073cdd 100644 --- a/tests/Opus/Model/FieldTest.php +++ b/tests/Opus/Model/FieldTest.php @@ -478,7 +478,7 @@ public function testAddingValuesSetsModifiedFlag() */ public function testAddingMoreValuesThenAllowedThrowsException() { - $this->expectException('InvalidArgumentException'); + $this->expectException(InvalidArgumentException::class); $field = new Field('MyField'); $field->setMultiplicity(3); $field->addValue([15, 16, 17, 18]); diff --git a/tests/Opus/Model/Xml/Version1Test.php b/tests/Opus/Model/Xml/Version1Test.php index b5b5f18c..8e245001 100644 --- a/tests/Opus/Model/Xml/Version1Test.php +++ b/tests/Opus/Model/Xml/Version1Test.php @@ -735,9 +735,7 @@ protected function init() { */ public function testCallToResolverWhenXlinkIsEncounteredForDeserializingModels() { - $mockResolver = $this->getMockBuilder(XlinkResolverInterface::class) - ->setProxyTarget(['get']) - ->getMock(); + $mockResolver = $this->getMockBuilder(XlinkResolverInterface::class)->getMock(); $xmlData = ''; diff --git a/tests/Opus/SeriesTest.php b/tests/Opus/SeriesTest.php index 5ea9b911..b55f496c 100644 --- a/tests/Opus/SeriesTest.php +++ b/tests/Opus/SeriesTest.php @@ -411,13 +411,13 @@ public function testAssignSortOrder() $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getSortOrder() === '0'); + $this->assertTrue($s->getSortOrder() === 0); - $s->setSortOrder('10'); + $s->setSortOrder(10); $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getSortOrder() === '10'); + $this->assertTrue($s->getSortOrder() === 10); $s->delete(); } From a4759a2e7030bdf71d0a7a20e825c4f1d2e27ff2 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 17:50:53 +0100 Subject: [PATCH 16/46] #320 Switch assertions --- library/Opus/Model/DatabaseTrait.php | 1 + tests/Opus/CollectionRoleTest.php | 27 +++++++++++---------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/library/Opus/Model/DatabaseTrait.php b/library/Opus/Model/DatabaseTrait.php index 0fd7a48d..1ff0661a 100644 --- a/library/Opus/Model/DatabaseTrait.php +++ b/library/Opus/Model/DatabaseTrait.php @@ -39,6 +39,7 @@ use Zend_Db_Table_Row; use function array_multisort; +use function array_values; use function call_user_func_array; use function get_class; use function implode; diff --git a/tests/Opus/CollectionRoleTest.php b/tests/Opus/CollectionRoleTest.php index ebb4ea4e..812b04be 100644 --- a/tests/Opus/CollectionRoleTest.php +++ b/tests/Opus/CollectionRoleTest.php @@ -43,6 +43,7 @@ use function count; use function in_array; +use function intval; use function is_array; use function is_object; use function rand; @@ -155,7 +156,7 @@ public function testStore() 'CollectionRole isNewRecord check failed on new record.' ); - $roleId = ( int )$this->object->store(); + $roleId = (int) $this->object->store(); $this->assertNotNull( $roleId, 'CollectionRole roleId return value check on stored record.' @@ -164,10 +165,7 @@ public function testStore() $this->object->getId(), 'CollectionRole getId check on stored record.' ); - $this->assertTrue( - $roleId === $this->object->getId(), - 'CollectionRole->store return value check failed.' - ); + $this->assertEquals($roleId, $this->object->getId(), 'CollectionRole->store return value check failed.'); $this->assertFalse( $this->object->isNewRecord(), 'CollectionRole isNewRecord check failed on stored record.' @@ -717,10 +715,7 @@ public function testPosition() // Reload object, otherwise the result will be trivial. $role = new CollectionRole($this->object->getId()); - $this->assertTrue( - $role->getPosition() === $position, - 'CollectionRole position check failed.' - ); + $this->assertEquals($position, $role->getPosition(), 'CollectionRole position check failed.'); } } @@ -754,15 +749,15 @@ public function testRoleData() $this->assertNotNull($role->getName(), 'CollectionRole name check failed.'); $this->assertNotNull($role->getOaiName(), 'CollectionRole oai_name check failed.'); - $this->assertTrue($role->getDisplayBrowsing() === 'Number, Name', 'CollectionRole display_browsing check failed.'); - $this->assertTrue($role->getDisplayFrontdoor() === 'Name', 'CollectionRole display_frontdoor check failed.'); + $this->assertEquals('Number, Name', $role->getDisplayBrowsing(), 'CollectionRole display_browsing check failed.'); + $this->assertEquals('Name', $role->getDisplayFrontdoor(), 'CollectionRole display_frontdoor check failed.'); - $this->assertTrue($role->getVisible() === 1, 'CollectionRole visible check failed.'); - $this->assertTrue($role->getVisibleBrowsingStart() === 1, 'CollectionRole visible_browsing_start check failed.'); - $this->assertTrue($role->getVisibleFrontdoor() === 0, 'CollectionRole visible_frontdoor check failed.'); - $this->assertTrue($role->getVisibleOai() === 1, 'CollectionRole visible_oai check failed.'); + $this->assertEquals(1, $role->getVisible(), 'CollectionRole visible check failed.'); + $this->assertEquals(1, $role->getVisibleBrowsingStart(), 'CollectionRole visible_browsing_start check failed.'); + $this->assertEquals(0, $role->getVisibleFrontdoor(), 'CollectionRole visible_frontdoor check failed.'); + $this->assertEquals(1, $role->getVisibleOai(), 'CollectionRole visible_oai check failed.'); - $this->assertTrue($role->getPosition() === 1, 'CollectionRole position check failed.'); + $this->assertEquals(1, $role->getPosition(), 'CollectionRole position check failed.'); } /** From 4c29ba3e511fc6c93e67fd65644f0f439e74fc06 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 18:08:28 +0100 Subject: [PATCH 17/46] #320 Replacing assertions --- .../Model/Dependent/Link/DocumentSeriesTest.php | 16 ++++++++-------- tests/Opus/SeriesTest.php | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php index 1a092d4d..ff5899c7 100644 --- a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php +++ b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php @@ -55,7 +55,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 0); + $this->assertEquals(0, $series[0]->getDocSortOrder()); $d = new Document(); $d->addSeries($s)->setNumber('II.'); @@ -63,7 +63,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 1); + $this->assertEquals(1, $series[0]->getDocSortOrder()); $d = new Document(); $d->addSeries($s)->setNumber('IV.')->setDocSortOrder(4); @@ -71,7 +71,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 4); + $this->assertEquals(4, $series[0]->getDocSortOrder()); $d = new Document(); $d->addSeries($s)->setNumber('V.'); @@ -79,7 +79,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 5); + $this->assertEquals(5, $series[0]->getDocSortOrder()); $d = new Document(); $d->addSeries($s)->setNumber('III.')->setDocSortOrder(3); @@ -87,22 +87,22 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 3); + $this->assertEquals(3, $series[0]->getDocSortOrder()); $series[0]->setDocSortOrder(10); $d->store(); $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertTrue($series[0]->getDocSortOrder() === 10); + $this->assertEquals(10,$series[0]->getDocSortOrder()); $series[0]->setDocSortOrder(null); $d->store(); $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertFalse($series[0]->getDocSortOrder() === 11); - $this->assertTrue($series[0]->getDocSortOrder() === 6); + $this->assertNotEquals(11, $series[0]->getDocSortOrder()); + $this->assertEquals(6, $series[0]->getDocSortOrder()); } public function testToArray() diff --git a/tests/Opus/SeriesTest.php b/tests/Opus/SeriesTest.php index b55f496c..ceac517e 100644 --- a/tests/Opus/SeriesTest.php +++ b/tests/Opus/SeriesTest.php @@ -389,17 +389,17 @@ public function testAssignVisibleStatus() $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getVisible() === 1); + $this->assertEquals(1, $s->getVisible()); $s = Series::get($s->getId()); $s->setVisible('0'); $s->store(); - $this->assertTrue($s->getVisible() === '0'); + $this->assertEquals(0, $s->getVisible()); $s = Series::get($s->getId()); $s->setVisible('1'); $s->store(); - $this->assertTrue($s->getVisible() === '1'); + $this->assertEquals(1, $s->getVisible()); $s->delete(); } @@ -411,13 +411,13 @@ public function testAssignSortOrder() $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getSortOrder() === 0); + $this->assertEquals(0, $s->getSortOrder()); $s->setSortOrder(10); $s->store(); $s = Series::get($s->getId()); - $this->assertTrue($s->getSortOrder() === 10); + $this->assertEquals(10, $s->getSortOrder()); $s->delete(); } From a777008258ecf5996389cfa169da3ad7cf3f564a Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 18:39:55 +0100 Subject: [PATCH 18/46] #320 Fixes for PHP 8 --- library/Opus/Document.php | 16 +++++++++++----- library/Opus/Series.php | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/library/Opus/Document.php b/library/Opus/Document.php index 397a5d80..91bf3813 100644 --- a/library/Opus/Document.php +++ b/library/Opus/Document.php @@ -1054,17 +1054,23 @@ public function hasEmbargoPassed($now = null) /** * Only consider files which are visible in frontdoor. * - * @return bool|void + * @return bool */ public function hasFulltext() { $files = $this->getFile(); - $files = array_filter($files, function ($file) { - return $file->getVisibleInFrontdoor() === 1; - }); + if (! is_array($files)) { + return false; + } + + foreach ($files as $file) { + if ($file->getVisibleInFrontdoor()) { + return true; + } + } - return count($files) > 0; + return false; } /** diff --git a/library/Opus/Series.php b/library/Opus/Series.php index b7df1be9..809f6258 100644 --- a/library/Opus/Series.php +++ b/library/Opus/Series.php @@ -225,7 +225,7 @@ public function getDocumentIdForNumber($number) public function isNumberAvailable($number) { $db = Zend_Db_Table::getDefaultAdapter(); - $count = $db->fetchOne( + $count = ( int )$db->fetchOne( 'SELECT COUNT(*) AS rows_count FROM link_documents_series ' . 'WHERE series_id = ? AND number = ?', [$this->getId(), $number] From b5e438e3fb022443f24c002421fc7e5bcb0dc17e Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 19:00:29 +0100 Subject: [PATCH 19/46] #320 Disabled tests that differ for PHP 7 and 8 --- tests/Opus/DatabaseTest.php | 2 ++ tests/Opus/FileTest.php | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/Opus/DatabaseTest.php b/tests/Opus/DatabaseTest.php index 49b6e900..9b8af420 100644 --- a/tests/Opus/DatabaseTest.php +++ b/tests/Opus/DatabaseTest.php @@ -161,6 +161,8 @@ public function testPdoExecErrorReportingFirstStatement() public function testPdoExecErrorReportingSecondStatement() { + $this->markTestIncomplete('PHP 7.1 does not produce exception.'); + $database = new Database(); $pdo = $database->getPdo($database->getName()); diff --git a/tests/Opus/FileTest.php b/tests/Opus/FileTest.php index 803c8b06..54975f88 100644 --- a/tests/Opus/FileTest.php +++ b/tests/Opus/FileTest.php @@ -577,6 +577,7 @@ public function testHashValueCachedOnceCalculated() */ public function testInvalidHashAlgorithmAfterStore() { + $this->markTestIncomplete('PHP 7.1 and 8 produce different error (Exception vs ValueError).'); $doc = $this->createDocumentWithFile("foobar.pdf"); $file = $doc->getFile(0); $doc->store(); From da5654e48f767a7d482cc7a8c7d582dad2ad4392 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 19:43:41 +0100 Subject: [PATCH 20/46] #320 Small PHP 8 fixes --- library/Opus/Doi/UserRecipientProvider.php | 2 +- tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/Opus/Doi/UserRecipientProvider.php b/library/Opus/Doi/UserRecipientProvider.php index 4765f654..0e1d6f77 100644 --- a/library/Opus/Doi/UserRecipientProvider.php +++ b/library/Opus/Doi/UserRecipientProvider.php @@ -61,7 +61,7 @@ public function getRecipients() foreach ($accounts as $account) { $email = $account->getEmail(); - if (strlen(trim($email)) === 0) { + if (strlen(trim($email ?? '')) === 0) { continue; // do not add to recipient list } diff --git a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php index ff5899c7..b3bad6ef 100644 --- a/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php +++ b/tests/Opus/Model/Dependent/Link/DocumentSeriesTest.php @@ -94,7 +94,7 @@ public function testAssignDocSortOrder() $d = new Document($d->getId()); $series = $d->getSeries(); - $this->assertEquals(10,$series[0]->getDocSortOrder()); + $this->assertEquals(10, $series[0]->getDocSortOrder()); $series[0]->setDocSortOrder(null); $d->store(); From a315b96371e4ff145cb67027ea437702b69b3ed7 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Nov 2022 20:11:19 +0100 Subject: [PATCH 21/46] #320 Make test mime-type independent --- tests/Opus/Doi/DataCiteXmlGeneratorTest.php | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/Opus/Doi/DataCiteXmlGeneratorTest.php b/tests/Opus/Doi/DataCiteXmlGeneratorTest.php index 989608aa..b5447443 100644 --- a/tests/Opus/Doi/DataCiteXmlGeneratorTest.php +++ b/tests/Opus/Doi/DataCiteXmlGeneratorTest.php @@ -747,10 +747,13 @@ public function testFileInformationVisible() $size = intval(round($file->getFileSize() / 1024)); $size2 = intval(round($file2->getFileSize() / 1024)); + $mimeType1 = $file->getMimeType(); + $mimeType2 = $file2->getMimeType(); + $sizesXpath1 = $xpath->query("//ns:size[1][text()=\"$size KB\"]"); $sizesXpath2 = $xpath->query("//ns:size[2][text()=\"$size2 KB\"]"); - $formatXpath1 = $xpath->query("//ns:format[1][text()=\"text/plain\"]"); - $formatXpath2 = $xpath->query("//ns:format[2][text()=\"text/plain\"]"); + $formatXpath1 = $xpath->query("//ns:format[1][text()=\"$mimeType1\"]"); + $formatXpath2 = $xpath->query("//ns:format[2][text()=\"$mimeType2\"]"); $this->assertEquals(1, $sizesXpath1->length); $this->assertEquals(1, $sizesXpath2->length); @@ -785,6 +788,9 @@ public function testMixedFileInformationVisibility() $doc->addFile($file2); $doc->store(); + $mimeType1 = $file->getMimeType(); + $mimeType2 = $file2->getMimeType(); + $generator = new DataCiteXmlGenerator(); $xml = $generator->getXml($doc); @@ -795,8 +801,8 @@ public function testMixedFileInformationVisibility() $sizesXpath1 = $xpath->query("//ns:size[1][text()=\"$size KB\"]"); $sizesXpath2 = $xpath->query("//ns:size[2][text()=\"$size2 KB\"]"); - $formatXpath1 = $xpath->query("//ns:format[1][text()=\"text/plain\"]"); - $formatXpath2 = $xpath->query("//ns:format[2][text()=\"text/plain\"]"); + $formatXpath1 = $xpath->query("//ns:format[1][text()=\"$mimeType1\"]"); + $formatXpath2 = $xpath->query("//ns:format[2][text()=\"$mimeType2\"]"); $this->assertEquals(1, $sizesXpath1->length); $this->assertEquals(0, $sizesXpath2->length); @@ -839,10 +845,13 @@ public function testDifferentOrderFileInformationVisibility() $size = intval(round($file->getFileSize() / 1024)); $size2 = intval(round($file2->getFileSize() / 1024)); + $mimeType1 = $file->getMimeType(); + $mimeType2 = $file2->getMimeType(); + $sizesXpath1 = $xpath->query("//ns:size[2][text()=\"$size KB\"]"); $sizesXpath2 = $xpath->query("//ns:size[1][text()=\"$size2 KB\"]"); - $formatXpath1 = $xpath->query("//ns:format[2][text()=\"text/plain\"]"); - $formatXpath2 = $xpath->query("//ns:format[1][text()=\"text/plain\"]"); + $formatXpath1 = $xpath->query("//ns:format[2][text()=\"$mimeType1\"]"); + $formatXpath2 = $xpath->query("//ns:format[1][text()=\"$mimeType2\"]"); $this->assertEquals(1, $sizesXpath2->length); $this->assertEquals(0, $sizesXpath1->length); From ee9cd469b669e452b8ab8644d1da04dc1b51240f Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 28 Nov 2022 09:54:43 +0100 Subject: [PATCH 22/46] #320 Don't call json_decode with NULL --- library/Opus/Job.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/Opus/Job.php b/library/Opus/Job.php index 0f2b123d..926878e2 100644 --- a/library/Opus/Job.php +++ b/library/Opus/Job.php @@ -47,8 +47,6 @@ /** * Job model used to manage job descriptions. - * - * phpcs:disable */ class Job extends AbstractDb implements JobInterface, JobRepositoryInterface { @@ -123,9 +121,12 @@ public function setData($value) public function getData($convertObjectsIntoAssociativeArrays = false) { $fieldData = $this->_getField('Data')->getValue(); + if ($fieldData === null) { + throw new Exception('No JSON data to decode.'); + } $jsonDecode = json_decode($fieldData, $convertObjectsIntoAssociativeArrays); - if ((null != $fieldData) and (null === $jsonDecode)) { - throw new Exception('Json decoding failed.'); + if (null === $jsonDecode) { + throw new Exception('JSON decoding failed.'); } return $jsonDecode; } From d8405a569a38d8edb411c6f9f2bcef0e82fed8da Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 28 Nov 2022 10:12:09 +0100 Subject: [PATCH 23/46] #320 Coding style fixes --- library/Opus/Job.php | 8 +++++--- phpcs.xml | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/library/Opus/Job.php b/library/Opus/Job.php index 926878e2..76289fc2 100644 --- a/library/Opus/Job.php +++ b/library/Opus/Job.php @@ -40,6 +40,7 @@ use Zend_Validate_NotEmpty; use function count; +use function func_get_args; use function json_decode; use function json_encode; use function serialize; @@ -106,7 +107,7 @@ protected function _preStore() public function setData($value) { $jsonEncode = json_encode($value); - if ((null !== $value) and (null === $jsonEncode)) { + if ((null !== $value) && (null === $jsonEncode)) { throw new Exception('Json encoding failed.'); } $this->_getField('Data')->setValue($jsonEncode); @@ -115,12 +116,13 @@ public function setData($value) /** * Intercept getter logic to do JSON decoding. * - * @throws Exception Thrown if json decoding failed. + * @param bool $convertObjectsIntoAssociativeArrays * @return mixed Value of field. + * @throws Exception Thrown if json decoding failed. */ public function getData($convertObjectsIntoAssociativeArrays = false) { - $fieldData = $this->_getField('Data')->getValue(); + $fieldData = $this->_getField('Data')->getValue(); if ($fieldData === null) { throw new Exception('No JSON data to decode.'); } diff --git a/phpcs.xml b/phpcs.xml index 4f91b54d..ed24413d 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -6,4 +6,9 @@ library tests + + + 0 + + From 98d498198a8ade7d1fe82b617bdf60dc0144b9d6 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 28 Nov 2022 10:23:16 +0100 Subject: [PATCH 24/46] #320 Coding style fix --- library/Opus/Job.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Opus/Job.php b/library/Opus/Job.php index 76289fc2..3d645384 100644 --- a/library/Opus/Job.php +++ b/library/Opus/Job.php @@ -222,7 +222,7 @@ public function getAll($ids = null) * @param array $labels Set of labels to get Jobs for. * @param null|string $limit (optional) Number of jobs to retrieve * @param null|string $state (optional) only retrieve jobs in given state - * @return array Set of Opus\Job objects. + * @return array|null Set of Opus\Job objects. */ public function getByLabels($labels, $limit = null, $state = null) { From b1fbf52f3931b51b38ad2bd3f198c685c494ce45 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 8 Dec 2022 16:24:28 +0100 Subject: [PATCH 25/46] #243 Find APPLICATION_PATH if used in packages --- db/createdb.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/db/createdb.php b/db/createdb.php index d34305b6..be954f19 100644 --- a/db/createdb.php +++ b/db/createdb.php @@ -41,11 +41,19 @@ defined('FRAMEWORK_PATH') || define('FRAMEWORK_PATH', realpath($frameworkPath)); +if (strpos($frameworkPath, 'vendor/opus4-repo/framework') === false) { + // Script used in opus4-repo/framework + $applicationPath = $frameworkPath; +} else { + // Script used in project depending on opus4-repo/framework + $applicationPath = dirname($frameworkPath, 3); +} + // Define path to application directory defined('APPLICATION_PATH') || define( 'APPLICATION_PATH', - getenv('APPLICATION_PATH') ? getenv('APPLICATION_PATH') : realpath($frameworkPath) + getenv('APPLICATION_PATH') ? getenv('APPLICATION_PATH') : $applicationPath ); // Define application environment From cbcd9a2dbc945992f72675c0f4ef7ac8331c3c85 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 4 Jan 2023 13:44:18 +0100 Subject: [PATCH 26/46] #320 Handle DB schema version as int --- library/Opus/Db/DatabaseBootstrap.php | 2 +- library/Opus/Version.php | 2 +- tests/Opus/VersionTest.php | 12 +++--------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/library/Opus/Db/DatabaseBootstrap.php b/library/Opus/Db/DatabaseBootstrap.php index fc65202c..2eb3cd59 100644 --- a/library/Opus/Db/DatabaseBootstrap.php +++ b/library/Opus/Db/DatabaseBootstrap.php @@ -115,7 +115,7 @@ protected function _initDatabase() $result = $query->fetch(); if (is_array($result) && array_key_exists('version', $result)) { - $version = $result['version']; + $version = (int) $result['version']; $expectedVersion = Version::getSchemaVersion(); if ($version !== $expectedVersion) { diff --git a/library/Opus/Version.php b/library/Opus/Version.php index f344c020..d3f8bd69 100644 --- a/library/Opus/Version.php +++ b/library/Opus/Version.php @@ -46,7 +46,7 @@ class Version /** * Version of database schema. */ - public const SCHEMA_VERSION = '19'; + public const SCHEMA_VERSION = 19; /** * Compare the specified Opus Framework version string $version diff --git a/tests/Opus/VersionTest.php b/tests/Opus/VersionTest.php index 4308c272..de464f95 100644 --- a/tests/Opus/VersionTest.php +++ b/tests/Opus/VersionTest.php @@ -26,13 +26,8 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @copyright Copyright (c) 2010-2018, OPUS 4 development team + * @copyright Copyright (c) 2010, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License - * - * @category Tests - * @package Opus - * @author Thoralf Klein - * @author Jens Schwidder */ namespace OpusTest; @@ -43,7 +38,6 @@ use function array_pop; use function basename; -use function ctype_digit; use function file_get_contents; use function preg_match; use function substr; @@ -61,8 +55,8 @@ public function testGetSchemaVersion() { $version = Version::getSchemaVersion(); - $this->assertInternalType('string', $version); - $this->assertTrue(ctype_digit($version)); + $this->assertInternalType('int', $version); + $this->assertGreaterThan(0, $version); } public function testSchemaVersionInUpdateScriptMatchesName() From 728fd6223f9fcf30f602404348e95108ba860c90 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 4 Jan 2023 13:58:54 +0100 Subject: [PATCH 27/46] #321 Return 'int' for model IDs --- library/Opus/Collection.php | 8 +++-- .../DocumentFinder/DefaultDocumentFinder.php | 3 +- library/Opus/Doi/DoiManager.php | 32 +++++++++++-------- library/Opus/Job.php | 2 +- library/Opus/Model/AbstractDb.php | 9 ++++-- library/Opus/Model/Field.php | 29 +++++++++++++++-- library/Opus/Series.php | 5 +-- tests/Opus/CollectionRoleTest.php | 3 +- tests/Opus/CollectionTest.php | 30 +++++++++++++++++ 9 files changed, 94 insertions(+), 27 deletions(-) diff --git a/library/Opus/Collection.php b/library/Opus/Collection.php index abbb8842..cb8d4a91 100644 --- a/library/Opus/Collection.php +++ b/library/Opus/Collection.php @@ -36,6 +36,7 @@ use Exception; use InvalidArgumentException; use Opus\Common\CollectionInterface; +use Opus\Common\CollectionRoleInterface; use Opus\Common\Config; use Opus\Common\Model\NotFoundException; use Opus\Db\TableGateway; @@ -154,11 +155,14 @@ public function getDefaultPlugins() */ protected function init() { + $roleId = new Field('RoleId'); + $roleId->setType('int'); + $this->addField($roleId); + $fields = [ 'Number', 'Name', 'OaiSubset', - 'RoleId', 'Role', 'RoleName', 'RoleDisplayFrontdoor', @@ -446,7 +450,7 @@ protected function _fetchRoleName() */ public function getDisplayName($context = 'browsing', $role = null) { - if ($role !== null && (! $role instanceof CollectionRole || $role->getId() !== $this->getRoleId())) { + if ($role !== null && (! $role instanceof CollectionRoleInterface || $role->getId() !== $this->getRoleId())) { throw new InvalidArgumentException('given Collection Role is not compatible'); } diff --git a/library/Opus/DocumentFinder/DefaultDocumentFinder.php b/library/Opus/DocumentFinder/DefaultDocumentFinder.php index 49a33d28..10d57d80 100644 --- a/library/Opus/DocumentFinder/DefaultDocumentFinder.php +++ b/library/Opus/DocumentFinder/DefaultDocumentFinder.php @@ -34,6 +34,7 @@ use Opus\Common\DocumentFinderInterface; use Opus\DocumentFinder; +use function array_map; use function is_array; /** @@ -59,7 +60,7 @@ public function __construct() */ public function getIds() { - return $this->finder->ids(); + return array_map('intval', $this->finder->ids()); } /** diff --git a/library/Opus/Doi/DoiManager.php b/library/Opus/Doi/DoiManager.php index 5c104a35..56a05fba 100644 --- a/library/Opus/Doi/DoiManager.php +++ b/library/Opus/Doi/DoiManager.php @@ -37,6 +37,7 @@ use Exception; use InvalidArgumentException; use Opus\Common\Config; +use Opus\Common\DocumentInterface; use Opus\Common\Identifier; use Opus\Common\Log; use Opus\Common\Log\LogService; @@ -155,10 +156,11 @@ public function getDoiLogger() * Liefert im Erfolgsfall die registrierte DOI zurück. Liefert null zurück, wenn das Dokument keine lokale * DOI besitzt, die registriert werden kann. * - * @param $doc Document oder ID eines Opus\Document als String - * @param $store wenn true, dann wird am Ende der Methode store() auf dem übergebenen $doc aufgerufen - * wenn die Methode im Kontext eines Store-Plugins aufgerufen wird, dann erfolgt der Aufruf - * von store() an anderer Stelle (sonst gibt es eine Endlosschleife) + * @param DocumentInterface $doc Document oder ID eines Opus\Document als String + * @param bool $store Wenn true, dann wird am Ende der Methode store() auf dem übergebenen $doc + * aufgerufen. Wenn die Methode im Kontext eines Store-Plugins aufgerufen wird, + * dann erfolgt der Aufruf von store() an anderer Stelle (sonst gibt es eine + * Endlosschleife). * @throws DoiException wenn das referenzierte Dokument nicht in der Datenbank existiert * @throws RegistrationException wenn bei dem Versuch der Registrierung bei DataCite ein Fehler auftritt */ @@ -345,7 +347,7 @@ private function checkForLocalRegistrableDoi($doc) /** * Liefert true, wenn der übergebene Wert den Wert einer lokale DOI darstellt; andernfalls false. * - * @param $value Wert einer DOI, der auf Lokalität geprüft werden soll + * @param string $value Wert einer DOI, der auf Lokalität geprüft werden soll */ private function isLocalDoi($value) { @@ -362,9 +364,10 @@ private function isLocalDoi($value) * alle Dokumente unabhängig von ihrem ServerState betrachtet werden, so muss als Aufrufargument null übergeben * werden. * - * @param $filterServerState Filter für Attribut ServerState (es werden nur Dokumente mit dem angegeben ServerState - * bei der Registrierung betrachtet); um alle Dokumente unabhängig vom ServerState zu - * betrachten, muss der Wert null übergeben werden (Default: published) + * @param string $filterServerState Filter für Attribut ServerState (es werden nur Dokumente mit dem angegeben + * ServerState bei der Registrierung betrachtet); um alle Dokumente unabhängig vom + * ServerState zu betrachten, muss der Wert null übergeben werden (Default: + * published) * @return DoiManagerStatus */ public function registerPending($filterServerState = 'published') @@ -464,10 +467,10 @@ public function verifyRegistered() * Ist eine Prüfung nicht möglich, so gibt die Methode null zurück, z.B. wenn das Dokument mit der übergebenen ID * gar keine DOI hat. * - * @param $docId ID des zu überprüfenden OPUS-Dokuments - * @param $allowReverification wenn true, dann werden DOIs, die bereits geprüft wurden, erneut geprüft - * @param $beforeDate Nur DOIs prüfen, deren Registrierung vor dem übergebenen Zeitpunkt liegt - * @param null|DoiManagerStatus $managerStatus Objekt zum Ablegen von Statusinformationen der DOI-Prüfung + * @param int $docId ID des zu überprüfenden OPUS-Dokuments + * @param bool $allowReverification wenn true, dann werden DOIs, die bereits geprüft wurden, erneut geprüft + * @param string $beforeDate Nur DOIs prüfen, deren Registrierung vor dem übergebenen Zeitpunkt liegt + * @param null|DoiManagerStatus $managerStatus Objekt zum Ablegen von Statusinformationen der DOI-Prüfung */ public function verify($docId, $allowReverification = true, $beforeDate = null, $managerStatus = null) { @@ -659,12 +662,13 @@ public function getAll($statusFilter = null) * Erzeugt auf Basis der konfigurierten DOI-Generator-Klasse einen DOI-Wert für das übergebene Dokument. * Gibt den Wert zurück oder wirft eine Exception, wenn die Generierung nicht möglich ist. * - * @param $doc Document, für das ein DOI-Wert generiert werden soll oder ID eines Dokuments - * für eine ID (string), wird versucht das zugehörige Opus\Document aus der Datenbank zu laden + * @param Document|int $doc Document für das ein DOI-Wert generiert werden soll oder ID eines Dokuments fuer eine + * ID (string), wird versucht das zugehörige Opus\Document aus der Datenbank zu laden * @throws DoiException */ public function generateNewDoi($doc) { + // TODO DESIGN move getting generator into separate function $generator = null; try { $generator = DoiGeneratorFactory::create(); diff --git a/library/Opus/Job.php b/library/Opus/Job.php index 3d645384..2236f811 100644 --- a/library/Opus/Job.php +++ b/library/Opus/Job.php @@ -124,7 +124,7 @@ public function getData($convertObjectsIntoAssociativeArrays = false) { $fieldData = $this->_getField('Data')->getValue(); if ($fieldData === null) { - throw new Exception('No JSON data to decode.'); + return []; // TODO WAS throw new Exception('No JSON data to decode.'); } $jsonDecode = json_decode($fieldData, $convertObjectsIntoAssociativeArrays); if (null === $jsonDecode) { diff --git a/library/Opus/Model/AbstractDb.php b/library/Opus/Model/AbstractDb.php index c379091a..742b9c4c 100644 --- a/library/Opus/Model/AbstractDb.php +++ b/library/Opus/Model/AbstractDb.php @@ -625,7 +625,7 @@ public function delete() /** * Get the models primary key. * - * @return mixed + * @return int|string|array|null */ public function getId() { @@ -640,7 +640,12 @@ public function getId() if (count($result) > 1) { return $result; } elseif (count($result) === 1) { - return $result[0]; + $modelId = $result[0]; + if ($modelId !== null && ctype_digit($modelId)) { + return (int) $modelId; + } else { + return $modelId; + } } else { return null; } diff --git a/library/Opus/Model/Field.php b/library/Opus/Model/Field.php index d64ded74..5398433d 100644 --- a/library/Opus/Model/Field.php +++ b/library/Opus/Model/Field.php @@ -94,7 +94,7 @@ class Field implements ModificationTrackingInterface, FieldInterface /** * Specifiy whether the field is required or not. * - * @var unknown_type + * @var bool */ protected $mandatory = false; @@ -181,6 +181,9 @@ class Field implements ModificationTrackingInterface, FieldInterface */ protected $pendingDeletes = []; + /** @var null|string */ + private $type; + /** * Create an new field instance and set the given name. * @@ -576,8 +579,8 @@ public function getValue($index = null) // Caller requested a specific array index if ($index !== null) { - if (true === is_array($this->value)) { - if (true === isset($this->value[$index])) { + if (is_array($this->value)) { + if (isset($this->value[$index])) { return $this->value[$index]; } else { throw new InvalidArgumentException('Unvalid index: ' . $index); @@ -587,6 +590,10 @@ public function getValue($index = null) } } + if ($this->value !== null && $this->getType() === 'int') { + return (int) $this->value; + } + return $this->value; } @@ -923,4 +930,20 @@ public function isCheckbox() { return $this->checkbox; } + + /** + * @param null|string $type + */ + public function setType($type) + { + $this->type = $type; + } + + /** + * @return string|null + */ + public function getType() + { + return $this->type; + } } diff --git a/library/Opus/Series.php b/library/Opus/Series.php index 809f6258..3a54c05c 100644 --- a/library/Opus/Series.php +++ b/library/Opus/Series.php @@ -37,6 +37,7 @@ use Opus\Model\AbstractDb; use Opus\Model\Field; use Zend_Db_Table; +use Zend_Db_Table_Row_Abstract; use Zend_Validate_Int; use Zend_Validate_NotEmpty; @@ -105,7 +106,7 @@ protected function init() * of Opus\Series. * * @param int $id - * @return TableGateway + * @return Zend_Db_Table_Row_Abstract */ public static function createRowWithCustomId($id) { @@ -213,7 +214,7 @@ public function getDocumentIdForNumber($number) [$this->getId(), $number] ); - return count($documentId) === 1 ? $documentId[0] : null; + return count($documentId) === 1 ? (int) $documentId[0] : null; } /** diff --git a/tests/Opus/CollectionRoleTest.php b/tests/Opus/CollectionRoleTest.php index 812b04be..85caf9ef 100644 --- a/tests/Opus/CollectionRoleTest.php +++ b/tests/Opus/CollectionRoleTest.php @@ -743,8 +743,7 @@ public function testRoleData() $this->assertNotNull($role->getId(), 'CollectionRole storing failed: should have an Id.'); // Restore object, validate. - $roleId = $role->getId(); - $role = new CollectionRole($roleId); + $role = new CollectionRole($role->getId()); $this->assertNotNull($role->getName(), 'CollectionRole name check failed.'); $this->assertNotNull($role->getOaiName(), 'CollectionRole oai_name check failed.'); diff --git a/tests/Opus/CollectionTest.php b/tests/Opus/CollectionTest.php index 26be0a72..2581d1b2 100644 --- a/tests/Opus/CollectionTest.php +++ b/tests/Opus/CollectionTest.php @@ -294,6 +294,15 @@ public function testGetDisplayName() $this->object->store(); $collection = Collection::get($this->object->getId()); + + $this->assertEquals($this->roleFixture->getId(), $collection->getRoleId()); + + $role = $collection->getRole(); + + $this->assertEquals($this->roleFixture->getId(), $role->getId()); + $this->assertEquals('Name', $role->getDisplayBrowsing()); + $this->assertEquals('Number', $role->getDisplayFrontdoor()); + $this->assertEquals('fooblablub', $collection->getDisplayName('browsing')); $this->assertEquals('thirteen', $collection->getDisplayName('frontdoor')); } @@ -1652,4 +1661,25 @@ public function testFindInRoles() $this->assertCount(1, Collection::find('TestCol', $role1->getId())); $this->assertCount(2, Collection::find('TestCol', [$role1->getId(), $role2->getId()])); } + + public function testGetRoleName() + { + $roleName = 'TestRole'; + + $role = CollectionRole::new(); + $role->setName($roleName); + $role->setOaiName('TestRoleOai'); + + $root = $role->addRootCollection(); + $root->setName('RootCol'); + + $roleId = $role->store(); + + $role = CollectionRole::get($roleId); + $this->assertEquals($roleName, $role->getName()); + + $root = $role->getRootCollection(); + $this->assertEquals($roleId, $root->getRoleId()); + $this->assertEquals($roleName, $root->getRoleName()); + } } From bc191787c6af8b6ee04087c14d1e348a5ee0e30f Mon Sep 17 00:00:00 2001 From: j3nsch Date: Wed, 4 Jan 2023 16:27:03 +0100 Subject: [PATCH 28/46] #320 Fix return type --- library/Opus/Job.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Opus/Job.php b/library/Opus/Job.php index 2236f811..c39d9a03 100644 --- a/library/Opus/Job.php +++ b/library/Opus/Job.php @@ -117,7 +117,7 @@ public function setData($value) * Intercept getter logic to do JSON decoding. * * @param bool $convertObjectsIntoAssociativeArrays - * @return mixed Value of field. + * @return array Value of field. * @throws Exception Thrown if json decoding failed. */ public function getData($convertObjectsIntoAssociativeArrays = false) From e7f84cda8b538f76ca671c65c30d6d2ba01883d5 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 10 Jan 2023 16:54:36 +0100 Subject: [PATCH 29/46] Update for fixed typo in PersonInterface --- library/Opus/Person.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/Opus/Person.php b/library/Opus/Person.php index 7d2759b9..c5255f88 100644 --- a/library/Opus/Person.php +++ b/library/Opus/Person.php @@ -555,8 +555,8 @@ public function getPersonValues($person) * * TODO filter by role? * - * @param $person Criteria for matching persons - * @param null $documents Array with ids of documents + * @param array $person Criteria for matching persons + * @param array|null $documents Array with ids of documents * @return array Array with IDs of persons */ public function getPersons($person, $documents = null) @@ -619,8 +619,8 @@ public function getPersonsAndDocuments($person, $documents = null) * * Optionally the scope can be limited to specified set of documents. * - * @param $person Criteria for matching persons - * @param $changes Map of column names and new values + * @param array $person Criteria for matching persons + * @param array $changes Map of column names and new values * @param null $documents Array with document Ids * * TODO update ServerDateModified for modified documents (How?) @@ -709,7 +709,7 @@ public function getDocuments($personIds, $documents = null) /** * Converts map with field names into array with column names. * - * @param $changes Map of field names and values + * @param array $changes Map of field names and values * @return array Map of column names and values */ public static function convertChanges($changes) @@ -1000,7 +1000,7 @@ public function getIdentifierGnd() * @return $this * @throws ModelException */ - public function setIdentifiertGnd($gndId) + public function setIdentifierGnd($gndId) { return $this->__call(__FUNCTION__, func_get_args()); } From a4b177ddd241eea0b93a16c1913689b4f9cdb0e7 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 3 Feb 2023 17:19:18 +0100 Subject: [PATCH 30/46] #320 Prevent calls with null param --- library/Opus/Model/AbstractDb.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Opus/Model/AbstractDb.php b/library/Opus/Model/AbstractDb.php index 742b9c4c..30c2bba3 100644 --- a/library/Opus/Model/AbstractDb.php +++ b/library/Opus/Model/AbstractDb.php @@ -641,7 +641,7 @@ public function getId() return $result; } elseif (count($result) === 1) { $modelId = $result[0]; - if ($modelId !== null && ctype_digit($modelId)) { + if (($modelId !== null && ! is_int($modelId) && ctype_digit($modelId))) { return (int) $modelId; } else { return $modelId; From ccb93bca02ae152648199463477126470573bab1 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 3 Feb 2023 17:19:36 +0100 Subject: [PATCH 31/46] #320 Fix return value --- library/Opus/Collection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Opus/Collection.php b/library/Opus/Collection.php index cb8d4a91..efb6afab 100644 --- a/library/Opus/Collection.php +++ b/library/Opus/Collection.php @@ -1132,7 +1132,7 @@ public function describe() public function isRoot() { if ($this->isNewRecord()) { - return; + return false; } return $this->primaryTableRow->getTable()->isRoot( From 1b3930e0a980a6a391fecbc3ae864d91f74d2ed1 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 3 Feb 2023 17:20:01 +0100 Subject: [PATCH 32/46] #320 Depend on main dev branches --- composer.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 06013187..928454dc 100644 --- a/composer.json +++ b/composer.json @@ -14,9 +14,9 @@ "ext-fileinfo": "*", "ext-json": "*", "ext-libxml": "*", - "shardj/zf1-future": "1.21.*", - "opus4-repo/opus4-common": "dev-4.7.2-zf1f", - "opus4-repo/opus4-doi": "dev-4.7.2-zf1f" + "opus4/zf1-future": "1.21.*", + "opus4-repo/opus4-common": "4.7.2.x-dev", + "opus4-repo/opus4-doi": "dev-master" }, "minimum-stability": "dev", "prefer-stable": true, From 3b9b86485ca0be834fc5ef4e0866eaa1ecedd8d7 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 17 Feb 2023 13:23:45 +0100 Subject: [PATCH 33/46] #319 Replace config.ini with database.ini --- .github/workflows/php.yml | 4 ++-- .gitignore | 4 +--- Vagrantfile | 14 ++++++------- bin/opus4db | 32 ++++++++++++++++++++++++++++ db/createdb.php | 2 +- tests/Bootstrap.php | 2 +- tests/config.ini.template | 44 --------------------------------------- 7 files changed, 44 insertions(+), 58 deletions(-) delete mode 100644 tests/config.ini.template diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 5a1a131b..35feda82 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -16,7 +16,7 @@ jobs: strategy: matrix: - php-versions: ['7.1', '7.4', '8.1'] + php-versions: ['8.1'] name: PHP ${{ matrix.php-versions }} Test @@ -44,7 +44,7 @@ jobs: run: bash bin/opus4db --sqlpwd root --adminpwd root --userpwd root - name: Setup OPUS 4 - run: ant prepare-workspace prepare-config create-database lint -DdbUserPassword=root -DdbAdminPassword=root + run: ant prepare-workspace create-database lint - name: Tests run: php composer.phar test diff --git a/.gitignore b/.gitignore index 3cc37921..b4802909 100644 --- a/.gitignore +++ b/.gitignore @@ -13,9 +13,7 @@ db/config.sh vendor # Local test files -tests/build -tests/config.ini -tests/config.ini.backup +database.ini* .phpunit.result.cache # IntelliJ IDEA files diff --git a/Vagrantfile b/Vagrantfile index c686f44e..3af701e4 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -31,18 +31,17 @@ $composer = <