500 custom prop type (kubeflow#914)

* improve error handling package

Signed-off-by: Adysen Rothman <[email protected]>

* add testing

Signed-off-by: Adysen Rothman <[email protected]>

* more specific err for mlmd

Signed-off-by: Adysen Rothman <[email protected]>

* api err for type val issues

Signed-off-by: Adysen Rothman <[email protected]>

---------

Signed-off-by: Adysen Rothman <[email protected]>

Author: adysenrothman

internal/converter/mlmd_openapi_converter_util.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/kubeflow/model-registry/internal/defaults"
 	"github.com/kubeflow/model-registry/internal/ml_metadata/proto"
+	"github.com/kubeflow/model-registry/pkg/api"
 	"github.com/kubeflow/model-registry/pkg/openapi"
 )
 
@@ -76,7 +77,7 @@ func MapMLMDCustomProperties(source map[string]*proto.Value) (map[string]openapi
 			b64 := base64.StdEncoding.EncodeToString(asJSON)
 			customValue.MetadataStructValue = NewMetadataStructValue(b64)
 		default:
-			return nil, fmt.Errorf("type mapping not found for %s:%v", key, v)
+			return nil, fmt.Errorf("%w: metadataType not found for %s: %v", api.ErrBadRequest, key, v)
 		}
 
 		data[key] = customValue

internal/converter/openapi_mlmd_converter_util.go

@@ -9,6 +9,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/kubeflow/model-registry/internal/defaults"
 	"github.com/kubeflow/model-registry/internal/ml_metadata/proto"
+	"github.com/kubeflow/model-registry/pkg/api"
 	"github.com/kubeflow/model-registry/pkg/openapi"
 	"google.golang.org/protobuf/types/known/structpb"
 )
@@ -65,7 +66,7 @@ func MapOpenAPICustomProperties(source *map[string]openapi.MetadataValue) (map[s
 			case v.MetadataIntValue != nil:
 				intValue, err := StringToInt64(&v.MetadataIntValue.IntValue)
 				if err != nil {
-					return nil, fmt.Errorf("unable to decode as int64 %w for key %s", err, key)
+					return nil, fmt.Errorf("%w: unable to decode as int64 %w for key %s", api.ErrBadRequest, err, key)
 				}
 				value.Value = &proto.Value_IntValue{IntValue: *intValue}
 			// double value
@@ -78,22 +79,22 @@ func MapOpenAPICustomProperties(source *map[string]openapi.MetadataValue) (map[s
 			case v.MetadataStructValue != nil:
 				data, err := base64.StdEncoding.DecodeString(v.MetadataStructValue.StructValue)
 				if err != nil {
-					return nil, fmt.Errorf("unable to decode %w for key %s", err, key)
+					return nil, fmt.Errorf("%w: unable to decode %w for key %s", api.ErrBadRequest, err, key)
 				}
 				var asMap map[string]interface{}
 				err = json.Unmarshal(data, &asMap)
 				if err != nil {
-					return nil, fmt.Errorf("unable to decode %w for key %s", err, key)
+					return nil, fmt.Errorf("%w: unable to decode %w for key %s", api.ErrBadRequest, err, key)
 				}
 				asStruct, err := structpb.NewStruct(asMap)
 				if err != nil {
-					return nil, fmt.Errorf("unable to decode %w for key %s", err, key)
+					return nil, fmt.Errorf("%w: unable to decode %w for key %s", api.ErrBadRequest, err, key)
 				}
 				value.Value = &proto.Value_StructValue{
 					StructValue: asStruct,
 				}
 			default:
-				return nil, fmt.Errorf("type mapping not found for %s:%v", key, v)
+				return nil, fmt.Errorf("%w: metadataType not found for %s: %v", api.ErrBadRequest, key, v)
 			}
 
 			props[key] = &value

pkg/api/error.go

@@ -26,12 +26,13 @@ func ErrToStatus(err error) int {
 		}
 	}
 
-	switch errors.Unwrap(err) {
-	case ErrBadRequest:
+	if errors.Is(err, ErrBadRequest) {
 		return http.StatusBadRequest
-	case ErrNotFound:
+	}
+	if errors.Is(err, ErrNotFound) {
 		return http.StatusNotFound
-	default:
-		return http.StatusInternalServerError
 	}
+
+	// Default error to return
+	return http.StatusInternalServerError
 }

pkg/core/registered_model_test.go

@@ -464,3 +464,24 @@ func (suite *CoreTestSuite) TestGetRegisteredModelsWithPageSize() {
 	suite.Equal(*secondModel.Id, *truncatedList.Items[0].Id)
 	suite.Equal(*thirdModel.Id, *truncatedList.Items[1].Id)
 }
+
+func (suite *CoreTestSuite) TestCreateRegisteredModelWithCustomPropFailure() {
+	// create mode registry service
+	service := suite.setupModelRegistryService()
+
+	// register a new model with incomplete customProperty fields
+	registeredModel := &openapi.RegisteredModel{
+		Name: modelName,
+		CustomProperties: &map[string]openapi.MetadataValue{
+			"myCustomProp1": {},
+		},
+	}
+
+	// test
+	_, err := service.UpsertRegisteredModel(registeredModel)
+
+	// checks
+	statusResp := api.ErrToStatus(err)
+	suite.NotNilf(err, "error creating registered model: %v", err)
+	suite.Equal(400, statusResp, "customProperties must include metadataType")
+}