6

Warning, this is a long winded question about module architecture. I'm not mad if it gets closed as opinion based, but I hope someone has a better idea than me.

In Magento 2 we have builders like the SearchCriteriaBuilder which we can inject into our classes similar to factories to let us create objects, in this case SearchCriteria (filters, sort order, page size).

What I like about the builder pattern in general (not Magento specific), is that you can create named constructors for predefined configurations (i.e. parameters for the object to be created) but still can change these configurations before the object is created.

Fictional Example:

$shop = ShopBuilder::demoShop()->withSampleData()->build();

demoShop() returns a preconfigured builder, withSampleData() changes it and build() creates the object.

Magento 2

In Magento 2, named constructors are not really a thing, because they cannot make use of the object manager. Virtual types might be an alternative but they are entirely defined in di.xml, no PHP code. This is a bit limiting and obscure IMHO.

What I want:

I have some search criteria combinations that I need in different places with slight variations and don't want to configure them from scratch all the time, given the verbosity of their creation.

What I have:

I currently created separate classes that delegate to the actual SearchCriteriaBuilder.

This is for example a builder for a "all varchar attributes, sorted by label" search criteria with an optional blacklist:

namespace IntegerNet\Solr\Model\SearchCriteria;

use Magento\Eav\Model\Entity\Collection\AbstractCollection;
use Magento\Framework\Api\Filter;
use Magento\Framework\Api\Search\SearchCriteriaBuilder;
use Magento\Framework\Api\SimpleBuilderInterface;

class VarcharAttributes implements SimpleBuilderInterface
{

    protected $searchCriteriaBuilder;

    public function __construct(SearchCriteriaBuilder $searchCriteriaBuilder)
    {
        $this->searchCriteriaBuilder = $searchCriteriaBuilder;
        $searchCriteriaBuilder->addSortOrder('frontend_label', AbstractCollection::SORT_ORDER_ASC);
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'backend_type',
            Filter::KEY_CONDITION_TYPE => 'in',
            Filter::KEY_VALUE => ['static', 'varchar']
        ]));
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'frontend_input',
            Filter::KEY_VALUE => 'text'
        ]));
    }
    /**
     * Exclude attributes by code
     *
     * @param array $attributeCodes
     * @return $this
     */
    public function except(array $attributeCodes)
    {
        $searchCriteriaBuilder = $this->searchCriteriaBuilder;
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'attribute_code',
            Filter::KEY_CONDITION_TYPE => 'nin',
            Filter::KEY_VALUE => $attributeCodes
        ]));
        return $this;
    }
    public function create()
    {
        return $this->searchCriteriaBuilder->create();
    }
    public function getData()
    {
        return $this->searchCriteriaBuilder->getData();
    }
}

In the constructor, the actual builder gets preconfigured and in except() this configuration can be extended. Now I can inject this class when I need this particular search criteria and use them like this:

$this->attributeRepository->getList(
    $this->varcharAttributes->except(['name'])->create()
);

But I'm not happy with it

Why?

The builder is not immutable. As soon as I make changes to its configuration (the except() call in the example), the default configuration is lost.

If I have these subsequent calls in one class:

$this->varcharAttributes->create()

$this->varcharAttributes->except(['name'])->create()

$this->varcharAttributes->create()

The second and third call create a SearchCriteria with the additional "name" filter.

Usually I would solve this by returning a new instance from the modifiers like except() but I did not find a good way to do that yet. clone $this does not help because the SearchCriteriaBuilder of Magento does not allow deep cloning and references a FilterGroupBuilder and a FilterBuilder.

Of course I could create a factory for each of my custom builders but that feels a bit too much, even if it's one factory for several related builders.

So my question is: Is there a clean way to use preconfigured and preferably immutable builders? Or do I really need to create a factory that creates custom builders that use the core builder?

4

