From d802655c322fda2a0f7df36729c3ece31572ee24 Mon Sep 17 00:00:00 2001
From: lukmzig <30526586+lukmzig@users.noreply.github.com>
Date: Wed, 13 Nov 2024 09:27:05 +0100
Subject: [PATCH] [Bug] Wring index name passed for data objects deletion
 (#251)

* fix: pass correct index name for data objects deletion

* Apply php-cs-fixer changes

* code style

---------

Co-authored-by: lukmzig <lukmzig@users.noreply.github.com>
---
 .../DataObjectTypeAdapter.php                 | 60 ++++++++++++++-----
 .../Functional/SearchIndex/IndexQueueTest.php | 54 +++++++++++------
 tests/Support/Helper/GenericDataIndex.php     | 18 ++++++
 3 files changed, 99 insertions(+), 33 deletions(-)

diff --git a/src/Service/SearchIndex/IndexService/ElementTypeAdapter/DataObjectTypeAdapter.php b/src/Service/SearchIndex/IndexService/ElementTypeAdapter/DataObjectTypeAdapter.php
index 8cb39284..f7f1ce02 100644
--- a/src/Service/SearchIndex/IndexService/ElementTypeAdapter/DataObjectTypeAdapter.php
+++ b/src/Service/SearchIndex/IndexService/ElementTypeAdapter/DataObjectTypeAdapter.php
@@ -22,6 +22,7 @@
 use InvalidArgumentException;
 use Pimcore\Bundle\GenericDataIndexBundle\Enum\SearchIndex\ElementType;
 use Pimcore\Bundle\GenericDataIndexBundle\Enum\SearchIndex\IndexName;
+use Pimcore\Bundle\GenericDataIndexBundle\Enum\SearchIndex\IndexQueueOperation;
 use Pimcore\Bundle\GenericDataIndexBundle\Event\DataObject\UpdateFolderIndexDataEvent;
 use Pimcore\Bundle\GenericDataIndexBundle\Event\DataObject\UpdateIndexDataEvent;
 use Pimcore\Bundle\GenericDataIndexBundle\Event\UpdateIndexDataEventInterface;
@@ -105,22 +106,10 @@ public function getRelatedItemsOnUpdateQuery(
         }
 
         if (!$element->getClass()->getAllowInherit()) {
-            if ($includeElement) {
-                return $this->dbConnection->createQueryBuilder()
-                    ->select([
-                        $element->getId(),
-                        "'" . ElementType::DATA_OBJECT->value . "'",
-                        'className',
-                        "'$operation'",
-                        "'$operationTime'",
-                        '0',
-                    ])
-                    ->from('objects') // just a dummy query to fit into the query builder interface
-                    ->where('id = :id')
-                    ->setMaxResults(1)
-                    ->setParameter('id', $element->getId());
-            }
+            return $this->getRelatedItemsQueryBuilder($element, $operation, $operationTime, $includeElement);
+        }
 
+        if ($operation !== IndexQueueOperation::UPDATE->value) {
             return null;
         }
 
@@ -163,4 +152,45 @@ public function getUpdateIndexDataEvent(
 
         throw new InvalidArgumentException('Element must be instance of ' . AbstractObject::class);
     }
+
+    private function getRelatedItemsQueryBuilder(
+        Concrete $element,
+        string $operation,
+        int $operationTime,
+        bool $includeElement = false
+    ): ?QueryBuilder {
+        if (!$includeElement) {
+            return null;
+        }
+
+        $queryBuilder = $this->dbConnection->createQueryBuilder()
+            ->select($this->getSelectParametersByOperation($element, $operation, $operationTime))
+            ->setMaxResults(1);
+
+        if ($operation === IndexQueueOperation::DELETE->value) {
+            return $queryBuilder->from('DUAL');
+        }
+
+        return $queryBuilder
+            ->from('objects')
+            ->where('id = :id')
+            ->setParameter('id', $element->getId());
+    }
+
+    private function getSelectParametersByOperation(Concrete $element, string $operation, int $operationTime): array
+    {
+        $classId = 'className';
+        if ($operation === IndexQueueOperation::DELETE->value) {
+            $classId = "'" . $element->getClassId() . "'";
+        }
+
+        return [
+            $element->getId(),
+            "'" . ElementType::DATA_OBJECT->value . "'",
+            $classId,
+            "'$operation'",
+            "'$operationTime'",
+            '0',
+        ];
+    }
 }
