feat(document): remove external ID usage and support deletefile for custom ID

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,

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)

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 {

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")
 	})
 }
 

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")
 

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