-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add support for all available options for CreateCollectionChange #78
Conversation
Made AbstractCollectionStatement.toJs abstract and fixed CountCollectionByNameStatement implementation of it.
@@ -50,4 +50,9 @@ public long queryForLong(final MongoConnection connection) { | |||
.filter(s -> s.equals(getCollectionName())) | |||
.count(); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A correct implementation of toJs
for the count method.
Is needed since I made the baseclass method abstract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually counting collections not documents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix in a subsequent commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the misunderstanding. I see now how this is used and is different from CountDocumentsInCollectionStatement
.
This could only ever return zero or one, so could potentially be made clearer by refactoring it to implement a new interface such as NoSqlQueryForBooleanStatement
:
public boolean exists(MongoConnection connection);
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, could worth, it's just I tend to stay similar with Executor methods from liquibase core, otherwise can get into unsupported features when new Liquibase release.
commandOptions.putAll(options); | ||
} | ||
return commandOptions.toBsonDocument(Document.class, getDefaultCodecRegistry()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create:<collectionName>
needs to be the first entry in the Document, followed by any options if present.
…ents, changed to AbstractRunCommandStatement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will add a cosmetic commit to your PR. Thank you for you genius proposal
@@ -50,4 +50,9 @@ public long queryForLong(final MongoConnection connection) { | |||
.filter(s -> s.equals(getCollectionName())) | |||
.count(); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually counting collections not documents
@@ -50,4 +50,9 @@ public long queryForLong(final MongoConnection connection) { | |||
.filter(s -> s.equals(getCollectionName())) | |||
.count(); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix in a subsequent commit
import org.bson.codecs.UuidCodec; | ||
import org.bson.codecs.UuidCodecProvider; | ||
import org.bson.codecs.ValueCodecProvider; | ||
import org.bson.codecs.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand from a wildchar. Will fix in sub commit
|
||
import static java.util.Optional.ofNullable; | ||
import static com.mongodb.MongoClientSettings.getDefaultCodecRegistry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will move into BsonUtils in a subsequent commit
Changed
CreateCollectionStatement
to execute as a mongoDBrunCommand
so that it supports all available server options.Made
AbstractCollectionStatement.toJs()
method abstract and fixed theCountCollectionByNameStatement
implementation of it.┆Issue is synchronized with this Jira Bug by Unito