Fix issues noticed by SonarCloud (#158)

pimcore · May 22, 2024 · 8926dbb · 8926dbb
1 parent 78df07c
commit 8926dbb
Show file tree

Hide file tree

Showing 50 changed files with 148 additions and 137 deletions.
diff --git a/doc/05_Extending_Data_Index/06_Extend_Search_Index.md b/doc/05_Extending_Data_Index/06_Extend_Search_Index.md
@@ -89,7 +89,7 @@ class FileSizeIndexSubscriber implements EventSubscriberInterface
     public function onUpdateIndexData(UpdateIndexDataEvent $event)
     {
         $asset = $event->getAsset();
-        if($asset instanceof Folder) {
+        if ($asset instanceof Folder) {
             return;
         }
 
@@ -98,9 +98,9 @@ class FileSizeIndexSubscriber implements EventSubscriberInterface
 
         $fileSize = $event->getAsset()->getFileSize();
         $fileSizeSelection = null;
-        if($fileSize < 3*1000) {
+        if ($fileSize < 3*1000) {
             $fileSizeSelection = 'small';
-        } elseif($fileSize <= 3*1000*1000) {
+        } elseif ($fileSize <= 3*1000*1000) {
             $fileSizeSelection = 'medium';
         } else {
             $fileSizeSelection = 'big';
@@ -174,7 +174,7 @@ class CarOwnerSubscriber implements EventSubscriberInterface
     public function onUpdateIndexData(UpdateIndexDataEvent $event)
     {
         $car = $event->getDataObject();
-        if(!$car instanceof Car) {
+        if (!$car instanceof Car) {
             return;
         }
 
@@ -188,7 +188,7 @@ class CarOwnerSubscriber implements EventSubscriberInterface
 
     public function onExtractMapping(ExtractMappingEvent $event)
     {
-        if($event->getClassDefinition()->getId() !== 'CAR') {
+        if ($event->getClassDefinition()->getId() !== 'CAR') {
             return;
         }
 

diff --git a/src/DependencyInjection/Compiler/SearchModifierHandlerPass.php b/src/DependencyInjection/Compiler/SearchModifierHandlerPass.php
@@ -24,6 +24,7 @@
 use Pimcore\Bundle\GenericDataIndexBundle\SearchIndexAdapter\Search\Modifier\SearchModifierServiceInterface;
 use ReflectionClass;
 use ReflectionException;
+use ReflectionMethod;
 use ReflectionNamedType;
 use ReflectionUnionType;
 use Symfony\Component\DependencyInjection\ChildDefinition;
@@ -49,15 +50,15 @@ public function process(ContainerBuilder $container): void
             foreach ($tags as $tag) {
                 $className = $this->getServiceClass($container, $serviceId);
                 $r = $container->getReflectionClass($className);
-                if($r === null) {
+                if ($r === null) {
                     continue;
                 }
 
                 $method = $tag['method'] ?? '__invoke';
 
                 $handles = $this->guessHandledClasses($r, $serviceId, $method);
 
-                foreach($handles as $handledClass) {
+                foreach ($handles as $handledClass) {
                     $searchModifierServiceDefinition->addMethodCall(
                         method: 'addSearchModifierHandler',
                         arguments: [$handledClass, new Reference($serviceId), $method]
@@ -73,31 +74,7 @@ private function guessHandledClasses(
         string $serviceId,
         string $methodName
     ): iterable {
-        try {
-            $method = $handlerClass->getMethod($methodName);
-        } catch (ReflectionException) {
-            throw new RuntimeException(
-                sprintf(
-                    'Invalid handler service "%s": class "%s" must have an "%s()" method.',
-                    $serviceId,
-                    $handlerClass->getName(),
-                    $methodName
-                )
-            );
-        }
-
-        if ($method->getNumberOfRequiredParameters() !== 2) {
-            throw new RuntimeException(
-                sprintf(
-                    'Invalid handler service "%s": method "%s::%s()" requires exactly two arguments, ' .
-                    'first one being the search modifier model it handles and ' .
-                    'second one the SearchModifierContext object.',
-                    $serviceId,
-                    $handlerClass->getName(),
-                    $methodName
-                )
-            );
-        }
+        $method = $this->getMethodFromHandlerClass($handlerClass, $methodName, $serviceId);
 
         $parameters = $method->getParameters();
 
@@ -109,7 +86,7 @@ private function guessHandledClasses(
         );
         //@todo check for ReflectionUnionType if !$searchModifierValid
 
-        if(!$searchModifierValid) {
+        if (!$searchModifierValid) {
             throw new RuntimeException(
                 sprintf(
                     'Invalid handler service "%s": argument "$%s" of method "%s::%s()" must have ' .
@@ -131,7 +108,7 @@ private function guessHandledClasses(
             true
         );
 
-        if(!$contextTypeValid) {
+        if (!$contextTypeValid) {
             throw new RuntimeException(
                 sprintf(
                     'Invalid handler service "%s": argument "$%s" of method "%s::%s()" must have ' .
@@ -174,6 +151,40 @@ private function guessHandledClasses(
         return [$searchModifierType->getName()];
     }
 
+    private function getMethodFromHandlerClass(
+        ReflectionClass $handlerClass,
+        string $methodName,
+        string $serviceId
+    ): ReflectionMethod {
+        try {
+            $method = $handlerClass->getMethod($methodName);
+        } catch (ReflectionException) {
+            throw new RuntimeException(
+                sprintf(
+                    'Invalid handler service "%s": class "%s" must have an "%s()" method.',
+                    $serviceId,
+                    $handlerClass->getName(),
+                    $methodName
+                )
+            );
+        }
+
+        if ($method->getNumberOfRequiredParameters() !== 2) {
+            throw new RuntimeException(
+                sprintf(
+                    'Invalid handler service "%s": method "%s::%s()" requires exactly two arguments, ' .
+                    'first one being the search modifier model it handles and ' .
+                    'second one the SearchModifierContext object.',
+                    $serviceId,
+                    $handlerClass->getName(),
+                    $methodName
+                )
+            );
+        }
+
+        return $method;
+    }
+
     private function checkArgumentInstanceOf(
         ReflectionNamedType|ReflectionUnionType|null $type,
         string $classOrInterface,
@@ -187,7 +198,7 @@ private function checkArgumentInstanceOf(
                     || in_array($classOrInterface, class_implements($type->getName()), true)
                 );
 
-        } catch(Exception) {
+        } catch (Exception) {
             return false;
         }
     }

diff --git a/src/DependencyInjection/Compiler/ServiceLocatorPass.php b/src/DependencyInjection/Compiler/ServiceLocatorPass.php
@@ -18,6 +18,7 @@
 
 use Pimcore\Bundle\GenericDataIndexBundle\Enum\DependencyInjection\ServiceTag;
 use Pimcore\Bundle\GenericDataIndexBundle\SearchIndexAdapter\OpenSearch\QueryLanguage\FieldNameTransformerInterface;
+use Pimcore\Bundle\GenericDataIndexBundle\SearchIndexAdapter\OpenSearch\QueryLanguage\FieldNameValidatorInterface;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
@@ -32,6 +33,12 @@ final class ServiceLocatorPass implements CompilerPassInterface
      * @param ContainerBuilder $container
      */
     public function process(ContainerBuilder $container): void
+    {
+        $this->handleServiceLocatorDefinitions($container);
+        $this->handleTaggedIteratorDefinitions($container);
+    }
+
+    private function handleServiceLocatorDefinitions(ContainerBuilder $container): void
     {
         $definitionList = [
             'pimcore.generic_data_index.object.search_index_field_definition_locator' =>
@@ -64,12 +71,16 @@ public function process(ContainerBuilder $container): void
             $serviceLocator = $container->getDefinition($definitionId);
             $serviceLocator->setArgument(0, $arguments);
         }
+    }
 
+    private function handleTaggedIteratorDefinitions(ContainerBuilder $container): void
+    {
         $definitionList = [
             ServiceTag::PQL_FIELD_NAME_TRANSFORMER->value => FieldNameTransformerInterface::class,
+            ServiceTag::PQL_FIELD_NAME_VALIDATOR->value => FieldNameValidatorInterface::class,
         ];
 
-        foreach($definitionList as $serviceTagName => $interfaceName) {
+        foreach ($definitionList as $serviceTagName => $interfaceName) {
             foreach ($container->findTaggedServiceIds($serviceTagName) as $taggedServiceId => $tags) {
                 $definition = $container->getDefinition($taggedServiceId);
                 if (!is_subclass_of($definition->getClass(), $interfaceName)) {
@@ -80,6 +91,5 @@ public function process(ContainerBuilder $container): void
                 }
             }
         }
-
     }
 }
diff --git a/src/Enum/SearchIndex/FieldCategory/SystemField/SystemFieldTrait.php b/src/Enum/SearchIndex/FieldCategory/SystemField/SystemFieldTrait.php
@@ -24,7 +24,7 @@ public function getPath(string $subField = null): string
     {
         $path = FieldCategory::SYSTEM_FIELDS->value . '.' . $this->value;
 
-        if($subField) {
+        if ($subField) {
             $path .= '.' . $subField;
         }
 

diff --git a/src/Model/OpenSearch/Query/BoolQuery.php b/src/Model/OpenSearch/Query/BoolQuery.php
@@ -48,10 +48,10 @@ public function getParams(): array
     public function addCondition(string $type, array $params): BoolQuery
     {
         $this->params[$type] = $this->params[$type] ?? [];
-        if(!empty($this->params[$type]) && !array_is_list($this->params[$type])) {
+        if (!empty($this->params[$type]) && !array_is_list($this->params[$type])) {
             $this->params[$type] = [$this->params[$type]];
         }
-        if(array_is_list($params)) {
+        if (array_is_list($params)) {
             $this->params[$type] = array_merge($this->params[$type], $params);
         } else {
             $this->params[$type][] = $params;

diff --git a/src/Model/OpenSearch/Query/QueryList.php b/src/Model/OpenSearch/Query/QueryList.php
@@ -34,7 +34,7 @@ public function __construct(
     public function addQuery(QueryInterface $query = null): QueryList
     {
         if ($query instanceof BoolQuery && !$query->isEmpty()) {
-            if($this->boolQuery !== null) {
+            if ($this->boolQuery !== null) {
                 $this->boolQuery->merge($query);
 
                 return $this;

diff --git a/src/Model/OpenSearch/Query/WildcardFilter.php b/src/Model/OpenSearch/Query/WildcardFilter.php
@@ -57,11 +57,11 @@ private function getWildcardQueryArray(): array
         $term = $this->term;
 
         if ($term !== '' && !str_contains($term, '*')) {
-            if($this->defaultWildcardMode === WildcardFilterMode::BOTH) {
+            if ($this->defaultWildcardMode === WildcardFilterMode::BOTH) {
                 $term = "*$term*";
-            } elseif($this->defaultWildcardMode === WildcardFilterMode::PREFIX) {
+            } elseif ($this->defaultWildcardMode === WildcardFilterMode::PREFIX) {
                 $term = "*$term";
-            } elseif($this->defaultWildcardMode === WildcardFilterMode::SUFFIX) {
+            } elseif ($this->defaultWildcardMode === WildcardFilterMode::SUFFIX) {
                 $term = "$term*";
             }
         }

diff --git a/src/Model/Search/Asset/SearchResult/AssetSearchResultItem.php b/src/Model/Search/Asset/SearchResult/AssetSearchResultItem.php
@@ -59,10 +59,6 @@ class AssetSearchResultItem
 
     private AssetPermissions $permissions;
 
-    public function __construct(
-    ) {
-    }
-
     public function getId(): int
     {
         return $this->id;

diff --git a/src/Model/Search/DataObject/SearchResult/DataObjectSearchResultItem.php b/src/Model/Search/DataObject/SearchResult/DataObjectSearchResultItem.php
@@ -56,10 +56,6 @@ class DataObjectSearchResultItem
 
     private DataObjectPermissions $permissions;
 
-    public function __construct(
-    ) {
-    }
-
     public function getId(): int
     {
         return $this->id;

diff --git a/src/Model/Search/Document/SearchResult/DocumentSearchResultItem.php b/src/Model/Search/Document/SearchResult/DocumentSearchResultItem.php
@@ -56,10 +56,6 @@ class DocumentSearchResultItem
 
     private DocumentPermissions $permissions;
 
-    public function __construct(
-    ) {
-    }
-
     public function getId(): int
     {
         return $this->id;