Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

QueryBuilder with fluent interface #76

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

nacmartin commented Oct 28, 2011

This a QueryBuilder with fluent interface that mimics the behaviour of the Doctrine DBAL QueryBuilder. It doesn't depend on other stuff of the DBAL, it depends only on PHPCR interfaces, so I think it can go either here or into PHPCR as "helper" code.

It is tested with unit tests and also this piece of "real code" using the QueryBuilder works:
https://github.com/nacmartin/SonataAdminBundle/blob/phpcr/Datagrid/ODM/PHPCR/ProxyQuery.php

PS: I have updated the jackalope submodule to current master to get the new names for andConstraint() and orConstraint() of the QOM.

Member

lsmith77 commented Oct 29, 2011

I think the ODM is the right place .. there might be an official JCR helper fluent interface and once that is out we should move something similar into PHPCR at which point we might decide to drop this one (or likely just adapt this one to their API and move the code).

@stof stof commented on an outdated diff Oct 29, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the LGPL. For more information, see
+ * <http://www.doctrine-project.org>.
+ */
+
+namespace Doctrine\ODM\PHPCR\Query;
+
+use PHPCR\Query\QOM\QueryObjectModelFactoryInterface;
+
+/**
+ * QueryBuilder clas ir responsible for dynamically create QOM queries.
@stof

stof Oct 29, 2011

Member

the phpdoc needs to be fixed

@stof stof commented on an outdated diff Oct 29, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ *
+ * @return arrray Orderings to apply.
+ */
+ public function getOrderings()
+ {
+ return $this->orderings;
+ }
+
+ /**
+ * Adds an ordering to the query results.
+ *
+ * @param PHPCR\Query\QOM\DynamicOperandInterface $sort The ordering expression.
+ * @param string $order The ordering direction.
+ * @return QueryBuilder This QueryBuilder instance.
+ */
+ public function addOrderBy(\PHPCR\Query\QOM\DynamicOperandInterface $sort, $order = null)
@stof

stof Oct 29, 2011

Member

you should add use statements for the classes used for the type-hint

@stof stof commented on an outdated diff Oct 29, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ private $params = array();
+
+ /**
+ * Initializes a new QueryBuilder
+ *
+ * @param PHPCR\Query\QOM\QueryObjectModelFactoryInterface $qomFactory
+ */
+ public function __construct(QueryObjectModelFactoryInterface $qomFactory)
+ {
+ $this->qomFactory = $qomFactory;
+ }
+
+ /**
+ * Get the associated QOMFactory for this query builder
+ *
+ * @return PHPCR\Query\QOM\QueryObjectModelFactoryInterface
@stof

stof Oct 29, 2011

Member

I would prefer having FQCN starting with \ in the phpdoc to be more friendly with the IDEs. Thus, the convention used in the Symfony2 world (and which will be implemented in PhpStorm 3 AFAIK) is to consider the class name in the phpdoc as relative to the current namespace (excatly like it work for typehints for instance)

@stof stof commented on the diff Oct 29, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+
+ /**
+ * Specifies an ordering for the query results.
+ * Replaces any previously specified orderings, if any.
+ *
+ * @param PHPCR\Query\QOM\DynamicOperandInterface $sort The ordering expression.
+ * @param string $order The ordering direction.
+ * @return QueryBuilder This QueryBuilder instance.
+ */
+ public function orderBy(\PHPCR\Query\QOM\DynamicOperandInterface $sort, $order = null)
+ {
+ $this->state = self::STATE_DIRTY;
+ if ($order == 'ASC' ) {
+ $ordering = $this->qomFactory->ascending($sort);
+ } else {
+ $ordering = $this->qomFactory->descending($sort);
@stof

stof Oct 29, 2011

Member

I think ASC should be used when $order is null (same previously). But it would even be better to change the default value to make it meaningful

@stof stof and 1 other commented on an outdated diff Oct 29, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ */
+ public function leftJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)
+ {
+ return $this->joinWithType($rightSource, \PHPCR\Query\QOM\QueryObjectModelConstantsInterface::JCR_JOIN_TYPE_LEFT_OUTER, $joinCondition);
+ }
+
+ /**
+ * Performs a right outer join between the stored source and the supplied source.
+ *
+ * @param \PHPCR\Query\QOM\SourceInterface $rightSource
+ * @param string $joinType as specified in PHPCR\Query\QOM\QueryObjectModelConstantsInterface
+ * @param string $joinCondition
+ * @return QueryBuilder This QueryBuilder instance.
+ * @trows RuntimeException if there is not an existing source.
+ */
+ public function rightJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)
@stof

stof Oct 29, 2011

Member

the right join does not make sense in the ODM (think about the way you would map the result)

