BUGFIX: Avoid duplicate results and respect filter in NodeSearchService

Moves the "search by node id" logic from the `NodesController` to the `NodeSearchService` fixing the following regressions: * Duplicate results will be filtered * Respect `$searchableNodeTypeNames` argument * Don't execute `getNodeByIdentifier()` twice for every search Resolves: #2079 Related: #1894
neos · Jun 13, 2018 · 0d7a0b0 · 0d7a0b0
1 parent d4a0b8a
commit 0d7a0b0
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 13 deletions.
diff --git a/Classes/Controller/Service/NodesController.php b/Classes/Controller/Service/NodesController.php
@@ -11,7 +11,6 @@
  * source code.
  */
 
-use Neos\ContentRepository\Validation\Validator\NodeIdentifierValidator;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Mvc\Controller\ActionController;
 use Neos\Flow\Property\Exception;
@@ -109,19 +108,8 @@ public function indexAction($searchTerm = '', array $nodeIdentifiers = array(),
         }
 
         $contentContext = $this->createContentContext($workspaceName, $dimensions);
-        $nodes = [];
-
-        if (preg_match(NodeIdentifierValidator::PATTERN_MATCH_NODE_IDENTIFIER, $searchTerm) !== 0
-            && $contentContext->getNodeByIdentifier($searchTerm) instanceof NodeInterface
-        ) {
-            $nodes[] = $contentContext->getNodeByIdentifier($searchTerm);
-        }
-
         if ($nodeIdentifiers === array()) {
-            $nodes = array_merge(
-                $nodes,
-                $this->nodeSearchService->findByProperties($searchTerm, $searchableNodeTypeNames, $contentContext, $contextNode)
-            );
+            $nodes = $this->nodeSearchService->findByProperties($searchTerm, $searchableNodeTypeNames, $contentContext, $contextNode);
         } else {
             $nodes = array_map(function ($identifier) use ($contentContext) {
                 return $contentContext->getNodeByIdentifier($identifier);

diff --git a/Classes/Domain/Service/NodeSearchService.php b/Classes/Domain/Service/NodeSearchService.php
@@ -11,6 +11,7 @@
  * source code.
  */
 
+use Neos\ContentRepository\Validation\Validator\NodeIdentifierValidator;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Persistence\PersistenceManagerInterface;
 use Neos\ContentRepository\Domain\Factory\NodeFactory;
@@ -59,8 +60,16 @@ public function findByProperties($term, array $searchNodeTypes, Context $context
         if (empty($term)) {
             throw new \InvalidArgumentException('"term" cannot be empty: provide a term to search for.', 1421329285);
         }
+
         $searchResult = array();
         $nodeTypeFilter = implode(',', $searchNodeTypes);
+
+        if (preg_match(NodeIdentifierValidator::PATTERN_MATCH_NODE_IDENTIFIER, $term) !== 0) {
+            $nodeByIdentifier = $context->getNodeByIdentifier($term);
+            if ($nodeByIdentifier !== null && $this->nodeSatisfiesSearchNodeTypes($nodeByIdentifier, $searchNodeTypes)) {
+                $searchResult[$nodeByIdentifier->getPath()] = $nodeByIdentifier;
+            }
+        }
         $nodeDataRecords = $this->nodeDataRepository->findByProperties($term, $nodeTypeFilter, $context->getWorkspace(), $context->getDimensions(), $startingPoint ? $startingPoint->getPath() : null);
         foreach ($nodeDataRecords as $nodeData) {
             $node = $this->nodeFactory->createFromNodeData($nodeData, $context);
@@ -71,4 +80,21 @@ public function findByProperties($term, array $searchNodeTypes, Context $context
 
         return $searchResult;
     }
+
+    /**
+     * Whether or not the given $node satisfies the specified types
+     *
+     * @param NodeInterface $node
+     * @param array $searchNodeTypes
+     * @return bool
+     */
+    protected function nodeSatisfiesSearchNodeTypes(NodeInterface $node, array $searchNodeTypes): bool
+    {
+        foreach ($searchNodeTypes as $nodeTypeName) {
+            if ($node->getNodeType()->isOfType($nodeTypeName)) {
+                return true;
+            }
+        }
+        return false;
+    }
 }