From 8821bea569b0d6402bfdc428d007c15b3a56accc Mon Sep 17 00:00:00 2001 From: Sneha Gunta Date: Thu, 30 Apr 2026 16:00:04 -0400 Subject: [PATCH 1/4] use Version tiny type for all version attributes --- internal/biz/model/representations.go | 10 +++--- internal/biz/model/resource_repository.go | 2 +- internal/biz/model/schema_service_test.go | 32 +++++++++++-------- .../resources/resource_service_test.go | 3 +- internal/consumer/consumer.go | 16 +++------- internal/data/fake_resource_repository.go | 23 +++++++------ internal/data/resource_repository.go | 19 +++++------ internal/data/resource_repository_test.go | 16 +++++----- 8 files changed, 60 insertions(+), 61 deletions(-) diff --git a/internal/biz/model/representations.go b/internal/biz/model/representations.go index ab14358d5..e6ebf9367 100644 --- a/internal/biz/model/representations.go +++ b/internal/biz/model/representations.go @@ -10,9 +10,9 @@ import ( // If a representation is present, its version must also be present (and vice versa). type Representations struct { commonData Representation - commonVersion *uint + commonVersion *Version reporterData Representation - reporterRepresentationVersion *uint + reporterRepresentationVersion *Version } // NewRepresentations creates a Representations with optional common and reporter data. @@ -20,9 +20,9 @@ type Representations struct { // If a representation is provided, its version must also be provided. func NewRepresentations( commonData Representation, - commonVersion *uint, + commonVersion *Version, reporterData Representation, - reporterRepresentationVersion *uint, + reporterRepresentationVersion *Version, ) (*Representations, error) { // Validate that at least one representation is present hasCommon := len(commonData) > 0 && commonVersion != nil @@ -56,7 +56,7 @@ func (r *Representations) CommonData() Representation { } // CommonVersion returns a pointer to the common version, or nil if not present. -func (r *Representations) CommonVersion() *uint { +func (r *Representations) CommonVersion() *Version { return r.commonVersion } diff --git a/internal/biz/model/resource_repository.go b/internal/biz/model/resource_repository.go index 24686449c..296db7744 100644 --- a/internal/biz/model/resource_repository.go +++ b/internal/biz/model/resource_repository.go @@ -9,7 +9,7 @@ type ResourceRepository interface { NextReporterResourceId() (ReporterResourceId, error) Save(tx *gorm.DB, resource Resource, operationType EventOperationType, txid string) error FindResourceByKeys(tx *gorm.DB, key ReporterResourceKey) (*Resource, error) - FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key ReporterResourceKey, currentVersion *uint, operationType EventOperationType) (*Representations, *Representations, error) + FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key ReporterResourceKey, currentVersion *Version, operationType EventOperationType) (*Representations, *Representations, error) FindLatestRepresentations(tx *gorm.DB, key ReporterResourceKey) (*Representations, error) GetDB() *gorm.DB GetTransactionManager() TransactionManager diff --git a/internal/biz/model/schema_service_test.go b/internal/biz/model/schema_service_test.go index 73fa57ac7..e12ad947b 100644 --- a/internal/biz/model/schema_service_test.go +++ b/internal/biz/model/schema_service_test.go @@ -85,19 +85,21 @@ func TestCalculateTuples(t *testing.T) { if tt.currentWorkspaceID != "" { currentData = map[string]interface{}{"workspace_id": tt.currentWorkspaceID} } + ver := model.NewVersion(tt.version) current, err = model.NewRepresentations( model.Representation(currentData), - &tt.version, + &ver, nil, nil, ) require.NoError(t, err) if tt.previousWorkspaceID != "" { - prevVer := uint(0) + prevUint := uint(0) if tt.version > 0 { - prevVer = tt.version - 1 + prevUint = tt.version - 1 } + prevVer := model.NewVersion(prevUint) previous, err = model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": tt.previousWorkspaceID}), &prevVer, @@ -158,18 +160,18 @@ func TestGetWorkspaceVersions(t *testing.T) { ) require.NoError(t, err) - version := uint(1) + ver := model.NewVersion(1) current, err := model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": "ws-current"}), - &version, + &ver, nil, nil, ) require.NoError(t, err) - prevVersion := version - 1 + prevVer := model.NewVersion(0) previous, err := model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": "ws-prev"}), - &prevVersion, + &prevVer, nil, nil, ) @@ -253,19 +255,19 @@ func TestDetermineTupleOperations(t *testing.T) { ) require.NoError(t, err) - version2 := uint(2) + ver2 := model.NewVersion(2) current, err := model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": "workspace-new"}), - &version2, + &ver2, nil, nil, ) require.NoError(t, err) - version1 := uint(1) + ver1 := model.NewVersion(1) previous, err := model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": "workspace-old"}), - &version1, + &ver1, nil, nil, ) @@ -343,9 +345,10 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { // Build current representation if tc.currentWorkspaceID != "" { currentData := map[string]interface{}{"workspace_id": tc.currentWorkspaceID} + curVer := model.NewVersion(tc.version) currentRep, err := model.NewRepresentations( model.Representation(currentData), - &tc.version, + &curVer, nil, nil, ) @@ -358,10 +361,11 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { // Build previous representation if tc.previousWorkspaceID != "" { - prevVer := uint(0) + prevUint := uint(0) if tc.version > 0 { - prevVer = tc.version - 1 + prevUint = tc.version - 1 } + prevVer := model.NewVersion(prevUint) previous, err = model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": tc.previousWorkspaceID}), &prevVer, diff --git a/internal/biz/usecase/resources/resource_service_test.go b/internal/biz/usecase/resources/resource_service_test.go index 59ed390d0..40784aead 100644 --- a/internal/biz/usecase/resources/resource_service_test.go +++ b/internal/biz/usecase/resources/resource_service_test.go @@ -1121,9 +1121,10 @@ func TestGetCurrentAndPreviousWorkspaceID(t *testing.T) { // Helper function to create a Representations for testing func createTestRep(t *testing.T, version uint, data map[string]interface{}) *model.Representations { + v := model.NewVersion(version) rep, err := model.NewRepresentations( model.Representation(data), - &version, + &v, nil, nil, ) diff --git a/internal/consumer/consumer.go b/internal/consumer/consumer.go index 38508b968..31dfd2956 100644 --- a/internal/consumer/consumer.go +++ b/internal/consumer/consumer.go @@ -297,7 +297,7 @@ func (i *InventoryConsumer) ProcessMessage(headers map[string]string, relationsE case string(model.OperationTypeCreated): if relationsEnabled { return i.processRelationsOperation(operation, txid, msg, operationConfig{ - fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *uint) (*model.Representations, *model.Representations, error) { + fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *model.Version) (*model.Representations, *model.Representations, error) { return i.ResourceRepository.FindCurrentAndPreviousVersionedRepresentations(nil, key, version, model.OperationTypeCreated) }, executeSpiceDB: func(i *InventoryConsumer, tuples model.TuplesToReplicate) (string, error) { @@ -310,7 +310,7 @@ func (i *InventoryConsumer) ProcessMessage(headers map[string]string, relationsE case string(model.OperationTypeUpdated): if relationsEnabled { return i.processRelationsOperation(operation, txid, msg, operationConfig{ - fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *uint) (*model.Representations, *model.Representations, error) { + fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *model.Version) (*model.Representations, *model.Representations, error) { return i.ResourceRepository.FindCurrentAndPreviousVersionedRepresentations(nil, key, version, model.OperationTypeUpdated) }, executeSpiceDB: func(i *InventoryConsumer, tuples model.TuplesToReplicate) (string, error) { @@ -322,7 +322,7 @@ func (i *InventoryConsumer) ProcessMessage(headers map[string]string, relationsE case string(model.OperationTypeDeleted): if relationsEnabled { return i.processRelationsOperation(operation, txid, msg, operationConfig{ - fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *uint) (*model.Representations, *model.Representations, error) { + fetchRepresentations: func(i *InventoryConsumer, key model.ReporterResourceKey, version *model.Version) (*model.Representations, *model.Representations, error) { previous, err := i.ResourceRepository.FindLatestRepresentations(nil, key) return nil, previous, err }, @@ -342,7 +342,7 @@ func (i *InventoryConsumer) ProcessMessage(headers map[string]string, relationsE } type operationConfig struct { - fetchRepresentations func(i *InventoryConsumer, key model.ReporterResourceKey, version *uint) (*model.Representations, *model.Representations, error) + fetchRepresentations func(i *InventoryConsumer, key model.ReporterResourceKey, version *model.Version) (*model.Representations, *model.Representations, error) executeSpiceDB func(i *InventoryConsumer, tuples model.TuplesToReplicate) (string, error) metricName string } @@ -365,13 +365,7 @@ func (i *InventoryConsumer) processRelationsOperation( key := tupleEvent.ReporterResourceKey() - var currentVersion *uint - if tupleEvent.CommonVersion() != nil { - version := tupleEvent.CommonVersion().Uint() - currentVersion = &version - } - - current, previous, err := config.fetchRepresentations(i, key, currentVersion) + current, previous, err := config.fetchRepresentations(i, key, tupleEvent.CommonVersion()) if err != nil { metricscollector.Incr(i.MetricsCollector.MsgProcessFailures, "FindRepresentations") i.Logger.Errorf("failed to find representations: %v", err) diff --git a/internal/data/fake_resource_repository.go b/internal/data/fake_resource_repository.go index ac7a5028b..24b1af747 100644 --- a/internal/data/fake_resource_repository.go +++ b/internal/data/fake_resource_repository.go @@ -253,7 +253,7 @@ func (f *fakeResourceRepository) FindResourceByKeys(tx *gorm.DB, key bizmodel.Re return nil, gorm.ErrRecordNotFound } -func (f *fakeResourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key bizmodel.ReporterResourceKey, currentVersion *uint, operationType bizmodel.EventOperationType) (*bizmodel.Representations, *bizmodel.Representations, error) { +func (f *fakeResourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key bizmodel.ReporterResourceKey, currentVersion *bizmodel.Version, operationType bizmodel.EventOperationType) (*bizmodel.Representations, *bizmodel.Representations, error) { if currentVersion == nil { return nil, nil, nil } @@ -273,14 +273,16 @@ func (f *fakeResourceRepository) FindCurrentAndPreviousVersionedRepresentations( return nil, nil, fmt.Errorf("no representations found for key") } + cv := currentVersion.Uint() var current *bizmodel.Representations var previous *bizmodel.Representations - if entry, ok := versionMap[*currentVersion]; ok { + if entry, ok := versionMap[cv]; ok { + v := bizmodel.NewVersion(entry.commonVersion) var err error current, err = bizmodel.NewRepresentations( bizmodel.Representation(cloneJsonObject(entry.commonData)), - uintPtr(entry.commonVersion), + &v, nil, nil, ) @@ -289,12 +291,13 @@ func (f *fakeResourceRepository) FindCurrentAndPreviousVersionedRepresentations( } } - if *currentVersion > 0 { - if entry, ok := versionMap[*currentVersion-1]; ok { + if cv > 0 { + if entry, ok := versionMap[cv-1]; ok { + v := bizmodel.NewVersion(entry.commonVersion) var err error previous, err = bizmodel.NewRepresentations( bizmodel.Representation(cloneJsonObject(entry.commonData)), - uintPtr(entry.commonVersion), + &v, nil, nil, ) @@ -332,9 +335,10 @@ func (f *fakeResourceRepository) FindLatestRepresentations(tx *gorm.DB, key bizm } } + v := bizmodel.NewVersion(latest.commonVersion) return bizmodel.NewRepresentations( bizmodel.Representation(cloneJsonObject(latest.commonData)), - uintPtr(latest.commonVersion), + &v, nil, nil, ) @@ -369,11 +373,6 @@ func (f *fakeResourceRepository) markTransactionIdAsProcessed(transactionId stri f.processedTransactionIds[transactionId] = true } -func uintPtr(v uint) *uint { - value := v - return &value -} - func cloneJsonObject(src internal.JsonObject) internal.JsonObject { if src == nil { return nil diff --git a/internal/data/resource_repository.go b/internal/data/resource_repository.go index 4b2066ad4..f8d0297f8 100644 --- a/internal/data/resource_repository.go +++ b/internal/data/resource_repository.go @@ -286,7 +286,7 @@ func (r *resourceRepository) GetTransactionManager() bizmodel.TransactionManager return r.transactionManager } -func (r *resourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key bizmodel.ReporterResourceKey, currentCommonVersion *uint, operationType bizmodel.EventOperationType) (*bizmodel.Representations, *bizmodel.Representations, error) { +func (r *resourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key bizmodel.ReporterResourceKey, currentCommonVersion *bizmodel.Version, operationType bizmodel.EventOperationType) (*bizmodel.Representations, *bizmodel.Representations, error) { if currentCommonVersion == nil { return nil, nil, nil } @@ -310,11 +310,11 @@ func (r *resourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx * query = r.buildReporterResourceKeyQuery(query, key) + cv := currentCommonVersion.Uint() if operationType.OperationType() == bizmodel.OperationTypeCreated { - query = query.Where("cr.version = ?", *currentCommonVersion) + query = query.Where("cr.version = ?", cv) } else { - query = query.Where("(cr.version = ? OR cr.version = ?)", *currentCommonVersion, *currentCommonVersion-1) - + query = query.Where("(cr.version = ? OR cr.version = ?)", cv, cv-1) } err := query.Find(&results).Error @@ -324,14 +324,15 @@ func (r *resourceRepository) FindCurrentAndPreviousVersionedRepresentations(tx * var current, previous *bizmodel.Representations for _, row := range results { - rep, err := bizmodel.NewRepresentations(bizmodel.Representation(row.Data), &row.Version, nil, nil) + v := bizmodel.NewVersion(row.Version) + rep, err := bizmodel.NewRepresentations(bizmodel.Representation(row.Data), &v, nil, nil) if err != nil { return nil, nil, fmt.Errorf("failed to create representation: %w", err) } - if row.Version == *currentCommonVersion { + if row.Version == cv { current = rep - } else if *currentCommonVersion > 0 && row.Version == *currentCommonVersion-1 { + } else if cv > 0 && row.Version == cv-1 { previous = rep } } @@ -358,10 +359,10 @@ func (r *resourceRepository) FindLatestRepresentations(tx *gorm.DB, key bizmodel return nil, fmt.Errorf("failed to find latest representations: %w", err) } - // Convert to Representations + v := bizmodel.NewVersion(result.Version) rep, err := bizmodel.NewRepresentations( bizmodel.Representation(result.Data), - &result.Version, + &v, nil, nil, ) diff --git a/internal/data/resource_repository_test.go b/internal/data/resource_repository_test.go index 14a747647..84062bac9 100644 --- a/internal/data/resource_repository_test.go +++ b/internal/data/resource_repository_test.go @@ -19,9 +19,9 @@ import ( "github.com/project-kessel/inventory-api/internal/testutil" ) -// Helper function to create a pointer to a uint -func ptrUint(v uint) *uint { - return &v +func ptrVersion(v uint) *bizmodel.Version { + ver := bizmodel.NewVersion(v) + return &ver } func TestResourceRepositoryContract(t *testing.T) { @@ -2145,7 +2145,7 @@ func TestFindLatestRepresentations(t *testing.T) { // Both implementations should return the latest version (version 2) assert.Equal(t, "workspace-v2-latest", result.CommonData()["workspace_id"]) - assert.Equal(t, uint(2), *result.CommonVersion()) + assert.Equal(t, bizmodel.NewVersion(2), *result.CommonVersion()) // Verify it contains the latest data assert.Equal(t, "production", result.CommonData()["environment"]) @@ -2217,7 +2217,7 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeUpdated, "tx-ws-update")) // Get current and previous versions - version := uint(1) + version := bizmodel.NewVersion(1) cur, prev, err := repo.FindCurrentAndPreviousVersionedRepresentations(db, key, &version, bizmodel.OperationTypeUpdated) require.NoError(t, err) @@ -2238,7 +2238,7 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { require.NoError(t, err) // Get version 0 representations - version := uint(0) + version := bizmodel.NewVersion(0) cur, prev, err := repo.FindCurrentAndPreviousVersionedRepresentations(db, key, &version, bizmodel.OperationTypeCreated) require.NoError(t, err) @@ -2258,13 +2258,13 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { // Test the function directly with invalid data current, _ := bizmodel.NewRepresentations( bizmodel.Representation(map[string]interface{}{"workspace_id": 123}), // non-string - ptrUint(1), + ptrVersion(1), nil, nil, ) previous, _ := bizmodel.NewRepresentations( bizmodel.Representation(map[string]interface{}{"other_field": "value"}), // missing workspace_id - ptrUint(0), + ptrVersion(0), nil, nil, ) From e20dddf8e3071f5ee496bd840f12387e63a725ce Mon Sep 17 00:00:00 2001 From: Sneha Gunta Date: Thu, 30 Apr 2026 16:36:13 -0400 Subject: [PATCH 2/4] Use TransactionId tiny type everywhere --- internal/biz/model/resource_repository.go | 4 +- internal/biz/model_legacy/outboxevents.go | 4 +- .../biz/model_legacy/outboxevents_test.go | 4 +- .../biz/usecase/resources/resource_service.go | 16 +- internal/consumer/consumer_test.go | 14 +- internal/data/fake_resource_repository.go | 11 +- internal/data/resource_repository.go | 13 +- internal/data/resource_repository_test.go | 230 +++++++++--------- .../resources/kesselinventoryservice_test.go | 4 +- 9 files changed, 148 insertions(+), 152 deletions(-) diff --git a/internal/biz/model/resource_repository.go b/internal/biz/model/resource_repository.go index 296db7744..5bcb3c01a 100644 --- a/internal/biz/model/resource_repository.go +++ b/internal/biz/model/resource_repository.go @@ -7,11 +7,11 @@ import ( type ResourceRepository interface { NextResourceId() (ResourceId, error) NextReporterResourceId() (ReporterResourceId, error) - Save(tx *gorm.DB, resource Resource, operationType EventOperationType, txid string) error + Save(tx *gorm.DB, resource Resource, operationType EventOperationType, txid TransactionId) error FindResourceByKeys(tx *gorm.DB, key ReporterResourceKey) (*Resource, error) FindCurrentAndPreviousVersionedRepresentations(tx *gorm.DB, key ReporterResourceKey, currentVersion *Version, operationType EventOperationType) (*Representations, *Representations, error) FindLatestRepresentations(tx *gorm.DB, key ReporterResourceKey) (*Representations, error) GetDB() *gorm.DB GetTransactionManager() TransactionManager - HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId string) (bool, error) + HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId TransactionId) (bool, error) } diff --git a/internal/biz/model_legacy/outboxevents.go b/internal/biz/model_legacy/outboxevents.go index 3ac282bf1..062cbf3a2 100644 --- a/internal/biz/model_legacy/outboxevents.go +++ b/internal/biz/model_legacy/outboxevents.go @@ -200,7 +200,7 @@ func convertResourceToTupleEvent(reporterResourceKey bizmodel.ReporterResourceKe return payload, nil } -func NewOutboxEventsFromResourceEvent(domainResourceEvent bizmodel.ResourceEvent, operationType bizmodel.EventOperationType, txid string) (*OutboxEvent, *OutboxEvent, error) { +func NewOutboxEventsFromResourceEvent(domainResourceEvent bizmodel.ResourceEvent, operationType bizmodel.EventOperationType, txid bizmodel.TransactionId) (*OutboxEvent, *OutboxEvent, error) { var payload internal.JsonObject var tuplePayload internal.JsonObject var err error @@ -238,7 +238,7 @@ func NewOutboxEventsFromResourceEvent(domainResourceEvent bizmodel.ResourceEvent Operation: operationType, AggregateType: TupleAggregateType, AggregateID: domainResourceEvent.Id().String(), - TxId: txid, + TxId: txid.String(), Payload: tuplePayload, } diff --git a/internal/biz/model_legacy/outboxevents_test.go b/internal/biz/model_legacy/outboxevents_test.go index 7142fb38e..3c3ab91a3 100644 --- a/internal/biz/model_legacy/outboxevents_test.go +++ b/internal/biz/model_legacy/outboxevents_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" ) -const txid = "txid" +var txid = bizmodel.NewTransactionId("txid") func createTestResourceReportEvent() bizmodel.ResourceReportEvent { resourceIdUUID := uuid.MustParse("550e8400-e29b-41d4-a716-446655440001") @@ -101,7 +101,7 @@ func TestNewOutboxEventsFromResourceEventDeleted(t *testing.T) { func assertTupleEventFromDomainEvent(t *testing.T, resourceEvent bizmodel.ResourceReportEvent, event *OutboxEvent) { assert.NotNil(t, event) - assert.Equal(t, txid, event.TxId) + assert.Equal(t, txid.String(), event.TxId) assert.Equal(t, TupleAggregateType, event.AggregateType) assert.Equal(t, resourceEvent.Id().String(), event.AggregateID) diff --git a/internal/biz/usecase/resources/resource_service.go b/internal/biz/usecase/resources/resource_service.go index 1f44ffe14..28f7e9708 100644 --- a/internal/biz/usecase/resources/resource_service.go +++ b/internal/biz/usecase/resources/resource_service.go @@ -150,7 +150,7 @@ func (uc *Usecase) ReportResource(ctx context.Context, cmd ReportResourceCommand // provides the actual correctness guarantee, if a duplicate sneaks past this // advisory check due to a concurrent commit. if uc.Config.IdempotencyCheckEnabled && cmd.TransactionId != nil { - alreadyProcessed, err := uc.resourceRepository.HasTransactionIdBeenProcessed(uc.resourceRepository.GetDB(), cmd.TransactionId.String()) + alreadyProcessed, err := uc.resourceRepository.HasTransactionIdBeenProcessed(uc.resourceRepository.GetDB(), *cmd.TransactionId) if err != nil { return fmt.Errorf("failed to check transaction ID: %w", err) } @@ -174,12 +174,12 @@ func (uc *Usecase) ReportResource(ctx context.Context, cmd ReportResourceCommand if err == nil && res != nil { log.Info("Resource already exists, updating: ") operationType = model.OperationTypeUpdated - return uc.updateResource(tx, cmd, res, txid.String()) + return uc.updateResource(tx, cmd, res, txid) } log.Info("Creating new resource") operationType = model.OperationTypeCreated - return uc.createResource(tx, cmd, txid.String()) + return uc.createResource(tx, cmd, txid) }, ) } @@ -237,7 +237,7 @@ func (uc *Usecase) ReportResource(ctx context.Context, cmd ReportResourceCommand return nil } -func (uc *Usecase) createResource(tx *gorm.DB, cmd ReportResourceCommand, txidStr string) error { +func (uc *Usecase) createResource(tx *gorm.DB, cmd ReportResourceCommand, txid model.TransactionId) error { resourceId, err := uc.resourceRepository.NextResourceId() if err != nil { return err @@ -266,10 +266,10 @@ func (uc *Usecase) createResource(tx *gorm.DB, cmd ReportResourceCommand, txidSt return err } - return uc.resourceRepository.Save(tx, resource, model.OperationTypeCreated, txidStr) + return uc.resourceRepository.Save(tx, resource, model.OperationTypeCreated, txid) } -func (uc *Usecase) updateResource(tx *gorm.DB, cmd ReportResourceCommand, existingResource *model.Resource, txidStr string) error { +func (uc *Usecase) updateResource(tx *gorm.DB, cmd ReportResourceCommand, existingResource *model.Resource, txid model.TransactionId) error { reporterResourceKey, err := model.NewReporterResourceKey( cmd.LocalResourceId, cmd.ResourceType, @@ -293,7 +293,7 @@ func (uc *Usecase) updateResource(tx *gorm.DB, cmd ReportResourceCommand, existi return fmt.Errorf("failed to update resource: %w", err) } - return uc.resourceRepository.Save(tx, *existingResource, model.OperationTypeUpdated, txidStr) + return uc.resourceRepository.Save(tx, *existingResource, model.OperationTypeUpdated, txid) } func (uc *Usecase) Delete(ctx context.Context, reporterResourceKey model.ReporterResourceKey) error { @@ -322,7 +322,7 @@ func (uc *Usecase) Delete(ctx context.Context, reporterResourceKey model.Reporte if err != nil { return fmt.Errorf("failed to delete resource: %w", err) } - return uc.resourceRepository.Save(tx, *res, model.OperationTypeDeleted, txid.String()) + return uc.resourceRepository.Save(tx, *res, model.OperationTypeDeleted, txid) } else { if errors.Is(err, gorm.ErrRecordNotFound) { return ErrResourceNotFound diff --git a/internal/consumer/consumer_test.go b/internal/consumer/consumer_test.go index d55a6a5a9..7c46a54e8 100644 --- a/internal/consumer/consumer_test.go +++ b/internal/consumer/consumer_test.go @@ -330,7 +330,7 @@ func TestInventoryConsumer_ProcessMessage(t *testing.T) { setupData: func(t *testing.T, repo model.ResourceRepository, db *gorm.DB) { testData, err := model.NewResourceFixture("test-resource-4321", "integration", "notifications", "test-instance-1", "test-workspace-v0") require.NoError(t, err) - err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, string(testData.InitialTransactionId)) + err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, testData.InitialTransactionId) require.NoError(t, err) }, }, @@ -346,7 +346,7 @@ func TestInventoryConsumer_ProcessMessage(t *testing.T) { setupData: func(t *testing.T, repo model.ResourceRepository, db *gorm.DB) { testData, err := model.NewResourceFixture("test-resource-4321", "integration", "notifications", "test-instance-1", "test-workspace-v0") require.NoError(t, err) - err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, string(testData.InitialTransactionId)) + err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, testData.InitialTransactionId) require.NoError(t, err) updatedCommon, err := model.NewRepresentation(map[string]interface{}{"workspace_id": "test-workspace-v1"}) @@ -354,7 +354,7 @@ func TestInventoryConsumer_ProcessMessage(t *testing.T) { txId := model.TransactionId("tx-v1") err = testData.Resource.Update(testData.Key, testData.ApiHref, &testData.ConsoleHref, nil, &testData.ReporterRepresentation, &updatedCommon, txId) require.NoError(t, err) - err = repo.Save(db, *testData.Resource, model.OperationTypeUpdated, "tx-v1") + err = repo.Save(db, *testData.Resource, model.OperationTypeUpdated, model.NewTransactionId("tx-v1")) require.NoError(t, err) }, }, @@ -370,7 +370,7 @@ func TestInventoryConsumer_ProcessMessage(t *testing.T) { setupData: func(t *testing.T, repo model.ResourceRepository, db *gorm.DB) { testData, err := model.NewResourceFixture("test-resource-4321", "integration", "notifications", "test-instance-1", "test-workspace-v0") require.NoError(t, err) - err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, string(testData.InitialTransactionId)) + err = repo.Save(db, *testData.Resource, model.OperationTypeCreated, testData.InitialTransactionId) require.NoError(t, err) updatedCommon, err := model.NewRepresentation(map[string]interface{}{"workspace_id": "test-workspace-v1"}) @@ -378,7 +378,7 @@ func TestInventoryConsumer_ProcessMessage(t *testing.T) { txId := model.TransactionId("tx-v1") err = testData.Resource.Update(testData.Key, testData.ApiHref, &testData.ConsoleHref, nil, &testData.ReporterRepresentation, &updatedCommon, txId) require.NoError(t, err) - err = repo.Save(db, *testData.Resource, model.OperationTypeUpdated, "tx-v1") + err = repo.Save(db, *testData.Resource, model.OperationTypeUpdated, model.NewTransactionId("tx-v1")) require.NoError(t, err) }, }, @@ -941,7 +941,7 @@ func TestInventoryConsumer_UpdateWithSameWorkspace_NoOp(t *testing.T) { require.NoError(t, err) fakeRepo := data.NewFakeResourceRepository() - require.NoError(t, fakeRepo.Save(nil, *testData.Resource, model.OperationTypeCreated, string(testData.InitialTransactionId))) + require.NoError(t, fakeRepo.Save(nil, *testData.Resource, model.OperationTypeCreated, testData.InitialTransactionId)) txId := model.TransactionId("tx-update") err = testData.Resource.Update( @@ -955,7 +955,7 @@ func TestInventoryConsumer_UpdateWithSameWorkspace_NoOp(t *testing.T) { ) require.NoError(t, err) - require.NoError(t, fakeRepo.Save(nil, *testData.Resource, model.OperationTypeUpdated, "tx-update")) + require.NoError(t, fakeRepo.Save(nil, *testData.Resource, model.OperationTypeUpdated, model.NewTransactionId("tx-update"))) tester.inv.ResourceRepository = fakeRepo diff --git a/internal/data/fake_resource_repository.go b/internal/data/fake_resource_repository.go index 24b1af747..f263f4978 100644 --- a/internal/data/fake_resource_repository.go +++ b/internal/data/fake_resource_repository.go @@ -74,7 +74,7 @@ func (f *fakeResourceRepository) NextReporterResourceId() (bizmodel.ReporterReso return bizmodel.NewReporterResourceId(uuidV7) } -func (f *fakeResourceRepository) Save(tx *gorm.DB, resource bizmodel.Resource, operationType bizmodel.EventOperationType, txid string) error { +func (f *fakeResourceRepository) Save(tx *gorm.DB, resource bizmodel.Resource, operationType bizmodel.EventOperationType, txid bizmodel.TransactionId) error { f.mu.Lock() defer f.mu.Unlock() @@ -394,15 +394,10 @@ func cloneJsonObject(src internal.JsonObject) internal.JsonObject { // HasTransactionIdBeenProcessed checks if a transaction ID has been processed before // Returns true if the transaction has already been processed, false otherwise -func (f *fakeResourceRepository) HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId string) (bool, error) { - if transactionId == "" { - return false, nil - } - +func (f *fakeResourceRepository) HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId bizmodel.TransactionId) (bool, error) { f.mu.RLock() defer f.mu.RUnlock() - // Check if this transaction ID has been processed before - _, exists := f.processedTransactionIds[transactionId] + _, exists := f.processedTransactionIds[transactionId.String()] return exists, nil } diff --git a/internal/data/resource_repository.go b/internal/data/resource_repository.go index f8d0297f8..7e4af8a65 100644 --- a/internal/data/resource_repository.go +++ b/internal/data/resource_repository.go @@ -130,7 +130,7 @@ func (r *resourceRepository) NextReporterResourceId() (bizmodel.ReporterResource return bizmodel.NewReporterResourceId(uuidV7) } -func (r *resourceRepository) Save(tx *gorm.DB, resource bizmodel.Resource, operationType bizmodel.EventOperationType, txid string) error { +func (r *resourceRepository) Save(tx *gorm.DB, resource bizmodel.Resource, operationType bizmodel.EventOperationType, txid bizmodel.TransactionId) error { resourceSnapshot, reporterResourceSnapshot, reporterRepresentationSnapshot, commonRepresentationSnapshot, err := resource.Serialize() if err != nil { return fmt.Errorf("failed to serialize resource: %w", err) @@ -187,7 +187,7 @@ func (r *resourceRepository) Save(tx *gorm.DB, resource bizmodel.Resource, opera return nil } -func (r *resourceRepository) handleOutboxEvents(tx *gorm.DB, resourceEvent bizmodel.ResourceEvent, operationType bizmodel.EventOperationType, txid string) error { +func (r *resourceRepository) handleOutboxEvents(tx *gorm.DB, resourceEvent bizmodel.ResourceEvent, operationType bizmodel.EventOperationType, txid bizmodel.TransactionId) error { resourceMessage, tupleMessage, err := model_legacy.NewOutboxEventsFromResourceEvent(resourceEvent, operationType, txid) if err != nil { return err @@ -375,11 +375,8 @@ func (r *resourceRepository) FindLatestRepresentations(tx *gorm.DB, key bizmodel // HasTransactionIdBeenProcessed checks if a transaction ID exists in either the // reporter_representations or common_representations tables. // Returns true if the transaction has already been processed, false otherwise. -func (r *resourceRepository) HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId string) (bool, error) { - if transactionId == "" { - return false, nil - } - // Check representations tables using lightweight EXISTS query +func (r *resourceRepository) HasTransactionIdBeenProcessed(tx *gorm.DB, transactionId bizmodel.TransactionId) (bool, error) { + tid := transactionId.String() var exists bool err := tx.Raw(` SELECT EXISTS ( @@ -388,7 +385,7 @@ func (r *resourceRepository) HasTransactionIdBeenProcessed(tx *gorm.DB, transact OR EXISTS ( SELECT 1 FROM common_representations WHERE transaction_id = ? ) - `, transactionId, transactionId).Scan(&exists).Error + `, tid, tid).Scan(&exists).Error if err != nil { return false, fmt.Errorf("failed to check representations for the transaction_id: %w", err) diff --git a/internal/data/resource_repository_test.go b/internal/data/resource_repository_test.go index 84062bac9..ae5171015 100644 --- a/internal/data/resource_repository_test.go +++ b/internal/data/resource_repository_test.go @@ -24,6 +24,10 @@ func ptrVersion(v uint) *bizmodel.Version { return &ver } +func txid(id string) bizmodel.TransactionId { + return bizmodel.NewTransactionId(id) +} + func TestResourceRepositoryContract(t *testing.T) { implementations := []struct { name string @@ -85,7 +89,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("Save and FindResourceByKeys basic workflow", func(t *testing.T) { resource := createTestResourceWithLocalId(t, "contract-test-1") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-tx-1") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-tx-1")) require.NoError(t, err, "Save should succeed") key := createContractReporterResourceKey(t, "contract-test-1", "k8s_cluster", "ocm", "ocm-instance-1") @@ -107,7 +111,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("Save-Update-Save workflow", func(t *testing.T) { // Create initial resource resource := createTestResourceWithLocalId(t, "contract-update-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-tx-create") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-tx-create")) require.NoError(t, err, "Initial save should succeed") // Find and update @@ -128,7 +132,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * require.NoError(t, err, "Update should succeed") // Save updated resource - err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, "contract-tx-update") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, txid("contract-tx-update")) require.NoError(t, err, "Updated save should succeed") // Verify update persisted @@ -140,7 +144,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("Save-Delete workflow", func(t *testing.T) { // Create resource resource := createTestResourceWithLocalId(t, "contract-delete-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-tx-create") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-tx-create")) require.NoError(t, err, "Initial save should succeed") // Find and delete @@ -155,7 +159,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * require.NoError(t, err, "Delete should succeed") // Save deleted resource - err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, "contract-tx-delete") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, txid("contract-tx-delete")) require.NoError(t, err, "Delete save should succeed") // Verify deletion behavior is consistent @@ -171,12 +175,12 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("Unique constraint enforcement", func(t *testing.T) { // Create first resource resource1 := createTestResourceWithLocalId(t, "contract-unique-test") - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "contract-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("contract-tx-1")) require.NoError(t, err, "First save should succeed") // Try to create second resource with same composite key resource2 := createTestResourceWithLocalId(t, "contract-unique-test") - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "contract-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("contract-tx-2")) require.Error(t, err, "Second save with duplicate key should fail") // Error should indicate constraint violation @@ -188,7 +192,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("Case insensitive key matching for non ID fields", func(t *testing.T) { // Create resource with mixed case resource := createTestResourceWithReporter(t, "Contract-Case-Test", "OCM", "Instance-1") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-case-tx") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-case-tx")) require.NoError(t, err, "Save should succeed") // Find with different casing @@ -204,7 +208,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * if db == nil { // Fake repository test resource := createTestResourceWithLocalId(t, "contract-nil-tx-test") - err := repo.Save(nil, resource, bizmodel.OperationTypeCreated, "contract-nil-tx") + err := repo.Save(nil, resource, bizmodel.OperationTypeCreated, txid("contract-nil-tx")) require.NoError(t, err, "Save with nil transaction should succeed in fake repo") key := createContractReporterResourceKey(t, "contract-nil-tx-test", "k8s_cluster", "ocm", "ocm-instance-1") @@ -215,7 +219,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * } else { // Real repository test - use actual db transaction resource := createTestResourceWithLocalId(t, "contract-real-tx-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-real-tx") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-real-tx")) require.NoError(t, err, "Save with db transaction should succeed in real repo") key := createContractReporterResourceKey(t, "contract-real-tx-test", "k8s_cluster", "ocm", "ocm-instance-1") @@ -231,7 +235,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * // 1. Create resource := createTestResourceWithLocalId(t, localResourceId) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "contract-create") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("contract-create")) require.NoError(t, err, "Create should succeed") key := createContractReporterResourceKey(t, localResourceId, "k8s_cluster", "ocm", "ocm-instance-1") @@ -249,7 +253,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * err = foundResource.Update(key, apiHref, &consoleHref, nil, &reporterData, &commonData, transactionId) require.NoError(t, err, "Update should succeed") - err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, "contract-update") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, txid("contract-update")) require.NoError(t, err, "Update save should succeed") // 3. Delete @@ -259,7 +263,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * err = deletedResource.Delete(key) require.NoError(t, err, "Delete should succeed") - err = repo.Save(db, *deletedResource, bizmodel.OperationTypeDeleted, "contract-delete") + err = repo.Save(db, *deletedResource, bizmodel.OperationTypeDeleted, txid("contract-delete")) require.NoError(t, err, "Delete save should succeed") // 4. Verify delete behavior @@ -274,7 +278,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * // 5. Recreate (this should work the same way in both implementations) newResource := createTestResourceWithLocalId(t, localResourceId) - err = repo.Save(db, newResource, bizmodel.OperationTypeCreated, "contract-recreate") + err = repo.Save(db, newResource, bizmodel.OperationTypeCreated, txid("contract-recreate")) // The behavior should be identical between implementations recreateResource, findErr := repo.FindResourceByKeys(db, key) @@ -329,7 +333,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("CommonVersion lifecycle consistency", func(t *testing.T) { t.Run("create-with update-with update-without: increments then resets to nil", func(t *testing.T) { resource, key := makeResource(t, "cv-contract-inc-reset", "cv-inc-reset", true) - require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, txid(""))) found, err := repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -338,7 +342,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * assert.Equal(t, uint(0), *snap.CommonVersion, "should be 0 after create-with") doUpdate(t, found, key, "cv-inc-reset-upd1", true) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -347,7 +351,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * assert.Equal(t, uint(1), *snap.CommonVersion, "should be 1 after update-with") doUpdate(t, found, key, "cv-inc-reset-upd2", false) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -358,12 +362,12 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("create-with update-without update-with: drop then re-add resumes from last version", func(t *testing.T) { resource, key := makeResource(t, "cv-contract-drop-readd", "cv-drop-readd", true) - require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, txid(""))) found, err := repo.FindResourceByKeys(db, key) require.NoError(t, err) doUpdate(t, found, key, "cv-drop-readd-upd1", false) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -372,7 +376,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * assert.Nil(t, snap.CommonVersion, "should be nil after update-without") doUpdate(t, found, key, "cv-drop-readd-upd2", true) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -384,7 +388,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("create-without update-with: first-time add via update initializes to 0", func(t *testing.T) { resource, key := makeResource(t, "cv-contract-first-add", "cv-first-add", false) - require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, txid(""))) found, err := repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -393,7 +397,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * assert.Nil(t, snap.CommonVersion, "should be nil after create-without") doUpdate(t, found, key, "cv-first-add-upd1", true) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -405,20 +409,20 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * t.Run("create-with delete create-with update-with: second lifecycle starts fresh", func(t *testing.T) { resource1, key := makeResource(t, "cv-contract-del-recreate", "cv-del-rec-1", true) - require.NoError(t, repo.Save(db, resource1, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid(""))) found, err := repo.FindResourceByKeys(db, key) require.NoError(t, err) doUpdate(t, found, key, "cv-del-rec-upd1", true) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) require.NoError(t, found.Delete(key)) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeDeleted, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeDeleted, txid(""))) resource2, _ := makeResource(t, "cv-contract-del-recreate", "cv-del-rec-2", true) - require.NoError(t, repo.Save(db, resource2, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -427,7 +431,7 @@ func testRepositoryContract(t *testing.T, repo bizmodel.ResourceRepository, db * assert.Equal(t, uint(0), *snap.CommonVersion, "second lifecycle should start at 0") doUpdate(t, found, key, "cv-del-rec-upd2", true) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -486,7 +490,7 @@ func TestFindResourceByKeys(t *testing.T) { repo, db := getFreshInstances() resource := createTestResource(t) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-123") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-123")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey( @@ -525,9 +529,9 @@ func TestFindResourceByKeys(t *testing.T) { repo, db := getFreshInstances() - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err) - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "test-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("test-tx-2")) require.NoError(t, err) key1, err := bizmodel.NewReporterResourceKey("resource-1", "k8s_cluster", "ocm", "ocm-instance-1") @@ -555,7 +559,7 @@ func TestFindResourceByKeys(t *testing.T) { repo, db := getFreshInstances() resource := createTestResource(t) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey( @@ -592,7 +596,7 @@ func TestFindResourceByKeys(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithLocalId(t, "test-resource-no-instance-lookup") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-no-instance") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-no-instance")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey( @@ -615,7 +619,7 @@ func TestFindResourceByKeys(t *testing.T) { // Create a resource with mixed case values resource := createTestResourceWithMixedCase(t) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-case") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-case")) require.NoError(t, err) testCases := []struct { @@ -746,7 +750,7 @@ func TestFindResourceByKeys_TombstoneFilter(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithLocalId(t, "tombstoned-resource") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-tombstone") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-tombstone")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey( @@ -764,7 +768,7 @@ func TestFindResourceByKeys_TombstoneFilter(t *testing.T) { err = foundResource.Delete(key) require.NoError(t, err) - err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, "test-tx-delete") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, txid("test-tx-delete")) require.NoError(t, err) // With tombstone filter removed, we should be able to find the tombstoned resource @@ -829,13 +833,13 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { // Create first resource resource1 := createTestResourceWithLocalId(t, "duplicate-key-test") - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err, "First save should succeed") // Create second resource with same composite key components // (same LocalResourceID, ReporterType, ResourceType, ReporterInstanceID, RepresentationVersion=0, Generation=0) resource2 := createTestResourceWithLocalId(t, "duplicate-key-test") // Same local ID - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "test-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("test-tx-2")) // Both implementations should reject this duplicate require.Error(t, err, "Second save with duplicate composite key should fail") @@ -852,7 +856,7 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { // Create and save initial resource resource := createTestResourceWithLocalId(t, "version-test-resource") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-create") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-create")) require.NoError(t, err, "Initial save should succeed") // Update the resource (this increments representation version and potentially generation) @@ -874,7 +878,7 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { require.NoError(t, err, "Update should succeed") // Save the updated resource (different version/generation should be allowed) - err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, "test-tx-update") + err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, txid("test-tx-update")) require.NoError(t, err, "Save with different version should succeed") }) @@ -883,12 +887,12 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { // Create first resource with k8s_cluster type resource1 := createTestResourceWithLocalIdAndType(t, "multi-type-test", "k8s_cluster") - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err, "First save should succeed") // Create second resource with same local ID but different resource type resource2 := createTestResourceWithLocalIdAndType(t, "multi-type-test", "host") - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "test-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("test-tx-2")) require.NoError(t, err, "Save with different resource type should succeed") }) @@ -897,12 +901,12 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { // Create resource with OCM reporter resource1 := createTestResourceWithReporter(t, "reporter-test", "ocm", "ocm-instance-1") - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err, "First save should succeed") // Create resource with same local ID but different reporter type resource2 := createTestResourceWithReporter(t, "reporter-test", "hbi", "hbi-instance-1") - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "test-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("test-tx-2")) require.NoError(t, err, "Save with different reporter type should succeed") }) @@ -911,12 +915,12 @@ func TestUniqueConstraint_ReporterResourceCompositeKey(t *testing.T) { // Create resource with instance-1 resource1 := createTestResourceWithReporter(t, "instance-test", "ocm", "ocm-instance-1") - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err, "First save should succeed") // Create resource with same components but different reporter instance resource2 := createTestResourceWithReporter(t, "instance-test", "ocm", "ocm-instance-2") - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "test-tx-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("test-tx-2")) require.NoError(t, err, "Save with different reporter instance should succeed") }) }) @@ -972,7 +976,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { // 1. REPORT: Create initial resource resource := createTestResourceWithLocalId(t, localResourceId) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "repo-create-1") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("repo-create-1")) require.NoError(t, err, "Initial save should succeed") key := createContractReporterResourceKey(t, localResourceId, "k8s_cluster", "ocm", "ocm-instance-1") @@ -994,7 +998,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource.Delete(key) require.NoError(t, err, "Delete operation should succeed") - err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, "repo-delete-1") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeDeleted, txid("repo-delete-1")) require.NoError(t, err, "Delete save should succeed") // Verify delete state @@ -1024,7 +1028,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource2.Delete(key) require.NoError(t, err, "Duplicate delete operation should succeed") - err = repo.Save(db, *foundResource2, bizmodel.OperationTypeDeleted, "repo-delete-2") + err = repo.Save(db, *foundResource2, bizmodel.OperationTypeDeleted, txid("repo-delete-2")) require.NoError(t, err, "Duplicate delete save should succeed") // Verify state after duplicate delete @@ -1047,7 +1051,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { // 1. REPORT: Create initial resource resource1 := createTestResourceWithLocalId(t, localResourceId) - err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, "repo-create-1") + err := repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("repo-create-1")) require.NoError(t, err, "Initial save should succeed") key := createContractReporterResourceKey(t, localResourceId, "k8s_cluster", "ocm", "ocm-instance-1") @@ -1066,7 +1070,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource1.Update(key, apiHref, &consoleHref, nil, &reporterData, &commonData, transactionId) require.NoError(t, err, "Update should succeed") - err = repo.Save(db, *foundResource1, bizmodel.OperationTypeUpdated, "repo-update-1") + err = repo.Save(db, *foundResource1, bizmodel.OperationTypeUpdated, txid("repo-update-1")) require.NoError(t, err, "Duplicate report save should succeed") // 3. DELETE: Delete the resource @@ -1077,7 +1081,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource2.Delete(key) require.NoError(t, err, "Delete operation should succeed") - err = repo.Save(db, *foundResource2, bizmodel.OperationTypeDeleted, "repo-delete-1") + err = repo.Save(db, *foundResource2, bizmodel.OperationTypeDeleted, txid("repo-delete-1")) require.NoError(t, err, "Delete save should succeed") // 4. RESUBMIT SAME DELETE: Should succeed @@ -1092,7 +1096,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource3.Delete(key) require.NoError(t, err, "Duplicate delete operation should succeed") - err = repo.Save(db, *foundResource3, bizmodel.OperationTypeDeleted, "repo-delete-2") + err = repo.Save(db, *foundResource3, bizmodel.OperationTypeDeleted, txid("repo-delete-2")) require.NoError(t, err, "Duplicate delete save should succeed") } }) @@ -1114,7 +1118,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { // Resource doesn't exist - create new one t.Logf("Cycle %d: Creating new resource", cycle) resource := createTestResourceWithLocalId(t, localResourceId) - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, fmt.Sprintf("repo-cycle-%d-create", cycle)) + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid(fmt.Sprintf("repo-cycle-%d-create", cycle))) require.NoError(t, err, "Save should succeed in cycle %d", cycle) } else { // Resource exists (potentially tombstoned) - update it @@ -1131,7 +1135,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = foundResource.Update(key, apiHref, &consoleHref, nil, &reporterData, &commonData, transactionId) require.NoError(t, err, "Update should succeed in cycle %d", cycle) - err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, fmt.Sprintf("repo-cycle-%d-update", cycle)) + err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, txid(fmt.Sprintf("repo-cycle-%d-update", cycle))) require.NoError(t, err, "Update save should succeed in cycle %d", cycle) } @@ -1147,7 +1151,7 @@ func TestResourceRepository_IdempotentOperations(t *testing.T) { err = currentResource.Delete(key) require.NoError(t, err, "Delete should succeed in cycle %d", cycle) - err = repo.Save(db, *currentResource, bizmodel.OperationTypeDeleted, fmt.Sprintf("repo-cycle-%d-delete", cycle)) + err = repo.Save(db, *currentResource, bizmodel.OperationTypeDeleted, txid(fmt.Sprintf("repo-cycle-%d-delete", cycle))) require.NoError(t, err, "Delete save should succeed in cycle %d", cycle) // Verify state after delete @@ -1215,7 +1219,7 @@ func TestSave(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithLocalId(t, "update-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-1") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-1")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey("update-test", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1244,7 +1248,7 @@ func TestSave(t *testing.T) { err = resource.Update(key, apiHref, &consoleHref, nil, &updatedReporterData, &updatedCommonData, updatedTransactionId) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, "test-tx-2") + err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, txid("test-tx-2")) require.NoError(t, err) foundResource, err := repo.FindResourceByKeys(db, key) @@ -1258,7 +1262,7 @@ func TestSave(t *testing.T) { resource := createTestResourceWithLocalId(t, "save-new-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-save") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-save")) require.NoError(t, err) key, err := bizmodel.NewReporterResourceKey("save-new-test", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1278,7 +1282,7 @@ func TestSave(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithLocalId(t, "zero-pk-test") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "test-tx-zero-pk") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("test-tx-zero-pk")) require.NoError(t, err, "Save should succeed and skip representations with zero value primary keys") key, err := bizmodel.NewReporterResourceKey("zero-pk-test", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1340,10 +1344,10 @@ func TestResourceRepository_MultipleHostsLifecycle(t *testing.T) { host1 := createTestResourceWithLocalIdAndType(t, "host-1", "host") host2 := createTestResourceWithLocalIdAndType(t, "host-2", "host") - err := repo.Save(db, host1, bizmodel.OperationTypeCreated, "tx-create-host1") + err := repo.Save(db, host1, bizmodel.OperationTypeCreated, txid("tx-create-host1")) require.NoError(t, err, "Should create host1") - err = repo.Save(db, host2, bizmodel.OperationTypeCreated, "tx-create-host2") + err = repo.Save(db, host2, bizmodel.OperationTypeCreated, txid("tx-create-host2")) require.NoError(t, err, "Should create host2") // Verify both hosts can be found @@ -1385,10 +1389,10 @@ func TestResourceRepository_MultipleHostsLifecycle(t *testing.T) { err = foundHost2.Update(key2, apiHref, &consoleHref, nil, &updatedReporterData, &updatedCommonData, updatedTransactionId2) require.NoError(t, err, "Should update host2") - err = repo.Save(db, *foundHost1, bizmodel.OperationTypeUpdated, "tx-update-host1") + err = repo.Save(db, *foundHost1, bizmodel.OperationTypeUpdated, txid("tx-update-host1")) require.NoError(t, err, "Should save updated host1") - err = repo.Save(db, *foundHost2, bizmodel.OperationTypeUpdated, "tx-update-host2") + err = repo.Save(db, *foundHost2, bizmodel.OperationTypeUpdated, txid("tx-update-host2")) require.NoError(t, err, "Should save updated host2") // Verify both updated hosts can still be found @@ -1407,10 +1411,10 @@ func TestResourceRepository_MultipleHostsLifecycle(t *testing.T) { err = updatedHost2.Delete(key2) require.NoError(t, err, "Should delete host2") - err = repo.Save(db, *updatedHost1, bizmodel.OperationTypeDeleted, "tx-delete-host1") + err = repo.Save(db, *updatedHost1, bizmodel.OperationTypeDeleted, txid("tx-delete-host1")) require.NoError(t, err, "Should save deleted host1") - err = repo.Save(db, *updatedHost2, bizmodel.OperationTypeDeleted, "tx-delete-host2") + err = repo.Save(db, *updatedHost2, bizmodel.OperationTypeDeleted, txid("tx-delete-host2")) require.NoError(t, err, "Should save deleted host2") // Verify both hosts can be found (tombstoned) with tombstone filter removed @@ -1473,7 +1477,7 @@ func TestResourceRepository_PartialDataScenarios(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithReporterDataOnly(t, "reporter-only-resource") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-reporter-only") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-reporter-only")) require.NoError(t, err, "Should save resource with only reporter data") key, err := bizmodel.NewReporterResourceKey("reporter-only-resource", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1488,7 +1492,7 @@ func TestResourceRepository_PartialDataScenarios(t *testing.T) { repo, db := getFreshInstances() resource := createTestResourceWithCommonDataOnly(t, "common-only-resource") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-common-only") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-common-only")) require.NoError(t, err, "Should save resource with only common data") key, err := bizmodel.NewReporterResourceKey("common-only-resource", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1504,7 +1508,7 @@ func TestResourceRepository_PartialDataScenarios(t *testing.T) { // 1. Report with both reporter and common data resourceBoth := createTestResourceWithLocalId(t, "progressive-resource") - err := repo.Save(db, resourceBoth, bizmodel.OperationTypeCreated, "tx-both") + err := repo.Save(db, resourceBoth, bizmodel.OperationTypeCreated, txid("tx-both")) require.NoError(t, err, "Should save resource with both data types") key, err := bizmodel.NewReporterResourceKey("progressive-resource", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1534,7 +1538,7 @@ func TestResourceRepository_PartialDataScenarios(t *testing.T) { err = foundResource.Update(key, apiHref, &consoleHref, nil, &reporterOnlyData, &emptyCommonData, updatedTransactionId1) require.NoError(t, err, "Should update with reporter data only") - err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, "tx-reporter-update") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, txid("tx-reporter-update")) require.NoError(t, err, "Should save resource with reporter-only update") // 3. Update with just common data @@ -1556,7 +1560,7 @@ func TestResourceRepository_PartialDataScenarios(t *testing.T) { err = foundResource.Update(key, apiHref, &consoleHref, nil, &emptyReporterData, &commonOnlyData, updatedTransactionId2) require.NoError(t, err, "Should update with common data only") - err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, "tx-common-update") + err = repo.Save(db, *foundResource, bizmodel.OperationTypeUpdated, txid("tx-common-update")) require.NoError(t, err, "Should save resource with common-only update") // Verify final resource can still be found @@ -1604,7 +1608,7 @@ func TestSerializableCreateFails(t *testing.T) { foundResource, err := repo.FindResourceByKeys(conflictTx, resource.ReporterResources()[0].ReporterResourceKey) assert.NotNil(t, err) assert.Nil(t, foundResource) - assert.NoError(t, repo.Save(conflictTx, resource, bizmodel.OperationTypeCreated, "tx-conflict")) + assert.NoError(t, repo.Save(conflictTx, resource, bizmodel.OperationTypeCreated, txid("tx-conflict"))) // Do NOT commit yet to hold locks // Attempt to create the same resource via a separate serializable transaction managed by TM @@ -1612,7 +1616,7 @@ func TestSerializableCreateFails(t *testing.T) { foundResource, err := repo.FindResourceByKeys(tx, resource.ReporterResources()[0].ReporterResourceKey) assert.NotNil(t, err) assert.Nil(t, foundResource) - return repo.Save(tx, resource, bizmodel.OperationTypeCreated, "tx-create") + return repo.Save(tx, resource, bizmodel.OperationTypeCreated, txid("tx-create")) }) assert.Error(t, err) assert.ErrorContains(t, err, "transaction failed") @@ -1653,7 +1657,7 @@ func TestSerializableUpdateFails(t *testing.T) { // Create initial resource (committed) resource := createTestResourceWithLocalId(t, "serializable-update-conflict") - assert.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-initial")) + assert.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-initial"))) // Prepare an updated version key, err := bizmodel.NewReporterResourceKey("serializable-update-conflict", "k8s_cluster", "ocm", "ocm-instance-1") @@ -1671,12 +1675,12 @@ func TestSerializableUpdateFails(t *testing.T) { foundResource, err := repo.FindResourceByKeys(conflictTx, resource.ReporterResources()[0].ReporterResourceKey) assert.Nil(t, err) assert.NotNil(t, foundResource) - assert.NoError(t, repo.Save(conflictTx, resource, bizmodel.OperationTypeUpdated, "tx-conflict")) + assert.NoError(t, repo.Save(conflictTx, resource, bizmodel.OperationTypeUpdated, txid("tx-conflict"))) // Do NOT commit yet to hold locks // Attempt to update the same resource via TM-managed serializable transaction err = tm.HandleSerializableTransaction("test_update_resource", db, func(tx *gorm.DB) error { - return repo.Save(tx, resource, bizmodel.OperationTypeUpdated, "tx-update") + return repo.Save(tx, resource, bizmodel.OperationTypeUpdated, txid("tx-update")) }) assert.Error(t, err) assert.ErrorContains(t, err, "transaction failed") @@ -2099,7 +2103,7 @@ func TestFindLatestRepresentations(t *testing.T) { resource := createTestResourceWithLocalIdAndType(t, "localResourceId-latest", "host") // Save initial version (version 0) - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-latest-v0") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-latest-v0")) require.NoError(t, err) // Update to version 1 @@ -2118,7 +2122,7 @@ func TestFindLatestRepresentations(t *testing.T) { require.NoError(t, err) err = resource.Update(key, placeholderApiHref, nil, nil, &updatedReporter1, &updatedCommon1, transactionId1) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, "tx-latest-v1") + err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, txid("tx-latest-v1")) require.NoError(t, err) // Update to version 2 (this should be the latest) @@ -2136,7 +2140,7 @@ func TestFindLatestRepresentations(t *testing.T) { transactionId2 := bizmodel.NewTransactionId("test-transaction-id-v2") err = resource.Update(key, placeholderApiHref, nil, nil, &updatedReporter2, &updatedCommon2, transactionId2) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, "tx-latest-v2") + err = repo.Save(db, resource, bizmodel.OperationTypeUpdated, txid("tx-latest-v2")) require.NoError(t, err) // Test FindLatestRepresentations @@ -2197,7 +2201,7 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { // Create and update a resource to have versioned representations (same for both implementations) resource := createTestResourceWithLocalIdAndType(t, "workspace-test-resource", "host") - err := repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-ws-test") + err := repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-ws-test")) require.NoError(t, err) // Update to create version 1 @@ -2214,7 +2218,7 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { require.NoError(t, err) err = resource.Update(key, placeholderApiHref, nil, nil, &updatedReporter, &updatedCommon, transactionId) require.NoError(t, err) - require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeUpdated, "tx-ws-update")) + require.NoError(t, repo.Save(db, resource, bizmodel.OperationTypeUpdated, txid("tx-ws-update"))) // Get current and previous versions version := bizmodel.NewVersion(1) @@ -2234,7 +2238,7 @@ func TestFindCurrentAndPreviousVersionedRepresentations(t *testing.T) { // Create a resource without updates (version 0) - same for both implementations resource := createTestResourceWithLocalIdAndType(t, "test-resource-v0", "host") - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "tx-v0-test") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("tx-v0-test")) require.NoError(t, err) // Get version 0 representations @@ -2314,7 +2318,7 @@ func TestHasTransactionIdBeenProcessed(t *testing.T) { func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepository, db *gorm.DB) { t.Run("Empty transaction ID returns false", func(t *testing.T) { - processed, err := repo.HasTransactionIdBeenProcessed(db, "") + processed, err := repo.HasTransactionIdBeenProcessed(db, txid("")) require.NoError(t, err) assert.False(t, processed, "Empty transaction ID should return false") }) @@ -2322,7 +2326,7 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos t.Run("Non-existent transaction ID returns false", func(t *testing.T) { transactionId := "non-existent-transaction-123" - processed, err := repo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err := repo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.False(t, processed, "Non-existent transaction ID should return false") }) @@ -2333,7 +2337,7 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos transactionId := "test-transaction-456" // Initially should not be processed - processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.False(t, processed, "Transaction ID should not be processed initially") @@ -2341,13 +2345,13 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos fakeRepo.markTransactionIdAsProcessed(transactionId) // Now should be processed - processed, err = fakeRepo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err = fakeRepo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.True(t, processed, "Transaction ID should be processed after marking") // Different transaction ID should still be false differentTransactionId := "different-transaction-789" - processed, err = fakeRepo.HasTransactionIdBeenProcessed(db, differentTransactionId) + processed, err = fakeRepo.HasTransactionIdBeenProcessed(db, txid(differentTransactionId)) require.NoError(t, err) assert.False(t, processed, "Different transaction ID should not be processed") } @@ -2360,12 +2364,12 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos transactionId := "test-transaction-789" // Test that the method doesn't crash and returns false for non-existent transaction - processed, err := realRepo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err := realRepo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.False(t, processed, "Non-existent transaction ID should return false") // Test empty transaction ID - processed, err = realRepo.HasTransactionIdBeenProcessed(db, "") + processed, err = realRepo.HasTransactionIdBeenProcessed(db, txid("")) require.NoError(t, err) assert.False(t, processed, "Empty transaction ID should return false") } @@ -2377,15 +2381,15 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos transactionId3 := "transaction-3" // Initially none should be processed - processed1, err := repo.HasTransactionIdBeenProcessed(db, transactionId1) + processed1, err := repo.HasTransactionIdBeenProcessed(db, txid(transactionId1)) require.NoError(t, err) assert.False(t, processed1, "Transaction ID 1 should not be processed initially") - processed2, err := repo.HasTransactionIdBeenProcessed(db, transactionId2) + processed2, err := repo.HasTransactionIdBeenProcessed(db, txid(transactionId2)) require.NoError(t, err) assert.False(t, processed2, "Transaction ID 2 should not be processed initially") - processed3, err := repo.HasTransactionIdBeenProcessed(db, transactionId3) + processed3, err := repo.HasTransactionIdBeenProcessed(db, txid(transactionId3)) require.NoError(t, err) assert.False(t, processed3, "Transaction ID 3 should not be processed initially") @@ -2394,15 +2398,15 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos fakeRepo.markTransactionIdAsProcessed(transactionId2) // Check all again - processed1, err = repo.HasTransactionIdBeenProcessed(db, transactionId1) + processed1, err = repo.HasTransactionIdBeenProcessed(db, txid(transactionId1)) require.NoError(t, err) assert.False(t, processed1, "Transaction ID 1 should still not be processed") - processed2, err = repo.HasTransactionIdBeenProcessed(db, transactionId2) + processed2, err = repo.HasTransactionIdBeenProcessed(db, txid(transactionId2)) require.NoError(t, err) assert.True(t, processed2, "Transaction ID 2 should now be processed") - processed3, err = repo.HasTransactionIdBeenProcessed(db, transactionId3) + processed3, err = repo.HasTransactionIdBeenProcessed(db, txid(transactionId3)) require.NoError(t, err) assert.False(t, processed3, "Transaction ID 3 should still not be processed") } @@ -2417,7 +2421,7 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos done := make(chan bool, 10) for i := 0; i < 10; i++ { go func() { - processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.False(t, processed, "Concurrent read should return false") done <- true @@ -2435,7 +2439,7 @@ func testHasTransactionIdBeenProcessed(t *testing.T, repo bizmodel.ResourceRepos // Test concurrent reads after marking for i := 0; i < 10; i++ { go func() { - processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, transactionId) + processed, err := fakeRepo.HasTransactionIdBeenProcessed(db, txid(transactionId)) require.NoError(t, err) assert.True(t, processed, "Concurrent read should return true after marking") done <- true @@ -2493,7 +2497,7 @@ func testTransactionIDUniqueConstraint(t *testing.T, repo bizmodel.ResourceRepos err := resource1.Update(key1, apiHref, &consoleHref, nil, &reporterData, &commonData, duplicateTxID) require.NoError(t, err) - err = repo.Save(db, resource1, bizmodel.OperationTypeCreated, "tx-duplicate-1") + err = repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid("tx-duplicate-1")) require.NoError(t, err, "First save should succeed") // Create second resource with the same TransactionID @@ -2504,7 +2508,7 @@ func testTransactionIDUniqueConstraint(t *testing.T, repo bizmodel.ResourceRepos require.NoError(t, err) // This should fail due to unique constraint violation - err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, "tx-duplicate-2") + err = repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid("tx-duplicate-2")) require.Error(t, err, "Second save should fail due to duplicate TransactionID") assert.Contains(t, err.Error(), bizmodel.ReasonNonUniqueTransactionID) }) @@ -2702,7 +2706,7 @@ func TestCommonVersionIncrementAndResetCycle(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("")) require.NoError(t, err) found, err := repo.FindResourceByKeys(db, key) @@ -2724,7 +2728,7 @@ func TestCommonVersionIncrementAndResetCycle(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, "") + err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid("")) require.NoError(t, err) found, err = repo.FindResourceByKeys(db, key) @@ -2745,7 +2749,7 @@ func TestCommonVersionIncrementAndResetCycle(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, "") + err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid("")) require.NoError(t, err) found, err = repo.FindResourceByKeys(db, key) @@ -2798,7 +2802,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { nil, ) require.NoError(t, err) - require.NoError(t, repo.Save(db, resource1, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource1, bizmodel.OperationTypeCreated, txid(""))) found, err := repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -2809,7 +2813,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { &drCommon2, newUniqueTxID("dr-update-1a"), )) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -2820,7 +2824,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { &drCommon3, newUniqueTxID("dr-update-1b"), )) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -2830,7 +2834,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { // Delete the first lifecycle resource require.NoError(t, found.Delete(key)) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeDeleted, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeDeleted, txid(""))) // --- Second lifecycle: new UUIDs, same logical identity --- // The system always generates fresh UUIDs on recreation; common_representations @@ -2853,7 +2857,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { nil, ) require.NoError(t, err) - require.NoError(t, repo.Save(db, resource2, bizmodel.OperationTypeCreated, "")) + require.NoError(t, repo.Save(db, resource2, bizmodel.OperationTypeCreated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -2868,7 +2872,7 @@ func TestCommonVersionAfterDeleteAndRecreate(t *testing.T) { &drCommon2New, newUniqueTxID("dr-update-2a"), )) - require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, "")) + require.NoError(t, repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid(""))) found, err = repo.FindResourceByKeys(db, key) require.NoError(t, err) @@ -2936,7 +2940,7 @@ func TestNullCommonVersionPersistence(t *testing.T) { _ = emptyCommonRepresentation require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("")) require.NoError(t, err, "Should save resource without common representation") key, err := bizmodel.NewReporterResourceKey( @@ -3006,7 +3010,7 @@ func TestCommonVersionDropThenReAdd(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("")) require.NoError(t, err) found, err := repo.FindResourceByKeys(db, key) @@ -3026,7 +3030,7 @@ func TestCommonVersionDropThenReAdd(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, "") + err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid("")) require.NoError(t, err) found, err = repo.FindResourceByKeys(db, key) @@ -3047,7 +3051,7 @@ func TestCommonVersionDropThenReAdd(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, "") + err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid("")) require.NoError(t, err) found, err = repo.FindResourceByKeys(db, key) @@ -3107,7 +3111,7 @@ func TestCommonVersionFirstAddedOnUpdate(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, resource, bizmodel.OperationTypeCreated, "") + err = repo.Save(db, resource, bizmodel.OperationTypeCreated, txid("")) require.NoError(t, err) found, err := repo.FindResourceByKeys(db, key) @@ -3128,7 +3132,7 @@ func TestCommonVersionFirstAddedOnUpdate(t *testing.T) { ) require.NoError(t, err) - err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, "") + err = repo.Save(db, *found, bizmodel.OperationTypeUpdated, txid("")) require.NoError(t, err) found, err = repo.FindResourceByKeys(db, key) diff --git a/internal/service/resources/kesselinventoryservice_test.go b/internal/service/resources/kesselinventoryservice_test.go index 35ec25fbf..96cb50eb5 100644 --- a/internal/service/resources/kesselinventoryservice_test.go +++ b/internal/service/resources/kesselinventoryservice_test.go @@ -3225,7 +3225,7 @@ func TestInventoryService_ReportResource_AllOptionalMetadataFields(t *testing.T) require.NotNil(t, reps) assert.Equal(t, "ws-all-optional", string(reps.CommonData()["workspace_id"].(string))) - processed, err := repo.HasTransactionIdBeenProcessed(db, txId) + processed, err := repo.HasTransactionIdBeenProcessed(db, model.NewTransactionId(txId)) require.NoError(t, err) assert.True(t, processed, "transaction_id should be recorded as processed") } @@ -3742,7 +3742,7 @@ func TestInventoryService_ReportResource_TransactionIdIdempotency(t *testing.T) res1 := tr.Invoke(ctx, withBody(makeReq("https://api.example.com/v1"), ReportResource, httpEndpoint("POST /api/kessel/v1beta2/resources"))) Assert(t, res1, requireSuccess()) - processed, err := repo.HasTransactionIdBeenProcessed(db, txId) + processed, err := repo.HasTransactionIdBeenProcessed(db, model.NewTransactionId(txId)) require.NoError(t, err) assert.True(t, processed, "transaction_id should be recorded after first report") From 0d372916d985aec9565ccd34a48efa166eea5113 Mon Sep 17 00:00:00 2001 From: Sneha Gunta Date: Thu, 30 Apr 2026 17:04:15 -0400 Subject: [PATCH 3/4] Add Version.Decrement() and fix missed schema test conversion Add Decrement() method to Version type with underflow protection, and update TestCalculateTuples to use Version type instead of raw uint. Changes: - Add Version.Decrement() method that returns 0 when already at 0 - Update schema_service_test.go to construct Version values with NewVersion() - Completes the Version type conversion started in previous commits This is a follow-up to the Version tiny type conversion (2A) to ensure all tests use the domain type consistently. Related: RHCLOUD-45005 Co-Authored-By: Claude Sonnet 4.5 --- internal/biz/model/common.go | 9 ++++++ internal/biz/model/schema_service_test.go | 38 +++++++++-------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/internal/biz/model/common.go b/internal/biz/model/common.go index 029f95675..88385c799 100644 --- a/internal/biz/model/common.go +++ b/internal/biz/model/common.go @@ -157,6 +157,15 @@ func (v Version) Increment() Version { return Version(uint(v) + 1) } +// Decrement returns a new Version with the value decremented by 1. +// Returns 0 if the current value is already 0 (prevents underflow). +func (v Version) Decrement() Version { + if uint(v) == 0 { + return Version(0) + } + return Version(uint(v) - 1) +} + func (v Version) Serialize() uint { return SerializeUint(v) } diff --git a/internal/biz/model/schema_service_test.go b/internal/biz/model/schema_service_test.go index e12ad947b..36e76ea44 100644 --- a/internal/biz/model/schema_service_test.go +++ b/internal/biz/model/schema_service_test.go @@ -13,7 +13,7 @@ import ( func TestCalculateTuples(t *testing.T) { tests := []struct { name string - version uint + version model.Version currentWorkspaceID string previousWorkspaceID string expectTuplesToCreate bool @@ -25,7 +25,7 @@ func TestCalculateTuples(t *testing.T) { }{ { name: "version 0 creates initial tuple", - version: 0, + version: model.NewVersion(0), currentWorkspaceID: "workspace-initial", previousWorkspaceID: "", expectTuplesToCreate: true, @@ -35,7 +35,7 @@ func TestCalculateTuples(t *testing.T) { }, { name: "workspace change creates and deletes tuples", - version: 2, + version: model.NewVersion(2), currentWorkspaceID: "workspace-new", previousWorkspaceID: "workspace-old", expectTuplesToCreate: true, @@ -47,7 +47,7 @@ func TestCalculateTuples(t *testing.T) { }, { name: "workspace change creates and deletes tuples version 1", - version: 1, + version: model.NewVersion(1), currentWorkspaceID: "workspace-new", previousWorkspaceID: "workspace-old", expectTuplesToCreate: true, @@ -59,7 +59,7 @@ func TestCalculateTuples(t *testing.T) { }, { name: "same workspace does not create or delete tuples", - version: 2, + version: model.NewVersion(2), currentWorkspaceID: "workspace-same", previousWorkspaceID: "workspace-same", expectTuplesToCreate: false, @@ -85,7 +85,7 @@ func TestCalculateTuples(t *testing.T) { if tt.currentWorkspaceID != "" { currentData = map[string]interface{}{"workspace_id": tt.currentWorkspaceID} } - ver := model.NewVersion(tt.version) + ver := tt.version current, err = model.NewRepresentations( model.Representation(currentData), &ver, @@ -95,11 +95,7 @@ func TestCalculateTuples(t *testing.T) { require.NoError(t, err) if tt.previousWorkspaceID != "" { - prevUint := uint(0) - if tt.version > 0 { - prevUint = tt.version - 1 - } - prevVer := model.NewVersion(prevUint) + prevVer := tt.version.Decrement() previous, err = model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": tt.previousWorkspaceID}), &prevVer, @@ -283,7 +279,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { testCases := []struct { name string operationType model.EventOperationType // kept for scenario naming; not used by CalculateTuples - version uint + version model.Version currentWorkspaceID string previousWorkspaceID string expectTuplesToCreate bool @@ -292,7 +288,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { { name: "CREATE operation should only create tuples", operationType: model.OperationTypeCreated, - version: 0, + version: model.NewVersion(0), currentWorkspaceID: "workspace-new", previousWorkspaceID: "", expectTuplesToCreate: true, @@ -301,7 +297,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { { name: "UPDATE operation with workspace change should create and delete tuples", operationType: model.OperationTypeUpdated, - version: 1, + version: model.NewVersion(1), currentWorkspaceID: "workspace-new", previousWorkspaceID: "workspace-old", expectTuplesToCreate: true, @@ -310,7 +306,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { { name: "UPDATE operation with same workspace should not create or delete tuples", operationType: model.OperationTypeUpdated, - version: 1, + version: model.NewVersion(1), currentWorkspaceID: "workspace-same", previousWorkspaceID: "workspace-same", expectTuplesToCreate: false, @@ -319,7 +315,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { { name: "DELETE operation should only delete tuples", operationType: model.OperationTypeDeleted, - version: 1, + version: model.NewVersion(1), currentWorkspaceID: "", // synthetic empty current previousWorkspaceID: "workspace-current", // previous holds latest expectTuplesToCreate: false, @@ -345,7 +341,7 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { // Build current representation if tc.currentWorkspaceID != "" { currentData := map[string]interface{}{"workspace_id": tc.currentWorkspaceID} - curVer := model.NewVersion(tc.version) + curVer := tc.version currentRep, err := model.NewRepresentations( model.Representation(currentData), &curVer, @@ -355,17 +351,11 @@ func TestCalculateTuples_OperationTypeScenarios(t *testing.T) { require.NoError(t, err) current = currentRep } else { - // For DELETE: current is nil (no new/current state) current = nil } - // Build previous representation if tc.previousWorkspaceID != "" { - prevUint := uint(0) - if tc.version > 0 { - prevUint = tc.version - 1 - } - prevVer := model.NewVersion(prevUint) + prevVer := tc.version.Decrement() previous, err = model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": tc.previousWorkspaceID}), &prevVer, From 8bc5ab6a125fa3eddcc6f2937a6b90ee1f24d9ff Mon Sep 17 00:00:00 2001 From: Sneha Gunta Date: Fri, 1 May 2026 09:15:47 -0400 Subject: [PATCH 4/4] fix: use ver.Decrement() instead of hardcoded NewVersion(0) in test Made-with: Cursor --- internal/biz/model/schema_service_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/biz/model/schema_service_test.go b/internal/biz/model/schema_service_test.go index 36e76ea44..08d35504c 100644 --- a/internal/biz/model/schema_service_test.go +++ b/internal/biz/model/schema_service_test.go @@ -164,7 +164,7 @@ func TestGetWorkspaceVersions(t *testing.T) { nil, ) require.NoError(t, err) - prevVer := model.NewVersion(0) + prevVer := ver.Decrement() previous, err := model.NewRepresentations( model.Representation(map[string]interface{}{"workspace_id": "ws-prev"}), &prevVer,