Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Add ability to prime collections.

  • Loading branch information...
commit 2da6e9c92d515edae7c63233740349e0d272b19b 1 parent fd247ce
@jwage jwage authored
View
46 lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
@@ -38,6 +38,7 @@
Doctrine\ODM\MongoDB\Cursor,
Doctrine\ODM\MongoDB\LoggableCursor,
Doctrine\MongoDB\Cursor as BaseCursor,
+ Doctrine\MongoDB\Iterator,
Doctrine\MongoDB\LoggableCursor as BaseLoggableCursor;
/**
@@ -493,6 +494,51 @@ private function createDocument($result, $document = null, array $hints = array(
}
/**
+ * Prime a collection of documents property with an efficient single query instead of
+ * lazily loading each field with a single query.
+ *
+ * @param Iterator $collection
+ * @param string $fieldName
+ * @param Closure|boolean $primer
+ * @param array $hints
+ */
+ public function primeCollection(Iterator $collection, $fieldName, $primer, array $hints = array())
+ {
+ $collection = $collection->toArray();
+ $collectionMetaData = $this->dm->getClassMetaData(get_class(current($collection)));
+
+ $fieldMapping = $collectionMetaData->fieldMappings[$fieldName];
+
+ $ids = array();
+ foreach ($collection as $element) {
+ if ($fieldMapping['type'] == 'many') {
+ foreach ($collectionMetaData->getFieldValue($element, $fieldName)->getMongoData() as $data) {
+ $ids[] = $data['$id'];
+ }
+ } else if ($fieldMapping['type'] == 'one') {
+ if ($document = $collectionMetaData->getFieldValue($element, $fieldName)) {
+ $ids[] = $this->uow->getDocumentIdentifier($document);
+ }
+ }
+ }
+
+ if (! empty($ids)) {
+ if ($primer instanceof \Closure) {
+ $primer($this->dm);
+ } else {
+ $fieldRepository = $this->dm->getRepository($fieldMapping['targetDocument']);
+ $qb = $fieldRepository->createQueryBuilder()
+ ->field('id')->in($ids);
@AD7six
AD7six added a note

shouldn't that be

->field('_id')

?

@jwage Owner
jwage added a note

Doctrine will convert it to _id. It should actually be ->field($collectionMetadata->identifier)->in($ids)

@AD7six
AD7six added a note

Thanks for clarifying,

Would it not be beneficial to have the odm treat both id and _id as the same field - always, or to canonicalize internally on one or the other? If documents have an $_id property (which is logical, since that´s the key name on the db) - doctrine raises no errors for code referencing ´id´ but any meta-data conversions will be missing, i.e. passed values are not converted to object ids before inserting into queries.

As you know mongo doesn't permit the primary key to be named anything other than _id - so looking up meta data to get a string which doesn´t change is perhaps a means of simplifying some parts of the code and probably therefore a small performance gain too (in cases like this where you only want to look up the key name and not the meta data for the primary key).

@jwage Owner
jwage added a note

The semantics are simple and consistent already I think. I just don't have them properly documented probably.

When you use _id, no preparation is performed and you are responsible for passing through the appropriate type you have in the database. Since _id is not restricted to being a MongoId, it can be a string, integer, or any type you want. So assuming we can convert it to a MongoId assumes that it is mapped that way, which I think would be wrong. If you had it mapped as a string like this then your queries would break because it tries to convert it to MongoId:

/** @ODM\Document */
class User
{
    /** @ODM\Id(type="string") */
    private $username;
}

Now this query would be wrong:

$qb->field('_id')->equals('jwage');

It would essentially try to run this query:

 db.users.find({"_id" : ObjectId("jwage")})

When this is what is stored in the database:

{
    "_id" : "jwage"
}

Now take a normal id mapped as a MongoId:

class User
{
    /** @ODM\Id */
    private $theIdentifier;
}

If you pass just _id you would have to do:

$qb->field('_id')->equals(new MongoId($user->getId()));

But if you use the class property name you mapped as an identifier, it will apply the appropriate conversion to the database type you have mapped:

$qb->field('theIdentifier')->equals($user->getId());

And you would end up with a query like the following, assuming $user->getId() returns a value of 1234:

db.users.find({"_id" : ObjectId("1234")})
@jwage Owner
jwage added a note

I don't think it can be simplified because we have the id generation abstracted so you can have different id generation strategies.

/** @ODM\Id(strategy="increment") */
private $id;

Instead of creating a MongoId it will maintain a count in another collection and $inc to get the next integer identifier.

@AD7six
AD7six added a note

I think you've misunderstood my comment to be a request to assume _id is always a mongoid.

@jwage Owner
jwage added a note

Maybe I don't understand then. What is it that you desire?

@AD7six
AD7six added a note

Do developers really create documents like this:

/** @ODM\Document */
class User
{
    /** @ODM\Id(type="string") */
    private $username;
}

?

Mongo's primary key is always named _id, for that reason it would seem logical to create

/** @ODM\Document */
class User
{
    /** @ODM\Id(strategy="X") */
    protected $_id;
}

(That's how all our documents looked before this conversation.)

Yet it would appear that the odm code/doctrine in general anticipates

/** @ODM\Document */
class User
{
    /** @ODM\Id(strategy="X") */
    protected $id;
}

It seems wasteful to need to derive/lookup "_id" - I guess that´s unavoidable if the ODM permits developers to name the primary key property whatever they want.

@jwage Owner
jwage added a note

Yes, they do and can create documents like that. We make no assumptions about your model or force you to do anything one way.

If you want your class property to be named $_id, that is no problem. You can do that. The fact that I have id hardcoded here is a bug/mistake on my part and should be fixed. It should be using the $collectionMetadata->identifier property of the metadata. I will commit a fix.

@jwage Owner
jwage added a note

Fixed here b18faea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (isset($hints[Query::HINT_SLAVE_OKAY])) {
+ $qb->slaveOkay(true);
+ }
+ $query = $qb->getQuery();
+ $query->execute()->toArray();
+ }
+ }
+ }
+
+ /**
* Loads a PersistentCollection data. Used in the initialize() method.
*
* @param PersistentCollection $collection
View
25 lib/Doctrine/ODM/MongoDB/Query/Builder.php
@@ -61,6 +61,13 @@ class Builder extends \Doctrine\MongoDB\Query\Builder
*/
private $refresh = false;
+ /**
+ * Array of primer Closure instances.
+ *
+ * @var array
+ */
+ private $primers = array();
+
public function __construct(DocumentManager $dm, $cmd, $documentName = null)
{
$this->dm = $dm;
@@ -72,6 +79,21 @@ public function __construct(DocumentManager $dm, $cmd, $documentName = null)
}
/**
+ * Use a primer to load the current fields referenced data efficiently.
+ *
+ * $qb->field('user')->prime(true);
+ * $qb->field('user')->prime(function(DocumentManager $dm) {
+ * // do something that will prime all the associated users in one query
+ * });
+ *
+ * @param Closure|boolean $primer
+ */
+ public function prime($primer = true)
+ {
+ $this->primers[$this->currentField] = $primer;
+ }
+
+ /**
* @param bool $bool
* @return Builder
*/
@@ -202,7 +224,8 @@ public function getQuery(array $options = array())
$options,
$this->cmd,
$this->hydrate,
- $this->refresh
+ $this->refresh,
+ $this->primers
);
}
View
19 lib/Doctrine/ODM/MongoDB/Query/Query.php
@@ -71,13 +71,21 @@ class Query extends \Doctrine\MongoDB\Query\Query
*/
private $refresh = false;
- public function __construct(DocumentManager $dm, ClassMetadata $class, Database $database, Collection $collection, array $query = array(), array $options = array(), $cmd = '$', $hydrate = true, $refresh = false)
+ /**
+ * Array of primer Closure instances.
+ *
+ * @var array
+ */
+ private $primers = array();
+
+ public function __construct(DocumentManager $dm, ClassMetadata $class, Database $database, Collection $collection, array $query = array(), array $options = array(), $cmd = '$', $hydrate = true, $refresh = false, array $primers = array())
{
parent::__construct($database, $collection, $query, $options, $cmd);
$this->dm = $dm;
$this->class = $class;
$this->hydrate = $hydrate;
$this->refresh = $refresh;
+ $this->primers = $primers;
}
/**
@@ -167,6 +175,15 @@ public function execute()
$results->reset();
}
+ if ($this->primers) {
+ $documentPersister = $this->dm->getUnitOfWork()->getDocumentPersister($this->class->name);
+ foreach ($this->primers as $fieldName => $primer) {
+ if ($primer) {
+ $documentPersister->primeCollection($results, $fieldName, $primer, $hints);
+ }
+ }
+ }
+
if ($this->hydrate && is_array($results) && isset($results['_id'])) {
// Convert a single document array to a document object
$results = $uow->getOrCreateDocument($this->class->name, $results, $hints);

4 comments on commit 2da6e9c

@lsmith77

hmm in PHPCR ODM (and afaik CouchDB ODM) we do not have DocumentPersister .. but we should also eventually support ways to load entire collections and ideally this should become part of the persistence interfaces.

/cc @beberlei

@henrikbjorn

Any documentation about how this would be used?

@jwage
Owner

I added tests for it in another commit here: d148acf

$qb->field('groups')->prime(true);

Will load all the groups in the resultset in one query.

@moecre

That's great! A few days ago I've coded this feature 'manual' in our project so good to know that's now available in doctrine.

@henrikbjorn: http://doctrine-mongodb-odm.readthedocs.org/en/latest/reference/priming-references.html is the site where I got to know about this change.

@jwage: Is there a way to execute native MongoDB query's with doctrine? I was wondering whether there is a way to construct an $or query - or is it possible by bypass doctrine and using the native PHP driver only? Thank you.

Please sign in to comment.
Something went wrong with that request. Please try again.