Refactor cursor classes to compose CursorInterface #1206
Conversation
| * @param ReferencePrimer $referencePrimer | ||
| * @return self | ||
| */ | ||
| public function enableReferencePriming(array $primers, ReferencePrimer $referencePrimer) |
There was a problem hiding this comment.
As discussed in video chat, let's have this method (and primeReferences()) assert that the wrapped cursor is eager. If not, we can throw an exception.
This addresses the manual case of users creating their own ODM Cursor, since the query builder should not allow this anyway.
There was a problem hiding this comment.
Done, enableReferencePriming now throws a BadMethodCallException if the wrapped cursor is not an EagerCursor.
alcaeus
force-pushed
the
eagercursor-deprecation
branch
from
264d024 to
db0fa7a
Compare
alcaeus
changed the title
alcaeus
changed the title
| @@ -319,6 +387,19 @@ public function hydrate($hydrate = true) | |||
| } | |||
|
|
|||
| /** | |||
| * @param array $document | |||
| * @return object | |||
There was a problem hiding this comment.
This seem to return array|object|null as for instance getSingleResult()?
alcaeus
force-pushed
the
eagercursor-deprecation
branch
from
db0fa7a to
6681db4
Compare
alcaeus
force-pushed
the
eagercursor-deprecation
branch
2 times, most recently
from
39fd3fc to
d3768c6
Compare
alcaeus
changed the title
| @@ -207,6 +218,20 @@ public function batchSize($num) | |||
| } | |||
|
|
|||
| /** | |||
| * Wrapper method for MongoCursor::count(). | |||
| * | |||
| * @see \Doctrine\MongoDB\Cursor::count() | |||
There was a problem hiding this comment.
Would it make more sense for these @see lines to point to the CursorInterface? I believe that also has doc-blocks, and then you could also do without the full namespace prefix, too (since the class name isn't ambiguous).
There was a problem hiding this comment.
Yes, definitely. Will change the docblocks accordingly.
alcaeus
force-pushed
the
eagercursor-deprecation
branch
from
d3768c6 to
ec5e37c
Compare
| /** | ||
| * EagerCursor wraps a Cursor instance and fetches all of its results upon | ||
| * initialization. | ||
| * | ||
| * @since 1.0 | ||
| * @author Jonathan H. Wage <[email protected]> | ||
| * @deprecated Deprecated in favor of using Cursor |
There was a problem hiding this comment.
Can we append "; will be removed in 2.0"? (done for other @deprecated lines).
This deprecates the EagerCursor class since its main goal is already handled by Doctrine\MongoDB\EagerCursor. This also allows us to fix a long-standing issue where calling getSingleResult() on a query with reference priming enabled would prime references for the entire dataset.
alcaeus
force-pushed
the
eagercursor-deprecation
branch
from
ec5e37c to
6f13cf4
Compare
|
LGTM. |
|
|
Refactor cursor classes to compose CursorInterface
Fixes #1018, Closes #639.
This change refactors the Cursor class to no longer extend Doctrine\MongoDB\Cursor and instead implement the new cursor interface.
Currently tests are failing due to an issue with complex id types and toArray functionality, this can be fixed by always defining useKeys = false for eager cursors after doctrine/mongodb#223 has been merged.Composing CursorInterface instead of the base cursor class also allows us to deprecate the ODM\EagerCursor class since the special handling related to eager cursors is done by doctrine/mongodb and does not need any special handling.
This also fixes a long-standing issue (#520) where using reference priming in combination with
getSingleResultwould still prime references for the entire dataset, not only the one document being fetched from the cursor.Last but not least, this introduces
~1.2as new version requirement for doctrine/mongodb and removes special treatment for eager cursors in DocumentPersister.Note: since a few methods are still missing (see doctrine/mongodb#225) and there is no stable release of doctrine/mongodb containing the interface yet, this branch temporarily uses a dev version of doctrine/mongodb.