diff --git a/core/resource/mocks/project_service.go b/core/resource/mocks/project_service.go index 9fd776116..d74d9d424 100644 --- a/core/resource/mocks/project_service.go +++ b/core/resource/mocks/project_service.go @@ -79,6 +79,65 @@ func (_c *ProjectService_Get_Call) RunAndReturn(run func(context.Context, string return _c } +// List provides a mock function with given fields: ctx, f +func (_m *ProjectService) List(ctx context.Context, f project.Filter) ([]project.Project, error) { + ret := _m.Called(ctx, f) + + if len(ret) == 0 { + panic("no return value specified for List") + } + + var r0 []project.Project + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, project.Filter) ([]project.Project, error)); ok { + return rf(ctx, f) + } + if rf, ok := ret.Get(0).(func(context.Context, project.Filter) []project.Project); ok { + r0 = rf(ctx, f) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]project.Project) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, project.Filter) error); ok { + r1 = rf(ctx, f) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ProjectService_List_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'List' +type ProjectService_List_Call struct { + *mock.Call +} + +// List is a helper method to define mock.On call +// - ctx context.Context +// - f project.Filter +func (_e *ProjectService_Expecter) List(ctx interface{}, f interface{}) *ProjectService_List_Call { + return &ProjectService_List_Call{Call: _e.mock.On("List", ctx, f)} +} + +func (_c *ProjectService_List_Call) Run(run func(ctx context.Context, f project.Filter)) *ProjectService_List_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(project.Filter)) + }) + return _c +} + +func (_c *ProjectService_List_Call) Return(_a0 []project.Project, _a1 error) *ProjectService_List_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *ProjectService_List_Call) RunAndReturn(run func(context.Context, project.Filter) ([]project.Project, error)) *ProjectService_List_Call { + _c.Call.Return(run) + return _c +} + // NewProjectService creates a new instance of ProjectService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewProjectService(t interface { diff --git a/core/resource/service.go b/core/resource/service.go index 99cf4fabc..7f20326a6 100644 --- a/core/resource/service.go +++ b/core/resource/service.go @@ -34,6 +34,7 @@ type AuthnService interface { type ProjectService interface { Get(ctx context.Context, idOrName string) (project.Project, error) + List(ctx context.Context, f project.Filter) ([]project.Project, error) } type OrgService interface { @@ -181,6 +182,24 @@ func (s Service) getParent(ctx context.Context, res Resource) (parent, error) { if err != nil { return parent{}, fmt.Errorf("failed to get project: %w", err) } + return projectParent(proj), nil +} + +func (s Service) getParentToDelete(ctx context.Context, res Resource) (parent, error) { + if res.ProjectID == "" { + return s.getParent(ctx, res) + } + projects, err := s.projectService.List(ctx, project.Filter{ProjectIDs: []string{res.ProjectID}, IncludeDisabled: true}) + if err != nil { + return parent{}, fmt.Errorf("failed to get project: %w", err) + } + if len(projects) == 0 { + return parent{}, fmt.Errorf("failed to get project: %w", project.ErrNotExist) + } + return projectParent(projects[0]), nil +} + +func projectParent(proj project.Project) parent { return parent{ projectID: proj.ID, org: proj.Organization, @@ -188,7 +207,7 @@ func (s Service) getParent(ctx context.Context, res Resource) (parent, error) { link: relation.Subject{ID: proj.ID, Namespace: schema.ProjectNamespace}, linkName: schema.ProjectRelationName, audit: auditmodels.Resource{ID: proj.ID, Type: pkgauditrecord.ProjectType, Name: proj.Title}, - }, nil + } } func (s Service) createAuditRecord(ctx context.Context, event pkgauditrecord.Event, res Resource, p parent) { @@ -481,7 +500,7 @@ func (s Service) Delete(ctx context.Context, namespaceID, id string) error { if err != nil { return err } - p, err := s.getParent(ctx, res) + p, err := s.getParentToDelete(ctx, res) if err != nil { return err } diff --git a/core/resource/service_test.go b/core/resource/service_test.go index 49b0a1411..837acdadd 100644 --- a/core/resource/service_test.go +++ b/core/resource/service_test.go @@ -665,12 +665,13 @@ func TestDelete(t *testing.T) { Organization: organization.Organization{ID: uuid.New().String(), Title: "Org One"}, } res := resource.Resource{ID: "r1", Name: "res-1", Title: "Resource One", NamespaceID: "resource/item", ProjectID: proj.ID} + lookup := project.Filter{ProjectIDs: []string{proj.ID}, IncludeDisabled: true} t.Run("deletes relations then the row, and writes the audit record", func(t *testing.T) { repo, relationSvc, _, projectSvc, _, _, auditRepo, _, svc := newTestService(t) repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) - projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{proj}, nil) relationSvc.EXPECT().Delete(mock.Anything, relation.Relation{ Object: relation.Object{ID: "r1", Namespace: "resource/item"}, }).Return(nil) @@ -719,7 +720,7 @@ func TestDelete(t *testing.T) { repo, relationSvc, _, projectSvc, _, _, auditRepo, _, svc := newTestService(t) repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) - projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{proj}, nil) relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(relation.ErrNotExist) repo.EXPECT().Delete(mock.Anything, "r1").Return(nil) auditRepo.EXPECT().Create(mock.Anything, mock.AnythingOfType("models.AuditRecord")). @@ -733,7 +734,7 @@ func TestDelete(t *testing.T) { repo, relationSvc, _, projectSvc, _, _, _, _, svc := newTestService(t) repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) - projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{proj}, nil) relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(errors.New("spicedb down")) err := svc.Delete(ctx, "resource/item", "r1") @@ -744,13 +745,40 @@ func TestDelete(t *testing.T) { repo, relationSvc, _, projectSvc, _, _, _, _, svc := newTestService(t) repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) - projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{proj}, nil) relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil) repo.EXPECT().Delete(mock.Anything, "r1").Return(errors.New("db down")) err := svc.Delete(ctx, "resource/item", "r1") assert.ErrorContains(t, err, "db down") }) + + t.Run("deletes a resource of a disabled project", func(t *testing.T) { + repo, relationSvc, _, projectSvc, _, _, auditRepo, _, svc := newTestService(t) + disabled := proj + disabled.State = project.Disabled + + repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{disabled}, nil) + relationSvc.EXPECT().Delete(mock.Anything, mock.Anything).Return(nil) + repo.EXPECT().Delete(mock.Anything, "r1").Return(nil) + auditRepo.EXPECT().Create(mock.Anything, mock.MatchedBy(func(rec auditmodels.AuditRecord) bool { + return rec.Resource.ID == proj.ID + })).Return(auditmodels.AuditRecord{}, nil) + + err := svc.Delete(ctx, "resource/item", "r1") + assert.NoError(t, err) + }) + + t.Run("deletes nothing when the project is gone", func(t *testing.T) { + repo, _, _, projectSvc, _, _, _, _, svc := newTestService(t) + + repo.EXPECT().GetByID(mock.Anything, "r1").Return(res, nil) + projectSvc.EXPECT().List(mock.Anything, lookup).Return([]project.Project{}, nil) + + err := svc.Delete(ctx, "resource/item", "r1") + assert.ErrorIs(t, err, project.ErrNotExist) + }) } func TestPurge(t *testing.T) { diff --git a/internal/store/postgres/project_repository_test.go b/internal/store/postgres/project_repository_test.go index 575ab2176..cd8373bbc 100644 --- a/internal/store/postgres/project_repository_test.go +++ b/internal/store/postgres/project_repository_test.go @@ -541,4 +541,15 @@ func (s *ProjectRepositoryTestSuite) TestListIncludeDisabled() { ids = append(ids, p.ID) } s.Assert().Contains(ids, disabled.ID) + + byID, err := s.repository.List(s.ctx, project.Filter{ProjectIDs: []string{disabled.ID}, IncludeDisabled: true}) + s.Require().NoError(err) + s.Require().Len(byID, 1) + s.Assert().Equal(disabled.ID, byID[0].ID) + + _, err = s.client.ExecContext(s.ctx, "UPDATE projects SET deleted_at = now() WHERE id = $1", disabled.ID) + s.Require().NoError(err) + deleted, err := s.repository.List(s.ctx, project.Filter{ProjectIDs: []string{disabled.ID}, IncludeDisabled: true}) + s.Require().NoError(err) + s.Assert().Empty(deleted) }