diff --git a/internal/store/postgres/policy_repository.go b/internal/store/postgres/policy_repository.go index 0003146e1..55a558db0 100644 --- a/internal/store/postgres/policy_repository.go +++ b/internal/store/postgres/policy_repository.go @@ -39,7 +39,7 @@ func (r PolicyRepository) buildListQuery() *goqu.SelectDataset { "p.principal_type", "p.role_id", "p.grant_relation", - ).From(goqu.T(TABLE_POLICIES).As("p")) + ).From(goqu.T(TABLE_POLICIES).As("p")).Where(live("p")) } func (r PolicyRepository) Get(ctx context.Context, id string) (policy.Policy, error) { @@ -181,7 +181,7 @@ func (r PolicyRepository) List(ctx context.Context, flt policy.Filter) ([]policy func (r PolicyRepository) Count(ctx context.Context, flt policy.Filter) (int64, error) { var count int64 - stmt := dialect.Select(goqu.COUNT(goqu.Star()).As("count")).From(goqu.T(TABLE_POLICIES).As("p")) + stmt := dialect.Select(goqu.COUNT(goqu.Star()).As("count")).From(goqu.T(TABLE_POLICIES).As("p")).Where(live("p")) stmt = applyListFilter(stmt, flt) query, params, err := stmt.ToSQL() @@ -292,7 +292,7 @@ func (r PolicyRepository) Update(ctx context.Context, toUpdate policy.Policy) (s "updated_at": goqu.L("now()"), }).Where(goqu.Ex{ "id": toUpdate.ID, - }).Returning("id", "updated_at").ToSQL() + }, live(TABLE_POLICIES)).Returning("id", "updated_at").ToSQL() if err != nil { return "", fmt.Errorf("%w: %s", errQuery, err) } @@ -461,7 +461,7 @@ func (r PolicyRepository) GroupMemberCount(ctx context.Context, groupIDs []strin if len(groupIDs) == 0 { return nil, policy.ErrInvalidID } - stmt := dialect.From("policies"). + stmt := fromLive(TABLE_POLICIES). Select(goqu.I("resource_id").As("id"), goqu.COUNT(goqu.DISTINCT(goqu.I("principal_id"))).As("count")). Where(goqu.Ex{ "resource_type": schema.GroupNamespace, @@ -498,7 +498,7 @@ func (r PolicyRepository) ProjectMemberCount(ctx context.Context, projectIDs []s if len(projectIDs) == 0 { return nil, policy.ErrInvalidID } - stmt := dialect.From("policies"). + stmt := fromLive(TABLE_POLICIES). Select(goqu.I("resource_id").As("id"), goqu.COUNT(goqu.DISTINCT(goqu.I("principal_id"))).As("count")). Where(goqu.Ex{ "resource_type": schema.ProjectNamespace, @@ -534,7 +534,7 @@ func (r PolicyRepository) OrgMemberCount(ctx context.Context, id string) (policy if len(id) == 0 { return policy.MemberCount{}, policy.ErrInvalidID } - stmt := dialect.From("policies"). + stmt := fromLive(TABLE_POLICIES). Select(goqu.I("resource_id").As("id"), goqu.COUNT(goqu.DISTINCT(goqu.I("principal_id"))).As("count")). Where(goqu.Ex{ "resource_type": schema.OrganizationNamespace, @@ -600,7 +600,7 @@ func (r PolicyRepository) buildPolicyAuditRecord(ctx context.Context, tx *sqlx.T // getPolicyByConstraint fetches a policy by unique constraint fields // Returns the policy and true if found, empty policy and false if not found func (r PolicyRepository) getPolicyByConstraint(ctx context.Context, pol policy.Policy) (Policy, bool) { - query, params, _ := dialect.From(TABLE_POLICIES). + query, params, _ := fromLive(TABLE_POLICIES). Select("id", "resource_type", "resource_id", "principal_id", "principal_type", "role_id"). Where(goqu.Ex{ "role_id": pol.RoleID, @@ -625,19 +625,19 @@ func (r PolicyRepository) getResourceInfo(ctx context.Context, tx *sqlx.Tx, reso switch resourceType { case schema.OrganizationNamespace: orgID = resourceID - orgQuery, orgParams, _ := dialect.From(TABLE_ORGANIZATIONS). + orgQuery, orgParams, _ := fromLive(TABLE_ORGANIZATIONS). Select("title"). Where(goqu.Ex{"id": resourceID}). ToSQL() _ = tx.QueryRowContext(ctx, orgQuery, orgParams...).Scan(&resourceName) case schema.ProjectNamespace: - projQuery, projParams, _ := dialect.From(TABLE_PROJECTS). + projQuery, projParams, _ := fromLive(TABLE_PROJECTS). Select("org_id", "title"). Where(goqu.Ex{"id": resourceID}). ToSQL() _ = tx.QueryRowContext(ctx, projQuery, projParams...).Scan(&orgID, &resourceName) case schema.GroupNamespace: - grpQuery, grpParams, _ := dialect.From(TABLE_GROUPS). + grpQuery, grpParams, _ := fromLive(TABLE_GROUPS). Select("org_id", "title"). Where(goqu.Ex{"id": resourceID}). ToSQL() diff --git a/internal/store/postgres/policy_repository_test.go b/internal/store/postgres/policy_repository_test.go index fb5f002aa..e4e45854e 100644 --- a/internal/store/postgres/policy_repository_test.go +++ b/internal/store/postgres/policy_repository_test.go @@ -535,3 +535,34 @@ func (s *PolicyRepositoryTestSuite) TestOrgMemberCount() { }) } } + +func (s *PolicyRepositoryTestSuite) TestSkipsSoftDeletedPolicies() { + deleted := s.policies[0] + flt := policy.Filter{PrincipalID: s.userID} + before, err := s.repository.List(s.ctx, flt) + s.Require().NoError(err) + countBefore, err := s.repository.Count(s.ctx, flt) + s.Require().NoError(err) + + _, err = s.client.ExecContext(s.ctx, "UPDATE policies SET deleted_at = now() WHERE id = $1", deleted.ID) + if err != nil { + s.T().Fatal(err) + } + + _, err = s.repository.Get(s.ctx, deleted.ID) + s.Assert().ErrorIs(err, policy.ErrNotExist) + + got, err := s.repository.List(s.ctx, flt) + s.Assert().NoError(err) + s.Assert().Len(got, len(before)-1) + for _, p := range got { + s.Assert().NotEqual(deleted.ID, p.ID) + } + + count, err := s.repository.Count(s.ctx, flt) + s.Assert().NoError(err) + s.Assert().Equal(countBefore-1, count) + + _, err = s.repository.Update(s.ctx, deleted) + s.Assert().ErrorIs(err, policy.ErrNotExist) +} diff --git a/internal/store/postgres/relation_repository.go b/internal/store/postgres/relation_repository.go index 1d79cd68e..9d93b9007 100644 --- a/internal/store/postgres/relation_repository.go +++ b/internal/store/postgres/relation_repository.go @@ -59,7 +59,7 @@ func (r RelationRepository) Upsert(ctx context.Context, relationToCreate relatio } func (r RelationRepository) List(ctx context.Context, flt relation.Filter) ([]relation.Relation, error) { - stmt := dialect.Select(&relationCols{}).From(TABLE_RELATIONS) + stmt := fromLive(TABLE_RELATIONS).Select(&relationCols{}) if flt.Subject.ID != "" { stmt = stmt.Where(goqu.Ex{ "subject_id": flt.Subject.ID, @@ -101,7 +101,7 @@ func (r RelationRepository) Get(ctx context.Context, id string) (relation.Relati return relation.Relation{}, relation.ErrInvalidID } - query, params, err := dialect.Select(&relationCols{}).From(TABLE_RELATIONS). + query, params, err := fromLive(TABLE_RELATIONS).Select(&relationCols{}). Where(goqu.Ex{ "id": id, }).ToSQL() @@ -166,7 +166,7 @@ func (r RelationRepository) DeleteByID(ctx context.Context, id string) error { func (r RelationRepository) GetByFields(ctx context.Context, rel relation.Relation) ([]relation.Relation, error) { var fetchedRelations []Relation - stmt := dialect.Select(&relationCols{}).From(TABLE_RELATIONS) + stmt := fromLive(TABLE_RELATIONS).Select(&relationCols{}) if rel.Object.ID != "" { stmt = stmt.Where(goqu.Ex{ "object_id": rel.Object.ID, @@ -230,7 +230,7 @@ func (r RelationRepository) ListByFields(ctx context.Context, rel relation.Relat if len(rel.Object.ID) != 0 { exprs = append(exprs, goqu.Ex{"object_id": rel.Object.ID}) } - query, params, err := dialect.Select(&relationCols{}).From(TABLE_RELATIONS).Where(exprs...).ToSQL() + query, params, err := fromLive(TABLE_RELATIONS).Select(&relationCols{}).Where(exprs...).ToSQL() if err != nil { return nil, fmt.Errorf("%w: %s", errQuery, err) } diff --git a/internal/store/postgres/relation_repository_test.go b/internal/store/postgres/relation_repository_test.go index 632764022..571229fc2 100644 --- a/internal/store/postgres/relation_repository_test.go +++ b/internal/store/postgres/relation_repository_test.go @@ -323,3 +323,26 @@ func (s *RelationRepositoryTestSuite) TestDeleteByID() { func TestRelationRepository(t *testing.T) { suite.Run(t, new(RelationRepositoryTestSuite)) } + +func (s *RelationRepositoryTestSuite) TestSkipsSoftDeletedRelations() { + deleted := s.relations[0] + _, err := s.client.ExecContext(s.ctx, "UPDATE relations SET deleted_at = now() WHERE id = $1", deleted.ID) + if err != nil { + s.T().Fatal(err) + } + + _, err = s.repository.Get(s.ctx, deleted.ID) + s.Assert().ErrorIs(err, relation.ErrNotExist) + + got, err := s.repository.List(s.ctx, relation.Filter{Subject: deleted.Subject, Object: deleted.Object}) + s.Assert().NoError(err) + for _, r := range got { + s.Assert().NotEqual(deleted.ID, r.ID) + } + + byFields, err := s.repository.GetByFields(s.ctx, deleted) + s.Assert().NoError(err) + for _, r := range byFields { + s.Assert().NotEqual(deleted.ID, r.ID) + } +} diff --git a/internal/store/postgres/resource_repository.go b/internal/store/postgres/resource_repository.go index bfb324ee5..4fec0982d 100644 --- a/internal/store/postgres/resource_repository.go +++ b/internal/store/postgres/resource_repository.go @@ -90,6 +90,8 @@ func (r ResourceRepository) Create(ctx context.Context, res resource.Resource) ( return resource.Resource{}, fmt.Errorf("%w: %w", err, resource.ErrInvalidDetail) case errors.Is(err, ErrInvalidTextRepresentation): return resource.Resource{}, fmt.Errorf("%w: %w", err, resource.ErrInvalidUUID) + case errors.Is(err, ErrDuplicateKey): + return resource.Resource{}, resource.ErrConflict default: return resource.Resource{}, err } @@ -101,7 +103,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 := dialect.From(TABLE_RESOURCES) + sqlStatement := fromLive(TABLE_RESOURCES) if flt.ProjectID != "" { sqlStatement = sqlStatement.Where(goqu.Ex{"project_id": flt.ProjectID}) } @@ -149,7 +151,7 @@ func (r ResourceRepository) GetByID(ctx context.Context, id string) (resource.Re return resource.Resource{}, resource.ErrInvalidID } - query, params, err := dialect.From(TABLE_RESOURCES).Where(goqu.Ex{ + query, params, err := fromLive(TABLE_RESOURCES).Where(goqu.Ex{ "id": id, }).ToSQL() if err != nil { @@ -189,7 +191,7 @@ func (r ResourceRepository) Update(ctx context.Context, res resource.Resource) ( "metadata": marshaledMetadata, "updated_at": goqu.L("now()"), }, - ).Where(goqu.Ex{"id": res.ID}).Returning(&ResourceCols{}).ToSQL() + ).Where(goqu.Ex{"id": res.ID}, live(TABLE_RESOURCES)).Returning(&ResourceCols{}).ToSQL() if err != nil { return resource.Resource{}, fmt.Errorf("%w: %s", errQuery, err) } @@ -221,7 +223,7 @@ func (r ResourceRepository) GetByURN(ctx context.Context, urn string) (resource. return resource.Resource{}, resource.ErrInvalidURN } - query, params, err := dialect.Select(&ResourceCols{}).From(TABLE_RESOURCES).Where( + query, params, err := fromLive(TABLE_RESOURCES).Select(&ResourceCols{}).Where( goqu.Ex{ "urn": urn, }).ToSQL() diff --git a/internal/store/postgres/resource_repository_test.go b/internal/store/postgres/resource_repository_test.go index 1bb565848..304bab8b9 100644 --- a/internal/store/postgres/resource_repository_test.go +++ b/internal/store/postgres/resource_repository_test.go @@ -411,3 +411,40 @@ func (s *ResourceRepositoryTestSuite) TestUpdate() { func TestResourceRepository(t *testing.T) { suite.Run(t, new(ResourceRepositoryTestSuite)) } + +func (s *ResourceRepositoryTestSuite) TestSkipsSoftDeletedResources() { + deleted := s.resources[0] + _, err := s.client.ExecContext(s.ctx, "UPDATE resources SET deleted_at = now() WHERE id = $1", deleted.ID) + if err != nil { + s.T().Fatal(err) + } + + _, err = s.repository.GetByID(s.ctx, deleted.ID) + s.Assert().ErrorIs(err, resource.ErrNotExist) + + _, err = s.repository.GetByURN(s.ctx, deleted.URN) + s.Assert().ErrorIs(err, resource.ErrNotExist) + + got, err := s.repository.List(s.ctx, resource.Filter{ProjectID: deleted.ProjectID}) + s.Assert().NoError(err) + for _, r := range got { + s.Assert().NotEqual(deleted.ID, r.ID) + } + + updated := deleted + updated.Title = "changed" + _, err = s.repository.Update(s.ctx, updated) + s.Assert().ErrorIs(err, resource.ErrNotExist) + + // the id of a deleted row is still taken; a create that reuses it with a new URN must conflict + _, err = s.repository.Create(s.ctx, resource.Resource{ + ID: deleted.ID, + URN: "urn-reusing-a-deleted-id", + Name: "reused-id", + ProjectID: deleted.ProjectID, + NamespaceID: deleted.NamespaceID, + PrincipalID: deleted.PrincipalID, + PrincipalType: deleted.PrincipalType, + }) + s.Assert().ErrorIs(err, resource.ErrConflict) +}