refactor(api): replace DeviceLookup with DeviceResolve

Author: heiytor

api/services/device.go

@@ -190,9 +190,14 @@ func (s *service) RenameDevice(ctx context.Context, uid models.UID, name, tenant
 // It receives a context, used to "control" the request flow and, the namespace name from a models.Namespace and a
 // device name from models.Device.
 func (s *service) LookupDevice(ctx context.Context, namespace, name string) (*models.Device, error) {
-	device, err := s.store.DeviceLookup(ctx, namespace, name)
+	n, err := s.store.NamespaceGetByName(ctx, namespace)
+	if err != nil {
+		return nil, NewErrNamespaceNotFound(namespace, err)
+	}
+
+	device, err := s.store.DeviceResolve(ctx, store.DeviceHostnameResolver, name, s.store.Options().InNamespace(n.TenantID))
 	if err != nil || device == nil {
-		return nil, NewErrDeviceLookupNotFound(namespace, name, err)
+		return nil, NewErrDeviceNotFound(models.UID(name), err)
 	}
 
 	return device, nil

api/services/device_test.go

@@ -1177,7 +1177,9 @@ func TestRenameDevice(t *testing.T) {
 }
 
 func TestLookupDevice(t *testing.T) {
-	mock := new(storemock.Store)
+	storeMock := new(storemock.Store)
+	queryOptionsMock := new(storemock.QueryOptions)
+	storeMock.On("Options").Return(queryOptionsMock)
 
 	ctx := context.TODO()
 
@@ -1194,37 +1196,60 @@ func TestLookupDevice(t *testing.T) {
 		expected      Expected
 	}{
 		{
-			description: "fails when store device lookup fails",
+			description: "fails when namespace does not exists",
 			namespace:   "namespace",
 			device:      &models.Device{UID: "uid", Name: "name", TenantID: "tenant", Identity: &models.DeviceIdentity{MAC: "00:00:00:00:00:00"}, Status: "accepted"},
-			requiredMocks: func(device *models.Device, namespace string) {
-				mock.On("DeviceLookup", ctx, namespace, device.Name).Return(nil, errors.New("error", "", 0)).Once()
+			requiredMocks: func(_ *models.Device, namespace string) {
+				storeMock.
+					On("NamespaceGetByName", ctx, namespace).
+					Return(nil, errors.New("error", "", 0)).
+					Once()
 			},
 			expected: Expected{
 				nil,
-				NewErrDeviceLookupNotFound("namespace", "name", errors.New("error", "", 0)),
+				NewErrNamespaceNotFound("namespace", errors.New("error", "", 0)),
 			},
 		},
 		{
-			description: "fails when the device is not found",
+			description: "fails when namespace does not exists",
 			namespace:   "namespace",
 			device:      &models.Device{UID: "uid", Name: "name", TenantID: "tenant", Identity: &models.DeviceIdentity{MAC: "00:00:00:00:00:00"}, Status: "accepted"},
 			requiredMocks: func(device *models.Device, namespace string) {
-				mock.On("DeviceLookup", ctx, namespace, device.Name).
-					Return(nil, store.ErrNoDocuments).Once()
+				storeMock.
+					On("NamespaceGetByName", ctx, namespace).
+					Return(&models.Namespace{TenantID: "00000000-0000-0000-0000-000000000000"}, nil).
+					Once()
+				queryOptionsMock.
+					On("InNamespace", "00000000-0000-0000-0000-000000000000").
+					Return(nil).
+					Once()
+				storeMock.
+					On("DeviceResolve", ctx, store.DeviceHostnameResolver, "name", mock.AnythingOfType("store.QueryOption")).
+					Return(nil, errors.New("error", "", 0)).
+					Once()
 			},
 			expected: Expected{
 				nil,
-				NewErrDeviceLookupNotFound("namespace", "name", store.ErrNoDocuments),
+				NewErrDeviceNotFound(models.UID("name"), errors.New("error", "", 0)),
 			},
 		},
 		{
 			description: "succeeds",
 			namespace:   "namespace",
 			device:      &models.Device{UID: "uid", Name: "name", TenantID: "tenant", Identity: &models.DeviceIdentity{MAC: "00:00:00:00:00:00"}, Status: "accepted"},
 			requiredMocks: func(device *models.Device, namespace string) {
-				mock.On("DeviceLookup", ctx, namespace, device.Name).
-					Return(device, nil).Once()
+				storeMock.
+					On("NamespaceGetByName", ctx, namespace).
+					Return(&models.Namespace{TenantID: "00000000-0000-0000-0000-000000000000"}, nil).
+					Once()
+				queryOptionsMock.
+					On("InNamespace", "00000000-0000-0000-0000-000000000000").
+					Return(nil).
+					Once()
+				storeMock.
+					On("DeviceResolve", ctx, store.DeviceHostnameResolver, "name", mock.AnythingOfType("store.QueryOption")).
+					Return(device, nil).
+					Once()
 			},
 			expected: Expected{
 				&models.Device{UID: "uid", Name: "name", TenantID: "tenant", Identity: &models.DeviceIdentity{MAC: "00:00:00:00:00:00"}, Status: "accepted"},
@@ -1237,12 +1262,12 @@ func TestLookupDevice(t *testing.T) {
 		t.Run(tc.description, func(t *testing.T) {
 			tc.requiredMocks(tc.device, tc.namespace)
 
-			service := NewService(store.Store(mock), privateKey, publicKey, storecache.NewNullCache(), clientMock)
+			service := NewService(store.Store(storeMock), privateKey, publicKey, storecache.NewNullCache(), clientMock)
 			returnedDevice, err := service.LookupDevice(ctx, tc.namespace, tc.device.Name)
 			assert.Equal(t, tc.expected, Expected{returnedDevice, err})
 		})
 	}
-	mock.AssertExpectations(t)
+	storeMock.AssertExpectations(t)
 }
 
 func TestOfflineDevice(t *testing.T) {

api/services/errors.go

@@ -2,7 +2,6 @@ package services
 
 import (
 	stderrors "errors"
-	"fmt"
 
 	"github.com/shellhub-io/shellhub/pkg/errors"
 	"github.com/shellhub-io/shellhub/pkg/models"
@@ -100,7 +99,6 @@ var (
 	ErrDeviceNotFound                  = errors.New("device not found", ErrLayer, ErrCodeNotFound)
 	ErrDeviceInvalid                   = errors.New("device invalid", ErrLayer, ErrCodeInvalid)
 	ErrDeviceDuplicated                = errors.New("device duplicated", ErrLayer, ErrCodeDuplicated)
-	ErrDeviceLookupNotFound            = errors.New("device lookup not found", ErrLayer, ErrCodeNotFound)
 	ErrDeviceLimit                     = errors.New("device limit reached", ErrLayer, ErrCodePayment)
 	ErrDeviceStatusInvalid             = errors.New("device status invalid", ErrLayer, ErrCodeInvalid)
 	ErrDeviceStatusAccepted            = errors.New("device status accepted", ErrLayer, ErrCodeInvalid)
@@ -378,11 +376,6 @@ func NewErrDeviceDuplicated(name string, next error) error {
 	return NewErrDuplicated(ErrDeviceDuplicated, []string{name}, next)
 }
 
-// NewErrDeviceLookupNotFound returns an error to be used when the device lookup is not found.
-func NewErrDeviceLookupNotFound(namespace, name string, next error) error {
-	return NewErrNotFound(ErrDeviceLookupNotFound, fmt.Sprintf("device %s on namespace %s", name, namespace), next)
-}
-
 // NewErrDeviceLimit returns an error to be used when the device limit is reached.
 func NewErrDeviceLimit(limit int, next error) error {
 	return NewErrLimit(ErrDeviceLimit, limit, next)

api/store/device.go

@@ -53,7 +53,6 @@ type DeviceStore interface {
 	DeviceDelete(ctx context.Context, uid models.UID) error
 	DeviceCreate(ctx context.Context, d models.Device, hostname string) error
 	DeviceRename(ctx context.Context, uid models.UID, hostname string) error
-	DeviceLookup(ctx context.Context, namespace, hostname string) (*models.Device, error)
 	DeviceUpdateStatus(ctx context.Context, uid models.UID, status models.DeviceStatus) error
 	DeviceSetPosition(ctx context.Context, uid models.UID, position models.DevicePosition) error
 	DeviceListByUsage(ctx context.Context, tenantID string) ([]models.UID, error)

api/store/mocks/store.go

@@ -321,20 +321,6 @@ func (s *Store) DeviceRename(ctx context.Context, uid models.UID, hostname strin
 	return nil
 }
 
-func (s *Store) DeviceLookup(ctx context.Context, namespace, hostname string) (*models.Device, error) {
-	ns := new(models.Namespace)
-	if err := s.db.Collection("namespaces").FindOne(ctx, bson.M{"name": namespace}).Decode(&ns); err != nil {
-		return nil, FromMongoError(err)
-	}
-
-	device := new(models.Device)
-	if err := s.db.Collection("devices").FindOne(ctx, bson.M{"tenant_id": ns.TenantID, "name": hostname, "status": "accepted"}).Decode(&device); err != nil {
-		return nil, FromMongoError(err)
-	}
-
-	return device, nil
-}
-
 // DeviceUpdateStatus updates the status of a specific device in the devices collection
 func (s *Store) DeviceUpdateStatus(ctx context.Context, uid models.UID, status models.DeviceStatus) error {
 	updateOptions := options.FindOneAndUpdate().SetReturnDocument(options.After)

api/store/mongo/device.go

@@ -658,101 +658,6 @@ func TestDeviceResolve(t *testing.T) {
 	}
 }
 
-func TestDeviceLookup(t *testing.T) {
-	type Expected struct {
-		dev *models.Device
-		err error
-	}
-	cases := []struct {
-		description string
-		namespace   string
-		hostname    string
-		fixtures    []string
-		expected    Expected
-	}{
-		{
-			description: "fails when namespace does not exist",
-			namespace:   "nonexistent",
-			hostname:    "device-3",
-			fixtures:    []string{fixtureNamespaces, fixtureDevices},
-			expected: Expected{
-				dev: nil,
-				err: store.ErrNoDocuments,
-			},
-		},
-		{
-			description: "fails when device does not exist due to name",
-			namespace:   "namespace-1",
-			hostname:    "nonexistent",
-			fixtures:    []string{fixtureNamespaces, fixtureDevices},
-			expected: Expected{
-				dev: nil,
-				err: store.ErrNoDocuments,
-			},
-		},
-		{
-			description: "fails when device does not exist due to tenant-id",
-			namespace:   "namespace-1",
-			hostname:    "invalid_tenant",
-			fixtures:    []string{fixtureNamespaces, fixtureDevices},
-			expected: Expected{
-				dev: nil,
-				err: store.ErrNoDocuments,
-			},
-		},
-		{
-			description: "fails when device does not exist due to status other than accepted",
-			namespace:   "namespace-1",
-			hostname:    "pending",
-			fixtures:    []string{fixtureNamespaces, fixtureDevices},
-			expected: Expected{
-				dev: nil,
-				err: store.ErrNoDocuments,
-			},
-		},
-		{
-			description: "succeeds when namespace exists and hostname status is accepted",
-			namespace:   "namespace-1",
-			hostname:    "device-3",
-			fixtures:    []string{fixtureNamespaces, fixtureDevices},
-			expected: Expected{
-				dev: &models.Device{
-					CreatedAt:       time.Date(2023, 1, 3, 12, 0, 0, 0, time.UTC),
-					StatusUpdatedAt: time.Date(2023, 1, 3, 12, 0, 0, 0, time.UTC),
-					LastSeen:        time.Date(2023, 1, 3, 12, 0, 0, 0, time.UTC),
-					UID:             "2300230e3ca2f637636b4d025d2235269014865db5204b6d115386cbee89809c",
-					Name:            "device-3",
-					Identity:        &models.DeviceIdentity{MAC: "mac-3"},
-					Info:            nil,
-					PublicKey:       "",
-					TenantID:        "00000000-0000-4000-0000-000000000000",
-					Online:          false,
-					Status:          "accepted",
-					RemoteAddr:      "",
-					Position:        nil,
-					Tags:            []string{"tag-1"},
-					Acceptable:      false,
-				},
-				err: nil,
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.description, func(t *testing.T) {
-			ctx := context.Background()
-
-			assert.NoError(t, srv.Apply(tc.fixtures...))
-			t.Cleanup(func() {
-				assert.NoError(t, srv.Reset())
-			})
-
-			dev, err := s.DeviceLookup(ctx, tc.namespace, tc.hostname)
-			assert.Equal(t, tc.expected, Expected{dev: dev, err: err})
-		})
-	}
-}
-
 func TestDeviceCreate(t *testing.T) {
 	cases := []struct {
 		description string