refactor: general quality improvements (#13)

Author: dieppa

ANALYSIS.md

@@ -27,7 +27,7 @@ Structural validation runs at **load time** — before any change executes — v
 - `OperationValidator` — Interface for per-operation parameter validators + unrecognized key utility
 - 8 parameter validators (`InsertParametersValidator`, `UpdateParametersValidator`, `DeleteParametersValidator`, `CreateIndexParametersValidator`, `DropIndexParametersValidator`, `RenameCollectionParametersValidator`, `CreateViewParametersValidator`, `ModifyCollectionParametersValidator`)
 - 11 operator classes (`CreateCollectionOperator`, `InsertOperator`, etc.)
-- 6 mapper classes (`IndexOptionsMapper`, `InsertOptionsMapper`, `UpdateOptionsMapper`, `CreateViewOptionsMapper`, `RenameCollectionOptionsMapper`, `MapperUtil`)
+- 7 mapper classes (`IndexOptionsMapper`, `InsertOptionsMapper`, `UpdateOptionsMapper`, `CreateViewOptionsMapper`, `RenameCollectionOptionsMapper`, `MapperUtil`, `BsonConverter`)
 
 ---
 
@@ -227,11 +227,11 @@ All 7 changes from the original analysis have been implemented across multiple P
 - Unrecognized parameter key detection catches typos at load time
 
 ### Negative
-- Heavy code duplication in `InsertOperator` and `UpdateOperator` (4 branches for session x options combinations)
-- `MongoOperation` is a god object — it has getters for every operation type's parameters, even though each getter is only relevant to 1–2 operation types
+- ~~Heavy code duplication in `InsertOperator` and `UpdateOperator`~~ RESOLVED — Flattened nested if/else to single chain; extracted options to local variable to avoid duplicate mapper calls
+- ~~`MongoOperation` is a god object~~ REDUCED — Moved 9 single-use getters to their respective operator classes as private methods. `MongoOperation` retains 4 shared getters (`getOptions`, `getFilter`, `isMulti`, `getKeys`) used by multiple operators
 - No builder pattern or factory for `MongoOperation` in tests — all tests manually construct via setters
-- `MapperUtil` mixes concerns: type extraction + BSON conversion + Collation building in one class
-- Test infrastructure duplication — every integration test class independently sets up MongoDBContainer with identical boilerplate
+- ~~`MapperUtil` mixes concerns: type extraction + BSON conversion + Collation building~~ RESOLVED — Split into `MapperUtil` (parameter extraction + collation) and `BsonConverter` (BSON serialization)
+- ~~Test infrastructure duplication~~ RESOLVED — Extracted `AbstractMongoOperatorTest` base class with shared MongoDBContainer setup; 12 test classes now extend it
 
 ---
 
@@ -244,8 +244,8 @@ All 7 changes from the original analysis have been implemented across multiple P
 | Validation & Error Handling    |   20%    |      9       |     1.80     |
 | Test Coverage                  |   20%    |      6       |     1.20     |
 | Security & Safety              |   10%    |      8       |     0.80     |
-| Code Quality & Maintainability |    5%    |      7       |     0.35     |
-| **Total**                      | **100%** |              | **8.2 / 10** |
+| Code Quality & Maintainability |    5%    |      8       |     0.40     |
+| **Total**                      | **100%** |              | **8.3 / 10** |
 
 ### Score Justification
 
@@ -259,8 +259,8 @@ All 7 changes from the original analysis have been implemented across multiple P
 
 **Security (8/10):** Developer-authored context makes injection-style concerns not applicable. Collection name `$`/`\0` checks serve as guardrails, now applied to both `collection` and `target` parameters. `modifyCollection` parameters are validated against known values. YAML deserialization is delegated to the framework.
 
-**Code Quality (7/10):** Clean code style, good naming, proper license headers. Deductions remain for code duplication in operators (session x options branches), the `MongoOperation` god-object pattern, and test infrastructure boilerplate. These are maintainability concerns, not correctness issues.
+**Code Quality (8/10):** Clean code style, good naming, proper license headers. Four of five code quality negatives have been addressed: operator duplication flattened, `MongoOperation` god-object reduced (9 getters moved to operators), `MapperUtil` split into extraction + BSON conversion, and test boilerplate extracted to `AbstractMongoOperatorTest`. Remaining deduction: no builder/factory for `MongoOperation` in tests, and `MongoOperation` still has 4 shared getters.
 
 ### Bottom Line
 
-The module has **matured significantly** from its initial state. All 10 identified issues have been resolved, the validation architecture was overhauled to leverage framework-level load-time validation, and the test suite grew from ~90 to ~199 tests. The remaining gaps are in integration-level testing (transactional paths, options end-to-end) and code-level cleanup (operator duplication, god-object pattern). The module is **ready for production use** with the understanding that transactional behavior is covered by the framework's integration test suite.
+The module has **matured significantly** from its initial state. All 10 identified issues have been resolved, the validation architecture was overhauled to leverage framework-level load-time validation, and the test suite grew from ~90 to ~199 tests. Code quality improvements addressed 4 of 5 negatives: operator duplication flattened, god-object reduced, `MapperUtil` concerns separated, and test boilerplate extracted. The remaining gaps are in integration-level testing (transactional paths, options end-to-end). The module is **ready for production use** with the understanding that transactional behavior is covered by the framework's integration test suite.

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

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2025 Flamingock (https://www.flamingock.io)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.flamingock.template.mongodb.mapper;
+
+import org.bson.BsonArray;
+import org.bson.BsonDocument;
+import org.bson.BsonValue;
+
+import java.util.List;
+import java.util.Map;
+
+public final class BsonConverter {
+    private BsonConverter() {}
+
+    public static BsonDocument toBsonDocument(Map<String, Object> map) {
+        BsonDocument document = new BsonDocument();
+        map.forEach((key, value) -> document.append(key, toBsonValue(value)));
+        return document;
+    }
+
+    @SuppressWarnings("unchecked")
+    public static BsonValue toBsonValue(Object value) {
+        if (value == null) {
+            return org.bson.BsonNull.VALUE;
+        } else if (value instanceof String) {
+            return new org.bson.BsonString((String) value);
+        } else if (value instanceof Integer) {
+            return new org.bson.BsonInt32((Integer) value);
+        } else if (value instanceof Long) {
+            return new org.bson.BsonInt64((Long) value);
+        } else if (value instanceof Double) {
+            return new org.bson.BsonDouble((Double) value);
+        } else if (value instanceof Boolean) {
+            return new org.bson.BsonBoolean((Boolean) value);
+        } else if (value instanceof List) {
+            return toBsonArray((List<?>) value);
+        } else if (value instanceof Map) {
+            return toBsonDocument((Map<String, Object>) value);
+        }
+        throw new IllegalArgumentException("Unsupported BSON type: " + value.getClass().getSimpleName());
+    }
+
+    public static BsonArray toBsonArray(List<?> list) {
+        BsonArray array = new BsonArray();
+        for (Object item : list) {
+            array.add(toBsonValue(item));
+        }
+        return array;
+    }
+}

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

@@ -20,12 +20,8 @@
 import com.mongodb.client.model.CollationCaseFirst;
 import com.mongodb.client.model.CollationMaxVariable;
 import com.mongodb.client.model.CollationStrength;
-import org.bson.BsonArray;
-import org.bson.BsonDocument;
-import org.bson.BsonValue;
 import org.bson.conversions.Bson;
 
-import java.util.List;
 import java.util.Map;
 
 public final class MapperUtil {
@@ -82,7 +78,7 @@ public static Bson getBson(Map<String, Object> options, String key) {
         if (value instanceof Bson) {
             return (Bson) value;
         } else if (value instanceof Map) {
-            return toBsonDocument((Map<String, Object>) value);
+            return BsonConverter.toBsonDocument((Map<String, Object>) value);
         } else {
             throw new IllegalArgumentException(String.format("field[%s] should be Bson", key));
         }
@@ -135,42 +131,4 @@ private static Collation buildCollationFromMap(Map<String, Object> map) {
         return builder.build();
     }
 
-    // Recursively converts a Map<String, Object> to BsonDocument
-    public static BsonDocument toBsonDocument(Map<String, Object> map) {
-        BsonDocument document = new BsonDocument();
-        map.forEach((key, value) -> document.append(key, toBsonValue(value)));
-        return document;
-    }
-
-    // Converts Java types into BSON types
-    @SuppressWarnings("unchecked")
-    public static BsonValue toBsonValue(Object value) {
-        if (value == null) {
-            return org.bson.BsonNull.VALUE;
-        } else if (value instanceof String) {
-            return new org.bson.BsonString((String) value);
-        } else if (value instanceof Integer) {
-            return new org.bson.BsonInt32((Integer) value);
-        } else if (value instanceof Long) {
-            return new org.bson.BsonInt64((Long) value);
-        } else if (value instanceof Double) {
-            return new org.bson.BsonDouble((Double) value);
-        } else if (value instanceof Boolean) {
-            return new org.bson.BsonBoolean((Boolean) value);
-        } else if (value instanceof List) {
-            return toBsonArray((List<?>) value);
-        } else if (value instanceof Map) {
-            return toBsonDocument((Map<String, Object>) value);
-        }
-        throw new IllegalArgumentException("Unsupported BSON type: " + value.getClass().getSimpleName());
-    }
-
-    // Converts a List to BsonArray
-    public static BsonArray toBsonArray(List<?> list) {
-        BsonArray array = new BsonArray();
-        for (Object item : list) {
-            array.add(toBsonValue(item));
-        }
-        return array;
-    }
 }

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

@@ -49,7 +49,7 @@ public static UpdateOptions mapToUpdateOptions(Map<String, Object> options) {
                     if (filter instanceof Map) {
                         @SuppressWarnings("unchecked")
                         Map<String, Object> filterMap = (Map<String, Object>) filter;
-                        bsonFilters.add(MapperUtil.toBsonDocument(filterMap));
+                        bsonFilters.add(BsonConverter.toBsonDocument(filterMap));
                     } else if (filter instanceof org.bson.conversions.Bson) {
                         bsonFilters.add((org.bson.conversions.Bson) filter);
                     }

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

@@ -28,7 +28,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 @NonLockGuarded(NonLockGuardedType.NONE)
 public class MongoOperation implements TemplatePayload {
@@ -48,20 +47,11 @@ public class MongoOperation implements TemplatePayload {
 
     public void setParameters(Map<String, Object> parameters) { this.parameters = parameters; }
 
-    @SuppressWarnings("unchecked")
-    public List<Document> getDocuments() {
-        return ((List<Map<String, Object>>) parameters.get("documents"))
-                .stream().map(Document::new)
-                .collect(Collectors.toList());
-    }
-
     @SuppressWarnings("unchecked")
     public Document getKeys() {
         return new Document((Map<String, Object>) parameters.get("keys"));
     }
 
-
-
     @SuppressWarnings("unchecked")
     public Document getOptions() {
         return parameters.containsKey("options")
@@ -74,46 +64,6 @@ public Document getFilter() {
         return new Document((Map<String, Object>) parameters.get("filter"));
     }
 
-    public String getIndexName() {
-        Object value = parameters.get("indexName");
-        return value != null ? (String) value : null;
-    }
-
-    public String getTarget() {
-        return (String) parameters.get("target");
-    }
-
-    @SuppressWarnings("unchecked")
-    public Document getValidator() {
-        Object value = parameters.get("validator");
-        return value != null ? new Document((Map<String, Object>) value) : null;
-    }
-
-    public String getValidationLevel() {
-        return (String) parameters.get("validationLevel");
-    }
-
-    public String getValidationAction() {
-        return (String) parameters.get("validationAction");
-    }
-
-    public String getViewOn() {
-        return (String) parameters.get("viewOn");
-    }
-
-    @SuppressWarnings("unchecked")
-    public List<Document> getPipeline() {
-        List<Map<String, Object>> rawPipeline = (List<Map<String, Object>>) parameters.get("pipeline");
-        return rawPipeline != null
-                ? rawPipeline.stream().map(Document::new).collect(Collectors.toList())
-                : null;
-    }
-
-    @SuppressWarnings("unchecked")
-    public Document getUpdate() {
-        return new Document((Map<String, Object>) parameters.get("update"));
-    }
-
     public boolean isMulti() {
         Object multi = parameters.get("multi");
         return multi != null && (Boolean) multi;

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

@@ -20,6 +20,11 @@
 import com.mongodb.client.model.CreateViewOptions;
 import io.flamingock.template.mongodb.mapper.CreateViewOptionsMapper;
 import io.flamingock.template.mongodb.model.MongoOperation;
+import org.bson.Document;
+
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 public class CreateViewOperator extends MongoOperator {
 
@@ -30,6 +35,18 @@ public CreateViewOperator(MongoDatabase mongoDatabase, MongoOperation operation)
     @Override
     protected void applyInternal(ClientSession clientSession) {
         CreateViewOptions options = CreateViewOptionsMapper.map(op.getOptions());
-        mongoDatabase.createView(op.getCollection(), op.getViewOn(), op.getPipeline(), options);
+        mongoDatabase.createView(op.getCollection(), getViewOn(), getPipeline(), options);
+    }
+
+    private String getViewOn() {
+        return (String) op.getParameters().get("viewOn");
+    }
+
+    @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;
     }
 }

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

@@ -27,11 +27,16 @@ public DropIndexOperator(MongoDatabase mongoDatabase, MongoOperation operation)
 
     @Override
     protected void applyInternal(ClientSession clientSession) {
-        String indexName = op.getIndexName();
+        String indexName = getIndexName();
         if (indexName != null) {
             mongoDatabase.getCollection(op.getCollection()).dropIndex(indexName);
         } else {
             mongoDatabase.getCollection(op.getCollection()).dropIndex(op.getKeys());
         }
     }
+
+    private String getIndexName() {
+        Object value = op.getParameters().get("indexName");
+        return value != null ? (String) value : null;
+    }
 }