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

Check single entity satisfiability #261

Closed
vshmakov opened this issue Dec 6, 2020 · 8 comments · Fixed by #273
Closed

Check single entity satisfiability #261

vshmakov opened this issue Dec 6, 2020 · 8 comments · Fixed by #273
Assignees
Milestone

Comments

@vshmakov
Copy link

vshmakov commented Dec 6, 2020

Hello! Have your library functionality for checking single entity satisfiability?

$specification->isSatisfiedBy($entity);

It lets use one definition for generation sql and checking single object in code at the same time. Do you provide such aportunities? Are you planing to realize it in future?

@peter-gribanov
Copy link
Member

peter-gribanov commented Dec 7, 2020

Interest question. No. This project does not support transforming specifications to check specific entities. This project is designed to work with Doctrine. That is, the specifications are transformed into DQL and they are optimized for this.

Checking specific entities requires a completely different approach to organizing and describing specifications. It is rather difficult to translate DCL into rules that can be applied to a specific object.

A couple of years ago, in my project, i implemented a mechanism for applying specifications to specific objects by adding a special interface for specifications. As a result, we duplicate business logic in different forms in two methods of the same class. You can use this approach right now. I'll think about it and maybe add something similar to this project.

interface Satisfiable
{
    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool;
}

// example spec
final class ContestRequireModeration extends BaseSpecification implements Satisfiable
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::eq('join_options.moderation', true);
    }

    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool
    {
        if (!$candidate instanceof Contest) {
            throw new \InvalidArgumentException(
                sprintf('Expects "%s", got "%d" instead.', Contest::class, gettype($candidate))
            );
        }

        return $candidate->joinOptions()->isModerationRequired();
    }
}

// example composition
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))
        );
    }

    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool
    {
        if (!$candidate instanceof Contestant) {
            throw new \InvalidArgumentException(
                sprintf('Expects "%s", got "%d" instead.', Contestant::class, gettype($candidate))
            );
        }

        return
            $candidate->permission()->isApproved() ||
            !(new ContestRequireModeration())->isSatisfiedBy($candidate->contest())
        ;
    }
}

@vshmakov
Copy link
Author

vshmakov commented Dec 7, 2020

Thank you for the answer! Yes, it will work, but this solution keeps code duplication. I suppose, for example, generating expression by specs may solve this problem. But I am not sure it will work in all cases.
There is a project wich allows using specs both for generating dql and checking single entities. But, unfortunately, it's not supported anymore.

@peter-gribanov
Copy link
Member

Yes, it will work, but this solution keeps code duplication.

It may be possible to avoid duplication.

I suppose, for example, generating expression by specs may solve this problem. But I am not sure it will work in all cases.
There is a project wich allows using specs both for generating dql and checking single entities.

Yes. Thanks. I am familiar with the expressions from Symfony and the RulerZ project (our direct competitors). RulerZ is a really interesting project, but it has several problems:

  • There are problems in the work of relations (join)
  • The rules are designed to be described in one line. Poorly compatible with the composition. That is, it is impossible to describe a rule that will be equally applicable for an entity and for a relations.
  • The project is limited to filter expressions only. The Happyr Doctrine Specification provides many more functions for managing queries.

Maybe i don't know something. I haven't watched the RulerZ project for a long time.

As the name of the project suggests, the main problem of the Happyr Doctrine Specification is the rigid dependence on Doctrine. Work was under way to solve this problem (#82, #150), but it makes no sense, since Happyr Doctrine Specification without Doctrine is a completely different project.

@vshmakov
Copy link
Author

vshmakov commented Dec 8, 2020

Could you explain how to avoid duplication?

// example spec
final class ContestRequireModeration extends BaseSpecification implements Satisfiable
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::eq('join_options.moderation', true);
    }

    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool
    {
        if (!$candidate instanceof Contest) {
            throw new \InvalidArgumentException(
                sprintf('Expects "%s", got "%d" instead.', Contest::class, gettype($candidate))
            );
        }

        return $candidate->joinOptions()->isModerationRequired();
    }
}

I see double defining spec logic here:

  1. For generating dql
    return Spec::eq('join_options.moderation', true);

  2. And for checking in code.
    return $candidate->joinOptions()->isModerationRequired();

@peter-gribanov
Copy link
Member

If we implement support for isSatisfiedBy() at the level of this project, then expressions will work the same as direct calls.

Spec::eq('join_options.moderation_required', true)->isSatisfiedBy($candidate);
// equivalent to
$candidate->getJoinOptions()->isModerationRequired();

That is, the specification can look like:

final class ContestRequireModeration extends BaseSpecification implements Satisfiable
{
    /**
     * @return Filter|QueryModifier
     */
    protected function getSpec()
    {
        return Spec::eq('join_options.moderation_required', true);
    }

    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool
    {
        // this code will be in BaseSpecification
        return $this->getSpec()->isSatisfiedBy($candidate);
    }
}

However, no one can automatically guess that the value of property moderation is available through method isModerationRequired(). It is necessary to bring the names of all access methods to a simple form that the Symfony PropertyAccess Component will understand.

And if we want to preserve the reusability of the specifications, we will have to manually manage the related entities. In the following example, specification ContestRequireModeration does not know that it is being used in context Contestant and not Contest.

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))
        );
    }

    /**
     * @param mixed $candidate
     */
    public function isSatisfiedBy($candidate): bool
    {
        return
            Spec::eq('permission', Permission::approved()->value())->isSatisfiedBy($candidate) ||
            Spec::not(new ContestRequireModeration($this->contest_alias))->isSatisfiedBy($candidate->contest())
        ;
    }
}

In RulerZ terms, next rules will be incompatible due to the difference in contexts.

$rule = 'join_options.moderation_required = true';
$rule = 'contest.join_options.moderation_required = true';
$rule = 'contestant.contest.join_options.moderation_required = true';

I do not yet see an easy way to pass the context of invoking the specifications. The DQL alias is linear and does not contain the full path to the related entity.

@vshmakov
Copy link
Author

vshmakov commented Dec 9, 2020

Will using understandable by property accessor paths as dql aliases solve the problem?

@peter-gribanov
Copy link
Member

Probably yes. I am currently testing this option.

@peter-gribanov
Copy link
Member

Fixed in #273 and released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants