feat(deleter): project delete soft-deletes its resources

core/deleter/service.go

@@ -72,7 +72,6 @@ type PolicyService interface {
 type ResourceService interface {
 	List(ctx context.Context, flt resource.Filter) ([]resource.Resource, error)
 	Delete(ctx context.Context, namespaceID, id string) error
-	Purge(ctx context.Context, namespaceID, id string) error
 }
 
 type GroupService interface {
@@ -254,16 +253,15 @@ func (d Service) deleteProject(ctx context.Context, prj project.Project) error {
 		}
 	}
 
-	// TODO(fix): soft-delete the live resources instead of purging every row
-	resources, err := d.resService.List(ctx, resource.Filter{
-		ProjectID:      prj.ID,
-		IncludeDeleted: true,
-	})
+	resources, err := d.resService.List(ctx, resource.Filter{ProjectID: prj.ID})
 	if err != nil {
 		return err
 	}
 	for _, r := range resources {
-		if err = d.resService.Purge(ctx, r.NamespaceID, r.ID); err != nil {
+		if err = d.resService.Delete(ctx, r.NamespaceID, r.ID); err != nil {
+			if errors.Is(err, resource.ErrNotExist) {
+				continue
+			}
 			return fmt.Errorf("failed to delete project while deleting a resource[%s]: %w", r.Name, err)
 		}
 	}

core/deleter/service_test.go

@@ -142,9 +142,9 @@ func TestDeleteProject(t *testing.T) {
 		m.polSvc.EXPECT().Delete(mock.Anything, "pol-2").
 			Run(func(_ context.Context, id string) { steps = append(steps, "policy:"+id) }).Return(nil)
 
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
 			Return([]resource.Resource{{ID: "res-1", NamespaceID: "ns-1", Name: "r1"}}, nil)
-		m.resSvc.EXPECT().Purge(mock.Anything, "ns-1", "res-1").
+		m.resSvc.EXPECT().Delete(mock.Anything, "ns-1", "res-1").
 			Run(func(_ context.Context, _ string, id string) { steps = append(steps, "resource:"+id) }).Return(nil)
 
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).
@@ -161,14 +161,29 @@ func TestDeleteProject(t *testing.T) {
 
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(project.ErrNotExist)
 
 		err := m.build().DeleteProject(context.Background(), projectID)
 		assert.NoError(t, err)
 	})
 
+	t.Run("a resource that is already gone does not stop the delete", func(t *testing.T) {
+		m := newMocks(t)
+		expectProject(m)
+
+		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
+			Return([]policy.Policy{}, nil)
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
+			Return([]resource.Resource{{ID: "res-1", NamespaceID: "ns-1", Name: "r1"}}, nil)
+		m.resSvc.EXPECT().Delete(mock.Anything, "ns-1", "res-1").Return(resource.ErrNotExist)
+		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(nil)
+
+		err := m.build().DeleteProject(context.Background(), projectID)
+		assert.NoError(t, err)
+	})
+
 	t.Run("returns error when policy list fails", func(t *testing.T) {
 		m := newMocks(t)
 		expectProject(m)
@@ -199,7 +214,7 @@ func TestDeleteProject(t *testing.T) {
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{{ID: "pol-gone"}}, nil)
 		m.polSvc.EXPECT().Delete(mock.Anything, "pol-gone").Return(policy.ErrNotExist)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(nil)
 
@@ -213,7 +228,7 @@ func TestDeleteProject(t *testing.T) {
 
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: projectID}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID, IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: projectID}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, projectID).Return(nil)
 
@@ -319,7 +334,7 @@ func TestDeleteOrganization(t *testing.T) {
 			Return([]project.Project{{ID: "proj-1", Name: "p1"}}, nil)
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").Return(nil)
 
@@ -403,7 +418,7 @@ func TestDeleteOrganization(t *testing.T) {
 		for _, id := range []string{"proj-on", "proj-off"} {
 			m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: id}).
 				Return([]policy.Policy{}, nil)
-			m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: id, IncludeDeleted: true}).
+			m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: id}).
 				Return([]resource.Resource{}, nil)
 			m.projSvc.EXPECT().DeleteModel(mock.Anything, id).
 				Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil)