@lsmith77

lsmith77 Oct 29, 2011

Member

well in JCR there are outer joins. search for "sql2 grammar" on google

On 29.10.2011, at 10:59, Christophe Coevoet reply@reply.github.com wrote:

  • */
    
  • public function leftJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)
  • {
  •    return $this->joinWithType($rightSource, \PHPCR\Query\QOM\QueryObjectModelConstantsInterface::JCR_JOIN_TYPE_LEFT_OUTER, $joinCondition);
    
  • }
  • /**
  • \* Performs a right outer join between the stored source and the supplied source.
    
  • *
    
  • \* @param \PHPCR\Query\QOM\SourceInterface $rightSource
    
  • \* @param string $joinType as specified in PHPCR\Query\QOM\QueryObjectModelConstantsInterface
    
  • \* @param string $joinCondition
    
  • \* @return QueryBuilder This QueryBuilder instance.
    
  • \* @trows RuntimeException if there is not an existing source.
    
  • */
    
  • public function rightJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)

the right join does not make sense in the ODM (think about the way you would map the result)

Reply to this email directly or view it on GitHub:
https://github.com/doctrine/phpcr-odm/pull/76/files#r198089

@stof

stof Oct 29, 2011

Member

for PHPCR, it makes sense. but here, the QueryBuilder is in the ODM repo where you are querying for objects. Let me know if you find a way to hydrate the relation of a null result.

@lsmith77

lsmith77 Oct 29, 2011

Member

well the thing is that there is nothing forcing you to hydrate the results to objects. thats only the common, yet optional step after executing the query.

On 29.10.2011, at 11:13, Christophe Coevoet reply@reply.github.com wrote:

  • */
    
  • public function leftJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)
  • {
  •    return $this->joinWithType($rightSource, \PHPCR\Query\QOM\QueryObjectModelConstantsInterface::JCR_JOIN_TYPE_LEFT_OUTER, $joinCondition);
    
  • }
  • /**
  • \* Performs a right outer join between the stored source and the supplied source.
    
  • *
    
  • \* @param \PHPCR\Query\QOM\SourceInterface $rightSource
    
  • \* @param string $joinType as specified in PHPCR\Query\QOM\QueryObjectModelConstantsInterface
    
  • \* @param string $joinCondition
    
  • \* @return QueryBuilder This QueryBuilder instance.
    
  • \* @trows RuntimeException if there is not an existing source.
    
  • */
    
  • public function rightJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)

for PHPCR, it makes sense. but here, the QueryBuilder is in the ODM repo where you are querying for objects. Let me know if you find a way to hydrate the relation of a null result.

Reply to this email directly or view it on GitHub:
https://github.com/doctrine/phpcr-odm/pull/76/files#r198098

@stof

stof Oct 29, 2011

Member

Well, if this QueryBuilder is about every PHPCR queries (more similar to the DBAL query builder than the ORM one), it should not be placed in the ODM but in PHPCR.

@lsmith77

lsmith77 Oct 29, 2011

Member

it may be moved to PHPCR eventually once we work out API details with the JCR people. that being said DQL also supports unhydrateable queries.

Lukas

On 29.10.2011, at 11:30, Christophe Coevoet reply@reply.github.com wrote:

  • */
    
  • public function leftJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)
  • {
  •    return $this->joinWithType($rightSource, \PHPCR\Query\QOM\QueryObjectModelConstantsInterface::JCR_JOIN_TYPE_LEFT_OUTER, $joinCondition);
    
  • }
  • /**
  • \* Performs a right outer join between the stored source and the supplied source.
    
  • *
    
  • \* @param \PHPCR\Query\QOM\SourceInterface $rightSource
    
  • \* @param string $joinType as specified in PHPCR\Query\QOM\QueryObjectModelConstantsInterface
    
  • \* @param string $joinCondition
    
  • \* @return QueryBuilder This QueryBuilder instance.
    
  • \* @trows RuntimeException if there is not an existing source.
    
  • */
    
  • public function rightJoin(\PHPCR\Query\QOM\SourceInterface $rightSource, \PHPCR\Query\QOM\JoinConditionInterface $joinCondition)

Well, if this QueryBuilder is about every PHPCR queries (more similar to the DBAL query builder than the ORM one), it should not be placed in the ODM but in PHPCR.

Reply to this email directly or view it on GitHub:
https://github.com/doctrine/phpcr-odm/pull/76/files#r198113

@stof

stof Oct 29, 2011

Member

DQL does not support right joins, exactly for the reason given here. Some DQL queries return a mixed result if you add scalar results along the object but they are hydrated

Member

lsmith77 commented Oct 29, 2011

