EZP-25792: Deleted location don't affect subitems (#46)

* First Petar prototype * EZP-25792: Fix delete location affect also subitems * Code review fixes
ezsystems · Jun 8, 2016 · ce42dc7 · ce42dc7
1 parent 1408192
commit ce42dc7
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 33 deletions.
diff --git a/lib/Gateway.php b/lib/Gateway.php
@@ -40,6 +40,14 @@ abstract public function findContent(Query $query, array $fieldFilters = array()
      */
     abstract public function findLocations(Query $query, array $fieldFilters = array());
 
+    /**
+     * Returns all search hits for given query, that will be performed on all endpoints.
+     *
+     * @param Query $query
+     * @return mixed
+     */
+    abstract public function searchAllEndpoints(Query $query);
+
     /**
      * Indexes an array of documents.
      *

diff --git a/lib/Gateway/Native.php b/lib/Gateway/Native.php
@@ -151,26 +151,19 @@ protected function internalFind(array $parameters, array $languageSettings = arr
             $parameters['shards'] = $searchTargets;
         }
 
-        $queryString = $this->generateQueryString($parameters);
-
-        $response = $this->client->request(
-            'GET',
-            $this->endpointRegistry->getEndpoint(
-                $this->endpointResolver->getEntryEndpoint()
-            ),
-            "/select?{$queryString}"
-        );
+        return $this->search($parameters);
+    }
 
-        // @todo: Error handling?
-        $result = json_decode($response->body);
+    public function searchAllEndpoints(Query $query)
+    {
+        $parameters = $this->contentQueryConverter->convert($query);
 
-        if (!isset($result->response)) {
-            throw new RuntimeException(
-                '->response not set: ' . var_export(array($result, $parameters), true)
-            );
+        $searchTargets = $this->getAllSearchTargets();
+        if (!empty($searchTargets)) {
+            $parameters['shards'] = $searchTargets;
         }
 
-        return $result;
+        return $this->search($parameters);
     }
 
     /**
@@ -185,11 +178,15 @@ protected function internalFind(array $parameters, array $languageSettings = arr
      */
     protected function generateQueryString(array $parameters)
     {
-        return preg_replace(
+        $removedArrayCharacters = preg_replace(
             '/%5B[0-9]+%5D=/',
             '=',
             http_build_query($parameters)
         );
+
+        $removedDuplicatedEscapingForUrlPath = str_replace('%5C%5C%2F', '%5C%2F', $removedArrayCharacters);
+
+        return $removedDuplicatedEscapingForUrlPath;
     }
 
     /**
@@ -213,6 +210,24 @@ protected function getSearchTargets($languageSettings)
         return implode(',', $shards);
     }
 
+    /**
+     * Returns all search targets without language constraint.
+     *
+     * @return string
+     */
+    protected function getAllSearchTargets()
+    {
+        $shards = [];
+        $searchTargets = $this->endpointResolver->getEndpoints();
+        if (!empty($searchTargets)) {
+            foreach ($searchTargets as $endpointName) {
+                $shards[] = $this->endpointRegistry->getEndpoint($endpointName)->getIdentifier();
+            }
+        }
+
+        return  implode(',', $shards);
+    }
+
     /**
      * Indexes an array of documents.
      *
@@ -517,4 +532,35 @@ protected function writeField(XmlWriter $xmlWriter, Field $field)
             $xmlWriter->endElement();
         }
     }
+
+    /**
+     * Perform request to client to search for records with query string.
+     *
+     * @param array $parameters
+     *
+     * @return mixed
+     */
+    protected function search(array $parameters)
+    {
+        $queryString = $this->generateQueryString($parameters);
+
+        $response = $this->client->request(
+            'GET',
+            $this->endpointRegistry->getEndpoint(
+                $this->endpointResolver->getEntryEndpoint()
+            ),
+            "/select?{$queryString}"
+        );
+
+        // @todo: Error handling?
+        $result = json_decode($response->body);
+
+        if (!isset($result->response)) {
+            throw new RuntimeException(
+                '->response not set: ' . var_export(array($result, $parameters), true)
+            );
+        }
+
+        return $result;
+    }
 }
diff --git a/lib/Handler.php b/lib/Handler.php
@@ -236,7 +236,7 @@ public function indexContent(Content $content)
      *       However it is not added to an official SPI interface yet as we anticipate adding a bulkIndexDocument
      *       using eZ\Publish\SPI\Search\Document instead of bulkIndexContent based on Content objects. However
      *       that won't be added until we have several stable or close to stable advance search engines to make
-     *       sure we match the features of these. 
+     *       sure we match the features of these.
      *       See also {@see Solr\Content\Search\Gateway\Native::bulkIndexContent} for further Solr specific info.
      *
      * @param \eZ\Publish\SPI\Persistence\Content[] $contentObjects
@@ -286,19 +286,8 @@ public function deleteContent($contentId, $versionId = null)
      */
     public function deleteLocation($locationId, $contentId)
     {
-        $idPrefix = $this->mapper->generateContentDocumentId($contentId);
-
-        $this->gateway->deleteByQuery("_root_:{$idPrefix}*");
-
-        // TODO it seems this part of location deletion (not last location) misses integration tests
-        try {
-            $contentInfo = $this->contentHandler->loadContentInfo($contentId);
-        } catch (NotFoundException $e) {
-            return;
-        }
-
-        $content = $this->contentHandler->load($contentId, $contentInfo->currentVersionNo);
-        $this->bulkIndexContent(array($content));
+        $this->deleteAllItemsWithoutAdditionalLocation($locationId);
+        $this->updateAllElementsWithAdditionalLocation($locationId);
     }
 
     /**
@@ -319,11 +308,104 @@ public function purgeIndex()
      * Passing true will also write the data to the safe storage, ensuring durability.
      *
      * @see bulkIndexContent() For info on why this is not on an SPI Interface yet.
-     * 
+     *
      * @param bool $flush
      */
     public function commit($flush = false)
     {
         $this->gateway->commit($flush);
     }
+
+    /**
+     * @param $locationId
+     */
+    protected function deleteAllItemsWithoutAdditionalLocation($locationId)
+    {
+        $query = $this->prepareQuery();
+        $query->filter = new Criterion\LogicalAnd(
+            [
+                $this->allItemsWithinLocation($locationId),
+                new Criterion\LogicalNot($this->allItemsWithinLocationWithAdditionalLocation($locationId)),
+            ]
+        );
+
+        $searchResult = $this->resultExtractor->extract(
+            $this->gateway->searchAllEndpoints($query)
+        );
+
+        foreach ($searchResult->searchHits as $hit) {
+            $idPrefix = $this->mapper->generateContentDocumentId($hit->valueObject->id);
+            $this->gateway->deleteByQuery("_root_:{$idPrefix}*");
+        }
+    }
+
+    /**
+     * @param $locationId
+     */
+    protected function updateAllElementsWithAdditionalLocation($locationId)
+    {
+        $query = $this->prepareQuery();
+        $query->filter = new Criterion\LogicalAnd(
+            [
+                $this->allItemsWithinLocation($locationId),
+                $this->allItemsWithinLocationWithAdditionalLocation($locationId),
+            ]
+        );
+
+        $searchResult = $this->resultExtractor->extract(
+            $this->gateway->searchAllEndpoints($query)
+        );
+
+        $contentItems = [];
+        foreach ($searchResult->searchHits as $searchHit) {
+            try {
+                $contentInfo = $this->contentHandler->loadContentInfo($searchHit->valueObject->id);
+            } catch (NotFoundException $e) {
+                continue;
+            }
+
+            $contentItems[] = $this->contentHandler->load($contentInfo->id, $contentInfo->currentVersionNo);
+        }
+
+        $this->bulkIndexContent($contentItems);
+    }
+
+    /**
+     * Prepare standard query for delete purpose.
+     *
+     * @return Query
+     */
+    protected function prepareQuery()
+    {
+        return new Query(
+            [
+                'query' => new Criterion\MatchAll(),
+                'limit' => 1000,
+                'offset' => 0,
+            ]
+        );
+    }
+
+    /**
+     * @param int $locationId
+     * @return Criterion\CustomField
+     */
+    protected function allItemsWithinLocation($locationId)
+    {
+        return new Criterion\CustomField('location_path_string_mid', Criterion\Operator::EQ,
+            "/.*\\/{$locationId}\\/.*/");
+    }
+
+    /**
+     * @param int $locationId
+     * @return Criterion\CustomField
+     */
+    protected function allItemsWithinLocationWithAdditionalLocation($locationId)
+    {
+        return new Criterion\CustomField(
+            'location_path_string_mid',
+            Criterion\Operator::EQ,
+            "/@&~(.*\\/{$locationId}\\/.*)/"
+        );
+    }
 }
diff --git a/lib/Query/Content/CriterionVisitor/CustomField/CustomFieldIn.php b/lib/Query/Content/CriterionVisitor/CustomField/CustomFieldIn.php
@@ -53,9 +53,18 @@ public function visit(Criterion $criterion, CriterionVisitor $subVisitor = null)
         foreach ($values as $value) {
             $preparedValue = $this->escapeQuote($this->toString($value), true);
 
-            $queries[] = $criterion->target . ':"' . $preparedValue . '"';
+            if ($this->isRegExp($preparedValue)) {
+                $queries[] = $criterion->target . ':' . $preparedValue;
+            } else {
+                $queries[] = $criterion->target . ':"' . $preparedValue . '"';
+            }
         }
 
         return '(' . implode(' OR ', $queries) . ')';
     }
+
+    private function isRegExp($preparedValue)
+    {
+        return preg_match('#^/.*/$#', $preparedValue);
+    }
 }