Giter Site home page Giter Site logo

happyr / doctrine-specification Goto Github PK

View Code? Open in Web Editor NEW
445.0 22.0 38.0 788 KB

This library gives you a new way for writing queries. Using the Specification pattern you will get small Specification classes that are highly reusable.

License: MIT License

PHP 100.00%

doctrine-specification's Introduction

Happyr Doctrine Specification

Build Status Travis (.org) Latest Stable Version Packagist Monthly Downloads Packagist Total Downloads Packagist Quality Score

This library gives you a new way for writing queries. Using the Specification pattern you will get small Specification classes that are highly reusable.

The problem with writing Doctrine queries is that it soon will be messy. When your application grows you will have 20+ function in your Doctrine repositories. All with long and complicated QueryBuilder calls. You will also find that you are using a lot of parameters to the same method to accommodate different use cases.

After a discussion with Kacper Gunia on Sound of Symfony podcast about how to test your Doctrine repositories properly, we (Kacper and Tobias) decided to create this library. We have been inspired by Benjamin Eberlei's thoughts in his blog post.

Table of contents

  1. Motivation and basic understanding (this page)
  2. Usage examples
  3. Create your own spec
  4. Contributing to the library

Why do we need this lib?

You are probably wondering why we created this library. Your entity repositories are working just fine as they are, right?

But if your friend open one of your repository classes he/she would probably find that the code is not as perfect as you thought. Entity repositories have a tendency to get messy. Problems may include:

  • Too many functions (findActiveUser, findActiveUserWithPicture, findUserToEmail, etc)
  • Some functions have too many arguments
  • Code duplication
  • Difficult to test

Requirements of the solution

The solution should have the following features:

  • Easy to test
  • Easy to extend, store and run
  • Re-usable code
  • Single responsibility principle
  • Hides the implementation details of the ORM. (This might seen like nitpicking, however it leads to bloated client code doing the query builder work over and over again.)

The practical differences

This is an example of how you use the lib. Say that you want to fetch some Adverts and close them. We should select all Adverts that have their endDate in the past. If endDate is null make it 4 weeks after the startDate.

// Not using the lib
$qb = $this->em->getRepository('HappyrRecruitmentBundle:Advert')
    ->createQueryBuilder('r');

return $qb->where('r.ended = 0')
    ->andWhere(
        $qb->expr()->orX(
            'r.endDate < :now',
            $qb->expr()->andX(
                'r.endDate IS NULL',
                'r.startDate < :timeLimit'
            )
        )
    )
    ->setParameter('now', new \DateTime())
    ->setParameter('timeLimit', new \DateTime('-4weeks'))
    ->getQuery()
    ->getResult();
// Using the lib
$spec = Spec::andX(
    Spec::eq('ended', 0),
    Spec::orX(
        Spec::lt('endDate', new \DateTime()),
        Spec::andX(
            Spec::isNull('endDate'),
            Spec::lt('startDate', new \DateTime('-4weeks'))
        )
    )
);

return $this->em->getRepository('HappyrRecruitmentBundle:Advert')->match($spec);

Yes, it looks pretty much the same. But the later is reusable. Say you want another query to fetch Adverts that we should close but only for a specific company.

Doctrine Specification

class AdvertsWeShouldClose extends BaseSpecification
{
    public function getSpec()
    {
        return Spec::andX(
            Spec::eq('ended', 0),
            Spec::orX(
                Spec::lt('endDate', new \DateTime()),
                Spec::andX(
                    Spec::isNull('endDate'),
                    Spec::lt('startDate', new \DateTime('-4weeks'))
                )
            )
        );
    }
}

class OwnedByCompany extends BaseSpecification
{
    private $companyId;

    public function __construct(Company $company, ?string $context = null)
    {
        parent::__construct($context);
        $this->companyId = $company->getId();
    }

    public function getSpec()
    {
        return Spec::andX(
            Spec::join('company', 'c'),
            Spec::eq('id', $this->companyId, 'c')
        );
    }
}

class SomeService
{
    /**
     * Fetch Adverts that we should close but only for a specific company
     */
    public function myQuery(Company $company)
    {
        $spec = Spec::andX(
            new AdvertsWeShouldClose(),
            new OwnedByCompany($company)
        );

        return $this->em->getRepository('HappyrRecruitmentBundle:Advert')->match($spec);
    }
}

