feat(metadata): Add query parameters for device parents/children. (#5053)

* feat(metadata): Add query parameters for device parents/children.

Closes: #4769

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

* fix: Repair a broken UT due to the added check in device deletion.

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

* fix: Stop tree queries if they get into a loop, which could happen if a
device was its own parent. Also try to keep that from happening in the first
place.

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

* fix: Add unit tests for tree queries and add/update/delete checks.

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

* fix: gofmt

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

---------

Signed-off-by: Corey Mutter <CoreyMutter@eaton.com>

Author: eaton-coreymutter

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 := validateProfileAndAutoEvent(dic, d)
+	err := validateParentProfileAndAutoEvent(dic, d)
 	if err != nil {
 		return "", errors.NewCommonEdgeXWrapper(err)
 	}
@@ -147,6 +147,13 @@ func DeleteDeviceByName(name string, ctx context.Context, dic *di.Container) err
 	if err != nil {
 		return errors.NewCommonEdgeXWrapper(err)
 	}
+	childcount, _, err := dbClient.DeviceTree(name, 1, 0, 1, nil)
+	if err != nil {
+		return errors.NewCommonEdgeXWrapper(err)
+	}
+	if childcount != 0 {
+		return errors.NewCommonEdgeX(errors.KindStatusConflict, "cannot delete device with children", nil)
+	}
 	err = dbClient.DeleteDeviceByName(name)
 	if err != nil {
 		return errors.NewCommonEdgeXWrapper(err)
@@ -225,7 +232,7 @@ func PatchDevice(dto dtos.UpdateDevice, ctx context.Context, dic *di.Container,
 
 	requests.ReplaceDeviceModelFieldsWithDTO(&device, dto)
 
-	err = validateProfileAndAutoEvent(dic, device)
+	err = validateParentProfileAndAutoEvent(dic, device)
 	if err != nil {
 		return errors.NewCommonEdgeXWrapper(err)
 	}
@@ -295,22 +302,30 @@ func deviceByDTO(dbClient interfaces.DBClient, dto dtos.UpdateDevice) (device mo
 }
 
 // AllDevices query the devices with offset, limit, and labels
-func AllDevices(offset int, limit int, labels []string, dic *di.Container) (devices []dtos.Device, totalCount uint32, err errors.EdgeX) {
+func AllDevices(offset int, limit int, labels []string, parent string, maxLevels int, dic *di.Container) (devices []dtos.Device, totalCount uint32, err errors.EdgeX) {
 	dbClient := container.DBClientFrom(dic.Get)
+	var deviceModels []models.Device
+	if parent != "" {
+		totalCount, deviceModels, err = dbClient.DeviceTree(parent, maxLevels, offset, limit, labels)
+		if err != nil {
+			return devices, totalCount, errors.NewCommonEdgeXWrapper(err)
+		}
+	} else {
+		totalCount, err = dbClient.DeviceCountByLabels(labels)
+		if err != nil {
+			return devices, totalCount, errors.NewCommonEdgeXWrapper(err)
+		}
+		cont, err := utils.CheckCountRange(totalCount, offset, limit)
+		if !cont {
+			return []dtos.Device{}, totalCount, err
+		}
 
-	totalCount, err = dbClient.DeviceCountByLabels(labels)
-	if err != nil {
-		return devices, totalCount, errors.NewCommonEdgeXWrapper(err)
-	}
-	cont, err := utils.CheckCountRange(totalCount, offset, limit)
-	if !cont {
-		return []dtos.Device{}, totalCount, err
+		deviceModels, err = dbClient.AllDevices(offset, limit, labels)
+		if err != nil {
+			return devices, totalCount, errors.NewCommonEdgeXWrapper(err)
+		}
 	}
 
-	deviceModels, err := dbClient.AllDevices(offset, limit, labels)
-	if err != nil {
-		return devices, totalCount, errors.NewCommonEdgeXWrapper(err)
-	}
 	devices = make([]dtos.Device, len(deviceModels))
 	for i, d := range deviceModels {
 		devices[i] = dtos.FromDeviceModelToDTO(d)
@@ -361,11 +376,14 @@ 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 validateProfileAndAutoEvent(dic *di.Container, d models.Device) errors.EdgeX {
+func validateParentProfileAndAutoEvent(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
 	}
+	if (d.Name == d.Parent) && (d.Name != "") {
+		return errors.NewCommonEdgeX(errors.KindContractInvalid, "a device cannot be its own parent", nil)
+	}
 	dbClient := container.DBClientFrom(dic.Get)
 	dp, err := dbClient.DeviceProfileByName(d.ProfileName)
 	if err != nil {

internal/core/metadata/application/device_test.go

@@ -24,7 +24,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestValidateProfileAndAutoEvents(t *testing.T) {
+func TestValidateParentProfileAndAutoEvents(t *testing.T) {
 	profile := "test-profile"
 	notFountProfileName := "notFoundProfile"
 	source1 := "source1"
@@ -107,10 +107,18 @@ func TestValidateProfileAndAutoEvents(t *testing.T) {
 			},
 			false,
 		},
+		{"is own parent",
+			models.Device{
+				ProfileName: profile,
+				Parent:      "me",
+				Name:        "me",
+			},
+			true,
+		},
 	}
 	for _, testCase := range tests {
 		t.Run(testCase.name, func(t *testing.T) {
-			err := validateProfileAndAutoEvent(dic, testCase.device)
+			err := validateParentProfileAndAutoEvent(dic, testCase.device)
 			if testCase.errorExpected {
 				assert.Error(t, err)
 			} else {

internal/core/metadata/controller/http/device.go

@@ -235,7 +235,15 @@ func (dc *DeviceController) AllDevices(c echo.Context) error {
 	if err != nil {
 		return utils.WriteErrorResponse(w, ctx, lc, err, "")
 	}
-	devices, totalCount, err := application.AllDevices(offset, limit, labels, dc.dic)
+	parent := utils.ParseQueryStringToString(r, common.DescendantsOf, "")
+	levels, err := utils.ParseQueryStringToInt(c, common.MaxLevels, 0, -1, math.MaxInt32)
+	if err != nil {
+		return utils.WriteErrorResponse(w, ctx, lc, err, "")
+	}
+	if levels < 0 {
+		levels = math.MaxInt32
+	}
+	devices, totalCount, err := application.AllDevices(offset, limit, labels, parent, levels, dc.dic)
 	if err != nil {
 		return utils.WriteErrorResponse(w, ctx, lc, err, "")
 	}

internal/core/metadata/controller/http/device_test.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -183,6 +184,8 @@ func TestAddDevice(t *testing.T) {
 	dbClientMock.On("AddDevice", emptyProtocolsModel).Return(emptyProtocolsModel, nil)
 	invalidProtocols := testDevice
 	invalidProtocols.Device.Protocols = map[string]dtos.ProtocolProperties{"others": {}}
+	ownParent := testDevice
+	ownParent.Device.Parent = ownParent.Device.Name
 
 	dic.Update(di.ServiceConstructorMap{
 		container.DBClientInterfaceName: func(get di.Get) interface{} {
@@ -217,6 +220,7 @@ func TestAddDevice(t *testing.T) {
 		{"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},
+		{"Invalid - own parent", []requests.AddDeviceRequest{ownParent}, http.StatusMultiStatus, http.StatusBadRequest, false, false, false},
 	}
 	for _, testCase := range tests {
 		t.Run(testCase.name, func(t *testing.T) {
@@ -319,14 +323,18 @@ func TestDeleteDeviceByName(t *testing.T) {
 	device := dtos.ToDeviceModel(buildTestDeviceRequest().Device)
 	noName := ""
 	notFoundName := "notFoundName"
+	deviceParent := device
+	deviceParent.Name = "someOtherName"
 
 	dic := mockDic()
 	dbClientMock := &dbMock.DBClient{}
+	dbClientMock.On("DeviceTree", device.Name, 1, 0, 1, []string(nil)).Return(uint32(0), nil, nil)
 	dbClientMock.On("DeleteDeviceByName", device.Name).Return(nil)
 	dbClientMock.On("DeleteDeviceByName", notFoundName).Return(edgexErr.NewCommonEdgeX(edgexErr.KindEntityDoesNotExist, "device doesn't exist in the database", nil))
 	dbClientMock.On("DeviceByName", notFoundName).Return(device, edgexErr.NewCommonEdgeX(edgexErr.KindEntityDoesNotExist, "device doesn't exist in the database", nil))
 	dbClientMock.On("DeviceByName", device.Name).Return(device, nil)
-	dbClientMock.On("DeviceServiceByName", device.ServiceName).Return(models.DeviceService{BaseAddress: testBaseAddress}, nil)
+	dbClientMock.On("DeviceByName", deviceParent.Name).Return(device, nil)
+	dbClientMock.On("DeviceTree", deviceParent.Name, 1, 0, 1, []string(nil)).Return(uint32(1), []models.Device{device}, nil)
 	dic.Update(di.ServiceConstructorMap{
 		container.DBClientInterfaceName: func(get di.Get) interface{} {
 			return dbClientMock
@@ -344,6 +352,7 @@ func TestDeleteDeviceByName(t *testing.T) {
 		{"Valid - delete device by name", device.Name, http.StatusOK},
 		{"Invalid - name parameter is empty", noName, http.StatusBadRequest},
 		{"Invalid - device not found by name", notFoundName, http.StatusNotFound},
+		{"Invalid - device has children", deviceParent.Name, http.StatusConflict},
 	}
 	for _, testCase := range tests {
 		t.Run(testCase.name, func(t *testing.T) {
@@ -603,14 +612,17 @@ func TestPatchDevice(t *testing.T) {
 	notFoundService.Device.ServiceName = &notFoundServiceName
 	dbClientMock.On("DeviceServiceNameExists", *notFoundService.Device.ServiceName).Return(false, nil)
 
-	notFountProfileName := "notFoundProfile"
+	notFoundProfileName := "notFoundProfile"
 	notFoundProfile := testReq
-	notFoundProfile.Device.ProfileName = &notFountProfileName
+	notFoundProfile.Device.ProfileName = &notFoundProfileName
 	notFoundProfileDeviceModel := dsModels
-	notFoundProfileDeviceModel.ProfileName = notFountProfileName
+	notFoundProfileDeviceModel.ProfileName = notFoundProfileName
 	dbClientMock.On("UpdateDevice", notFoundProfileDeviceModel).Return(
 		edgexErr.NewCommonEdgeX(edgexErr.KindEntityDoesNotExist,
-			fmt.Sprintf("device profile '%s' does not exists", notFountProfileName), nil))
+			fmt.Sprintf("device profile '%s' does not exists", notFoundProfileName), nil))
+
+	ownParent := testReq
+	ownParent.Device.Parent = ownParent.Device.Name
 
 	dic.Update(di.ServiceConstructorMap{
 		container.DBClientInterfaceName: func(get di.Get) interface{} {
@@ -642,7 +654,8 @@ func TestPatchDevice(t *testing.T) {
 		{"Invalid - invalid protocols", []requests.UpdateDeviceRequest{invalidProtocols}, http.StatusMultiStatus, http.StatusInternalServerError, true, false},
 		{"Invalid - not found device service", []requests.UpdateDeviceRequest{notFoundService}, http.StatusMultiStatus, http.StatusBadRequest, false, false},
 		{"Invalid - device service unavailable", []requests.UpdateDeviceRequest{valid}, http.StatusMultiStatus, http.StatusServiceUnavailable, true, false},
-		{"Valid - empty profile", []requests.UpdateDeviceRequest{emptyProfile}, http.StatusMultiStatus, http.StatusOK, true, true}}
+		{"Valid - empty profile", []requests.UpdateDeviceRequest{emptyProfile}, http.StatusMultiStatus, http.StatusOK, true, true},
+		{"Invalid - own parent", []requests.UpdateDeviceRequest{ownParent}, http.StatusMultiStatus, http.StatusBadRequest, false, false}}
 	for _, testCase := range tests {
 		t.Run(testCase.name, func(t *testing.T) {
 			e := echo.New()
@@ -749,6 +762,8 @@ func TestAllDevices(t *testing.T) {
 	dbClientMock.On("AllDevices", 0, 5, testDeviceLabels).Return([]models.Device{devices[0], devices[1]}, nil)
 	dbClientMock.On("AllDevices", 1, 2, []string(nil)).Return([]models.Device{devices[1], devices[2]}, nil)
 	dbClientMock.On("AllDevices", 4, 1, testDeviceLabels).Return([]models.Device{}, edgexErr.NewCommonEdgeX(edgexErr.KindRangeNotSatisfiable, "query objects bounds out of range.", nil))
+	dbClientMock.On("DeviceTree", "foo", 4, 0, 10, []string(nil)).Return(uint32(expectedDeviceTotalCount), devices, nil)
+	dbClientMock.On("DeviceTree", "foo", math.MaxInt32, 0, 10, testDeviceLabels).Return(uint32(expectedDeviceTotalCount), devices, nil)
 	dic.Update(di.ServiceConstructorMap{
 		container.DBClientInterfaceName: func(get di.Get) interface{} {
 			return dbClientMock
@@ -762,15 +777,20 @@ func TestAllDevices(t *testing.T) {
 		offset             string
 		limit              string
 		labels             string
+		descendantsOf      string
+		maxLevels          string
 		errorExpected      bool
 		expectedCount      int
 		expectedTotalCount uint32
 		expectedStatusCode int
 	}{
-		{"Valid - get devices without labels", "0", "10", "", false, 3, expectedDeviceTotalCount, http.StatusOK},
-		{"Valid - get devices with labels", "0", "5", strings.Join(testDeviceLabels, ","), false, 2, expectedDeviceTotalCount, http.StatusOK},
-		{"Valid - get devices with offset and no labels", "1", "2", "", false, 2, expectedDeviceTotalCount, http.StatusOK},
-		{"Invalid - offset out of range", "4", "1", strings.Join(testDeviceLabels, ","), true, 0, expectedDeviceTotalCount, http.StatusRequestedRangeNotSatisfiable},
+		{"Valid - get devices without labels", "0", "10", "", "", "", false, 3, expectedDeviceTotalCount, http.StatusOK},
+		{"Valid - get devices with labels", "0", "5", strings.Join(testDeviceLabels, ","), "", "", false, 2, expectedDeviceTotalCount, http.StatusOK},
+		{"Valid - get devices with offset and no labels", "1", "2", "", "", "", false, 2, expectedDeviceTotalCount, http.StatusOK},
+		{"Invalid - offset out of range", "4", "1", strings.Join(testDeviceLabels, ","), "", "", true, 0, expectedDeviceTotalCount, http.StatusRequestedRangeNotSatisfiable},
+		{"Valid - get tree without labels", "0", "10", "", "foo", "4", false, 3, expectedDeviceTotalCount, http.StatusOK},
+		{"Valid - get tree with labels", "0", "10", strings.Join(testDeviceLabels, ","), "foo", "-1", false, 3, expectedDeviceTotalCount, http.StatusOK},
+		{"Invalid - maxLevels bad integer", "4", "1", strings.Join(testDeviceLabels, ","), "foo", "bar", true, 0, 0, http.StatusBadRequest},
 	}
 	for _, testCase := range tests {
 		t.Run(testCase.name, func(t *testing.T) {
@@ -782,6 +802,12 @@ func TestAllDevices(t *testing.T) {
 			if len(testCase.labels) > 0 {
 				query.Add(common.Labels, testCase.labels)
 			}
+			if testCase.descendantsOf != "" {
+				query.Add(common.DescendantsOf, testCase.descendantsOf)
+			}
+			if testCase.maxLevels != "" {
+				query.Add(common.MaxLevels, testCase.maxLevels)
+			}
 			req.URL.RawQuery = query.Encode()
 			require.NoError(t, err)
 

internal/core/metadata/infrastructure/interfaces/db.go

@@ -53,7 +53,7 @@ type DBClient interface {
 	DeviceCountByLabels(labels []string) (uint32, errors.EdgeX)
 	DeviceCountByProfileName(profileName string) (uint32, errors.EdgeX)
 	DeviceCountByServiceName(serviceName string) (uint32, errors.EdgeX)
-
+	DeviceTree(parent string, levels int, offset int, limit int, labels []string) (uint32, []model.Device, errors.EdgeX)
 	AddProvisionWatcher(pw model.ProvisionWatcher) (model.ProvisionWatcher, errors.EdgeX)
 	ProvisionWatcherById(id string) (model.ProvisionWatcher, errors.EdgeX)
 	ProvisionWatcherByName(name string) (model.ProvisionWatcher, errors.EdgeX)

internal/core/metadata/infrastructure/interfaces/mocks/DBClient.go

@@ -84,6 +84,7 @@ const (
 	categoriesField       = "Categories"
 	createdField          = "Created"
 	labelsField           = "Labels"
+	parentField           = "Parent"
 	manufacturerField     = "Manufacturer"
 	modelField            = "Model"
 	nameField             = "Name"