From c4d76d9c571be40b0fbf065b33e3a8c5ee074dbb Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Tue, 13 Oct 2020 11:17:05 +0300 Subject: [PATCH 1/9] Provide a failing test for the ::notExists. --- tests/src/Kernel/SparqlEntityQueryTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/src/Kernel/SparqlEntityQueryTest.php b/tests/src/Kernel/SparqlEntityQueryTest.php index 1f3801d..dc194cb 100644 --- a/tests/src/Kernel/SparqlEntityQueryTest.php +++ b/tests/src/Kernel/SparqlEntityQueryTest.php @@ -448,6 +448,26 @@ public function idStringComparisonDataProvider() { ]; } + /** + * Tests the NOT EXISTS operator. + */ + public function testNotExists() { + $entity = SparqlTest::create([ + 'id' => 'http://fruit.example.com/not_exists', + 'title' => 'fruit title not exists', + 'type' => 'fruit', + ]); + $entity->save(); + $this->entities[] = $entity; + + $results = $this->getQuery() + ->condition('type', 'fruit') + ->notExists('text') + ->execute(); + + $this->assertNotEmpty($results); + } + /** * Asserts that arrays are identical. */ From b4d265c032f637bb51ad9db919b4211ddec42d10 Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Sun, 28 Mar 2021 20:05:04 +0300 Subject: [PATCH 2/9] ISAICP-6242: Fix the not exists condition. --- src/Entity/Query/Sparql/SparqlCondition.php | 32 +++++++++++++++------ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index 75ea027..7e47a8c 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -271,10 +271,6 @@ public function condition($field = NULL, $value = NULL, $operator = NULL, $lang 'lang' => $lang, 'column' => $column, ]; - - if (!in_array($operator, ['EXISTS', 'NOT EXISTS'])) { - $this->requiresDefaultPattern = FALSE; - } } return $this; @@ -396,7 +392,7 @@ public function addFieldMappingRequirement(string $entity_type_id, string $field // loaded by the database. There is no way that in a single request, // the same predicate is found with a single and multiple mappings. // There is no filter per bundle in the query. - $this->fieldMappingConditions[] = [ + $this->fieldMappingConditions[$field_name] = [ 'field' => $field, 'column' => $column, 'value' => array_values(array_unique($mappings)), @@ -504,7 +500,7 @@ protected function conditionsToString(): void { case 'EXISTS': case 'NOT EXISTS': - $this->addConditionFragment($this->compileExists($condition)); + $this->compileExists($condition); break; case 'CONTAINS': @@ -597,10 +593,30 @@ protected function compileBundleCondition(array $condition): void { * @return string * A condition fragment string. */ - protected function compileExists(array $condition): string { + protected function compileExists(array $condition): void { $prefix = self::$filterOperatorMap[$condition['operator']]['prefix']; $suffix = self::$filterOperatorMap[$condition['operator']]['suffix']; - return $prefix . self::ID_KEY . ' ' . $this->escapePredicate($this->fieldMappings[$condition['field']]) . ' ' . SparqlArg::toVar($condition['field']) . $suffix; + + $field_predicate = $this->fieldMappings[$condition['field']]; + $condition_string = self::ID_KEY . ' ' . $this->escapePredicate($field_predicate) . ' ' . SparqlArg::toVar($condition['field']); + + if (isset($this->fieldMappingConditions[$condition['field']])) { + $mapping_condition = $this->fieldMappingConditions[$condition['field']]; + $mapping_condition['value'] = SparqlArg::toResourceUris($mapping_condition['value']); + $mapping_condition['field'] = $field_predicate; + $condition_string .= ' . ' . $this->compileValuesFilter($mapping_condition); + } + + $key = array_search($condition_string, $this->conditionFragments); + // Only add a condition if the mapping condition is not found. If found, + // replace it, in order to avoid creating an EXISTS and NOT EXISTS on the + // same property. + if ($key !== FALSE) { + $this->conditionFragments[$key] = $prefix . $this->conditionFragments[$key] . $suffix; + } + else { + $this->addConditionFragment($prefix . $condition_string . $suffix); + } } /** From 242420a265fae657889befe22c394fab2940c579 Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Sun, 28 Mar 2021 20:38:32 +0300 Subject: [PATCH 3/9] ISAICP-6242: Fix codesniffs. --- src/Entity/Query/Sparql/SparqlCondition.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index 7e47a8c..f2fcef1 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -589,9 +589,6 @@ protected function compileBundleCondition(array $condition): void { * * @param array $condition * An array that contains the 'field', 'value', 'operator' values. - * - * @return string - * A condition fragment string. */ protected function compileExists(array $condition): void { $prefix = self::$filterOperatorMap[$condition['operator']]['prefix']; @@ -612,7 +609,7 @@ protected function compileExists(array $condition): void { // replace it, in order to avoid creating an EXISTS and NOT EXISTS on the // same property. if ($key !== FALSE) { - $this->conditionFragments[$key] = $prefix . $this->conditionFragments[$key] . $suffix; + $this->conditionFragments[$key] = $prefix . $this->conditionFragments[$key] . $suffix; } else { $this->addConditionFragment($prefix . $condition_string . $suffix); From 6d1d53f8a57967231b629d339b62ddd994b3dbfb Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Mon, 5 Apr 2021 21:48:14 +0300 Subject: [PATCH 4/9] ISAICP-6242: Avoid using filter condition for a single mapping. --- src/Entity/Query/Sparql/SparqlCondition.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index f2fcef1..7ca7191 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -380,6 +380,7 @@ public function addFieldMappingRequirement(string $entity_type_id, string $field } $mappings = $this->fieldHandler->getFieldPredicates($entity_type_id, $field, $column); + $mappings = array_values(array_unique($mappings)); $field_name = $field . '__' . $column; if (count($mappings) === 1) { $this->fieldMappings[$field_name] = reset($mappings); @@ -395,7 +396,7 @@ public function addFieldMappingRequirement(string $entity_type_id, string $field $this->fieldMappingConditions[$field_name] = [ 'field' => $field, 'column' => $column, - 'value' => array_values(array_unique($mappings)), + 'value' => $mappings, 'operator' => 'IN', ]; } From b6a9b95b41bea132b7209e6918ea99b20256ff1a Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Tue, 6 Apr 2021 09:05:14 +0300 Subject: [PATCH 5/9] ISAICP-6242: Remove the need to explicitly install the branch to test. --- tests/travis-ci/fixtures/composer.json.dist | 1 - tests/travis-ci/scripts/run_tests.sh | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/travis-ci/fixtures/composer.json.dist b/tests/travis-ci/fixtures/composer.json.dist index be8721f..0890be0 100644 --- a/tests/travis-ci/fixtures/composer.json.dist +++ b/tests/travis-ci/fixtures/composer.json.dist @@ -5,7 +5,6 @@ "drupal/core": "${DRUPAL}", "drupal/core-composer-scaffold": "*", "drupal/pathauto": "*", - "drupal/sparql_entity_storage": "*", "drupal/token": "*", "drush/drush": "~10", "easyrdf/easyrdf": "1.0.0 as 0.9.2", diff --git a/tests/travis-ci/scripts/run_tests.sh b/tests/travis-ci/scripts/run_tests.sh index 67612a0..9cd7236 100755 --- a/tests/travis-ci/scripts/run_tests.sh +++ b/tests/travis-ci/scripts/run_tests.sh @@ -6,6 +6,7 @@ install_codebase () { cd ${SITE_DIR} perl -i -pe's/\$\{([^}]+)\}/$ENV{$1}/' composer.json COMPOSER_MEMORY_LIMIT=-1 composer install --no-interaction --prefer-dist + ln -s ${TRAVIS_BUILD_DIR} ${TRAVIS_BUILD_DIR}/web/modules/sparql_entity_storage } case "${TEST}" in From 1bf03e05cc7464422914fef5f2e7f3fba03911a9 Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Tue, 6 Apr 2021 22:45:23 +0300 Subject: [PATCH 6/9] ISAICP-6242: Turn exists into a simple list of triples as it works that way in SPARQL. --- src/Entity/Query/Sparql/Query.php | 10 ++++++++++ src/Entity/Query/Sparql/SparqlCondition.php | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Entity/Query/Sparql/Query.php b/src/Entity/Query/Sparql/Query.php index 6b1bdd1..576d178 100644 --- a/src/Entity/Query/Sparql/Query.php +++ b/src/Entity/Query/Sparql/Query.php @@ -355,6 +355,16 @@ public function sort($field, $direction = 'ASC', $langcode = NULL) { if (!in_array($direction, ['ASC', 'DESC'])) { throw new \RuntimeException('Only "ASC" and "DESC" are allowed as sort order.'); } + + // Unlike the normal SQL queries where a column not defined can be used for + // sorting if exists in the table, in SPARQL, the sort argument must be + // defined in the WHERE clause. Any sort property, therefore, must will be + // included with an EXISTS condition. + // Also, the $idKey and $bundleKey properties cannot be added as triples as + // they cannot be the object of the triple. + if (!in_array($field, [$this->idKey, $this->bundleKey])) { + $this->exists($field); + } return parent::sort($field, $direction, $langcode); } diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index 7ca7191..7632c04 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -70,7 +70,7 @@ class SparqlCondition extends ConditionFundamentals implements SparqlConditionIn 'ENDS WITH' => ['prefix' => 'FILTER(STRENDS(', 'suffix' => '))'], 'LIKE' => ['prefix' => 'FILTER(CONTAINS(', 'suffix' => '))'], 'NOT LIKE' => ['prefix' => 'FILTER(!CONTAINS(', 'suffix' => '))'], - 'EXISTS' => ['prefix' => 'FILTER EXISTS {', 'suffix' => '}'], + 'EXISTS' => ['prefix' => '', 'suffix' => ''], 'NOT EXISTS' => ['prefix' => 'FILTER NOT EXISTS {', 'suffix' => '}'], '<' => ['prefix' => '', 'suffix' => ''], '>' => ['prefix' => '', 'suffix' => ''], From f4ba8e632a137294367d42c7ef2d016d7beb84ac Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Wed, 7 Apr 2021 10:39:51 +0300 Subject: [PATCH 7/9] ISAICP-6242: Add condition fragments independently to allow remove duplicate entries. --- src/Entity/Query/Sparql/SparqlCondition.php | 63 ++++++++++++++++----- 1 file changed, 49 insertions(+), 14 deletions(-) diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index 7632c04..7928cb7 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -442,10 +442,11 @@ protected function fieldMappingConditionsToString(): void { $field_name = $condition['field'] . '__' . $condition['column']; $field_predicate = $this->fieldMappings[$field_name]; $condition_string = self::ID_KEY . ' ' . $this->escapePredicate($field_predicate) . ' ' . SparqlArg::toVar($field_name); + $this->addConditionFragment($condition_string); $condition['value'] = SparqlArg::toResourceUris($condition['value']); $condition['field'] = $field_predicate; - $condition_string .= ' . ' . $this->compileValuesFilter($condition); + $condition_string = $this->compileValuesFilter($condition); $this->addConditionFragment($condition_string); } } @@ -500,10 +501,13 @@ protected function conditionsToString(): void { break; case 'EXISTS': - case 'NOT EXISTS': $this->compileExists($condition); break; + case 'NOT EXISTS': + $this->compileNotExists($condition); + break; + case 'CONTAINS': case 'LIKE': case 'NOT LIKE': @@ -586,35 +590,66 @@ protected function compileBundleCondition(array $condition): void { } /** - * Compiles a filter exists (or not exists) condition. + * Compiles a filter exists condition. + * + * Since a triple in SPARQL works just like EXISTS does, for EXISTS we add + * any condition missing from the field mapping fragments. * * @param array $condition * An array that contains the 'field', 'value', 'operator' values. */ protected function compileExists(array $condition): void { + $field_predicate = $this->fieldMappings[$condition['field']]; + $condition_strings = []; + $condition_strings[] = self::ID_KEY . ' ' . $this->escapePredicate($field_predicate) . ' ' . SparqlArg::toVar($condition['field']); + + if (isset($this->fieldMappingConditions[$condition['field']])) { + $mapping_condition = $this->fieldMappingConditions[$condition['field']]; + $mapping_condition['value'] = SparqlArg::toResourceUris($mapping_condition['value']); + $mapping_condition['field'] = $field_predicate; + $condition_strings[] = $this->compileValuesFilter($mapping_condition); + } + + foreach ($condition_strings as $condition_string) { + if (array_search($condition_string, $this->conditionFragments) === FALSE) { + $this->addConditionFragment($condition_string); + } + } + } + + /** + * Compiles a filter not exists condition. + * + * @param array $condition + * An array that contains the 'field', 'value', 'operator' values. + */ + protected function compileNotExists(array $condition): void { $prefix = self::$filterOperatorMap[$condition['operator']]['prefix']; $suffix = self::$filterOperatorMap[$condition['operator']]['suffix']; $field_predicate = $this->fieldMappings[$condition['field']]; - $condition_string = self::ID_KEY . ' ' . $this->escapePredicate($field_predicate) . ' ' . SparqlArg::toVar($condition['field']); + $condition_strings = []; + $condition_strings[] = self::ID_KEY . ' ' . $this->escapePredicate($field_predicate) . ' ' . SparqlArg::toVar($condition['field']); if (isset($this->fieldMappingConditions[$condition['field']])) { $mapping_condition = $this->fieldMappingConditions[$condition['field']]; $mapping_condition['value'] = SparqlArg::toResourceUris($mapping_condition['value']); $mapping_condition['field'] = $field_predicate; - $condition_string .= ' . ' . $this->compileValuesFilter($mapping_condition); + $condition_strings[] = $this->compileValuesFilter($mapping_condition); } - $key = array_search($condition_string, $this->conditionFragments); - // Only add a condition if the mapping condition is not found. If found, - // replace it, in order to avoid creating an EXISTS and NOT EXISTS on the - // same property. - if ($key !== FALSE) { - $this->conditionFragments[$key] = $prefix . $this->conditionFragments[$key] . $suffix; - } - else { - $this->addConditionFragment($prefix . $condition_string . $suffix); + foreach ($condition_strings as $condition_string) { + $key = array_search($condition_string, $this->conditionFragments); + // Since field mapping conditions act also as EXISTS (the triple patterns + // MUST exist), remove any pattern added in the mapping conditions so that + // only the negative condition below exists. + if ($key !== FALSE) { + unset($this->conditionFragments[$key]); + } } + + $this->addConditionFragment($prefix . implode(' . ', $condition_strings) . $suffix); + } /** From d3327a67bc66de11faadbe02032cc517f243121a Mon Sep 17 00:00:00 2001 From: Ilias Dimopoulos Date: Fri, 9 Apr 2021 16:35:19 +0300 Subject: [PATCH 8/9] ISAICP-6242: Accidentally moved from a negative to a positive condition. Revert which cases do not require a default triple. --- src/Entity/Query/Sparql/SparqlCondition.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Entity/Query/Sparql/SparqlCondition.php b/src/Entity/Query/Sparql/SparqlCondition.php index 7928cb7..2913460 100644 --- a/src/Entity/Query/Sparql/SparqlCondition.php +++ b/src/Entity/Query/Sparql/SparqlCondition.php @@ -271,6 +271,10 @@ public function condition($field = NULL, $value = NULL, $operator = NULL, $lang 'lang' => $lang, 'column' => $column, ]; + + if ($operator !== 'NOT EXISTS') { + $this->requiresDefaultPattern = FALSE; + } } return $this; From 0c2926684bd75c0dc8bfa8387bff1e962a7ca768 Mon Sep 17 00:00:00 2001 From: Claudiu Cristea Date: Tue, 13 Apr 2021 18:55:31 +0300 Subject: [PATCH 9/9] ISAICP-6242: More strong assertion. --- tests/src/Kernel/SparqlEntityQueryTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/src/Kernel/SparqlEntityQueryTest.php b/tests/src/Kernel/SparqlEntityQueryTest.php index ace3e25..29fec6e 100644 --- a/tests/src/Kernel/SparqlEntityQueryTest.php +++ b/tests/src/Kernel/SparqlEntityQueryTest.php @@ -465,7 +465,8 @@ public function testNotExists() { ->notExists('text') ->execute(); - $this->assertNotEmpty($results); + $this->assertCount(1, $results); + $this->assertContains('http://fruit.example.com/not_exists', $results); } /**