QueryBuilder

If you were about to do the same thing with only the QueryBuilder it would look like this:

class AdvertRepository extends EntityRepository
{
    protected function filterAdvertsWeShouldClose(QueryBuilder $qb)
    {
        $qb
            ->andWhere('r.ended = 0')
            ->andWhere(
                $qb->expr()->orX(
                    'r.endDate < :now',
                    $qb->expr()->andX('r.endDate IS NULL', 'r.startDate < :timeLimit')
                )
            )
            ->setParameter('now', new \DateTime())
            ->setParameter('timeLimit', new \DateTime('-4weeks'))
        ;
    }

    protected function filterOwnedByCompany(QueryBuilder $qb, Company $company)
    {
        $qb
            ->join('company', 'c')
            ->andWhere('c.id = :company_id')
            ->setParameter('company_id', $company->getId())
        ;
    }

    public function myQuery(Company $company)
    {
        $qb = $this->em->getRepository('HappyrRecruitmentBundle:Advert')->createQueryBuilder('r');
        $this->filterAdvertsWeShouldClose($qb);
        $this->filterOwnedByCompany($qb, $company);

        return $qb->getQuery()->getResult();
    }
}

The issues with the QueryBuilder implementation are:

  • You may only use the filters filterOwnedByCompany and filterAdvertsWeShouldClose inside AdvertRepository.
  • You can not build a tree with And/Or/Not. Say that you want every Advert but not those owned by $company. There is no way to reuse filterOwnedByCompany() in that case.
  • Different parts of the QueryBuilder filtering cannot be composed together, because of the way the API is created. Assume we have a filterGroupsForApi() call, there is no way to combine it with another call filterGroupsForPermissions(). Instead reusing this code will lead to a third method filterGroupsForApiAndPermissions().

Check single entity

You can apply specifications to validate specific entities or dataset.

$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

You can apply specifications to filter collection of entities or datasets.

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

Continue reading

You may want to take a look at some usage examples or find out how to create your own spec.

doctrine-specification's People

Contributors

adamquaile avatar ap-hunt avatar cakper avatar cordoval avatar dragosprotung avatar drewclauson avatar edefimov avatar kdederichs avatar kgilden avatar kix avatar nyholm avatar peter-gribanov avatar strontium-90 avatar theviniciusmartins avatar thundo avatar timroberson avatar vudaltsov avatar xwb avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

doctrine-specification's Issues

Doctrine ignores hydration mode set via Doctrine\ORM\AbstractQuery::setHydrationMode()

I am trying to use the AsArray query option, but Doctrine does not seem to honor the hydration mode when it is set using the Doctrine\ORM\AbstractQuery::setHydrationMode() method. If you look at Doctrine\ORM\AbstractQuery::getResults(), it accepts the hydration mode as a parameter and ignores the hydration mode property.

Can a workaround be implemented in this project, or do you consider this a Doctrine bug?

I'm excited about this project, thank you for your work on it.

Add contribution file

The file should contain something about the MIT license. We should also mention what you PR should include when you writing a new spec.

  1. You spec
  2. A PHPSpec test
  3. A factory funciton

API could be extended to be more fluent

Using this library, I've had trouble building specifications in scenarios where the structure of the specification is dynamic, based on some input. Take for example a search feature, in which fields may or may not be present.

public function search($parameters)
{
    if(isset($parameters["foo"]))
    {
        // Add foo to specification
    }

    if(isset($parameters["bar"]))
    {
        // Add bar to specification
    }
}

The current API in both the factory class and the individual specifications takes all values in the constructor, and is immutable. However, this is not conducive to the scenario outlined above.

public function search($parameters)
{
    $spec = S\Spec::andX();
    if(isset($parameters["foo"]))
    {
        $spec = S\Spec::andX(
            $spec 
            S\Spec::eq("foo", $parameters["foo"])
        );
    }

    if(isset($parameters["bar"]))
    {
        $spec = S\Spec::andX(
            $spec 
            S\Spec::eq("bar", $parameters["bar"])
        );
    }
}

Here I suggest a more fluent extension to the API. I'm envisioning something as below.

