pharo-nosql / mongotalk

A Pharo driver for MongoDB
MIT License
19 stars 13 forks source link

Fix MongoDatabase>>collections #27

Closed tinchodias closed 6 years ago

tinchodias commented 6 years ago

MongoDB 3.0 changed the way to get the list of a database's collections, as explained by Udo Schneider (@krodelin) in #12 .

Changes in MongoDatabase:

collections
    "Answer the collections in this database"

    | collectionNames |
    collectionNames := root majorVersion < 3
        ifTrue: [ self mongo2CollectionNames ]
        ifFalse: [ self mongo3CollectionNames ].
    ^ collectionNames
        collect: [:each | MongoCollection database: self name: each ]

The old implementation of #collections is now named like this:

mongo2CollectionNames
    "Answer the names of the collections in this database (for MongoDB < 3.0). 
    Before 3.0, it is possible to obtain this information via a query to <database>.system.namespaces.
    This has been deprecated since MongoDB 3.0, then such query returns an empty collection. See more at: https://docs.mongodb.com/manual/reference/system-collections/#%3Cdatabase%3E.system.namespaces"

    | query reply names real |
    query := MongoQuery new collection: (MongoMetaCollection name: 'system.namespaces').
    reply := self query: query.
    "Drop options on the floor for now"
    names := reply collect: [ :each | each at: 'name' ].
    real := names select: [ :each | (each occurrencesOf: $.) = 1 ].
    ^real collect: [:each | each readStream upTo: $.; upToEnd ]

and the new implementation to get collection names is:

mongo3CollectionNames
    "Answer the names of the collections in this database (for MongoDB >= 3.0).
    See more at https://docs.mongodb.com/manual/reference/command/listCollections/"

    | reply |
    reply := self command: {(#listCollections -> 1)} asDictionary.
    ^ ((reply at: 'cursor') at: 'firstBatch') collect: [ :each | each at: 'name' ]

In fact, this implementation for 3.0 is buggy because it answers only a "first batch" of collections. However, I propose this fix as a workaround to have green tests, and after we can provide a better fix in another PR.

tinchodias commented 6 years ago

It's failing in pharo 4:

Got startup errors: 
    FileDoesNotExist: File @ /home/travis/build/tinchodias/mongotalk/mc/Mongo-BSON.package/monticello.meta/version

It's true, in this recently merged commit this file and many other .json were deleted: https://github.com/pharo-nosql/mongotalk/commit/74a6255292bd2ace6f539ad50eb1204d074b659b#diff-3a0cef526df4e252a4daa4f6ec449778

Don't know why... Maybe I worked in a version of Pharo that created the commit metadata-less.

In summary, the CI error is independent from this PR. But I would like to know what to do with the error... revert the deletions?

tinchodias commented 6 years ago

Aha, I see Norbert already had to fix a similar issue with metadata in 36866185175175f8081b3040588e5ff240353c6a

tinchodias commented 6 years ago

@noha how did you re-create the metadata in https://github.com/pharo-nosql/mongotalk/commit/36866185175175f8081b3040588e5ff240353c6a?

I've just did commit (locally) with Iceberg from Pharo 6.1 (60540) and it doesn't create any metadata files.

However, it'd be great to work without all that metadata to simplify reviews and merges. Do we need to stop Pharo 4 support to achieve that? Is it viable?

tinchodias commented 6 years ago

I've found "[Pharo-users] Latest Voyage for Pharo4" thread from last year (17/02/2017), so it seems users could appreciate keeping support for Pharo 4.

tinchodias commented 6 years ago

well, I will merge it

tinchodias commented 6 years ago

For the record:

IMHO Mongo class shouldn't implement mongodb 2.6 and 3.0. Not sure what's the solution... Maybe it could implement Mongo as an abstract class and Mongo26, Mongo30, etc. as subclasses with the particular stuff. Maybe it's better to ensure master branch is a driver for latest mongodb version, and support for older mongodb versions are other branches.

tinchodias commented 6 years ago

I've just learnt something: there is a official specification for mongodb drivers, and this "#collections" stuff is defined:

https://github.com/mongodb/specifications/blob/55b2c2e08b22a1b43c02d0e18d1a44188f0b126c/source/enumerate-collections.rst#getting-collection-names

"Drivers MAY implement a method to enumerate all collections, and return only the collection names."

So I understand this fix is right.

tinchodias commented 6 years ago

I've found how to restore filetree metadata: by doing package-level commits in a Monticello filetree repo. Done in #31 .