fix(metadata): Invoke Validate function when adding YAML DeviceProfile

Add YAML format DeviceProfile should invoke Validate function to prevent uploading invalid DeviceProfile. Signed-off-by: weichou <[email protected]>
edgexfoundry · Jul 20, 2020 · 2c86856 · 2c86856
1 parent d3bef6b
commit 2c86856
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 10 deletions.
diff --git a/internal/core/metadata/rest_deviceprofile.go b/internal/core/metadata/rest_deviceprofile.go
@@ -37,6 +37,8 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
+const duplicatedCommandNameError = "duplicate names in device profile commands"
+
 func restGetAllDeviceProfiles(
 	w http.ResponseWriter,
 	lc logger.LoggingClient,
@@ -281,6 +283,16 @@ func restAddProfileByYaml(
 		return
 	}
 
+	err = validateDeviceProfileStruct(dp)
+	if err != nil {
+		if err.Error() == duplicatedCommandNameError {
+			errorHandler.Handle(w, err, errorconcept.DeviceProfile.ContractInvalid_StatusConflict)
+		} else {
+			errorHandler.Handle(w, err, errorconcept.Common.InvalidRequest_StatusBadRequest)
+		}
+		return
+	}
+
 	if configuration.Writable.EnableValueDescriptorManagement {
 		// Check if the device profile name is unique so that we do not create ValueDescriptors for a DeviceProfile that
 		// will fail during the creation process later on.
@@ -355,6 +367,16 @@ func restAddProfileByYamlRaw(
 		return
 	}
 
+	err = validateDeviceProfileStruct(dp)
+	if err != nil {
+		if err.Error() == duplicatedCommandNameError {
+			errorHandler.Handle(w, err, errorconcept.DeviceProfile.ContractInvalid_StatusConflict)
+		} else {
+			errorHandler.Handle(w, err, errorconcept.Common.InvalidRequest_StatusBadRequest)
+		}
+		return
+	}
+
 	if configuration.Writable.EnableValueDescriptorManagement {
 		// Check if the device profile name is unique so that we do not create ValueDescriptors for a DeviceProfile that
 		// will fail during the creation process later on.
@@ -658,3 +680,34 @@ func notifyProfileAssociates(
 
 	return nil
 }
+
+func validateDeviceProfileStruct(dp models.DeviceProfile) error {
+	_, err := dp.Validate()
+	if err != nil {
+		return err
+	}
+
+	for _, deviceCmd := range dp.DeviceCommands {
+		for _, ro := range deviceCmd.Get {
+			_, err := ro.Validate()
+			if err != nil {
+				return err
+			}
+		}
+		for _, ro := range deviceCmd.Set {
+			_, err := ro.Validate()
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	for _, coreCmd := range dp.CoreCommands {
+		_, err := coreCmd.Validate()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
diff --git a/internal/core/metadata/rest_deviceprofile_test.go b/internal/core/metadata/rest_deviceprofile_test.go
@@ -953,6 +953,16 @@ func TestAddProfileByYaml(t *testing.T) {
 	emptyName.Name = ""
 	emptyBody, _ := yaml.Marshal(emptyName)
 
+	emptyDeviceResource := createTestDeviceProfile()
+	emptyDeviceResource.DeviceCommands[0].Get = []contract.ResourceOperation{
+		{Index: "test index", Operation: "test operation", DeviceResource: ""},
+	}
+	emptyDeviceResourceBody, _ := yaml.Marshal(emptyDeviceResource)
+
+	emptyCoreCmdName := createTestDeviceProfile()
+	emptyCoreCmdName.CoreCommands[0].Name = ""
+	emptyCoreCmdNameBody, _ := yaml.Marshal(emptyCoreCmdName)
+
 	emptyFileRequest := createDeviceProfileRequestWithFile(okBody)
 	emptyFileRequest.MultipartForm = new(multipart.Form)
 	emptyFileRequest.MultipartForm.File = nil
@@ -1006,13 +1016,8 @@ func TestAddProfileByYaml(t *testing.T) {
 		{
 			"Duplicate command name",
 			createDeviceProfileRequestWithFile(dupeBody),
-			createDBClientWithOutlines([]mockOutline{
-				{"AddDeviceProfile", []interface{}{dp}, []interface{}{TestDeviceProfileID, nil}},
-				{"GetDeviceProfileByName", []interface{}{TestDeviceProfileName}, []interface{}{contract.DeviceProfile{}, db.ErrNotFound}},
-			}),
-			createMockValueDescriptorClient([]mockOutline{
-				{"Add", []interface{}{TestDeviceProfileValidated}, []interface{}{}},
-			}, requestContext, TestDeviceProfile, nil),
+			createDBClientWithOutlines([]mockOutline{}),
+			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
 			true,
 			http.StatusConflict,
 		},
@@ -1064,6 +1069,22 @@ func TestAddProfileByYaml(t *testing.T) {
 			false,
 			http.StatusOK,
 		},
+		{
+			"Resource operation's deviceResource is empty",
+			createDeviceProfileRequestWithFile(emptyDeviceResourceBody),
+			createDBClientWithOutlines([]mockOutline{}),
+			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
+			true,
+			http.StatusBadRequest,
+		},
+		{
+			"Core command name is empty",
+			createDeviceProfileRequestWithFile(emptyCoreCmdNameBody),
+			createDBClientWithOutlines([]mockOutline{}),
+			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
+			true,
+			http.StatusBadRequest,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1100,6 +1121,17 @@ func TestAddProfileByYamlRaw(t *testing.T) {
 	emptyName := dp
 	emptyName.Name = ""
 	emptyBody, _ := yaml.Marshal(emptyName)
+
+	emptyDeviceResource := createTestDeviceProfile()
+	emptyDeviceResource.DeviceCommands[0].Get = []contract.ResourceOperation{
+		{Index: "test index", Operation: "test operation", DeviceResource: ""},
+	}
+	emptyDeviceResourceBody, _ := yaml.Marshal(emptyDeviceResource)
+
+	emptyCoreCmdName := createTestDeviceProfile()
+	emptyCoreCmdName.CoreCommands[0].Name = ""
+	emptyCoreCmdNameBody, _ := yaml.Marshal(emptyCoreCmdName)
+
 	requestContext := httptest.NewRequest(http.MethodPost, AddressableTestURI, nil).Context()
 
 	tests := []struct {
@@ -1124,12 +1156,12 @@ func TestAddProfileByYamlRaw(t *testing.T) {
 			http.StatusOK,
 		},
 		{
-			"YAML unmarshal error",
-			createDeviceProfileRequestWithFile(dupeBody),
+			"Duplicate command name",
+			httptest.NewRequest(http.MethodPut, AddressableTestURI, bytes.NewBuffer(dupeBody)),
 			nil,
 			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
 			true,
-			http.StatusServiceUnavailable,
+			http.StatusConflict,
 		},
 		{
 			"Empty device profile name",
@@ -1168,6 +1200,22 @@ func TestAddProfileByYamlRaw(t *testing.T) {
 			false,
 			http.StatusOK,
 		},
+		{
+			"Resource operation's deviceResource is empty",
+			httptest.NewRequest(http.MethodPut, AddressableTestURI, bytes.NewBuffer(emptyDeviceResourceBody)),
+			createDBClientWithOutlines([]mockOutline{}),
+			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
+			true,
+			http.StatusBadRequest,
+		},
+		{
+			"Core command name is empty",
+			httptest.NewRequest(http.MethodPut, AddressableTestURI, bytes.NewBuffer(emptyCoreCmdNameBody)),
+			createDBClientWithOutlines([]mockOutline{}),
+			createMockValueDescriptorClient([]mockOutline{}, requestContext, TestDeviceProfile, nil),
+			true,
+			http.StatusBadRequest,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {