Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions ANALYSIS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

---

Expand Down Expand Up @@ -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

---

Expand All @@ -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

Expand All @@ -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.
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Loading
Loading