public function search($parameters)
{
    $spec = S\FluentSpec::create();
    // ... If foo is set
    $spec->andX(S\Spec::eq("foo", $parameters["foo"]));
    // ... If bar ir set
    $spec->andX(S\Spec::eq("bar", $parameters["bar"]));

    $spec->orX(
        S\FluentSpec::create(S\Spec::lt("fizz", 4))->andX(S\Spec::eq("bing", false))
        S\Spec::gt("buzz", 99)
    );

    $results = $repo->match($spec);
}

I've taken the liberty of drafting what I think the internals of the fluent API may look like, and I'd love to hear your feedback on this idea for the future (once some of the more major issues are completed)

namespace Happyr\DoctrineSpecification;

class FluentSpec implements Specification
{
    /**
     * @var Specification
     */
    private $spec;

    private function __construct(Specification $spec)
    {
        $this->spec = $spec;
    }

    /**
     * @param Specification $spec
     * @return FluentSpec
     */
    public static function create(Specification $spec)
    {
        return new FluentSpec($spec);
    }

    public function andX()
    {
        $specifications = array_merge(array($this->spec), func_get_args());
        $this->spec = call_user_func_array(__NAMESPACE__."\Spec::andX", $specifications);
        return $this;
    }

    public function orX()
    {
        $specifications = array_merge(array($this->spec), func_get_args());
        $this->spec = call_user_func_array(__NAMESPACE__."\Spec::orX", $specifications);
        return $this;
    }

    // Specification interface implementations

    /**
     * @param QueryBuilder $qb
     * @param string $dqlAlias
     *
     * @return Expr
     */
    public function match(QueryBuilder $qb, $dqlAlias)
    {
        return $this->spec->match($qb, $dqlAlias);
    }

    /**
     * @param AbstractQuery $query
     */
    public function modifyQuery(AbstractQuery $query)
    {
        return $this->spec->modifyQuery($query);
    }

    /**
     * @param string $className
     *
     * @return bool
     */
    public function supports($className)
    {
        return $this->spec->supports($className);
    }
}

The best way to fetch Array or Object

Maybe it's not about this library but I'll ask. What is the best way to make repository methods to be possible to return both Array and Objects?
It's not a good idea to make two versions of one method (findBy() and findByAsArray()).

The second way I used before is bad too. I used setFetchMode() for the whole repository. It causes the promblems with unknown method result without knowing the context, IDE autocomplete and so on.

May be I just should use return $qb->getQuery() and then calls proper method in service or controller class?

$repository->findBy(/*...*/)->getArrayResult();

How to name such methods? findX() seems inappropriate.

Any other ideas?

Add In specification

return $qb->expr()->in(sprintf('%s.%s', $dqlAlias, $this->field), sprintf(':%s', $paramName));

LogicX() gets multidimensional array for arguments

After 6ea70f5 the chain of calls Spec::andX/orX() โ†’ new AndX/OrX() โ†’ new LogicX() ends with LogicX's arguments being a multidimensional array, which leads to failing $child instanceof Filter/QueryModifier checks. For instance, in the following example orX part is completely ignored:

$specs = new AndX(
    Spec::orX(
        Spec::eq('field1', 42),
        Spec::eq('field2', 42), 
    ),
    Spec::eq('field3', 42)
);

Of cause replacing Spec::orX() with new OrX() fixes the issue. But is it intended behaviour?
P.S. Thanks for the lib, BTW!

Update readme

Show the different ways of creating specs

  • new Comparison(Comparison::GT, 'age', 18)
  • new GreaterThan('age', 18)
  • Spec::gt('age', 18)

Use Query instead of AbstractQuery

The discussion on #39 made me convinced that we should use Query instead of AbstractQuery in the method declaration of modifyQuery.

From the EntitySpecificationRepository:

$query = $qb->where($expr)->getQuery();

//give the Specification a change to modify the query
$specification->modifyQuery($query);

CountOf or having usage?

Hi,

I'm wondering if it is possible to use these functions to filter out entities that do not have any related records, e.g.: Select all galleries that have at least 1 photo added to it.

I expected something like Spec::gt(Spec::countOf('photos'), 0) would work but it doesn't. I also unsuccessfully tried Spec::gt('COUNT(photos)', 0).

Thanks!

Add Doctrine like addGroupBy methods

I'm not sure if it is a common approach to use Specs for adding GROUP BY only.

Before I had a single repository method:

    public function getTotalParttypes(
        array $criteria = null,
        $groupByIntegratorNumber = false,
        $groupByShippingNumber = false,
        $groupByDate = false,
        $groupByLocation = false
    ) {}

There are about several use cases - use all the grouping or at least one or two at the same time.

I added a spec for each grouping case:

        $spec = Spec::andX(
            new DoctrineGroupFooSpecification(),
            new DoctrineGroupBarSpecification()
        );

Unfortunately - though I am using andX every next groupBy overrides the previous:

class DoctrineGroupBarSpecification
    extends BaseSpecification
{
    public function getSpec()
    {
        return Spec::groupBy('bar');
    }

Currently I use this spec for all grouping using the query builder addGroupBy method:

    public function modify(QueryBuilder $qb, $dqlAlias)
    {
        $qb
            ->addGroupBy($dqlAlias . '.date')
            ->addGroupBy($dqlAlias . '.shippingnumber')
            ->addGroupBy('p.integratorNumber')
            ->addGroupBy($dqlAlias . '.location');
    }

Using the modify method I could add each grouping via addGroupBy on the query builder.

I just expected the andX spec to us the addGroupBy method and not override existing grouping.

Should this behaviour be improved?

Usage custom types in Condition value

For example:

Spec::orX(
  Spec::eq('type', new OfferType(OfferType::GROUP)),
  Spec::eq('type', new OfferType(OfferType::PERSONAL))
);

This OfferType use Custom Doctrine Type OfferTypeType:

class OfferTypeType extends Type{
  ...
  public function convertToPHPValue($value, AbstractPlatform $platform){
    return new OfferType(self::convertCodeToName((int) $value));
  }

  public function convertToDatabaseValue($value, AbstractPlatform $platform){
    return self::convertNameToCode($value->getNmae());
  }
}

class Offer{
  ...

  /**
   * @ORM\Column(type="offerType")
    */
  private $type;

  ...
}

But OfferTypeType not using in Spec condition:
Catchable fatal error: Object of class OfferType could not be converted to string in /var/www/work/ga/backend/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php on line 67

P.S. Sorry for my English.

OrderBy (and others) not working with aliased fields

The OrderBy Query Modifier (and others) does not support using aliased fields.

$qb->addOrderBy(sprintf('%s.%s', $dqlAlias, $this->field), $this->order);

This line enforces a table.field regardless of the dqlAlias passed in. Whereas I will often want to order on an aliased field (with no dot).

Create tag 0.3.0?

I feel that we are close to a stable release. I suggest to create a tag 0.3.0 with what we got at the moment. We could then improve the docs and make some last fixes on the interfaces and our API. That will also give the lib a change to mature and we could test in in our own projects easier.

Migrating from 0.3.0 to 1.0.0 will probably be painless.

Library scope

We need to discuss the scope of this library. Should we remove AsArray Spec to only include selectors?

Or maybe we should add Specs like Limit and OrderBy.

@cakper What do you think?

Readability

It is quite hard to understand the current structure of the Specification. When you look at:

$spec = Spec::andX(
    Spec::eq('ended', 0),
    Spec::orX(
        Spec::lt('endDate', new \DateTime()),
        Spec::andX(
            Spec::isNull('endDate'),
            Spec::lt('startDate', new \DateTime('-4weeks'))
        )
    )
);

return $this->em->getRepository('HappyrRecruitmentBundle:Advert')->match($spec);

it's quite similar to Doctrine version (in the context of readability). I think it could be improved by such creation Specifications:

$spec = (new Property('ended'))->equals(0)->and(
    (new Property('endDate'))->lessThan(new \DateTime())->or(
        (new Property('endDate'))->isNull()->and(
            (new Property('startDate'))->lowerThan(new \DateTime('-4weeks'))
        )
    )
);

advantage is that you can read it:
Property ended is equals to "0" and property end date i less than DateTime (or $now = new \DateTime) (...). But it also has disadvantages:

  • Property class is a little bit virtual,
  • there is no new property at all,
  • it doesn't have to be a property,
  • implementation may seems tricky
  • and probably something else I did not notice

Recipe

  • Introduce new Property class:
class Property
{
    use Conditions;

    public function __construct($name)
    {
        $this->name = $name;
    }
}
  • Adding this trait to each filter (Equals, LessThan, isNull etc) and to Property class:
trait Conditions
{
    protected $propertyName;

    public function equals($value)
    {
        return new Equals($this->propertyName, $value);
    }

    public function lessThan($value)
    {
        return new LessThan($this->propertyName, $value);
    }

    public function isNull()
    {
        return new IsNull($this->propertyName);
    }
}
  • Adding this trait to LogicX class:
trait LogicSpecification
{
    public function and($value)
    {
        return new AndX($this, $value);
    }

    public function or($value)
    {
        return new OrX($this, $value);
    }
}

after correcting any typos it should work. Voilร .

Conclusion

This may not be the best solution, or it may be even worse than the existing one. I would like to ask you to comment on readability and/or alternative ideas.

Improve usage of dqlAlias when using having and / or custom function e.g. from Doctrine Extensions

Here is the workaround I currently use for a little more complex use case:

final class OverduePriceQuote extends BaseSpecification
{
    private $dueDays;

    public function __construct(DueDays $days, $dqlAlias = null)
    {
        parent::__construct($dqlAlias);
        $this->dueDays = $days->days();
    }

    public function getSpec()
    {
        return Spec::andX(
            Spec::eq('isArchived', false),
            Spec::isNull('deletedAt'),
            Spec::isNull('remindedAt'),
            Spec::isNotNull('assignedAt'),
            sprintf('DATE_DIFF(CURRENT_DATE(), %s.assignedAt) > %s', 'priceQuote', $this->dueDays)
        );
    }
}

As you can see in the last Spec I had to set the dqlAlias manually to priceQuote since the property is private on the BaseSpecification. Any reason why it is not protected?

When I started I tried something different:

#Spec::gt(sprintf('DATE_DIFF(CURRENT_DATE(), %s.assignedAt)', $this->dqlAlias), $this->dueDays)
Spec::gt(sprintf('DATE_DIFF(CURRENT_DATE(), %s.assignedAt)', 'priceQuote'), $this->dueDays)

Unfortunately this is not possible since gt adds the dqlAlias since it expects a column:

AND priceQuote.DATE_DIFF(CURRENT_DATE(), priceQuote.assignedAt) > :comparison_2) 

