Check single entity satisfiability

[peter-gribanov]

The main idea of the feature #261 is to use existing specifications to check the conformity of specific entities. The idea is to add method isSatisfiedBy() to check a specific entity and to add method filterCollection() to remove entities from the list that do not meet the specification.

Usage

Check single entity

$highRankFemalesSpec = Spec::andX(
    Spec::eq('gender', 'F'),
    Spec::gt('points', 9000),
);

// an array of arrays
$playersArr = [
    ['pseudo' => 'Joe',   'gender' => 'M', 'points' => 2500],
    ['pseudo' => 'Moe',   'gender' => 'M', 'points' => 1230],
    ['pseudo' => 'Alice', 'gender' => 'F', 'points' => 9001],
];

// or an array of objects
$playersObj = [
    new Player('Joe',   'M', 40, 2500),
    new Player('Moe',   'M', 55, 1230),
    new Player('Alice', 'F', 27, 9001),
];

foreach ($playersArr as $playerArr) {
    if ($highRankFemalesSpec->isSatisfiedBy($playerArr)) {
        // do something
    }
}

foreach ($playersObj as $playerObj) {
    if ($highRankFemalesSpec->isSatisfiedBy($playerObj)) {
        // do something
    }
}

Filter collection

$highRankFemalesSpec = Spec::andX(
    Spec::eq('gender', 'F'),
    Spec::gt('points', 9000),
);

// an array of arrays
$playersArr = [
    ['pseudo' => 'Joe',   'gender' => 'M', 'points' => 2500],
    ['pseudo' => 'Moe',   'gender' => 'M', 'points' => 1230],
    ['pseudo' => 'Alice', 'gender' => 'F', 'points' => 9001],
];

// or an array of objects
$playersObj = [
    new Player('Joe',   'M', 40, 2500),
    new Player('Moe',   'M', 55, 1230),
    new Player('Alice', 'F', 27, 9001),
];

$highRankFemales = $highRankFemalesSpec->filterCollection($playersArr);
$highRankFemales = $highRankFemalesSpec->filterCollection($playersObj);
$highRankFemales = $this->em->getRepository(Player::class)->match($highRankFemalesSpec);

Problems

Paths to field and DQL aliases

Unable to access fields of related entities by DQL alias. We can use the name of the field through which the related entity is available as DQL alias. We cannot know what the name of the field with the entity is, we just declare a convention that the DQL alias should be the name of the field (contest, contestant, user), not an arbitrary set of characters (c, ct, u).
However, this is not sufficient to access a field in a related entity of a related entity. For these cases, we will have to pass the full path to the field (contestant.contest.enabled), however, Doctrine does not allow using aliases like contestant.contest. We can use valid characters as a path separator, but this paths (contestantXcontest.enabled) seems counterintuitive to me since aliases are not only an internal element, they are also used by users in specifications such as SelectEntity. I prefer a uniform method of describing the path. We can use the full path in the specifications, and the last part of the path as DQL alias, that is, the name of the field containing the related entity.

Using real fields as DQL alias can lead to conflicts. We can resolve conflicts automatically, but we have to do this every time we access the fields, even if there are no conflicts. The QueryBuilder contains a list of aliases with no alias information. The simplest thing is to focus on this list. To get information about the available aliases, we need to recursively read the list of joins, which will negatively affect performance.

A small bonus of using field names for DQL aliases is the ability to automatically make joins.

Operands

Operands Alias and CountDistinct are not applicable in specifications applied to specific entities. Attempting to use them will result in an error.

Platform functions

Since our specifications support DQL functions, we also need to add support for them when validating specific entities. Most functions are easy to make executable. There are functions such as TRIM() that are difficult to make executable due to the peculiarities of the syntax #275. The behavior of some functions is ambiguous. And there are functions AVG(), COUNT(), IDENTITY(), MAX(), MIN() and SUM() are not applicable in specifications applied to specific entities. Attempting to use them will result in an error.

You can add your own executable functions or override existing ones.

PlatformFunction::getExecutorRegistry()->register('custom_fun', function ($argument) {
    // do something
});

Functions CURENT_DATE(), CURENT_TIME() and CURENT_TIMESTAMP() have ambiguous behavior. They return object DateTimeImmutable. Function DATE_DIFF() return object DateInterval. This can cause problems in expressions.

Filters

The MemberOfX filter are not applicable in specifications applied to specific entities.

Property access

For simplicity, i'm using a Symfony PropertyAccess Component. It understands a paths like contestant.contest.enabled and [contestant][contest][enabled]. However, accessing the fields in the list requires a special syntax that is not applicable in our specifications ([groups][0][enabled]). Also, due to the difference in syntax, you cannot combine data types. That is, you cannot describe a multidimensional array that contains objects, just as you cannot use associative arrays in objects (contestant[contest].enabled).

