Skip to content

Commit ebf155f

Browse files
heiytorheiytor
committed
refactor(api): update APIKey interface to use entity objects
- Change APIKeySave to accept *models.APIKey instead of separate parameters - Change APIKeyDelete to accept *models.APIKey instead of tenantID and name - Update service layer to resolve API key before save/delete operations - Add proper error handling for APIKeyResolve in update/delete flows - Remove APIKeyChanges model as it's no longer needed - Update all tests to match new interface signatures - Add QueryOptions mock for proper test isolation - Improve UI layout for device details with responsive grid - Fix cursor handling in APIKeyResolve to return ErrNoDocuments when empty BREAKING CHANGE: APIKeySave and APIKeyDelete method signatures changed
1 parent 52cbcf2 commit ebf155f

File tree

7 files changed

+302
-142
lines changed

7 files changed

+302
-142
lines changed

‎api/services/api-key.go‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,21 +115,49 @@ func (s *service) UpdateAPIKey(ctx context.Context, req *requests.UpdateAPIKey)
115115
}
116116
}
117117

118-
if conflicts, has, _ := s.store.APIKeyConflicts(ctx, req.TenantID, &models.APIKeyConflicts{Name: req.Name}); has {
119-
return NewErrAPIKeyDuplicated(conflicts)
118+
apiKey, err := s.store.APIKeyResolve(ctx, store.APIKeyNameResolver, req.CurrentName, s.store.Options().InNamespace(req.TenantID))
119+
if err != nil {
120+
switch {
121+
case errors.Is(err, store.ErrNoDocuments):
122+
return NewErrAPIKeyNotFound(req.CurrentName, err)
123+
default:
124+
return err
125+
}
126+
}
127+
128+
if apiKey.Name != req.Name {
129+
if conflicts, has, _ := s.store.APIKeyConflicts(ctx, req.TenantID, &models.APIKeyConflicts{Name: req.Name}); has {
130+
return NewErrAPIKeyDuplicated(conflicts)
131+
}
132+
}
133+
134+
if req.Name != "" {
135+
apiKey.Name = req.Name
136+
}
137+
if string(req.Role) != "" {
138+
apiKey.Role = req.Role
120139
}
121140

122-
change := &models.APIKeyChanges{Name: req.Name, Role: req.Role}
123-
if err := s.store.APIKeyUpdate(ctx, req.TenantID, req.CurrentName, change); err != nil {
124-
return NewErrAPIKeyNotFound(req.CurrentName, err)
141+
if err := s.store.APIKeySave(ctx, apiKey); err != nil { //nolint:revive
142+
return err
125143
}
126144

127145
return nil
128146
}
129147

130148
func (s *service) DeleteAPIKey(ctx context.Context, req *requests.DeleteAPIKey) error {
131-
if err := s.store.APIKeyDelete(ctx, req.TenantID, req.Name); err != nil {
132-
return NewErrAPIKeyNotFound(req.Name, err)
149+
apiKey, err := s.store.APIKeyResolve(ctx, store.APIKeyNameResolver, req.Name, s.store.Options().InNamespace(req.TenantID))
150+
if err != nil {
151+
switch {
152+
case errors.Is(err, store.ErrNoDocuments):
153+
return NewErrAPIKeyNotFound(req.Name, err)
154+
default:
155+
return err
156+
}
157+
}
158+
159+
if err := s.store.APIKeyDelete(ctx, apiKey); err != nil { //nolint:revive
160+
return err
133161
}
134162

135163
return nil

‎api/services/api-key_test.go‎

Lines changed: 190 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,8 @@ func TestListAPIKey(t *testing.T) {
513513

514514
func TestUpdateAPIKey(t *testing.T) {
515515
storeMock := new(storemock.Store)
516+
queryOptionsMock := new(storemock.QueryOptions)
517+
storeMock.On("Options").Return(queryOptionsMock)
516518

517519
cases := []struct {
518520
description string
@@ -554,6 +556,31 @@ func TestUpdateAPIKey(t *testing.T) {
554556
},
555557
expected: NewErrRoleInvalid(),
556558
},
559+
{
560+
description: "fails when api key does not exist for resolve",
561+
req: &requests.UpdateAPIKey{
562+
UserID: "000000000000000000000000",
563+
TenantID: "00000000-0000-4000-0000-000000000000",
564+
CurrentName: "nonexistent",
565+
Name: "newName",
566+
Role: "administrator",
567+
},
568+
requiredMocks: func(ctx context.Context) {
569+
storeMock.
570+
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
571+
Return(&models.Namespace{Members: []models.Member{{ID: "000000000000000000000000", Role: "owner"}}}, nil).
572+
Once()
573+
queryOptionsMock.
574+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
575+
Return(nil).
576+
Once()
577+
storeMock.
578+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "nonexistent", mock.AnythingOfType("store.QueryOption")).
579+
Return(nil, store.ErrNoDocuments).
580+
Once()
581+
},
582+
expected: NewErrAPIKeyNotFound("nonexistent", store.ErrNoDocuments),
583+
},
557584
{
558585
description: "fails when a conflict is found",
559586
req: &requests.UpdateAPIKey{
@@ -564,10 +591,25 @@ func TestUpdateAPIKey(t *testing.T) {
564591
Role: "administrator",
565592
},
566593
requiredMocks: func(ctx context.Context) {
594+
existingAPIKey := &models.APIKey{
595+
ID: "existing-id",
596+
Name: "dev",
597+
TenantID: "00000000-0000-4000-0000-000000000000",
598+
Role: "operator",
599+
}
600+
567601
storeMock.
568602
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
569603
Return(&models.Namespace{Members: []models.Member{{ID: "000000000000000000000000", Role: "owner"}}}, nil).
570604
Once()
605+
queryOptionsMock.
606+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
607+
Return(nil).
608+
Once()
609+
storeMock.
610+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
611+
Return(existingAPIKey, nil).
612+
Once()
571613
storeMock.
572614
On("APIKeyConflicts", ctx, "00000000-0000-4000-0000-000000000000", &models.APIKeyConflicts{Name: "newName"}).
573615
Return([]string{"name"}, true, nil).
@@ -576,7 +618,7 @@ func TestUpdateAPIKey(t *testing.T) {
576618
expected: NewErrAPIKeyDuplicated([]string{"name"}),
577619
},
578620
{
579-
description: "fails when api key does not exists",
621+
description: "fails when api key save fails",
580622
req: &requests.UpdateAPIKey{
581623
UserID: "000000000000000000000000",
582624
TenantID: "00000000-0000-4000-0000-000000000000",
@@ -585,20 +627,42 @@ func TestUpdateAPIKey(t *testing.T) {
585627
Role: "administrator",
586628
},
587629
requiredMocks: func(ctx context.Context) {
630+
existingAPIKey := &models.APIKey{
631+
ID: "existing-id",
632+
Name: "dev",
633+
TenantID: "00000000-0000-4000-0000-000000000000",
634+
Role: "operator",
635+
}
636+
637+
updatedAPIKey := &models.APIKey{
638+
ID: "existing-id",
639+
Name: "newName",
640+
TenantID: "00000000-0000-4000-0000-000000000000",
641+
Role: "administrator",
642+
}
643+
588644
storeMock.
589645
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
590646
Return(&models.Namespace{Members: []models.Member{{ID: "000000000000000000000000", Role: "owner"}}}, nil).
591647
Once()
648+
queryOptionsMock.
649+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
650+
Return(nil).
651+
Once()
652+
storeMock.
653+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
654+
Return(existingAPIKey, nil).
655+
Once()
592656
storeMock.
593657
On("APIKeyConflicts", ctx, "00000000-0000-4000-0000-000000000000", &models.APIKeyConflicts{Name: "newName"}).
594658
Return([]string{}, false, nil).
595659
Once()
596660
storeMock.
597-
On("APIKeyUpdate", ctx, "00000000-0000-4000-0000-000000000000", "dev", &models.APIKeyChanges{Name: "newName", Role: "administrator"}).
598-
Return(errors.New("error")).
661+
On("APIKeySave", ctx, updatedAPIKey).
662+
Return(errors.New("save error")).
599663
Once()
600664
},
601-
expected: NewErrAPIKeyNotFound("dev", errors.New("error")),
665+
expected: errors.New("save error"),
602666
},
603667
{
604668
description: "succeeds",
@@ -610,16 +674,81 @@ func TestUpdateAPIKey(t *testing.T) {
610674
Role: "administrator",
611675
},
612676
requiredMocks: func(ctx context.Context) {
677+
existingAPIKey := &models.APIKey{
678+
ID: "existing-id",
679+
Name: "dev",
680+
TenantID: "00000000-0000-4000-0000-000000000000",
681+
Role: "operator",
682+
}
683+
684+
updatedAPIKey := &models.APIKey{
685+
ID: "existing-id",
686+
Name: "newName",
687+
TenantID: "00000000-0000-4000-0000-000000000000",
688+
Role: "administrator",
689+
}
690+
613691
storeMock.
614692
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
615693
Return(&models.Namespace{Members: []models.Member{{ID: "000000000000000000000000", Role: "owner"}}}, nil).
616694
Once()
695+
queryOptionsMock.
696+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
697+
Return(nil).
698+
Once()
699+
storeMock.
700+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
701+
Return(existingAPIKey, nil).
702+
Once()
617703
storeMock.
618704
On("APIKeyConflicts", ctx, "00000000-0000-4000-0000-000000000000", &models.APIKeyConflicts{Name: "newName"}).
619705
Return([]string{}, false, nil).
620706
Once()
621707
storeMock.
622-
On("APIKeyUpdate", ctx, "00000000-0000-4000-0000-000000000000", "dev", &models.APIKeyChanges{Name: "newName", Role: "administrator"}).
708+
On("APIKeySave", ctx, updatedAPIKey).
709+
Return(nil).
710+
Once()
711+
},
712+
expected: nil,
713+
},
714+
{
715+
description: "succeeds whithout updating the name",
716+
req: &requests.UpdateAPIKey{
717+
UserID: "000000000000000000000000",
718+
TenantID: "00000000-0000-4000-0000-000000000000",
719+
CurrentName: "dev",
720+
Name: "dev", // mesmo nome
721+
Role: "administrator",
722+
},
723+
requiredMocks: func(ctx context.Context) {
724+
existingAPIKey := &models.APIKey{
725+
ID: "existing-id",
726+
Name: "dev",
727+
TenantID: "00000000-0000-4000-0000-000000000000",
728+
Role: "operator",
729+
}
730+
731+
updatedAPIKey := &models.APIKey{
732+
ID: "existing-id",
733+
Name: "dev",
734+
TenantID: "00000000-0000-4000-0000-000000000000",
735+
Role: "administrator",
736+
}
737+
738+
storeMock.
739+
On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "00000000-0000-4000-0000-000000000000").
740+
Return(&models.Namespace{Members: []models.Member{{ID: "000000000000000000000000", Role: "owner"}}}, nil).
741+
Once()
742+
queryOptionsMock.
743+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
744+
Return(nil).
745+
Once()
746+
storeMock.
747+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
748+
Return(existingAPIKey, nil).
749+
Once()
750+
storeMock.
751+
On("APIKeySave", ctx, updatedAPIKey).
623752
Return(nil).
624753
Once()
625754
},
@@ -647,27 +776,61 @@ func TestUpdateAPIKey(t *testing.T) {
647776

648777
func TestDeleteAPIKey(t *testing.T) {
649778
storeMock := new(storemock.Store)
779+
queryOptionsMock := new(storemock.QueryOptions)
780+
storeMock.On("Options").Return(queryOptionsMock)
650781

651782
cases := []struct {
652783
description string
653-
tenantID string
654784
req *requests.DeleteAPIKey
655785
requiredMocks func(context.Context)
656786
expected error
657787
}{
658788
{
659-
description: "fails when api key does not exists",
789+
description: "fails when api key does not exist for resolve",
790+
req: &requests.DeleteAPIKey{
791+
TenantID: "00000000-0000-4000-0000-000000000000",
792+
Name: "nonexistent",
793+
},
794+
requiredMocks: func(ctx context.Context) {
795+
queryOptionsMock.
796+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
797+
Return(nil).
798+
Once()
799+
storeMock.
800+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "nonexistent", mock.AnythingOfType("store.QueryOption")).
801+
Return(nil, store.ErrNoDocuments).
802+
Once()
803+
},
804+
expected: NewErrAPIKeyNotFound("nonexistent", store.ErrNoDocuments),
805+
},
806+
{
807+
description: "fails when api key delete fails",
660808
req: &requests.DeleteAPIKey{
661809
TenantID: "00000000-0000-4000-0000-000000000000",
662810
Name: "dev",
663811
},
664812
requiredMocks: func(ctx context.Context) {
813+
existingAPIKey := &models.APIKey{
814+
ID: "existing-id",
815+
Name: "dev",
816+
TenantID: "00000000-0000-4000-0000-000000000000",
817+
Role: "operator",
818+
}
819+
820+
queryOptionsMock.
821+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
822+
Return(nil).
823+
Once()
665824
storeMock.
666-
On("APIKeyDelete", ctx, "00000000-0000-4000-0000-000000000000", "dev").
667-
Return(errors.New("error")).
825+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
826+
Return(existingAPIKey, nil).
827+
Once()
828+
storeMock.
829+
On("APIKeyDelete", ctx, existingAPIKey).
830+
Return(errors.New("delete error")).
668831
Once()
669832
},
670-
expected: NewErrAPIKeyNotFound("dev", errors.New("error")),
833+
expected: errors.New("delete error"),
671834
},
672835
{
673836
description: "succeeds",
@@ -676,8 +839,23 @@ func TestDeleteAPIKey(t *testing.T) {
676839
Name: "dev",
677840
},
678841
requiredMocks: func(ctx context.Context) {
842+
existingAPIKey := &models.APIKey{
843+
ID: "existing-id",
844+
Name: "dev",
845+
TenantID: "00000000-0000-4000-0000-000000000000",
846+
Role: "operator",
847+
}
848+
849+
queryOptionsMock.
850+
On("InNamespace", "00000000-0000-4000-0000-000000000000").
851+
Return(nil).
852+
Once()
853+
storeMock.
854+
On("APIKeyResolve", ctx, store.APIKeyNameResolver, "dev", mock.AnythingOfType("store.QueryOption")).
855+
Return(existingAPIKey, nil).
856+
Once()
679857
storeMock.
680-
On("APIKeyDelete", ctx, "00000000-0000-4000-0000-000000000000", "dev").
858+
On("APIKeyDelete", ctx, existingAPIKey).
681859
Return(nil).
682860
Once()
683861
},
@@ -695,7 +873,7 @@ func TestDeleteAPIKey(t *testing.T) {
695873
ctx := context.Background()
696874
tc.requiredMocks(ctx)
697875

698-
err = s.DeleteAPIKey(ctx, tc.req)
876+
err := s.DeleteAPIKey(ctx, tc.req)
699877
require.Equal(t, tc.expected, err)
700878
})
701879
}

‎api/store/api_key.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,9 @@ type APIKeyStore interface {
3232
// Returns the list of API keys, the total count of matched documents, and an error if any.
3333
APIKeyList(ctx context.Context, opts ...QueryOption) (apiKeys []models.APIKey, count int, err error)
3434

35-
// APIKeyUpdate updates an API key with the specified name and tenant ID using the given changes.
36-
// Any zero values in the changes (e.g., empty strings) will be ignored during the update.
37-
// Returns an error if any.
38-
APIKeyUpdate(ctx context.Context, tenantID, name string, changes *models.APIKeyChanges) (err error)
35+
// APIKeySave updates an API key. It returns an error if any.
36+
APIKeySave(ctx context.Context, apiKey *models.APIKey) (err error)
3937

40-
// APIKeyDelete deletes an API key with the specified name and tenant ID. Returns an error if any.
41-
APIKeyDelete(ctx context.Context, tenantID, name string) (err error)
38+
// APIKeyDelete deletes an API key. It returns an error if any.
39+
APIKeyDelete(ctx context.Context, apiKey *models.APIKey) (err error)
4240
}

0 commit comments

Comments
 (0)