From a09553d9be624636f508e9dfb2ede440f6f57f87 Mon Sep 17 00:00:00 2001 From: Ginny Guan Date: Mon, 14 Oct 2024 15:18:20 +0800 Subject: [PATCH] fix: Check profile existence before adding device/provision watcher close #4957 Signed-off-by: Ginny Guan --- internal/core/metadata/application/device.go | 14 +++++------ .../core/metadata/application/device_test.go | 19 +++++++++++---- .../metadata/application/provisionwatcher.go | 10 ++++++++ .../controller/http/provisionwatcher_test.go | 23 +------------------ 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/internal/core/metadata/application/device.go b/internal/core/metadata/application/device.go index ee67afea6b..ed13a2bed6 100644 --- a/internal/core/metadata/application/device.go +++ b/internal/core/metadata/application/device.go @@ -54,7 +54,7 @@ func AddDevice(d models.Device, ctx context.Context, dic *di.Container, bypassVa return id, errors.NewCommonEdgeX(errors.KindContractInvalid, fmt.Sprintf("device service '%s' does not exists", d.ServiceName), nil) } - err := validateAutoEvent(dic, d) + err := validateProfileAndAutoEvent(dic, d) if err != nil { return "", errors.NewCommonEdgeXWrapper(err) } @@ -218,7 +218,7 @@ func PatchDevice(dto dtos.UpdateDevice, ctx context.Context, dic *di.Container, requests.ReplaceDeviceModelFieldsWithDTO(&device, dto) - err = validateAutoEvent(dic, device) + err = validateProfileAndAutoEvent(dic, device) if err != nil { return errors.NewCommonEdgeXWrapper(err) } @@ -340,10 +340,7 @@ func DevicesByProfileName(offset int, limit int, profileName string, dic *di.Con var noMessagingClientError = goErrors.New("MessageBus Client not available. Please update RequireMessageBus and MessageBus configuration to enable sending System Events via the EdgeX MessageBus") -func validateAutoEvent(dic *di.Container, d models.Device) errors.EdgeX { - if len(d.AutoEvents) == 0 { - return nil - } +func validateProfileAndAutoEvent(dic *di.Container, d models.Device) errors.EdgeX { if d.ProfileName == "" { // if the profile is not set, skip the validation until we have the profile return nil @@ -351,7 +348,10 @@ func validateAutoEvent(dic *di.Container, d models.Device) errors.EdgeX { dbClient := container.DBClientFrom(dic.Get) dp, err := dbClient.DeviceProfileByName(d.ProfileName) if err != nil { - return errors.NewCommonEdgeX(errors.Kind(err), fmt.Sprintf("device profile '%s' not found during validating device '%s' auto event", d.ProfileName, d.Name), err) + return errors.NewCommonEdgeX(errors.Kind(err), fmt.Sprintf("device profile '%s' not found during validating device '%s'", d.ProfileName, d.Name), err) + } + if len(d.AutoEvents) == 0 { + return nil } for _, a := range d.AutoEvents { _, err := time.ParseDuration(a.Interval) diff --git a/internal/core/metadata/application/device_test.go b/internal/core/metadata/application/device_test.go index 1d97f75a63..00c65ecca8 100644 --- a/internal/core/metadata/application/device_test.go +++ b/internal/core/metadata/application/device_test.go @@ -21,12 +21,12 @@ import ( "github.com/edgexfoundry/go-mod-core-contracts/v3/models" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -func TestValidateAutoEvents(t *testing.T) { +func TestValidateProfileAndAutoEvents(t *testing.T) { profile := "test-profile" + notFountProfileName := "notFoundProfile" source1 := "source1" command1 := "command1" deviceProfile := models.DeviceProfile{ @@ -37,7 +37,8 @@ func TestValidateAutoEvents(t *testing.T) { dic := di.NewContainer(di.ServiceConstructorMap{}) dbClientMock := &mocks.DBClient{} - dbClientMock.On("DeviceProfileByName", mock.Anything).Return(deviceProfile, nil) + dbClientMock.On("DeviceProfileByName", profile).Return(deviceProfile, nil) + dbClientMock.On("DeviceProfileByName", notFountProfileName).Return(models.DeviceProfile{}, errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, "not found", nil)) dic.Update(di.ServiceConstructorMap{ container.DBClientInterfaceName: func(get di.Get) interface{} { return dbClientMock @@ -49,6 +50,16 @@ func TestValidateAutoEvents(t *testing.T) { device models.Device errorExpected bool }{ + {"empty profile", + models.Device{}, + false, + }, + {"not found profile", + models.Device{ + ProfileName: notFountProfileName, + }, + true, + }, {"no auto events", models.Device{ ProfileName: profile, @@ -99,7 +110,7 @@ func TestValidateAutoEvents(t *testing.T) { } for _, testCase := range tests { t.Run(testCase.name, func(t *testing.T) { - err := validateAutoEvent(dic, testCase.device) + err := validateProfileAndAutoEvent(dic, testCase.device) if testCase.errorExpected { assert.Error(t, err) } else { diff --git a/internal/core/metadata/application/provisionwatcher.go b/internal/core/metadata/application/provisionwatcher.go index 7db0cbaef8..ccbc696df6 100644 --- a/internal/core/metadata/application/provisionwatcher.go +++ b/internal/core/metadata/application/provisionwatcher.go @@ -29,6 +29,16 @@ func AddProvisionWatcher(pw models.ProvisionWatcher, ctx context.Context, dic *d lc := bootstrapContainer.LoggingClientFrom(dic.Get) correlationId := correlation.FromContext(ctx) + // check the associated ProfileName existence + if pw.DiscoveredDevice.ProfileName != "" { + exists, err := dbClient.DeviceProfileNameExists(pw.DiscoveredDevice.ProfileName) + if err != nil { + return "", errors.NewCommonEdgeXWrapper(err) + } else if !exists { + return "", errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, fmt.Sprintf("device profile '%s' does not exists", pw.DiscoveredDevice.ProfileName), err) + } + } + addProvisionWatcher, err := dbClient.AddProvisionWatcher(pw) if err != nil { return "", errors.NewCommonEdgeXWrapper(err) diff --git a/internal/core/metadata/controller/http/provisionwatcher_test.go b/internal/core/metadata/controller/http/provisionwatcher_test.go index 5b4dee0919..97b6e2f7ce 100644 --- a/internal/core/metadata/controller/http/provisionwatcher_test.go +++ b/internal/core/metadata/controller/http/provisionwatcher_test.go @@ -107,8 +107,6 @@ func TestProvisionWatcherController_AddProvisionWatcher_Created(t *testing.T) { dic := mockDic() dbClientMock := &mocks.DBClient{} dbClientMock.On("AddProvisionWatcher", pwModel).Return(pwModel, nil) - dbClientMock.On("DeviceServiceByName", pwModel.ServiceName).Return(models.DeviceService{}, nil) - dbClientMock.On("DeviceServiceNameExists", pwModel.ServiceName).Return(true, nil) dbClientMock.On("DeviceProfileNameExists", pwModel.DiscoveredDevice.ProfileName).Return(true, nil) dic.Update(di.ServiceConstructorMap{ container.DBClientInterfaceName: func(get di.Get) interface{} { @@ -171,25 +169,10 @@ func TestProvisionWatcherController_AddProvisionWatcher_BadRequest(t *testing.T) noName := provisionWatcher noName.ProvisionWatcher.Name = "" - notFountServiceName := "notFoundService" - notFoundService := provisionWatcher - notFoundService.ProvisionWatcher.ServiceName = notFountServiceName - dbClientMock.On("DeviceServiceNameExists", notFoundService.ProvisionWatcher.ServiceName).Return(false, nil) notFountProfileName := "notFoundProfile" notFoundProfile := provisionWatcher notFoundProfile.ProvisionWatcher.DiscoveredDevice.ProfileName = notFountProfileName - notFoundServiceProvisionWatcherModel := requests.AddProvisionWatcherReqToProvisionWatcherModels( - []requests.AddProvisionWatcherRequest{notFoundService})[0] - dbClientMock.On("AddProvisionWatcher", notFoundServiceProvisionWatcherModel).Return( - notFoundServiceProvisionWatcherModel, - errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, fmt.Sprintf("device service '%s' does not exists", - notFountServiceName), nil)) - notFoundProfileProvisionWatcherModel := requests.AddProvisionWatcherReqToProvisionWatcherModels( - []requests.AddProvisionWatcherRequest{notFoundProfile})[0] - dbClientMock.On("AddProvisionWatcher", notFoundProfileProvisionWatcherModel).Return( - notFoundProfileProvisionWatcherModel, - errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, fmt.Sprintf("device profile '%s' does not exists", - notFountProfileName), nil)) + dbClientMock.On("DeviceProfileNameExists", notFountProfileName).Return(false, nil) dic.Update(di.ServiceConstructorMap{ container.DBClientInterfaceName: func(get di.Get) interface{} { @@ -207,7 +190,6 @@ func TestProvisionWatcherController_AddProvisionWatcher_BadRequest(t *testing.T) }{ {"Invalid - Bad requestId", []requests.AddProvisionWatcherRequest{badRequestId}, http.StatusBadRequest, http.StatusBadRequest}, {"Invalid - Bad name", []requests.AddProvisionWatcherRequest{noName}, http.StatusBadRequest, http.StatusBadRequest}, - {"Invalid - not found service", []requests.AddProvisionWatcherRequest{notFoundService}, http.StatusMultiStatus, http.StatusNotFound}, {"Invalid - not found profile", []requests.AddProvisionWatcherRequest{notFoundProfile}, http.StatusMultiStatus, http.StatusNotFound}, } for _, testCase := range tests { @@ -259,7 +241,6 @@ func TestProvisionWatcherController_AddProvisionWatcher_Duplicated(t *testing.T) dbClientMock := &mocks.DBClient{} dbClientMock.On("AddProvisionWatcher", duplicateNameModel).Return(duplicateNameModel, duplicateNameDBError) dbClientMock.On("AddProvisionWatcher", duplicateIdModel).Return(duplicateIdModel, duplicateIdDBError) - dbClientMock.On("DeviceServiceNameExists", duplicateIdRequest.ProvisionWatcher.ServiceName).Return(true, nil) dbClientMock.On("DeviceProfileNameExists", duplicateIdRequest.ProvisionWatcher.DiscoveredDevice.ProfileName).Return(true, nil) dic.Update(di.ServiceConstructorMap{ container.DBClientInterfaceName: func(get di.Get) interface{} { @@ -635,7 +616,6 @@ func TestProvisionWatcherController_DeleteProvisionWatcherByName(t *testing.T) { dbClientMock.On("ProvisionWatcherByName", provisionWatcher.Name).Return(provisionWatcher, nil) dbClientMock.On("ProvisionWatcherByName", notFoundName).Return(provisionWatcher, errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, "provision watcher doesn't exist in the database", nil)) dbClientMock.On("DeleteProvisionWatcherByName", provisionWatcher.Name).Return(nil) - dbClientMock.On("DeviceServiceByName", provisionWatcher.ServiceName).Return(models.DeviceService{}, nil) dic.Update(di.ServiceConstructorMap{ container.DBClientInterfaceName: func(get di.Get) interface{} { return dbClientMock @@ -712,7 +692,6 @@ func TestProvisionWatcherController_PatchProvisionWatcher(t *testing.T) { dbClientMock.On("DeviceProfileNameExists", *valid.ProvisionWatcher.DiscoveredDevice.ProfileName).Return(true, nil) dbClientMock.On("ProvisionWatcherByName", *valid.ProvisionWatcher.Name).Return(pwModels, nil) dbClientMock.On("UpdateProvisionWatcher", pwModels).Return(nil) - dbClientMock.On("DeviceServiceByName", *valid.ProvisionWatcher.ServiceName).Return(models.DeviceService{}, nil) validWithNoReqID := testReq validWithNoReqID.RequestId = "" validWithNoId := testReq