Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues noticed by SonarCloud #158

Merged
merged 8 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions doc/05_Extending_Data_Index/06_Extend_Search_Index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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';
Expand Down Expand Up @@ -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;
}

Expand All @@ -188,7 +188,7 @@ class CarOwnerSubscriber implements EventSubscriberInterface

public function onExtractMapping(ExtractMappingEvent $event)
{
if($event->getClassDefinition()->getId() !== 'CAR') {
if ($event->getClassDefinition()->getId() !== 'CAR') {
return;
}

Expand Down
71 changes: 41 additions & 30 deletions src/DependencyInjection/Compiler/SearchModifierHandlerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand All @@ -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();

Expand All @@ -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 ' .
Expand All @@ -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 ' .
Expand Down Expand Up @@ -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,
Expand All @@ -187,7 +198,7 @@ private function checkArgumentInstanceOf(
|| in_array($classOrInterface, class_implements($type->getName()), true)
);

} catch(Exception) {
} catch (Exception) {
return false;
}
}
Expand Down
14 changes: 12 additions & 2 deletions src/DependencyInjection/Compiler/ServiceLocatorPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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' =>
Expand Down Expand Up @@ -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)) {
Expand All @@ -80,6 +91,5 @@ public function process(ContainerBuilder $container): void
}
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function getPath(string $subField = null): string
{
$path = FieldCategory::SYSTEM_FIELDS->value . '.' . $this->value;

if($subField) {
if ($subField) {
$path .= '.' . $subField;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Model/OpenSearch/Query/BoolQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Model/OpenSearch/Query/QueryList.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Model/OpenSearch/Query/WildcardFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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*";
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/Model/Search/Asset/SearchResult/AssetSearchResultItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ class AssetSearchResultItem

private AssetPermissions $permissions;

public function __construct(
) {
}

public function getId(): int
{
return $this->id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ class DataObjectSearchResultItem

private DataObjectPermissions $permissions;

public function __construct(
) {
}

public function getId(): int
{
return $this->id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ class DocumentSearchResultItem

private DocumentPermissions $permissions;

public function __construct(
) {
}

public function getId(): int
{
return $this->id;
Expand Down
Loading
Loading