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

Add adapter to support Mongo Aggregation Builder #277

Open
webdevilopers opened this issue Apr 11, 2019 · 2 comments
Open

Add adapter to support Mongo Aggregation Builder #277

webdevilopers opened this issue Apr 11, 2019 · 2 comments

Comments

@webdevilopers
Copy link

webdevilopers commented Apr 11, 2019

Currently the DoctrineODMMongoDBAdapter only supports the Query Builder:

But there is also the Aggregation Builder:

Adding an additional adapter based on the existing one is actually easy:

use Doctrine\ODM\MongoDB\Aggregation\Builder;
use Pagerfanta\Adapter\AdapterInterface;

class DoctrineODMMongoDBAggregationAdapter implements AdapterInterface
{
    private $aggregationBuilder;

    /**
     * Constructor.
     *
     * @param Builder $aggregationBuilder A DoctrineMongo aggregation builder.
     */
    public function __construct(Builder $aggregationBuilder)
    {
        $this->aggregationBuilder = $aggregationBuilder;
    }

    /**
     * Returns the query builder.
     *
     * @return Builder The query builder.
     */
    public function getAggregationBuilder()
    {
        return $this->aggregationBuilder;
    }

    /**
     * {@inheritdoc}
     */
    public function getNbResults()
    {
        return $this->aggregationBuilder->execute()->count();
    }

    /**
     * {@inheritdoc}
     */
    public function getSlice($offset, $length)
    {
        return $this->aggregationBuilder
            ->limit($length)
            ->skip($offset)
            ->execute();
    }
}

Unfortunately none of the two builders implement a common Interface. But there is another reason why using a single adapter for both use cases could be problematic!

The aggregation builder uses a pipeline. Each time a so called "stage" e.g. limit oder sort is added the return value of the assigned variable changes.

$qb = $this
    ->createAggregationBuilder();
    ->sort(['leavingDate' => 1])
    ->limit($query->limit);

dump($qb); // Instance of Limit extends Stage

$qb = $this
    ->createAggregationBuilder();
$qb
    ->sort(['leavingDate' => 1])
    ->limit($query->limit);

dump($qb); // Instance of Builder

As soon as you pass a Stage instance you can not retrieve the original builder:

Also there are plans for ODM version 2 which may change this behaviour:

Maybe @alcaeus or @malarzm could comment on this too? Thanks in advance!

Possibly related issues:

@alcaeus
Copy link

alcaeus commented Apr 11, 2019

The aggregation builder is a bit different from the query builder in terms of how it handles its data. I'm not sure which way I like better, but it seemed sensible to build it like this at the time. Either way, while this will store a reference to a query builder:

$builder = $this->createQueryBuilder()
    ->field('foo')->equals('bar');

this will not store a reference to an aggregation builder, but rather to a single match stage:

$builder = $this->createAggregationBuilder()
    ->match()
         ->field('foo')->equals('bar');

If you want to store the builder object, you definitely want to first create an aggregation builder, then modify it further:

$builder = $this->createAggregationBuilder();
$builder
    ->match()
         ->field('foo')->equals('bar');

Since adding a stage to the builder modifies the builder instance, you'll want to create a clone of $this->aggregationBuilder in your getSlice method, otherwise you'll just end up with multiple conflicting $limit and $skip stages in your pipeline when retrieving multiple pages.

Also there are plans for ODM version 2 which may change this behaviour

Not sure where in that PR you saw that, but right now that sounds like news to me 😉

@malarzm
Copy link

malarzm commented Apr 11, 2019

Also there are plans for ODM version 2 which may change this behaviour

Not sure where in that PR you saw that, but right now that sounds like news to me ;)

I think this might have been related to doctrine/mongodb-odm#1988 (comment) where the Query' is changing :)

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

No branches or pull requests

3 participants