diff --git a/cmd/monaco/generate/deletefile/deletefile.go b/cmd/monaco/generate/deletefile/deletefile.go index 081970175..1c0f6bd78 100644 --- a/cmd/monaco/generate/deletefile/deletefile.go +++ b/cmd/monaco/generate/deletefile/deletefile.go @@ -227,6 +227,14 @@ func createDeleteEntry(c config.Config, apis api.APIs, project project.Project) return createConfigAPIEntry(c, apis, project) } + // An externalID can't be used if there is a customID set on a DocumentType + if docType, isDocument := c.Type.(config.DocumentType); isDocument && docType.CustomID != "" { + return persistence.DeleteEntry{ + Type: c.Coordinate.Type, + ObjectId: docType.CustomID, + }, nil + } + return persistence.DeleteEntry{ Project: c.Coordinate.Project, Type: c.Coordinate.Type, diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index c57de016e..2b4d72d3c 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -38,7 +38,6 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/automation" "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/buckets" - "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/documents" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/idutils" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log" "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils/matcher" @@ -954,10 +953,7 @@ func TestDelete_Documents(t *testing.T) { externalID := idutils.GenerateExternalID(given.AsCoordinate()) c := client.NewMockDocumentClient(gomock.NewController(t)) - c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))). - Times(1). - Return(documents.ListResponse{Responses: []documents.Response{{Metadata: documents.Metadata{ID: "originObjectID"}}}}, nil) - c.EXPECT().Delete(gomock.Any(), gomock.Eq("originObjectID")).Times(1) + c.EXPECT().Delete(gomock.Any(), gomock.Eq(externalID)).Times(1) entriesToDelete := delete.DeleteEntries{given.Type: {given}} err := delete.Configs(t.Context(), client.ClientSet{DocumentClient: c}, entriesToDelete) @@ -973,33 +969,13 @@ func TestDelete_Documents(t *testing.T) { externalID := idutils.GenerateExternalID(given.AsCoordinate()) c := client.NewMockDocumentClient(gomock.NewController(t)) - c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))). - Times(1). - Return(documents.ListResponse{Responses: []documents.Response{}}, nil) + c.EXPECT().Delete(gomock.Any(), gomock.Eq(externalID)).Times(1).Return(libAPI.Response{}, coreapi.APIError{StatusCode: http.StatusNotFound}) entriesToDelete := delete.DeleteEntries{given.Type: {given}} err := delete.Configs(t.Context(), client.ClientSet{DocumentClient: c}, entriesToDelete) assert.NoError(t, err) }) - t.Run("config declared via coordinate have multiple match - no delete, no error", func(t *testing.T) { - given := pointer.DeletePointer{ - Type: "document", - Identifier: "monaco_identifier", - Project: "project", - } - - externalID := idutils.GenerateExternalID(given.AsCoordinate()) - c := client.NewMockDocumentClient(gomock.NewController(t)) - c.EXPECT().List(gomock.Any(), gomock.Eq(fmt.Sprintf("externalId=='%s'", externalID))). - Times(1). - Return(documents.ListResponse{Responses: []documents.Response{{Metadata: documents.Metadata{ID: "originObjectID_1"}}, {Metadata: documents.Metadata{ID: "originObjectID_2"}}}}, nil) - - entriesToDelete := delete.DeleteEntries{given.Type: {given}} - err := delete.Configs(t.Context(), client.ClientSet{DocumentClient: c}, entriesToDelete) - assert.Error(t, err) - }) - t.Run("delete via originID", func(t *testing.T) { c := client.NewMockDocumentClient(gomock.NewController(t)) c.EXPECT().Delete(gomock.Any(), gomock.Eq("originObjectID")).Times(1) diff --git a/pkg/resource/document/delete.go b/pkg/resource/document/delete.go index 6f62c4e4e..ed9316a1d 100644 --- a/pkg/resource/document/delete.go +++ b/pkg/resource/document/delete.go @@ -63,22 +63,8 @@ func (d Deleter) deleteSingle(ctx context.Context, dp pointer.DeletePointer) err if id == "" { coordinate := dp.AsCoordinate() - logger = slog.With(log.CoordinateAttr(coordinate)) - extID := idutils.GenerateExternalID(coordinate) - - var err error - id, err = d.tryGetDocumentIDByExternalID(ctx, extID) - if err != nil { - logger.ErrorContext(ctx, "Failed to get document by external ID", slog.String("externalId", extID), log.ErrorAttr(err)) - return err - } - - if id == "" { - logger.DebugContext(ctx, "No document found with external ID", slog.String("externalId", extID)) - return nil - } - - logger = logger.With(slog.String("id", id)) + id = idutils.GenerateExternalID(coordinate) + logger = slog.With(log.CoordinateAttr(coordinate), slog.String("id", id)) } _, err := d.source.Delete(ctx, id) @@ -91,23 +77,6 @@ func (d Deleter) deleteSingle(ctx context.Context, dp pointer.DeletePointer) err return nil } -func (d Deleter) tryGetDocumentIDByExternalID(ctx context.Context, externalId string) (string, error) { - switch listResponse, err := d.source.List(ctx, fmt.Sprintf("externalId=='%s'", externalId)); { - case err != nil: - return "", err - case len(listResponse.Responses) == 0: - return "", nil - case len(listResponse.Responses) > 1: - var ids []string - for _, r := range listResponse.Responses { - ids = append(ids, r.ID) - } - return "", fmt.Errorf("found more than one document with same externalId (%s); matching document IDs: %s", externalId, ids) - default: - return listResponse.Responses[0].ID, nil - } -} - func (d Deleter) DeleteAll(ctx context.Context) error { listResponse, err := d.source.List(ctx, getFilterForAllSupportedDocumentTypes()) if err != nil { diff --git a/pkg/resource/document/delete_test.go b/pkg/resource/document/delete_test.go index eb849d566..dc4c7a1a0 100644 --- a/pkg/resource/document/delete_test.go +++ b/pkg/resource/document/delete_test.go @@ -21,7 +21,6 @@ package document_test import ( "context" "errors" - "fmt" "net/http" "testing" @@ -49,10 +48,6 @@ func (s *deleteStubClient) Delete(_ context.Context, id string) (libAPI.Response return s.delete(id) } -func externalIDToFilter(externalId string) string { - return fmt.Sprintf("externalId=='%s'", externalId) -} - func TestDeleteByCoordinate(t *testing.T) { t.Run("success if one document matches generated external ID", func(t *testing.T) { given := pointer.DeletePointer{ @@ -63,20 +58,8 @@ func TestDeleteByCoordinate(t *testing.T) { externalID := idutils.GenerateExternalID(given.AsCoordinate()) c := deleteStubClient{ - list: func(filter string) (documents.ListResponse, error) { - assert.Equal(t, externalIDToFilter(externalID), filter) - return documents.ListResponse{ - libAPI.Response{}, - []documents.Response{ - { - libAPI.Response{}, - documents.Metadata{ID: "uid_1"}, - }, - }, - }, nil - }, delete: func(id string) (libAPI.Response, error) { - assert.Equal(t, "uid_1", id) + assert.Equal(t, externalID, id) return libAPI.Response{}, nil }, } @@ -95,71 +78,15 @@ func TestDeleteByCoordinate(t *testing.T) { externalID := idutils.GenerateExternalID(given.AsCoordinate()) c := deleteStubClient{ - list: func(filter string) (documents.ListResponse, error) { - assert.Equal(t, externalIDToFilter(externalID), filter) - return documents.ListResponse{ - libAPI.Response{}, - []documents.Response{}, - }, nil + delete: func(id string) (libAPI.Response, error) { + assert.Equal(t, externalID, id) + return libAPI.Response{}, libAPI.APIError{StatusCode: http.StatusNotFound} }, } err := document.NewDeleter(&c).Delete(t.Context(), []pointer.DeletePointer{given}) assert.NoError(t, err) - assert.False(t, c.deleteCalled, "delete command was invoked") - }) - - t.Run("error if multiple documents are matching generated external ID", func(t *testing.T) { - given := pointer.DeletePointer{ - Type: "document", - Identifier: "monaco_identifier", - Project: "project", - } - - externalID := idutils.GenerateExternalID(given.AsCoordinate()) - c := deleteStubClient{ - list: func(filter string) (documents.ListResponse, error) { - assert.Equal(t, externalIDToFilter(externalID), filter) - return documents.ListResponse{ - libAPI.Response{}, - []documents.Response{ - { - libAPI.Response{}, - documents.Metadata{ID: "uid_1"}, - }, - { - libAPI.Response{}, - documents.Metadata{ID: "uid_2"}, - }, - }, - }, nil - }, - } - - err := document.NewDeleter(&c).Delete(t.Context(), []pointer.DeletePointer{given}) - assert.Error(t, err) - assert.False(t, c.deleteCalled, "it's not known what needs to be deleted") - }) - - t.Run("error if list fails", func(t *testing.T) { - given := pointer.DeletePointer{ - Type: "document", - Identifier: "monaco_identifier", - Project: "project", - } - - c := deleteStubClient{ - list: func(filter string) (documents.ListResponse, error) { - return documents.ListResponse{ - libAPI.Response{}, - []documents.Response{}, - }, errors.New("some unpredictable error") - }, - } - - err := document.NewDeleter(&c).Delete(t.Context(), []pointer.DeletePointer{given}) - assert.Error(t, err) - assert.False(t, c.deleteCalled, "delete command was invoked") + assert.True(t, c.deleteCalled, "delete command was invoked") }) } diff --git a/test/commands/deletefile_test.go b/test/commands/deletefile_test.go index dc5695f65..6f1097b9d 100644 --- a/test/commands/deletefile_test.go +++ b/test/commands/deletefile_test.go @@ -101,7 +101,7 @@ func TestGeneratesValidDeleteFile(t *testing.T) { assertDeleteEntries(t, entries, "scheduling-rule", "ca-scheduling-rule") assertDeleteEntries(t, entries, "workflow", "ca-jira-issue-workflow") assertDeleteEntries(t, entries, "bucket", "my-bucket") - assertDeleteEntries(t, entries, "document", "my-dashboard", "my-notebook") + assertDeleteEntries(t, entries, "document", "my-dashboard", "my-notebook", "" /*non-coordinate references (direct ones) don't have an identifier */) assertDeleteEntries(t, entries, "segment", "segmentID") assert.Empty(t, entries[api.DashboardShareSettings]) @@ -134,6 +134,34 @@ func TestGeneratesValidDeleteFileWithCustomValues(t *testing.T) { }) } +func TestGeneratesValidDeleteFileWithCorrectDocumentPointers(t *testing.T) { + t.Setenv("TOKEN", "some-value") + + fs := testutils.CreateTestFileSystem() + outputFolder := "output-folder" + err := monaco.Run(t, fs, fmt.Sprintf("monaco generate deletefile ./testdata/deletefile/manifest.yaml --types=document --output-folder=%s", outputFolder)) + require.NoError(t, err) + + expectedFile := filepath.Join(outputFolder, "delete.yaml") + assertFileExists(t, fs, expectedFile) + + deleteFileContent := readFile(t, fs, expectedFile) + + var deleteEntries persistence.FullFileDefinition + err = yaml.Unmarshal(deleteFileContent, &deleteEntries) + require.NoError(t, err) + + assert.Contains(t, deleteEntries.DeleteEntries, persistence.DeleteEntry{ + Type: string(config.DocumentTypeID), + ObjectId: "custom-id", + }) + assert.Contains(t, deleteEntries.DeleteEntries, persistence.DeleteEntry{ + Type: string(config.DocumentTypeID), + Project: "project", + ConfigId: "my-dashboard", + }) +} + func TestGeneratesValidDeleteFileWithFilter(t *testing.T) { t.Setenv("TOKEN", "some-value") diff --git a/test/commands/testdata/deletefile/project/document-dashboard/config.yaml b/test/commands/testdata/deletefile/project/document-dashboard/config.yaml index c451bf3da..fb88f8993 100644 --- a/test/commands/testdata/deletefile/project/document-dashboard/config.yaml +++ b/test/commands/testdata/deletefile/project/document-dashboard/config.yaml @@ -7,3 +7,12 @@ configs: type: document: kind: dashboard +- id: my-dashboard-with-custom-id + config: + name: My dashboard with a custom ID + template: dashboard.json + skip: false + type: + document: + kind: dashboard + id: custom-id