From bc65257542a3700c95ee8af7834b182760d5ab42 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Wed, 1 Jul 2026 22:47:48 -0300 Subject: [PATCH 1/9] HYPERFLEET-1192 - fix: prevent premature parent hard-delete with soft-deleted children Adds ExistsSoftDeletedByOwner DAO method to check for soft-deleted children. Parents are now soft-deleted (not hard-deleted) when they have soft-deleted children waiting for adapter finalization, preventing referential integrity issues and ensuring proper cleanup ordering. Co-Authored-By: Claude Sonnet 4.5 --- pkg/dao/mocks/resource.go | 9 +++ pkg/dao/resource.go | 12 ++++ pkg/services/resource.go | 29 ++++++++ pkg/services/resource_test.go | 132 ++++++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+) diff --git a/pkg/dao/mocks/resource.go b/pkg/dao/mocks/resource.go index 4a31cfae..6d90af73 100644 --- a/pkg/dao/mocks/resource.go +++ b/pkg/dao/mocks/resource.go @@ -72,6 +72,15 @@ func (d *resourceDaoMock) ExistsByOwner(_ context.Context, kind, ownerID string) return false, nil } +func (d *resourceDaoMock) ExistsSoftDeletedByOwner(_ context.Context, kind, ownerID string) (bool, error) { + for _, r := range d.resources { + if r.Kind == kind && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { + return true, nil + } + } + return false, nil +} + func (d *resourceDaoMock) FindByKind(_ context.Context, kind string) (api.ResourceList, error) { var result api.ResourceList for _, r := range d.resources { diff --git a/pkg/dao/resource.go b/pkg/dao/resource.go index d0c69a25..5b0abdfd 100644 --- a/pkg/dao/resource.go +++ b/pkg/dao/resource.go @@ -18,6 +18,7 @@ type ResourceDao interface { Save(ctx context.Context, resource *api.Resource) error Delete(ctx context.Context, kind, id string) error ExistsByOwner(ctx context.Context, kind, ownerID string) (bool, error) + ExistsSoftDeletedByOwner(ctx context.Context, kind, ownerID string) (bool, error) FindByKind(ctx context.Context, kind string) (api.ResourceList, error) FindByKindAndOwner(ctx context.Context, kind, ownerID string) (api.ResourceList, error) FindByKindAndOwnerForUpdate(ctx context.Context, kind, ownerID string) (api.ResourceList, error) @@ -111,6 +112,17 @@ func (d *sqlResourceDao) ExistsByOwner(ctx context.Context, kind, ownerID string return exists, nil } +func (d *sqlResourceDao) ExistsSoftDeletedByOwner(ctx context.Context, kind, ownerID string) (bool, error) { + g2 := d.sessionFactory.New(ctx) + var exists bool + if err := g2.Raw( + "SELECT EXISTS(SELECT 1 FROM resources WHERE kind = ? AND owner_id = ? AND deleted_time IS NOT NULL)", + kind, ownerID).Scan(&exists).Error; err != nil { + return false, err + } + return exists, nil +} + func (d *sqlResourceDao) FindByKind(ctx context.Context, kind string) (api.ResourceList, error) { g2 := d.sessionFactory.New(ctx) var resources api.ResourceList diff --git a/pkg/services/resource.go b/pkg/services/resource.go index 64f03cf7..f56e0808 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -195,8 +195,37 @@ func (s *sqlResourceService) deleteResourceTree( } } + // Determine whether to soft-delete or hard-delete this resource. + // Soft-delete is required when: + // 1. Resource has RequiredAdapters (existing behavior) + // 2. Resource has soft-deleted children waiting for adapter finalization desc := registry.MustGet(resource.Kind) + shouldSoftDelete := false + + // Reason 1: Resource has RequiredAdapters if len(desc.RequiredAdapters) > 0 { + shouldSoftDelete = true + } + + // Reason 2: Resource has soft-deleted children + // Parent must remain in DB until all children (active or soft-deleted) are gone + if !shouldSoftDelete { + for _, child := range children { + exists, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, child.Kind, resource.ID) + if err != nil { + return errors.GeneralError( + "Unable to check soft-deleted %s children: %s", child.Kind, err, + ) + } + if exists { + shouldSoftDelete = true + break + } + } + } + + // Execute delete based on decision + if shouldSoftDelete { if saveErr := s.resourceDao.Save(ctx, resource); saveErr != nil { return handleSoftDeleteError(resource.Kind, saveErr) } diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index 42126fb7..94ba70af 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -105,6 +105,15 @@ func (d *mockResourceDao) ExistsByOwner(_ context.Context, kind, ownerID string) return false, nil } +func (d *mockResourceDao) ExistsSoftDeletedByOwner(_ context.Context, kind, ownerID string) (bool, error) { + for _, r := range d.resources { + if r.Kind == kind && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { + return true, nil + } + } + return false, nil +} + func (d *mockResourceDao) FindByKind(_ context.Context, kind string) (api.ResourceList, error) { var result api.ResourceList for _, r := range d.resources { @@ -1119,3 +1128,126 @@ func TestResourceService_ListByOwner_UnknownKind(t *testing.T) { Expect(svcErr).ToNot(BeNil()) Expect(svcErr.HTTPCode).To(Equal(400)) } + +// --- Parent/Child Delete with RequiredAdapters --- + +// setupDescriptorsWithRequiredAdapters creates Channel (parent) and Version (child with RequiredAdapters) +func setupDescriptorsWithRequiredAdapters() { + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "Channel", + Plural: "channels", + }) + registry.Register(registry.EntityDescriptor{ + Kind: "Version", + Plural: "versions", + ParentKind: "Channel", + OnParentDelete: registry.OnParentDeleteRestrict, + RequiredAdapters: []string{"adapter1"}, // Version needs adapter finalization + }) +} + +func testResourceWithOwner(kind, id, name, ownerID string) *api.Resource { + spec, _ := json.Marshal(map[string]interface{}{"key": "value"}) + r := &api.Resource{ + Kind: kind, + Name: name, + Spec: spec, + Generation: 1, + OwnerID: &ownerID, + } + r.ID = id + return r +} + +// TestResourceService_Delete_ParentSoftDeletedWhileChildSoftDeleted verifies that when a child +// resource with RequiredAdapters is soft-deleted (waiting for adapter finalization), deleting +// the parent soft-deletes the parent instead of hard-deleting it. +// +// Parent should be soft-deleted (not hard-deleted) while any child row exists in the database, +// regardless of whether the child is active or soft-deleted. +func TestResourceService_Delete_ParentSoftDeletedWhileChildSoftDeleted(t *testing.T) { + RegisterTestingT(t) + setupDescriptorsWithRequiredAdapters() + + mockDao := newMockResourceDao() + svc, _, _ := newTestResourceService(mockDao) + + // Setup: Create Channel (parent) + channel := testResource("Channel", "ch-1", "stable") + mockDao.addResource(channel) + + // Setup: Create Version (child with RequiredAdapters) + version := testResourceWithOwner("Version", "v-1", "1.0.0", "ch-1") + mockDao.addResource(version) + + // Step 1: Delete the Version (soft-delete because of RequiredAdapters) + versionResult, svcErr := svc.Delete(context.Background(), "Version", "v-1") + Expect(svcErr).To(BeNil(), "Version delete should succeed") + Expect(versionResult.DeletedTime).ToNot(BeNil(), "Version should be soft-deleted") + + // Verify Version is still in the DAO (soft-deleted) + versionAfterDelete := mockDao.resources[resourceKey("Version", "v-1")] + Expect(versionAfterDelete).ToNot(BeNil(), "Version row should still exist after soft-delete") + Expect(versionAfterDelete.DeletedTime).ToNot(BeNil(), "Version should have deleted_time set") + + // Step 2: Delete the Channel + // Expected: Channel should be soft-deleted (not hard-deleted) because Version still exists in DB + _, svcErr = svc.Delete(context.Background(), "Channel", "ch-1") + Expect(svcErr).To(BeNil(), "Channel delete should succeed") + + // Verify: Channel should be soft-deleted (row still exists) + channelAfterDelete := mockDao.resources[resourceKey("Channel", "ch-1")] + Expect(channelAfterDelete).ToNot(BeNil(), "Channel should still exist in DB (soft-deleted)") + Expect(channelAfterDelete.DeletedTime).ToNot(BeNil(), "Channel should have deleted_time set") +} + +// TestResourceService_Delete_ParentHardDeletedAfterChildGone verifies that after all children +// are hard-deleted (adapter finalized), deleting a parent with no children hard-deletes it. +func TestResourceService_Delete_ParentHardDeletedAfterChildGone(t *testing.T) { + RegisterTestingT(t) + setupDescriptorsWithRequiredAdapters() + + mockDao := newMockResourceDao() + svc, _, _ := newTestResourceService(mockDao) + + // Setup + channel := testResource("Channel", "ch-1", "stable") + mockDao.addResource(channel) + + version := testResourceWithOwner("Version", "v-1", "1.0.0", "ch-1") + mockDao.addResource(version) + + // Delete Version (soft-delete) + _, svcErr := svc.Delete(context.Background(), "Version", "v-1") + Expect(svcErr).To(BeNil()) + + // Delete Channel (soft-delete because Version still exists) + _, svcErr = svc.Delete(context.Background(), "Channel", "ch-1") + Expect(svcErr).To(BeNil()) + + // Verify Channel is soft-deleted + channelSoftDeleted := mockDao.resources[resourceKey("Channel", "ch-1")] + Expect(channelSoftDeleted).ToNot(BeNil(), "Channel should be soft-deleted") + Expect(channelSoftDeleted.DeletedTime).ToNot(BeNil()) + + // Simulate adapter finalization: hard-delete Version directly from DB + err := mockDao.Delete(context.Background(), "Version", "v-1") + Expect(err).To(BeNil()) + Expect(mockDao.resources[resourceKey("Version", "v-1")]).To(BeNil(), "Version should be gone from DB") + + // Note: In production, a cleanup job would detect that Channel has no children + // and hard-delete it. For now, we verify that a fresh delete (of a non-deleted Channel) + // with no children does hard-delete. + + // Test with fresh Channel (no children) + channel2 := testResource("Channel", "ch-2", "beta") + mockDao.addResource(channel2) + + _, svcErr = svc.Delete(context.Background(), "Channel", "ch-2") + Expect(svcErr).To(BeNil()) + + // Channel2 should be hard-deleted immediately (no children) + channel2Gone := mockDao.resources[resourceKey("Channel", "ch-2")] + Expect(channel2Gone).To(BeNil(), "Channel should be hard-deleted when no children exist") +} From c55e10776f5c48cc2abbbe8d81d5a16ca1eb13ad Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Wed, 1 Jul 2026 23:30:35 -0300 Subject: [PATCH 2/9] HYPERFLEET-1192 - fix: prevent parent delete with pending children --- pkg/services/resource.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/services/resource.go b/pkg/services/resource.go index f56e0808..e6c7db16 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -142,15 +142,14 @@ func (s *sqlResourceService) Delete(ctx context.Context, kind, id string) (*api. return nil, handleSoftDeleteError(kind, err) } - if resource.DeletedTime != nil { - return resource, nil - } - deletedBy := actorFromContext(ctx) deletedAt := time.Now().UTC().Truncate(time.Microsecond) - resource.MarkDeleted(deletedBy, deletedAt) - resource.IncrementGeneration() + // Mark for deletion if not already soft-deleted + if resource.DeletedTime == nil { + resource.MarkDeleted(deletedBy, deletedAt) + resource.IncrementGeneration() + } if svcErr := s.deleteResourceTree(ctx, resource, deletedBy, deletedAt); svcErr != nil { db.MarkForRollback(ctx, svcErr) @@ -200,12 +199,9 @@ func (s *sqlResourceService) deleteResourceTree( // 1. Resource has RequiredAdapters (existing behavior) // 2. Resource has soft-deleted children waiting for adapter finalization desc := registry.MustGet(resource.Kind) - shouldSoftDelete := false // Reason 1: Resource has RequiredAdapters - if len(desc.RequiredAdapters) > 0 { - shouldSoftDelete = true - } + shouldSoftDelete := len(desc.RequiredAdapters) > 0 // Reason 2: Resource has soft-deleted children // Parent must remain in DB until all children (active or soft-deleted) are gone From 42989a668e56d021f71de41e521b2fb88338ef10 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Wed, 1 Jul 2026 23:54:08 -0300 Subject: [PATCH 3/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/services/resource.go | 59 ++++++++++++++++++++--------------- pkg/services/resource_test.go | 40 +++++++++++++++++++++--- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/pkg/services/resource.go b/pkg/services/resource.go index e6c7db16..d159eafc 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -194,33 +194,11 @@ func (s *sqlResourceService) deleteResourceTree( } } - // Determine whether to soft-delete or hard-delete this resource. - // Soft-delete is required when: - // 1. Resource has RequiredAdapters (existing behavior) - // 2. Resource has soft-deleted children waiting for adapter finalization - desc := registry.MustGet(resource.Kind) - - // Reason 1: Resource has RequiredAdapters - shouldSoftDelete := len(desc.RequiredAdapters) > 0 - - // Reason 2: Resource has soft-deleted children - // Parent must remain in DB until all children (active or soft-deleted) are gone - if !shouldSoftDelete { - for _, child := range children { - exists, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, child.Kind, resource.ID) - if err != nil { - return errors.GeneralError( - "Unable to check soft-deleted %s children: %s", child.Kind, err, - ) - } - if exists { - shouldSoftDelete = true - break - } - } + shouldSoftDelete, svcErr := s.shouldSoftDelete(ctx, resource, children) + if svcErr != nil { + return svcErr } - // Execute delete based on decision if shouldSoftDelete { if saveErr := s.resourceDao.Save(ctx, resource); saveErr != nil { return handleSoftDeleteError(resource.Kind, saveErr) @@ -235,6 +213,37 @@ func (s *sqlResourceService) deleteResourceTree( return nil } +// shouldSoftDelete determines whether a resource requires soft-deletion. +// Soft-delete is required when: +// 1. Resource has RequiredAdapters (must wait for adapter finalization) +// 2. Resource has soft-deleted children (parent must remain until children are gone) +func (s *sqlResourceService) shouldSoftDelete( + ctx context.Context, resource *api.Resource, children []registry.EntityDescriptor, +) (bool, *errors.ServiceError) { + desc := registry.MustGet(resource.Kind) + + // Reason 1: Resource has RequiredAdapters + if len(desc.RequiredAdapters) > 0 { + return true, nil + } + + // Reason 2: Resource has soft-deleted children + // Parent must remain in DB until all children (active or soft-deleted) are gone + for _, child := range children { + exists, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, child.Kind, resource.ID) + if err != nil { + return false, errors.GeneralError( + "Unable to check soft-deleted %s children: %s", child.Kind, err, + ) + } + if exists { + return true, nil + } + } + + return false, nil +} + func (s *sqlResourceService) checkCanDelete( ctx context.Context, resource *api.Resource, child registry.EntityDescriptor, ) *errors.ServiceError { diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index 94ba70af..eb5e348d 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -38,10 +38,11 @@ func setupTestDescriptors() { // mockResourceDao implements dao.ResourceDao for testing. type mockResourceDao struct { - resources map[string]*api.Resource - createErr error - saveErr error - deleteErr error + resources map[string]*api.Resource + createErr error + saveErr error + deleteErr error + existsSoftDeletedByOwnerErr error } func newMockResourceDao() *mockResourceDao { @@ -106,6 +107,9 @@ func (d *mockResourceDao) ExistsByOwner(_ context.Context, kind, ownerID string) } func (d *mockResourceDao) ExistsSoftDeletedByOwner(_ context.Context, kind, ownerID string) (bool, error) { + if d.existsSoftDeletedByOwnerErr != nil { + return false, d.existsSoftDeletedByOwnerErr + } for _, r := range d.resources { if r.Kind == kind && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { return true, nil @@ -1251,3 +1255,31 @@ func TestResourceService_Delete_ParentHardDeletedAfterChildGone(t *testing.T) { channel2Gone := mockDao.resources[resourceKey("Channel", "ch-2")] Expect(channel2Gone).To(BeNil(), "Channel should be hard-deleted when no children exist") } + +func TestResourceService_Delete_DAOErrorCheckingSoftDeletedChildren(t *testing.T) { + RegisterTestingT(t) + setupDescriptorsWithRequiredAdapters() + + svc, mockDao, _ := newTestResourceService(newMockResourceDao()) + + // Create parent and child + parent := testResource("Channel", "ch-1", "beta") + child := testResource("Version", "v-1", "1.0.0") + child.OwnerID = &parent.ID + mockDao.addResource(parent) + mockDao.addResource(child) + + // First delete: child is soft-deleted + _, err := svc.Delete(context.Background(), "Version", "v-1") + Expect(err).To(BeNil()) + Expect(mockDao.resources[resourceKey("Version", "v-1")].DeletedTime).NotTo(BeNil()) + + // Inject DAO error for ExistsSoftDeletedByOwner + mockDao.existsSoftDeletedByOwnerErr = gorm.ErrInvalidDB + + // Attempt to delete parent - should fail with GeneralError + _, svcErr := svc.Delete(context.Background(), "Channel", "ch-1") + Expect(svcErr).NotTo(BeNil()) + Expect(svcErr.RFC9457Code).To(Equal("HYPERFLEET-INT-001")) + Expect(svcErr.Reason).To(ContainSubstring("Unable to check soft-deleted Version children")) +} From 5f3dd6c8c30a9c0238155eec851162f08a453da7 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 00:26:53 -0300 Subject: [PATCH 4/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/services/resource_test.go | 19 +-- test/integration/resource_delete_test.go | 191 +++++++++++++++++++++++ 2 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 test/integration/resource_delete_test.go diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index eb5e348d..69973667 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -1240,20 +1240,15 @@ func TestResourceService_Delete_ParentHardDeletedAfterChildGone(t *testing.T) { Expect(err).To(BeNil()) Expect(mockDao.resources[resourceKey("Version", "v-1")]).To(BeNil(), "Version should be gone from DB") - // Note: In production, a cleanup job would detect that Channel has no children - // and hard-delete it. For now, we verify that a fresh delete (of a non-deleted Channel) - // with no children does hard-delete. - - // Test with fresh Channel (no children) - channel2 := testResource("Channel", "ch-2", "beta") - mockDao.addResource(channel2) - - _, svcErr = svc.Delete(context.Background(), "Channel", "ch-2") + // Re-delete the already soft-deleted Channel - should now hard-delete + // This exercises the re-evaluation path: parent was soft-deleted, child is now gone, + // so calling Delete() again should detect no blockers and hard-delete the parent. + _, svcErr = svc.Delete(context.Background(), "Channel", "ch-1") Expect(svcErr).To(BeNil()) - // Channel2 should be hard-deleted immediately (no children) - channel2Gone := mockDao.resources[resourceKey("Channel", "ch-2")] - Expect(channel2Gone).To(BeNil(), "Channel should be hard-deleted when no children exist") + // Channel should now be hard-deleted (removed from DB) + channelAfterRedelete := mockDao.resources[resourceKey("Channel", "ch-1")] + Expect(channelAfterRedelete).To(BeNil(), "Channel should be hard-deleted after re-evaluation with no children") } func TestResourceService_Delete_DAOErrorCheckingSoftDeletedChildren(t *testing.T) { diff --git a/test/integration/resource_delete_test.go b/test/integration/resource_delete_test.go new file mode 100644 index 00000000..ab8b5391 --- /dev/null +++ b/test/integration/resource_delete_test.go @@ -0,0 +1,191 @@ +package integration + +import ( + "fmt" + "testing" + + "github.com/google/uuid" + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" +) + +// TestResourceDelete_ParentChildWithRequiredAdapters tests the parent/child delete behavior +// when the child has RequiredAdapters configured. +// +// This test validates the fix for preventing parent hard-delete while child is soft-deleted. +func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { + t.Run("ParentSoftDeletedWhileChildSoftDeleted", func(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + // Verify Version has RequiredAdapters configured + versionDesc := registry.MustGet("Version") + if len(versionDesc.RequiredAdapters) == 0 { + t.Skip("Version does not have RequiredAdapters - cannot test soft-delete behavior") + } + + // Create Channel (parent) + channelName := fmt.Sprintf("test-delete-channel-%s", uuid.NewString()[:8]) + channel := newChannelResource(channelName) + createdChannel, svcErr := svc.Create(t.Context(), "Channel", channel) + Expect(svcErr).To(BeNil(), "Channel creation should succeed") + Expect(createdChannel.ID).NotTo(BeEmpty()) + + // Create Version (child with RequiredAdapters) + versionName := fmt.Sprintf("v1.0.0-%s", uuid.NewString()[:8]) + version := newVersionResource(versionName, createdChannel.ID) + createdVersion, svcErr := svc.Create(t.Context(), "Version", version) + Expect(svcErr).To(BeNil(), "Version creation should succeed") + Expect(createdVersion.ID).NotTo(BeEmpty()) + + // Verify Version was created + retrievedVersion, svcErr := svc.Get(t.Context(), "Version", createdVersion.ID) + Expect(svcErr).To(BeNil()) + Expect(retrievedVersion.DeletedTime).To(BeNil(), "Version should not be deleted yet") + + // Try to delete Channel with active child - should fail with 409 + _, svcErr = svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil(), "Deleting Channel with active child should fail") + Expect(svcErr.HTTPCode).To(Equal(409), "Should return 409 Conflict") + Expect(svcErr.Reason).To(ContainSubstring("active"), "Error should mention active children") + + // Delete Version - should be soft-deleted (has RequiredAdapters) + deletedVersion, svcErr := svc.Delete(t.Context(), "Version", createdVersion.ID) + Expect(svcErr).To(BeNil(), "Version deletion should succeed") + Expect(deletedVersion.DeletedTime).ToNot(BeNil(), "Version should be soft-deleted") + + // Verify Version is soft-deleted in database + versionAfterDelete, svcErr := svc.Get(t.Context(), "Version", createdVersion.ID) + Expect(svcErr).To(BeNil(), "Should retrieve soft-deleted Version") + Expect(versionAfterDelete.DeletedTime).ToNot(BeNil(), "Version should have deleted_time set") + + // Delete Channel - should be SOFT-DELETED (not hard-deleted) + // This is the key test: parent must be soft-deleted while child is soft-deleted + deletedChannel, svcErr := svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).To(BeNil(), "Channel deletion should succeed") + Expect(deletedChannel.DeletedTime).ToNot(BeNil(), "Channel should be soft-deleted") + + // VALIDATION: Verify Channel is soft-deleted in database (not hard-deleted) + channelAfterDelete, svcErr := svc.Get(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).To(BeNil(), "Should retrieve soft-deleted Channel") + Expect(channelAfterDelete.DeletedTime).ToNot(BeNil(), "Channel should have deleted_time set") + + // Verify both resources still exist in database (soft-deleted) + err := checkResourceCount(t.Context(), h, []string{createdChannel.ID, createdVersion.ID}, 2) + Expect(err).To(BeNil(), "Both Channel and Version should still exist in DB (soft-deleted)") + + t.Logf("✓ Channel was soft-deleted while Version is soft-deleted (fix working)") + }) + + t.Run("ParentHardDeletedAfterChildrenGone", func(t *testing.T) { + RegisterTestingT(t) + svc, h := setupResourceTest(t) + + // Create Channel without children + channelName := fmt.Sprintf("test-delete-orphan-%s", uuid.NewString()[:8]) + channel := newChannelResource(channelName) + createdChannel, svcErr := svc.Create(t.Context(), "Channel", channel) + Expect(svcErr).To(BeNil(), "Channel creation should succeed") + + // Delete Channel (no children) - should be HARD-DELETED + deletedChannel, svcErr := svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).To(BeNil(), "Channel deletion should succeed") + + // Channel has no RequiredAdapters and no children, so it should be hard-deleted + channelDesc := registry.MustGet("Channel") + if len(channelDesc.RequiredAdapters) == 0 { + // Verify Channel is hard-deleted (removed from DB) + _, svcErr := svc.Get(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil(), "Should not retrieve hard-deleted Channel") + Expect(svcErr.HTTPCode).To(Equal(404), "Should return 404 for hard-deleted resource") + + // Verify Channel no longer exists in database + err := checkResourceCount(t.Context(), h, []string{createdChannel.ID}, 0) + Expect(err).To(BeNil(), "Channel should be hard-deleted (not in DB)") + + t.Logf("✓ Channel was hard-deleted when it has no children (no regression)") + } else { + // Channel has RequiredAdapters - will be soft-deleted + Expect(deletedChannel.DeletedTime).ToNot(BeNil(), "Channel should be soft-deleted") + t.Logf("ℹ Channel has RequiredAdapters - soft-deleted (expected)") + } + }) + + t.Run("ActiveChildBlocksParentDelete", func(t *testing.T) { + RegisterTestingT(t) + svc, _ := setupResourceTest(t) + + // Create Channel + channelName := fmt.Sprintf("test-restrict-%s", uuid.NewString()[:8]) + channel := newChannelResource(channelName) + createdChannel, svcErr := svc.Create(t.Context(), "Channel", channel) + Expect(svcErr).To(BeNil()) + + // Create active Version + versionName := fmt.Sprintf("v1.0.0-%s", uuid.NewString()[:8]) + version := newVersionResource(versionName, createdChannel.ID) + _, svcErr = svc.Create(t.Context(), "Version", version) + Expect(svcErr).To(BeNil()) + + // Try to delete Channel - should fail (OnParentDelete=Restrict) + _, svcErr = svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil(), "Should not delete Channel with active child") + Expect(svcErr.HTTPCode).To(Equal(409)) + Expect(svcErr.Reason).To(ContainSubstring("active"), "Error should mention active children") + + t.Logf("✓ Active child blocks parent delete (AC1 validated)") + }) +} + +// TestResourceDelete_WithoutRequiredAdapters tests delete behavior when Version +// does NOT have RequiredAdapters configured (hard-delete scenario). +func TestResourceDelete_WithoutRequiredAdapters(t *testing.T) { + t.Run("ChildHardDeletedImmediately", func(t *testing.T) { + RegisterTestingT(t) + + // Check if Version has RequiredAdapters + versionDesc := registry.MustGet("Version") + if len(versionDesc.RequiredAdapters) > 0 { + t.Skip("Version has RequiredAdapters - this test requires no RequiredAdapters") + } + + svc, h := setupResourceTest(t) + + // Create Channel + channelName := fmt.Sprintf("test-harddelete-%s", uuid.NewString()[:8]) + channel := newChannelResource(channelName) + createdChannel, svcErr := svc.Create(t.Context(), "Channel", channel) + Expect(svcErr).To(BeNil()) + + // Create Version (no RequiredAdapters) + versionName := fmt.Sprintf("v1.0.0-%s", uuid.NewString()[:8]) + version := newVersionResource(versionName, createdChannel.ID) + createdVersion, svcErr := svc.Create(t.Context(), "Version", version) + Expect(svcErr).To(BeNil()) + + // Delete Version - should be HARD-DELETED (no RequiredAdapters) + _, svcErr = svc.Delete(t.Context(), "Version", createdVersion.ID) + Expect(svcErr).To(BeNil()) + + // Verify Version is hard-deleted (404) + _, svcErr = svc.Get(t.Context(), "Version", createdVersion.ID) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(404)) + + // Verify Version removed from database + err := checkResourceCount(t.Context(), h, []string{createdVersion.ID}, 0) + Expect(err).To(BeNil(), "Version should be hard-deleted") + + // Delete Channel - should also be hard-deleted (no children left) + _, svcErr = svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).To(BeNil()) + + // Verify Channel is hard-deleted + _, svcErr = svc.Get(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil()) + Expect(svcErr.HTTPCode).To(Equal(404)) + + t.Logf("ℹ Without RequiredAdapters: both parent and child hard-deleted immediately") + }) +} From d2c83e3c42c2012134f4732df1c7692c38eb5a56 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 10:50:20 -0300 Subject: [PATCH 5/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- test/integration/resource_delete_test.go | 94 +++++++++++++++++------- test/integration/resource_helpers.go | 8 ++ 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/test/integration/resource_delete_test.go b/test/integration/resource_delete_test.go index ab8b5391..e6aaa792 100644 --- a/test/integration/resource_delete_test.go +++ b/test/integration/resource_delete_test.go @@ -19,10 +19,16 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { RegisterTestingT(t) svc, h := setupResourceTest(t) - // Verify Version has RequiredAdapters configured + // Ensure Version has RequiredAdapters for this test versionDesc := registry.MustGet("Version") - if len(versionDesc.RequiredAdapters) == 0 { - t.Skip("Version does not have RequiredAdapters - cannot test soft-delete behavior") + originalAdapters := versionDesc.RequiredAdapters + if len(originalAdapters) == 0 { + versionDesc.RequiredAdapters = []string{"test-adapter"} + registry.Register(versionDesc) + defer func() { + versionDesc.RequiredAdapters = originalAdapters + registry.Register(versionDesc) + }() } // Create Channel (parent) @@ -76,40 +82,70 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { Expect(err).To(BeNil(), "Both Channel and Version should still exist in DB (soft-deleted)") t.Logf("✓ Channel was soft-deleted while Version is soft-deleted (fix working)") + + // CONVERGENCE TEST: Simulate adapter finalization and verify parent can be hard-deleted + // This tests the re-evaluation path: soft-deleted parent → child removed → hard-delete + err = hardDeleteResource(t.Context(), h, "Version", createdVersion.ID) + Expect(err).To(BeNil(), "Direct DB deletion should succeed (simulates adapter finalization)") + + // Verify Version is gone from database + err = checkResourceCount(t.Context(), h, []string{createdVersion.ID}, 0) + Expect(err).To(BeNil(), "Version should be hard-deleted from DB") + + // Re-delete the soft-deleted Channel - should now hard-delete + // This exercises the re-evaluation: Channel was soft-deleted, child is now gone, + // so Delete() should detect no blockers and hard-delete the parent + _, svcErr = svc.Delete(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).To(BeNil(), "Channel re-delete should succeed") + + // Verify Channel is hard-deleted (404) + _, svcErr = svc.Get(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil(), "Should not retrieve hard-deleted Channel") + Expect(svcErr.HTTPCode).To(Equal(404), "Should return 404 for hard-deleted Channel") + + // Verify Channel is gone from database + err = checkResourceCount(t.Context(), h, []string{createdChannel.ID}, 0) + Expect(err).To(BeNil(), "Channel should be hard-deleted after re-evaluation") + + t.Logf("✓ Channel hard-deleted after child finalization (convergence working)") }) t.Run("ParentHardDeletedAfterChildrenGone", func(t *testing.T) { RegisterTestingT(t) svc, h := setupResourceTest(t) + // Ensure Channel has NO RequiredAdapters for this test + channelDesc := registry.MustGet("Channel") + originalAdapters := channelDesc.RequiredAdapters + if len(originalAdapters) > 0 { + channelDesc.RequiredAdapters = []string{} + registry.Register(channelDesc) + defer func() { + channelDesc.RequiredAdapters = originalAdapters + registry.Register(channelDesc) + }() + } + // Create Channel without children channelName := fmt.Sprintf("test-delete-orphan-%s", uuid.NewString()[:8]) channel := newChannelResource(channelName) createdChannel, svcErr := svc.Create(t.Context(), "Channel", channel) Expect(svcErr).To(BeNil(), "Channel creation should succeed") - // Delete Channel (no children) - should be HARD-DELETED - deletedChannel, svcErr := svc.Delete(t.Context(), "Channel", createdChannel.ID) + // Delete Channel (no children, no RequiredAdapters) - should be HARD-DELETED + _, svcErr = svc.Delete(t.Context(), "Channel", createdChannel.ID) Expect(svcErr).To(BeNil(), "Channel deletion should succeed") - // Channel has no RequiredAdapters and no children, so it should be hard-deleted - channelDesc := registry.MustGet("Channel") - if len(channelDesc.RequiredAdapters) == 0 { - // Verify Channel is hard-deleted (removed from DB) - _, svcErr := svc.Get(t.Context(), "Channel", createdChannel.ID) - Expect(svcErr).ToNot(BeNil(), "Should not retrieve hard-deleted Channel") - Expect(svcErr.HTTPCode).To(Equal(404), "Should return 404 for hard-deleted resource") - - // Verify Channel no longer exists in database - err := checkResourceCount(t.Context(), h, []string{createdChannel.ID}, 0) - Expect(err).To(BeNil(), "Channel should be hard-deleted (not in DB)") - - t.Logf("✓ Channel was hard-deleted when it has no children (no regression)") - } else { - // Channel has RequiredAdapters - will be soft-deleted - Expect(deletedChannel.DeletedTime).ToNot(BeNil(), "Channel should be soft-deleted") - t.Logf("ℹ Channel has RequiredAdapters - soft-deleted (expected)") - } + // Verify Channel is hard-deleted (removed from DB) + _, svcErr = svc.Get(t.Context(), "Channel", createdChannel.ID) + Expect(svcErr).ToNot(BeNil(), "Should not retrieve hard-deleted Channel") + Expect(svcErr.HTTPCode).To(Equal(404), "Should return 404 for hard-deleted resource") + + // Verify Channel no longer exists in database + err := checkResourceCount(t.Context(), h, []string{createdChannel.ID}, 0) + Expect(err).To(BeNil(), "Channel should be hard-deleted (not in DB)") + + t.Logf("✓ Channel was hard-deleted when it has no children (no regression)") }) t.Run("ActiveChildBlocksParentDelete", func(t *testing.T) { @@ -144,10 +180,16 @@ func TestResourceDelete_WithoutRequiredAdapters(t *testing.T) { t.Run("ChildHardDeletedImmediately", func(t *testing.T) { RegisterTestingT(t) - // Check if Version has RequiredAdapters + // Ensure Version has NO RequiredAdapters for this test versionDesc := registry.MustGet("Version") - if len(versionDesc.RequiredAdapters) > 0 { - t.Skip("Version has RequiredAdapters - this test requires no RequiredAdapters") + originalAdapters := versionDesc.RequiredAdapters + if len(originalAdapters) > 0 { + versionDesc.RequiredAdapters = []string{} + registry.Register(versionDesc) + defer func() { + versionDesc.RequiredAdapters = originalAdapters + registry.Register(versionDesc) + }() } svc, h := setupResourceTest(t) diff --git a/test/integration/resource_helpers.go b/test/integration/resource_helpers.go index c8ed12ed..1de8976f 100644 --- a/test/integration/resource_helpers.go +++ b/test/integration/resource_helpers.go @@ -58,3 +58,11 @@ func newVersionResource(name, channelID string) *api.Resource { UpdatedBy: "test@example.com", } } + +// hardDeleteResource directly deletes a resource from the database, bypassing service layer. +// Used to simulate adapter finalization in tests. +func hardDeleteResource(ctx context.Context, h *test.Helper, kind, id string) error { + dbSession := h.DBFactory.New(ctx) + result := dbSession.Where("kind = ? AND id = ?", kind, id).Delete(&api.Resource{}) + return result.Error +} From b01745ed17957a6b1ab543a6db2d5a61ca0d5559 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 11:27:39 -0300 Subject: [PATCH 6/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/registry/registry.go | 11 +++++++++ test/integration/resource_delete_test.go | 30 ++++++++++++++---------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index e609fb15..eabc8a95 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -117,3 +117,14 @@ func ValidateSpecSchemas(schemaExists func(string) bool) { func Reset() { descriptors = make(map[string]EntityDescriptor) } + +// UpdateDescriptor modifies an existing descriptor in-place. Panics if kind not found. +// Used by tests to temporarily override descriptor fields. +func UpdateDescriptor(kind string, updateFn func(*EntityDescriptor)) { + desc, ok := descriptors[kind] + if !ok { + panic(fmt.Sprintf("entity kind %q not registered", kind)) + } + updateFn(&desc) + descriptors[kind] = desc +} diff --git a/test/integration/resource_delete_test.go b/test/integration/resource_delete_test.go index e6aaa792..090613d1 100644 --- a/test/integration/resource_delete_test.go +++ b/test/integration/resource_delete_test.go @@ -23,11 +23,13 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { versionDesc := registry.MustGet("Version") originalAdapters := versionDesc.RequiredAdapters if len(originalAdapters) == 0 { - versionDesc.RequiredAdapters = []string{"test-adapter"} - registry.Register(versionDesc) + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = []string{"test-adapter"} + }) defer func() { - versionDesc.RequiredAdapters = originalAdapters - registry.Register(versionDesc) + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = originalAdapters + }) }() } @@ -118,11 +120,13 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { channelDesc := registry.MustGet("Channel") originalAdapters := channelDesc.RequiredAdapters if len(originalAdapters) > 0 { - channelDesc.RequiredAdapters = []string{} - registry.Register(channelDesc) + registry.UpdateDescriptor("Channel", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = []string{} + }) defer func() { - channelDesc.RequiredAdapters = originalAdapters - registry.Register(channelDesc) + registry.UpdateDescriptor("Channel", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = originalAdapters + }) }() } @@ -184,11 +188,13 @@ func TestResourceDelete_WithoutRequiredAdapters(t *testing.T) { versionDesc := registry.MustGet("Version") originalAdapters := versionDesc.RequiredAdapters if len(originalAdapters) > 0 { - versionDesc.RequiredAdapters = []string{} - registry.Register(versionDesc) + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = []string{} + }) defer func() { - versionDesc.RequiredAdapters = originalAdapters - registry.Register(versionDesc) + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = originalAdapters + }) }() } From b3399559b735a3ddd6a78178d3e4bba9e5148c62 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 13:08:50 -0300 Subject: [PATCH 7/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/services/resource_test.go | 51 +++++++++++++++++++++++- test/integration/resource_delete_test.go | 50 +++++++++-------------- 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index 69973667..a4160575 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -1151,6 +1151,21 @@ func setupDescriptorsWithRequiredAdapters() { }) } +func setupDescriptorsWithCascadeAndRequiredAdapters() { + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "Workspace", + Plural: "workspaces", + }) + registry.Register(registry.EntityDescriptor{ + Kind: "Task", + Plural: "tasks", + ParentKind: "Workspace", + OnParentDelete: registry.OnParentDeleteCascade, + RequiredAdapters: []string{"adapter1"}, + }) +} + func testResourceWithOwner(kind, id, name, ownerID string) *api.Resource { spec, _ := json.Marshal(map[string]interface{}{"key": "value"}) r := &api.Resource{ @@ -1259,8 +1274,7 @@ func TestResourceService_Delete_DAOErrorCheckingSoftDeletedChildren(t *testing.T // Create parent and child parent := testResource("Channel", "ch-1", "beta") - child := testResource("Version", "v-1", "1.0.0") - child.OwnerID = &parent.ID + child := testResourceWithOwner("Version", "v-1", "1.0.0", parent.ID) mockDao.addResource(parent) mockDao.addResource(child) @@ -1278,3 +1292,36 @@ func TestResourceService_Delete_DAOErrorCheckingSoftDeletedChildren(t *testing.T Expect(svcErr.RFC9457Code).To(Equal("HYPERFLEET-INT-001")) Expect(svcErr.Reason).To(ContainSubstring("Unable to check soft-deleted Version children")) } + +// TestResourceService_Delete_CascadeParentSoftDeletedWhileChildSoftDeleted validates AC #4: +// "For generic resources using OnParentDeleteCascade, a parent with a soft-deleted child +// that has RequiredAdapters is not hard-deleted while the child row remains." +func TestResourceService_Delete_CascadeParentSoftDeletedWhileChildSoftDeleted(t *testing.T) { + RegisterTestingT(t) + setupDescriptorsWithCascadeAndRequiredAdapters() + + mockDao := newMockResourceDao() + svc, _, _ := newTestResourceService(mockDao) + + // Create parent and child + workspace := testResource("Workspace", "ws-1", "dev") + mockDao.addResource(workspace) + + task := testResourceWithOwner("Task", "t-1", "build", "ws-1") + mockDao.addResource(task) + + // Delete Workspace → cascade-deletes Task (soft-delete because Task has RequiredAdapters) + // → Workspace should be soft-deleted (soft-deleted child exists) + _, svcErr := svc.Delete(context.Background(), "Workspace", "ws-1") + Expect(svcErr).To(BeNil()) + + // Verify Workspace is soft-deleted (not hard-deleted) + ws := mockDao.resources[resourceKey("Workspace", "ws-1")] + Expect(ws).ToNot(BeNil(), "Workspace should still exist (soft-deleted)") + Expect(ws.DeletedTime).ToNot(BeNil(), "Workspace should have deleted_time set") + + // Verify Task was cascade-deleted and is also soft-deleted + tk := mockDao.resources[resourceKey("Task", "t-1")] + Expect(tk).ToNot(BeNil(), "Task should still exist (soft-deleted)") + Expect(tk.DeletedTime).ToNot(BeNil(), "Task should have deleted_time set") +} diff --git a/test/integration/resource_delete_test.go b/test/integration/resource_delete_test.go index 090613d1..9735a045 100644 --- a/test/integration/resource_delete_test.go +++ b/test/integration/resource_delete_test.go @@ -19,19 +19,15 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { RegisterTestingT(t) svc, h := setupResourceTest(t) - // Ensure Version has RequiredAdapters for this test - versionDesc := registry.MustGet("Version") - originalAdapters := versionDesc.RequiredAdapters - if len(originalAdapters) == 0 { + // Temporarily add RequiredAdapters to Version for this test + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = []string{"test-adapter"} + }) + t.Cleanup(func() { registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = []string{"test-adapter"} + d.RequiredAdapters = nil }) - defer func() { - registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = originalAdapters - }) - }() - } + }) // Create Channel (parent) channelName := fmt.Sprintf("test-delete-channel-%s", uuid.NewString()[:8]) @@ -117,18 +113,14 @@ func TestResourceDelete_ParentChildWithRequiredAdapters(t *testing.T) { svc, h := setupResourceTest(t) // Ensure Channel has NO RequiredAdapters for this test - channelDesc := registry.MustGet("Channel") - originalAdapters := channelDesc.RequiredAdapters - if len(originalAdapters) > 0 { + registry.UpdateDescriptor("Channel", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = nil + }) + t.Cleanup(func() { registry.UpdateDescriptor("Channel", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = []string{} + d.RequiredAdapters = nil }) - defer func() { - registry.UpdateDescriptor("Channel", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = originalAdapters - }) - }() - } + }) // Create Channel without children channelName := fmt.Sprintf("test-delete-orphan-%s", uuid.NewString()[:8]) @@ -185,18 +177,14 @@ func TestResourceDelete_WithoutRequiredAdapters(t *testing.T) { RegisterTestingT(t) // Ensure Version has NO RequiredAdapters for this test - versionDesc := registry.MustGet("Version") - originalAdapters := versionDesc.RequiredAdapters - if len(originalAdapters) > 0 { + registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { + d.RequiredAdapters = nil + }) + t.Cleanup(func() { registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = []string{} + d.RequiredAdapters = nil }) - defer func() { - registry.UpdateDescriptor("Version", func(d *registry.EntityDescriptor) { - d.RequiredAdapters = originalAdapters - }) - }() - } + }) svc, h := setupResourceTest(t) From 6a470a04f86e8e74efa4c34719abacfa94bba212 Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 15:51:46 -0300 Subject: [PATCH 8/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/dao/mocks/resource.go | 11 +++++++++-- pkg/dao/resource.go | 11 +++++++---- pkg/services/resource.go | 10 +++++++--- pkg/services/resource_test.go | 13 ++++++++++--- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/pkg/dao/mocks/resource.go b/pkg/dao/mocks/resource.go index 6d90af73..da32face 100644 --- a/pkg/dao/mocks/resource.go +++ b/pkg/dao/mocks/resource.go @@ -72,9 +72,16 @@ func (d *resourceDaoMock) ExistsByOwner(_ context.Context, kind, ownerID string) return false, nil } -func (d *resourceDaoMock) ExistsSoftDeletedByOwner(_ context.Context, kind, ownerID string) (bool, error) { +func (d *resourceDaoMock) ExistsSoftDeletedByOwner(_ context.Context, kinds []string, ownerID string) (bool, error) { + if len(kinds) == 0 { + return false, nil + } + kindSet := make(map[string]bool, len(kinds)) + for _, k := range kinds { + kindSet[k] = true + } for _, r := range d.resources { - if r.Kind == kind && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { + if kindSet[r.Kind] && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { return true, nil } } diff --git a/pkg/dao/resource.go b/pkg/dao/resource.go index 5b0abdfd..b7ee7786 100644 --- a/pkg/dao/resource.go +++ b/pkg/dao/resource.go @@ -18,7 +18,7 @@ type ResourceDao interface { Save(ctx context.Context, resource *api.Resource) error Delete(ctx context.Context, kind, id string) error ExistsByOwner(ctx context.Context, kind, ownerID string) (bool, error) - ExistsSoftDeletedByOwner(ctx context.Context, kind, ownerID string) (bool, error) + ExistsSoftDeletedByOwner(ctx context.Context, kinds []string, ownerID string) (bool, error) FindByKind(ctx context.Context, kind string) (api.ResourceList, error) FindByKindAndOwner(ctx context.Context, kind, ownerID string) (api.ResourceList, error) FindByKindAndOwnerForUpdate(ctx context.Context, kind, ownerID string) (api.ResourceList, error) @@ -112,12 +112,15 @@ func (d *sqlResourceDao) ExistsByOwner(ctx context.Context, kind, ownerID string return exists, nil } -func (d *sqlResourceDao) ExistsSoftDeletedByOwner(ctx context.Context, kind, ownerID string) (bool, error) { +func (d *sqlResourceDao) ExistsSoftDeletedByOwner(ctx context.Context, kinds []string, ownerID string) (bool, error) { + if len(kinds) == 0 { + return false, nil + } g2 := d.sessionFactory.New(ctx) var exists bool if err := g2.Raw( - "SELECT EXISTS(SELECT 1 FROM resources WHERE kind = ? AND owner_id = ? AND deleted_time IS NOT NULL)", - kind, ownerID).Scan(&exists).Error; err != nil { + "SELECT EXISTS(SELECT 1 FROM resources WHERE kind IN (?) AND owner_id = ? AND deleted_time IS NOT NULL)", + kinds, ownerID).Scan(&exists).Error; err != nil { return false, err } return exists, nil diff --git a/pkg/services/resource.go b/pkg/services/resource.go index d159eafc..e44794ee 100644 --- a/pkg/services/resource.go +++ b/pkg/services/resource.go @@ -229,11 +229,15 @@ func (s *sqlResourceService) shouldSoftDelete( // Reason 2: Resource has soft-deleted children // Parent must remain in DB until all children (active or soft-deleted) are gone - for _, child := range children { - exists, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, child.Kind, resource.ID) + if len(children) > 0 { + childKinds := make([]string, len(children)) + for i, child := range children { + childKinds[i] = child.Kind + } + exists, err := s.resourceDao.ExistsSoftDeletedByOwner(ctx, childKinds, resource.ID) if err != nil { return false, errors.GeneralError( - "Unable to check soft-deleted %s children: %s", child.Kind, err, + "Unable to check soft-deleted children: %s", err, ) } if exists { diff --git a/pkg/services/resource_test.go b/pkg/services/resource_test.go index a4160575..31d0cbd1 100644 --- a/pkg/services/resource_test.go +++ b/pkg/services/resource_test.go @@ -106,12 +106,19 @@ func (d *mockResourceDao) ExistsByOwner(_ context.Context, kind, ownerID string) return false, nil } -func (d *mockResourceDao) ExistsSoftDeletedByOwner(_ context.Context, kind, ownerID string) (bool, error) { +func (d *mockResourceDao) ExistsSoftDeletedByOwner(_ context.Context, kinds []string, ownerID string) (bool, error) { if d.existsSoftDeletedByOwnerErr != nil { return false, d.existsSoftDeletedByOwnerErr } + if len(kinds) == 0 { + return false, nil + } + kindSet := make(map[string]bool, len(kinds)) + for _, k := range kinds { + kindSet[k] = true + } for _, r := range d.resources { - if r.Kind == kind && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { + if kindSet[r.Kind] && r.OwnerID != nil && *r.OwnerID == ownerID && r.DeletedTime != nil { return true, nil } } @@ -1290,7 +1297,7 @@ func TestResourceService_Delete_DAOErrorCheckingSoftDeletedChildren(t *testing.T _, svcErr := svc.Delete(context.Background(), "Channel", "ch-1") Expect(svcErr).NotTo(BeNil()) Expect(svcErr.RFC9457Code).To(Equal("HYPERFLEET-INT-001")) - Expect(svcErr.Reason).To(ContainSubstring("Unable to check soft-deleted Version children")) + Expect(svcErr.Reason).To(ContainSubstring("Unable to check soft-deleted children")) } // TestResourceService_Delete_CascadeParentSoftDeletedWhileChildSoftDeleted validates AC #4: From 3d0e49be2442aade5b79a4277b2c76e653a2105d Mon Sep 17 00:00:00 2001 From: Leonardo Dorneles Date: Thu, 2 Jul 2026 16:36:23 -0300 Subject: [PATCH 9/9] HYPERFLEET-1192 - fix: prevent parent hard-delete with soft-deleted children --- pkg/dao/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dao/resource.go b/pkg/dao/resource.go index b7ee7786..213a25a7 100644 --- a/pkg/dao/resource.go +++ b/pkg/dao/resource.go @@ -121,7 +121,7 @@ func (d *sqlResourceDao) ExistsSoftDeletedByOwner(ctx context.Context, kinds []s if err := g2.Raw( "SELECT EXISTS(SELECT 1 FROM resources WHERE kind IN (?) AND owner_id = ? AND deleted_time IS NOT NULL)", kinds, ownerID).Scan(&exists).Error; err != nil { - return false, err + return false, fmt.Errorf("failed to check soft-deleted children: %w", err) } return exists, nil }