fix: OpenAPI ModelVersion shall contain registeredModelId property (#61)

need to adapt property definition in OpenAPI to accomodate openapi-codegen result; according to contract (as also visible in swagger) the ModelVersion is to contain property: registeredModelId Signed-off-by: Matteo Mortari <[email protected]>
kubeflow · Apr 16, 2024 · 734eb9e · 734eb9e
1 parent 65613f9
commit 734eb9e
Show file tree

Hide file tree

Showing 12 changed files with 195 additions and 107 deletions.
diff --git a/api/openapi/model-registry.yaml b/api/openapi/model-registry.yaml
@@ -1047,10 +1047,11 @@ components:
       allOf:
         - $ref: "#/components/schemas/BaseResourceCreate"
         - $ref: "#/components/schemas/ModelVersionUpdate"
-      properties:
-        registeredModelId:
-          description: ID of the `RegisteredModel` to which this version belongs.
-          type: string
+        - type: object
+          properties:
+            registeredModelId:
+              description: ID of the `RegisteredModel` to which this version belongs.
+              type: string
     ModelVersionUpdate:
       description: Represents a ModelVersion belonging to a RegisteredModel.
       allOf:

diff --git a/internal/converter/generated/mlmd_openapi_converter.gen.go b/internal/converter/generated/mlmd_openapi_converter.gen.go
diff --git a/internal/converter/generated/openapi_converter.gen.go b/internal/converter/generated/openapi_converter.gen.go
diff --git a/internal/converter/mlmd_converter_util_test.go b/internal/converter/mlmd_converter_util_test.go
@@ -570,6 +570,24 @@ func TestMapNameFromOwned(t *testing.T) {
 	assertion.Nil(name)
 }
 
+func TestMapRegisteredModelIdFromOwned(t *testing.T) {
+	assertion := setup(t)
+
+	result, err := MapRegisteredModelIdFromOwned(of("prefix:name"))
+	assertion.Nil(err)
+	assertion.Equal("prefix", result)
+
+	_, err = MapRegisteredModelIdFromOwned(of("name"))
+	assertion.NotNil(err)
+
+	_, err = MapRegisteredModelIdFromOwned(of("prefix:name:postfix"))
+	assertion.NotNil(err)
+
+	result, err = MapRegisteredModelIdFromOwned(nil)
+	assertion.Nil(err)
+	assertion.Equal("", result)
+}
+
 func TestMapArtifactType(t *testing.T) {
 	assertion := setup(t)
 

diff --git a/internal/converter/mlmd_openapi_converter.go b/internal/converter/mlmd_openapi_converter.go
@@ -19,6 +19,7 @@ type MLMDToOpenAPIConverter interface {
 	ConvertRegisteredModel(source *proto.Context) (*openapi.RegisteredModel, error)
 
 	// goverter:map Name | MapNameFromOwned
+	// goverter:map Name RegisteredModelId | MapRegisteredModelIdFromOwned
 	// goverter:map Properties Description | MapDescription
 	// goverter:map Properties State | MapModelVersionState
 	// goverter:map Properties Author | MapPropertyAuthor

diff --git a/internal/converter/mlmd_openapi_converter_util.go b/internal/converter/mlmd_openapi_converter_util.go
@@ -108,6 +108,18 @@ func MapPropertyAuthor(properties map[string]*proto.Value) *string {
 	return MapStringProperty(properties, "author")
 }
 
+func MapRegisteredModelIdFromOwned(source *string) (string, error) {
+	if source == nil {
+		return "", nil
+	}
+
+	exploded := strings.Split(*source, ":")
+	if len(exploded) != 2 {
+		return "", fmt.Errorf("wrong owned format")
+	}
+	return exploded[0], nil
+}
+
 // ARTIFACT
 
 func MapArtifactType(source *proto.Artifact) (string, error) {

diff --git a/internal/converter/openapi_converter.go b/internal/converter/openapi_converter.go
@@ -22,7 +22,7 @@ type OpenAPIConverter interface {
 	// goverter:ignore Id CreateTimeSinceEpoch LastUpdateTimeSinceEpoch
 	ConvertModelVersionCreate(source *openapi.ModelVersionCreate) (*openapi.ModelVersion, error)
 
-	// goverter:ignore Id CreateTimeSinceEpoch LastUpdateTimeSinceEpoch Name
+	// goverter:ignore Id CreateTimeSinceEpoch LastUpdateTimeSinceEpoch Name RegisteredModelId
 	ConvertModelVersionUpdate(source *openapi.ModelVersionUpdate) (*openapi.ModelVersion, error)
 
 	// goverter:ignore Id CreateTimeSinceEpoch LastUpdateTimeSinceEpoch ArtifactType