Rewrite specifications

For the new version, you will need to rewrite all the specifications in your project.

Before:

Spec::andX(
    Spec::innerJoin('contestant', 'ct'),
    Spec::innerJoin('contest', 'c', 'ct'),
    Spec::innerJoin('user', 'u', 'ct'),
    Spec::eq('state', State::active()->value(), 'u'),
    Spec::eq('enabled', true, 'c'),
);

After:

Spec::andX(
    Spec::eq('state', State::active()->value(), 'contestant.user'),
    Spec::eq('enabled', true, 'contestant.contest'),
);

Before:

final class PublishedQuestionnaires extends BaseSpecification
{
    private string $contest_alias;

    private string $contestant_alias;

    private string $user_alias;

    public function __construct(
        string $contest_alias = 'c',
        string $contestant_alias = 'ct',
        string $user_alias = 'u',
        ?string $dql_alias = null,
    ) {
        $this->contest_alias = $contest_alias;
        $this->contestant_alias = $contestant_alias;
        $this->user_alias = $user_alias;
        parent::__construct($dql_alias);
    }

    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::andX(
            Spec::innerJoin('contestant', $this->contestant_alias),
            new ContestantPublished($this->contest_alias, $this->user_alias, $this->contestant_alias),
        );
    }
}

final class ContestantPublished extends BaseSpecification
{
    private string $contest_alias;

    private string $user_alias;

    public function __construct(string $contest_alias = 'c', string $user_alias = 'u', ?string $dql_alias = null)
    {
        $this->contest_alias = $contest_alias;
        $this->user_alias = $user_alias;
        parent::__construct($dql_alias);
    }

    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::andX(
            new JoinedContestant($this->contest_alias, $this->user_alias),
            new ContestantApproved($this->contest_alias),
        );
    }
}

final class ContestantApproved extends BaseSpecification implements Satisfiable
{
    private string $contest_alias;

    public function __construct(string $contest_alias = 'c', ?string $dql_alias = null)
    {
        $this->contest_alias = $contest_alias;
        parent::__construct($dql_alias);
    }

    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::orX(
            Spec::eq('permission', Permission::approved()->value()),
            Spec::not(new ContestRequireModeration($this->contest_alias)),
        );
    }
}

After:

final class PublishedQuestionnaires extends BaseSpecification
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return new ContestantPublished('contestant');
    }
}

final class ContestantPublished extends BaseSpecification
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::andX(
            new JoinedContestant(),
            new ContestantApproved(),
        );
    }
}

final class ContestantApproved extends BaseSpecification implements Satisfiable
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::orX(
            Spec::eq('permission', Permission::approved()->value()),
            Spec::not(new ContestRequireModeration('contest')),
        );
    }
}

[Nyholm]

Interesting.

Sorry I failed to review this. I did a quick review now and I really like it =)

[vshmakov]

However, this is not sufficient to access a field in a related entity of a related entity.

I am sorry. I saw the request, when it is already merged... But couldn't my solution solve the issue? It supports nasted joins.
https://github.com/vshmakov/specifications/blob/4375449/src/Specification/Task/IsViewable.php
https://github.com/vshmakov/specifications/blob/4375449/src/Specification/Join.php

Thank you for great job!

[peter-gribanov]

@vshmakov sorry, but i do not see how you solve the described problem.

The problem is that we can't use path for PropertyAccess and DQL alias in the same way (contestant.contest.enabled). Attempting to declare contestant.contest DQL alias will result in error.

We solve this problem by parsing the path into its component parts and automatically creating the necessary joins #273 (review) .

[vshmakov]

My solution is to pass specification to join.

class Task
{
    private Project $project;
//...
}

class Project
{
    private User $user;
//...
}

class  User
{
    private bool $isEnabled = true;
//...
}

class  IsTaskOfEnabledUser extends CompositeSpecification
{
    public function getSpecification(): Specification
    {
        return new Join(
            'project',
            new Join(
                'user',
                new Equals('enabled', true)
            )
        );
    }
}

And for your example.

After:
Spec::andX(
    Spec::eq('contestant.user.state', State::active()->value()),
    Spec::eq('contestant.contest.enabled', true)
);

Nasted join:
Spec::join(
    'contestant',
    Spec::andX(
        Spec::join(
            'user',
            Spec::eq('state', State::active()->value())
        ),
        Spec::join(
            'contest',
            Spec::eq('enabled', true)
        )
    )
);

[peter-gribanov]

The use of conditions in joins has already been discussed earlier. This is not a very working solution.

I have already described a similar problem earlier. The problem is that specifications are described as a composition, and conditions for related entities can be used in different specifications at different nesting levels. If we use explicit joining of related entities in the specifications, then we get a conflict if the same related entity is used in multiple places.

