Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public function mapFields(Content $content)
$ancestorLocationsContentIds[] = $contentInfo->ownerId;

$section = $this->sectionHandler->load($contentInfo->sectionId);
$contentType = $this->contentTypeHandler->load($contentInfo->contentTypeId);

return [
new Field(
Expand Down Expand Up @@ -173,9 +174,14 @@ public function mapFields(Content $content)
),
new Field(
'content_type_group_ids',
$this->contentTypeHandler->load($contentInfo->contentTypeId)->groupIds,
$contentType->groupIds,
new FieldType\MultipleIdentifierField()
),
new Field(
'content_type_is_container',
$contentType->isContainer,
new FieldType\BooleanField()
),
new Field(
'content_object_state_ids',
$this->getObjectStateIds($contentInfo->id),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use Ibexa\Contracts\Core\Persistence\Content\Handler as ContentHandler;
use Ibexa\Contracts\Core\Persistence\Content\Location;
use Ibexa\Contracts\Core\Persistence\Content\Type\Handler as ContentTypeHandler;
use Ibexa\Contracts\Core\Search\Field;
use Ibexa\Contracts\Core\Search\FieldType;
use Ibexa\Contracts\Solr\DocumentMapper;
Expand All @@ -23,9 +24,14 @@ class LocationDocumentBaseFields extends LocationFieldMapper
*/
protected $contentHandler;

public function __construct(ContentHandler $contentHandler)
{
protected ContentTypeHandler $contentTypeHandler;

public function __construct(
ContentHandler $contentHandler,
ContentTypeHandler $contentTypeHandler
) {
$this->contentHandler = $contentHandler;
$this->contentTypeHandler = $contentTypeHandler;
}

public function accept(Location $location)
Expand All @@ -36,6 +42,7 @@ public function accept(Location $location)
public function mapFields(Location $location)
{
$contentInfo = $this->contentHandler->loadContentInfo($location->contentId);
$contentType = $this->contentTypeHandler->load($contentInfo->contentTypeId);

return [
new Field(
Expand Down Expand Up @@ -109,6 +116,11 @@ public function mapFields(Location $location)
($location->id == $contentInfo->mainLocationId),
new FieldType\BooleanField()
),
new Field(
'is_container',
$contentType->isContainer,
new FieldType\BooleanField()
),
];
}

Expand Down
41 changes: 41 additions & 0 deletions src/lib/Query/Common/CriterionVisitor/BaseIsContainer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Solr\Query\Common\CriterionVisitor;

use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Operator;
use Ibexa\Contracts\Solr\Query\CriterionVisitor;

/**
* @internal
*/
abstract class BaseIsContainer extends CriterionVisitor
{
abstract protected function getTargetField(): string;

public function canVisit(Criterion $criterion): bool
{
return $criterion instanceof Criterion\IsContainer && $criterion->operator === Operator::EQ;
}

public function visit(Criterion $criterion, CriterionVisitor $subVisitor = null): string
{
$value = $criterion->value;

if (!is_array($value) || !is_bool($value[0])) {
throw new \LogicException(sprintf(
'%s value should be of type array<bool>, received %s.',
Criterion\IsContainer::class,
get_debug_type($value),
));
}

return $this->getTargetField() . ':' . $this->toString($value[0]);
}
}
22 changes: 22 additions & 0 deletions src/lib/Query/Content/CriterionVisitor/IsContainer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Solr\Query\Content\CriterionVisitor;

use Ibexa\Solr\Query\Common\CriterionVisitor\BaseIsContainer;

/**
* @internal
*/
final class IsContainer extends BaseIsContainer
{
public function getTargetField(): string
{
return 'content_type_is_container_b';
}
}
22 changes: 22 additions & 0 deletions src/lib/Query/Location/CriterionVisitor/IsContainer.php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost the same code as the Content\CriterionVisitor. Consider extracting common code.

Copy link
Contributor Author

@tischsoic tischsoic Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I have checked, we do not extract the common part (e.g. LocationIdIn, LocationRemoteIdIn etc.) so I didn't want to introduce another abstraction just for IsContainer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I have checked, we do not extract the common part (e.g. LocationIdIn, LocationRemoteIdIn etc.) so I didn't want to introduce another abstraction just for IsContainer.

This is a technical debt that can be easily mitigated. Sonar Cloud analysis is valid.
My question here would be if this can be refactored into a common base for all visitors or just for IsContainer? The latter approach is a minimum requirement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that extracting this code into an abstract is debatable in this case. Those classes share code, but are for different domains.

I would argue that a trait is a candidate, especially since there are valid cases where code related to checking value is different for a Criterion. We basically need to inject a validation method to ensure that argument passed is correct. This is a specific behavior that we need for a particular set of Criterions.

Thoughts?

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Solr\Query\Location\CriterionVisitor;

use Ibexa\Solr\Query\Common\CriterionVisitor\BaseIsContainer;

/**
* @internal
*/
final class IsContainer extends BaseIsContainer
{
public function getTargetField(): string
{
return 'is_container_b';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ services:
- {name: ibexa.search.solr.query.content.criterion.visitor}
- {name: ibexa.search.solr.query.location.criterion.visitor}

Ibexa\Solr\Query\Content\CriterionVisitor\IsContainer:
tags:
- { name: ibexa.search.solr.query.content.criterion.visitor }

# Location search
Ibexa\Solr\Query\Location\CriterionVisitor\Ancestor:
tags:
Expand Down Expand Up @@ -291,6 +295,10 @@ services:
tags:
- {name: ibexa.search.solr.query.location.criterion.visitor}

Ibexa\Solr\Query\Location\CriterionVisitor\IsContainer:
tags:
- { name: ibexa.search.solr.query.location.criterion.visitor }

Ibexa\Solr\Query\Location\CriterionVisitor\Factory\LocationFullTextFactory:
parent: Ibexa\Solr\Query\Common\CriterionVisitor\Factory\FullTextFactoryAbstract

Expand Down
1 change: 1 addition & 0 deletions src/lib/Resources/config/container/solr/field_mappers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ services:
Ibexa\Solr\FieldMapper\LocationFieldMapper\LocationDocumentBaseFields:
arguments:
- '@Ibexa\Contracts\Core\Persistence\Content\Handler'
- '@Ibexa\Contracts\Core\Persistence\Content\Type\Handler'
tags:
- {name: ibexa.search.solr.field.mapper.location}

Expand Down