I love the ideas you present. Immutability doesn't go well with builders, having state is part of their job. So how about this, which is very close to what you suggest anyway, if I understand you correctly:

  1. Inject a generated SearchCriteriaBuilderFactory instead of the SearchCriteriaBuilder.
  2. Store the except() method arguments in an $except array property.
  3. In the create() method, get a new SearchCriteriaBuilder instance from the factory, configure it with the default values, apply the exceptions, and then reset $except before you return the SearchCriteria instance.

This way each call to create() will use a fresh builder instance and return a fresh SearchCriteria instance just as you want.
And you don't have to create any factories manually. The one factory you need can be generated and that can even be shared between your custom builders.

<?php

namespace IntegerNet\Solr\Model\SearchCriteria;

use Magento\Eav\Model\Entity\Collection\AbstractCollection;
use Magento\Framework\Api\Filter;
use Magento\Framework\Api\Search\SearchCriteriaBuilderFactory;
use Magento\Framework\Api\SimpleBuilderInterface;

class VarcharAttributes implements SimpleBuilderInterface
{
    private $searchCriteriaBuilderFactory;
    private $except = [];

    public function __construct(SearchCriteriaBuilderFactory $searchCriteriaBuilderFactory)
    {
        $this->searchCriteriaBuilderFactory = $searchCriteriaBuilderFactory;
    }

    private function createVarcharSearchCriteriaBuilder()
    {
        $searchCriteriaBuilder = $this->searchCriteriaBuilderFactory->create();
        $this->applyDefaultFilters($searchCriteriaBuilder);
        return $searchCriteriaBuilder;
    }

    private function applyDefaultFilters(SearchCriteriaBuilder $searchCriteriaBuilder)
    {
        $searchCriteriaBuilder->addSortOrder('frontend_label', AbstractCollection::SORT_ORDER_ASC);
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'backend_type',
            Filter::KEY_CONDITION_TYPE => 'in',
            Filter::KEY_VALUE => ['static', 'varchar']
        ]));
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'frontend_input',
            Filter::KEY_VALUE => 'text'
        ]));
    }

    public function except(array $attributeCodes)
    {
        $self = clone $this;
        $self->except = array_merge($this->except, $attributeCodes);
        return $self;
    }

    public function create()
    {
        $searchCriteriaBuilder = $this->createVarcharSearchCriteriaBuilder();
        $this->applyExceptions($searchCriteriaBuilder);
        return $searchCriteriaBuilder->create();
    }

    private function applyExceptions(SearchCriteriaBuilder $searchCriteriaBuilder)
    {
        $searchCriteriaBuilder->addFilter(new Filter([
            Filter::KEY_FIELD => 'attribute_code',
            Filter::KEY_CONDITION_TYPE => 'nin',
            Filter::KEY_VALUE => $this->except
        ]));
    }
  • Sounds good! I still have to get used to these generated classes :) – Fabian Schmengler Mar 24 '16 at 16:23
  • Added some untested example code based on what you wrote in the question. – Vinai Mar 24 '16 at 16:29
  • 1
    Couple questions: Why use builder in factory? Why not just instantiate object with arguments? And why do you need method except()? Why not just accept $exept as argument of create()? – Anton Kril Mar 24 '16 at 16:56
  • 1
    @Vinai Your example code still has the same problem, that this class itself carries state, but with the SearchCriteriaBuilderFactory it should be easy to refactor it into an API like $varcharAttributes->all()->except(['name'])->create() where all() returns a clone of the object. I'll think about exact naming and implementation, but this is the direction. Thanks again for the idea. – Fabian Schmengler Mar 24 '16 at 19:36
  • 1
    Right, now you don't need a deep clone anymore. – Vinai Mar 24 '16 at 20:42
2

I came up with the following solution:

<?php
namespace IntegerNet\Solr\Model\SearchCriteria;

use Magento\Catalog\Api\Data\EavAttributeInterface;
use Magento\Eav\Model\Entity\Collection\AbstractCollection;
use Magento\Framework\Api\Filter;
use Magento\Framework\Api\Search\SearchCriteriaBuilder;
use Magento\Framework\Api\Search\SearchCriteriaBuilderFactory;
use Magento\Framework\Api\SimpleBuilderInterface;

