From e44351275f925851a510660ab712fdfc8a0c9ce3 Mon Sep 17 00:00:00 2001 From: Alexandru Slobodcicov Date: Sun, 7 Feb 2021 19:17:27 +0200 Subject: [PATCH] Issue 90: Convert CreateIndex to execute as a runCommand --- .../change/CreateCollectionChange.java | 2 +- .../ext/mongodb/change/CreateIndexChange.java | 3 +- .../AbstractRunCommandStatement.java | 9 +++- .../ext/mongodb/statement/BsonUtils.java | 26 ++-------- .../statement/CreateIndexStatement.java | 52 ++++++++----------- .../statement/InsertManyStatement.java | 16 +++--- .../mongodb/statement/InsertOneStatement.java | 2 +- .../dbchangelog-ext.xsd | 2 +- .../java/liquibase/ext/mongodb/TestUtils.java | 12 ----- .../ext/mongodb/statement/BsonUtilsTest.java | 7 --- .../CreateCollectionStatementTest.java | 7 ++- .../statement/CreateIndexStatementIT.java | 35 +++++++++---- .../statement/InsertManyStatementIT.java | 9 ++-- .../statement/InsertOneStatementIT.java | 5 +- .../ext/changelog.implicit-rollback.test.xml | 5 +- 15 files changed, 83 insertions(+), 109 deletions(-) diff --git a/src/main/java/liquibase/ext/mongodb/change/CreateCollectionChange.java b/src/main/java/liquibase/ext/mongodb/change/CreateCollectionChange.java index abd2d5f0..c3e04a59 100644 --- a/src/main/java/liquibase/ext/mongodb/change/CreateCollectionChange.java +++ b/src/main/java/liquibase/ext/mongodb/change/CreateCollectionChange.java @@ -34,7 +34,7 @@ @DatabaseChange(name = "createCollection", description = "Create collection. Supports all options available: " + "https://docs.mongodb.com/manual/reference/method/db.createCollection/#db.createCollection\n" + - "https://docs.mongodb.com/manual/reference/method/db.runCommand/#db.runCommand", + "https://docs.mongodb.com/manual/reference/command/create/", priority = ChangeMetaData.PRIORITY_DEFAULT, appliesTo = "collection") @NoArgsConstructor @Getter diff --git a/src/main/java/liquibase/ext/mongodb/change/CreateIndexChange.java b/src/main/java/liquibase/ext/mongodb/change/CreateIndexChange.java index 94caf392..921f5ef3 100644 --- a/src/main/java/liquibase/ext/mongodb/change/CreateIndexChange.java +++ b/src/main/java/liquibase/ext/mongodb/change/CreateIndexChange.java @@ -32,7 +32,8 @@ @DatabaseChange(name = "createIndex", description = "Creates index for collection" + - "https://docs.mongodb.com/manual/reference/method/db.collection.createIndex/#db.collection.createIndex", + "https://docs.mongodb.com/manual/reference/method/db.collection.createIndex/#db.collection.createIndex\n" + + "https://docs.mongodb.com/manual/reference/command/createIndexes/", priority = ChangeMetaData.PRIORITY_DATABASE, appliesTo = "collection") @NoArgsConstructor @Getter diff --git a/src/main/java/liquibase/ext/mongodb/statement/AbstractRunCommandStatement.java b/src/main/java/liquibase/ext/mongodb/statement/AbstractRunCommandStatement.java index 41daeb5c..d1c3e2f5 100644 --- a/src/main/java/liquibase/ext/mongodb/statement/AbstractRunCommandStatement.java +++ b/src/main/java/liquibase/ext/mongodb/statement/AbstractRunCommandStatement.java @@ -39,6 +39,8 @@ public abstract class AbstractRunCommandStatement extends AbstractMongoStatement public static final String COMMAND_NAME = "runCommand"; public static final String SHELL_DB_PREFIX = "db."; + public static final String OK = "ok"; + public static final String WRITE_ERRORS = "writeErrors"; @Getter protected final Document command; @@ -66,8 +68,11 @@ public Document run(final MongoLiquibaseDatabase database) { * Check the response and throw an appropriate exception if the command was not successful */ protected void checkResponse(final Document responseDocument) throws MongoException { - final List writeErrors = responseDocument.getList(BsonUtils.WRITE_ERRORS, Document.class); - if (nonNull(writeErrors) && !writeErrors.isEmpty()) { + final Double ok = responseDocument.getDouble(OK); + final List writeErrors = responseDocument.getList(WRITE_ERRORS, Document.class); + + if (nonNull(ok) && !ok.equals(1.0d) + || nonNull(writeErrors) && !writeErrors.isEmpty()) { throw new MongoException("Command failed. The full response is " + responseDocument.toJson()); } } diff --git a/src/main/java/liquibase/ext/mongodb/statement/BsonUtils.java b/src/main/java/liquibase/ext/mongodb/statement/BsonUtils.java index b1fd7337..286dd68b 100644 --- a/src/main/java/liquibase/ext/mongodb/statement/BsonUtils.java +++ b/src/main/java/liquibase/ext/mongodb/statement/BsonUtils.java @@ -22,7 +22,6 @@ import com.mongodb.DBRefCodecProvider; import com.mongodb.MongoClientSettings; -import com.mongodb.client.model.IndexOptions; import lombok.NoArgsConstructor; import org.bson.Document; import org.bson.UuidRepresentation; @@ -37,7 +36,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.concurrent.TimeUnit; import static java.util.Objects.nonNull; import static java.util.Optional.ofNullable; @@ -48,9 +46,6 @@ @NoArgsConstructor(access = PRIVATE) public final class BsonUtils { - public static final String WRITE_ERRORS = "writeErrors"; - public static final String DOCUMENTS = "documents"; - public static final DocumentCodec DOCUMENT_CODEC = new DocumentCodec(fromProviders( new UuidCodecProvider(UuidRepresentation.STANDARD), @@ -59,6 +54,8 @@ public final class BsonUtils { new DocumentCodecProvider(), new DBRefCodecProvider())); + public static final String ITEMS = "items"; + public static CodecRegistry uuidCodecRegistry() { return CodecRegistries.fromRegistries( CodecRegistries.fromCodecs(new UuidCodec(UuidRepresentation.STANDARD)), @@ -77,28 +74,13 @@ public static Document orEmptyDocument(final String json) { public static List orEmptyList(final String json) { return ( ofNullable(trimToNull(json)) - .map(jn -> "{ items: " + jn + "}") + .map(jn -> "{ " + ITEMS + ": " + jn + "}") .map(s -> Document.parse(s, DOCUMENT_CODEC)) - .map(d -> d.getList("items", Document.class, new ArrayList<>())) + .map(d -> d.getList(ITEMS, Document.class, new ArrayList<>())) .orElseGet(ArrayList::new) ); } - public static IndexOptions orEmptyIndexOptions(final Document options) { - //TODO: add POJO codec - final IndexOptions indexOptions = new IndexOptions(); - if (options.containsKey("unique") && options.getBoolean("unique")) { - indexOptions.unique(true); - } - if (options.containsKey("name")) { - indexOptions.name(options.getString("name")); - } - if (options.containsKey("expireAfterSeconds")) { - indexOptions.expireAfter(options.getLong("expireAfterSeconds"), TimeUnit.SECONDS); - } - return indexOptions; - } - public static String toJson(final Document document) { return ofNullable(document).map(Document::toJson).orElse(null); } diff --git a/src/main/java/liquibase/ext/mongodb/statement/CreateIndexStatement.java b/src/main/java/liquibase/ext/mongodb/statement/CreateIndexStatement.java index 10821dd1..12ee1395 100644 --- a/src/main/java/liquibase/ext/mongodb/statement/CreateIndexStatement.java +++ b/src/main/java/liquibase/ext/mongodb/statement/CreateIndexStatement.java @@ -26,52 +26,46 @@ import lombok.Getter; import org.bson.Document; -import static java.util.Optional.ofNullable; -import static liquibase.ext.mongodb.statement.AbstractRunCommandStatement.SHELL_DB_PREFIX; +import static java.util.Collections.singletonList; +import static java.util.Objects.nonNull; import static liquibase.ext.mongodb.statement.BsonUtils.orEmptyDocument; +import static liquibase.ext.mongodb.statement.BsonUtils.toCommand; +/** + * Creates a index via the database runCommand method + * For a list of supported options see the reference page: + * @see createIndexes + */ @Getter @EqualsAndHashCode(callSuper = true) -public class CreateIndexStatement extends AbstractCollectionStatement +public class CreateIndexStatement extends AbstractRunCommandStatement implements NoSqlExecuteStatement { - public static final String COMMAND_NAME = "createIndex"; + public static final String RUN_COMMAND_NAME = "createIndexes"; + private static final String KEY = "key"; + private static final String INDEXES = "indexes"; - private final Document keys; - private final Document options; + @Override + public String getRunCommandName() { + return RUN_COMMAND_NAME; + } public CreateIndexStatement(final String collectionName, final Document keys, final Document options) { - super(collectionName); - this.keys = keys; - this.options = options; + super(toCommand(RUN_COMMAND_NAME, collectionName, combine(keys, options))); } public CreateIndexStatement(final String collectionName, final String keys, final String options) { this(collectionName, orEmptyDocument(keys), orEmptyDocument(options)); } - @Override - public String getCommandName() { - return COMMAND_NAME; - } + private static Document combine(final Document key, final Document options) { - @Override - public String toJs() { - return - SHELL_DB_PREFIX - + getCollectionName() - + ". " - + getCommandName() - + "(" - + ofNullable(keys).map(Document::toJson).orElse(null) - + ", " - + ofNullable(options).map(Document::toJson).orElse(null) - + ");"; - } + final Document indexDocument = new Document(KEY, key); + if (nonNull(options)) { + indexDocument.putAll(options); + } - @Override - public void execute(final MongoLiquibaseDatabase database) { - getMongoDatabase(database).getCollection(collectionName).createIndex(keys, BsonUtils.orEmptyIndexOptions(options)); + return new Document(INDEXES, singletonList(indexDocument)); } } diff --git a/src/main/java/liquibase/ext/mongodb/statement/InsertManyStatement.java b/src/main/java/liquibase/ext/mongodb/statement/InsertManyStatement.java index 5acd7518..8a9e8bdf 100644 --- a/src/main/java/liquibase/ext/mongodb/statement/InsertManyStatement.java +++ b/src/main/java/liquibase/ext/mongodb/statement/InsertManyStatement.java @@ -41,9 +41,12 @@ public class InsertManyStatement extends AbstractRunCommandStatement { public static final String RUN_COMMAND_NAME = "insert"; + public static final String DOCUMENTS = "documents"; - private final List documents; - private final Document options; + @Override + public String getRunCommandName() { + return RUN_COMMAND_NAME; + } public InsertManyStatement(final String collectionName, final String documents, final String options) { this(collectionName, new ArrayList<>(orEmptyList(documents)), orEmptyDocument(options)); @@ -51,21 +54,14 @@ public InsertManyStatement(final String collectionName, final String documents, public InsertManyStatement(final String collectionName, final List documents, final Document options) { super(BsonUtils.toCommand(RUN_COMMAND_NAME, collectionName, combine(documents, options))); - this.documents = documents; - this.options = options; } private static Document combine(final List documents, final Document options) { - final Document combined = new Document(BsonUtils.DOCUMENTS, documents); + final Document combined = new Document(DOCUMENTS, documents); if (nonNull(options)) { combined.putAll(options); } return combined; } - @Override - public String getRunCommandName() { - return RUN_COMMAND_NAME; - } - } diff --git a/src/main/java/liquibase/ext/mongodb/statement/InsertOneStatement.java b/src/main/java/liquibase/ext/mongodb/statement/InsertOneStatement.java index 5b3bd383..d8715394 100644 --- a/src/main/java/liquibase/ext/mongodb/statement/InsertOneStatement.java +++ b/src/main/java/liquibase/ext/mongodb/statement/InsertOneStatement.java @@ -30,7 +30,7 @@ /** * Inserts a document via the database runCommand method * For a list of supported options see the reference page: - * https://docs.mongodb.com/manual/reference/command/insert/ + * @see Insert */ @Getter @EqualsAndHashCode(callSuper = true) diff --git a/src/main/resources/liquibase.parser.core.xml/dbchangelog-ext.xsd b/src/main/resources/liquibase.parser.core.xml/dbchangelog-ext.xsd index cddf4330..c52dd6ec 100644 --- a/src/main/resources/liquibase.parser.core.xml/dbchangelog-ext.xsd +++ b/src/main/resources/liquibase.parser.core.xml/dbchangelog-ext.xsd @@ -65,7 +65,7 @@ - + diff --git a/src/test/java/liquibase/ext/mongodb/TestUtils.java b/src/test/java/liquibase/ext/mongodb/TestUtils.java index 7c85851e..9d22748c 100644 --- a/src/test/java/liquibase/ext/mongodb/TestUtils.java +++ b/src/test/java/liquibase/ext/mongodb/TestUtils.java @@ -101,16 +101,4 @@ public static List getChangesets(final String changeSetPath, final Mo parser.parse(changeSetPath, new ChangeLogParameters(database), resourceAccessor); return changeLog.getChangeSets(); } - - /** - * Helper to format and convert a string containing single quotes to one with double quotes. - * Use this to declare readable strings in tests - * @param singleQuoted a string containing single quotes - * @param args format args - * @return original string formatted with double quotes - */ - public static final String formatDoubleQuoted(final String singleQuoted, final String... args) { - String formatted = String.format(singleQuoted, args); - return formatted.replaceAll("'","\""); - } } diff --git a/src/test/java/liquibase/ext/mongodb/statement/BsonUtilsTest.java b/src/test/java/liquibase/ext/mongodb/statement/BsonUtilsTest.java index 4ca9dafd..a9327eb9 100644 --- a/src/test/java/liquibase/ext/mongodb/statement/BsonUtilsTest.java +++ b/src/test/java/liquibase/ext/mongodb/statement/BsonUtilsTest.java @@ -25,7 +25,6 @@ import org.junit.jupiter.api.Test; import java.util.UUID; -import java.util.concurrent.TimeUnit; import static java.util.stream.Collectors.toList; import static liquibase.ext.mongodb.statement.BsonUtils.DOCUMENT_CODEC; @@ -76,10 +75,4 @@ void uuidParseTest() { .isEqualTo(uuid1); } - @Test - void orEmptyIndexOptionsTest() { - assertThat(BsonUtils.orEmptyIndexOptions( - BsonUtils.orEmptyDocument("{expireAfterSeconds: NumberLong(\"60\")}")).getExpireAfter(TimeUnit.SECONDS)) - .isEqualTo(60L); - } } diff --git a/src/test/java/liquibase/ext/mongodb/statement/CreateCollectionStatementTest.java b/src/test/java/liquibase/ext/mongodb/statement/CreateCollectionStatementTest.java index 9d357bd1..db11913d 100644 --- a/src/test/java/liquibase/ext/mongodb/statement/CreateCollectionStatementTest.java +++ b/src/test/java/liquibase/ext/mongodb/statement/CreateCollectionStatementTest.java @@ -25,14 +25,13 @@ import static liquibase.ext.mongodb.TestUtils.COLLECTION_NAME_1; import static liquibase.ext.mongodb.TestUtils.EMPTY_OPTION; -import static liquibase.ext.mongodb.TestUtils.formatDoubleQuoted; import static org.assertj.core.api.Assertions.assertThat; class CreateCollectionStatementTest { // Some of the extra options that create collection supports - private static final String CREATE_OPTIONS = "'capped': true, 'size': 100, 'max': 200"; + private static final String CREATE_OPTIONS = "\"capped\": true, \"size\": 100, \"max\": 200"; private String collectionName; @@ -43,7 +42,7 @@ public void createCollectionName() { @Test void toStringJsWithoutOptions() { - String expected = formatDoubleQuoted("db.runCommand({'create': '%s'});", collectionName); + String expected = String.format("db.runCommand({\"create\": \"%s\"});", collectionName); final CreateCollectionStatement statement = new CreateCollectionStatement(collectionName, EMPTY_OPTION); assertThat(statement.toJs()) .isEqualTo(expected) @@ -53,7 +52,7 @@ void toStringJsWithoutOptions() { @Test void toStringJsWithOptions() { String options = String.format("{ %s }", CREATE_OPTIONS); - String expected = formatDoubleQuoted("db.runCommand({'create': '%s', %s});", collectionName, CREATE_OPTIONS); + String expected = String.format("db.runCommand({\"create\": \"%s\", %s});", collectionName, CREATE_OPTIONS); final CreateCollectionStatement statement = new CreateCollectionStatement(collectionName, options); assertThat(statement.toJs()) .isEqualTo(expected) diff --git a/src/test/java/liquibase/ext/mongodb/statement/CreateIndexStatementIT.java b/src/test/java/liquibase/ext/mongodb/statement/CreateIndexStatementIT.java index b572b46a..3285ee7a 100644 --- a/src/test/java/liquibase/ext/mongodb/statement/CreateIndexStatementIT.java +++ b/src/test/java/liquibase/ext/mongodb/statement/CreateIndexStatementIT.java @@ -20,6 +20,7 @@ * #L% */ +import com.mongodb.MongoException; import liquibase.ext.AbstractMongoIntegrationTest; import org.bson.Document; import org.junit.jupiter.api.Test; @@ -28,6 +29,7 @@ import static liquibase.ext.mongodb.TestUtils.COLLECTION_NAME_1; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; class CreateIndexStatementIT extends AbstractMongoIntegrationTest { @@ -35,10 +37,10 @@ class CreateIndexStatementIT extends AbstractMongoIntegrationTest { void toStringJs() { final String indexName = "locale_indx"; final CreateIndexStatement createIndexStatement = new CreateIndexStatement(COLLECTION_NAME_1, "{ locale: 1 }", - "{ name: \"" + indexName + "\", unique: true}"); + "{ name: \"" + indexName + "\", unique: true}"); assertThat(createIndexStatement.toString()) - .isEqualTo(createIndexStatement.toJs()) - .isEqualTo("db.collectionName. createIndex({\"locale\": 1}, {\"name\": \"locale_indx\", \"unique\": true});"); + .isEqualTo(createIndexStatement.toJs()) + .isEqualTo("db.runCommand({\"createIndexes\": \"collectionName\", \"indexes\": [{\"key\": {\"locale\": 1}, \"name\": \"locale_indx\", \"unique\": true}]});"); } @Test @@ -48,16 +50,29 @@ void execute() { mongoDatabase.createCollection(COLLECTION_NAME_1); mongoDatabase.getCollection(COLLECTION_NAME_1).insertOne(initialDocument); final CreateIndexStatement createIndexStatement = new CreateIndexStatement(COLLECTION_NAME_1, "{ locale: 1 }", - "{ name: \"" + indexName + "\", unique: true, expireAfterSeconds: NumberLong(\"30\") }"); + "{ name: \"" + indexName + "\", unique: true, expireAfterSeconds: NumberLong(\"30\") }"); createIndexStatement.execute(database); final Document document = StreamSupport.stream(mongoDatabase.getCollection(COLLECTION_NAME_1).listIndexes().spliterator(), false) - .filter(doc -> doc.get("name").equals(indexName)) - .findAny() - .orElseThrow(() -> new IllegalStateException("Index not found")); + .filter(doc -> doc.get("name").equals(indexName)) + .findAny() + .orElseThrow(() -> new IllegalStateException("Index not found")); - assertThat(document.get("unique")).isEqualTo(true); - assertThat(document.get("key")).isEqualTo(Document.parse("{ locale: 1 }")); - assertThat(document.get("expireAfterSeconds")).isEqualTo(30L); + assertThat(document.get("unique")).isEqualTo(true); + assertThat(document.get("key")).isEqualTo(Document.parse("{ locale: 1 }")); + assertThat(document.get("expireAfterSeconds")).isEqualTo(30L); + + // Same index name exception + final CreateIndexStatement createDuplicateNameIndexStatement = new CreateIndexStatement(COLLECTION_NAME_1, "{ otherField: 1 }", "{ name: \"" + indexName + "\" }"); + assertThatExceptionOfType(MongoException.class).isThrownBy(() -> createDuplicateNameIndexStatement.execute(database)) + .withMessageStartingWith("Command failed with error") + .withMessageContaining("Index must have unique name"); + + // Same index name exception + final CreateIndexStatement createSameFieldsIndexStatement = new CreateIndexStatement(COLLECTION_NAME_1, "{ locale: 1 }", "{ name: \"otherName\" }"); + assertThatExceptionOfType(MongoException.class).isThrownBy(() -> createSameFieldsIndexStatement.execute(database)) + .withMessageStartingWith("Command failed with error") + .withMessageContaining("Index") + .withMessageContaining("already exists with different options"); } } diff --git a/src/test/java/liquibase/ext/mongodb/statement/InsertManyStatementIT.java b/src/test/java/liquibase/ext/mongodb/statement/InsertManyStatementIT.java index 5eb5f97e..b5e1689d 100644 --- a/src/test/java/liquibase/ext/mongodb/statement/InsertManyStatementIT.java +++ b/src/test/java/liquibase/ext/mongodb/statement/InsertManyStatementIT.java @@ -33,7 +33,6 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import static liquibase.ext.mongodb.TestUtils.formatDoubleQuoted; import static liquibase.ext.mongodb.TestUtils.COLLECTION_NAME_1; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -50,10 +49,10 @@ public void createCollectionName() { @Test void toStringTest() { - String expected = formatDoubleQuoted( - "db.runCommand({'insert': '%s', " + - "'documents': [{'key1': 'value1'}, {'key1': 'value2'}], " + - "'ordered': false});", collectionName); + String expected = String.format( + "db.runCommand({\"insert\": \"%s\", " + + "\"documents\": [{\"key1\": \"value1\"}, {\"key1\": \"value2\"}], " + + "\"ordered\": false});", collectionName); final InsertManyStatement statement = new InsertManyStatement( collectionName, diff --git a/src/test/java/liquibase/ext/mongodb/statement/InsertOneStatementIT.java b/src/test/java/liquibase/ext/mongodb/statement/InsertOneStatementIT.java index fab2a680..ed70e829 100644 --- a/src/test/java/liquibase/ext/mongodb/statement/InsertOneStatementIT.java +++ b/src/test/java/liquibase/ext/mongodb/statement/InsertOneStatementIT.java @@ -23,7 +23,6 @@ import com.mongodb.MongoException; import com.mongodb.client.FindIterable; import liquibase.ext.AbstractMongoIntegrationTest; -import liquibase.ext.mongodb.TestUtils; import lombok.SneakyThrows; import org.bson.Document; import org.junit.jupiter.api.BeforeEach; @@ -71,8 +70,8 @@ void executeForString() { @Test @SneakyThrows void toStringJs() { - String expected = TestUtils.formatDoubleQuoted( - "db.runCommand({'insert': '%s', 'documents': [{'key1': 'value1'}], 'ordered': false});", collectionName); + String expected = String.format( + "db.runCommand({\"insert\": \"%s\", \"documents\": [{\"key1\": \"value1\"}], \"ordered\": false});", collectionName); final InsertOneStatement statement = new InsertOneStatement( collectionName, new Document("key1", "value1"), diff --git a/src/test/resources/liquibase/ext/changelog.implicit-rollback.test.xml b/src/test/resources/liquibase/ext/changelog.implicit-rollback.test.xml index c655650e..3f89870c 100644 --- a/src/test/resources/liquibase/ext/changelog.implicit-rollback.test.xml +++ b/src/test/resources/liquibase/ext/changelog.implicit-rollback.test.xml @@ -35,7 +35,7 @@ { id: 1, type: 1} - {unique: true, name: "ui_namedIndex"} + {unique: true, name: "ui_namedIndex1"} @@ -43,6 +43,9 @@ { id: 1, type: 1} + + {unique: true, name: "ui_namedIndex2"} +