From 50c38cc3f4814f7dacdbb3605b9c3bcad2cff7aa Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 19 Aug 2025 16:28:14 +0300 Subject: [PATCH 1/2] Add isUniqueChangelog rule. Fix OrcidSource token index issue. --- app/plugins/OrcidSource/config/plugin.json | 3 +- .../resources/locales/en_US/orcid_source.po | 3 + .../src/Model/Table/OrcidTokensTable.php | 27 ++++ app/src/Lib/Rules/IsUniqueChangelog.php | 124 ++++++++++++++++++ 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 app/src/Lib/Rules/IsUniqueChangelog.php diff --git a/app/plugins/OrcidSource/config/plugin.json b/app/plugins/OrcidSource/config/plugin.json index f7e625558..e3113e8b1 100644 --- a/app/plugins/OrcidSource/config/plugin.json +++ b/app/plugins/OrcidSource/config/plugin.json @@ -49,8 +49,7 @@ }, "indexes": { "orcid_source_tokens_i1": { - "columns": [ "orcid_source_id", "orcid_identifier"], - "unique": true + "columns": [ "orcid_source_id", "orcid_identifier"] }, "orcid_source_tokens_i2": { "columns": [ "orcid_identifier" ] }, "orcid_source_tokens_i3": { "needed": false, "columns": [ "orcid_source_id" ] } diff --git a/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po b/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po index a670369b1..500306bcf 100644 --- a/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po +++ b/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po @@ -58,6 +58,9 @@ msgstr "{0} was not found" msgid "error.response.no_orcid" msgstr "ORCID identifier missing from response." +msgid "error.exists" +msgstr "{0} already exists with this Identitifier" + msgid "field.OrcidSources.api_type" msgstr "API Type" diff --git a/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php b/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php index 9a2519a1c..6d6242a59 100644 --- a/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php +++ b/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php @@ -29,6 +29,8 @@ namespace OrcidSource\Model\Table; +use App\Lib\Rules\IsUniqueChangelog; +use Cake\ORM\RulesChecker; use Cake\ORM\Table; use Cake\Utility\Security; use Cake\Validation\Validator; @@ -119,6 +121,31 @@ public function beforeMarshal(EventInterface $event, \ArrayObject $data, \ArrayO } + /** + * Define business rules. + * + * @since COmanage Registry v5.2.0 + * @param RulesChecker $rules RulesChecker object + * @return RulesChecker + */ + + public function buildRules(RulesChecker $rules): RulesChecker { + // We don't want to perform the uniqueness check until after then namespacing + // check in order to avoid information leakage. This requires more complicated + // rule building. + + $rules->add( + new IsUniqueChangelog(["orcid_source_id", "orcid_identifier"]), + '_isUniqueChangelog', + [ + 'errorField' => 'orcid_identifier', + 'message' => __d('orcid_source', 'error.exists', [__d('controller', 'OrcidTokens', [1])]), + ] + ); + + return $rules; + } + /** * Unencrypt a value previously encrypted using salt * diff --git a/app/src/Lib/Rules/IsUniqueChangelog.php b/app/src/Lib/Rules/IsUniqueChangelog.php new file mode 100644 index 000000000..f9e56ebf1 --- /dev/null +++ b/app/src/Lib/Rules/IsUniqueChangelog.php @@ -0,0 +1,124 @@ + + */ + protected $_fields; + + /** + * The unique check options + * + * @var array + */ + protected $_options = [ + 'allowMultipleNulls' => false, + ]; + + /** + * Constructor. + * + * ### Options + * + * - `allowMultipleNulls` Allows any field to have multiple null values. Defaults to false. + * + * @param array $fields The list of fields to check uniqueness for + * @param array $options The options for unique checks. + */ + public function __construct(array $fields, array $options = []) + { + $this->_fields = $fields; + $this->_options = $options + $this->_options; + } + + /** + * Performs the uniqueness check + * + * @param \Cake\Datasource\EntityInterface $entity The entity from where to extract the fields + * where the `repository` key is required. + * @param array $options Options passed to the check, + * @return bool + */ + public function __invoke(EntityInterface $entity, array $options): bool + { + if (!$entity->extract($this->_fields, true)) { + return true; + } + + $fields = $entity->extract($this->_fields); + if ($this->_options['allowMultipleNulls'] && array_filter($fields, 'is_null')) { + return true; + } + + $alias = $options['repository']->getAlias(); + $conditions = $this->_alias($alias, $fields); + // Changelog + $ascParentfk = Inflector::singularize($options['repository']->getTable()) . '_id'; + $conditions["$alias.deleted !="] = true; + $conditions["$alias.$ascParentfk IS"] = null; + if ($entity->isNew() === false) { + $keys = (array)$options['repository']->getPrimaryKey(); + $keys = $this->_alias($alias, $entity->extract($keys)); + if (Hash::filter($keys)) { + $conditions['NOT'] = $keys; + } + } + + return !$options['repository']->exists($conditions); + } + + /** + * Add a model alias to all the keys in a set of conditions. + * + * @param string $alias The alias to add. + * @param array $conditions The conditions to alias. + * @return array + */ + protected function _alias(string $alias, array $conditions): array + { + $aliased = []; + foreach ($conditions as $key => $value) { + $aliased["$alias.$key IS"] = $value; + } + + return $aliased; + } +} From 561839161e937ed3cf4fcdd62d58d80b3d4e6cf0 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 25 Aug 2025 10:40:59 +0300 Subject: [PATCH 2/2] Fix review changes --- .../resources/locales/en_US/orcid_source.po | 2 +- .../src/Model/Table/OrcidTokensTable.php | 14 +- app/src/Lib/Rules/IsUniqueChangelog.php | 124 ------------------ 3 files changed, 4 insertions(+), 136 deletions(-) delete mode 100644 app/src/Lib/Rules/IsUniqueChangelog.php diff --git a/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po b/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po index 500306bcf..b8cf16f62 100644 --- a/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po +++ b/app/plugins/OrcidSource/resources/locales/en_US/orcid_source.po @@ -59,7 +59,7 @@ msgid "error.response.no_orcid" msgstr "ORCID identifier missing from response." msgid "error.exists" -msgstr "{0} already exists with this Identitifier" +msgstr "Orcid Token already exists with this Identifier" msgid "field.OrcidSources.api_type" msgstr "API Type" diff --git a/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php b/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php index 6d6242a59..6db305dc1 100644 --- a/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php +++ b/app/plugins/OrcidSource/src/Model/Table/OrcidTokensTable.php @@ -29,7 +29,6 @@ namespace OrcidSource\Model\Table; -use App\Lib\Rules\IsUniqueChangelog; use Cake\ORM\RulesChecker; use Cake\ORM\Table; use Cake\Utility\Security; @@ -130,17 +129,10 @@ public function beforeMarshal(EventInterface $event, \ArrayObject $data, \ArrayO */ public function buildRules(RulesChecker $rules): RulesChecker { - // We don't want to perform the uniqueness check until after then namespacing - // check in order to avoid information leakage. This requires more complicated - // rule building. - $rules->add( - new IsUniqueChangelog(["orcid_source_id", "orcid_identifier"]), - '_isUniqueChangelog', - [ - 'errorField' => 'orcid_identifier', - 'message' => __d('orcid_source', 'error.exists', [__d('controller', 'OrcidTokens', [1])]), - ] + $rules->isUnique( + ["orcid_source_id", "orcid_identifier"]), + __d('orcid_source', 'error.exists') ); return $rules; diff --git a/app/src/Lib/Rules/IsUniqueChangelog.php b/app/src/Lib/Rules/IsUniqueChangelog.php deleted file mode 100644 index f9e56ebf1..000000000 --- a/app/src/Lib/Rules/IsUniqueChangelog.php +++ /dev/null @@ -1,124 +0,0 @@ - - */ - protected $_fields; - - /** - * The unique check options - * - * @var array - */ - protected $_options = [ - 'allowMultipleNulls' => false, - ]; - - /** - * Constructor. - * - * ### Options - * - * - `allowMultipleNulls` Allows any field to have multiple null values. Defaults to false. - * - * @param array $fields The list of fields to check uniqueness for - * @param array $options The options for unique checks. - */ - public function __construct(array $fields, array $options = []) - { - $this->_fields = $fields; - $this->_options = $options + $this->_options; - } - - /** - * Performs the uniqueness check - * - * @param \Cake\Datasource\EntityInterface $entity The entity from where to extract the fields - * where the `repository` key is required. - * @param array $options Options passed to the check, - * @return bool - */ - public function __invoke(EntityInterface $entity, array $options): bool - { - if (!$entity->extract($this->_fields, true)) { - return true; - } - - $fields = $entity->extract($this->_fields); - if ($this->_options['allowMultipleNulls'] && array_filter($fields, 'is_null')) { - return true; - } - - $alias = $options['repository']->getAlias(); - $conditions = $this->_alias($alias, $fields); - // Changelog - $ascParentfk = Inflector::singularize($options['repository']->getTable()) . '_id'; - $conditions["$alias.deleted !="] = true; - $conditions["$alias.$ascParentfk IS"] = null; - if ($entity->isNew() === false) { - $keys = (array)$options['repository']->getPrimaryKey(); - $keys = $this->_alias($alias, $entity->extract($keys)); - if (Hash::filter($keys)) { - $conditions['NOT'] = $keys; - } - } - - return !$options['repository']->exists($conditions); - } - - /** - * Add a model alias to all the keys in a set of conditions. - * - * @param string $alias The alias to add. - * @param array $conditions The conditions to alias. - * @return array - */ - protected function _alias(string $alias, array $conditions): array - { - $aliased = []; - foreach ($conditions as $key => $value) { - $aliased["$alias.$key IS"] = $value; - } - - return $aliased; - } -}