Feature / Model versioning semantics (#467)

martin-traverse · web-flow · commit e58b5cd354ae · 2024-11-11T13:45:00.000Z
* Provide a model version validator

* Provide a flow version validator (no-op)

* Add models and flows to the list of objects that support versioning

* Functional test case for model version validation

* Add new version validation test at the metadata API level
diff --git a/tracdap-libs/tracdap-lib-common/src/main/java/org/finos/tracdap/common/metadata/MetadataConstants.java b/tracdap-libs/tracdap-lib-common/src/main/java/org/finos/tracdap/common/metadata/MetadataConstants.java
@@ -47,7 +47,9 @@ public class MetadataConstants {
             ObjectType.SCHEMA,
             ObjectType.FILE,
             ObjectType.STORAGE,
-            ObjectType.CUSTOM);
+            ObjectType.CUSTOM,
+            ObjectType.MODEL,
+            ObjectType.FLOW);
 
     // Valid identifiers are made up of alphanumeric characters, numbers and the underscore, not starting with a number
     // Use \\A - \\Z to match the whole input
diff --git a/tracdap-libs/tracdap-lib-test/src/main/java/org/finos/tracdap/test/meta/TestData.java b/tracdap-libs/tracdap-lib-test/src/main/java/org/finos/tracdap/test/meta/TestData.java
@@ -84,10 +84,37 @@ public static ObjectDefinition dummyVersionForType(ObjectDefinition definition)
             case CUSTOM: return nextCustomDef(definition);
             case STORAGE: return nextStorageDef(definition);
             case SCHEMA: return nextSchemaDef(definition);
+            case FILE: return nextFileDef(definition);
+
+            case FLOW:
+            case JOB:
+
+                return definition;
+
+            default:
+                throw new RuntimeException("No second version available in dummy data for object type " + objectType.name());
+        }
+    }
+
+    public static ObjectDefinition dummyBadVersionForType(ObjectDefinition definition) {
+
+        // Not all object types have semantics defined for versioning
+        // It is sometimes helpful to create versions anyway for testing
+        // E.g. to test that version increments are rejected for objects that don't support versioning!
+
+        var objectType = definition.getObjectType();
+
+        switch (objectType) {
+
+            case DATA: return nextBadDataDef(definition);
+            case MODEL: return nextBadModelDef(definition);
+            case CUSTOM: return nextBadCustomDef(definition);
+            case STORAGE: return nextBadStorageDef(definition);
+            case SCHEMA: return nextBadSchemaDef(definition);
+            case FILE: return nextBadFileDef(definition);
 
             case FLOW:
             case JOB:
-            case FILE:
 
                 return definition;
 
@@ -185,6 +212,16 @@ public static ObjectDefinition nextDataDef(ObjectDefinition origDef) {
                 .build();
     }
 
+    public static ObjectDefinition nextBadDataDef(ObjectDefinition origDef) {
+
+        var newSchema = changeBadSchemaField(origDef.getData().getSchema());
+
+        return origDef.toBuilder()
+                .setData(origDef.getData().toBuilder()
+                .setSchema(newSchema))
+                .build();
+    }
+
     public static ObjectDefinition dummySchemaDef() {
 
         var dataDef = dummyDataDef();
@@ -202,6 +239,13 @@ public static ObjectDefinition nextSchemaDef(ObjectDefinition origDef) {
                 .build();
     }
 
+    public static ObjectDefinition nextBadSchemaDef(ObjectDefinition origDef) {
+
+        return origDef.toBuilder()
+                .setSchema(changeBadSchemaField(origDef.getSchema()))
+                .build();
+    }
+
     private static SchemaDefinition addFieldToSchema(SchemaDefinition origSchema) {
 
         var fieldName = "extra_field_" + (origSchema.getTable().getFieldsCount() + 1);
@@ -219,6 +263,19 @@ private static SchemaDefinition addFieldToSchema(SchemaDefinition origSchema) {
                 .build();
     }
 
+    private static SchemaDefinition changeBadSchemaField(SchemaDefinition origSchema) {
+
+        var originalField = origSchema.getTable().getFields(0);
+        var newField = originalField.toBuilder()
+                .setFieldType(BasicType.DATE)
+                .build();
+
+        return origSchema.toBuilder()
+                .setTable(origSchema.getTable().toBuilder()
+                .setFields(0, newField))
+                .build();
+    }
+
     public static ObjectDefinition dummyStorageDef() {
 
         var storageTimestamp = OffsetDateTime.now();
@@ -265,6 +322,26 @@ public static ObjectDefinition nextStorageDef(ObjectDefinition definition) {
             .build();
     }
 
+    public static ObjectDefinition nextBadStorageDef(ObjectDefinition definition) {
+
+        var originalItem = definition.getStorage().getDataItemsOrThrow("dummy_item");
+        var originalCopy = originalItem.getIncarnations(0).getCopies(0);
+
+        var newCopy = originalCopy.toBuilder()
+                .setStorageFormat("PARQUET")
+                .build();
+
+        var newItem = originalItem.toBuilder()
+                .setIncarnations(0, originalItem.getIncarnations(0).toBuilder()
+                .setCopies(0, newCopy))
+                .build();
+
+        return definition.toBuilder()
+                .setStorage(definition.getStorage().toBuilder()
+                .putDataItems("dummy_item", newItem))
+                .build();
+    }
+
     public static ObjectDefinition dummyModelDef() {
 
         return ObjectDefinition.newBuilder()
@@ -315,6 +392,15 @@ public static ObjectDefinition nextModelDef(ObjectDefinition origDef) {
                 .build();
     }
 
+    public static ObjectDefinition nextBadModelDef(ObjectDefinition origDef) {
+
+        return origDef.toBuilder()
+                .setModel(origDef.getModel()
+                .toBuilder()
+                .setPath("altered/layout/src"))
+                .build();
+    }
+
     public static ObjectDefinition dummyFlowDef() {
 
         return ObjectDefinition.newBuilder()
@@ -374,6 +460,27 @@ public static ObjectDefinition dummyFileDef() {
                 .build();
     }
 
+    public static ObjectDefinition nextFileDef(ObjectDefinition origDef) {
+
+        return origDef.toBuilder()
+                .setFile(origDef.getFile().toBuilder()
+                .setName("magic_template_v2_updated")  // File names are likely to be changed with suffixes etc
+                .setSize(87533)
+                .setDataItem("file/FILE_ID/version-2"))
+                .build();
+    }
+
+    public static ObjectDefinition nextBadFileDef(ObjectDefinition origDef) {
+
+        return origDef.toBuilder()
+                .setFile(origDef.getFile().toBuilder()
+                .setExtension("txt")
+                .setMimeType("text/plain")
+                .setSize(87533)
+                .setDataItem("file/FILE_ID/version-2"))
+                .build();
+    }
+
     public static ObjectDefinition dummyCustomDef() {
 
         var jsonReportDef = "{ reportType: 'magic', mainGraph: { content: 'more_magic' } }";
@@ -382,7 +489,7 @@ public static ObjectDefinition dummyCustomDef() {
                 .setObjectType(ObjectType.CUSTOM)
                 .setCustom(CustomDefinition.newBuilder()
                 .setCustomSchemaType("REPORT")
-                .setCustomSchemaVersion(2)
+                .setCustomSchemaVersion(1)
                 .setCustomData(ByteString.copyFromUtf8(jsonReportDef)))
                 .build();
     }
@@ -395,10 +502,20 @@ public static ObjectDefinition nextCustomDef(ObjectDefinition origDef) {
         return origDef.toBuilder()
                 .setCustom(origDef.getCustom()
                 .toBuilder()
+                .setCustomSchemaVersion(2)
                 .setCustomData(ByteString.copyFromUtf8(jsonReportDef)))
                 .build();
     }
 
+    public static ObjectDefinition nextBadCustomDef(ObjectDefinition origDef) {
+
+        return origDef.toBuilder()
+                .setCustom(origDef.getCustom()
+                .toBuilder()
+                .setCustomSchemaType("DASHBOARD"))
+                .build();
+    }
+
     public static Tag dummyTag(ObjectDefinition definition, boolean includeHeader) {
 
         var tag = Tag.newBuilder()
diff --git a/tracdap-libs/tracdap-lib-validation/src/main/java/org/finos/tracdap/common/validation/version/FlowVersionValidator.java b/tracdap-libs/tracdap-lib-validation/src/main/java/org/finos/tracdap/common/validation/version/FlowVersionValidator.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2024 finTRAC Limited
+ *
+ * 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 org.finos.tracdap.common.validation.version;
+
+import org.finos.tracdap.common.validation.core.ValidationContext;
+import org.finos.tracdap.common.validation.core.ValidationType;
+import org.finos.tracdap.common.validation.core.Validator;
+import org.finos.tracdap.metadata.FlowDefinition;
+
+@Validator(type = ValidationType.VERSION)
+public class FlowVersionValidator {
+
+    @Validator
+    public static ValidationContext flowVersion(FlowDefinition current, FlowDefinition prior, ValidationContext ctx) {
+
+        // There are no structural constraints on what constitutes a flow version
+        // This is to stay in line with model version validation, since flows have no "coordinates"
+        // What constitutes a flow version vs a new flow is left upto the client application to decide
+
+        return ctx;
+    }
+}
diff --git a/tracdap-libs/tracdap-lib-validation/src/main/java/org/finos/tracdap/common/validation/version/ModelVersionValidator.java b/tracdap-libs/tracdap-lib-validation/src/main/java/org/finos/tracdap/common/validation/version/ModelVersionValidator.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2024 finTRAC Limited
+ *
+ * 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 org.finos.tracdap.common.validation.version;
+
+import org.finos.tracdap.metadata.ModelDefinition;
+import org.finos.tracdap.common.validation.core.ValidationContext;
+import org.finos.tracdap.common.validation.core.ValidationType;
+import org.finos.tracdap.common.validation.core.Validator;
+
+import com.google.protobuf.Descriptors;
+
+import static org.finos.tracdap.common.validation.core.ValidatorUtils.field;
+
+
+@Validator(type = ValidationType.VERSION)
+public class ModelVersionValidator {
+
+    private static final Descriptors.Descriptor MODEL_DEFINITION;
+    private static final Descriptors.FieldDescriptor MD_MODEL_TYPE;
+    private static final Descriptors.FieldDescriptor MD_LANGUAGE;
+    private static final Descriptors.FieldDescriptor MD_REPOSITORY;
+    private static final Descriptors.FieldDescriptor MD_PACKAGE_GROUP;
+    private static final Descriptors.FieldDescriptor MD_PACKAGE;
+    private static final Descriptors.FieldDescriptor MD_VERSION;
+    private static final Descriptors.FieldDescriptor MD_PATH;
+    private static final Descriptors.FieldDescriptor MD_ENTRY_POINT;
+
+    static {
+
+        MODEL_DEFINITION = ModelDefinition.getDescriptor();
+        MD_MODEL_TYPE = field(MODEL_DEFINITION, ModelDefinition.MODELTYPE_FIELD_NUMBER);
+        MD_LANGUAGE = field(MODEL_DEFINITION, ModelDefinition.LANGUAGE_FIELD_NUMBER);
+        MD_REPOSITORY = field(MODEL_DEFINITION, ModelDefinition.REPOSITORY_FIELD_NUMBER);
+        MD_PACKAGE_GROUP = field(MODEL_DEFINITION, ModelDefinition.PACKAGEGROUP_FIELD_NUMBER);
+        MD_PACKAGE = field(MODEL_DEFINITION, ModelDefinition.PACKAGE_FIELD_NUMBER);
+        MD_VERSION = field(MODEL_DEFINITION, ModelDefinition.VERSION_FIELD_NUMBER);
+        MD_PATH = field(MODEL_DEFINITION, ModelDefinition.PATH_FIELD_NUMBER);
+        MD_ENTRY_POINT = field(MODEL_DEFINITION, ModelDefinition.ENTRYPOINT_FIELD_NUMBER);
+    }
+
+    @Validator
+    public static ValidationContext modelVersion(ModelDefinition current, ModelDefinition prior, ValidationContext ctx) {
+
+        // Two models are considered versions of the same model if all coordinates match between versions
+        // I.e. model compatibility is not based on functional compatibility
+        // If a model is moved to a different repo or namespace, it is considered a new model
+        // So, the evolution of one model in code can be tracked by a single model object in TRAC
+
+        ctx = ctx.push(MD_MODEL_TYPE)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_LANGUAGE)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_REPOSITORY)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_PACKAGE_GROUP)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_PACKAGE)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_VERSION)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_PATH)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        ctx = ctx.push(MD_ENTRY_POINT)
+                .apply(CommonValidators::exactMatch)
+                .pop();
+
+        return ctx;
+    }
+}
diff --git a/tracdap-libs/tracdap-lib-validation/src/test/java/org/finos/tracdap/common/validation/test/BaseValidatorTest.java b/tracdap-libs/tracdap-lib-validation/src/test/java/org/finos/tracdap/common/validation/test/BaseValidatorTest.java
@@ -19,6 +19,7 @@
 
 import com.google.protobuf.Message;
 import org.finos.tracdap.common.exception.EInputValidation;
+import org.finos.tracdap.common.exception.EVersionValidation;
 import org.finos.tracdap.common.validation.Validator;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -46,4 +47,20 @@ protected static <TMsg extends Message> void expectInvalid(TMsg msg) {
                 () -> validator.validateFixedObject(msg),
                 "Validation passed for an invalid message");
     }
+
+    protected static <TMsg extends Message>
+    void expectValidVersion(TMsg current, TMsg prior) {
+
+        Assertions.assertDoesNotThrow(
+                () -> validator.validateVersion(current, prior),
+                "Versioning validation failed for a valid version update");
+    }
+
+    protected static <TMsg extends Message>
+    void expectInvalidVersion(TMsg current, TMsg prior) {
+
+        Assertions.assertThrows(EVersionValidation.class,
+                () -> validator.validateVersion(current, prior),
+                "Versioning validation passed for an invalid version update");
+    }
 }
diff --git a/tracdap-libs/tracdap-lib-validation/src/test/java/org/finos/tracdap/common/validation/version/ModelVersionValidationTest.java b/tracdap-libs/tracdap-lib-validation/src/test/java/org/finos/tracdap/common/validation/version/ModelVersionValidationTest.java
diff --git a/tracdap-services/tracdap-svc-meta/src/test/java/org/finos/tracdap/svc/meta/api/MetadataWriteApiTest.java b/tracdap-services/tracdap-svc-meta/src/test/java/org/finos/tracdap/svc/meta/api/MetadataWriteApiTest.java
