Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add the force query param to add device metadata API #4929

Merged
merged 2 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions internal/core/metadata/application/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const minAutoEventInterval = 1 * time.Millisecond

// The AddDevice function accepts the new device model from the controller function
// and then invokes AddDevice function of infrastructure layer to add new device
func AddDevice(d models.Device, ctx context.Context, dic *di.Container, bypassValidation bool) (id string, edgeXerr errors.EdgeX) {
func AddDevice(d models.Device, ctx context.Context, dic *di.Container, bypassValidation bool, force bool) (id string, edgeXerr errors.EdgeX) {
dbClient := container.DBClientFrom(dic.Get)
lc := bootstrapContainer.LoggingClientFrom(dic.Get)

Expand All @@ -59,9 +59,23 @@ func AddDevice(d models.Device, ctx context.Context, dic *di.Container, bypassVa
return "", errors.NewCommonEdgeXWrapper(err)
}

// Execute the Device Service Validation when bypassValidation is false by default
// Skip the Device Service Validation if bypassValidation is true
if !bypassValidation {
// check if device name already exists
exists, err = dbClient.DeviceNameExists(d.Name)
if err != nil {
return "", errors.NewCommonEdgeXWrapper(err)
}
if exists {
if force {
// invoke forceAddDevice if force flag is enabled
return forceAddDevice(d, ctx, dic)
} else {
return "", errors.NewCommonEdgeX(errors.KindDuplicateName, fmt.Sprintf("device name %s already exists", d.Name), nil)
}
}

// Execute the Device Service Validation when both bypassValidation/force values are false by default
// Skip the Device Service Validation if either bypassValidation or force is true
if !(bypassValidation || force) {
err = validateDeviceCallback(dtos.FromDeviceModelToDTO(d), dic)
if err != nil {
return "", errors.NewCommonEdgeXWrapper(err)
Expand Down Expand Up @@ -90,6 +104,49 @@ func AddDevice(d models.Device, ctx context.Context, dic *di.Container, bypassVa
return addedDevice.Id, nil
}

// forceAddDevice accepts the updated device model from AddDevice function if force flag is enabled
// and then invokes UpdateDevice function of infrastructure layer to update the existing device
// however, "delete device" and "add device" system events will be published to the msg bus instead of "add device" which indicates a force add device action
func forceAddDevice(d models.Device, ctx context.Context, dic *di.Container) (id string, edgeXerr errors.EdgeX) {
cloudxxx8 marked this conversation as resolved.
Show resolved Hide resolved
dbClient := container.DBClientFrom(dic.Get)
lc := bootstrapContainer.LoggingClientFrom(dic.Get)

oldDevice, err := dbClient.DeviceByName(d.Name)
if err != nil {
return "", errors.NewCommonEdgeXWrapper(edgeXerr)
}

// set the id and created fields from the old device
if d.Id == "" {
d.Id = oldDevice.Id
}
if d.Created == 0 {
d.Created = oldDevice.Created
}

err = dbClient.UpdateDevice(d)
if err != nil {
return "", errors.NewCommonEdgeXWrapper(err)
}

lc.Debugf(
"Force add device executed successfully. Correlation-ID: %s ",
correlation.FromContext(ctx),
)

// If device is successfully updated, check each AutoEvent interval value and display a warning if it's smaller than the suggested 10ms value
for _, autoEvent := range d.AutoEvents {
utils.CheckMinInterval(autoEvent.Interval, minAutoEventInterval, lc)
}

deviceDTO := dtos.FromDeviceModelToDTO(d)
// publish the "delete device" and "add device" system events continuously
publishSystemEvent(common.DeviceSystemEventType, common.SystemEventActionDelete, d.ServiceName, deviceDTO, ctx, dic)
go publishSystemEvent(common.DeviceSystemEventType, common.SystemEventActionAdd, d.ServiceName, deviceDTO, ctx, dic)
cloudxxx8 marked this conversation as resolved.
Show resolved Hide resolved

return d.Id, nil
}

// DeleteDeviceByName deletes the device by name
func DeleteDeviceByName(name string, ctx context.Context, dic *di.Container) errors.EdgeX {
if name == "" {
Expand Down
66 changes: 65 additions & 1 deletion internal/core/metadata/application/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,23 @@
package application

import (
"github.com/edgexfoundry/edgex-go/internal/core/metadata/container"
"context"
"testing"

"github.com/edgexfoundry/edgex-go/internal/core/metadata/config"
"github.com/edgexfoundry/edgex-go/internal/core/metadata/container"
"github.com/edgexfoundry/edgex-go/internal/core/metadata/infrastructure/interfaces/mocks"
"github.com/edgexfoundry/edgex-go/internal/pkg/correlation"

bootstrapContainer "github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/container"
"github.com/edgexfoundry/go-mod-bootstrap/v3/di"
"github.com/edgexfoundry/go-mod-core-contracts/v3/clients/logger"
"github.com/edgexfoundry/go-mod-core-contracts/v3/errors"
"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) {
Expand Down Expand Up @@ -100,3 +108,59 @@ func TestValidateAutoEvents(t *testing.T) {
})
}
}

func TestForceAddDevice(t *testing.T) {
invalidDeviceName := "invalidDevice"
validDeviceName := "validDevice"
invalidDeviceName2 := "invalidDevice2"
invalidDevice := models.Device{Name: invalidDeviceName}
invalidDevice2 := models.Device{Name: invalidDeviceName2}
returnedDevice := models.Device{Name: validDeviceName}

dic := di.NewContainer(di.ServiceConstructorMap{
bootstrapContainer.LoggingClientInterfaceName: func(get di.Get) interface{} {
return logger.NewMockClient()
},
container.ConfigurationName: func(get di.Get) interface{} {
return &config.ConfigurationStruct{
Writable: config.WritableInfo{
LogLevel: "DEBUG",
},
}
},
})

dbClientMock := &mocks.DBClient{}
dbClientMock.On("DeviceByName", invalidDeviceName).Return(models.Device{}, errors.NewCommonEdgeX(errors.KindDatabaseError, "failed to query", nil))
dbClientMock.On("DeviceByName", validDeviceName).Return(returnedDevice, nil)
dbClientMock.On("DeviceByName", invalidDeviceName2).Return(invalidDevice2, nil)
dbClientMock.On("UpdateDevice", returnedDevice).Return(nil)
dbClientMock.On("UpdateDevice", invalidDevice2).Return(errors.NewCommonEdgeX(errors.KindDatabaseError, "failed to update", nil))
dic.Update(di.ServiceConstructorMap{
container.DBClientInterfaceName: func(get di.Get) interface{} {
return dbClientMock
},
})

tests := []struct {
name string
device models.Device
errorExpected bool
}{
{"invalid - DeviceByName error", invalidDevice, true},
{"valid", returnedDevice, false},
{"invalid - UpdateDevice error", invalidDevice2, true},
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
ctx, _ := correlation.FromContextOrNew(context.Background())
result, err := forceAddDevice(testCase.device, ctx, dic)
if testCase.errorExpected {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, returnedDevice.Id, result)
}
})
}
}
14 changes: 11 additions & 3 deletions internal/core/metadata/controller/http/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import (
"github.com/labstack/echo/v4"
)

const bypassValidationQueryParam = "bypassValidation" // query param to specify whether to skip the Device Service Validation API call
const (
bypassValidationQueryParam = "bypassValidation" // query param to specify whether to skip the Device Service Validation API call
forceQueryParam = "force" // query param to specify whether to force add a device
)

type DeviceController struct {
reader io.DtoReader
Expand All @@ -54,12 +57,17 @@ func (dc *DeviceController) AddDevice(c echo.Context) error {
ctx := r.Context()
correlationId := correlation.FromContext(ctx)

var bypassValidation bool
var bypassValidation, force bool
// parse URL query string for bypassValidation
bypassValidationParamStr := utils.ParseQueryStringToString(r, bypassValidationQueryParam, common.ValueFalse)
if bypassValidationParamStr == common.ValueTrue {
bypassValidation = true
}
// parse URL query string for force add device
forceParamStr := utils.ParseQueryStringToString(r, forceQueryParam, common.ValueFalse)
if forceParamStr == common.ValueTrue {
force = true
}

var reqDTOs []requests.AddDeviceRequest
err := dc.reader.Read(r.Body, &reqDTOs)
Expand All @@ -72,7 +80,7 @@ func (dc *DeviceController) AddDevice(c echo.Context) error {
for i, d := range devices {
var response interface{}
reqId := reqDTOs[i].RequestId
newId, err := application.AddDevice(d, ctx, dc.dic, bypassValidation)
newId, err := application.AddDevice(d, ctx, dc.dic, bypassValidation, force)
if err != nil {
lc.Error(err.Error(), common.CorrelationHeader, correlationId)
lc.Debug(err.DebugMessages(), common.CorrelationHeader, correlationId)
Expand Down
56 changes: 40 additions & 16 deletions internal/core/metadata/controller/http/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,17 @@ func TestAddDevice(t *testing.T) {

valid := testDevice
dbClientMock.On("DeviceServiceNameExists", deviceModel.ServiceName).Return(true, nil)
dbClientMock.On("DeviceNameExists", deviceModel.Name).Return(false, nil)
dbClientMock.On("AddDevice", deviceModel).Return(deviceModel, nil)
dbClientMock.On("DeviceProfileByName", mock.Anything).Return(models.DeviceProfile{Name: "test-profile", DeviceResources: []models.DeviceResource{{Name: "TestResource"}}}, nil)

validForceAdd := testDevice
validForceAdd.Device.Name = "forceAdd"
forceAddDm := dtos.ToDeviceModel(validForceAdd.Device)
dbClientMock.On("DeviceNameExists", validForceAdd.Device.Name).Return(true, nil)
dbClientMock.On("DeviceByName", validForceAdd.Device.Name).Return(forceAddDm, nil)
dbClientMock.On("UpdateDevice", forceAddDm).Return(nil)

notFoundProfile := testDevice
notFoundProfile.Device.ProfileName = "notFoundProfile"
notFoundProfileDeviceModel := requests.AddDeviceReqToDeviceModels([]requests.AddDeviceRequest{notFoundProfile})[0]
Expand Down Expand Up @@ -190,23 +198,25 @@ func TestAddDevice(t *testing.T) {
expectedResponseCode int
expectedValidation bool
expectedSystemEvent bool
forceAdd bool
}{
{"Valid", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusCreated, true, true},
{"Valid - bypassValidation", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusCreated, false, true},
{"Valid - no profile name and no auto events", []requests.AddDeviceRequest{noProfileAndAutoEvents}, http.StatusMultiStatus, http.StatusCreated, true, true},
{"Invalid - not found profile", []requests.AddDeviceRequest{notFoundProfile}, http.StatusMultiStatus, http.StatusNotFound, true, false},
{"Invalid - no name", []requests.AddDeviceRequest{noName}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Invalid - no adminState", []requests.AddDeviceRequest{noAdminState}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Invalid - no operatingState", []requests.AddDeviceRequest{noOperatingState}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Invalid - invalid adminState", []requests.AddDeviceRequest{invalidAdminState}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Invalid - invalid operatingState", []requests.AddDeviceRequest{invalidOperatingState}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Invalid - no service name", []requests.AddDeviceRequest{noServiceName}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Valid - no profile name", []requests.AddDeviceRequest{noProfileName}, http.StatusMultiStatus, http.StatusCreated, true, true},
{"Invalid - no protocols", []requests.AddDeviceRequest{noProtocols}, http.StatusBadRequest, http.StatusBadRequest, false, false},
{"Valid - empty protocols", []requests.AddDeviceRequest{emptyProtocols}, http.StatusMultiStatus, http.StatusCreated, true, true},
{"Invalid - invalid protocols", []requests.AddDeviceRequest{invalidProtocols}, http.StatusMultiStatus, http.StatusInternalServerError, true, false},
{"Invalid - not found device service", []requests.AddDeviceRequest{notFoundService}, http.StatusMultiStatus, http.StatusBadRequest, false, false},
{"Invalid - device service unavailable", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusServiceUnavailable, true, false},
{"Valid", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusCreated, true, true, false},
{"Valid - bypassValidation", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusCreated, false, true, false},
{"Valid - no profile name and no auto events", []requests.AddDeviceRequest{noProfileAndAutoEvents}, http.StatusMultiStatus, http.StatusCreated, true, true, false},
{"Invalid - not found profile", []requests.AddDeviceRequest{notFoundProfile}, http.StatusMultiStatus, http.StatusNotFound, true, false, false},
{"Invalid - no name", []requests.AddDeviceRequest{noName}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Invalid - no adminState", []requests.AddDeviceRequest{noAdminState}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Invalid - no operatingState", []requests.AddDeviceRequest{noOperatingState}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Invalid - invalid adminState", []requests.AddDeviceRequest{invalidAdminState}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Invalid - invalid operatingState", []requests.AddDeviceRequest{invalidOperatingState}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Invalid - no service name", []requests.AddDeviceRequest{noServiceName}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Valid - no profile name", []requests.AddDeviceRequest{noProfileName}, http.StatusMultiStatus, http.StatusCreated, true, true, false},
{"Invalid - no protocols", []requests.AddDeviceRequest{noProtocols}, http.StatusBadRequest, http.StatusBadRequest, false, false, false},
{"Valid - empty protocols", []requests.AddDeviceRequest{emptyProtocols}, http.StatusMultiStatus, http.StatusCreated, true, true, false},
{"Invalid - invalid protocols", []requests.AddDeviceRequest{invalidProtocols}, http.StatusMultiStatus, http.StatusInternalServerError, true, false, false},
{"Invalid - not found device service", []requests.AddDeviceRequest{notFoundService}, http.StatusMultiStatus, http.StatusBadRequest, false, false, false},
{"Invalid - device service unavailable", []requests.AddDeviceRequest{valid}, http.StatusMultiStatus, http.StatusServiceUnavailable, true, false, false},
{"Valid - force add device", []requests.AddDeviceRequest{validForceAdd}, http.StatusMultiStatus, http.StatusCreated, false, true, true},
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
Expand Down Expand Up @@ -241,6 +251,14 @@ func TestAddDevice(t *testing.T) {
mockMessaging.On("Publish", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
wg.Done()
}).Return(nil)

// if forceAdd flag is enabled, add the second publish event
if testCase.forceAdd {
wg.Add(1)
mockMessaging.On("Publish", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
wg.Done()
}).Return(nil)
}
}

dic.Update(di.ServiceConstructorMap{
Expand All @@ -259,6 +277,12 @@ func TestAddDevice(t *testing.T) {
req.URL.RawQuery = query.Encode()
}

if testCase.forceAdd {
query := req.URL.Query()
query.Add(forceQueryParam, common.ValueTrue)
req.URL.RawQuery = query.Encode()
}

// Act
recorder := httptest.NewRecorder()
c := e.NewContext(req, recorder)
Expand Down
5 changes: 0 additions & 5 deletions internal/pkg/infrastructure/postgres/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ func (c *Client) AddDevice(d model.Device) (model.Device, errors.EdgeX) {
d.Id = uuid.New().String()
}

exists, _ := deviceNameExists(ctx, c.ConnPool, d.Name)
cloudxxx8 marked this conversation as resolved.
Show resolved Hide resolved
if exists {
return model.Device{}, errors.NewCommonEdgeX(errors.KindDuplicateName, fmt.Sprintf("device name %s already exists", d.Name), nil)
}

timestamp := pkgCommon.MakeTimestamp()
d.Created = timestamp
d.Modified = timestamp
Expand Down
7 changes: 0 additions & 7 deletions internal/pkg/infrastructure/redis/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,6 @@ func addDevice(conn redis.Conn, d models.Device) (models.Device, errors.EdgeX) {
return d, errors.NewCommonEdgeX(errors.KindDuplicateName, fmt.Sprintf("device id %s already exists", d.Id), edgeXerr)
}

exists, edgeXerr = deviceNameExists(conn, d.Name)
if edgeXerr != nil {
return d, errors.NewCommonEdgeXWrapper(edgeXerr)
} else if exists {
return d, errors.NewCommonEdgeX(errors.KindDuplicateName, fmt.Sprintf("device name %s already exists", d.Name), edgeXerr)
}

ts := pkgCommon.MakeTimestamp()
if d.Created == 0 {
d.Created = ts
Expand Down
10 changes: 10 additions & 0 deletions openapi/v3/core-metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,14 @@ components:
type: boolean
description: "Indicates whether to skip the Device Service Validation API call."
default: false
forceParam:
in: query
name: force
required: false
schema:
type: boolean
description: "Indicates whether to force add the device if device name already exists."
default: false
headers:
correlatedResponseHeader:
description: "A response header that returns the unique correlation ID used to initiate the request."
Expand Down Expand Up @@ -1661,6 +1669,8 @@ paths:
- $ref: '#/components/parameters/bypassValidationParam'
post:
summary: "Allows provisioning of a new device"
parameters:
- $ref: '#/components/parameters/forceParam'
requestBody:
required: true
content:
Expand Down
Loading