but you can choose not to hydrate to objects. at any rate i am even hydrating to objects in one case and i am still using a left outer join, because it seems there are some limitations as to what is possible inside an ON clause with SQL2 (which is the serialization format for QOM).

Contributor

nacmartin commented Oct 31, 2011

Added three commits to fix the issues pointed by Stof, except for the right join functionality.

Member

lsmith77 commented Oct 31, 2011

@bergie seems to think we could just move it to PHPCR .. i don't have a really solid opinion either way .. @dbu?

@stof stof commented on an outdated diff Oct 31, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ private $source = null;
+
+ /**
+ * @var \PHPCR\Query\QueryObjectModelInterface
+ */
+ private $query = null;
+
+ /**
+ * @var array The query parameters.
+ */
+ private $params = array();
+
+ /**
+ * Initializes a new QueryBuilder
+ *
+ * @return \PHPCR\Query\QOM\QueryObjectModelFactoryInterface $qomFactory
@stof

stof Oct 31, 2011

Member

you should not have changed @param to @return as it is a param

@stof stof commented on an outdated diff Oct 31, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ {
+ $this->state = self::STATE_DIRTY;
+ if ($order == 'ASC' ) {
+ $ordering = $this->qomFactory->ascending($sort);
+ } else {
+ $ordering = $this->qomFactory->descending($sort);
+ }
+ $this->orderings = array($ordering);
+ return $this;
+ }
+
+ /**
+ * Specifies one restriction (may be simple or composed).
+ * Replaces any previously specified restrictions, if any.
+ *
+ * @return \PHPCR\Query\QOM\ConstraintInterface $constraint
@stof

stof Oct 31, 2011

Member

the first one is a param, not a return

@stof stof commented on an outdated diff Oct 31, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ $ordering = $this->qomFactory->ascending($sort);
+ } else {
+ $ordering = $this->qomFactory->descending($sort);
+ }
+ $this->orderings = array($ordering);
+ return $this;
+ }
+
+ /**
+ * Specifies one restriction (may be simple or composed).
+ * Replaces any previously specified restrictions, if any.
+ *
+ * @return \PHPCR\Query\QOM\ConstraintInterface $constraint
+ * @return QueryBuilder This QueryBuilder instance.
+ */
+ public function where($constraint)
@stof

stof Oct 31, 2011

Member

what about type-hinting the constraint if it must be an object implementing the needed interface (according to the phpdoc) ?

@stof stof commented on an outdated diff Oct 31, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ }
+
+ /**
+ * Creates a new constraint formed by applying a logical AND to the
+ * existing constraint and the new one
+ *
+ * Order of ands is important:
+ *
+ * Given $this->constraint = $constraint1
+ * running andWhere($constraint2)
+ * resulting constraint will be $constraint1 AND $constraint2
+ *
+ * If there is no previous constraint then it will simply store the
+ * provided one
+ *
+ * @return \PHPCR\Query\QOM\ConstraintInterface $constraint
@stof

stof Oct 31, 2011

Member

same issue here for the phpdoc

@stof stof commented on an outdated diff Oct 31, 2011

lib/Doctrine/ODM/PHPCR/Query/QueryBuilder.php
+ }
+
+ /**
+ * Creates a new constraint formed by applying a logical OR to the
+ * existing constraint and the new one
+ *
+ * Order of ands is important:
+ *
+ * Given $this->constraint = $constraint1
+ * running orWhere($constraint2)
+ * resulting constraint will be $constraint1 OR $constraint2
+ *
+ * If there is no previous constraint then it will simply store the
+ * provided one
+ *
+ * @return \PHPCR\Query\QOM\ConstraintInterface $constraint
@stof

stof Oct 31, 2011

Member

and here

Contributor

nacmartin commented Oct 31, 2011

fix fix, thanks @stof !

Member

dbu commented Nov 1, 2011

as its building 100% on the interfaces, it does not add any burden to the phpcr implementors, so i think having it in phpcr is fine. even more so if the java world is going to add a fluent interface as well.

Member

lsmith77 commented Nov 1, 2011

ok@nacmartin .. could you add the fluent interface to PHPCR itself then? but where? in the Util namespace?
https://github.com/phpcr/phpcr/tree/master/src/PHPCR/Util

Member

dbu commented Nov 1, 2011

i think PHPCR\Util is a good place. and then we should add a getQueryBuilder method to the QueryManagerInterface. this would allow implementations to replace the query builder if for some reason they want to replace it.

Contributor

nacmartin commented Nov 1, 2011

Ok, fine. Closing this PR then

@nacmartin nacmartin closed this Nov 1, 2011

Member

dbu commented Nov 1, 2011

thanks nacmartin, sorry for the back and forth about where to add this.

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