Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to Database from Connection in Statement Implementations #67

Closed
alexandru-slobodcicov opened this issue Dec 9, 2020 · 2 comments · Fixed by #93 or #94
Closed

Move to Database from Connection in Statement Implementations #67

alexandru-slobodcicov opened this issue Dec 9, 2020 · 2 comments · Fixed by #93 or #94

Comments

@alexandru-slobodcicov
Copy link

alexandru-slobodcicov commented Dec 9, 2020

In order to be closer to CustomChange implementations and have more generic Abstract Classes would be better to operate with Database Implementations instead of Connection Implementations passed to Statements/Executor etc.

┆Issue is synchronized with this Jira Bug by Unito

@jsonking
Copy link

@alexandru-slobodcicov I'm thinking that the Statement implementations could execute the runCommand operation instead of the higher level driver helper methods.
There are two principal benefits:

1: options can be specified in the migrations themselves and are passed through to the database directly. No need to manually add support for options that change over time.

2: Works with a larger set of underlying mongo-driver versions; increasing adoption of this library.

Example: liquibase.ext.mongodb.statement.InsertOneStatement
Current Code:

@Override
public void execute(final MongoConnection connection) {
    final MongoCollection<Document> collection = connection.getDatabase().getCollection(getCollectionName());
    collection.insertOne(document);
}

Note that the current code doesn't support any options!

Modified Code:

@Override
public void execute(final MongoConnection connection) {
    Bson bson = createCommand();
    connection.getDatabase().runCommand(bson);
}

private Bson createCommand() {
    Document command = new Document("insert", getCollectionName());
    command.put("documents", Collections.singletonList(document));
    command.putAll(options);
    return command.toBsonDocument(Document.class, getDefaultCodecRegistry());
}

This is the strategy I used in PR #78 to add support for all options in the CreateCollectionStatement class

Finally there are currently two open issues (#71 and #80) relating to supporting the latest driver version.

The signature of the runCommand methods in the driver has not changed between version 3.12.x and 4.x; but collection.insertOne is no longer void and therefore fails at runtime. i.e using runCommand is a more stable interface to use and would provide greater chances of forward-compatibility.

Keen to hear your thoughts?

Jason

@alexandru-slobodcicov
Copy link
Author

Hi @jsonking. Indeed worth changing where possible the Statemens to db.runCommand as you did for CreateCollectionStatement. Just want to mention that the initial approach was to stick to driver APIs until we identified that approximatively everything can be done from runCommand and adminCommand. The same is mentioned in README: A couple of Changes were implemented until identified that majority of the operations can be achieved using db.runCommand() and db.adminCommand(). So there is already the option to define runCommand and adminCommand in changesets with all the flexibility.

This particular issue is about having the Database instead of Connection in statements:
execute(final MongoConnection connection) -> execute(final MongoLiquibaseDatabase database)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants