openapi: require name for contexts (#253)

Signed-off-by: Isabella do Amaral <[email protected]>

api/openapi/model-registry.yaml

@@ -1023,10 +1023,18 @@ components:
         - $ref: "#/components/schemas/BaseResourceList"
     RegisteredModelCreate:
       description: A registered model in model registry. A registered model has ModelVersion children.
+      required:
+        - name
       allOf:
         - type: object
         - $ref: "#/components/schemas/BaseResourceCreate"
         - $ref: "#/components/schemas/RegisteredModelUpdate"
+          properties:
+            name:
+              description: |-
+                The client provided name of the model. It must be unique among all the RegisteredModels of the same
+                type within a Model Registry instance and cannot be changed once set.
+              type: string
     RegisteredModelUpdate:
       description: A registered model in model registry. A registered model has ModelVersion children.
       allOf:
@@ -1045,6 +1053,7 @@ components:
     ModelVersionCreate:
       description: Represents a ModelVersion belonging to a RegisteredModel.
       required:
+        - name
         - registeredModelId
       allOf:
         - $ref: "#/components/schemas/BaseResourceCreate"
@@ -1054,6 +1063,11 @@ components:
             registeredModelId:
               description: ID of the `RegisteredModel` to which this version belongs.
               type: string
+            name:
+              description: |-
+                The client provided name of the model's version. It must be unique among all the ModelVersions of the same
+                type within a Model Registry instance and cannot be changed once set.
+              type: string
     ModelVersionUpdate:
       description: Represents a ModelVersion belonging to a RegisteredModel.
       allOf:

internal/converter/mlmd_converter_util_test.go

@@ -216,7 +216,7 @@ func TestMapModelVersionProperties(t *testing.T) {
 		ParentResourceId: of("123"),
 		ModelName:        of("MyModel"),
 		Model: &openapi.ModelVersion{
-			Name:        of("v1"),
+			Name:        "v1",
 			Description: of("my model version description"),
 			Author:      of("John Doe"),
 		},
@@ -244,7 +244,7 @@ func TestMapModelVersionName(t *testing.T) {
 		ParentResourceId: of("123"),
 		ModelName:        of("MyModel"),
 		Model: &openapi.ModelVersion{
-			Name: of("v1"),
+			Name: "v1",
 		},
 	})
 	assertion.NotNil(name)

internal/converter/mlmd_openapi_converter.go

@@ -19,7 +19,7 @@ type MLMDToOpenAPIConverter interface {
 	// goverter:map Properties State | MapRegisteredModelState
 	ConvertRegisteredModel(source *proto.Context) (*openapi.RegisteredModel, error)
 
-	// goverter:map Name | MapNameFromOwned
+	// goverter:map Name | MapName
 	// goverter:map Name RegisteredModelId | MapRegisteredModelIdFromOwned
 	// goverter:map Properties Description | MapDescription
 	// goverter:map Properties State | MapModelVersionState

internal/converter/mlmd_openapi_converter_util.go

@@ -100,6 +100,20 @@ func MapNameFromOwned(source *string) *string {
 	return &exploded[1]
 }
 
+// MapName derive the entity name from the mlmd fullname
+// for owned entity such as ModelVersion
+func MapName(source *string) string {
+	if source == nil {
+		return ""
+	}
+
+	exploded := strings.Split(*source, ":")
+	if len(exploded) == 1 {
+		return *source
+	}
+	return exploded[1]
+}
+
 // REGISTERED MODEL
 
 // MODEL VERSION

internal/converter/openapi_mlmd_converter.go

@@ -8,10 +8,10 @@ import (
 type OpenAPIModelWrapper[
 	M OpenAPIModel,
 ] struct {
-	TypeId           int64
 	Model            *M
-	ParentResourceId *string // optional parent id
-	ModelName        *string // optional registered model name
+	ParentResourceId *string
+	ModelName        *string
+	TypeId           int64
 }
 
 // goverter:converter

internal/converter/openapi_mlmd_converter_util.go

@@ -177,7 +177,7 @@ func MapModelVersionProperties(source *OpenAPIModelWrapper[openapi.ModelVersion]
 		}
 		props["version"] = &proto.Value{
 			Value: &proto.Value_StringValue{
-				StringValue: *(*source.Model).Name,
+				StringValue: (*source.Model).Name,
 			},
 		}
 
@@ -208,7 +208,7 @@ func MapModelVersionType(_ *openapi.ModelVersion) *string {
 // MapModelVersionName maps the user-provided name into MLMD one, i.e., prefixing it with
 // either the parent resource id or a generated uuid
 func MapModelVersionName(source *OpenAPIModelWrapper[openapi.ModelVersion]) *string {
-	return of(PrefixWhenOwned(source.ParentResourceId, *(*source).Model.Name))
+	return of(PrefixWhenOwned(source.ParentResourceId, (*source).Model.Name))
 }
 
 // ARTIFACT

internal/mapper/mapper.go

@@ -24,6 +24,11 @@ func NewMapper(mlmdTypes map[string]int64) *Mapper {
 	}
 }
 
+// of returns a pointer to the provided literal/const input
+func of[E any](e E) *E {
+	return &e
+}
+
 // Utilities for OpenAPI --> MLMD mapping, make use of generated Converters
 
 func (m *Mapper) MapFromRegisteredModel(registeredModel *openapi.RegisteredModel) (*proto.Context, error) {
@@ -33,12 +38,12 @@ func (m *Mapper) MapFromRegisteredModel(registeredModel *openapi.RegisteredModel
 	})
 }
 
-func (m *Mapper) MapFromModelVersion(modelVersion *openapi.ModelVersion, registeredModelId string, registeredModelName *string) (*proto.Context, error) {
+func (m *Mapper) MapFromModelVersion(modelVersion *openapi.ModelVersion, registeredModelId string, registeredModelName string) (*proto.Context, error) {
 	return m.OpenAPIConverter.ConvertModelVersion(&converter.OpenAPIModelWrapper[openapi.ModelVersion]{
 		TypeId:           m.MLMDTypes[defaults.ModelVersionTypeName],
 		Model:            modelVersion,
 		ParentResourceId: &registeredModelId,
-		ModelName:        registeredModelName,
+		ModelName:        of(registeredModelName),
 	})
 }
 

internal/mapper/mapper_test.go

@@ -40,7 +40,7 @@ func setup(t *testing.T) (*assert.Assertions, *Mapper) {
 func TestMapFromRegisteredModel(t *testing.T) {
 	assertion, m := setup(t)
 
-	ctx, err := m.MapFromRegisteredModel(&openapi.RegisteredModel{Name: of("ModelName")})
+	ctx, err := m.MapFromRegisteredModel(&openapi.RegisteredModel{Name: "ModelName"})
 	assertion.Nil(err)
 	assertion.Equal("ModelName", ctx.GetName())
 	assertion.Equal(registeredModelTypeId, ctx.GetTypeId())
@@ -49,7 +49,7 @@ func TestMapFromRegisteredModel(t *testing.T) {
 func TestMapFromModelVersion(t *testing.T) {
 	assertion, m := setup(t)
 
-	ctx, err := m.MapFromModelVersion(&openapi.ModelVersion{Name: of("v1")}, "1", of("ModelName"))
+	ctx, err := m.MapFromModelVersion(&openapi.ModelVersion{Name: "v1"}, "1", "ModelName")
 	assertion.Nil(err)
 	assertion.Equal("1:v1", ctx.GetName())
 	assertion.Equal(modelVersionTypeId, ctx.GetTypeId())
@@ -277,12 +277,7 @@ func TestMapToServeModelInvalid(t *testing.T) {
 }
 
 func TestMapTo(t *testing.T) {
-	_, err := mapTo[*proto.Execution, any](&proto.Execution{TypeId: of(registeredModelTypeId)}, typesMap, "notExisitingTypeName", func(e *proto.Execution) (*any, error) { return nil, nil })
+	_, err := mapTo(&proto.Execution{TypeId: of(registeredModelTypeId)}, typesMap, "notExisitingTypeName", func(e *proto.Execution) (*any, error) { return nil, nil })
 	assert.NotNil(t, err)
 	assert.Equal(t, "unknown type name provided: notExisitingTypeName", err.Error())
 }
-
-// of returns a pointer to the provided literal/const input
-func of[E any](e E) *E {
-	return &e
-}

internal/server/openapi/impl.go

@@ -11,6 +11,6 @@ package openapi
 
 // ImplResponse defines an implementation response with error code and the associated body
 type ImplResponse struct {
-	Code int
 	Body interface{}
+	Code int
 }

internal/server/openapi/type_asserts.go

@@ -524,6 +524,7 @@ func AssertModelArtifactUpdateConstraints(obj model.ModelArtifactUpdate) error {
 // AssertModelVersionRequired checks if the required fields are not zero-ed
 func AssertModelVersionRequired(obj model.ModelVersion) error {
 	elements := map[string]interface{}{
+		"name":              obj.Name,
 		"registeredModelId": obj.RegisteredModelId,
 	}
 	for name, el := range elements {
@@ -543,6 +544,7 @@ func AssertModelVersionConstraints(obj model.ModelVersion) error {
 // AssertModelVersionCreateRequired checks if the required fields are not zero-ed
 func AssertModelVersionCreateRequired(obj model.ModelVersionCreate) error {
 	elements := map[string]interface{}{
+		"name":              obj.Name,
 		"registeredModelId": obj.RegisteredModelId,
 	}
 	for name, el := range elements {
@@ -617,6 +619,15 @@ func AssertOrderByFieldConstraints(obj model.OrderByField) error {
 
 // AssertRegisteredModelRequired checks if the required fields are not zero-ed
 func AssertRegisteredModelRequired(obj model.RegisteredModel) error {
+	elements := map[string]interface{}{
+		"name": obj.Name,
+	}
+	for name, el := range elements {
+		if isZero := IsZeroValue(el); isZero {
+			return &RequiredError{Field: name}
+		}
+	}
+
 	return nil
 }
 
@@ -627,6 +638,15 @@ func AssertRegisteredModelConstraints(obj model.RegisteredModel) error {
 
 // AssertRegisteredModelCreateRequired checks if the required fields are not zero-ed
 func AssertRegisteredModelCreateRequired(obj model.RegisteredModelCreate) error {
+	elements := map[string]interface{}{
+		"name": obj.Name,
+	}
+	for name, el := range elements {
+		if isZero := IsZeroValue(el); isZero {
+			return &RequiredError{Field: name}
+		}
+	}
+
 	return nil
 }