[vshmakov]

My solution doesn't use join conditions. It just allows to join related entity as usual and apply passed specification to it.

What about alias conflicts... For example, we may pass unique join alias as additional parameter: Spec::join($field, $specification, $alias);

Or we can generate join alias automatically, like in Api Platform: user -> user_j1, contest -> contest_j2.

The whole solution may look like this:

$specification = Spec::andX(
    Spec::join(
        'user',
        Spec::eq('state', State::active()->value())
    ),
    Spec::join(
        'contest',
        Spec::eq('enabled', true)
    )
);
$contestantRepository->match($specification);

DQL:
select contestant from Contestant contestant
	join contestant.user user_j1
	join contestant.contest contest_j2
	where user_j1.state = :state_p1
		and contest_j2.enabled = :enabled_p2

[peter-gribanov]

My solution doesn't use join conditions. It just allows to join related entity as usual and apply passed specification to it.

As far as i understand your code, it is not. Please explain to me what happens if you execute such a code?

$specification = Spec::andX(
    Spec::join(
        'user',
        Spec::eq('state', State::active()->value()),
    ),
    Spec::join(
        'user', // same field in different join
        Spec::eq('id', $id),
    ),
);
$contestantRepository->match($specification);

What about alias conflicts... For example, we may pass unique join alias as additional parameter: Spec::join($field, $specification, $alias);

This is exactly the approach that has always been used in this project.

The whole solution may look like this

I am guessing that we can get the same result using different syntax.

- new ContestantPublished('contestant')
+ Spec::join('contestant', new ContestantPublished())

Your example:

$specification = Spec::join(
    'contestant',
    Spec::andX(
        Spec::join(
            'user',
            Spec::eq('state', State::active()->value()),
        ),
        Spec::join(
            'contest',
            Spec::eq('enabled', true),
        ),
    ),
);

Specifications allocated into separate objects:

final class PublishedQuestionnaires extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::join('contestant', new ContestantPublished());
    }
}

final class ContestantPublished extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            new JoinedContestant(),
            // new ContestantApproved(),
        );
    }
}

final class JoinedContestant extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            Spec::join('user', new UserActivated()),
            Spec::join('contest', new ContestPublished()),
        );
    }
}

final class UserActivated extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::eq('state', State::active()->value());
    }
}

final class ContestPublished extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::eq('enabled', true);
    }
}

The syntax used in this project:

$specification = Spec::andX(
    Spec::eq('state', State::active()->value(), 'contestant.user'),
    Spec::eq('enabled', true, 'contestant.contest'),
);

Specifications allocated into separate objects:

final class PublishedQuestionnaires extends BaseSpecification
{
    protected function getSpec()
    {
        return new ContestantPublished('contestant');
    }
}

final class ContestantPublished extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            new JoinedContestant(),
            // new ContestantApproved(),
        );
    }
}

final class JoinedContestant extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            new UserActivated('user'),
            new ContestPublished('contest'),
        );
    }
}

final class UserActivated extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::eq('state', State::active()->value());
    }
}

final class ContestPublished extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::eq('enabled', true);
    }
}

[vshmakov]

$specification = Spec::andX(
    Spec::join(
        'user',
        Spec::eq('state', State::active()->value()),
    ),
    Spec::join(
        'user', // same field in different join
        Spec::eq('id', $id),
    ),
);

It will tries to generate such DQL:

select contestant from Contestant contestant
	join contestant.user user
	join contestant.user user
	where user.state = :state
		and user.id = :id

It has alias conflict. But with different aliases dql will be still bad. That's why we should rewrite the rule for this example:

$specification = Spec::join(
    'user',
    Spec::andX(
        Spec::eq('state', State::active()->value()),
        Spec::eq('id', $id)
    )
);

DQL:
select contestant from Contestant contestant
	join contestant.user user
	where user.state = :state
		and user.id = :id

In next steps we can define IsActive, HasId and JoinUser specifications separately.

new JoinUser(
    Spec::andX(
        new IsActive(),
        new HasId($id)
    )
);

[peter-gribanov]

It has alias conflict. But with different aliases dql will be still bad. That's why we should rewrite the rule for this example:

This is exactly the problem i was talking about. We may not be able to rewrite specifications since we describe specifications as compositions. Specifications can be declared at different nesting levels.

Composition of specifications:

final class PublishedQuestionnaires extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::join('contestant', new ContestantPublished());
    }
}

final class ContestantPublished extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            new JoinedContestant(),
            // new ContestantApproved(),
        );
    }
}

final class JoinedContestant extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::andX(
            Spec::join('user', new UserActivated()),
            // Spec::join('contest', new ContestPublished()),
        );
    }
}