diff --git a/tests/Functional/SearchIndex/IndexQueueTest.php b/tests/Functional/SearchIndex/IndexQueueTest.php
index 90d8a170..04aa188f 100644
--- a/tests/Functional/SearchIndex/IndexQueueTest.php
+++ b/tests/Functional/SearchIndex/IndexQueueTest.php
@@ -137,34 +137,52 @@ public function testAssetSaveProcessQueue(): void
     /**
      * @throws Exception
      */
-    public function testElementDeleteWithQueue(): void
+    public function testAssetDeleteWithQueue(): void
     {
-        $this->checkAndDeleteElement(
-            TestHelper::createImageAsset(),
-            $this->searchIndexConfigService->getIndexName(self::ASSET_INDEX_NAME)
-        );
+        $asset = TestHelper::createImageAsset();
+        $assetIndex = $this->searchIndexConfigService->getIndexName(self::ASSET_INDEX_NAME);
+        $this->consume();
 
-        $this->checkAndDeleteElement(
-            TestHelper::createEmptyDocument(),
-            $this->searchIndexConfigService->getIndexName(self::DOCUMENT_INDEX_NAME)
-        );
+        $this->checkAndDeleteElement($asset, $assetIndex);
+        $this->consume();
 
-        $object = TestHelper::createEmptyObject('', false);
-        $this->checkAndDeleteElement(
-            $object,
-            $this->searchIndexConfigService->getIndexName($object->getClassName())
-        );
+        $this->tester->checkDeletedIndexEntry($asset->getId(), $assetIndex);
     }
 
-    private function checkAndDeleteElement(ElementInterface $element, string $indexName): void
+    /**
+     * @throws Exception
+     */
+    public function testDocumentDeleteWithQueue(): void
     {
+        $document = TestHelper::createEmptyDocument();
+        $documentIndex = $this->searchIndexConfigService->getIndexName(self::DOCUMENT_INDEX_NAME);
         $this->consume();
-        $this->tester->checkIndexEntry($element->getId(), $indexName);
 
-        $element->delete();
+        $this->checkAndDeleteElement($document, $documentIndex);
         $this->consume();
-        $this->expectException(Missing404Exception::class);
+
+        $this->tester->checkDeletedIndexEntry($document->getId(), $documentIndex);
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testDataObjectDeleteWithQueue(): void
+    {
+        $object = TestHelper::createEmptyObject();
+        $objectIndex = $this->searchIndexConfigService->getIndexName($object->getClassName());
+        $this->consume();
+
+        $this->checkAndDeleteElement($object, $objectIndex);
+        $this->consume();
+
+        $this->tester->checkDeletedIndexEntry($object->getId(), $objectIndex);
+    }
+
+    private function checkAndDeleteElement(ElementInterface $element, string $indexName): void
+    {
         $this->tester->checkIndexEntry($element->getId(), $indexName);
+        $element->delete();
     }
 
     private function consume(): void
diff --git a/tests/Support/Helper/GenericDataIndex.php b/tests/Support/Helper/GenericDataIndex.php
index c60a8132..c9f82e49 100644
--- a/tests/Support/Helper/GenericDataIndex.php
+++ b/tests/Support/Helper/GenericDataIndex.php
@@ -147,6 +147,24 @@ public function checkIndexEntry(string $id, string $index): array
         return $response;
     }
 
+    public function checkDeletedIndexEntry(string $id, string $index): void
+    {
+        $client = $this->getIndexSearchClient();
+        $response = $client->get([
+            'id' => $id,
+            'index' => $index,
+            'client' => ['ignore' => [404]],
+        ]);
+
+        if (isset($response['found'])) {
+            $this->assertFalse($response['found'], 'Check OpenSearch document id of element');
+
+            return;
+        }
+
+        $this->assertNotContains($id, $response);
+    }
+
     public function flushIndex()
     {
         $client = $this->getIndexSearchClient();