class VarcharAttributes implements SimpleBuilderInterface
{
    /**
     * @var SearchCriteriaBuilderFactory
     */
    private $searchCriteriaBuilderFactory;

    /** @var callable[] */
    private $buildCallbacks = [];

    /**
     * @param SearchCriteriaBuilderFactory $searchCriteriaBuilderFactory
     */
    public function __construct(SearchCriteriaBuilderFactory $searchCriteriaBuilderFactory)
    {
        $this->searchCriteriaBuilderFactory = $searchCriteriaBuilderFactory;
        $this->buildCallbacks[] = function(SearchCriteriaBuilder $builder) {
            $builder->addFilter(new Filter([
                Filter::KEY_FIELD => EavAttributeInterface::BACKEND_TYPE,
                Filter::KEY_CONDITION_TYPE => 'in',
                Filter::KEY_VALUE => ['static', 'varchar']
            ]));
            $builder->addFilter(new Filter([
                Filter::KEY_FIELD => EavAttributeInterface::FRONTEND_INPUT,
                Filter::KEY_VALUE => 'text'
            ]));
        };
    }

    public function sortedByLabel()
    {
        $new = clone $this;
        $new->buildCallbacks[] = function(SearchCriteriaBuilder $builder) {
            $builder->addSortOrder(EavAttributeInterface::FRONTEND_LABEL, AbstractCollection::SORT_ORDER_ASC);
        };
        return $new;
    }

    /**
     * Exclude attributes by code
     *
     * @param array $attributeCodes
     * @return $this
     */
    public function except(array $attributeCodes)
    {
        $new = clone $this;
        $new->buildCallbacks[] = function(SearchCriteriaBuilder $searchCriteriaBuilder) use ($attributeCodes) {
            $searchCriteriaBuilder->addFilter(new Filter([
                Filter::KEY_FIELD => EavAttributeInterface::ATTRIBUTE_CODE,
                Filter::KEY_CONDITION_TYPE => 'nin',
                Filter::KEY_VALUE => $attributeCodes
            ]));
        };
        return $new;
    }

    public function create()
    {
        return $this->getBuilder()->create();
    }

    public function getData()
    {
        return $this->getBuilder()->getData();
    }

    /**
     * @return SearchCriteriaBuilder
     */
    private function getBuilder()
    {
        $searchCriteriaBuilder = $this->searchCriteriaBuilderFactory->create();
        foreach ($this->buildCallbacks as $buildCallback) {
            $buildCallback($searchCriteriaBuilder);
        }
        return $searchCriteriaBuilder;
    }

}

Explanation:

I use the auto generated SearchCriteriaBuilderFactory to be able to instantiate a new builder, as suggested by @Vinai

Then I defer all manipulations to the builder to the create method, using callbacks. My own builder class contains an array of build callbacks and methods that change it, clone my builder so that the original instance is untouched. A callback for the default filters is added in the constructor.

Usage:

The class can be injected via constructor and the Magento DI system anywhere, then used multiple times without side effects:

  • $repository->getList($this->varcharAttributes->sortedByLabel()->create());
    
  • $repository->getList($this->varcharAttributes->except(['name'])->sortedByLabel()->create());
    
  • $repository->getList($this->varcharAttributes->create());
    
  • Maybe you can make the example code under Usage clearer so that it shows that all three are different usecases, e.g. $repository->getList($this->varcharAttributes->allSortedByLabel()->create()); ... At first I missread it to be one usecase and that was confusing. – Vinai May 24 '16 at 12:49
  • 1
    @Vinai good idea, thanks. I updated it and also changed the class a bit to make the chaining more clear. Now the default filter (varchar) is always added and there are two modifications, sortedByLabel() and except() – Fabian Schmengler May 24 '16 at 12:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.