@@ -441,9 +456,9 @@ func TestDeleteOrganization(t *testing.T) {
 			Return([]project.Project{{ID: "proj-1", Name: "p1"}}, nil)
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-1"}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-1"}).
 			Return([]resource.Resource{{ID: "m1", NamespaceID: "compute/machine", Name: "machine-1"}}, nil)
-		m.resSvc.EXPECT().Purge(mock.Anything, "compute/machine", "m1").Return(nil)
+		m.resSvc.EXPECT().Delete(mock.Anything, "compute/machine", "m1").Return(nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-1").
 			Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil)
 		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{OrgID: "org-1"}).
@@ -470,7 +485,7 @@ func TestDeleteOrganization(t *testing.T) {
 			Return([]project.Project{{ID: "proj-on", Name: "on"}}, nil)
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-on"}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-on", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-on"}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-on").
 			Run(func(_ context.Context, id string) { steps = append(steps, "project:"+id) }).Return(nil)
@@ -500,7 +515,7 @@ func TestDeleteOrganization(t *testing.T) {
 			Return([]project.Project{{ID: "proj-off", Name: "off", State: project.Disabled}}, nil)
 		m.polSvc.EXPECT().List(mock.Anything, policy.Filter{ProjectID: "proj-off"}).
 			Return([]policy.Policy{}, nil)
-		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-off", IncludeDeleted: true}).
+		m.resSvc.EXPECT().List(mock.Anything, resource.Filter{ProjectID: "proj-off"}).
 			Return([]resource.Resource{}, nil)
 		m.projSvc.EXPECT().DeleteModel(mock.Anything, "proj-off").Return(errors.New("boom"))
 		m.roleSvc.EXPECT().Get(mock.Anything, schema.RoleOrganizationOwner).

core/resource/filter.go

@@ -6,7 +6,4 @@ type Filter struct {
 	ServiceUserID string
 	NamespaceID   string
 	OrgID         string
-	// IncludeDeleted also returns soft-deleted rows.
-	// TODO(fix): remove once the project delete cascade no longer purges
-	IncludeDeleted bool
 }

core/resource/resource.go

@@ -17,7 +17,6 @@ type Repository interface {
 	List(ctx context.Context, flt Filter) ([]Resource, error)
 	Update(ctx context.Context, resource Resource) (Resource, error)
 	Delete(ctx context.Context, id string) error
-	Purge(ctx context.Context, id string) error
 }
 
 type Resource struct {

core/resource/service.go

@@ -521,21 +521,6 @@ func (s Service) Delete(ctx context.Context, namespaceID, id string) error {
 	return nil
 }
 
-// Purge removes the resource and its SpiceDB tuples for good. Only the project
-// delete cascade uses it; the API uses Delete, which keeps the row.
-// TODO(fix): remove once the project delete cascade soft-deletes resources
-func (s Service) Purge(ctx context.Context, namespaceID, id string) error {
-	if err := s.relationService.Delete(ctx, relation.Relation{
-		Object: relation.Object{
-			ID:        id,
-			Namespace: namespaceID,
-		},
-	}); err != nil && !errors.Is(err, relation.ErrNotExist) {
-		return err
-	}
-	return s.repository.Purge(ctx, id)
-}
-
 func (s Service) RemovePrincipalAccess(ctx context.Context, principalID, principalType, orgID string, projectIDs []string) error {
 	var scopes []Filter
 	if orgID != "" {

core/resource/service_test.go

@@ -781,39 +781,6 @@ func TestDelete(t *testing.T) {
 	})
 }
 
-func TestPurge(t *testing.T) {
-	t.Run("removes relations then the row", func(t *testing.T) {
-		repo, relationSvc, _, _, _, _, _, _, svc := newTestService(t)
-
-		relationSvc.EXPECT().Delete(mock.Anything, relation.Relation{
-			Object: relation.Object{ID: "r1", Namespace: "resource/item"},
-		}).Return(nil)
-		repo.EXPECT().Purge(mock.Anything, "r1").Return(nil)
-
-		err := svc.Purge(context.Background(), "resource/item", "r1")
-		assert.NoError(t, err)
-	})
-
-	t.Run("ignores relation not exist error", func(t *testing.T) {
-		repo, relationSvc, _, _, _, _, _, _, svc := newTestService(t)
-
-		relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(relation.ErrNotExist)
-		repo.EXPECT().Purge(mock.Anything, "r1").Return(nil)
-
-		err := svc.Purge(context.Background(), "resource/item", "r1")
-		assert.NoError(t, err)
-	})
-
-	t.Run("returns relation delete error", func(t *testing.T) {
-		_, relationSvc, _, _, _, _, _, _, svc := newTestService(t)
-
-		relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(errors.New("spicedb down"))
-
-		err := svc.Purge(context.Background(), "resource/item", "r1")
-		assert.ErrorContains(t, err, "spicedb down")
-	})
-}
-
 func TestAddResourceOwner(t *testing.T) {
 	t.Run("creates owner relation with user principal", func(t *testing.T) {
 		_, relationSvc, _, _, _, _, _, _, svc := newTestService(t)

internal/store/postgres/resource_repository.go

@@ -113,11 +113,7 @@ func (r ResourceRepository) Create(ctx context.Context, res resource.Resource) (
 func (r ResourceRepository) List(ctx context.Context, flt resource.Filter) ([]resource.Resource, error) {
 	var fetchedResources []Resource
 
-	sqlStatement := fromLive(TABLE_RESOURCES)
-	if flt.IncludeDeleted {
-		sqlStatement = dialect.From(TABLE_RESOURCES)
-	}
-	sqlStatement = sqlStatement.Order(goqu.C("created_at").Asc())
+	sqlStatement := fromLive(TABLE_RESOURCES).Order(goqu.C("created_at").Asc())
 	if flt.ProjectID != "" {
 		sqlStatement = sqlStatement.Where(goqu.Ex{"project_id": flt.ProjectID})
 	}
@@ -285,24 +281,3 @@ func (r ResourceRepository) Delete(ctx context.Context, id string) error {
 	}
 	return nil
 }
-
-// Purge removes the row for good. Only the project delete cascade uses it.
-// TODO(fix): remove once the project delete cascade soft-deletes resources
-func (r ResourceRepository) Purge(ctx context.Context, id string) error {
-	query, params, err := dialect.Delete(TABLE_RESOURCES).Where(
-		goqu.Ex{
-			"id": id,
-		},
-	).ToSQL()
-	if err != nil {
-		return fmt.Errorf("%w: %s", errQuery, err)
-	}
-
-	if err = r.dbc.WithTimeout(ctx, TABLE_RESOURCES, "Purge", func(ctx context.Context) error {
-		_, err := r.dbc.DB.ExecContext(ctx, query, params...)
-		return err
-	}); err != nil {
-		return checkPostgresError(err)
-	}
-	return nil
-}

internal/store/postgres/resource_repository_test.go

@@ -636,21 +636,6 @@ func (s *ResourceRepositoryTestSuite) TestDelete() {
 	s.Assert().ErrorIs(err, resource.ErrNotExist)
 }
 
-func (s *ResourceRepositoryTestSuite) TestPurge() {
-	target := s.resources[0]
-
-	err := s.repository.Purge(s.ctx, target.ID)
-	s.Require().NoError(err)
-
-	var rows int
-	err = s.client.QueryRowxContext(s.ctx, "SELECT count(*) FROM resources WHERE id = $1", target.ID).Scan(&rows)
-	s.Require().NoError(err)
-	s.Assert().Equal(0, rows)
-
-	err = s.repository.Purge(s.ctx, utils.NewString())
-	s.Assert().NoError(err)
-}
-
 func TestResourceRepository(t *testing.T) {
 	suite.Run(t, new(ResourceRepositoryTestSuite))
 }
@@ -678,10 +663,6 @@ func (s *ResourceRepositoryTestSuite) TestSkipsSoftDeletedResources() {
 		s.Assert().NotEqual(deleted.ID, r.ID)
 	}
 
-	all, err := s.repository.List(s.ctx, resource.Filter{IncludeDeleted: true})
-	s.Assert().NoError(err)
-	s.Assert().Len(all, len(before))
-
 	updated := deleted
 	updated.Title = "changed"
 	_, err = s.repository.Update(s.ctx, updated)