I also tried having:

Spec::having(sprintf('DATE_DIFF(CURRENT_DATE(), %s.assignedAt) > %s', 'priceQuote', $this->dueDays))

Unfortunately the column is not found though the dqlAlias is correct:

SELECT p0_.preisanfrage_id AS preisanfrage_id_0 FROM preisanfrage p0_
...
HAVING DATEDIFF(CURRENT_DATE, p0_.date_assigned) > 7

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'p0_.date_assigned' in 'having clause'

This will also not work using an alias e.g.:

$qb->addSelect('DATE_DIFF(CURRENT_DATE(), priceQuote.assignedAt) AS due_days);

Since it is not supported: #129 (comment)

Any suggestions how to improve this use case and working with custom functions e.g. DATEDIFF from DoctrineExtensions in general?

add Upgrade.md

it is very important to document the upgrade from 0.2.0 to new tag to be tagged, so i think we can bootstrap the documentation for that. I think the most indicated person to do this is who did the refactor. I will help cleaning it up and validating it.

InSpec problem

the setParameters('in_10', $this->Value ) should value be an array? why it is said to be a string?

Should Filter specs implement Specification interface?

Its imposible now

$repository->match(Spec::eq('name', 'foo'))

but this works

$repository->match(Spec::orX(
    Spec::eq('name', 'foo'),
    Spec::eq('name', 'bar')   
))

Should Comparisons implements Specification interface? Or repository method match should be like:

public function match($specification
      //....
        if ($specification instanceof QueryModifier) {
            $specification->modify($queryBuilder, $this->getAlias());
        }
        if ($specification instanceof Filter) {
            $queryBuilder->where($specification->getFilter($queryBuilder, $this->getAlias()));
        }

set branch alias

please set a branch alias in composer.json

this is the least ideal thing to do since i see that this package is not tagged regularly
I don't think alpha helps with the dependencies from a top composer that is stable that is our project from where we specify the dependencies

Show bad examples

Write some examples that shows the problems.

  • Too many functions
  • Too many arguments
  • Code duplicate
  • Difficult to test

concern

just right before you merge the refactor PR or any other big changes, please tag once more the library before and one after. So two tags. To me it is very important because if you don't it will break my project. So it is very critical the before and after tags.

Violating Interface Segregation Principle

Hello,

PR #35 and question asked by @Nyholm forced me to think about design of our library and I have some thoughts on it:

  • All classes in Comparison and Logic namespaces implement only Specification::match but not Specification::modifyQuery
  • Some classes in Query namespaces implement Specification::match others implement Specification::modifyQuery but never both
  • In all cases if we don't implement match/modifyQuery we only call parents
  • It doesn't make sense to do LogicX logical operations on Specifications that only implement match - example: LogicOr(AsArray, Cache) will not take any sensible effect
  • All Specification::modifyQuery are logical And's, have to be matched otherwise don't make sense
  • We are using match and modifyQuery sequentially, it's always: $query = $qb->where($specification->match($qb, 'e'))->getQuery(); $specification->modifyQuery($query);

Above thoughts led me to conclusion that we are violating Interface Segregation Principle which states:

no client should be forced to depend on methods it does not use

So basically we have two different concepts here - one that is a Specification and allows us to build query, the other that I don't have name for that allows to manipulate query result (cache it, hydrate as array etc).

In my opinion we should split these two things into two separate interfaces and allow clients to implement single or both. Also because they look like separate things should be passed to EntitySpecificationRepository::match separately so we will need to change it's interface to something like match(Specification $specification, QueryOption $option).

That will enable us to write more sensible queries to repository. Example:

  • we have Specification that contains rules for selecting power users, name could be PowerUserSpecification
  • we have option that allows us to cache queries for a standard amount of seconds (we don't really care how log it is, as in PowerUserSpecification we don't care what are the exact rules for selecting those users), name of this option could be StandardCacheOption
  • now for the standard display we can get results from our repository with EntitySpecificationRepository::match(PowerUserSpecification,StandardCacheOption)
  • but for administration we need non cached version of it so we can simply call EntitySpecificationRepository::match(PowerUserSpecification) and get non cached results

@Nyholm please let me know what you think. I know it changes lib quite dramatically but because we are in pre release mode I think it's the best moment to discuss such thing.

One remaining issue is way of grouping so-called options together (just highlighting it).

LogicX child is array of children

Im writing a seperate issue for the bug in #114

I have a Handler Superclass for data management. Its subclasses are Entity Specific and can overwrite the following getEntities Method:

public function getEntities($limit, $offset, $spec = null) {
    return $this->repository->match(
        Spec::andX(
            $spec,
            Spec::limit($limit),
            Spec::offset($offset)
        )
    );
}

Now if i dont overwrite the method to add specific spec it works. In my current case i override it with the following:

public function getEntities($limit, $offset, $spec = null) {
    if($this->user) {
        return parent::getEntities($limit, $offset, Spec::andX(new InMyStores($this->user->getStores()), $spec));
    }
    return parent::getEntities($limit,$offset);
}

Now in the array_map callback $spec is actually an array of children, so its skipped right now. What can i do to fix this @Nyholm ?

Cannot use custom fields / aliases from subquery in having

I need to add a custom field on my select clause so I can use it inside a HAVING clause:

    public function modify(QueryBuilder $qb, $dqlAlias)
    {
        $qb2 = clone $qb;
        $qb2->resetDQLParts();

        $qb2->select("GroupConcat(DISTINCT bpt.integratorNumber SEPARATOR ', ')")
            ->from('AppBundle:Bundle', 'b')
            ->join('b.parttype', 'bpt', Join::WITH, 'b.parttype IS NOT NULL')
            ->groupBy('b.parttype')
        ;

        $qb
            ->addSelect(sprintf("(%s) AS partNumbers", $qb2->getDql()))
        ;

        parent::modify($qb, $dqlAlias);
    }

Using $qb->having() would work. But I need to add it to the Specs:

class FullTextSearch
{
    public function getSpec()
    {
        return Spec::orX(
            Spec::like('number', $this->fullText),
            Spec::like('problemNumber', $this->fullText),
            Spec::having(
                Spec::like('partNumbers', $this->fullText, Like::CONTAINS)
            )
        );
    }

Unfortunately the custom field is not recognized:
[Semantical Error] line 0, col 1788 near 'partNumbers LIKE': Error: Class has no field or association named partNumbers

Is there any quick workaround maybe using creating an extra Spec and combine it with this FullTextSearch?

What does having here actually do since #109 ?

can we remove useless information

can we remove @author and class names which are redundant?

they will help a lot with readability and increasing contribution. The composer and git take care of the @author that of old some very old php projects use to have. This is the present and is Sept 16, 2014. ๐Ÿ‘ถ

If you agree I can remove those and clean a bit more.

Thanks!

adding count feature?

I am adding this, i know still using an earlier version but i was wondering if it was worth considering.

<?php
public function countOf(Specification $specification)
    {
        if (!$specification->supports($this->getEntityName())) {
            throw new \InvalidArgumentException("Specification not supported by this repository.");
        }

        /** @var QueryBuilder $qb */
        $qb = $this->createQueryBuilder($this->getAlias());
        $qb->select('COUNT('.$this->getAlias().')');

        $expr = $specification->match($qb, $this->getAlias());

        $query = $qb->where($expr)->getQuery();

        $specification->modifyQuery($query);

        return (int) $query->getSingleScalarResult();
    }

Difference between QueryModifier and ResultModifer

Consider these two interfaces:

interface QueryModifier
{
    /**
     * @param QueryBuilder $qb
     * @param string       $dqlAlias
     * @return void
     */
    public function modify(QueryBuilder $qb, $dqlAlias);
} 
interface ResultModifier
{
    /**
     * @param AbstractQuery $query
     * @return void
     */
    public function modify(AbstractQuery $query);
}  

They are very similar. A QueryModifier is ment to be used inside a specification (like Join or Limit). It makes sense to have a spec like GetTop5Users.

A ResultModifier should not be a part of the specification but be applied as a second parameter to EntitySpecificationRepository::match (like AsArray). A specification should be atomic and should not bother about how the result is displayed.

However, it does also makes sense to have Limit as a ResultModifier.

$spec = Spec::andX(
  new ActiveUsersSpec(),
  new Limit(10)
);
$repo->match($spec);
$spec = Spec::andX(
  new ActiveUsersSpec()
);
$repo->match($spec, new Limit(10));

I want to allow for Limit to be used both as QueryModifier and ResultModifier. My first thought was to merge the interfaces into one. The problem is that it's only some QueryModifiers (Limit, OrderBy) can be used as both QueryModifiers and ResultModifiers but others (like Join) should only be a QueryModifier.

My second thought was to duplicate the Limit (and others). To have one Happyr\DoctrineSpecification\Query\Limit and one Happyr\DoctrineSpecification\Result. But that does not look very fancy at all. I believe there is need for some rethinking and refactoring here.

Any idea how this problem should be tackled?

docblock discussion

Some specification has this:

    /**
     * @param \Doctrine\ORM\QueryBuilder $qb
     * @param string $dqlAlias
     *
     */
    public function match(QueryBuilder $qb, $dqlAlias)
    {
        $qb->setMaxResults($this->limit);
    }

The docblock should be {@inheritdoc} but i see we do no return, can we return something?

Jetbrains IDE autocomplete?

Anyone know how to make autocomplete work for Jetbrains IDEs for the field names like they do for querybuilders? (using PHPStorm)

No Documentation On Contributing

Hi. I stumbled upon this library and I really like the concept, I even had a similar idea I was thinking of writing a library for myself but given you are this far along I think I'd rather contribute to your. I noticed there wasn't any documentation on contributing. So adding a Contributing.MD would be beneficial (if you are open to contributions).

Unrelated, are there any issues/features outside of the the current open issues/PRs that need to be resolved before a stable version is released?

Cloning QueryBuilder in modify method does not reset clause parts

I have a more complex query that needs to a second query builder to create extra DQL:

        $qb2->select('MAX(cs2.createdAt)')
            ->from('AppBundle:ContractState', 'cs2')
            ->where('cs2.contract = ' . sprintf('%s.id', $dqlAlias))
            ->addOrderBy('cs2.date', 'desc')
            ->addOrderBy('cs2.createdAt', 'desc')
        ;

        $qb
            ->andWhere('cs.createdAt = (' . $qb2->getDql() . ')')
            ->andWhere('cs.state = :state')
            ->setParameter('state', $this->state);

This worked fine so far in my repository. For using it with specs I found no other solution than cloning the original query builder:

    public function modify(QueryBuilder $qb, $dqlAlias)
    {
        $qb2 = clone $qb;
        $qb2->select('MAX(cs2.createdAt)')
            ->from('AppBundle:ContractState', 'cs2')
            ->where('cs2.contract = ' . sprintf('%s.id', $dqlAlias))
            ->addOrderBy('cs2.date', 'desc')
            ->addOrderBy('cs2.createdAt', 'desc');

        $qb
            ->andWhere('cs.createdAt = (' . $qb2->getDql() . ')')
            ->andWhere('cs.state = :state')
            ->setParameter('state', $this->state);

        parent::modify($qb, $dqlAlias);
    }

My service looked like this in the first place:

        $listQuery = $this->contractRepository->getQuery(
            Spec::andX(
                new ContractedByBranch(BranchId::create(2)),
                new WithCurrentState(ContractState::create($query->state()))
            )
        );

This give me the following DQL for my clone subquery:

SELECT MAX(cs2.createdAt) FROM AppBundle\Entity\Contract e INNER JOIN e.branch br, AppBundle:ContractState cs2 WHERE cs2.contract = e.id ORDER BY cs2.date desc, cs2.createdAt desc

I recognized that the JOIN parts were not resetted. I changed the order of my specs:

        $listQuery = $this->contractRepository->getQuery(
            Spec::andX(
                new WithCurrentState(ContractState::create($query->state())),
                new ContractedByBranch(BranchId::create(2))
            )
        );

Giving me:

SELECT MAX(cs2.createdAt) FROM Plusquam\Bundle\ContractBundle\Entity\Contract e, PlusquamContractBundle:ContractState cs2 WHERE cs2.contract = e.id ORDER BY cs2.date desc, cs2.createdAt desc

Which looked better. But as you can see the FROM part hast 2 entities - including the "root" with the $dqlAliase.

Since the query used to work fine before I'm not sure if this is a pure query builder issue.

The workaround is to always reset the DQL parts manually:

        $qb2 = clone $qb;
        $qb2->resetDQLParts();

Is there a different way to get (another) query builder from the repository / entity manager?

BTW: I will refactor the query to use a HAVING clause on the subquery result. But anyways a second query builder will be required.

API review

I think these are valid Specifications. Please review.

class PowerUser extends BaseSpecification
{
    /**
     * @return Expression
     */
    public function getWrappedExpression()
    {
        return Spec::andX(
            Spec::eq('test', 2),
            Spec::gt('length', 4, 'c')
        );
    }

    /**
     * @return Modifier
     */
    public function getWrappedModifier()
    {
        return new ModifierCollection(
            Spec::join('comments', 'c'),
            new OrderBy('name')
        );
    }
}
class AnyUser extends BaseSpecification
{
    function __construct($limit, $dqlAlias=null)
    {
        parent::__construct($dqlAlias);
        $this->spec = Spec::orX(new PowerUser(), new OtherUser());
        $this->limit = $limit;
    }

    /**
     * @return Expression
     */
    public function getWrappedExpression()
    {
        return $this->spec;
    }

    /**
     * @return Modifier
     */
    public function getWrappedModifier()
    {
        return new ModifierCollection(new Limit($this->limit), $this->spec);
    }
}
class SomeService
{
  public function foo()
  {
    $users=$this->repo->match(new AnyUser(5), new ModifierCollection(new AsArray(), new SomethingElse());
  }

}

HHVM support

This commit breaks the build on HHVM.

It is probably an error with HHVM. But I just wanted to record the commit.

Support for Paginator

Right now the EntitySpecificationRepository::match method builds and executes a query directly. There is no way I can see to implement Doctrines Paginator for paginating complex queries. Would it break any principles to add a third "paginate" boolean parameter to the match method that wraps the query in a Paginator before execution, or to add a second "getPaginator" method to the EntitySpecificationRepository that returns a Paginator rather than results?

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.