From 03163fec1d60e86697985669dda8fbc76ee74abc Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 5 May 2023 18:14:50 +0200 Subject: [PATCH 01/16] #43 Basic scaffolding for import rules --- src/ImportRuleConditionInterface.php | 36 +++++ src/ImportRuleInterface.php | 36 +++++ src/ImportRules.php | 85 ++++++++++++ src/Importer.php | 2 + src/Rules/AddCollection.php | 70 ++++++++++ src/Rules/Conditions/AccountCondition.php | 52 +++++++ src/Rules/Options/CollectionOption.php | 126 +++++++++++++++++ src/Sword/ImportCollection.php | 5 + test/ImportRulesTest.php | 95 +++++++++++++ test/Rules/AddCollectionTest.php | 42 ++++++ test/Rules/Options/CollectionOptionTest.php | 144 ++++++++++++++++++++ 11 files changed, 693 insertions(+) create mode 100644 src/ImportRuleConditionInterface.php create mode 100644 src/ImportRuleInterface.php create mode 100644 src/ImportRules.php create mode 100644 src/Rules/AddCollection.php create mode 100644 src/Rules/Conditions/AccountCondition.php create mode 100644 src/Rules/Options/CollectionOption.php create mode 100644 test/ImportRulesTest.php create mode 100644 test/Rules/AddCollectionTest.php create mode 100644 test/Rules/Options/CollectionOptionTest.php diff --git a/src/ImportRuleConditionInterface.php b/src/ImportRuleConditionInterface.php new file mode 100644 index 0000000..b2af759 --- /dev/null +++ b/src/ImportRuleConditionInterface.php @@ -0,0 +1,36 @@ +getConfig(); + + if (isset($config->import->rules)) { + $rulesConfig = $config->import->rules->toArray(); + + foreach ($rulesConfig as $name => $options) { + $type = $options['type']; + + $rule = $this->createRule($type, $options); + + $this->rules[] = $rule; + } + } + } + + /** + * @return ImportRuleInterface[] + */ + public function getRules() + { + return $this->rules; + } + + /** + * @param string $type + * @param array $options + * @return ImportRuleInterface|null + */ + public function createRule($type, $options) + { + $ruleClass = "Opus\Import\Rules\\${type}"; + + $rule = new $ruleClass(); + $rule->setOptions($options); + + return $rule; + } +} diff --git a/src/Importer.php b/src/Importer.php index 6fafe3d..059259c 100644 --- a/src/Importer.php +++ b/src/Importer.php @@ -306,6 +306,8 @@ public function run() $doc->addCollection($this->importCollection); } + // TODO add aditional collections + try { $doc->store(); $this->document = $doc; diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php new file mode 100644 index 0000000..dd4aaf4 --- /dev/null +++ b/src/Rules/AddCollection.php @@ -0,0 +1,70 @@ +condition = new RuleCondition($options['condition']); + } + + if (isset($options['collection'])) { + $this->collection = new CollectionOption($options['collection']); + } + } + + /** + * @return CollectionInterface|null + */ + public function getCollection() + { + return $this->collection->getCollection() ?? null; + } +} diff --git a/src/Rules/Conditions/AccountCondition.php b/src/Rules/Conditions/AccountCondition.php new file mode 100644 index 0000000..fa4c743 --- /dev/null +++ b/src/Rules/Conditions/AccountCondition.php @@ -0,0 +1,52 @@ +setOptions($options); + } + + /** + * @param array|null $options + */ + public function setOptions($options) + { + } +} diff --git a/src/Rules/Options/CollectionOption.php b/src/Rules/Options/CollectionOption.php new file mode 100644 index 0000000..d51f2b3 --- /dev/null +++ b/src/Rules/Options/CollectionOption.php @@ -0,0 +1,126 @@ +setOptions($options); + } + + /** + * @param string[]|null $options + */ + public function setOptions($options) + { + $this->options = $options; + } + + /** + * @return CollectionInterface|null + */ + public function getCollection() + { + if ($this->collection === null) { + $this->collection = $this->loadCollection(); + } + return $this->collection; + } + + /** + * @return CollectionInterface|null + * @throws NotFoundException + */ + protected function loadCollection() + { + // if collection ID is configured, load collection directly + if (isset($this->options['id']) && is_numeric($this->options['id'])) { + return Collection::get($this->options['id']); + } + + $role = null; + + // try to get collection role ('roleName' or 'roleOaiName') + if (isset($this->options['roleName'])) { + $role = CollectionRole::fetchByName($this->options['roleName']); + } elseif (isset($this->options['roleOaiName'])) { + $role = CollectionRole::fetchByOaiName($this->options['roleOaiName']); + } + + if ($role === null) { + // TODO log, throw exception? + return null; + } + + $collections = []; + + // try to get collection ('number' or 'name') + if (isset($this->options['number'])) { + $collections = Collection::fetchCollectionsByRoleNumber($role->getId(), $this->options['number']); + } elseif (isset($this->options['name'])) { + $collections = Collection::fetchCollectionsByRoleName($role->getId(), $this->options['name']); + } + + if (count($collections) > 0) { + return $collections[0]; + } + + return null; + } +} diff --git a/src/Sword/ImportCollection.php b/src/Sword/ImportCollection.php index 6e67d6c..9872c8f 100644 --- a/src/Sword/ImportCollection.php +++ b/src/Sword/ImportCollection.php @@ -40,6 +40,11 @@ use function count; use function trim; +/** + * TODO rename this class - is is not a collection, it is a helper class for getting the "import collection" - Is this + * class necessary at all? Couldn't it be just a function, somewhere? What is the purpose, the reasoning behind + * it? + */ class ImportCollection { /** @var CollectionInterface */ diff --git a/test/ImportRulesTest.php b/test/ImportRulesTest.php new file mode 100644 index 0000000..a245122 --- /dev/null +++ b/test/ImportRulesTest.php @@ -0,0 +1,95 @@ +prepareCollections(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addCol' => [ + 'type' => 'AddCollection', + 'collection' => [ + 'id' => $this->colId, + ], + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $rules = $importRules->getRules(); + + $this->assertCount(1, $rules); + + $rule = $rules[0]; + + $this->assertInstanceOf(AddCollection::class, $rule); + + $col = $rule->getCollection(); + + $this->assertInstanceOf(CollectionInterface::class, $col); + $this->assertEquals('col1', $col->getName()); + } + + protected function prepareCollections() + { + $role = CollectionRole::new(); + $role->setName('import'); + $role->setOaiName('oaiImport'); + $role->addRootCollection(); + + $col = Collection::new(); + $col->setName('col1'); + $col->setNumber('col1number'); + $role->getRootCollection()->addFirstChild($col); + $role->store(); + + $this->colId = $col->getId(); + } +} diff --git a/test/Rules/AddCollectionTest.php b/test/Rules/AddCollectionTest.php new file mode 100644 index 0000000..58c634c --- /dev/null +++ b/test/Rules/AddCollectionTest.php @@ -0,0 +1,42 @@ +prepareCollections(); + + $colId = $this->colId; + + $option = new CollectionOption(); + $option->setOptions([ + 'id' => $colId, + ]); + + $col = $option->getCollection(); + + $this->assertNotNull($col); + $this->assertEquals($colId, $col->getId()); + } + + public function testConfigRoleNameColNumber() + { + $this->prepareCollections(); + + $option = new CollectionOption(); + $option->setOptions([ + 'roleName' => 'import', + 'number' => 'col1number', + ]); + + $col = $option->getCollection(); + + $this->assertNotNull($col); + $this->assertEquals($this->colId, $col->getId()); + } + + public function testConfigRoleNameColName() + { + $this->prepareCollections(); + + $option = new CollectionOption(); + $option->setOptions([ + 'roleName' => 'import', + 'name' => 'col1', + ]); + + $col = $option->getCollection(); + + $this->assertNotNull($col); + $this->assertEquals($this->colId, $col->getId()); + } + + public function testConfigRoleOaiNameColNumber() + { + $this->prepareCollections(); + + $option = new CollectionOption(); + $option->setOptions([ + 'roleOaiName' => 'oaiImport', + 'number' => 'col1number', + ]); + + $col = $option->getCollection(); + + $this->assertNotNull($col); + $this->assertEquals($this->colId, $col->getId()); + } + + public function testConfigRoleOaiNameColName() + { + $this->prepareCollections(); + + $option = new CollectionOption(); + $option->setOptions([ + 'roleOaiName' => 'oaiImport', + 'name' => 'col1', + ]); + + $col = $option->getCollection(); + + $this->assertNotNull($col); + $this->assertEquals($this->colId, $col->getId()); + } + + protected function prepareCollections() + { + $role = CollectionRole::new(); + $role->setName('import'); + $role->setOaiName('oaiImport'); + $role->addRootCollection(); + $this->roleId = $role->store(); + + $col = Collection::new(); + $col->setName('col1'); + $col->setNumber('col1number'); + $role->getRootCollection()->addFirstChild($col); + $role->store(); + + $this->colId = $col->getId(); + } +} From a55bfba927cc2223ff6264a345f2befe586a0751 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 5 May 2023 18:17:50 +0200 Subject: [PATCH 02/16] #43 Depend on development version of opus4-common --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6f446ce..54bb068 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,7 @@ "ext-fileinfo": "*", "ext-libxml": "*", "ext-zip": "*", - "opus4-repo/opus4-common": "^4.8", + "opus4-repo/opus4-common": "dev-master as 4.8", "opus4-repo/opus4-job": "^4.8" }, "require-dev": { From c15360631c3edc6a3899ad6d980c4b63e04f8591 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Fri, 5 May 2023 18:25:19 +0200 Subject: [PATCH 03/16] #43 Checking condition --- src/ImportRuleConditionInterface.php | 4 ++++ src/Rules/AddCollection.php | 8 ++++++++ src/Rules/Conditions/AccountCondition.php | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/src/ImportRuleConditionInterface.php b/src/ImportRuleConditionInterface.php index b2af759..d688cf7 100644 --- a/src/ImportRuleConditionInterface.php +++ b/src/ImportRuleConditionInterface.php @@ -33,4 +33,8 @@ interface ImportRuleConditionInterface { + /** + * @return bool + */ + public function applies(); } diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php index dd4aaf4..2769e9a 100644 --- a/src/Rules/AddCollection.php +++ b/src/Rules/AddCollection.php @@ -67,4 +67,12 @@ public function getCollection() { return $this->collection->getCollection() ?? null; } + + public function apply() + { + if ($this->condition->applies()) { + $col = $this->getCollection(); + // TODO apply collection to document + } + } } diff --git a/src/Rules/Conditions/AccountCondition.php b/src/Rules/Conditions/AccountCondition.php index fa4c743..faf8be9 100644 --- a/src/Rules/Conditions/AccountCondition.php +++ b/src/Rules/Conditions/AccountCondition.php @@ -49,4 +49,12 @@ public function __construct($options = null) public function setOptions($options) { } + + /** + * @return bool + */ + public function applies() + { + return true; + } } From 0406327732f2641c14a0b25da488267ab20bea57 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 8 May 2023 15:53:17 +0200 Subject: [PATCH 04/16] #43 AccountCondition implemented --- src/Rules/AddCollection.php | 6 +- src/Rules/Conditions/AccountCondition.php | 43 ++++++++- .../Rules/Conditions/AccountConditionTest.php | 94 +++++++++++++++++++ test/TestAsset/MockAuthAdapter.php | 58 ++++++++++++ 4 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 test/Rules/Conditions/AccountConditionTest.php create mode 100644 test/TestAsset/MockAuthAdapter.php diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php index 2769e9a..06771a0 100644 --- a/src/Rules/AddCollection.php +++ b/src/Rules/AddCollection.php @@ -33,6 +33,7 @@ use Opus\Common\CollectionInterface; use Opus\Import\ImportRuleInterface; +use Opus\Import\Rules\Conditions\AccountCondition; use Opus\Import\Rules\Options\CollectionOption; /** @@ -52,7 +53,10 @@ class AddCollection implements ImportRuleInterface public function setOptions($options) { if (isset($options['condition'])) { - $this->condition = new RuleCondition($options['condition']); + $condition = $options['condition']; + if (isset($condition['account'])) { + $this->condition = new AccountCondition($condition); + } } if (isset($options['collection'])) { diff --git a/src/Rules/Conditions/AccountCondition.php b/src/Rules/Conditions/AccountCondition.php index faf8be9..191375a 100644 --- a/src/Rules/Conditions/AccountCondition.php +++ b/src/Rules/Conditions/AccountCondition.php @@ -32,9 +32,13 @@ namespace Opus\Import\Rules\Conditions; use Opus\Import\ImportRuleConditionInterface; +use Zend_Auth; // TODO SECURITY depend on OPUS classes instead class AccountCondition implements ImportRuleConditionInterface { + /** @var string|null */ + protected $expectedUser; + /** * @param array|null $options */ @@ -48,6 +52,9 @@ public function __construct($options = null) */ public function setOptions($options) { + if (isset($options['account'])) { + $this->expectedUser = $options['account']; + } } /** @@ -55,6 +62,40 @@ public function setOptions($options) */ public function applies() { - return true; + $currentUser = $this->getUserName(); + if ($this->expectedUser !== null && $currentUser !== null) { + return strcasecmp($this->expectedUser, $currentUser) === 0; + } else { + return false; + } + } + + /** + * @return string|null + */ + protected function getUserName() + { + $identity = Zend_Auth::getInstance()->getIdentity(); + if (isset($identity['username'])) { + return $identity['username']; + } else { + return null; + } + } + + /** + * @return string|null + */ + public function getExpectedUser() + { + return $this->expectedUser; + } + + /** + * @param string|null $user + */ + public function setExpectedUser($user) + { + $this->expectedUser = $user; } } diff --git a/test/Rules/Conditions/AccountConditionTest.php b/test/Rules/Conditions/AccountConditionTest.php new file mode 100644 index 0000000..6514d86 --- /dev/null +++ b/test/Rules/Conditions/AccountConditionTest.php @@ -0,0 +1,94 @@ +authStorage = new Zend_Auth_Storage_NonPersistent(); + Zend_Auth::getInstance()->setStorage($this->authStorage); + } + + public function tearDown(): void + { + Zend_Auth::getInstance() + ->setStorage($this->authStorage) + ->clearIdentity(); + parent::tearDown(); + } + + public function testConstruct() + { + $condition = new AccountCondition([ + 'account' => 'sword1' + ]); + + $this->assertEquals('sword1', $condition->getExpectedUser()); + } + + public function testAppliesTrue() + { + Zend_Auth::getInstance()->authenticate(new MockAuthAdapter('sword1')); + + $condition = new AccountCondition([ + 'account' => 'sword1' + ]); + + $this->assertTrue($condition->applies()); + } + + public function testAppliesFalse() + { + Zend_Auth::getInstance()->authenticate(new MockAuthAdapter('sword2')); + + $condition = new AccountCondition([ + 'account' => 'sword1' + ]); + + $this->assertFalse($condition->applies()); + + $condition->setExpectedUser('sword2'); + + $this->assertTrue($condition->applies()); + } +} diff --git a/test/TestAsset/MockAuthAdapter.php b/test/TestAsset/MockAuthAdapter.php new file mode 100644 index 0000000..da6846a --- /dev/null +++ b/test/TestAsset/MockAuthAdapter.php @@ -0,0 +1,58 @@ +user = $user; + } + + /** + * @return Zend_Auth_Result + */ + public function authenticate() + { + $identity = ['username'=> $this->user]; + return new Zend_Auth_Result(Zend_Auth_Result::SUCCESS, $identity); + } +} From bad0916827b6c6614d894846f6e9af8065390d8d Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 8 May 2023 15:55:13 +0200 Subject: [PATCH 05/16] #43 Coding style --- src/Rules/Conditions/AccountCondition.php | 6 +++++- test/Rules/Conditions/AccountConditionTest.php | 8 +++++--- test/TestAsset/MockAuthAdapter.php | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Rules/Conditions/AccountCondition.php b/src/Rules/Conditions/AccountCondition.php index 191375a..7ca0433 100644 --- a/src/Rules/Conditions/AccountCondition.php +++ b/src/Rules/Conditions/AccountCondition.php @@ -32,7 +32,11 @@ namespace Opus\Import\Rules\Conditions; use Opus\Import\ImportRuleConditionInterface; -use Zend_Auth; // TODO SECURITY depend on OPUS classes instead +use Zend_Auth; + +use function strcasecmp; + +// TODO SECURITY depend on OPUS classes instead class AccountCondition implements ImportRuleConditionInterface { diff --git a/test/Rules/Conditions/AccountConditionTest.php b/test/Rules/Conditions/AccountConditionTest.php index 6514d86..026cafc 100644 --- a/test/Rules/Conditions/AccountConditionTest.php +++ b/test/Rules/Conditions/AccountConditionTest.php @@ -35,10 +35,12 @@ use OpusTest\Import\TestAsset\MockAuthAdapter; use OpusTest\Import\TestAsset\TestCase; use Zend_Auth; +use Zend_Auth_Storage_Interface; use Zend_Auth_Storage_NonPersistent; class AccountConditionTest extends TestCase { + /** @var Zend_Auth_Storage_Interface */ private $authStorage; public function setUp(): void @@ -60,7 +62,7 @@ public function tearDown(): void public function testConstruct() { $condition = new AccountCondition([ - 'account' => 'sword1' + 'account' => 'sword1', ]); $this->assertEquals('sword1', $condition->getExpectedUser()); @@ -71,7 +73,7 @@ public function testAppliesTrue() Zend_Auth::getInstance()->authenticate(new MockAuthAdapter('sword1')); $condition = new AccountCondition([ - 'account' => 'sword1' + 'account' => 'sword1', ]); $this->assertTrue($condition->applies()); @@ -82,7 +84,7 @@ public function testAppliesFalse() Zend_Auth::getInstance()->authenticate(new MockAuthAdapter('sword2')); $condition = new AccountCondition([ - 'account' => 'sword1' + 'account' => 'sword1', ]); $this->assertFalse($condition->applies()); diff --git a/test/TestAsset/MockAuthAdapter.php b/test/TestAsset/MockAuthAdapter.php index da6846a..007d387 100644 --- a/test/TestAsset/MockAuthAdapter.php +++ b/test/TestAsset/MockAuthAdapter.php @@ -52,7 +52,7 @@ public function __construct($user) */ public function authenticate() { - $identity = ['username'=> $this->user]; + $identity = ['username' => $this->user]; return new Zend_Auth_Result(Zend_Auth_Result::SUCCESS, $identity); } } From 82742e086d5171478162c9d168e1030a91ece24c Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 9 May 2023 18:22:58 +0200 Subject: [PATCH 06/16] #43 Add collection rule --- src/ImportRules.php | 21 ++++- src/Rules/AddCollection.php | 13 +++- test/ImportRulesTest.php | 34 ++++++++ test/Rules/AddCollectionTest.php | 128 ++++++++++++++++++++++++++++++- 4 files changed, 191 insertions(+), 5 deletions(-) diff --git a/src/ImportRules.php b/src/ImportRules.php index 5e55454..42b55c0 100644 --- a/src/ImportRules.php +++ b/src/ImportRules.php @@ -32,11 +32,16 @@ namespace Opus\Import; use Opus\Common\ConfigTrait; +use Opus\Common\DocumentInterface; + +use function class_exists; class ImportRules { use ConfigTrait; + public const IMPORT_RULE_CLASS_PREFIX = 'Opus\\Import\\Rules\\'; + /** @var ImportRuleInterface[] */ private $rules = []; @@ -75,11 +80,25 @@ public function getRules() */ public function createRule($type, $options) { - $ruleClass = "Opus\Import\Rules\\${type}"; + if (class_exists($type, false)) { + $ruleClass = $type; + } else { + $ruleClass = self::IMPORT_RULE_CLASS_PREFIX . $type; + } $rule = new $ruleClass(); $rule->setOptions($options); return $rule; } + + /** + * @param DocumentInterface $document + */ + public function apply($document) + { + foreach ($this->rules as $rule) { + $rule->apply($document); + } + } } diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php index 06771a0..1936fe9 100644 --- a/src/Rules/AddCollection.php +++ b/src/Rules/AddCollection.php @@ -32,6 +32,8 @@ namespace Opus\Import\Rules; use Opus\Common\CollectionInterface; +use Opus\Common\DocumentInterface; +use Opus\Import\ImportRuleConditionInterface; use Opus\Import\ImportRuleInterface; use Opus\Import\Rules\Conditions\AccountCondition; use Opus\Import\Rules\Options\CollectionOption; @@ -72,11 +74,16 @@ public function getCollection() return $this->collection->getCollection() ?? null; } - public function apply() + /** + * @param DocumentInterface $document + */ + public function apply($document) { - if ($this->condition->applies()) { + if ($this->condition === null || $this->condition->applies()) { $col = $this->getCollection(); - // TODO apply collection to document + if ($col !== null) { + $document->addCollection($col); + } } } } diff --git a/test/ImportRulesTest.php b/test/ImportRulesTest.php index a245122..381b7dd 100644 --- a/test/ImportRulesTest.php +++ b/test/ImportRulesTest.php @@ -77,6 +77,40 @@ public function testInit() $this->assertEquals('col1', $col->getName()); } + public function testConfigFullClassname() + { + $this->prepareCollections(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addCol' => [ + 'type' => AddCollection::class, + 'collection' => [ + 'id' => $this->colId, + ], + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $rules = $importRules->getRules(); + + $this->assertCount(1, $rules); + + $rule = $rules[0]; + + $this->assertInstanceOf(AddCollection::class, $rule); + + $col = $rule->getCollection(); + + $this->assertInstanceOf(CollectionInterface::class, $col); + $this->assertEquals('col1', $col->getName()); + } + protected function prepareCollections() { $role = CollectionRole::new(); diff --git a/test/Rules/AddCollectionTest.php b/test/Rules/AddCollectionTest.php index 58c634c..e53e6bd 100644 --- a/test/Rules/AddCollectionTest.php +++ b/test/Rules/AddCollectionTest.php @@ -31,12 +31,138 @@ namespace OpusTest\Import\Rules; +use Opus\Common\Collection; +use Opus\Common\CollectionRole; +use Opus\Common\Document; +use Opus\Import\ImportRules; +use Opus\Import\Rules\AddCollection; +use OpusTest\Import\TestAsset\MockAuthAdapter; use OpusTest\Import\TestAsset\TestCase; +use Zend_Auth; +use Zend_Auth_Storage_Interface; +use Zend_Auth_Storage_NonPersistent; class AddCollectionTest extends TestCase { + /** @var Zend_Auth_Storage_Interface */ + private $authStorage; + + public function setUp(): void + { + parent::setUp(); + + $this->authStorage = new Zend_Auth_Storage_NonPersistent(); + Zend_Auth::getInstance()->setStorage($this->authStorage); + } + + public function tearDown(): void + { + Zend_Auth::getInstance() + ->setStorage($this->authStorage) + ->clearIdentity(); + parent::tearDown(); + } + public function testAddCollection() { - // TODO test + $this->prepareCollections(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addCol' => [ + 'type' => AddCollection::class, + 'collection' => [ + 'id' => $this->colId, + ], + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $doc = Document::new(); + + $importRules->apply($doc); + + $collections = $doc->getCollection(); + + $this->assertCount(1, $collections); + $this->assertEquals($this->colId, $collections[0]->getId()); + } + + public function testAddCollectionForAccount() + { + $this->prepareCollections(); + + $col1 = Collection::get($this->colId); + $role = $col1->getRole(); + + $col1id = $col1->getId(); + + $col2 = Collection::new(); + $col2->setName('col1'); + $col2->setNumber('col1number'); + $role->getRootCollection()->addFirstChild($col2); + $role->store(); + + $col2id = $col2->getId(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addCol1' => [ + 'type' => 'AddCollection', + 'condition' => [ + 'account' => 'sword1', + ], + 'collection' => [ + 'id' => $this->colId, + ], + ], + 'addCol2' => [ + 'type' => 'AddCollection', + 'condition' => [ + 'account' => 'sword2', + ], + 'collection' => [ + 'id' => $col2id, + ], + ], + ], + ], + ]); + + $rules = new ImportRules(); + $rules->init(); + + Zend_Auth::getInstance()->authenticate(new MockAuthAdapter('sword1')); + + $doc = Document::new(); + + $rules->apply($doc); + + $collections = $doc->getCollection(); + + $this->assertCount(1, $collections); + $this->assertEquals($col1id, $collections[0]->getId()); + } + + protected function prepareCollections() + { + $role = CollectionRole::new(); + $role->setName('import'); + $role->setOaiName('oaiImport'); + $role->addRootCollection(); + + $col = Collection::new(); + $col->setName('col1'); + $col->setNumber('col1number'); + $role->getRootCollection()->addFirstChild($col); + $role->store(); + + $this->colId = $col->getId(); } } From f64e02515bcacbc983c9243b26d3652e360a6afe Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 25 May 2023 16:43:31 +0200 Subject: [PATCH 07/16] #43 Add keyword condition --- src/ImportRuleConditionInterface.php | 5 +- src/Rules/AddCollection.php | 1 + src/Rules/Conditions/AccountCondition.php | 4 +- src/Rules/Conditions/KeywordCondition.php | 117 ++++++++++++++++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 src/Rules/Conditions/KeywordCondition.php diff --git a/src/ImportRuleConditionInterface.php b/src/ImportRuleConditionInterface.php index d688cf7..b0a9206 100644 --- a/src/ImportRuleConditionInterface.php +++ b/src/ImportRuleConditionInterface.php @@ -31,10 +31,13 @@ namespace Opus\Import; +use Opus\Common\DocumentInterface; + interface ImportRuleConditionInterface { /** + * @param DocumentInterface $document * @return bool */ - public function applies(); + public function applies($document); } diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php index 1936fe9..fd3b22a 100644 --- a/src/Rules/AddCollection.php +++ b/src/Rules/AddCollection.php @@ -56,6 +56,7 @@ public function setOptions($options) { if (isset($options['condition'])) { $condition = $options['condition']; + // TODO support KeywordCondition if (isset($condition['account'])) { $this->condition = new AccountCondition($condition); } diff --git a/src/Rules/Conditions/AccountCondition.php b/src/Rules/Conditions/AccountCondition.php index 7ca0433..141ac07 100644 --- a/src/Rules/Conditions/AccountCondition.php +++ b/src/Rules/Conditions/AccountCondition.php @@ -31,6 +31,7 @@ namespace Opus\Import\Rules\Conditions; +use Opus\Common\DocumentInterface; use Opus\Import\ImportRuleConditionInterface; use Zend_Auth; @@ -62,9 +63,10 @@ public function setOptions($options) } /** + * @param DocumentInterface $document * @return bool */ - public function applies() + public function applies($document) { $currentUser = $this->getUserName(); if ($this->expectedUser !== null && $currentUser !== null) { diff --git a/src/Rules/Conditions/KeywordCondition.php b/src/Rules/Conditions/KeywordCondition.php new file mode 100644 index 0000000..9aca83a --- /dev/null +++ b/src/Rules/Conditions/KeywordCondition.php @@ -0,0 +1,117 @@ +setOptions($options); + } + + /** + * @param array|null $options + */ + public function setOptions($options) + { + if (isset($options['keyword'])) { + $this->expectedUser = $options['keyword']; + } + } + + /** + * @param DocumentInterface $document + * @return bool + */ + public function applies($document) + { + $keywords = $this->getKeywords($document); + + if ($this->expectedKeyword !== null && $keywords !== null) { + foreach ($keywords as $keyword) { + if (strcasecmp($this->expectedKeyword, $keyword) === 0) { + return true; + } + } + } + + return false; + } + + /** + * @param DocumentInterface $document + * @return string[] + */ + protected function getKeywords($document) + { + $keywords = []; + $subjects = $document->getSubject(); + + foreach ($subjects as $subject) { + $keywords[] = $subject->getValue(); + } + + return $keywords; + } + + /** + * @return string|null + */ + public function getExpectedKeyword() + { + return $this->expectedKeyword; + } + + /** + * @param string|null $keyword + */ + public function setExpectedKeyword($keyword) + { + $this->expectedKeyword = $keyword; + } +} From 6695a42f774b4dc1aa23395ba6e5d1fd618a5d06 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 7 Aug 2023 11:38:25 +0200 Subject: [PATCH 08/16] #43 Adding more import rules and tests --- composer.json | 2 +- src/ImportRules.php | 39 ++- src/Rules/AbstractImportRule.php | 80 ++++++ src/Rules/AddCollection.php | 23 +- src/Rules/AddKeyword.php | 82 ++++++ src/Rules/AddLicence.php | 79 ++++++ src/Rules/Conditions/KeywordCondition.php | 113 ++++++-- src/Rules/RemoveKeyword.php | 72 +++++ test/ImportRulesTest.php | 31 ++- test/ImporterTest.php | 92 +++++++ test/Rules/AddLicenceTest.php | 182 +++++++++++++ .../Rules/Conditions/AccountConditionTest.php | 11 +- .../Rules/Conditions/KeywordConditionTest.php | 257 ++++++++++++++++++ test/Xml/XmlDocumentTest.php | 2 +- test/_files/import-rules-test.xml | 28 ++ test/_files/import-rules.ini | 6 + test/test.ini | 3 + 17 files changed, 1051 insertions(+), 51 deletions(-) create mode 100644 src/Rules/AbstractImportRule.php create mode 100644 src/Rules/AddKeyword.php create mode 100644 src/Rules/AddLicence.php create mode 100644 src/Rules/RemoveKeyword.php create mode 100644 test/Rules/AddLicenceTest.php create mode 100644 test/Rules/Conditions/KeywordConditionTest.php create mode 100644 test/_files/import-rules-test.xml create mode 100644 test/_files/import-rules.ini diff --git a/composer.json b/composer.json index 54bb068..0922235 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "phpunit/phpunit": "<9", "opus4-repo/codesniffer": "dev-laminas", "phpmetrics/phpmetrics": "2.7.4", - "opus4-repo/framework": "^4.8" + "opus4-repo/framework": "dev-master as 4.8" }, "config": { "allow-plugins": { diff --git a/src/ImportRules.php b/src/ImportRules.php index 42b55c0..a2f6eb1 100644 --- a/src/ImportRules.php +++ b/src/ImportRules.php @@ -33,8 +33,14 @@ use Opus\Common\ConfigTrait; use Opus\Common\DocumentInterface; +use Zend_Config_Ini; use function class_exists; +use function filter_var; +use function is_array; +use function is_readable; + +use const FILTER_VALIDATE_BOOLEAN; class ImportRules { @@ -52,15 +58,38 @@ public function init() { $config = $this->getConfig(); - if (isset($config->import->rules)) { + if ( + ! isset($config->sword->enableImportRules) || + ! filter_var($config->sword->enableImportRules, FILTER_VALIDATE_BOOLEAN) + ) { + // TODO does this belong here? There should not be anything SWORD specific here! + return; // don't load any rules + } + + $rulesConfig = null; + + if (isset($config->import->rulesConfigFile)) { + $rulesConfigFile = $config->import->rulesConfigFile; + if (is_readable($rulesConfigFile)) { + $rulesConfig = new Zend_Config_Ini($rulesConfigFile); + $rulesConfig = $rulesConfig->toArray(); + } + } + + // Get rules from main configuration as fallback + if ($rulesConfig === null && isset($config->import->rules)) { $rulesConfig = $config->import->rules->toArray(); + } + if (is_array($rulesConfig)) { foreach ($rulesConfig as $name => $options) { $type = $options['type']; $rule = $this->createRule($type, $options); - $this->rules[] = $rule; + if ($rule !== null) { + $this->rules[] = $rule; + } } } } @@ -80,10 +109,14 @@ public function getRules() */ public function createRule($type, $options) { - if (class_exists($type, false)) { + if (class_exists($type)) { $ruleClass = $type; } else { $ruleClass = self::IMPORT_RULE_CLASS_PREFIX . $type; + if (! class_exists($ruleClass)) { + // TODO throw exception + return null; + } } $rule = new $ruleClass(); diff --git a/src/Rules/AbstractImportRule.php b/src/Rules/AbstractImportRule.php new file mode 100644 index 0000000..631dc69 --- /dev/null +++ b/src/Rules/AbstractImportRule.php @@ -0,0 +1,80 @@ +condition = new AccountCondition($condition); + } + if (isset($condition['keyword'])) { + $this->condition = new KeywordCondition($condition); + } + } + } + + /** + * @return ImportRuleConditionInterface + */ + public function getCondition() + { + return $this->condition; + } + + /** + * @param DocumentInterface $document + */ + public function apply($document) + { + // TODO condition check in base class? + } +} diff --git a/src/Rules/AddCollection.php b/src/Rules/AddCollection.php index fd3b22a..927c7c3 100644 --- a/src/Rules/AddCollection.php +++ b/src/Rules/AddCollection.php @@ -33,34 +33,19 @@ use Opus\Common\CollectionInterface; use Opus\Common\DocumentInterface; -use Opus\Import\ImportRuleConditionInterface; -use Opus\Import\ImportRuleInterface; -use Opus\Import\Rules\Conditions\AccountCondition; use Opus\Import\Rules\Options\CollectionOption; -/** - * TODO add base class for common code - */ -class AddCollection implements ImportRuleInterface +class AddCollection extends AbstractImportRule { /** @var CollectionOption */ private $collection; - /** @var ImportRuleConditionInterface */ - private $condition; - /** * @param array $options */ public function setOptions($options) { - if (isset($options['condition'])) { - $condition = $options['condition']; - // TODO support KeywordCondition - if (isset($condition['account'])) { - $this->condition = new AccountCondition($condition); - } - } + parent::setOptions($options); if (isset($options['collection'])) { $this->collection = new CollectionOption($options['collection']); @@ -80,7 +65,9 @@ public function getCollection() */ public function apply($document) { - if ($this->condition === null || $this->condition->applies()) { + $condition = $this->getCondition(); + + if ($condition === null || $condition->applies($document)) { $col = $this->getCollection(); if ($col !== null) { $document->addCollection($col); diff --git a/src/Rules/AddKeyword.php b/src/Rules/AddKeyword.php new file mode 100644 index 0000000..7a0cb05 --- /dev/null +++ b/src/Rules/AddKeyword.php @@ -0,0 +1,82 @@ +setValue($options['keyword']); + $subject->setType(Subject::TYPE_UNCONTROLLED); + $this->subject = $subject; + } + } + + /** + * @return SubjectInterface|null + */ + public function getSubject() + { + return $this->subject; + } + + /** + * @param DocumentInterface $document + */ + public function apply($document) + { + $condition = $this->getCondition(); + if ($condition === null || $condition->applies($document)) { + $subject = $this->getSubject(); + if ($subject !== null) { + $document->addSubject($subject); + } + } + } +} diff --git a/src/Rules/AddLicence.php b/src/Rules/AddLicence.php new file mode 100644 index 0000000..1fab238 --- /dev/null +++ b/src/Rules/AddLicence.php @@ -0,0 +1,79 @@ +licence = Licence::get($options['licenceId']); + } + } + + /** + * @return LicenceInterface|null + */ + public function getLicence() + { + return $this->licence; + } + + /** + * @param DocumentInterface $document + */ + public function apply($document) + { + $condition = $this->getCondition(); + if ($condition === null || $condition->applies($document)) { + $licence = $this->getLicence(); + if ($licence !== null) { + $document->addLicence($licence); + } + } + } +} diff --git a/src/Rules/Conditions/KeywordCondition.php b/src/Rules/Conditions/KeywordCondition.php index 9aca83a..dc9e5ad 100644 --- a/src/Rules/Conditions/KeywordCondition.php +++ b/src/Rules/Conditions/KeywordCondition.php @@ -34,7 +34,10 @@ use Opus\Common\DocumentInterface; use Opus\Import\ImportRuleConditionInterface; -use function strcasecmp; +use function filter_var; +use function is_array; + +use const FILTER_VALIDATE_BOOLEAN; /** * Checks if a keyword/subject is present in a document. @@ -44,7 +47,16 @@ class KeywordCondition implements ImportRuleConditionInterface { /** @var string|null */ - protected $expectedKeyword; + private $expectedKeyword; + + /** @var string|null */ + private $keywordType; + + /** @var bool */ + private $removeKeyword = false; + + /** @var bool */ + private $caseSensitive = false; /** * @param array|null $options @@ -60,58 +72,113 @@ public function __construct($options = null) public function setOptions($options) { if (isset($options['keyword'])) { - $this->expectedUser = $options['keyword']; + $keyword = $options['keyword']; + if (is_array($keyword)) { + $this->expectedKeyword = $keyword['value'] ?? null; + $this->keywordType = $keyword['type'] ?? null; + if (isset($keyword['remove'])) { + $this->removeKeyword = filter_var($keyword['remove'], FILTER_VALIDATE_BOOLEAN); + } + if (isset($keyword['caseSensitive'])) { + $this->caseSensitive = filter_var($keyword['caseSensitive'], FILTER_VALIDATE_BOOLEAN); + } + } else { + $this->expectedKeyword = $options['keyword']; + } } } /** * @param DocumentInterface $document * @return bool + * + * TODO remove keywords */ public function applies($document) { - $keywords = $this->getKeywords($document); + $caseSensitive = $this->isCaseSensitive(); + $expectedKeyword = $this->getExpectedKeyword(); + $keywordType = $this->getKeywordType(); - if ($this->expectedKeyword !== null && $keywords !== null) { - foreach ($keywords as $keyword) { - if (strcasecmp($this->expectedKeyword, $keyword) === 0) { - return true; - } + if ($document->hasSubject($expectedKeyword, $keywordType, $caseSensitive)) { + if ($this->isRemoveEnabled()) { + $document->removeSubject($expectedKeyword, $keywordType, $caseSensitive); } + return true; } return false; } /** - * @param DocumentInterface $document - * @return string[] + * @return string|null */ - protected function getKeywords($document) + public function getExpectedKeyword() { - $keywords = []; - $subjects = $document->getSubject(); + return $this->expectedKeyword; + } - foreach ($subjects as $subject) { - $keywords[] = $subject->getValue(); - } + /** + * @param string|null $keyword + * @return $this + */ + public function setExpectedKeyword($keyword) + { + $this->expectedKeyword = $keyword; + return $this; + } + + /** + * @return bool + */ + public function isRemoveEnabled() + { + return $this->removeKeyword; + } - return $keywords; + /** + * @param bool $enabled + * @return $this + */ + public function setRemoveEnabled($enabled) + { + $this->removeKeyword = $enabled; + return $this; + } + + /** + * @return bool + */ + public function isCaseSensitive() + { + return $this->caseSensitive; + } + + /** + * @param bool $enabled + * @return $this + */ + public function setCaseSensitive($enabled) + { + $this->caseSensitive = $enabled; + return $this; } /** * @return string|null */ - public function getExpectedKeyword() + public function getKeywordType() { - return $this->expectedKeyword; + return $this->keywordType; } /** - * @param string|null $keyword + * @param string|null $type + * @return $this */ - public function setExpectedKeyword($keyword) + public function setKeywordType($type) { - $this->expectedKeyword = $keyword; + $this->keywordType = $type; + return $this; } } diff --git a/src/Rules/RemoveKeyword.php b/src/Rules/RemoveKeyword.php new file mode 100644 index 0000000..d89db34 --- /dev/null +++ b/src/Rules/RemoveKeyword.php @@ -0,0 +1,72 @@ +licence = Licence::get($options['licenceId']); + } + } + + /** + * @param DocumentInterface $document + */ + public function apply($document) + { + $condition = $this->getCondition(); + if ($condition === null || $condition->applies($document)) { + // TODO remove keyword + } + } +} diff --git a/test/ImportRulesTest.php b/test/ImportRulesTest.php index 381b7dd..baba5cf 100644 --- a/test/ImportRulesTest.php +++ b/test/ImportRulesTest.php @@ -38,6 +38,9 @@ use Opus\Import\Rules\AddCollection; use OpusTest\Import\TestAsset\TestCase; +/** + * TODO sword.enableImportRules should not matter for ImportRules class + */ class ImportRulesTest extends TestCase { /** @var int */ @@ -49,7 +52,7 @@ public function testInit() $this->adjustConfiguration([ 'import' => [ - 'rules' => [ + 'rules' => [ 'addCol' => [ 'type' => 'AddCollection', 'collection' => [ @@ -57,6 +60,10 @@ public function testInit() ], ], ], + 'rulesConfigFile' => null, + ], + 'sword' => [ + 'enableImportRules' => true, ], ]); @@ -83,7 +90,7 @@ public function testConfigFullClassname() $this->adjustConfiguration([ 'import' => [ - 'rules' => [ + 'rules' => [ 'addCol' => [ 'type' => AddCollection::class, 'collection' => [ @@ -91,6 +98,10 @@ public function testConfigFullClassname() ], ], ], + 'rulesConfigFile' => null, + ], + 'sword' => [ + 'enableImportRules' => true, ], ]); @@ -111,6 +122,22 @@ public function testConfigFullClassname() $this->assertEquals('col1', $col->getName()); } + public function testLoadConfigIni() + { + $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + 'import' => ['rulesConfigFile' => APPLICATION_PATH . '/test/_files/import-rules.ini'], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $rules = $importRules->getRules(); + + $this->assertCount(2, $rules); + $this->assertInstanceOf(AddCollection::class, $rules[0]); + } + protected function prepareCollections() { $role = CollectionRole::new(); diff --git a/test/ImporterTest.php b/test/ImporterTest.php index dff2644..0b204f4 100644 --- a/test/ImporterTest.php +++ b/test/ImporterTest.php @@ -35,9 +35,14 @@ use Opus\Common\DocumentInterface; use Opus\Common\EnrichmentKey; use Opus\Common\Log; +use Opus\Common\Model\ModelException; +use Opus\Common\Security\SecurityException; +use Opus\Common\Subject; use Opus\Import\Importer; +use Opus\Import\Xml\MetadataImportInvalidXmlException; use Opus\Import\Xml\MetadataImportSkippedDocumentsException; use OpusTest\Import\TestAsset\TestCase; +use Zend_Exception; use function file_get_contents; @@ -131,4 +136,91 @@ public function testFromArray() // var_dump($doc->toArray()); } + + /** + * @return DocumentInterface + * @throws MetadataImportSkippedDocumentsException + * @throws ModelException + * @throws SecurityException + * @throws MetadataImportInvalidXmlException + * @throws Zend_Exception + */ + protected function getTestImportDocument() + { + $xml = file_get_contents(APPLICATION_PATH . '/test/_files/import-rules-test.xml'); + + $importer = new Importer($xml, false, Log::get()); + + $importer->run(); + + $document = $importer->getDocument(); + + $this->assertNotNull($document); + $this->assertInstanceOf(DocumentInterface::class, $document); + + return $document; + } + + public function testImport() + { + $document = $this->getTestImportDocument(); + + $authors = $document->getPersonAuthor(); + + $this->assertCount(1, $authors); + + $this->assertEquals('deu', $document->getLanguage()); + $this->assertEquals('Der Titel', $document->getMainTitle()->getValue()); + } + + public function testImportRulesAddCollectionByAccount() + { + $doc = $this->getTestImportDocument(); + } + + public function testImportRulesAddCollectionForKeyword() + { + // TODO add/enable rules (use global config fallback?) + } + + public function testImportRulesAddLicenceForKeyword() + { + } + + public function testImportRulesRemoveKeyword() + { + } + + public function testImportRulesAddCollectionAndRemoveKeyword() + { + } + + public function testImportRulesDisabled() + { + $doc = $this->getTestImportDocument(); + $this->assertFalse($doc->hasSubject('RulesEnabled')); + } + + public function testImportRulesEnabled() + { + $this->adjustConfiguration([ + 'sword' => [ + 'enableImportRules' => true, + ], + ]); + $doc = $this->getTestImportDocument(); + $this->assertTrue($doc->hasSubject('RulesEnabled')); + } + + public function testKeywordTypeDefaultUncontrolled() + { + $doc = $this->getTestImportDocument(); + + $keywords = $doc->getSubject(); + + $this->assertCount(3, $keywords); + + $this->assertTrue($doc->hasSubject('oa-green', Subject::TYPE_UNCONTROLLED)); + $this->assertTrue($doc->hasSubject('oa-gold', Subject::TYPE_UNCONTROLLED)); + } } diff --git a/test/Rules/AddLicenceTest.php b/test/Rules/AddLicenceTest.php new file mode 100644 index 0000000..ee86d2b --- /dev/null +++ b/test/Rules/AddLicenceTest.php @@ -0,0 +1,182 @@ +prepareLicences(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addLicence' => [ + 'type' => AddLicence::class, + 'licenceId' => $this->licenceId1, + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $doc = Document::new(); + + $importRules->apply($doc); + + $licences = $doc->getLicence(); + + $this->assertCount(1, $licences); + $this->assertEquals($this->licenceId1, $licences[0]->getModel()->getId()); + } + + public function testAddLicenceForKeyword() + { + $this->prepareLicences(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addLicence1' => [ + 'type' => 'AddLicence', + 'licenceId' => $this->licenceId1, + 'condition' => [ + 'keyword' => 'ccby', + ], + ], + 'addLicence2' => [ + 'type' => 'AddLicence', + 'licenceId' => $this->licenceId2, + 'condition' => [ + 'keyword' => 'ccbyna', + ], + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccbyna'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $importRules->apply($doc); + + $licences = $doc->getLicence(); + + $this->assertCount(1, $licences); + $this->assertEquals($this->licenceId2, $licences[0]->getModel()->getId()); + } + + public function testAddMultipleLicences() + { + $this->prepareLicences(); + + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'addLicence1' => [ + 'type' => 'AddLicence', + 'licenceId' => $this->licenceId1, + 'condition' => [ + 'keyword' => 'ccby', + ], + ], + 'addLicence2' => [ + 'type' => 'AddLicence', + 'licenceId' => $this->licenceId2, + 'condition' => [ + 'keyword' => 'ccbyna', + ], + ], + ], + ], + ]); + + $importRules = new ImportRules(); + $importRules->init(); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccbyna'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_PSYNDEX); + + $importRules->apply($doc); + + $licences = $doc->getLicence(); + + $this->assertCount(2, $licences); + $this->assertNotEquals($licences[0]->getModel()->getId(), $licences[1]->getModel()->getId()); + $this->assertContains($licences[0]->getModel()->getId(), [$this->licenceId1, $this->licenceId2]); + $this->assertContains($licences[1]->getModel()->getId(), [$this->licenceId1, $this->licenceId2]); + } + + protected function prepareLicences() + { + $licence = Licence::fromArray([ + 'Name' => 'CC BY', + 'NameLong' => 'Test Licence 1', + 'LinkLicence' => 'https://www.kobv.de/licence1', + ]); + + $this->licenceId1 = $licence->store(); + + $licence = Licence::fromArray([ + 'name' => 'CC BY NA', + 'NameLong' => 'Test Licence 2', + 'LinkLicence' => 'https://www.kobv.de/licence2', + ]); + + $this->licenceId2 = $licence->store(); + } +} diff --git a/test/Rules/Conditions/AccountConditionTest.php b/test/Rules/Conditions/AccountConditionTest.php index 026cafc..32e451e 100644 --- a/test/Rules/Conditions/AccountConditionTest.php +++ b/test/Rules/Conditions/AccountConditionTest.php @@ -31,6 +31,7 @@ namespace OpusTest\Import\Rules\Conditions; +use Opus\Common\Document; use Opus\Import\Rules\Conditions\AccountCondition; use OpusTest\Import\TestAsset\MockAuthAdapter; use OpusTest\Import\TestAsset\TestCase; @@ -76,7 +77,9 @@ public function testAppliesTrue() 'account' => 'sword1', ]); - $this->assertTrue($condition->applies()); + $doc = Document::new(); + + $this->assertTrue($condition->applies($doc)); } public function testAppliesFalse() @@ -87,10 +90,12 @@ public function testAppliesFalse() 'account' => 'sword1', ]); - $this->assertFalse($condition->applies()); + $doc = Document::new(); + + $this->assertFalse($condition->applies($doc)); $condition->setExpectedUser('sword2'); - $this->assertTrue($condition->applies()); + $this->assertTrue($condition->applies($doc)); } } diff --git a/test/Rules/Conditions/KeywordConditionTest.php b/test/Rules/Conditions/KeywordConditionTest.php new file mode 100644 index 0000000..dd5c4a3 --- /dev/null +++ b/test/Rules/Conditions/KeywordConditionTest.php @@ -0,0 +1,257 @@ + [ + 'value' => 'ccby', + 'remove' => true, + 'caseSensitive' => 1, + 'type' => 'uncontrolled', + ], + ]); + + $this->assertEquals('ccby', $condition->getExpectedKeyword()); + $this->assertTrue($condition->isCaseSensitive()); + $this->assertEquals('uncontrolled', $condition->getKeywordType()); + $this->assertTrue($condition->isRemoveEnabled()); + } + + public function testConstructSimpleConfig() + { + $condition = new KeywordCondition([ + 'keyword' => 'ccby', + ]); + + $this->assertEquals('ccby', $condition->getExpectedKeyword()); + $this->assertFalse($condition->isCaseSensitive()); + $this->assertNull($condition->getKeywordType()); + $this->assertFalse($condition->isRemoveEnabled()); + } + + public function testAppliesTrue() + { + $condition = new KeywordCondition([ + 'keyword' => 'ccby', + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertTrue($condition->applies($doc)); + } + + public function testAppliesFalse() + { + $condition = new KeywordCondition([ + 'keyword' => 'ccbyna', + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertFalse($condition->applies($doc)); + } + + public function testUsingKeywordType() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'type' => 'psyndex', + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertFalse($condition->applies($doc)); + + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_PSYNDEX); + + $this->assertTrue($condition->applies($doc)); + } + + public function testNotCaseSensitive() + { + $condition = new KeywordCondition([ + 'keyword' => 'ccby', + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('CCBY'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertTrue($condition->applies($doc)); + } + + public function testCaseSensitive() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'caseSensitive' => true, + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('CCBY'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertFalse($condition->applies($doc)); + } + + public function testDoNotRemoveKeyword() + { + $condition = new KeywordCondition([ + 'keyword' => 'ccby', + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertTrue($condition->applies($doc)); + + $this->assertCount(1, $doc->getSubject()); + } + + public function testDoNotRemoveKeywordIfConditionDoesNotApply() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'remove' => true, + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccbyna'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertFalse($condition->applies($doc)); + + $this->assertCount(1, $doc->getSubject()); + } + + public function testRemoveKeyword() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'remove' => true, + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $this->assertTrue($condition->applies($doc)); + + $this->assertCount(0, $doc->getSubject()); + } + + public function testRemoveOnlyKeywordWithMatchingType() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'type' => Subject::TYPE_PSYNDEX, + 'remove' => true, + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_PSYNDEX); + + $this->assertTrue($condition->applies($doc)); + + $subjects = $doc->getSubject(); + + $this->assertCount(1, $subjects); + $this->assertEquals(Subject::TYPE_UNCONTROLLED, $subjects[0]->getType()); + } + + public function testRemoveAllMatchingKeywords() + { + $condition = new KeywordCondition([ + 'keyword' => [ + 'value' => 'ccby', + 'remove' => true, + ], + ]); + + $doc = Document::new(); + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_UNCONTROLLED); + + $subject = $doc->addSubject(); + $subject->setValue('ccby'); + $subject->setType(Subject::TYPE_PSYNDEX); + + $this->assertTrue($condition->applies($doc)); + + $subjects = $doc->getSubject(); + + $this->assertCount(0, $subjects); + } +} diff --git a/test/Xml/XmlDocumentTest.php b/test/Xml/XmlDocumentTest.php index 32f700c..0cb1bc9 100644 --- a/test/Xml/XmlDocumentTest.php +++ b/test/Xml/XmlDocumentTest.php @@ -48,7 +48,7 @@ public function testValidation() { foreach (new DirectoryIterator(APPLICATION_PATH . '/test/_files') as $fileInfo) { if ( - $fileInfo->getExtension() !== 'xsd' && ! $fileInfo->isDot() + $fileInfo->getExtension() === 'xml' && ! $fileInfo->isDot() && strpos($fileInfo->getBasename(), 'import') === 0 ) { $xml = file_get_contents($fileInfo->getRealPath()); diff --git a/test/_files/import-rules-test.xml b/test/_files/import-rules-test.xml new file mode 100644 index 0000000..0bc373f --- /dev/null +++ b/test/_files/import-rules-test.xml @@ -0,0 +1,28 @@ + + + + + Der Titel + + + The Title + + + Deutsch Zusammenfassung + + + + + + Test + oa-green + oa-gold + + + + + + + + + \ No newline at end of file diff --git a/test/_files/import-rules.ini b/test/_files/import-rules.ini new file mode 100644 index 0000000..d60e6ff --- /dev/null +++ b/test/_files/import-rules.ini @@ -0,0 +1,6 @@ +institute1.type = 'AddCollection' +institute1.condition.account = 'sword' +institute1.collection.id = [COLLECTION ID] + +enabledCheck.type = 'AddKeyword' +enabledCheck.keyword = 'RulesEnabled' \ No newline at end of file diff --git a/test/test.ini b/test/test.ini index 4174084..ac579ca 100644 --- a/test/test.ini +++ b/test/test.ini @@ -99,3 +99,6 @@ filetypes.html.mimeType = 'text/html' ; Allow displaying of PDF directly in browser (default for other types 'attachment') filetypes.pdf.contentDisposition = 'inline' + +sword.enableImportRules = 0 +import.rulesConfigFile = APPLICATION_PATH "/test/_files/import-rules.ini" From c47ad1551d96b4adec9ece7073689c5155a988bb Mon Sep 17 00:00:00 2001 From: j3nsch Date: Mon, 7 Aug 2023 11:39:40 +0200 Subject: [PATCH 09/16] #45 'uncontrolled' default keyword/subject type --- src/Importer.php | 22 ++++++++++++++++++++-- src/Xml/opus-import.xsd | 11 +++-------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/Importer.php b/src/Importer.php index 059259c..d313ca4 100644 --- a/src/Importer.php +++ b/src/Importer.php @@ -143,6 +143,9 @@ class Importer /** @var XmlDocument */ private $xmlDocument; + /** @var ImportRules */ + private $importRules; + /** * @param string $xml * @param bool $isFile @@ -306,7 +309,8 @@ public function run() $doc->addCollection($this->importCollection); } - // TODO add aditional collections + $importRules = $this->getImportRules(); + $importRules->apply($doc); try { $doc->store(); @@ -676,7 +680,8 @@ private function handleKeywords($node, $doc) if ($childNode instanceof DOMElement) { $s = Subject::new(); $s->setLanguage(trim($childNode->getAttribute('language'))); - $s->setType($childNode->getAttribute('type')); + $type = $childNode->getAttribute('type'); + $s->setType($type ?: 'uncontrolled'); $s->setValue(trim($childNode->textContent)); $doc->addSubject($s); } @@ -1090,4 +1095,17 @@ public function getDocument() { return $this->document; } + + /** + * @return ImportRules + */ + public function getImportRules() + { + if ($this->importRules === null) { + $this->importRules = new ImportRules(); + $this->importRules->init(); + } + + return $this->importRules; + } } diff --git a/src/Xml/opus-import.xsd b/src/Xml/opus-import.xsd index be28989..cf315e7 100644 --- a/src/Xml/opus-import.xsd +++ b/src/Xml/opus-import.xsd @@ -25,12 +25,7 @@ * along with OPUS; if not, write to the Free Software Foundation, Inc., 51 * Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * @category Application - * @package Import - * @author Sascha Szott - * @author Jens Schwidder - * @author Doreen Thiede - * @copyright Copyright (c) 2008-2016, OPUS 4 development team + * @copyright Copyright (c) 2008, OPUS 4 development team * @license http://www.gnu.org/licenses/gpl.html General Public License * * Notes: @@ -217,8 +212,8 @@ - - + + From d773d51b0a2af5d61b212c8569a49ecc6d5a42ef Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 24 Aug 2023 18:48:42 +0200 Subject: [PATCH 10/16] #43 Added more testing for import rules --- src/ImportRules.php | 2 +- src/Rules/RemoveKeyword.php | 16 +--- test/ImporterTest.php | 111 +++++++++++++++++++++++--- test/Rules/AddLicenceTest.php | 11 +++ test/_files/import-rules-keywords.ini | 14 ++++ test/_files/import-rules-test.xml | 1 + test/test.ini | 2 +- 7 files changed, 131 insertions(+), 26 deletions(-) create mode 100644 test/_files/import-rules-keywords.ini diff --git a/src/ImportRules.php b/src/ImportRules.php index a2f6eb1..f2d3e46 100644 --- a/src/ImportRules.php +++ b/src/ImportRules.php @@ -130,7 +130,7 @@ public function createRule($type, $options) */ public function apply($document) { - foreach ($this->rules as $rule) { + foreach ($this->getRules() as $rule) { $rule->apply($document); } } diff --git a/src/Rules/RemoveKeyword.php b/src/Rules/RemoveKeyword.php index d89db34..f8f5c2d 100644 --- a/src/Rules/RemoveKeyword.php +++ b/src/Rules/RemoveKeyword.php @@ -35,6 +35,7 @@ /** * TODO logging, error handling + * TODO support list of keywords (or should that be a RemoveKeywords rule?) */ class RemoveKeyword extends AbstractImportRule { @@ -47,18 +48,6 @@ class RemoveKeyword extends AbstractImportRule /** @var bool */ private $caseSensitive = false; - /** - * @param array $options - */ - public function setOptions($options) - { - parent::setOptions($options); - - if (isset($options['licenceId'])) { - $this->licence = Licence::get($options['licenceId']); - } - } - /** * @param DocumentInterface $document */ @@ -66,7 +55,8 @@ public function apply($document) { $condition = $this->getCondition(); if ($condition === null || $condition->applies($document)) { - // TODO remove keyword + // TODO remove keyword (that was matched in condition) + $keyword = 'TODO'; } } } diff --git a/test/ImporterTest.php b/test/ImporterTest.php index 0b204f4..a9c404f 100644 --- a/test/ImporterTest.php +++ b/test/ImporterTest.php @@ -31,9 +31,12 @@ namespace OpusTest\Import; +use Opus\Common\Collection; +use Opus\Common\CollectionRole; use Opus\Common\Document; use Opus\Common\DocumentInterface; use Opus\Common\EnrichmentKey; +use Opus\Common\Licence; use Opus\Common\Log; use Opus\Common\Model\ModelException; use Opus\Common\Security\SecurityException; @@ -62,6 +65,8 @@ public function setUp(): void $enrichmentKey = EnrichmentKey::new(); $enrichmentKey->setName('validtestkey'); $enrichmentKey->store(); + + // TODO enable import rules here? } public function tearDown(): void @@ -180,36 +185,84 @@ public function testImportRulesAddCollectionByAccount() public function testImportRulesAddCollectionForKeyword() { - // TODO add/enable rules (use global config fallback?) + $this->prepareCollections(); + + $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + 'import' => ['rulesConfigFile' => APPLICATION_PATH . '/test/_files/import-rules-keywords.ini'], + ]); + + $doc = $this->getTestImportDocument(); + + $collections = $doc->getCollection(); + + $this->assertCount(1, $collections); + $this->assertEquals('col1', $collections[0]->getName()); } public function testImportRulesAddLicenceForKeyword() { + $licence = Licence::new(); + $licence->setName('CC BY'); + $licence->setNameLong('CC BY Test Licence'); + $licence->setLinkLicence('URL'); + $licenceId = $licence->store(); + + $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + 'import' => [ + 'rulesConfigFile' => null, + 'rules' => [ + 'licence' => [ + 'type' => 'AddLicence', + 'condition' => [ + 'keyword' => 'ccby', + ], + 'licenceId' => $licenceId, + ], + ], + ], + ]); + + $doc = $this->getTestImportDocument(); + + $licences = $doc->getLicence(); + + $this->assertCount(1, $licences); + $this->assertEquals('CC BY', $licences[0]->getName()); } public function testImportRulesRemoveKeyword() { + $this->markTestIncomplete('TODO implement test'); } public function testImportRulesAddCollectionAndRemoveKeyword() { - } + $this->prepareCollections(); + + $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + 'import' => ['rulesConfigFile' => APPLICATION_PATH . '/test/_files/import-rules-keywords.ini'], + ]); - public function testImportRulesDisabled() - { $doc = $this->getTestImportDocument(); - $this->assertFalse($doc->hasSubject('RulesEnabled')); + + $collections = $doc->getCollection(); + + $this->assertCount(1, $collections); + + $this->markTestIncomplete('TODO check if collection was added and keyword removed'); } - public function testImportRulesEnabled() + public function testImportRulesDisabled() { $this->adjustConfiguration([ - 'sword' => [ - 'enableImportRules' => true, - ], + 'sword' => ['enableImportRules' => false], + 'import' => ['rulesConfigFile' => APPLICATION_PATH . '/test/_files/import-rules-keywords.ini'], ]); $doc = $this->getTestImportDocument(); - $this->assertTrue($doc->hasSubject('RulesEnabled')); + $this->assertFalse($doc->hasSubject('RulesEnabled')); } public function testKeywordTypeDefaultUncontrolled() @@ -218,9 +271,45 @@ public function testKeywordTypeDefaultUncontrolled() $keywords = $doc->getSubject(); - $this->assertCount(3, $keywords); + $this->assertCount(4, $keywords); $this->assertTrue($doc->hasSubject('oa-green', Subject::TYPE_UNCONTROLLED)); $this->assertTrue($doc->hasSubject('oa-gold', Subject::TYPE_UNCONTROLLED)); } + + public function testAddKeyword() + { + $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + 'import' => ['rulesConfigFile' => APPLICATION_PATH . '/test/_files/import-rules-keywords.ini'], + ]); + + $doc = $this->getTestImportDocument(); + + $this->assertTrue($doc->hasSubject('RulesEnabled')); + } + + protected function prepareCollections() + { + $role = CollectionRole::new(); + $role->setName('import'); + $role->setOaiName('oaiImport'); + $role->addRootCollection(); + $this->roleId = $role->store(); + + $rootCol = $role->getRootCollection(); + + $col = Collection::new(); + $col->setName('col1'); + $col->setNumber('col1number'); + $rootCol->addFirstChild($col); + + $col = Collection::new(); + $col->setName('green'); + $col->setNumber('col2number'); + $rootCol->addLastChild($col); + $role->store(); + + $this->colId = $col->getId(); + } } diff --git a/test/Rules/AddLicenceTest.php b/test/Rules/AddLicenceTest.php index ee86d2b..a9556f5 100644 --- a/test/Rules/AddLicenceTest.php +++ b/test/Rules/AddLicenceTest.php @@ -46,6 +46,17 @@ class AddLicenceTest extends TestCase /** @var int */ private $licenceId2; + public function setUp(): void + { + parent::setUp(); + + $this->adjustConfiguration([ + 'sword' => [ + 'enableImportRules' => true, + ], + ]); + } + public function testAddLicence() { $this->prepareLicences(); diff --git a/test/_files/import-rules-keywords.ini b/test/_files/import-rules-keywords.ini new file mode 100644 index 0000000..009e519 --- /dev/null +++ b/test/_files/import-rules-keywords.ini @@ -0,0 +1,14 @@ +enabledCheck.type = 'AddKeyword' +enabledCheck.keyword = 'RulesEnabled' + +addCol.type = 'AddCollection' +addCol.condition.keyword = 'oa-gold' +addCol.collection.roleName = 'import' +addCol.collection.name = 'col1' + +; TODO add rules just for specific tests +; addCol2.type = 'AddCollection' +; addCol2.condition.keyword.value = 'oa-green' +; addCol2.condition.keyword.remove = true +; addCol2.collection.roleName = 'import' +; addCol2.collection.name = 'green' diff --git a/test/_files/import-rules-test.xml b/test/_files/import-rules-test.xml index 0bc373f..61f11c1 100644 --- a/test/_files/import-rules-test.xml +++ b/test/_files/import-rules-test.xml @@ -17,6 +17,7 @@ Test oa-green oa-gold + ccby diff --git a/test/test.ini b/test/test.ini index ac579ca..f0fa4c7 100644 --- a/test/test.ini +++ b/test/test.ini @@ -101,4 +101,4 @@ filetypes.html.mimeType = 'text/html' filetypes.pdf.contentDisposition = 'inline' sword.enableImportRules = 0 -import.rulesConfigFile = APPLICATION_PATH "/test/_files/import-rules.ini" +import.rulesConfigFile = From f066bbd59fb91c676bce8ad387d86a2bd421dcfe Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 29 Aug 2023 16:55:36 +0200 Subject: [PATCH 11/16] #43 Fixed tests --- test/Rules/AddCollectionTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Rules/AddCollectionTest.php b/test/Rules/AddCollectionTest.php index e53e6bd..ae7f560 100644 --- a/test/Rules/AddCollectionTest.php +++ b/test/Rules/AddCollectionTest.php @@ -68,8 +68,9 @@ public function testAddCollection() $this->prepareCollections(); $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], 'import' => [ - 'rules' => [ + 'rules' => [ 'addCol' => [ 'type' => AddCollection::class, 'collection' => [ @@ -77,6 +78,7 @@ public function testAddCollection() ], ], ], + 'rulesConfigFile' => null, ], ]); @@ -111,8 +113,9 @@ public function testAddCollectionForAccount() $col2id = $col2->getId(); $this->adjustConfiguration([ + 'sword' => ['enableImportRules' => true], 'import' => [ - 'rules' => [ + 'rules' => [ 'addCol1' => [ 'type' => 'AddCollection', 'condition' => [ @@ -132,6 +135,7 @@ public function testAddCollectionForAccount() ], ], ], + 'rulesConfigFile' => null, ], ]); From 9d21aec727ee095978f267552ee5d02ebbda9ea1 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 29 Aug 2023 18:17:13 +0200 Subject: [PATCH 12/16] #43 RemoveKeywords, some tests --- .../{RemoveKeyword.php => RemoveKeywords.php} | 78 +++++++++- test/Rules/RemoveKeywordsTest.php | 137 ++++++++++++++++++ 2 files changed, 210 insertions(+), 5 deletions(-) rename src/Rules/{RemoveKeyword.php => RemoveKeywords.php} (56%) create mode 100644 test/Rules/RemoveKeywordsTest.php diff --git a/src/Rules/RemoveKeyword.php b/src/Rules/RemoveKeywords.php similarity index 56% rename from src/Rules/RemoveKeyword.php rename to src/Rules/RemoveKeywords.php index f8f5c2d..e96c1d0 100644 --- a/src/Rules/RemoveKeyword.php +++ b/src/Rules/RemoveKeywords.php @@ -33,14 +33,18 @@ use Opus\Common\DocumentInterface; +use function array_map; +use function is_array; +use function mb_split; + /** * TODO logging, error handling * TODO support list of keywords (or should that be a RemoveKeywords rule?) */ -class RemoveKeyword extends AbstractImportRule +class RemoveKeywords extends AbstractImportRule { - /** @var string */ - private $keyword; + /** @var string[] */ + private $keywords; /** @var string */ private $keywordType; @@ -55,8 +59,72 @@ public function apply($document) { $condition = $this->getCondition(); if ($condition === null || $condition->applies($document)) { - // TODO remove keyword (that was matched in condition) - $keyword = 'TODO'; + $keywords = $this->getKeywords(); + if ($keywords !== null) { + $caseSensitive = $this->isCaseSensitive(); + $keywordType = $this->getKeywordType(); + foreach ($this->getKeywords() as $keyword) { + $document->removeSubject($keyword, $keywordType, $caseSensitive); + } + } } } + + /** + * @return string[] + */ + public function getKeywords() + { + return $this->keywords; + } + + /** + * @param string|string[] $keywords + * @return $this + */ + public function setKeywords($keywords) + { + if (is_array($keywords) || $keywords === null) { + $this->keywords = $keywords; + } else { + $this->keywords = array_map('trim', mb_split(',', $keywords)); + } + return $this; + } + + /** + * @return null|string + */ + public function getKeywordType() + { + return $this->keywordType; + } + + /** + * @param null|string $type + * @return $this + */ + public function setKeywordType($type) + { + $this->keywordType = $type; + return $this; + } + + /** + * @return bool + */ + public function isCaseSensitive() + { + return $this->caseSensitive; + } + + /** + * @param bool $caseSensitive + * @return $this + */ + public function setCaseSensitive($caseSensitive) + { + $this->caseSensitive = $caseSensitive; + return $this; + } } diff --git a/test/Rules/RemoveKeywordsTest.php b/test/Rules/RemoveKeywordsTest.php new file mode 100644 index 0000000..8d5befd --- /dev/null +++ b/test/Rules/RemoveKeywordsTest.php @@ -0,0 +1,137 @@ +adjustConfiguration([ + 'sword' => ['enableImportRules' => true], + ]); + } + + public function testRemoveKeywordUsingCondition() + { + $this->adjustConfiguration([ + 'import' => [ + 'rules' => [ + 'keyword1' => [ + 'type' => 'RemoveKeywords', + 'condition' => [ + 'keyword' => [ + 'value' => 'RemoveMe', + 'remove' => true, + ], + ], + ], + ], + 'rulesConfigFile' => null, + ], + ]); + + $doc = Document::new(); + $keyword = $doc->addSubject(); + $keyword->setValue('RemoveMe'); + $keyword->setType('uncontrolled'); + $keyword->setLanguage('eng'); + + $rules = new ImportRules(); + $rules->init(); + + $rules->apply($doc); + + $keywords = $doc->getSubject(); + + $this->assertCount(0, $keywords); + } + + public function testSetKeywordsSingleValue() + { + $rule = new RemoveKeywords(); + } + + public function testSetKeywordsNull() + { + $rule = new RemoveKeywords(); + } + + public function testSetKeywordsCsv() + { + $rule = new RemoveKeywords(); + } + + public function testSetKeywordsArray() + { + $rule = new RemoveKeywords(); + } + + public function testRemoveKeyword() + { + $doc = Document::new(); + $keyword = $doc->addSubject(); + $keyword->setValue('RemoveMe'); + $keyword->setType('uncontrolled'); + $keyword->setLanguage('eng'); + + $rule = new RemoveKeywords(); + $rule->setKeywords('RemoveMe'); + + $rule->apply($doc); + + $keywords = $doc->getSubject(); + + $this->assertCount(0, $keywords); + } + + public function testRemoveMultipleKeywords() + { + $rule = new RemoveKeywords(); + } + + public function testRemoveKeywordsUsingType() + { + $rule = new RemoveKeywords(); + } + + public function testRemoveKeywordsCaseSensitive() + { + $rule = new RemoveKeywords(); + } +} From bbea3787c906b2f976d037e317a5bef9bc7d02cf Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 7 Sep 2023 17:47:55 +0200 Subject: [PATCH 13/16] #43 AddKeyword support type and lang --- src/Rules/AddKeyword.php | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/Rules/AddKeyword.php b/src/Rules/AddKeyword.php index 7a0cb05..0515bd2 100644 --- a/src/Rules/AddKeyword.php +++ b/src/Rules/AddKeyword.php @@ -35,14 +35,23 @@ use Opus\Common\Subject; use Opus\Common\SubjectInterface; +use function is_array; + /** * TODO logging, error handling + * TODO allow configuring type and language */ class AddKeyword extends AbstractImportRule { /** @var SubjectInterface */ private $subject; + /** @var string */ + private $subjectType = Subject::TYPE_UNCONTROLLED; + + /** @var string */ + private $language = 'deu'; + /** * @param array $options */ @@ -51,10 +60,23 @@ public function setOptions($options) parent::setOptions($options); if (isset($options['keyword'])) { - $subject = Subject::new(); - $subject->setValue($options['keyword']); - $subject->setType(Subject::TYPE_UNCONTROLLED); - $this->subject = $subject; + $config = $options['keyword']; + + if (is_array($config)) { + $keyword = $config['value'] ?? null; + $this->subjectType = $config['type'] ?? Subject::TYPE_UNCONTROLLED; + $this->language = $config['lang'] ?? 'deu'; + } else { + $keyword = $config; + } + + if (! empty($keyword)) { + $subject = Subject::new(); + $subject->setValue($keyword); + $subject->setType($this->subjectType); + $subject->setLanguage($this->language); + $this->subject = $subject; + } } } From b3323b9f9eec06e8b624b4051a4f1f1aa6417aeb Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 26 Mar 2026 16:29:15 +0100 Subject: [PATCH 14/16] PHP82 Fix dynamic variables --- test/ImporterTest.php | 4 +--- test/Rules/AddCollectionTest.php | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/ImporterTest.php b/test/ImporterTest.php index a9c404f..f86029d 100644 --- a/test/ImporterTest.php +++ b/test/ImporterTest.php @@ -295,7 +295,7 @@ protected function prepareCollections() $role->setName('import'); $role->setOaiName('oaiImport'); $role->addRootCollection(); - $this->roleId = $role->store(); + $role->store(); $rootCol = $role->getRootCollection(); @@ -309,7 +309,5 @@ protected function prepareCollections() $col->setNumber('col2number'); $rootCol->addLastChild($col); $role->store(); - - $this->colId = $col->getId(); } } diff --git a/test/Rules/AddCollectionTest.php b/test/Rules/AddCollectionTest.php index ae7f560..210b361 100644 --- a/test/Rules/AddCollectionTest.php +++ b/test/Rules/AddCollectionTest.php @@ -47,6 +47,9 @@ class AddCollectionTest extends TestCase /** @var Zend_Auth_Storage_Interface */ private $authStorage; + /** @var int */ + private $colId; + public function setUp(): void { parent::setUp(); From e82a5d40b3588467b964bae69da0ed15884fee01 Mon Sep 17 00:00:00 2001 From: j3nsch Date: Thu, 26 Mar 2026 17:04:01 +0100 Subject: [PATCH 15/16] #45 Keyword language optional; tests; psyndex support --- src/Importer.php | 5 +++-- src/Xml/opus-import.xsd | 3 ++- test/ImporterTest.php | 40 ++++++++++++++++++++++++++++++++++++++++ test/_files/import2.xml | 5 +++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/Importer.php b/src/Importer.php index 4f807ff..12d1c69 100644 --- a/src/Importer.php +++ b/src/Importer.php @@ -719,8 +719,9 @@ protected function handleKeywords($node, $doc) { foreach ($node->childNodes as $childNode) { if ($childNode instanceof DOMElement) { - $s = Subject::new(); - $s->setLanguage(trim($childNode->getAttribute('language'))); + $s = Subject::new(); + $language = $childNode->getAttribute('language'); + $s->setLanguage($language ?: 'deu'); $type = $childNode->getAttribute('type'); $s->setType($type ?: 'uncontrolled'); $s->setValue(trim($childNode->textContent)); diff --git a/src/Xml/opus-import.xsd b/src/Xml/opus-import.xsd index cf315e7..dd71b1a 100644 --- a/src/Xml/opus-import.xsd +++ b/src/Xml/opus-import.xsd @@ -213,7 +213,7 @@ - + @@ -224,6 +224,7 @@ + diff --git a/test/ImporterTest.php b/test/ImporterTest.php index f86029d..b9eed48 100644 --- a/test/ImporterTest.php +++ b/test/ImporterTest.php @@ -310,4 +310,44 @@ protected function prepareCollections() $rootCol->addLastChild($col); $role->store(); } + + public function testImportKeywordDefaults() + { + $xml = file_get_contents(APPLICATION_PATH . '/test/_files/import2.xml'); + + $importer = new Importer($xml, false, Log::get()); + + $importer->run(); + + $doc = $importer->getDocument(); + + $this->assertNotNull($doc); + $this->assertInstanceOf(DocumentInterface::class, $doc); + + $subjects = $doc->getSubject(); + + $this->assertCount(3, $subjects); + + // TODO this result check is overly complicated - better way? + foreach ($subjects as $subject) { + switch ($subject->getType()) { + case Subject::TYPE_SWD: + $this->assertEquals('Test', $subject->getValue()); + $this->assertEquals('deu', $subject->getLanguage()); + break; + case Subject::TYPE_UNCONTROLLED: + switch ($subject->getValue()) { + case 'engTest': + $this->assertEquals('eng', $subject->getLanguage()); + break; + case 'testWithDefaults': + $this->assertEquals('deu', $subject->getLanguage()); + break; + } + break; + default: + $this->fail('Unexpected subject type ' . $subject->getType()); + } + } + } } diff --git a/test/_files/import2.xml b/test/_files/import2.xml index a155a68..ffd7320 100644 --- a/test/_files/import2.xml +++ b/test/_files/import2.xml @@ -4,5 +4,10 @@ Der Titel + + Test + engTest + testWithDefaults + \ No newline at end of file From c88f257f446e4d5c43af1fc7de90f63437b181bd Mon Sep 17 00:00:00 2001 From: j3nsch Date: Tue, 31 Mar 2026 14:03:18 +0200 Subject: [PATCH 16/16] #43 Import rules for removing keywords. --- composer.json | 1 + src/ArrayImport.php | 7 +++ src/Rules/RemoveKeywords.php | 2 +- test/Rules/RemoveKeywordsTest.php | 80 +++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0fcd4bc..f3bd7b1 100644 --- a/composer.json +++ b/composer.json @@ -13,6 +13,7 @@ "ext-dom": "*", "ext-fileinfo": "*", "ext-libxml": "*", + "ext-mbstring": "*", "ext-zip": "*", "opus4-repo/opus4-common": "dev-master as 4.8.1", "opus4-repo/opus4-app-common": "dev-main", diff --git a/src/ArrayImport.php b/src/ArrayImport.php index 2b8c689..d060e41 100644 --- a/src/ArrayImport.php +++ b/src/ArrayImport.php @@ -33,6 +33,13 @@ use Opus\Common\Document; +/** + * Imports documents from array. + * + * TODO What is the use case, besides an easy way to test import mechanisms. + * TODO Interface? + * TODO support multiple documents? + */ class ArrayImport { /** diff --git a/src/Rules/RemoveKeywords.php b/src/Rules/RemoveKeywords.php index e96c1d0..b8cfabc 100644 --- a/src/Rules/RemoveKeywords.php +++ b/src/Rules/RemoveKeywords.php @@ -63,7 +63,7 @@ public function apply($document) if ($keywords !== null) { $caseSensitive = $this->isCaseSensitive(); $keywordType = $this->getKeywordType(); - foreach ($this->getKeywords() as $keyword) { + foreach ($keywords as $keyword) { $document->removeSubject($keyword, $keywordType, $caseSensitive); } } diff --git a/test/Rules/RemoveKeywordsTest.php b/test/Rules/RemoveKeywordsTest.php index 8d5befd..f5b192a 100644 --- a/test/Rules/RemoveKeywordsTest.php +++ b/test/Rules/RemoveKeywordsTest.php @@ -85,21 +85,29 @@ public function testRemoveKeywordUsingCondition() public function testSetKeywordsSingleValue() { $rule = new RemoveKeywords(); + $rule->setKeywords('RemoveMe'); + $this->assertEquals(['RemoveMe'], $rule->getKeywords()); } public function testSetKeywordsNull() { $rule = new RemoveKeywords(); + $rule->setKeywords(null); + $this->assertNull($rule->getKeywords()); } public function testSetKeywordsCsv() { $rule = new RemoveKeywords(); + $rule->setKeywords('keyword1,keyword2,keyword3'); + $this->assertEquals(['keyword1', 'keyword2', 'keyword3'], $rule->getKeywords()); } public function testSetKeywordsArray() { $rule = new RemoveKeywords(); + $rule->setKeywords(['keyword1', 'keyword2', 'keyword3']); + $this->assertEquals(['keyword1', 'keyword2', 'keyword3'], $rule->getKeywords()); } public function testRemoveKeyword() @@ -122,16 +130,88 @@ public function testRemoveKeyword() public function testRemoveMultipleKeywords() { + $doc = Document::fromArray([ + 'Subject' => [ + [ + 'Language' => 'eng', + 'Value' => 'key1', + 'Type' => 'uncontrolled', + ], + [ + 'Language' => 'eng', + 'Value' => 'key2', + 'Type' => 'psyndex', + ], + ], + ]); + + $this->assertCount(2, $doc->getSubject()); + $rule = new RemoveKeywords(); + $rule->setCaseSensitive(false); + $rule->setKeywords(['key1', 'KEY2']); + + $rule->apply($doc); + + $this->assertCount(0, $doc->getSubject()); } public function testRemoveKeywordsUsingType() { + $doc = Document::fromArray([ + 'Subject' => [ + [ + 'Language' => 'eng', + 'Value' => 'key1', + 'Type' => 'uncontrolled', + ], + [ + 'Language' => 'eng', + 'Value' => 'key2', + 'Type' => 'psyndex', + ], + ], + ]); + + $this->assertCount(2, $doc->getSubject()); + $rule = new RemoveKeywords(); + $rule->setCaseSensitive(false); + $rule->setKeywords(['key1', 'KEY2']); + $rule->setKeywordType('psyndex'); + + $rule->apply($doc); + + $this->assertCount(1, $doc->getSubject()); + $this->assertEquals('key1', $doc->getSubject(0)->getValue()); } public function testRemoveKeywordsCaseSensitive() { + $doc = Document::fromArray([ + 'Subject' => [ + [ + 'Language' => 'eng', + 'Value' => 'key1', + 'Type' => 'uncontrolled', + ], + [ + 'Language' => 'eng', + 'Value' => 'key2', + 'Type' => 'psyndex', + ], + ], + ]); + + $this->assertCount(2, $doc->getSubject()); + $rule = new RemoveKeywords(); + $rule->setCaseSensitive(true); + $rule->setKeywords(['key1', 'KEY2']); + + $rule->apply($doc); + + $this->assertCount(1, $doc->getSubject()); + $this->assertEquals('key2', $doc->getSubject(0)->getValue()); } }