diff --git a/schema-core/src/main/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandler.java b/schema-core/src/main/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandler.java index 39fde6061e67b5e5bf8bcc1f0732b20bedd1f4eb..cda56217a63c437042b64091e3ea8a5cbfe21747 100644 --- a/schema-core/src/main/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandler.java +++ b/schema-core/src/main/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandler.java @@ -35,14 +35,14 @@ public class AddOperationHandler implements SchemaValidationHandler{ @Override public void compare(SchemaHandlerVO schemaDiff, SchemaPatch patch, List<SchemaBreakingChanges> schemaBreakingChanges, Set<String> processedArrayPath) throws ApplicationException { - if(schemaDiff.getValidationType() == getValidationType() + if(schemaDiff.getValidationType() == getValidationType() && SchemaConstants.OP_ADD.equals(patch.getOp())) { String attribut = patch.getPath().charAt(0) == '/' ? patch.getPath().substring(1) : patch.getPath(); if(isRefIdPresent(schemaDiff, attribut)) return; - else if (isPresentInTarget(schemaDiff, patch)) + else if (isPresentInSource(schemaDiff, patch)) return; schemaBreakingChanges.add(new SchemaBreakingChanges(patch, "Adding attributes at Patch level is not permitted.")); }else if(null != nextHandler){ @@ -62,14 +62,14 @@ public class AddOperationHandler implements SchemaValidationHandler{ return schemaDiff.getChangedRefIds().containsValue(attributeName); } - private boolean isPresentInTarget(SchemaHandlerVO schemaDiff, SchemaPatch patch) { + private boolean isPresentInSource(SchemaHandlerVO schemaDiff, SchemaPatch patch) { Pattern pattern = Pattern.compile(SchemaConstants.SCHEMA_KIND_REGEX); String path = patch.getPath(); String sourceField = StringUtils.substringAfterLast(path, "/"); if(!pattern.matcher(path).matches() || !isAtRoot(path)) return false; - Iterator<String> fieldNameItr = schemaDiff.getTargetSchema().fieldNames(); + Iterator<String> fieldNameItr = schemaDiff.getSourceSchema().fieldNames(); while(fieldNameItr.hasNext()) { String fieldName = fieldNameItr.next(); if(pattern.matcher(fieldName).matches()) { diff --git a/schema-core/src/test/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandlerTest.java b/schema-core/src/test/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandlerTest.java index 7babc9db58a0bf00d5f5371337f6925af53b45a1..16d9823808e8a42be7b6817e579eb5adae05cbd3 100644 --- a/schema-core/src/test/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandlerTest.java +++ b/schema-core/src/test/java/org/opengroup/osdu/schema/validation/version/handler/patch/AddOperationHandlerTest.java @@ -10,7 +10,10 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; import org.opengroup.osdu.schema.exceptions.ApplicationException; +import org.opengroup.osdu.schema.util.SchemaUtil; import org.opengroup.osdu.schema.util.TestUtility; import org.opengroup.osdu.schema.validation.version.SchemaValidationType; import org.opengroup.osdu.schema.validation.version.model.SchemaBreakingChanges; @@ -25,7 +28,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; public class AddOperationHandlerTest { @InjectMocks AddOperationHandler addOperationHandler; - + + @Mock + SchemaUtil schemaUtil; + @Test public void testCompare_AddNewAttr_NotAllowed() throws IOException, ApplicationException { @@ -41,7 +47,23 @@ public class AddOperationHandlerTest { } } - + + @Test + public void testCompare_AddNewAttr_AllowedForAVersionUpgrade() throws IOException, ApplicationException { + + List<SchemaBreakingChanges> schemaBreakingChanges = new ArrayList<>(); + Set<String> processedArrayPath = new HashSet<>(); + SchemaHandlerVO schemaHandlerVO = getMockSchemaHandlerVO("/schema_compare/add_operation/base-schema-2.json" + ,"/schema_compare/add_operation/addattr-version-upgrade-patch.json"); + Mockito.when(schemaUtil.isValidSchemaVersionChange("osdu:wks:AbstractCommonResources:2.0.0", "osdu:wks:AbstractCommonResources:2.0.1", SchemaValidationType.PATCH)).thenReturn(true); + List<SchemaPatch> schemaPatchList = TestUtility.findSchemaPatch(schemaHandlerVO.getSourceSchema(), schemaHandlerVO.getTargetSchema()); + for(SchemaPatch patch : schemaPatchList) { + addOperationHandler.compare(schemaHandlerVO, patch, schemaBreakingChanges, processedArrayPath); + } + Assert.assertTrue(schemaBreakingChanges.size() == 0); + + } + @Test public void testCompare_NoChange() throws IOException, ApplicationException { @@ -57,7 +79,7 @@ public class AddOperationHandlerTest { } } - + @Test public void testCompare_AddAttr_Minor() throws IOException, ApplicationException { @@ -66,7 +88,7 @@ public class AddOperationHandlerTest { SchemaHandlerVO schemaHandlerVO = getMockSchemaHandlerVO("/schema_compare/add_operation/base-schema.json" ,"/schema_compare/add_operation/base-schema.json"); List<SchemaPatch> schemaPatchList = TestUtility.findSchemaPatch(schemaHandlerVO.getSourceSchema(), schemaHandlerVO.getTargetSchema()); - + schemaHandlerVO.setValidationType(SchemaValidationType.MINOR); for(SchemaPatch patch : schemaPatchList) { addOperationHandler.compare(schemaHandlerVO, patch, schemaBreakingChanges, processedArrayPath); @@ -75,7 +97,7 @@ public class AddOperationHandlerTest { } } - + private SchemaHandlerVO getMockSchemaHandlerVO(String baseSchemaPath, String newSchemaPath) throws IOException { JsonNode baseSchema = TestUtility.getJsonNodeFromFile(baseSchemaPath); JsonNode newSchema = TestUtility.getJsonNodeFromFile(newSchemaPath); diff --git a/schema-core/src/test/resources/schema_compare/add_operation/addattr-version-upgrade-patch.json b/schema-core/src/test/resources/schema_compare/add_operation/addattr-version-upgrade-patch.json new file mode 100644 index 0000000000000000000000000000000000000000..e67b0712366c36023769842942d76759afcf13e3 --- /dev/null +++ b/schema-core/src/test/resources/schema_compare/add_operation/addattr-version-upgrade-patch.json @@ -0,0 +1,33 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "title": "My schema", + "properties": { + "AddressLine1": { "type": "string" }, + "AddressLine2": { "type": "string" }, + "City": { "type": "string" } + + }, + "osdu:wks:AbstractCommonResources:2.0.1": { + "type": "string" + }, + "required": [ "AddressLine1", "AddressLine2" ], + "oneOf": [ + { + "type": "object", + "properties": { + "State": { "type": "string" }, + "ZipCode": { "type": "string" } + }, + "required": [ "ZipCode" ] + }, + { + "type": "object", + "properties": { + "County": { "type": "string" }, + "PostCode": { "type": "string" } + }, + "required": [ "PostCode" ] + } + ] +} \ No newline at end of file diff --git a/schema-core/src/test/resources/schema_compare/add_operation/base-schema-2.json b/schema-core/src/test/resources/schema_compare/add_operation/base-schema-2.json new file mode 100644 index 0000000000000000000000000000000000000000..12f539edb40a4e64d765f1a17a9c397fe613e71d --- /dev/null +++ b/schema-core/src/test/resources/schema_compare/add_operation/base-schema-2.json @@ -0,0 +1,34 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "title": "My schema", + "properties": { + "AddressLine1": { "type": "string" }, + "AddressLine2": { "type": "string" }, + "City": { "type": "string" } + + + }, + "osdu:wks:AbstractCommonResources:2.0.0": { + "type": "string" + }, + "required": [ "AddressLine1", "AddressLine2" ], + "oneOf": [ + { + "type": "object", + "properties": { + "State": { "type": "string" }, + "ZipCode": { "type": "string" } + }, + "required": [ "ZipCode" ] + }, + { + "type": "object", + "properties": { + "County": { "type": "string" }, + "PostCode": { "type": "string" } + }, + "required": [ "PostCode" ] + } + ] +} \ No newline at end of file