[2.0] Replace doctrine/mongodb for mongodb/mongodb and ext-mongodb

[alcaeus]

Note: update below.

This PR serves as the discussion point for most of the changes required by migrating from ext-mongo to ext-mongodb. In many instances, the replacement is rather painless, but there are a few heavy BC breaks involved. Note: the following lists are not exhaustive and may be expanded depending on the progress:

The current state is:

• doctrine/mongodb is still present and has been hacked to support the new driver in a dirty way. I intend to drop it completely, but first we need to find a new home for Doctrine\MongoDB\Query\Builder and Doctrine\MongoDB\Aggregation\Builder.
• GridFS changes in the underlying library require a rewrite of the persister logic for GridFS files. Unfortunately, there is a big BC break waiting to happen for people who have stored data at the root level
• Database commands like mapReduce or group currently don't work yet due to the query builder only receiving a collection object, which doesn't have any helpers for those methods. We'll have to figure out a solution when dropping doctrine/mongodb.

An incomplete list of BC breaks:

• slaveOkay is dropped completely - this should be handled via readPreference
• readPreference is no longer given as string and an optional array but instead a value object. Same goes for writeConcern.
• Storing information in the root document of a GridFS file is no longer supported by the library. All data must reside in a metadata embedded document. We'll need to figure out how to handle that BC break as it's a big one.
• Cursors can only be iterated over once. There's PHPLIB-81: Add caching iterator mongodb/mongo-php-library#327 which aims to add an iterator that allows multiple iterations of cursors (similar to EagerCursor)
• There is a BC break in inverse collections using repositoryMethod - the method may no longer return an iterator but must return a query builder. This is because the new cursors cannot be sorted like old cursors.
• Inverse references mapped with repositoryMethod can no longer be combined with the skip, limit and sort options. These have to be handled in the repositoryMethod directly.

Along with these BC breaks, there are a few things left to figure out:

• typemap setting on the driver needs to be set correctly, we might not want to deal with arrays, but there's a potential performance issue with using BSONDocument and BSONArray

[malarzm]

Thought I'll find the big 🎉 comment here :D

[csarrazi]

Hey there! Any update on this PR?

I would be glad to help if any help is needed.

[alcaeus]

I got bogged down with a bunch of other stuff and haven’t had time to work on this. Work will resume within the next few weeks, starting with finalizing the existing parts. The GridFS support will return at a later date, I’ll focus on getting the driver change done and then worry about GridFS.

[alcaeus]

It's time for an update:

• the bulk of the rewrite is done: doctrine/mongodb is no more, we fully use mongodb/mongodb now.
• GridFS support has been dropped for now but will be re-added later
• repository methods in inverse references need be cleaned up - I initially thought of changing them from cursors to query builder to allow applying skip, limit, sort and prime options. However, this doesn't make sense: if you want to use those options, use mappedBy. If you're already writing a repositoryMethod, you might as well apply those options there. The mapping will just throw an exception if you specify repositoryMethod along with either options. I believe that's the better choice as the new cursor architecture no longer allows changing that.
• travis builds currently fail due to BulkWrite exceptions and some sharding errors. I'll have to check those, but I believe they are related to some MongoDB server version problem.

[alcaeus]

@jmikola you might know the answer to this: the legacy driver returned a result object containing the error message for many of those commands, while ext-mongodb seems to throw an exception. Is it safe to remove this type of handling and always rely on exceptions being thrown?

[jmikola]

You can rely on the driver to throw an exception if the command reports { ok: 0 }. A good example of this is DropCollection::execute() in PHPLIB, as the server reports { ok: 0 } if the collection doesn't exist.

I believe this relates to your request for PHPC-578, so that you still have access to the raw command response after such an error.

[alcaeus]

That is correct: the server checks if an index covers the shard key before creation. If there is no suitable index, it will suggest an index that satisfies the requirement through the command result. We used that suggestion to create a new index and tried to enable sharding once more.

I suppose a workaround would be to attempt creating an index for the fields in the shard key (not sure if field order is relevant here) before sharding the collection.

[carusogabriel]

May I help something with tests here? 😅

[alcaeus]

I've been sick the past few days, which is why there wasn't any progress. I'll clean this up in the next couple of weeks and merge it down to odm-ng, including all BC breaks, temporarily broken stuff, etc. Once that's done, we'll have to tie up loose ends (e.g. re-adding GridFS support), remove a bunch of deprecated features, and so on. I'll create issues for all things to be done once I'm done with this pull request.

[carusogabriel]

@alcaeus Okay men. Get well soon!

[alcaeus]

Last update before merging:
Tests are now stable. There are a few skipped tests:

• Some use mocks to test which are no longer valid. These should be updated or (where possible), rewritten to test without mocks
• There is no equivalent to query logging via the Database and Collection classes. Tests using those features are currently skipped, waiting to be rewritten.

I believe it's more sensible to update these tests once this pull request has been merged: they don't affect the features (except maybe for bugs that we'll only catch later), so there's no point keeping a PR with this amount of changes open.

As for features, I've created issues to cover features that were dropped in this pull request or are not completely taken care of:

• [2.0] Document BC breaks and add deprecations in 1.3 #1696: deprecations and BC breaks
• [2.0] Re-implement GridFS #1697: GridFS support - either the way it was previously implemented or in a way that is more compatible to what MongoDB envisions (e.g., treating stored documents as immutable)
• [2.0] Finish sharding support #1698: sharding support - this is currently waiting on a driver feature or needs a workaround
• [2.0] Clean up query options for inverse references #1699: inverse collections, part 1: repositoryMethod vs. skip, limit and sort
• Performance improvements when counting inverse collections #1700: inverse collections, part 2: counting an inverse PersistentCollection initializes (loads and hydrates) the entire collection, bringing a mean performance penalty compared to ODM 1.x

Merging this pull request allows multiple people to work on this and makes it easier to apply changes like #1691 and #1692 without constantly having to rebase multiple branches. Once this pull request has been merged, 1.2.x will be branched from master and odm-ng will be merged down to master directly afterwards. This highlights 2.0.x-dev as being the new feature-development version with 1.x becoming feature frozen and moving to bugfixing only. As indicated before, there will be a 1.3 release that covers deprecations and helps migration to 2.0.

[malarzm]

[alcaeus]

dev-master has become 2.0.x-dev. 🎉