refactor: improvements and issues (#22)

Author: davidfrigolet

ANALYSIS.md

@@ -14,14 +14,15 @@
  * limitations under the License.
  */
 package io.flamingock.template.mongodb.mapper;
-import com.mongodb.client.model.IndexOptions;
 
+import com.mongodb.client.model.IndexOptions;
 
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import static io.flamingock.template.mongodb.mapper.MapperUtil.getBoolean;
 import static io.flamingock.template.mongodb.mapper.MapperUtil.getBson;
@@ -31,17 +32,19 @@
 import static io.flamingock.template.mongodb.mapper.MapperUtil.getLong;
 import static io.flamingock.template.mongodb.mapper.MapperUtil.getString;
 
-import java.util.concurrent.TimeUnit;
-
 
 public final class IndexOptionsMapper {
 
     public static final Set<String> RECOGNIZED_KEYS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
             "background", "unique", "name", "sparse", "expireAfterSeconds",
             "version", "weights", "defaultLanguage", "languageOverride",
             "textVersion", "sphereVersion", "bits", "min", "max",
-            "bucketSize", "storageEngine", "partialFilterExpression",
-            "collation", "wildcardProjection", "hidden"
+            "storageEngine", "partialFilterExpression", "collation"
+    )));
+
+    /** Options removed from the MongoDB Java driver: rejected at validation time with a clear error. */
+    public static final Set<String> UNSUPPORTED_KEYS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
+            "bucketSize", "wildcardProjection", "hidden"
     )));
 
     private IndexOptionsMapper() {}
@@ -91,9 +94,6 @@ public static IndexOptions mapToIndexOptions(Map<String, Object> options) {
         if (options.containsKey("max")) {
             indexOptions.max(getDouble(options, "max"));
         }
-        if (options.containsKey("bucketSize")) {
-            throw new UnsupportedOperationException("bulkSize option is not supported in MongoDB driver versions 4.4.0 and above");
-        }
         if (options.containsKey("storageEngine")) {
             indexOptions.storageEngine(getBson(options, "storageEngine"));
         }
@@ -103,12 +103,6 @@ public static IndexOptions mapToIndexOptions(Map<String, Object> options) {
         if (options.containsKey("collation")) {
             indexOptions.collation(getCollation(options, "collation"));
         }
-        if (options.containsKey("wildcardProjection")) {
-            throw new UnsupportedOperationException("wildcardProjection option is not supported in MongoDB driver versions 4.1.0 and above");
-        }
-        if (options.containsKey("hidden")) {
-            throw new UnsupportedOperationException("hidden option is not supported in MongoDB driver versions 4.1.0 and above");
-        }
 
         return indexOptions;
     }

src/main/java/io/flamingock/template/mongodb/mapper/IndexOptionsMapper.java

@@ -105,28 +105,28 @@ private static Collation buildCollationFromMap(Map<String, Object> map) {
         Collation.Builder builder = Collation.builder().locale((String) locale);
 
         if (map.containsKey("caseLevel")) {
-            builder.caseLevel((Boolean) map.get("caseLevel"));
+            builder.caseLevel(getBoolean(map, "caseLevel"));
         }
         if (map.containsKey("caseFirst")) {
-            builder.collationCaseFirst(CollationCaseFirst.fromString((String) map.get("caseFirst")));
+            builder.collationCaseFirst(CollationCaseFirst.fromString(getString(map, "caseFirst")));
         }
         if (map.containsKey("strength")) {
-            builder.collationStrength(CollationStrength.fromInt(((Number) map.get("strength")).intValue()));
+            builder.collationStrength(CollationStrength.fromInt(getInteger(map, "strength")));
         }
         if (map.containsKey("numericOrdering")) {
-            builder.numericOrdering((Boolean) map.get("numericOrdering"));
+            builder.numericOrdering(getBoolean(map, "numericOrdering"));
         }
         if (map.containsKey("alternate")) {
-            builder.collationAlternate(CollationAlternate.fromString((String) map.get("alternate")));
+            builder.collationAlternate(CollationAlternate.fromString(getString(map, "alternate")));
         }
         if (map.containsKey("maxVariable")) {
-            builder.collationMaxVariable(CollationMaxVariable.fromString((String) map.get("maxVariable")));
+            builder.collationMaxVariable(CollationMaxVariable.fromString(getString(map, "maxVariable")));
         }
         if (map.containsKey("normalization")) {
-            builder.normalization((Boolean) map.get("normalization"));
+            builder.normalization(getBoolean(map, "normalization"));
         }
         if (map.containsKey("backwards")) {
-            builder.backwards((Boolean) map.get("backwards"));
+            builder.backwards(getBoolean(map, "backwards"));
         }
         return builder.build();
     }

src/main/java/io/flamingock/template/mongodb/mapper/MapperUtil.java

@@ -52,23 +52,43 @@ public class MongoOperation implements TemplatePayload {
 
     @SuppressWarnings("unchecked")
     public Document getKeys() {
-        return new Document((Map<String, Object>) parameters.get("keys"));
+        Object value = parameters.get("keys");
+        if (!(value instanceof Map)) {
+            throw new IllegalStateException(
+                    "getKeys() called but 'keys' is not a Map — validate() must run before operator execution");
+        }
+        return new Document((Map<String, Object>) value);
     }
 
     @SuppressWarnings("unchecked")
     public Document getOptions() {
-        return parameters.containsKey("options")
-                ? new Document((Map<String, Object>) parameters.get("options"))
-                : new Document();
+        Object value = parameters.get("options");
+        if (value == null) {
+            return new Document();
+        }
+        if (!(value instanceof Map)) {
+            throw new IllegalStateException(
+                    "getOptions() called but 'options' is not a Map — validate() must run before operator execution");
+        }
+        return new Document((Map<String, Object>) value);
     }
 
     @SuppressWarnings("unchecked")
     public Document getFilter() {
-        return new Document((Map<String, Object>) parameters.get("filter"));
+        Object value = parameters.get("filter");
+        if (!(value instanceof Map)) {
+            throw new IllegalStateException(
+                    "getFilter() called but 'filter' is not a Map — validate() must run before operator execution");
+        }
+        return new Document((Map<String, Object>) value);
     }
 
     public boolean isMulti() {
         Object multi = parameters.get("multi");
+        if (multi != null && !(multi instanceof Boolean)) {
+            throw new IllegalStateException(
+                    "isMulti() called but 'multi' is not a Boolean — validate() must run before operator execution");
+        }
         return multi != null && (Boolean) multi;
     }
 

src/main/java/io/flamingock/template/mongodb/model/MongoOperation.java

@@ -39,7 +39,9 @@
 import io.flamingock.template.mongodb.validation.RenameCollectionParametersValidator;
 import io.flamingock.template.mongodb.validation.UpdateParametersValidator;
 
-import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.BiFunction;
 
@@ -57,6 +59,15 @@ public enum MongoOperationType {
     CREATE_VIEW("createView", CreateViewOperator::new, new CreateViewParametersValidator(), false),
     DROP_VIEW("dropView", DropViewOperator::new, new NoParametersValidator("DropView"), false);
 
+    private static final Map<String, MongoOperationType> LOOKUP;
+    static {
+        Map<String, MongoOperationType> map = new HashMap<>();
+        for (MongoOperationType t : values()) {
+            map.put(t.value, t);
+        }
+        LOOKUP = Collections.unmodifiableMap(map);
+    }
+
     private final String value;
     private final BiFunction<MongoDatabase, MongoOperation, MongoOperator> createOperatorFunction;
     private final OperationValidator operationValidator;
@@ -73,16 +84,15 @@ public enum MongoOperationType {
     }
 
     public static MongoOperationType findByTypeOrThrow(String typeValue) {
-        return Arrays.stream(MongoOperationType.values())
-                .filter(type -> type.matches(typeValue))
-                .findFirst()
-                .orElseThrow(() -> new IllegalArgumentException("MongoOperation not supported: " + typeValue));
+        MongoOperationType found = LOOKUP.get(typeValue);
+        if (found == null) {
+            throw new IllegalArgumentException("MongoOperation not supported: " + typeValue);
+        }
+        return found;
     }
 
     public static Optional<MongoOperationType> findByType(String typeValue) {
-        return Arrays.stream(MongoOperationType.values())
-                .filter(type -> type.matches(typeValue))
-                .findFirst();
+        return Optional.ofNullable(LOOKUP.get(typeValue));
     }
 
     public MongoOperator getOperator(MongoDatabase mongoDatabase, MongoOperation operation) {
@@ -96,8 +106,4 @@ public OperationValidator getOperationValidator() {
     public boolean isTransactional() {
         return transactional;
     }
-
-    private boolean matches(String operationType) {
-        return this.value.equals(operationType);
-    }
 }

src/main/java/io/flamingock/template/mongodb/model/MongoOperationType.java

@@ -48,9 +48,12 @@ private String getViewOn() {
 
     @SuppressWarnings("unchecked")
     private List<Document> getPipeline() {
-        List<Map<String, Object>> rawPipeline = (List<Map<String, Object>>) op.getParameters().get("pipeline");
-        return rawPipeline != null
-                ? rawPipeline.stream().map(Document::new).collect(Collectors.toList())
-                : null;
+        Object value = op.getParameters().get("pipeline");
+        if (!(value instanceof List)) {
+            throw new IllegalStateException(
+                    "getPipeline() called but 'pipeline' is not a List — validate() must run before operator execution");
+        }
+        List<Map<String, Object>> rawPipeline = (List<Map<String, Object>>) value;
+        return rawPipeline.stream().map(Document::new).collect(Collectors.toList());
     }
 }

src/main/java/io/flamingock/template/mongodb/model/operator/CreateViewOperator.java

@@ -39,6 +39,12 @@ protected void applyInternal(ClientSession clientSession) {
             return;
         }
 
+        if (!sourceExists) {
+            logger.warn("Source collection '{}' does not exist and target '{}' is also absent — skipping renameCollection",
+                    op.getCollection(), targetName);
+            return;
+        }
+
         MongoNamespace target = new MongoNamespace(mongoDatabase.getName(), targetName);
         RenameCollectionOptions options = RenameCollectionOptionsMapper.map(op.getOptions());
         mongoDatabase.getCollection(op.getCollection()).renameCollection(target, options);

src/main/java/io/flamingock/template/mongodb/model/operator/RenameCollectionOperator.java

@@ -39,6 +39,9 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
         } else if (collection.contains("\0")) {
             return Collections.singletonList(
                     new TemplatePayloadValidationError("collection", "Collection name cannot contain null character"));
+        } else if (collection.startsWith("system.")) {
+            return Collections.singletonList(
+                    new TemplatePayloadValidationError("collection", "Collection name cannot start with 'system.': " + collection));
         }
 
         return Collections.emptyList();

src/main/java/io/flamingock/template/mongodb/validation/CollectionValidator.java

@@ -21,15 +21,30 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import static java.lang.String.format;
+
 public class CreateIndexParametersValidator implements OperationValidator {
 
     private static final Set<String> RECOGNIZED_KEYS = new HashSet<>(Arrays.asList("keys", "options"));
 
+    /**
+     * Union of RECOGNIZED_KEYS and UNSUPPORTED_KEYS from the mapper. Used by the unrecognized-key
+     * check so that unsupported keys only generate their specific "not supported" error and do not
+     * also trigger the generic "unrecognized option" error.
+     */
+    private static final Set<String> ALL_KNOWN_OPTION_KEYS;
+    static {
+        Set<String> combined = new HashSet<>(IndexOptionsMapper.RECOGNIZED_KEYS);
+        combined.addAll(IndexOptionsMapper.UNSUPPORTED_KEYS);
+        ALL_KNOWN_OPTION_KEYS = Collections.unmodifiableSet(combined);
+    }
+
     @Override
     public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
         List<TemplatePayloadValidationError> errors = new ArrayList<>();
@@ -66,8 +81,16 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
         } else if (options instanceof Map) {
             @SuppressWarnings("unchecked")
             Map<String, Object> optionsMap = (Map<String, Object>) options;
+            for (String unsupported : IndexOptionsMapper.UNSUPPORTED_KEYS) {
+                if (optionsMap.containsKey(unsupported)) {
+                    errors.add(new TemplatePayloadValidationError(
+                            "parameters.options." + unsupported,
+                            format("'%s' is not supported — removed from the MongoDB Java driver 4.x+", unsupported)));
+                }
+            }
+            errors.addAll(OperationValidator.checkNullOptionValues(optionsMap, "CreateIndex"));
             errors.addAll(OperationValidator.checkUnrecognizedOptionKeys(
-                    optionsMap, IndexOptionsMapper.RECOGNIZED_KEYS, "CreateIndex"));
+                    optionsMap, ALL_KNOWN_OPTION_KEYS, "CreateIndex"));
         }
 
         errors.addAll(OperationValidator.checkUnrecognizedKeys(params, RECOGNIZED_KEYS, "CreateIndex"));

src/main/java/io/flamingock/template/mongodb/validation/CreateIndexParametersValidator.java

@@ -83,6 +83,7 @@ public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
         } else if (options instanceof Map) {
             @SuppressWarnings("unchecked")
             Map<String, Object> optionsMap = (Map<String, Object>) options;
+            errors.addAll(OperationValidator.checkNullOptionValues(optionsMap, "CreateView"));
             errors.addAll(OperationValidator.checkUnrecognizedOptionKeys(
                     optionsMap, CreateViewOptionsMapper.RECOGNIZED_KEYS, "CreateView"));
         }