Skip to content

Commit 6538255

Browse files
dieppadieppa
authored
fix/config validation and issue 3 (#8)
* refactor: add TemplateVoid for condiguration type * fix: add modifyCollection parameter validation (#3) ModifyCollection was the only non-trivial operation with zero parameter validation, allowing invalid values like validationLevel: "banana" to pass through unchecked until MongoDB runtime rejection.
1 parent e84cfe0 commit 6538255

5 files changed

Lines changed: 246 additions & 10 deletions

File tree

‎ANALYSIS.md‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,10 @@ YAML -> MongoOperation (deserialized) -> MongoOperationValidator -> MongoOperati
3838
**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.
3939
**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

41-
### #3 - HIGH: `modifyCollection` has ZERO parameter validation
42-
**File:** `MongoOperationValidator.java:239-240`
43-
**Impact:** The `validateByType()` switch falls through to `default: return new ArrayList<>()` for `MODIFY_COLLECTION`, `DROP_COLLECTION`, `DROP_VIEW`, and `CREATE_COLLECTION`. While the last three genuinely need only a collection name, `modifyCollection` accepts `validator`, `validationLevel`, and `validationAction` parameters. None are validated. A user could pass `validationLevel: "banana"` and it would pass validation, only to fail at MongoDB runtime.
44-
**Risk:** Silent misconfiguration of collection-level validation rules in production.
45-
**Fix:** Add `case MODIFY_COLLECTION: return validateModifyCollection(op, entityId);` with checks for valid `validationLevel` values (`off`, `strict`, `moderate`) and valid `validationAction` values (`error`, `warn`).
41+
### #3 - ~~HIGH: `modifyCollection` has ZERO parameter validation~~ RESOLVED
42+
**Status:** Fixed by adding `ModifyCollectionParametersValidator`.
43+
**Original issue:** `modifyCollection` used `OperationValidator.NO_OP` in `MongoOperationType`, so none of its three parameters (`validator`, `validationLevel`, `validationAction`) were validated. A user could pass `validationLevel: "banana"` and it would pass validation, only to fail at MongoDB runtime.
44+
**Resolution:** `ModifyCollectionParametersValidator` now validates at load time: requires at least one recognized parameter, checks `validator` is a document (Map), checks `validationLevel` is one of `off`/`strict`/`moderate`, and checks `validationAction` is one of `error`/`warn`. Multiple errors accumulate. Wired into `MongoOperationType.MODIFY_COLLECTION` replacing the `NO_OP` validator.
4645

4746
### #4 - HIGH: Type-unsafe parameter extraction throughout MongoOperation
4847
**File:** `MongoOperation.java:46-115`
@@ -103,7 +102,7 @@ While the validator catches some of these cases, the validator runs only on the
103102
| createIndex | `CREATE_INDEX` | `CreateIndexOperator` | **Yes (wrong)** | Full (keys) | `IndexOptionsMapper` | **Warns & ignores** | `CreateIndexOperatorTest` (1 test) | YAML `_0003`, `_0005` |
104103
| dropIndex | `DROP_INDEX` | `DropIndexOperator` | No | indexName or keys | None | **Ignores entirely** | `DropIndexOperatorTest` (1 test) | YAML `_0005` rollback |
105104
| renameCollection | `RENAME_COLLECTION` | `RenameCollectionOperator` | No | target required | `RenameCollectionOptionsMapper` | Ignores entirely | `RenameCollectionOperatorTest` (1 test) | None |
106-
| modifyCollection | `MODIFY_COLLECTION` | `ModifyCollectionOperator` | No | **None (bug)** | None | Ignores entirely | `ModifyCollectionOperatorTest` (1 test) | None |
105+
| modifyCollection | `MODIFY_COLLECTION` | `ModifyCollectionOperator` | No | Full (validator, validationLevel, validationAction) | None | Ignores entirely | `ModifyCollectionOperatorTest` (1 test) | None |
107106
| createView | `CREATE_VIEW` | `CreateViewOperator` | No | Full (viewOn, pipeline) | `CreateViewOptionsMapper` | Ignores entirely | `CreateViewOperatorTest` (1 test) | None |
108107
| dropView | `DROP_VIEW` | `DropViewOperator` | No | collection only | None | Ignores entirely | `DropViewOperatorTest` (1 test) | None |
109108

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.flamingock.api.annotations.Nullable;
2323
import io.flamingock.api.annotations.Rollback;
2424
import io.flamingock.api.template.AbstractChangeTemplate;
25+
import io.flamingock.api.template.wrappers.TemplateVoid;
2526
import io.flamingock.template.mongodb.model.MongoOperation;
2627
import org.slf4j.Logger;
2728
import org.slf4j.LoggerFactory;
@@ -73,7 +74,7 @@
7374
* @see MongoOperation
7475
*/
7576
@ChangeTemplate( name = "mongodb-sync-template", multiStep = true)
76-
public class MongoChangeTemplate extends AbstractChangeTemplate<Void, MongoOperation, MongoOperation> {
77+
public class MongoChangeTemplate extends AbstractChangeTemplate<TemplateVoid, MongoOperation, MongoOperation> {
7778

7879
private static final Logger log = LoggerFactory.getLogger(MongoChangeTemplate.class);
7980

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import io.flamingock.template.mongodb.validation.DeleteParametersValidator;
3434
import io.flamingock.template.mongodb.validation.DropIndexParametersValidator;
3535
import io.flamingock.template.mongodb.validation.InsertParametersValidator;
36+
import io.flamingock.template.mongodb.validation.ModifyCollectionParametersValidator;
3637
import io.flamingock.template.mongodb.validation.OperationValidator;
3738
import io.flamingock.template.mongodb.validation.RenameCollectionParametersValidator;
3839
import io.flamingock.template.mongodb.validation.UpdateParametersValidator;
@@ -51,7 +52,7 @@ public enum MongoOperationType {
5152
DROP_COLLECTION("dropCollection", DropCollectionOperator::new, OperationValidator.NO_OP),
5253
DROP_INDEX("dropIndex", DropIndexOperator::new, new DropIndexParametersValidator()),
5354
RENAME_COLLECTION("renameCollection", RenameCollectionOperator::new, new RenameCollectionParametersValidator()),
54-
MODIFY_COLLECTION("modifyCollection", ModifyCollectionOperator::new, OperationValidator.NO_OP),
55+
MODIFY_COLLECTION("modifyCollection", ModifyCollectionOperator::new, new ModifyCollectionParametersValidator()),
5556
CREATE_VIEW("createView", CreateViewOperator::new, new CreateViewParametersValidator()),
5657
DROP_VIEW("dropView", DropViewOperator::new, OperationValidator.NO_OP);
5758

‎src/main/java/io/flamingock/template/mongodb/validation/ModifyCollectionParametersValidator.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright 2026 Flamingock (https://www.flamingock.io)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.flamingock.template.mongodb.validation;
17+
18+
import io.flamingock.api.template.TemplatePayloadValidationError;
19+
import io.flamingock.template.mongodb.model.MongoOperation;
20+
21+
import java.util.ArrayList;
22+
import java.util.Arrays;
23+
import java.util.HashSet;
24+
import java.util.List;
25+
import java.util.Map;
26+
import java.util.Set;
27+
28+
public class ModifyCollectionParametersValidator implements OperationValidator {
29+
30+
private static final Set<String> VALID_VALIDATION_LEVELS = new HashSet<>(Arrays.asList("off", "strict", "moderate"));
31+
private static final Set<String> VALID_VALIDATION_ACTIONS = new HashSet<>(Arrays.asList("error", "warn"));
32+
private static final Set<String> RECOGNIZED_KEYS = new HashSet<>(Arrays.asList("validator", "validationLevel", "validationAction"));
33+
34+
@Override
35+
public List<TemplatePayloadValidationError> validate(MongoOperation operation) {
36+
List<TemplatePayloadValidationError> errors = new ArrayList<>();
37+
Map<String, Object> params = operation.getParameters();
38+
39+
if (params == null || !containsAnyRecognizedKey(params)) {
40+
errors.add(new TemplatePayloadValidationError("parameters",
41+
"ModifyCollection operation requires 'parameters' with at least one of: 'validator', 'validationLevel', 'validationAction'"));
42+
return errors;
43+
}
44+
45+
Object validator = params.get("validator");
46+
if (validator != null && !(validator instanceof Map)) {
47+
errors.add(new TemplatePayloadValidationError("parameters.validator",
48+
"'validator' must be a document (map)"));
49+
}
50+
51+
Object validationLevel = params.get("validationLevel");
52+
if (validationLevel != null && (!(validationLevel instanceof String) || !VALID_VALIDATION_LEVELS.contains((String) validationLevel))) {
53+
errors.add(new TemplatePayloadValidationError("parameters.validationLevel",
54+
"'validationLevel' must be one of: 'off', 'strict', 'moderate'"));
55+
}
56+
57+
Object validationAction = params.get("validationAction");
58+
if (validationAction != null && (!(validationAction instanceof String) || !VALID_VALIDATION_ACTIONS.contains((String) validationAction))) {
59+
errors.add(new TemplatePayloadValidationError("parameters.validationAction",
60+
"'validationAction' must be one of: 'error', 'warn'"));
61+
}
62+
63+
return errors;
64+
}
65+
66+
private boolean containsAnyRecognizedKey(Map<String, Object> params) {
67+
for (String key : RECOGNIZED_KEYS) {
68+
if (params.containsKey(key)) {
69+
return true;
70+
}
71+
}
72+
return false;
73+
}
74+
}

‎src/test/java/io/flamingock/template/mongodb/model/MongoOperationValidateTest.java‎

Lines changed: 163 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,12 +713,173 @@ void dropViewValidTest() {
713713
assertTrue(errors.isEmpty());
714714
}
715715

716+
}
717+
718+
@Nested
719+
@DisplayName("ModifyCollection Validation Tests")
720+
class ModifyCollectionValidationTests {
721+
722+
@Test
723+
@DisplayName("WHEN modifyCollection has null parameters THEN validation fails")
724+
void modifyCollectionNullParametersTest() {
725+
MongoOperation op = new MongoOperation();
726+
op.setType("modifyCollection");
727+
op.setCollection("test");
728+
op.setParameters(null);
729+
730+
List<TemplatePayloadValidationError> errors = op.validate();
731+
732+
assertEquals(1, errors.size());
733+
assertEquals("parameters", errors.get(0).getField());
734+
assertTrue(errors.get(0).getMessage().contains("at least one of"));
735+
}
736+
716737
@Test
717-
@DisplayName("WHEN modifyCollection is valid THEN validation passes")
718-
void modifyCollectionValidTest() {
738+
@DisplayName("WHEN modifyCollection has empty parameters THEN validation fails")
739+
void modifyCollectionEmptyParametersTest() {
719740
MongoOperation op = new MongoOperation();
720741
op.setType("modifyCollection");
721742
op.setCollection("test");
743+
op.setParameters(new HashMap<>());
744+
745+
List<TemplatePayloadValidationError> errors = op.validate();
746+
747+
assertEquals(1, errors.size());
748+
assertEquals("parameters", errors.get(0).getField());
749+
assertTrue(errors.get(0).getMessage().contains("at least one of"));
750+
}
751+
752+
@Test
753+
@DisplayName("WHEN modifyCollection has invalid validationLevel THEN validation fails")
754+
void modifyCollectionInvalidValidationLevelTest() {
755+
MongoOperation op = new MongoOperation();
756+
op.setType("modifyCollection");
757+
op.setCollection("test");
758+
Map<String, Object> params = new HashMap<>();
759+
params.put("validationLevel", "banana");
760+
op.setParameters(params);
761+
762+
List<TemplatePayloadValidationError> errors = op.validate();
763+
764+
assertEquals(1, errors.size());
765+
assertEquals("parameters.validationLevel", errors.get(0).getField());
766+
assertTrue(errors.get(0).getMessage().contains("'off', 'strict', 'moderate'"));
767+
}
768+
769+
@Test
770+
@DisplayName("WHEN modifyCollection has invalid validationAction THEN validation fails")
771+
void modifyCollectionInvalidValidationActionTest() {
772+
MongoOperation op = new MongoOperation();
773+
op.setType("modifyCollection");
774+
op.setCollection("test");
775+
Map<String, Object> params = new HashMap<>();
776+
params.put("validationAction", "ignore");
777+
op.setParameters(params);
778+
779+
List<TemplatePayloadValidationError> errors = op.validate();
780+
781+
assertEquals(1, errors.size());
782+
assertEquals("parameters.validationAction", errors.get(0).getField());
783+
assertTrue(errors.get(0).getMessage().contains("'error', 'warn'"));
784+
}
785+
786+
@Test
787+
@DisplayName("WHEN modifyCollection has validator as wrong type THEN validation fails")
788+
void modifyCollectionValidatorWrongTypeTest() {
789+
MongoOperation op = new MongoOperation();
790+
op.setType("modifyCollection");
791+
op.setCollection("test");
792+
Map<String, Object> params = new HashMap<>();
793+
params.put("validator", "not a map");
794+
op.setParameters(params);
795+
796+
List<TemplatePayloadValidationError> errors = op.validate();
797+
798+
assertEquals(1, errors.size());
799+
assertEquals("parameters.validator", errors.get(0).getField());
800+
assertTrue(errors.get(0).getMessage().contains("must be a document"));
801+
}
802+
803+
@Test
804+
@DisplayName("WHEN modifyCollection has multiple invalid params THEN multiple errors reported")
805+
void modifyCollectionMultipleInvalidParamsTest() {
806+
MongoOperation op = new MongoOperation();
807+
op.setType("modifyCollection");
808+
op.setCollection("test");
809+
Map<String, Object> params = new HashMap<>();
810+
params.put("validationLevel", "banana");
811+
params.put("validationAction", "ignore");
812+
op.setParameters(params);
813+
814+
List<TemplatePayloadValidationError> errors = op.validate();
815+
816+
assertEquals(2, errors.size());
817+
}
818+
819+
@Test
820+
@DisplayName("WHEN modifyCollection has only valid validator THEN validation passes")
821+
void modifyCollectionValidValidatorOnlyTest() {
822+
MongoOperation op = new MongoOperation();
823+
op.setType("modifyCollection");
824+
op.setCollection("test");
825+
Map<String, Object> params = new HashMap<>();
826+
Map<String, Object> validator = new HashMap<>();
827+
validator.put("$jsonSchema", new HashMap<>());
828+
params.put("validator", validator);
829+
op.setParameters(params);
830+
831+
List<TemplatePayloadValidationError> errors = op.validate();
832+
833+
assertTrue(errors.isEmpty());
834+
}
835+
836+
@Test
837+
@DisplayName("WHEN modifyCollection has only valid validationLevel THEN validation passes")
838+
void modifyCollectionValidValidationLevelOnlyTest() {
839+
for (String level : Arrays.asList("off", "strict", "moderate")) {
840+
MongoOperation op = new MongoOperation();
841+
op.setType("modifyCollection");
842+
op.setCollection("test");
843+
Map<String, Object> params = new HashMap<>();
844+
params.put("validationLevel", level);
845+
op.setParameters(params);
846+
847+
List<TemplatePayloadValidationError> errors = op.validate();
848+
849+
assertTrue(errors.isEmpty(), "Expected no errors for validationLevel='" + level + "'");
850+
}
851+
}
852+
853+
@Test
854+
@DisplayName("WHEN modifyCollection has only valid validationAction THEN validation passes")
855+
void modifyCollectionValidValidationActionOnlyTest() {
856+
for (String action : Arrays.asList("error", "warn")) {
857+
MongoOperation op = new MongoOperation();
858+
op.setType("modifyCollection");
859+
op.setCollection("test");
860+
Map<String, Object> params = new HashMap<>();
861+
params.put("validationAction", action);
862+
op.setParameters(params);
863+
864+
List<TemplatePayloadValidationError> errors = op.validate();
865+
866+
assertTrue(errors.isEmpty(), "Expected no errors for validationAction='" + action + "'");
867+
}
868+
}
869+
870+
@Test
871+
@DisplayName("WHEN modifyCollection has all three valid params THEN validation passes")
872+
void modifyCollectionAllValidParamsTest() {
873+
MongoOperation op = new MongoOperation();
874+
op.setType("modifyCollection");
875+
op.setCollection("test");
876+
Map<String, Object> params = new HashMap<>();
877+
Map<String, Object> validator = new HashMap<>();
878+
validator.put("$jsonSchema", new HashMap<>());
879+
params.put("validator", validator);
880+
params.put("validationLevel", "strict");
881+
params.put("validationAction", "error");
882+
op.setParameters(params);
722883

723884
List<TemplatePayloadValidationError> errors = op.validate();
724885

0 commit comments

Comments
 (0)