Skip to content

Commit e84cfe0

Browse files
authored
fix: remove silent guard in InsertOperator for null/empty documents (#7)
1 parent 4314528 commit e84cfe0

4 files changed

Lines changed: 29 additions & 13 deletions

File tree

ANALYSIS.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ YAML -> MongoOperation (deserialized) -> MongoOperationValidator -> MongoOperati
3333
**Original issue:** `MongoChangeTemplate.rollback()` did not call `MongoOperationValidator.validate()` before executing, so malformed rollback YAML ran unchecked.
3434
**Resolution:** Structural validation was moved into `MongoOperation.validate()` (implementing the `TemplatePayload` interface). The Flamingock framework now calls `validate()` on both apply and rollback payloads at load time via `AbstractTemplateLoadedChange.getValidationErrors()`, which invokes `validateApplyPayload()` and `validateRollbackPayload()` before any change executes. The separate `MongoOperationValidator`, `ValidationError`, and `MongoTemplateValidationException` classes were deleted as part of this refactoring. Validation is no longer a responsibility of the template's `apply()`/`rollback()` methods — the framework handles it uniformly for all payloads.
3535

36-
### #2 - HIGH: InsertOperator silently swallows null/empty documents, bypassing validation
37-
**File:** `InsertOperator.java:37-39`
38-
**Impact:** The validator correctly rejects empty/null documents (`MongoOperationValidator.java:268-271`), but `InsertOperator.applyInternal()` has a redundant guard: `if(op.getDocuments() == null || op.getDocuments().isEmpty()) { return; }`. If the validator is ever bypassed (e.g., rollback path per issue #1, or direct operator usage), the insert silently does nothing. A migration that is supposed to seed data will silently succeed without inserting anything - an invisible data loss scenario.
39-
**Fix:** Remove the silent return. Let the operator trust the validator. If documents are null/empty at this point, it's a bug that should throw, not be silenced.
36+
### #2 - ~~HIGH: InsertOperator silently swallows null/empty documents, bypassing validation~~ RESOLVED
37+
**Status:** Fixed by removing the silent guard.
38+
**Original issue:** `InsertOperator.applyInternal()` had a redundant guard `if(op.getDocuments() == null || op.getDocuments().isEmpty()) { return; }` that silently did nothing instead of failing when documents were missing.
39+
**Resolution:** The silent guard was removed from `InsertOperator.applyInternal()`. Structural validation now runs at load time via `InsertParametersValidator` (called from `MongoOperation.validate()`), which catches null, empty, and malformed documents before any change executes. The operator no longer needs a defensive check — if documents are invalid, the framework rejects the change at load time.
4040

4141
### #3 - HIGH: `modifyCollection` has ZERO parameter validation
4242
**File:** `MongoOperationValidator.java:239-240`

src/main/java/io/flamingock/template/mongodb/MongoChangeTemplate.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,26 @@ public MongoChangeTemplate() {
8181
super(MongoOperation.class);
8282
}
8383

84+
/**
85+
* Executes the apply operation for the current step.
86+
* <p>
87+
* Precondition: {@code applyPayload} has been structurally validated at load time
88+
* via {@link MongoOperation#validate()}, which delegates to the appropriate
89+
* {@code ParametersValidator} for each operation type. No validation is performed here.
90+
*/
8491
@Apply
8592
public void apply(MongoDatabase db, @Nullable ClientSession clientSession) {
8693
validateSession(clientSession);
8794
applyPayload.getOperator(db).apply(clientSession);
8895
}
8996

97+
/**
98+
* Executes the rollback operation for the current step.
99+
* <p>
100+
* Precondition: {@code rollbackPayload} has been structurally validated at load time
101+
* via {@link MongoOperation#validate()}, which delegates to the appropriate
102+
* {@code ParametersValidator} for each operation type. No validation is performed here.
103+
*/
90104
@Rollback
91105
public void rollback(MongoDatabase db, @Nullable ClientSession clientSession) {
92106
validateSession(clientSession);

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,15 @@ public InsertOperator(MongoDatabase mongoDatabase, MongoOperation operation) {
3131
super(mongoDatabase, operation, true);
3232
}
3333

34+
/**
35+
* Executes the insert operation against MongoDB.
36+
* <p>
37+
* Precondition: {@code op.getDocuments()} is guaranteed non-null and non-empty
38+
* by {@code InsertParametersValidator} at load time, so no defensive check is needed here.
39+
*/
3440
@Override
3541
protected void applyInternal(ClientSession clientSession) {
3642
MongoCollection<Document> collection = mongoDatabase.getCollection(op.getCollection());
37-
if(op.getDocuments() == null || op.getDocuments().isEmpty()) {
38-
return;
39-
}
4043

4144
if(op.getDocuments().size() == 1) {
4245
insertOne(clientSession, collection);

src/test/java/io/flamingock/template/mongodb/operations/InsertOperatorTest.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.flamingock.template.mongodb.model.MongoOperation;
2424
import io.flamingock.template.mongodb.model.operator.InsertOperator;
2525
import org.bson.Document;
26+
import org.junit.jupiter.api.Assertions;
2627
import org.junit.jupiter.api.BeforeAll;
2728
import org.junit.jupiter.api.BeforeEach;
2829
import org.junit.jupiter.api.DisplayName;
@@ -144,10 +145,8 @@ void insertManyDocumentsTest() {
144145
}
145146

146147
@Test
147-
@DisplayName("WHEN insert operator is applied with empty documents THEN nothing is inserted")
148+
@DisplayName("WHEN insert operator is applied with empty documents THEN exception is thrown")
148149
void insertEmptyDocumentsTest() {
149-
assertEquals(0, getDocumentCount(), "Collection should be empty before insert");
150-
151150
MongoOperation operation = new MongoOperation();
152151
operation.setType("insert");
153152
operation.setCollection(COLLECTION_NAME);
@@ -157,9 +156,9 @@ void insertEmptyDocumentsTest() {
157156
operation.setParameters(params);
158157

159158
InsertOperator operator = new InsertOperator(mongoDatabase, operation);
160-
operator.apply(null);
161-
162-
assertEquals(0, getDocumentCount(), "Collection should still be empty");
159+
// Empty documents should be caught by InsertParametersValidator at load time.
160+
// If the operator is called directly with empty documents, the MongoDB driver throws.
161+
Assertions.assertThrows(IllegalArgumentException.class, () -> operator.apply(null));
163162
}
164163

165164
private long getDocumentCount() {

0 commit comments

Comments
 (0)