final class UserActivated extends BaseSpecification
{
    protected function getSpec()
    {
        return Spec::eq('state', State::active()->value());
    }
}

final class InIdentity extends BaseSpecification
{
    private array $value;

    public function __construct($value)
    {
        $this->value = (array) $value;
    }

    protected function getSpec()
    {
        return Spec::in('id', $this->value);
    }
}

Using:

// $specification = Spec::andX(
//     new PublishedQuestionnaires(),
//     new InIdentity($id, 'contestant.user'),
// );
$specification = Spec::andX(
    new PublishedQuestionnaires(),
    Spec::join(
        'contestant',
        Spec::join(
            'user',
            new InIdentity($id),
        ),
    ),
);
$questionnaireRepository->match($specification);

From this example you can see that the rules for the user are defined at different nesting levels (PublishedQuestionnaires -> ContestantPublished -> JoinedContestant -> UserActivated) and we cannot reuse the previously declared join. To use the previously declared join, we must explicitly describe all the necessary rules at each place of their use. As a result, we get code duplication, the inability to reuse the code, and code maintenance problems.

Examples of reusing specifications:

// $specification = Spec::andX(
//     new PublishedQuestionnaires(),
//     Spec::orderBy('vote_total', 'DESC', 'contestant'),
// );
$specification = Spec::andX(
    new PublishedQuestionnaires(),
    Spec::join(
        'contestant',
        Spec::orderBy('vote_total', 'DESC'),
    ),
);
$questionnaireRepository->match($specification);

// $specification = Spec::andX(
//     new PublishedQuestionnaires(),
//     Spec::orderBy('join_at', 'DESC', 'contestant'),
// );
$specification = Spec::andX(
    new PublishedQuestionnaires(),
    Spec::join(
        'contestant',
        Spec::orderBy('join_at', 'DESC'),
    ),
);
$questionnaireRepository->match($specification);

// $specification = Spec::andX(
//     new PublishedQuestionnaires(),
//     Spec::eq('winner', true, 'contestant'),
// );
$specification = Spec::andX(
    new PublishedQuestionnaires(),
    Spec::join(
        'contestant',
        Spec::eq('winner', true),
    ),
);
$questionnaireRepository->match($specification);

If we follow your approach, then we should duplicate the rules from the PublishedQuestionnaires specification in each of these places.

[peter-gribanov]

@vshmakov I don't mean to say that your idea is bad. I also thought about this approach before. If you add to the Join specification the tracking and reuse of declared joins like in DQLContextResolver, then the problem with duplicate joins will be solved.

As i said earlier, we will get the same result using a different syntax.

$specification = Spec::andX(
    Spec::andX(
        Spec::join(
            'contestant',
            Spec::andX(
                Spec::join(
                    'user',
                    Spec::eq('state', State::active()->value()),
                ),
                Spec::join(
                    'contest',
                    Spec::eq('enabled', true),
                ),
            ),
        ),
    ),
    Spec::join(
        'contestant',
        Spec::eq('winner', true),
    ),
);

vs

$specification = Spec::andX(
    Spec::andX(
        Spec::eq('state', State::active()->value(), 'contestant.user'),
        Spec::eq('enabled', true, 'contestant.contest),
    ),
    Spec::eq('winner', true, 'contestant'),
);

In my opinion, Spec::join() is associated with JOIN SQL operation. But the specifications are not about the persistence level, but about the business logic level. Therefore, i consider declaring the context of the specification to be a more general solution.

[vshmakov]

Sorry for the delay!

I thought about your example. May be I would prefer to keep join logic in one place. It's more readable in my opinion. But it doesn't allow to compose nasted specifications by standard way. And I couldn't avoid code duplication in that situation.

Special join specification is more powerful than simple string of property accessor path format. But I don't see, what it can give in practice.

Does your solution cover all "satisfied by" use cases?

[peter-gribanov]

Special join specification is more powerful than simple string of property accessor path format. But I don't see, what it can give in practice.

The advantage of using paths is that you can compare values from different contexts.

// DQL: root.product.price < root.archive.price
$spec = Spec::lt(
    Spec::field('price', 'product'),
    Spec::field('price', 'archive'),
);

Does your solution cover all "satisfied by" use cases?

We have already implemented the new version of the specifications on our project. The functionality suits us and covers all our use cases.

[vshmakov]

The advantage of using paths is that you can compare values from different contexts.

It is just different syntax. I think, you can make join specification work like property accessor path.

[peter-gribanov]

It is just different syntax. I think, you can make join specification work like property accessor path.

Perhaps you are right. Although i still don't find this syntax more logical and convenient.

$spec = Spec::lt(
    Spec::join('product', Spec::field('price')),
    Spec::join('archive', Spec::field('price')),
);

[vshmakov]

Finally I agree with you. :) It's more complex and long. And it doesn't make a sense.