diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5035b52f..13c0e68a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,19 +7,24 @@ on: jobs: build: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/build.yml@v1.15.0 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/build.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" enable_dockerhub: true enable_ghcr: true dockerhub_org: lerianstudio enable_gitops_artifacts: true + shared_paths: | + go.mod + go.sum + pkg/ + Makefile secrets: inherit update_gitops: needs: [build] - if: needs.build.result == 'success' - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/gitops-update.yml@v1.17.0 + if: needs.build.result == 'success' && needs.build.outputs.has_builds == 'true' + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/gitops-update.yml@v1.18.1 with: runner_type: "firmino-lxc-runners" artifact_pattern: "gitops-tags-tracer" diff --git a/.github/workflows/go-combined-analysis.yml b/.github/workflows/go-combined-analysis.yml index ea7df42c..c0960da6 100644 --- a/.github/workflows/go-combined-analysis.yml +++ b/.github/workflows/go-combined-analysis.yml @@ -25,7 +25,7 @@ on: jobs: go-analysis: name: Go Analysis - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/go-pr-analysis.yml@v1.17.0 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/go-pr-analysis.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" app_name_prefix: "tracer" diff --git a/.github/workflows/gptchangelog.yml b/.github/workflows/gptchangelog.yml index 1a92d38c..fc0c9919 100644 --- a/.github/workflows/gptchangelog.yml +++ b/.github/workflows/gptchangelog.yml @@ -23,7 +23,7 @@ jobs: changelog: # Run if: manual trigger OR Release workflow succeeded if: github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/gptchangelog.yml@v1.17.0 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/gptchangelog.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" # No filter_paths = single app mode (non-monorepo) diff --git a/.github/workflows/pr-security-scan.yml b/.github/workflows/pr-security-scan.yml index a30d2431..04172631 100644 --- a/.github/workflows/pr-security-scan.yml +++ b/.github/workflows/pr-security-scan.yml @@ -24,8 +24,13 @@ on: jobs: security-scan: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-security-scan.yml@v1.17.0 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-security-scan.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" dockerhub_org: "lerianstudio" + shared_paths: | + go.mod + go.sum + pkg/ + Makefile secrets: inherit diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 79ef430f..2a50baf2 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -12,7 +12,7 @@ permissions: jobs: validate: - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.13.1 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" pr_title_types: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4c53d11d..474120dc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,7 +15,7 @@ permissions: jobs: release: name: Create Release - uses: LerianStudio/github-actions-shared-workflows/.github/workflows/release.yml@v1.17.0 + uses: LerianStudio/github-actions-shared-workflows/.github/workflows/release.yml@v1.18.1 with: runner_type: "blacksmith-4vcpu-ubuntu-2404" secrets: inherit diff --git a/Dockerfile b/Dockerfile index 2191f752..2fbdb2f3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,7 +34,8 @@ WORKDIR /app COPY --from=builder /app/tracer /app/tracer COPY --from=builder /tracer/migrations /app/migrations -# distroless:nonroot already runs as non-root user (uid 65532) +# Explicitly set non-root user for Docker Hub health score compliance +USER nonroot:nonroot EXPOSE 8080 7001 diff --git a/api/docs.go b/api/docs.go index 4990f09e..f8cf78cb 100644 --- a/api/docs.go +++ b/api/docs.go @@ -2660,7 +2660,7 @@ const docTemplate = `{ "type": "string" }, "scope": { - "description": "Scope is a human-readable string representation of the limit's scope\n(e.g., \"account:uuid\" or \"segment:uuid\" or \"global\").\nPer section 4.1.1.", + "description": "Scope is a human-readable string representation of the limit's scope\n(e.g., \"account:uuid\" or \"segment:uuid\" or \"global\").", "type": "string" }, "skipReason": { diff --git a/api/openapi.yaml b/api/openapi.yaml index e41ac683..ff07d9a3 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -2399,7 +2399,6 @@ components: description: |- Scope is a human-readable string representation of the limit's scope (e.g., "account:uuid" or "segment:uuid" or "global"). - Per section 4.1.1. type: string skipReason: description: |- diff --git a/api/swagger.json b/api/swagger.json index 97769ddf..2e7b4ed7 100644 --- a/api/swagger.json +++ b/api/swagger.json @@ -2658,7 +2658,7 @@ "type": "string" }, "scope": { - "description": "Scope is a human-readable string representation of the limit's scope\n(e.g., \"account:uuid\" or \"segment:uuid\" or \"global\").\nPer section 4.1.1.", + "description": "Scope is a human-readable string representation of the limit's scope\n(e.g., \"account:uuid\" or \"segment:uuid\" or \"global\").", "type": "string" }, "skipReason": { diff --git a/api/swagger.yaml b/api/swagger.yaml index 6add2ccb..0a587c68 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -494,7 +494,6 @@ definitions: description: |- Scope is a human-readable string representation of the limit's scope (e.g., "account:uuid" or "segment:uuid" or "global"). - Per section 4.1.1. type: string skipReason: description: |- diff --git a/docs/PROJECT_RULES.md b/docs/PROJECT_RULES.md index c6f6100b..b4a72e7a 100644 --- a/docs/PROJECT_RULES.md +++ b/docs/PROJECT_RULES.md @@ -3174,6 +3174,7 @@ otel.SetTracerProvider(tp) // Set global provider for lib-commons 13. **Priority-based rule evaluation** - All rules evaluated, DENY precedence 14. **Direct OTel attribute/codes imports** - Use lib-commons wrappers (SetSpanAttributesFromStruct, HandleSpanError) 15. **Unstructured logging** - Use `WithFields` instead of `Infof`/`Errorf` with string interpolation (see [Structured Logging](#structured-logging)) +16. **Task/ticket IDs in code** - Never reference task IDs (T-001, JIRA-123, etc.) in source code, comments, or test names. Code must be self-explanatory without project management context. Use descriptive names instead. --- @@ -3185,3 +3186,4 @@ otel.SetTracerProvider(tp) // Set global provider for lib-commons 2. **NEVER run `git push` without explicit user approval** 3. **NEVER modify files outside the scope of the requested task** 4. **Always ask for confirmation before destructive operations** +5. **NEVER include test execution results in commit messages** - Do not mention test counts, pass/fail status, or coverage numbers. Commit messages describe *what* changed and *why*, not verification results. diff --git a/internal/adapters/http/in/mocks/validation_service_mock.go b/internal/adapters/http/in/mocks/validation_service_mock.go index 8deaf879..fbb6a141 100644 --- a/internal/adapters/http/in/mocks/validation_service_mock.go +++ b/internal/adapters/http/in/mocks/validation_service_mock.go @@ -12,6 +12,7 @@ package mocks import ( context "context" reflect "reflect" + services "tracer/internal/services" model "tracer/pkg/model" gomock "go.uber.org/mock/gomock" @@ -42,10 +43,10 @@ func (m *MockValidationService) EXPECT() *MockValidationServiceMockRecorder { } // Validate mocks base method. -func (m *MockValidationService) Validate(ctx context.Context, request *model.ValidationRequest) (*model.ValidationResponse, error) { +func (m *MockValidationService) Validate(ctx context.Context, request *model.ValidationRequest) (*services.ValidateResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Validate", ctx, request) - ret0, _ := ret[0].(*model.ValidationResponse) + ret0, _ := ret[0].(*services.ValidateResult) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/internal/adapters/http/in/validation_handler.go b/internal/adapters/http/in/validation_handler.go index a6129c57..39dac4e9 100644 --- a/internal/adapters/http/in/validation_handler.go +++ b/internal/adapters/http/in/validation_handler.go @@ -18,6 +18,7 @@ import ( "github.com/gofiber/fiber/v2" "go.opentelemetry.io/otel/trace" + "tracer/internal/services" "tracer/pkg/clock" "tracer/pkg/constant" "tracer/pkg/logging" @@ -31,7 +32,7 @@ const maxPayloadSize = 100 * 1024 // ValidationService defines the interface for validation operations. // Interface defined locally per Ring pattern. type ValidationService interface { - Validate(ctx context.Context, request *model.ValidationRequest) (*model.ValidationResponse, error) + Validate(ctx context.Context, request *model.ValidationRequest) (*services.ValidateResult, error) } // ValidationHandler handles HTTP requests for transaction validation. @@ -147,7 +148,7 @@ func (h *ValidationHandler) Validate(c *fiber.Ctx) error { } // Call validation service - response, err := h.service.Validate(ctx, &request) + result, err := h.service.Validate(ctx, &request) if err != nil { return h.handleValidationError(c, &span, err) } @@ -155,11 +156,17 @@ func (h *ValidationHandler) Validate(c *fiber.Ctx) error { logger.WithFields( "operation", "handler.validations.validate", "request.id", request.RequestID.String(), - "decision", string(response.Decision), - "processing_time_ms", response.ProcessingTimeMs, + "decision", string(result.Response.Decision), + "processing_time_ms", result.Response.ProcessingTimeMs, + "is_duplicate", result.IsDuplicate, ).Info("Validation completed") - return libHTTP.OK(c, response) + // Return HTTP 201 for new requests, HTTP 200 for duplicate (idempotent) requests (DD-9) + if result.IsDuplicate { + return libHTTP.OK(c, result.Response) + } + + return libHTTP.Created(c, result.Response) } // validationErrorMapping maps validation errors to their specific error codes and messages. diff --git a/internal/adapters/http/in/validation_handler_idempotency_test.go b/internal/adapters/http/in/validation_handler_idempotency_test.go new file mode 100644 index 00000000..613f0a6b --- /dev/null +++ b/internal/adapters/http/in/validation_handler_idempotency_test.go @@ -0,0 +1,215 @@ +// Copyright (c) 2026 Lerian Studio. All rights reserved. +// Use of this source code is governed by the Elastic License 2.0 +// that can be found in the LICENSE file. + +package in + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "github.com/shopspring/decimal" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "tracer/internal/adapters/http/in/mocks" + "tracer/internal/services" + "tracer/internal/testutil" + "tracer/pkg/clock" + "tracer/pkg/model" +) + +// ============================================================================= +// Handler HTTP Status Codes for Idempotency Tests +// ============================================================================= +// These tests verify the HTTP status codes based on IsDuplicate flag: +// - New request (IsDuplicate=false): HTTP 201 Created +// - Duplicate request (IsDuplicate=true): HTTP 200 OK +// ============================================================================= + +// TestValidationHandler_Validate_ReturnsCorrectStatusCodes tests that the handler +// returns HTTP 201 for new requests and HTTP 200 for duplicate requests (DD-9). +func TestValidationHandler_Validate_ReturnsCorrectStatusCodes(t *testing.T) { + validRequestID := testutil.MustDeterministicUUID(7001) + accountID := testutil.MustDeterministicUUID(7002) + now := testutil.DefaultTestTime + + validRequest := model.ValidationRequest{ + RequestID: validRequestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("100"), + Currency: "USD", + TransactionTimestamp: now, + Account: model.AccountContext{ + ID: accountID, + }, + } + + tests := []struct { + name string + mockSetup func(ctrl *gomock.Controller) *mocks.MockValidationService + expectedStatus int + description string + }{ + { + name: "new request returns HTTP 201 Created", + mockSetup: func(ctrl *gomock.Controller) *mocks.MockValidationService { + mockService := mocks.NewMockValidationService(ctrl) + + // Service returns ValidateResult{Response, IsDuplicate: false} for new requests + mockService.EXPECT(). + Validate(gomock.Any(), gomock.Any()). + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(7010), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{testutil.MustDeterministicUUID(7011)}, + Reason: "No matching rules found", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, + ProcessingTimeMs: 15, + }, + IsDuplicate: false, + }, nil) + + return mockService + }, + // New requests should return 201 Created + expectedStatus: http.StatusCreated, + description: "Handler should return 201 for new (non-duplicate) requests", + }, + { + name: "duplicate request returns HTTP 200 OK", + mockSetup: func(ctrl *gomock.Controller) *mocks.MockValidationService { + mockService := mocks.NewMockValidationService(ctrl) + + // Service returns ValidateResult{Response, IsDuplicate: true} for duplicate requests + mockService.EXPECT(). + Validate(gomock.Any(), gomock.Any()). + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(7020), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{testutil.MustDeterministicUUID(7021)}, + Reason: "No matching rules found", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, + ProcessingTimeMs: 10, + }, + IsDuplicate: true, + }, nil) + + return mockService + }, + // Duplicate requests should return 200 OK + expectedStatus: http.StatusOK, + description: "Handler should return 200 for duplicate (idempotent) requests", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + + mockService := tt.mockSetup(ctrl) + + clk := clock.New() + handler, handlerErr := NewValidationHandler(mockService, clk) + require.NoError(t, handlerErr) + + app := fiber.New() + app.Post("/v1/validations", handler.Validate) + + body, err := json.Marshal(validRequest) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/v1/validations", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + resp, err := app.Test(req) + require.NoError(t, err) + defer resp.Body.Close() + + // ASSERTION: Verify correct status code based on IsDuplicate + assert.Equal(t, tt.expectedStatus, resp.StatusCode, tt.description) + }) + } +} + +// TestValidationHandler_Validate_IdempotencyHeader tests that the handler works +// correctly for both new and duplicate requests without requiring special headers. +func TestValidationHandler_Validate_IdempotencyHeader(t *testing.T) { + validRequestID := testutil.MustDeterministicUUID(8001) + accountID := testutil.MustDeterministicUUID(8002) + now := testutil.DefaultTestTime + + validRequest := model.ValidationRequest{ + RequestID: validRequestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("100"), + Currency: "USD", + TransactionTimestamp: now, + Account: model.AccountContext{ + ID: accountID, + Type: "checking", + Status: "active", + }, + } + + ctrl := gomock.NewController(t) + + mockService := mocks.NewMockValidationService(ctrl) + + // Simulate duplicate request scenario + mockService.EXPECT(). + Validate(gomock.Any(), gomock.Any()). + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(8010), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{}, + Reason: "Approved", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, + ProcessingTimeMs: 5, + EvaluatedAt: time.Now().UTC(), + }, + IsDuplicate: true, + }, nil) + + clk := clock.New() + handler, handlerErr := NewValidationHandler(mockService, clk) + require.NoError(t, handlerErr) + + app := fiber.New() + app.Post("/v1/validations", handler.Validate) + + body, err := json.Marshal(validRequest) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/v1/validations", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + resp, err := app.Test(req) + require.NoError(t, err) + defer resp.Body.Close() + + // Duplicate requests should return 200 OK + assert.Equal(t, http.StatusOK, resp.StatusCode, "Duplicate response should return 200 OK") +} diff --git a/internal/adapters/http/in/validation_handler_test.go b/internal/adapters/http/in/validation_handler_test.go index 7354baa7..864ba614 100644 --- a/internal/adapters/http/in/validation_handler_test.go +++ b/internal/adapters/http/in/validation_handler_test.go @@ -24,6 +24,7 @@ import ( "go.uber.org/mock/gomock" "tracer/internal/adapters/http/in/mocks" + "tracer/internal/services" "tracer/internal/testutil" "tracer/pkg/clock" "tracer/pkg/constant" @@ -61,21 +62,24 @@ func TestValidationHandler_Validate(t *testing.T) { mockService := mocks.NewMockValidationService(ctrl) mockService.EXPECT(). Validate(gomock.Any(), gomock.Any()). - Return(&model.ValidationResponse{ - ValidationID: testutil.MustDeterministicUUID(10), - RequestID: validRequestID, - EvaluationResult: model.EvaluationResult{ - Decision: model.DecisionAllow, - MatchedRuleIDs: []uuid.UUID{}, - EvaluatedRuleIDs: []uuid.UUID{testutil.MustDeterministicUUID(11)}, - Reason: "No matching rules found", + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(10), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{testutil.MustDeterministicUUID(11)}, + Reason: "No matching rules found", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, + ProcessingTimeMs: 15, }, - LimitUsageDetails: []model.LimitUsageDetail{}, - ProcessingTimeMs: 15, + IsDuplicate: false, }, nil) return mockService }, - expectedStatus: http.StatusOK, + expectedStatus: http.StatusCreated, expectedBody: func(t *testing.T, body []byte) { var response model.ValidationResponse err := json.Unmarshal(body, &response) @@ -93,21 +97,24 @@ func TestValidationHandler_Validate(t *testing.T) { matchedRuleID := testutil.MustDeterministicUUID(20) mockService.EXPECT(). Validate(gomock.Any(), gomock.Any()). - Return(&model.ValidationResponse{ - ValidationID: testutil.MustDeterministicUUID(21), - RequestID: validRequestID, - EvaluationResult: model.EvaluationResult{ - Decision: model.DecisionDeny, - MatchedRuleIDs: []uuid.UUID{matchedRuleID}, - EvaluatedRuleIDs: []uuid.UUID{matchedRuleID}, - Reason: "High-risk transaction blocked", + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(21), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionDeny, + MatchedRuleIDs: []uuid.UUID{matchedRuleID}, + EvaluatedRuleIDs: []uuid.UUID{matchedRuleID}, + Reason: "High-risk transaction blocked", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, + ProcessingTimeMs: 20, }, - LimitUsageDetails: []model.LimitUsageDetail{}, - ProcessingTimeMs: 20, + IsDuplicate: false, }, nil) return mockService }, - expectedStatus: http.StatusOK, + expectedStatus: http.StatusCreated, expectedBody: func(t *testing.T, body []byte) { var response model.ValidationResponse err := json.Unmarshal(body, &response) @@ -124,28 +131,31 @@ func TestValidationHandler_Validate(t *testing.T) { limitID := testutil.MustDeterministicUUID(30) mockService.EXPECT(). Validate(gomock.Any(), gomock.Any()). - Return(&model.ValidationResponse{ - ValidationID: testutil.MustDeterministicUUID(31), - RequestID: validRequestID, - EvaluationResult: model.EvaluationResult{ - Decision: model.DecisionAllow, - MatchedRuleIDs: []uuid.UUID{}, - EvaluatedRuleIDs: []uuid.UUID{}, - Reason: "Transaction approved", - }, - LimitUsageDetails: []model.LimitUsageDetail{ - { - LimitID: limitID, - LimitAmount: decimal.RequireFromString("1000"), // $1000.00 - CurrentUsage: decimal.RequireFromString("500"), // $500.00 - Exceeded: false, + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(31), + RequestID: validRequestID, + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{}, + Reason: "Transaction approved", }, + LimitUsageDetails: []model.LimitUsageDetail{ + { + LimitID: limitID, + LimitAmount: decimal.RequireFromString("1000"), // $1000.00 + CurrentUsage: decimal.RequireFromString("500"), // $500.00 + Exceeded: false, + }, + }, + ProcessingTimeMs: 25, }, - ProcessingTimeMs: 25, + IsDuplicate: false, }, nil) return mockService }, - expectedStatus: http.StatusOK, + expectedStatus: http.StatusCreated, expectedBody: func(t *testing.T, body []byte) { var response model.ValidationResponse err := json.Unmarshal(body, &response) @@ -516,7 +526,7 @@ func TestValidationHandler_Validate_PayloadSizeCheck(t *testing.T) { { name: "payload at limit (100KB) is accepted", payloadSize: 100 * 1024, // 100KB exactly - expectedStatus: http.StatusOK, + expectedStatus: http.StatusCreated, }, { name: "payload over limit (100KB+1) is rejected", @@ -532,19 +542,22 @@ func TestValidationHandler_Validate_PayloadSizeCheck(t *testing.T) { mockService := mocks.NewMockValidationService(ctrl) // Only expect service call if payload is within limit - if tt.expectedStatus == http.StatusOK { + if tt.expectedStatus == http.StatusCreated { mockService.EXPECT(). Validate(gomock.Any(), gomock.Any()). - Return(&model.ValidationResponse{ - ValidationID: testutil.MustDeterministicUUID(110), - RequestID: testutil.MustDeterministicUUID(111), - EvaluationResult: model.EvaluationResult{ - Decision: model.DecisionAllow, - MatchedRuleIDs: []uuid.UUID{}, - EvaluatedRuleIDs: []uuid.UUID{}, - Reason: "approved", + Return(&services.ValidateResult{ + Response: &model.ValidationResponse{ + ValidationID: testutil.MustDeterministicUUID(110), + RequestID: testutil.MustDeterministicUUID(111), + EvaluationResult: model.EvaluationResult{ + Decision: model.DecisionAllow, + MatchedRuleIDs: []uuid.UUID{}, + EvaluatedRuleIDs: []uuid.UUID{}, + Reason: "approved", + }, + LimitUsageDetails: []model.LimitUsageDetail{}, }, - LimitUsageDetails: []model.LimitUsageDetail{}, + IsDuplicate: false, }, nil) } diff --git a/internal/adapters/postgres/transaction_validation_repository.go b/internal/adapters/postgres/transaction_validation_repository.go index 7b087ed1..6d517b6e 100644 --- a/internal/adapters/postgres/transaction_validation_repository.go +++ b/internal/adapters/postgres/transaction_validation_repository.go @@ -14,6 +14,7 @@ import ( "time" libCommons "github.com/LerianStudio/lib-commons/v2/commons" + libLog "github.com/LerianStudio/lib-commons/v2/commons/log" libOtel "github.com/LerianStudio/lib-commons/v2/commons/opentelemetry" libPostgres "github.com/LerianStudio/lib-commons/v2/commons/postgres" sq "github.com/Masterminds/squirrel" @@ -120,12 +121,61 @@ func (r *TransactionValidationRepository) Insert(ctx context.Context, validation db, err := r.conn.GetDB() if err != nil { libOtel.HandleSpanError(&span, "Failed to get database connection", err) + return fmt.Errorf("failed to get database connection: %w", err) } + return r.insertInternal(ctx, db, validation, logger, &span, "repository.transaction_validation.insert") +} + +// InsertWithTx creates a new transaction validation record using the provided database connection. +// This allows callers to pass either a regular DB connection or a transaction (*sql.Tx), +// enabling atomic operations with other database changes. +// This maintains the immutability requirement for compliance (SOX/GLBA). +// Uses the ToEntity/FromEntity pattern from Ring Standards (golang/domain.md). +func (r *TransactionValidationRepository) InsertWithTx(ctx context.Context, db pgdb.DB, validation *model.TransactionValidation) error { + if validation == nil { + return errors.New("validation cannot be nil") + } + + if db == nil { + // Span not annotated here: span starts after this check to avoid + // OpenTelemetry overhead for invalid calls (nil db is a programming error). + return pgdb.ErrNilConnection + } + + logger, tracer, _, _ := libCommons.NewTrackingFromContext(ctx) + + ctx, span := tracer.Start(ctx, "repository.transaction_validation.insert_with_tx") + defer span.End() + + logger = logging.WithTrace(ctx, logger) + + return r.insertInternal(ctx, db, validation, logger, &span, "repository.transaction_validation.insert_with_tx") +} + +// insertInternal contains the shared INSERT logic for both Insert and InsertWithTx. +// It performs validation, entity conversion, query building, and execution. +// Uses the ToEntity/FromEntity pattern from Ring Standards (golang/domain.md). +func (r *TransactionValidationRepository) insertInternal( + ctx context.Context, + db pgdb.DB, + validation *model.TransactionValidation, + logger libLog.Logger, + span *trace.Span, + operationName string, +) error { + if validation == nil { + err := errors.New("validation cannot be nil") + libOtel.HandleSpanError(span, "Nil validation input", err) + + return err + } + // Convert domain entity to database model using FromEntity pattern var dbModel TransactionValidationPostgreSQLModel if err := dbModel.FromEntity(validation); err != nil { + libOtel.HandleSpanError(span, "Failed to convert entity to database model", err) return fmt.Errorf("failed to convert entity to database model: %w", err) } @@ -180,19 +230,21 @@ func (r *TransactionValidationRepository) Insert(ctx context.Context, validation sqlStr, args, err := qb.ToSql() if err != nil { - libOtel.HandleSpanError(&span, "Failed to build query", err) + libOtel.HandleSpanError(span, "Failed to build query", err) + return fmt.Errorf("failed to build query: %w", err) } logger.WithFields( - "operation", "repository.transaction_validation.insert", + "operation", operationName, "validation.id", validation.ID.String(), "validation.decision", string(validation.Decision), ).Info("Inserting transaction validation record") _, err = db.ExecContext(ctx, sqlStr, args...) if err != nil { - libOtel.HandleSpanError(&span, "Failed to insert transaction validation", err) + libOtel.HandleSpanError(span, "Failed to insert transaction validation", err) + return fmt.Errorf("failed to insert transaction validation: %w", err) } diff --git a/internal/adapters/postgres/transaction_validation_repository_test.go b/internal/adapters/postgres/transaction_validation_repository_test.go index bad875de..4e9eecd8 100644 --- a/internal/adapters/postgres/transaction_validation_repository_test.go +++ b/internal/adapters/postgres/transaction_validation_repository_test.go @@ -22,6 +22,7 @@ import ( "github.com/shopspring/decimal" + pgdb "tracer/internal/adapters/postgres/db" "tracer/internal/adapters/postgres/db/mocks" "tracer/internal/testutil" "tracer/pkg/constant" @@ -1393,3 +1394,175 @@ func TestTransactionValidationPostgresRepository_FindByRequestID(t *testing.T) { }) } } + +// ============================================================================= +// InsertWithTx Tests (Transactional Repository Methods) +// ============================================================================= + +// TestTransactionValidationPostgresRepository_InsertWithTx tests the InsertWithTx method +// that accepts a pgdb.DB parameter for transactional operations. +// This enables atomic operations with other database changes (e.g., limit checks + audit write). +func TestTransactionValidationPostgresRepository_InsertWithTx(t *testing.T) { + testutil.SetupTestTracing(t) + + tests := []struct { + name string + tv *model.TransactionValidation + mockSetup func(mock sqlmock.Sqlmock, tv *model.TransactionValidation) + wantErr bool + errMsg string + }{ + { + name: "inserts transaction validation using provided db connection", + tv: testTransactionValidation(), + mockSetup: func(mock sqlmock.Sqlmock, tv *model.TransactionValidation) { + mock.ExpectExec(regexp.QuoteMeta(`INSERT INTO transaction_validations`)). + WithArgs( + tv.ID, + tv.RequestID, + string(tv.TransactionType), + tv.SubType, + tv.Amount, + tv.Currency, + tv.TransactionTimestamp, + sqlmock.AnyArg(), // account (JSONB) + sqlmock.AnyArg(), // segment (JSONB) + sqlmock.AnyArg(), // portfolio (JSONB) + sqlmock.AnyArg(), // merchant (JSONB) + sqlmock.AnyArg(), // metadata (JSONB) + string(tv.Decision), + tv.Reason, + sqlmock.AnyArg(), // matched_rule_ids (UUID[]) + sqlmock.AnyArg(), // evaluated_rule_ids (UUID[]) + sqlmock.AnyArg(), // limit_usage_details (JSONB) + tv.ProcessingTimeMs, + tv.CreatedAt, + ). + WillReturnResult(sqlmock.NewResult(1, 1)) + }, + wantErr: false, + }, + { + name: "returns error when database insert fails", + tv: testTransactionValidation(), + mockSetup: func(mock sqlmock.Sqlmock, tv *model.TransactionValidation) { + mock.ExpectExec(regexp.QuoteMeta(`INSERT INTO transaction_validations`)). + WillReturnError(errors.New("database error")) + }, + wantErr: true, + errMsg: "failed to insert transaction validation", + }, + { + name: "returns error when validation is nil", + tv: nil, + mockSetup: func(mock sqlmock.Sqlmock, tv *model.TransactionValidation) {}, + wantErr: true, + errMsg: "validation cannot be nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + db, sqlMock, err := sqlmock.New() + require.NoError(t, err) + defer func() { + sqlMock.ExpectClose() + require.NoError(t, db.Close()) + }() + + ctrl := gomock.NewController(t) + mockConn := mocks.NewMockConnection(ctrl) + repo := NewTransactionValidationRepositoryWithConnection(mockConn) + + tt.mockSetup(sqlMock, tt.tv) + + ctx := context.Background() + + // Call InsertWithTx with the mock db directly + // This tests that the method uses the provided db, not r.conn.GetDB() + err = repo.InsertWithTx(ctx, db, tt.tv) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + } + + require.NoError(t, sqlMock.ExpectationsWereMet()) + }) + } +} + +// TestTransactionValidationPostgresRepository_InsertWithTx_UsesProvidedDB verifies that +// InsertWithTx uses the provided db parameter instead of calling r.conn.GetDB(). +// This is critical for transactional consistency - the provided db may be a transaction. +func TestTransactionValidationPostgresRepository_InsertWithTx_UsesProvidedDB(t *testing.T) { + testutil.SetupTestTracing(t) + + db, sqlMock, err := sqlmock.New() + require.NoError(t, err) + defer func() { + sqlMock.ExpectClose() + require.NoError(t, db.Close()) + }() + + ctrl := gomock.NewController(t) + mockConn := mocks.NewMockConnection(ctrl) + // Expect GetDB to NEVER be called - InsertWithTx should use the provided db + mockConn.EXPECT().GetDB().Times(0) + + repo := NewTransactionValidationRepositoryWithConnection(mockConn) + + tv := testTransactionValidation() + sqlMock.ExpectExec(regexp.QuoteMeta(`INSERT INTO transaction_validations`)). + WithArgs( + tv.ID, + tv.RequestID, + string(tv.TransactionType), + tv.SubType, + tv.Amount, + tv.Currency, + tv.TransactionTimestamp, + sqlmock.AnyArg(), // account (JSONB) + sqlmock.AnyArg(), // segment (JSONB) + sqlmock.AnyArg(), // portfolio (JSONB) + sqlmock.AnyArg(), // merchant (JSONB) + sqlmock.AnyArg(), // metadata (JSONB) + string(tv.Decision), + tv.Reason, + sqlmock.AnyArg(), // matched_rule_ids (UUID[]) + sqlmock.AnyArg(), // evaluated_rule_ids (UUID[]) + sqlmock.AnyArg(), // limit_usage_details (JSONB) + tv.ProcessingTimeMs, + tv.CreatedAt, + ). + WillReturnResult(sqlmock.NewResult(1, 1)) + + ctx := context.Background() + err = repo.InsertWithTx(ctx, db, tv) + + require.NoError(t, err) + require.NoError(t, sqlMock.ExpectationsWereMet()) +} + +// TestTransactionValidationPostgresRepository_InsertWithTx_NilDB verifies that +// InsertWithTx returns an error when called with a nil db parameter. +// This prevents panics when callers pass nil instead of a valid connection. +func TestTransactionValidationPostgresRepository_InsertWithTx_NilDB(t *testing.T) { + testutil.SetupTestTracing(t) + + ctrl := gomock.NewController(t) + mockConn := mocks.NewMockConnection(ctrl) + // Expect GetDB to NEVER be called - nil check should happen before any DB operations + mockConn.EXPECT().GetDB().Times(0) + + repo := NewTransactionValidationRepositoryWithConnection(mockConn) + + tv := testTransactionValidation() + + ctx := context.Background() + err := repo.InsertWithTx(ctx, nil, tv) + + require.ErrorIs(t, err, pgdb.ErrNilConnection) +} diff --git a/internal/bootstrap/config.go b/internal/bootstrap/config.go index 229777e9..83129a62 100644 --- a/internal/bootstrap/config.go +++ b/internal/bootstrap/config.go @@ -644,7 +644,8 @@ func initHTTPServer( } // Init ValidationService with audit writer for SOX/GLBA compliance - validationService, err := services.NewValidationService(evaluateRulesQuery, limitChecker, transactionValidationRepo, auditWriter, clk) + // Pass transactionValidationRepo for both command (insert) and query (FindByRequestID) operations + validationService, err := services.NewValidationService(evaluateRulesQuery, limitChecker, transactionValidationRepo, transactionValidationRepo, auditWriter, clk) if err != nil { return nil, err } diff --git a/internal/services/command/mocks/transaction_validation_repository_mock.go b/internal/services/command/mocks/transaction_validation_repository_mock.go index 17314c5c..15890d1c 100644 --- a/internal/services/command/mocks/transaction_validation_repository_mock.go +++ b/internal/services/command/mocks/transaction_validation_repository_mock.go @@ -12,6 +12,7 @@ package mocks import ( context "context" reflect "reflect" + db "tracer/internal/adapters/postgres/db" model "tracer/pkg/model" gomock "go.uber.org/mock/gomock" @@ -54,3 +55,17 @@ func (mr *MockTransactionValidationRepositoryMockRecorder) Insert(ctx, validatio mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Insert", reflect.TypeOf((*MockTransactionValidationRepository)(nil).Insert), ctx, validation) } + +// InsertWithTx mocks base method. +func (m *MockTransactionValidationRepository) InsertWithTx(ctx context.Context, arg1 db.DB, validation *model.TransactionValidation) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InsertWithTx", ctx, arg1, validation) + ret0, _ := ret[0].(error) + return ret0 +} + +// InsertWithTx indicates an expected call of InsertWithTx. +func (mr *MockTransactionValidationRepositoryMockRecorder) InsertWithTx(ctx, arg1, validation any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertWithTx", reflect.TypeOf((*MockTransactionValidationRepository)(nil).InsertWithTx), ctx, arg1, validation) +} diff --git a/internal/services/command/transaction_validation_repository.go b/internal/services/command/transaction_validation_repository.go index ff980784..faa33094 100644 --- a/internal/services/command/transaction_validation_repository.go +++ b/internal/services/command/transaction_validation_repository.go @@ -9,6 +9,7 @@ package command import ( "context" + pgdb "tracer/internal/adapters/postgres/db" "tracer/pkg/model" ) @@ -21,4 +22,11 @@ type TransactionValidationRepository interface { // This maintains the immutability requirement for compliance (SOX/GLBA). // Implementations MUST return a non-nil error if validation is nil (no panics). Insert(ctx context.Context, validation *model.TransactionValidation) error + + // InsertWithTx creates a new transaction validation record using the provided database connection. + // This allows callers to pass either a regular DB connection or a transaction (*sql.Tx), + // enabling atomic operations with other database changes. + // This maintains the immutability requirement for compliance (SOX/GLBA). + // Implementations MUST return a non-nil error if validation is nil (no panics). + InsertWithTx(ctx context.Context, db pgdb.DB, validation *model.TransactionValidation) error } diff --git a/internal/services/mocks/validation_service_mock.go b/internal/services/mocks/validation_service_mock.go index 246624f5..433a47da 100644 --- a/internal/services/mocks/validation_service_mock.go +++ b/internal/services/mocks/validation_service_mock.go @@ -14,6 +14,7 @@ import ( reflect "reflect" model "tracer/pkg/model" + uuid "github.com/google/uuid" gomock "go.uber.org/mock/gomock" ) @@ -108,3 +109,42 @@ func (mr *MockLimitCheckerMockRecorder) RollbackUsage(ctx, input, usageDetails a mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RollbackUsage", reflect.TypeOf((*MockLimitChecker)(nil).RollbackUsage), ctx, input, usageDetails) } + +// MockTransactionValidationQueryRepository is a mock of TransactionValidationQueryRepository interface. +type MockTransactionValidationQueryRepository struct { + ctrl *gomock.Controller + recorder *MockTransactionValidationQueryRepositoryMockRecorder + isgomock struct{} +} + +// MockTransactionValidationQueryRepositoryMockRecorder is the mock recorder for MockTransactionValidationQueryRepository. +type MockTransactionValidationQueryRepositoryMockRecorder struct { + mock *MockTransactionValidationQueryRepository +} + +// NewMockTransactionValidationQueryRepository creates a new mock instance. +func NewMockTransactionValidationQueryRepository(ctrl *gomock.Controller) *MockTransactionValidationQueryRepository { + mock := &MockTransactionValidationQueryRepository{ctrl: ctrl} + mock.recorder = &MockTransactionValidationQueryRepositoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockTransactionValidationQueryRepository) EXPECT() *MockTransactionValidationQueryRepositoryMockRecorder { + return m.recorder +} + +// FindByRequestID mocks base method. +func (m *MockTransactionValidationQueryRepository) FindByRequestID(ctx context.Context, requestID uuid.UUID) (*model.TransactionValidation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "FindByRequestID", ctx, requestID) + ret0, _ := ret[0].(*model.TransactionValidation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// FindByRequestID indicates an expected call of FindByRequestID. +func (mr *MockTransactionValidationQueryRepositoryMockRecorder) FindByRequestID(ctx, requestID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindByRequestID", reflect.TypeOf((*MockTransactionValidationQueryRepository)(nil).FindByRequestID), ctx, requestID) +} diff --git a/internal/services/query/limit_checker.go b/internal/services/query/limit_checker.go index ad53e010..38f5ad52 100644 --- a/internal/services/query/limit_checker.go +++ b/internal/services/query/limit_checker.go @@ -14,11 +14,13 @@ import ( "time" libCommons "github.com/LerianStudio/lib-commons/v2/commons" + libLog "github.com/LerianStudio/lib-commons/v2/commons/log" libOtel "github.com/LerianStudio/lib-commons/v2/commons/opentelemetry" "github.com/google/uuid" "github.com/shopspring/decimal" "go.opentelemetry.io/otel/trace" + pgdb "tracer/internal/adapters/postgres/db" "tracer/pkg/clock" "tracer/pkg/constant" "tracer/pkg/logging" @@ -75,6 +77,20 @@ type LimitChecker interface { // For PER_TRANSACTION limits: checks maxAmount directly without persistent counters CheckLimits(ctx context.Context, input *model.CheckLimitsInput) (*model.CheckLimitsOutput, error) + // CheckLimitsWithTx evaluates all applicable limits for a transaction. + // The db parameter is accepted for API consistency and will be used when + // underlying repositories support transactional operations. + // Currently behaves identically to CheckLimits. + // + // Returns CheckLimitsOutput with: + // - Allowed: true if no limits exceeded (or no active limits found) + // - ExceededLimitIDs: IDs of limits that would be exceeded + // - LimitUsageDetails: usage information for all checked limits + // + // For DAILY/MONTHLY limits: increments usage counters atomically + // For PER_TRANSACTION limits: checks maxAmount directly without persistent counters + CheckLimitsWithTx(ctx context.Context, db pgdb.DB, input *model.CheckLimitsInput) (*model.CheckLimitsOutput, error) + // RollbackUsage decrements usage counters for limits that were previously incremented. // Called when a transaction is denied by rules after limits were already incremented. // Only affects DAILY/MONTHLY limits (PER_TRANSACTION has no persistent counters). @@ -121,21 +137,57 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec logger = logging.WithTrace(ctx, logger) + return s.checkLimitsInternal(ctx, input, logger, &span, "service.limit_checker.check_limits") +} + +// CheckLimitsWithTx evaluates all applicable limits for a transaction using the provided database connection. +// This allows callers to pass either a regular DB connection or a transaction (*sql.Tx), +// enabling atomic operations with other database changes. +// Uses atomic upsert to prevent TOCTOU race conditions. +// +// NOTE: The db parameter is accepted for API consistency with InsertWithTx and to enable future +// transactional repository methods. Currently, the underlying repositories do not yet support +// transactional operations, so the db parameter is not passed through to repository calls. +// When repositories are updated to support WithTx variants, this method will pass db accordingly. +// +// NOTE: Wire db parameter through to LimitRepository and UsageCounterRepository +// when those repositories gain WithTx variants. +func (s *LimitCheckerService) CheckLimitsWithTx(ctx context.Context, _ pgdb.DB, input *model.CheckLimitsInput) (*model.CheckLimitsOutput, error) { + logger, tracer, _, _ := libCommons.NewTrackingFromContext(ctx) + + ctx, span := tracer.Start(ctx, "service.limit_checker.check_limits_with_tx") + defer span.End() + + logger = logging.WithTrace(ctx, logger) + + return s.checkLimitsInternal(ctx, input, logger, &span, "service.limit_checker.check_limits_with_tx") +} + +// checkLimitsInternal contains the shared logic for CheckLimits and CheckLimitsWithTx. +// It validates input, retrieves applicable limits, and processes each limit atomically. +// The operationName parameter is used for consistent logging across both public methods. +func (s *LimitCheckerService) checkLimitsInternal( + ctx context.Context, + input *model.CheckLimitsInput, + logger libLog.Logger, + span *trace.Span, + operationName string, +) (*model.CheckLimitsOutput, error) { if input == nil { - libOtel.HandleSpanBusinessErrorEvent(&span, "Nil input", constant.ErrCheckLimitsNilInput) + libOtel.HandleSpanBusinessErrorEvent(span, "Nil input", constant.ErrCheckLimitsNilInput) return nil, constant.ErrCheckLimitsNilInput } if err := input.Validate(); err != nil { - libOtel.HandleSpanBusinessErrorEvent(&span, "Invalid input", err) + libOtel.HandleSpanBusinessErrorEvent(span, "Invalid input", err) return nil, err } - if err := libOtel.SetSpanAttributesFromStruct(&span, "input", input); err != nil { - span.RecordError(err) + if err := libOtel.SetSpanAttributesFromStruct(span, "input", input); err != nil { + (*span).RecordError(err) logger.WithFields( - "operation", "service.limit_checker.check_limits", + "operation", operationName, "error", err.Error(), ).Warn("Failed to set span attributes for input") } @@ -143,7 +195,7 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec // Get applicable limits (active limits matching currency and scopes) limits, err := s.getApplicableLimits(ctx, input) if err != nil { - libOtel.HandleSpanError(&span, "Failed to get applicable limits", err) + libOtel.HandleSpanError(span, "Failed to get applicable limits", err) return nil, err } @@ -152,7 +204,7 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec if len(limits) == 0 { logger.WithFields( - "operation", "service.limit_checker.check_limits", + "operation", operationName, "currency", input.Currency, ).Info("No active limits found for criteria") @@ -162,7 +214,7 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec } logger.WithFields( - "operation", "service.limit_checker.check_limits", + "operation", operationName, "applicable_limits_count", len(limits), ).Info("Found applicable limits") @@ -197,7 +249,7 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec s.rollbackIncrementedCounters(rollbackCtx, input, incrementedDetails, txScope) } - libOtel.HandleSpanError(&span, "Failed to process limit atomically", err) + libOtel.HandleSpanError(span, "Failed to process limit atomically", err) return nil, err } @@ -235,12 +287,12 @@ func (s *LimitCheckerService) CheckLimits(ctx context.Context, input *model.Chec output = output.WithExceededLimits([]uuid.UUID{*exceededLimitID}) logger.WithFields( - "operation", "service.limit_checker.check_limits", + "operation", operationName, "exceeded_limit_id", exceededLimitID.String(), ).Info("Limit exceeded") } else { logger.WithFields( - "operation", "service.limit_checker.check_limits", + "operation", operationName, "checked_count", len(usageDetails), ).Info("All limits passed") } diff --git a/internal/services/query/limit_checker_mock.go b/internal/services/query/limit_checker_mock.go index f5759394..b18fec80 100644 --- a/internal/services/query/limit_checker_mock.go +++ b/internal/services/query/limit_checker_mock.go @@ -12,6 +12,7 @@ package query import ( context "context" reflect "reflect" + db "tracer/internal/adapters/postgres/db" model "tracer/pkg/model" gomock "go.uber.org/mock/gomock" @@ -56,6 +57,21 @@ func (mr *MockLimitCheckerMockRecorder) CheckLimits(ctx, input any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckLimits", reflect.TypeOf((*MockLimitChecker)(nil).CheckLimits), ctx, input) } +// CheckLimitsWithTx mocks base method. +func (m *MockLimitChecker) CheckLimitsWithTx(ctx context.Context, arg1 db.DB, input *model.CheckLimitsInput) (*model.CheckLimitsOutput, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckLimitsWithTx", ctx, arg1, input) + ret0, _ := ret[0].(*model.CheckLimitsOutput) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CheckLimitsWithTx indicates an expected call of CheckLimitsWithTx. +func (mr *MockLimitCheckerMockRecorder) CheckLimitsWithTx(ctx, arg1, input any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckLimitsWithTx", reflect.TypeOf((*MockLimitChecker)(nil).CheckLimitsWithTx), ctx, arg1, input) +} + // RollbackUsage mocks base method. func (m *MockLimitChecker) RollbackUsage(ctx context.Context, input *model.CheckLimitsInput, usageDetails []model.LimitUsageDetail) error { m.ctrl.T.Helper() diff --git a/internal/services/query/limit_checker_test.go b/internal/services/query/limit_checker_test.go index 54511d61..8c20ba44 100644 --- a/internal/services/query/limit_checker_test.go +++ b/internal/services/query/limit_checker_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + dbmocks "tracer/internal/adapters/postgres/db/mocks" "tracer/internal/testhelper" "tracer/internal/testutil" "tracer/pkg/clock" @@ -5286,3 +5287,261 @@ func TestCalculateCounterExpiresAt_RetentionDays(t *testing.T) { expected := resetAt.AddDate(0, 0, 90) // April 1, 2026 assert.Equal(t, expected, *result) } + +// ============================================================================= +// CheckLimitsWithTx Tests (Transactional Repository Methods) +// ============================================================================= + +// TestLimitCheckerService_CheckLimitsWithTx tests the CheckLimitsWithTx method +// that accepts a pgdb.DB parameter for transactional operations. +// This enables atomic operations with other database changes (e.g., validate + audit write). +// +// NOTE: These tests do not yet assert that mockDB is forwarded to repository calls because +// the underlying repositories (LimitRepository, UsageCounterRepository) do not yet accept +// a pgdb.DB parameter. When those repos gain WithTx variants, update these expectations +// to verify the db parameter is propagated (e.g., gomock.Eq(mockDB) instead of gomock.Any()). +// +// NOTE: When repos support WithTx, add gomock.Eq(mockDB) expectations +// to verify db parameter is forwarded correctly. +func TestLimitCheckerService_CheckLimitsWithTx(t *testing.T) { + // Test UUIDs - seed range: 15000-15100 + limitID1 := testutil.MustDeterministicUUID(15001) + accountID := testutil.MustDeterministicUUID(15100) + + timestamp := time.Date(2025, 12, 28, 10, 0, 0, 0, time.UTC) + periodKeyDaily := serverPeriodKeyDaily + + tests := []struct { + name string + input *model.CheckLimitsInput + setupMocks func(*MockLimitRepository, *MockUsageCounterRepository) + wantAllowed bool + wantExceeded []uuid.UUID + wantDetails int + wantErr bool + wantErrIs error + }{ + { + name: "checks limits using provided db connection - allowed", + input: &model.CheckLimitsInput{ + Amount: decimal.RequireFromString("50"), + Currency: "USD", + AccountID: accountID, + TransactionTimestamp: timestamp, + }, + setupMocks: func(lr *MockLimitRepository, ucr *MockUsageCounterRepository) { + status := model.LimitStatusActive + currency := "USD" + lr.EXPECT().List(gomock.Any(), &model.ListLimitsFilter{ + Status: &status, + Currency: ¤cy, + Limit: constant.MaxPaginationLimit, + }).Return(&model.ListLimitsResult{ + Limits: []model.Limit{ + { + ID: limitID1, + Name: "Daily Limit", + LimitType: model.LimitTypeDaily, + MaxAmount: decimal.RequireFromString("1000"), + Currency: "USD", + Scopes: []model.Scope{{AccountID: &accountID}}, + Status: model.LimitStatusActive, + }, + }, + HasMore: false, + }, nil) + + scopeKey := "acct:" + accountID.String() + // Atomic upsert: returns new usage (0 + 50 = 50) + ucr.EXPECT().UpsertAndIncrementAtomic(gomock.Any(), limitID1, scopeKey, periodKeyDaily, decimal.RequireFromString("50"), decimal.RequireFromString("1000"), gomock.Any()). + Return(decimal.RequireFromString("50"), nil) + }, + wantAllowed: true, + wantExceeded: nil, + wantDetails: 1, + wantErr: false, + }, + { + name: "checks limits using provided db connection - exceeded", + input: &model.CheckLimitsInput{ + Amount: decimal.RequireFromString("600"), + Currency: "USD", + AccountID: accountID, + TransactionTimestamp: timestamp, + }, + setupMocks: func(lr *MockLimitRepository, ucr *MockUsageCounterRepository) { + status := model.LimitStatusActive + currency := "USD" + lr.EXPECT().List(gomock.Any(), &model.ListLimitsFilter{ + Status: &status, + Currency: ¤cy, + Limit: constant.MaxPaginationLimit, + }).Return(&model.ListLimitsResult{ + Limits: []model.Limit{ + { + ID: limitID1, + Name: "Daily Limit", + LimitType: model.LimitTypeDaily, + MaxAmount: decimal.RequireFromString("1000"), + Currency: "USD", + Scopes: []model.Scope{{AccountID: &accountID}}, + Status: model.LimitStatusActive, + }, + }, + HasMore: false, + }, nil) + + scopeKey := "acct:" + accountID.String() + // Atomic upsert: returns ErrUsageCounterExceedsLimit when 500 + 600 > 1000 + ucr.EXPECT().UpsertAndIncrementAtomic(gomock.Any(), limitID1, scopeKey, periodKeyDaily, decimal.RequireFromString("600"), decimal.RequireFromString("1000"), gomock.Any()). + Return(decimal.RequireFromString("500"), constant.ErrUsageCounterExceedsLimit) + }, + wantAllowed: false, + wantExceeded: []uuid.UUID{limitID1}, + wantDetails: 1, + wantErr: false, + }, + { + name: "returns error on nil input", + input: nil, + setupMocks: func(lr *MockLimitRepository, ucr *MockUsageCounterRepository) { + // No mocks - validation fails before repository calls + }, + wantAllowed: false, + wantErr: true, + wantErrIs: constant.ErrCheckLimitsNilInput, + }, + { + name: "no active limits - allowed", + input: &model.CheckLimitsInput{ + Amount: decimal.RequireFromString("100"), + Currency: "USD", + AccountID: accountID, + TransactionTimestamp: timestamp, + }, + setupMocks: func(lr *MockLimitRepository, ucr *MockUsageCounterRepository) { + status := model.LimitStatusActive + currency := "USD" + lr.EXPECT().List(gomock.Any(), &model.ListLimitsFilter{ + Status: &status, + Currency: ¤cy, + Limit: constant.MaxPaginationLimit, + }).Return(&model.ListLimitsResult{ + Limits: []model.Limit{}, + HasMore: false, + }, nil) + }, + wantAllowed: true, + wantExceeded: nil, + wantDetails: 0, + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + + mockLimitRepo := NewMockLimitRepository(ctrl) + mockUsageRepo := NewMockUsageCounterRepository(ctrl) + + tc.setupMocks(mockLimitRepo, mockUsageRepo) + + ctx := setupTest(t) + + checker, err := NewLimitChecker(mockLimitRepo, mockUsageRepo, testutil.NewDefaultMockClock()) + require.NoError(t, err) + + mockDB := dbmocks.NewMockDB(ctrl) + + output, err := checker.CheckLimitsWithTx(ctx, mockDB, tc.input) + + if tc.wantErr { + require.Error(t, err) + if tc.wantErrIs != nil { + assert.ErrorIs(t, err, tc.wantErrIs) + } + assert.Nil(t, output) + } else { + require.NoError(t, err) + require.NotNil(t, output) + assert.Equal(t, tc.wantAllowed, output.Allowed) + assert.Len(t, output.LimitUsageDetails, tc.wantDetails) + + if tc.wantExceeded != nil { + assert.Equal(t, tc.wantExceeded, output.ExceededLimitIDs) + } else { + assert.Empty(t, output.ExceededLimitIDs) + } + } + }) + } +} + +// TestLimitCheckerService_CheckLimitsWithTx_UsesProvidedDB verifies that +// CheckLimitsWithTx accepts a db parameter for transactional consistency. +// +// NOTE: Currently this test only verifies the method accepts and runs with a mockDB. +// It does not assert the db is forwarded to repos because they lack WithTx variants. +// +// NOTE: When repos support WithTx, add gomock.Eq(mockDB) expectations here. +func TestLimitCheckerService_CheckLimitsWithTx_UsesProvidedDB(t *testing.T) { + // Test UUIDs - seed range: 15200-15300 + limitID1 := testutil.MustDeterministicUUID(15201) + accountID := testutil.MustDeterministicUUID(15200) + + timestamp := time.Date(2025, 12, 28, 10, 0, 0, 0, time.UTC) + periodKeyDaily := serverPeriodKeyDaily + + ctrl := gomock.NewController(t) + + mockLimitRepo := NewMockLimitRepository(ctrl) + mockUsageRepo := NewMockUsageCounterRepository(ctrl) + + status := model.LimitStatusActive + currency := "USD" + + mockLimitRepo.EXPECT().List(gomock.Any(), &model.ListLimitsFilter{ + Status: &status, + Currency: ¤cy, + Limit: constant.MaxPaginationLimit, + }).Return(&model.ListLimitsResult{ + Limits: []model.Limit{ + { + ID: limitID1, + Name: "Daily Limit", + LimitType: model.LimitTypeDaily, + MaxAmount: decimal.RequireFromString("1000"), + Currency: "USD", + Scopes: []model.Scope{{AccountID: &accountID}}, + Status: model.LimitStatusActive, + }, + }, + HasMore: false, + }, nil) + + scopeKey := "acct:" + accountID.String() + // Atomic upsert: returns new usage + mockUsageRepo.EXPECT().UpsertAndIncrementAtomic(gomock.Any(), limitID1, scopeKey, periodKeyDaily, decimal.RequireFromString("50"), decimal.RequireFromString("1000"), gomock.Any()). + Return(decimal.RequireFromString("50"), nil) + + ctx := setupTest(t) + + checker, err := NewLimitChecker(mockLimitRepo, mockUsageRepo, testutil.NewDefaultMockClock()) + require.NoError(t, err) + + input := &model.CheckLimitsInput{ + Amount: decimal.RequireFromString("50"), + Currency: "USD", + AccountID: accountID, + TransactionTimestamp: timestamp, + } + + mockDB := dbmocks.NewMockDB(ctrl) + + output, err := checker.CheckLimitsWithTx(ctx, mockDB, input) + + require.NoError(t, err) + require.NotNil(t, output) + assert.True(t, output.Allowed) +} diff --git a/internal/services/validation_service.go b/internal/services/validation_service.go index 9753afc2..ae7b5667 100644 --- a/internal/services/validation_service.go +++ b/internal/services/validation_service.go @@ -21,6 +21,7 @@ import ( libOpentelemetry "github.com/LerianStudio/lib-commons/v2/commons/opentelemetry" "tracer/internal/services/command" + "tracer/internal/services/query" "tracer/pkg/clock" "tracer/pkg/contextutil" "tracer/pkg/logging" @@ -39,12 +40,21 @@ const validationRollbackTimeout = 5 * time.Second // Sentinel errors for ValidationService constructor validation. var ( - ErrNilRuleEvaluator = errors.New("rule evaluator cannot be nil") - ErrNilLimitChecker = errors.New("limit checker cannot be nil") - ErrNilTransactionValidationRepo = errors.New("transaction validation repository cannot be nil") - ErrNilAuditWriter = errors.New("auditWriter cannot be nil") + ErrNilRuleEvaluator = errors.New("rule evaluator cannot be nil") + ErrNilLimitChecker = errors.New("limit checker cannot be nil") + ErrNilTransactionValidationRepo = errors.New("transaction validation repository cannot be nil") + ErrNilTransactionValidationQueryRepo = errors.New("transaction validation query repository cannot be nil") + ErrNilAuditWriter = errors.New("auditWriter cannot be nil") ) +// ValidateResult is the result of transaction validation including idempotency information. +// IsDuplicate is true when the same request_id was already processed - the handler uses +// this to return HTTP 200 (duplicate) vs HTTP 201 (new). +type ValidateResult struct { + Response *model.ValidationResponse + IsDuplicate bool +} + // RuleEvaluator evaluates transaction rules. type RuleEvaluator interface { Execute(ctx context.Context, req *model.ValidationRequest) (*model.EvaluationResult, error) @@ -56,13 +66,20 @@ type LimitChecker interface { RollbackUsage(ctx context.Context, input *model.CheckLimitsInput, usageDetails []model.LimitUsageDetail) error } +// TransactionValidationQueryRepository defines read operations for transaction validations. +// Used for idempotency checks via FindByRequestID. +type TransactionValidationQueryRepository interface { + FindByRequestID(ctx context.Context, requestID uuid.UUID) (*model.TransactionValidation, error) +} + // ValidationService orchestrates transaction validation. type ValidationService struct { - ruleEvaluator RuleEvaluator - limitChecker LimitChecker - transactionValidationRepo command.TransactionValidationRepository - auditWriter AuditWriter - clock clock.Clock + ruleEvaluator RuleEvaluator + limitChecker LimitChecker + transactionValidationRepo command.TransactionValidationRepository + transactionValidationQueryRepo query.TransactionValidationRepository + auditWriter AuditWriter + clock clock.Clock } // NewValidationService creates a new ValidationService with dependency validation. @@ -70,6 +87,7 @@ func NewValidationService( ruleEval RuleEvaluator, limitCheck LimitChecker, transactionValidationRepo command.TransactionValidationRepository, + transactionValidationQueryRepo query.TransactionValidationRepository, auditWriter AuditWriter, clk clock.Clock, ) (*ValidationService, error) { @@ -85,6 +103,10 @@ func NewValidationService( return nil, ErrNilTransactionValidationRepo } + if transactionValidationQueryRepo == nil { + return nil, ErrNilTransactionValidationQueryRepo + } + if auditWriter == nil { return nil, ErrNilAuditWriter } @@ -94,17 +116,19 @@ func NewValidationService( } return &ValidationService{ - ruleEvaluator: ruleEval, - limitChecker: limitCheck, - transactionValidationRepo: transactionValidationRepo, - auditWriter: auditWriter, - clock: clk, + ruleEvaluator: ruleEval, + limitChecker: limitCheck, + transactionValidationRepo: transactionValidationRepo, + transactionValidationQueryRepo: transactionValidationQueryRepo, + auditWriter: auditWriter, + clock: clk, }, nil } -// Validate orchestrates the transaction validation flow. +// Validate orchestrates the transaction validation flow with idempotency support. +// Returns ValidateResult with IsDuplicate=true for duplicate requests (DD-3: Stripe model). // Decision precedence: DENY > Limit Exceeded > REVIEW > ALLOW > Default. -func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationRequest) (*model.ValidationResponse, error) { +func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationRequest) (*ValidateResult, error) { // Check context cancellation FIRST if err := ctx.Err(); err != nil { return nil, err @@ -121,6 +145,31 @@ func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationR logger = logging.WithTrace(ctx, logger) + // Step 0: Check for duplicate request (DD-3: Stripe model deduplication) + // This is done BEFORE any processing to avoid double-counting limits. + existingValidation, err := s.transactionValidationQueryRepo.FindByRequestID(ctx, req.RequestID) + if err != nil { + libOpentelemetry.HandleSpanError(&span, "failed to check for duplicate request", err) + + return nil, fmt.Errorf("failed to check for duplicate request: %w", err) + } + + if existingValidation != nil { + // Duplicate detected - return cached response without processing + logger.WithFields( + "operation", "service.validation.orchestrate", + "request.id", req.RequestID, + "existing.validation.id", existingValidation.ID, + ).Info("Duplicate request detected - returning cached response") + + span.AddEvent("duplicate_request_detected") + + return &ValidateResult{ + Response: existingValidation.ToValidationResponse(), + IsDuplicate: true, + }, nil + } + startTime := time.Now() // Wall clock for latency measurement only evaluatedAt := s.clock.Now().UTC() @@ -168,7 +217,10 @@ func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationR "decision", "DENY", ).Info("Validation completed (by rule)") - return response, nil + return &ValidateResult{ + Response: response, + IsDuplicate: false, + }, nil } // Step 2: Check limits (only if not DENY by rule) @@ -205,7 +257,10 @@ func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationR "reason", "limit_exceeded", ).Info("Validation completed (limit exceeded)") - return response, nil + return &ValidateResult{ + Response: response, + IsDuplicate: false, + }, nil } // Step 3: If rules returned REVIEW, rollback usage increments @@ -269,7 +324,10 @@ func (s *ValidationService) Validate(ctx context.Context, req *model.ValidationR ).Info("Validation completed") } - return response, nil + return &ValidateResult{ + Response: response, + IsDuplicate: false, + }, nil } // persistTransactionValidation persists a transaction validation record synchronously. diff --git a/internal/services/validation_service_bench_test.go b/internal/services/validation_service_bench_test.go index 97e4e43d..fb3f26ce 100644 --- a/internal/services/validation_service_bench_test.go +++ b/internal/services/validation_service_bench_test.go @@ -15,6 +15,7 @@ import ( commandMocks "tracer/internal/services/command/mocks" "tracer/internal/services/mocks" + queryMocks "tracer/internal/services/query/mocks" "tracer/internal/testutil" "tracer/pkg/model" ) @@ -28,6 +29,7 @@ func BenchmarkValidationService_Validate(b *testing.B) { mockRuleEval := mocks.NewMockRuleEvaluator(ctrl) mockLimitCheck := mocks.NewMockLimitChecker(ctrl) mockAuditRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + mockAuditQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) // Setup mock responses evalResult, err := model.NewEvaluationResult( @@ -45,6 +47,12 @@ func BenchmarkValidationService_Validate(b *testing.B) { LimitUsageDetails: []model.LimitUsageDetail{}, } + // FindByRequestID returns nil (no existing record) - new request + mockAuditQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + AnyTimes() + mockRuleEval.EXPECT(). Execute(gomock.Any(), gomock.Any()). Return(evalResult, nil). @@ -66,7 +74,7 @@ func BenchmarkValidationService_Validate(b *testing.B) { Return(nil). AnyTimes() - service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditWriter, nil) + service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditQueryRepo, mockAuditWriter, nil) if err != nil { b.Fatal(err) } @@ -101,6 +109,7 @@ func BenchmarkValidationService_Validate_WithDenyRule(b *testing.B) { mockRuleEval := mocks.NewMockRuleEvaluator(ctrl) mockLimitCheck := mocks.NewMockLimitChecker(ctrl) mockAuditRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + mockAuditQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) // Setup mock to return DENY (should skip limit check) evalResult, err := model.NewEvaluationResult( @@ -113,6 +122,12 @@ func BenchmarkValidationService_Validate_WithDenyRule(b *testing.B) { b.Fatal(err) } + // FindByRequestID returns nil (no existing record) - new request + mockAuditQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + AnyTimes() + mockRuleEval.EXPECT(). Execute(gomock.Any(), gomock.Any()). Return(evalResult, nil). @@ -134,7 +149,7 @@ func BenchmarkValidationService_Validate_WithDenyRule(b *testing.B) { Return(nil). AnyTimes() - service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditWriter, nil) + service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditQueryRepo, mockAuditWriter, nil) if err != nil { b.Fatal(err) } @@ -169,6 +184,7 @@ func BenchmarkValidationService_Validate_Parallel(b *testing.B) { mockRuleEval := mocks.NewMockRuleEvaluator(ctrl) mockLimitCheck := mocks.NewMockLimitChecker(ctrl) mockAuditRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + mockAuditQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) evalResult, err := model.NewEvaluationResult( model.DecisionAllow, @@ -185,6 +201,12 @@ func BenchmarkValidationService_Validate_Parallel(b *testing.B) { LimitUsageDetails: []model.LimitUsageDetail{}, } + // FindByRequestID returns nil (no existing record) - new request + mockAuditQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + AnyTimes() + mockRuleEval.EXPECT(). Execute(gomock.Any(), gomock.Any()). Return(evalResult, nil). @@ -206,7 +228,7 @@ func BenchmarkValidationService_Validate_Parallel(b *testing.B) { Return(nil). AnyTimes() - service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditWriter, nil) + service, err := NewValidationService(mockRuleEval, mockLimitCheck, mockAuditRepo, mockAuditQueryRepo, mockAuditWriter, nil) if err != nil { b.Fatal(err) } diff --git a/internal/services/validation_service_idempotency_test.go b/internal/services/validation_service_idempotency_test.go new file mode 100644 index 00000000..6f5639d3 --- /dev/null +++ b/internal/services/validation_service_idempotency_test.go @@ -0,0 +1,604 @@ +// Copyright (c) 2026 Lerian Studio. All rights reserved. +// Use of this source code is governed by the Elastic License 2.0 +// that can be found in the LICENSE file. + +package services + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/shopspring/decimal" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + commandMocks "tracer/internal/services/command/mocks" + "tracer/internal/services/mocks" + queryMocks "tracer/internal/services/query/mocks" + "tracer/internal/testutil" + "tracer/pkg/model" +) + +// ============================================================================= +// Transactional Deduplication Tests +// ============================================================================= +// These tests verify the deduplication behavior for idempotent requests. +// Duplicate detection uses FindByRequestID to check if a request was already processed. +// ============================================================================= + +// TestValidate_DuplicateRequestID_ReturnsOriginal tests that when the same request_id +// is submitted twice, the second call returns the original response (DD-3: Stripe model). +// +// Expected behavior: +// - First call: processes validation, persists result, returns ValidateResult{Response, IsDuplicate: false} +// - Second call: finds existing record, returns ValidateResult{Response, IsDuplicate: true} +func TestValidate_DuplicateRequestID_ReturnsOriginal(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(1001) + accountID := testutil.MustDeterministicUUID(1002) + ruleID := testutil.MustDeterministicUUID(1010) + validationID := testutil.MustDeterministicUUID(1020) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("100"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + persistDone := make(chan struct{}) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // First call expectations - FindByRequestID returns nil (no existing record) + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(nil, nil). + Times(1) + + evalResult, err := model.NewEvaluationResult( + model.DecisionAllow, + []uuid.UUID{ruleID}, + []uuid.UUID{ruleID}, + "Transaction allowed", + ) + require.NoError(t, err) + + ruleEval.EXPECT(). + Execute(gomock.Any(), gomock.Any()). + Return(evalResult, nil). + Times(1) + + limitCheck.EXPECT(). + CheckLimits(gomock.Any(), gomock.Any()). + Return(&model.CheckLimitsOutput{Allowed: true, LimitUsageDetails: []model.LimitUsageDetail{}}, nil). + Times(1) + + transactionValidationRepo.EXPECT(). + Insert(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *model.TransactionValidation) error { + close(persistDone) + return nil + }). + Times(1) + + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + // First call + result1, err := service.Validate(context.Background(), request) + + // Wait for persistence to complete + select { + case <-persistDone: + case <-time.After(1 * time.Second): + t.Fatal("Timed out waiting for persistence to complete") + } + + require.NoError(t, err) + require.NotNil(t, result1) + require.NotNil(t, result1.Response) + assert.Equal(t, model.DecisionAllow, result1.Response.Decision) + assert.False(t, result1.IsDuplicate, "First call should NOT be marked as duplicate") + + // Second call expectations - FindByRequestID returns the existing record + existingTV, err := model.NewTransactionValidation(validationID, model.DecisionAllow, fixedTime) + require.NoError(t, err) + existingTV.RequestID = requestID + existingTV.TransactionType = model.TransactionTypeCard + existingTV.Amount = decimal.RequireFromString("100") + existingTV.Currency = "USD" + existingTV.TransactionTimestamp = fixedTime + existingTV.Account = model.AccountContext{ID: accountID} + existingTV.EvaluationResult = *evalResult + existingTV.LimitUsageDetails = []model.LimitUsageDetail{} + existingTV.ProcessingTimeMs = 15 + + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(existingTV, nil). + Times(1) + + // No other operations should be called for duplicates + // (No ruleEval.Execute, limitCheck.CheckLimits, transactionValidationRepo.Insert, auditWriter.RecordValidationEvent) + + // Second call + result2, err := service.Validate(context.Background(), request) + require.NoError(t, err) + require.NotNil(t, result2) + require.NotNil(t, result2.Response) + assert.Equal(t, validationID, result2.Response.ValidationID, "Duplicate should return same validation ID") + assert.True(t, result2.IsDuplicate, "Second call should be marked as duplicate") +} + +// TestValidate_DuplicateRequestID_NoDoubleCount tests that usage counters are NOT +// incremented when a duplicate request is detected (DD-3). +// +// Expected behavior: +// - When a duplicate request_id is detected, CheckLimits should NOT be called +// - This prevents double-counting the transaction against limits +func TestValidate_DuplicateRequestID_NoDoubleCount(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(2001) + accountID := testutil.MustDeterministicUUID(2002) + ruleID := testutil.MustDeterministicUUID(2010) + validationID := testutil.MustDeterministicUUID(2020) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("100"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // Prepare existing validation record + evalResult, err := model.NewEvaluationResult( + model.DecisionAllow, + []uuid.UUID{ruleID}, + []uuid.UUID{ruleID}, + "Transaction allowed", + ) + require.NoError(t, err) + + existingTV, err := model.NewTransactionValidation(validationID, model.DecisionAllow, fixedTime) + require.NoError(t, err) + existingTV.RequestID = requestID + existingTV.TransactionType = model.TransactionTypeCard + existingTV.Amount = decimal.RequireFromString("100") + existingTV.Currency = "USD" + existingTV.TransactionTimestamp = fixedTime + existingTV.Account = model.AccountContext{ID: accountID} + existingTV.EvaluationResult = *evalResult + existingTV.LimitUsageDetails = []model.LimitUsageDetail{} + existingTV.ProcessingTimeMs = 15 + + // EXPECTED BEHAVIOR: + // Service should first check if this request_id already exists in the database. + // If it does, it should return the cached response WITHOUT calling any other methods. + + // FindByRequestID returns existing record - duplicate detected + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(existingTV, nil). + Times(1) + + // CRITICAL: These should NOT be called for duplicates + limitCheck.EXPECT(). + CheckLimits(gomock.Any(), gomock.Any()). + Times(0) + + ruleEval.EXPECT(). + Execute(gomock.Any(), gomock.Any()). + Times(0) + + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Times(0) + + transactionValidationRepo.EXPECT(). + Insert(gomock.Any(), gomock.Any()). + Times(0) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + // Call with duplicate request + result, err := service.Validate(context.Background(), request) + + require.NoError(t, err) + require.NotNil(t, result) + require.NotNil(t, result.Response) + assert.True(t, result.IsDuplicate, "Duplicate request should be flagged") + assert.Equal(t, validationID, result.Response.ValidationID, "Should return original validation ID") +} + +// TestValidate_DuplicateRequestID_NoAudit tests that audit events are NOT created +// when a duplicate request is detected (DD-3). +// +// Expected behavior: +// - When a duplicate request_id is detected, audit should NOT be recorded +// - The original audit from the first request is sufficient +func TestValidate_DuplicateRequestID_NoAudit(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(3001) + accountID := testutil.MustDeterministicUUID(3002) + validationID := testutil.MustDeterministicUUID(3020) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("50"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // Prepare existing validation record + existingTV, err := model.NewTransactionValidation(validationID, model.DecisionAllow, fixedTime) + require.NoError(t, err) + existingTV.RequestID = requestID + existingTV.TransactionType = model.TransactionTypeCard + existingTV.Amount = decimal.RequireFromString("50") + existingTV.Currency = "USD" + existingTV.TransactionTimestamp = fixedTime + existingTV.Account = model.AccountContext{ID: accountID} + existingTV.MatchedRuleIDs = []uuid.UUID{} + existingTV.EvaluatedRuleIDs = []uuid.UUID{} + existingTV.LimitUsageDetails = []model.LimitUsageDetail{} + existingTV.ProcessingTimeMs = 10 + + // CRITICAL: For duplicate detection, FindByRequestID returns existing record + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(existingTV, nil). + Times(1) + + // Audit should NOT be called for duplicates + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Times(0) + + // No other operations should be called for duplicates + ruleEval.EXPECT().Execute(gomock.Any(), gomock.Any()).Times(0) + limitCheck.EXPECT().CheckLimits(gomock.Any(), gomock.Any()).Times(0) + transactionValidationRepo.EXPECT().Insert(gomock.Any(), gomock.Any()).Times(0) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + // Call with duplicate request + result, err := service.Validate(context.Background(), request) + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.IsDuplicate, "Duplicate request should be flagged") +} + +// TestValidate_DenyByRule_RetryTolerant tests that DENY-by-rule works correctly +// and checks for duplicates before processing. +func TestValidate_DenyByRule_RetryTolerant(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(4001) + accountID := testutil.MustDeterministicUUID(4002) + ruleID := testutil.MustDeterministicUUID(4010) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("1000"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + persistDone := make(chan struct{}) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(nil, nil). + Times(1) + + // Rule returns DENY + evalResult, err := model.NewEvaluationResult( + model.DecisionDeny, + []uuid.UUID{ruleID}, + []uuid.UUID{ruleID}, + "High-risk transaction blocked", + ) + require.NoError(t, err) + + ruleEval.EXPECT(). + Execute(gomock.Any(), gomock.Any()). + Return(evalResult, nil). + Times(1) + + // Limit check NOT called for DENY by rule + limitCheck.EXPECT().CheckLimits(gomock.Any(), gomock.Any()).Times(0) + + transactionValidationRepo.EXPECT(). + Insert(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *model.TransactionValidation) error { + close(persistDone) + return nil + }). + Times(1) + + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + result, err := service.Validate(context.Background(), request) + + select { + case <-persistDone: + case <-time.After(1 * time.Second): + t.Fatal("Timed out waiting for persistence to complete") + } + + require.NoError(t, err) + require.NotNil(t, result) + require.NotNil(t, result.Response) + assert.Equal(t, model.DecisionDeny, result.Response.Decision) + assert.False(t, result.IsDuplicate) +} + +// TestValidate_DenyByLimit_RollbackAtomic tests that limits are handled correctly +// when DENY-by-limit occurs. +func TestValidate_DenyByLimit_RollbackAtomic(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(5001) + accountID := testutil.MustDeterministicUUID(5002) + ruleID := testutil.MustDeterministicUUID(5010) + limitID := testutil.MustDeterministicUUID(5020) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("500"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + persistDone := make(chan struct{}) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(nil, nil). + Times(1) + + // Rule returns ALLOW + evalResult, err := model.NewEvaluationResult( + model.DecisionAllow, + []uuid.UUID{ruleID}, + []uuid.UUID{ruleID}, + "Transaction allowed", + ) + require.NoError(t, err) + + ruleEval.EXPECT(). + Execute(gomock.Any(), gomock.Any()). + Return(evalResult, nil). + Times(1) + + // Limit check returns EXCEEDED + limitOutput := &model.CheckLimitsOutput{ + Allowed: false, + LimitUsageDetails: []model.LimitUsageDetail{ + { + LimitID: limitID, + LimitAmount: decimal.RequireFromString("400"), + CurrentUsage: decimal.RequireFromString("450"), + Exceeded: true, + Period: model.LimitTypeDaily, + }, + }, + ExceededLimitIDs: []uuid.UUID{limitID}, + } + + limitCheck.EXPECT(). + CheckLimits(gomock.Any(), gomock.Any()). + Return(limitOutput, nil). + Times(1) + + transactionValidationRepo.EXPECT(). + Insert(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *model.TransactionValidation) error { + close(persistDone) + return nil + }). + Times(1) + + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + result, err := service.Validate(context.Background(), request) + + select { + case <-persistDone: + case <-time.After(1 * time.Second): + t.Fatal("Timed out waiting for persistence to complete") + } + + require.NoError(t, err) + require.NotNil(t, result) + require.NotNil(t, result.Response) + assert.Equal(t, model.DecisionDeny, result.Response.Decision) + assert.Equal(t, "limit_exceeded", result.Response.Reason) + assert.False(t, result.IsDuplicate) +} + +// TestValidate_Review_RollbackAtomic tests that limits are rolled back +// when REVIEW decision is made. +func TestValidate_Review_RollbackAtomic(t *testing.T) { + t.Parallel() + + fixedTime := time.Date(2025, 1, 15, 10, 0, 0, 0, time.UTC) + requestID := testutil.MustDeterministicUUID(6001) + accountID := testutil.MustDeterministicUUID(6002) + ruleID := testutil.MustDeterministicUUID(6010) + limitID := testutil.MustDeterministicUUID(6020) + + request := &model.ValidationRequest{ + RequestID: requestID, + TransactionType: model.TransactionTypeCard, + Amount: decimal.RequireFromString("200"), + Currency: "USD", + TransactionTimestamp: fixedTime, + Account: model.AccountContext{ID: accountID}, + } + + ctrl := gomock.NewController(t) + persistDone := make(chan struct{}) + + ruleEval := mocks.NewMockRuleEvaluator(ctrl) + limitCheck := mocks.NewMockLimitChecker(ctrl) + transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + auditWriter := mocks.NewMockAuditWriter(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), requestID). + Return(nil, nil). + Times(1) + + // Rule returns REVIEW + evalResult, err := model.NewEvaluationResult( + model.DecisionReview, + []uuid.UUID{ruleID}, + []uuid.UUID{ruleID}, + "Manual review required", + ) + require.NoError(t, err) + + ruleEval.EXPECT(). + Execute(gomock.Any(), gomock.Any()). + Return(evalResult, nil). + Times(1) + + // Limit check passes + limitOutput := &model.CheckLimitsOutput{ + Allowed: true, + LimitUsageDetails: []model.LimitUsageDetail{ + { + LimitID: limitID, + LimitAmount: decimal.RequireFromString("1000"), + CurrentUsage: decimal.RequireFromString("200"), + Exceeded: false, + Period: model.LimitTypeDaily, + InternalLimitType: model.LimitTypeDaily, + InternalPeriodKey: "2025-01-15", + }, + }, + ExceededLimitIDs: []uuid.UUID{}, + } + + limitCheck.EXPECT(). + CheckLimits(gomock.Any(), gomock.Any()). + Return(limitOutput, nil). + Times(1) + + // RollbackUsage is called for REVIEW decisions + limitCheck.EXPECT(). + RollbackUsage(gomock.Any(), gomock.Any(), gomock.Eq(limitOutput.LimitUsageDetails)). + Return(nil). + Times(1) + + transactionValidationRepo.EXPECT(). + Insert(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, _ *model.TransactionValidation) error { + close(persistDone) + return nil + }). + Times(1) + + auditWriter.EXPECT(). + RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil). + Times(1) + + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) + require.NoError(t, err) + + result, err := service.Validate(context.Background(), request) + + select { + case <-persistDone: + case <-time.After(1 * time.Second): + t.Fatal("Timed out waiting for persistence to complete") + } + + require.NoError(t, err) + require.NotNil(t, result) + require.NotNil(t, result.Response) + assert.Equal(t, model.DecisionReview, result.Response.Decision) + assert.False(t, result.IsDuplicate) +} diff --git a/internal/services/validation_service_test.go b/internal/services/validation_service_test.go index c0177da2..84fd115f 100644 --- a/internal/services/validation_service_test.go +++ b/internal/services/validation_service_test.go @@ -19,6 +19,8 @@ import ( "tracer/internal/services/command" commandMocks "tracer/internal/services/command/mocks" "tracer/internal/services/mocks" + "tracer/internal/services/query" + queryMocks "tracer/internal/services/query/mocks" "tracer/internal/testutil" "tracer/pkg/model" ) @@ -44,7 +46,7 @@ func TestValidateTransaction(t *testing.T) { tests := []struct { name string request *model.ValidationRequest - setupMocks func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) + setupMocks func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) expectedDecision model.Decision expectedReason string expectError bool @@ -54,10 +56,17 @@ func TestValidateTransaction(t *testing.T) { { name: "DENY by rule - rule evaluation returns DENY", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -85,7 +94,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionDeny, expectedReason: "Rule blocked transaction", @@ -94,10 +103,17 @@ func TestValidateTransaction(t *testing.T) { { name: "DENY by exceeded limit - limit check returns exceeded", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -141,7 +157,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionDeny, expectedReason: "limit_exceeded", @@ -150,10 +166,17 @@ func TestValidateTransaction(t *testing.T) { { name: "REVIEW when REVIEW rules match - no DENY", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -208,7 +231,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionReview, expectedReason: "Rule requires review", @@ -217,10 +240,17 @@ func TestValidateTransaction(t *testing.T) { { name: "REVIEW rollback failure is non-fatal", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -275,7 +305,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionReview, expectedReason: "Rule requires review", @@ -284,10 +314,17 @@ func TestValidateTransaction(t *testing.T) { { name: "DENY by limit takes precedence over REVIEW", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -331,7 +368,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionDeny, expectedReason: "limit_exceeded", @@ -340,10 +377,17 @@ func TestValidateTransaction(t *testing.T) { { name: "ALLOW with matched ALLOW rules", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -387,7 +431,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionAllow, expectedReason: "Rule allowed transaction", @@ -396,10 +440,17 @@ func TestValidateTransaction(t *testing.T) { { name: "ALLOW with default decision - no rules matched", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, persistDone chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -429,7 +480,7 @@ func TestValidateTransaction(t *testing.T) { return nil }) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectedDecision: model.DecisionAllow, expectedReason: "No matching rules found", @@ -438,21 +489,23 @@ func TestValidateTransaction(t *testing.T) { { name: "context cancellation at start - returns error immediately", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) - // AuditWriter mock - expects RecordValidationEvent call + // AuditWriter mock auditWriter := mocks.NewMockAuditWriter(ctrl) - auditWriter.EXPECT().RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(0) - // No calls should be made when context is cancelled + // No calls should be made when context is cancelled (checked before FindByRequestID) + transactionValidationQueryRepo.EXPECT().FindByRequestID(gomock.Any(), gomock.Any()).Times(0) ruleEval.EXPECT().Execute(gomock.Any(), gomock.Any()).Times(0) limitCheck.EXPECT().CheckLimits(gomock.Any(), gomock.Any()).Times(0) transactionValidationRepo.EXPECT().Insert(gomock.Any(), gomock.Any()).Times(0) + auditWriter.EXPECT().RecordValidationEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectError: true, expectedErr: context.Canceled, @@ -461,10 +514,17 @@ func TestValidateTransaction(t *testing.T) { { name: "error from rule evaluator - propagates error", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -481,17 +541,24 @@ func TestValidateTransaction(t *testing.T) { // Audit should NOT be inserted on error transactionValidationRepo.EXPECT().Insert(gomock.Any(), gomock.Any()).Times(0) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectError: true, }, { name: "error from limit checker - propagates error", request: baseRequest, - setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, AuditWriter) { + setupMocks: func(ctrl *gomock.Controller, _ chan struct{}) (RuleEvaluator, LimitChecker, command.TransactionValidationRepository, query.TransactionValidationRepository, AuditWriter) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -517,7 +584,7 @@ func TestValidateTransaction(t *testing.T) { // Audit should NOT be inserted on error transactionValidationRepo.EXPECT().Insert(gomock.Any(), gomock.Any()).Times(0) - return ruleEval, limitCheck, transactionValidationRepo, auditWriter + return ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter }, expectError: true, }, @@ -530,9 +597,9 @@ func TestValidateTransaction(t *testing.T) { // Create channel to signal when audit is done (for deterministic waiting) persistDone := make(chan struct{}) - ruleEval, limitCheck, transactionValidationRepo, auditWriter := tt.setupMocks(ctrl, persistDone) + ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter := tt.setupMocks(ctrl, persistDone) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Create context - cancelled for context cancellation test @@ -567,8 +634,8 @@ func TestValidateTransaction(t *testing.T) { } else { require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, tt.expectedDecision, result.Decision) - assert.Contains(t, result.Reason, tt.expectedReason) + assert.Equal(t, tt.expectedDecision, result.Response.Decision) + assert.Contains(t, result.Response.Reason, tt.expectedReason) } }) } @@ -615,6 +682,13 @@ func TestValidateTransaction_AuditFieldsPopulated(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -660,7 +734,7 @@ func TestValidateTransaction_AuditFieldsPopulated(t *testing.T) { return nil }) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -725,61 +799,77 @@ func TestNewValidationService_NilDependencies(t *testing.T) { validRuleEval := mocks.NewMockRuleEvaluator(ctrl) validLimitCheck := mocks.NewMockLimitChecker(ctrl) validTransactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + validTransactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) validAuditWriter := mocks.NewMockAuditWriter(ctrl) tests := []struct { - name string - ruleEval RuleEvaluator - limitCheck LimitChecker - transactionValidationRepo command.TransactionValidationRepository - auditWriter AuditWriter - expectedErr error + name string + ruleEval RuleEvaluator + limitCheck LimitChecker + transactionValidationRepo command.TransactionValidationRepository + transactionValidationQueryRepo query.TransactionValidationRepository + auditWriter AuditWriter + expectedErr error }{ { - name: "nil rule evaluator", - ruleEval: nil, - limitCheck: validLimitCheck, - transactionValidationRepo: validTransactionValidationRepo, - auditWriter: validAuditWriter, - expectedErr: ErrNilRuleEvaluator, + name: "nil rule evaluator", + ruleEval: nil, + limitCheck: validLimitCheck, + transactionValidationRepo: validTransactionValidationRepo, + transactionValidationQueryRepo: validTransactionValidationQueryRepo, + auditWriter: validAuditWriter, + expectedErr: ErrNilRuleEvaluator, }, { - name: "nil limit checker", - ruleEval: validRuleEval, - limitCheck: nil, - transactionValidationRepo: validTransactionValidationRepo, - auditWriter: validAuditWriter, - expectedErr: ErrNilLimitChecker, + name: "nil limit checker", + ruleEval: validRuleEval, + limitCheck: nil, + transactionValidationRepo: validTransactionValidationRepo, + transactionValidationQueryRepo: validTransactionValidationQueryRepo, + auditWriter: validAuditWriter, + expectedErr: ErrNilLimitChecker, }, { - name: "nil transaction validation repository", - ruleEval: validRuleEval, - limitCheck: validLimitCheck, - transactionValidationRepo: nil, - auditWriter: validAuditWriter, - expectedErr: ErrNilTransactionValidationRepo, + name: "nil transaction validation repository", + ruleEval: validRuleEval, + limitCheck: validLimitCheck, + transactionValidationRepo: nil, + transactionValidationQueryRepo: validTransactionValidationQueryRepo, + auditWriter: validAuditWriter, + expectedErr: ErrNilTransactionValidationRepo, }, { - name: "nil audit writer", - ruleEval: validRuleEval, - limitCheck: validLimitCheck, - transactionValidationRepo: validTransactionValidationRepo, - auditWriter: nil, - expectedErr: ErrNilAuditWriter, + name: "nil transaction validation query repository", + ruleEval: validRuleEval, + limitCheck: validLimitCheck, + transactionValidationRepo: validTransactionValidationRepo, + transactionValidationQueryRepo: nil, + auditWriter: validAuditWriter, + expectedErr: ErrNilTransactionValidationQueryRepo, }, { - name: "all valid dependencies", - ruleEval: validRuleEval, - limitCheck: validLimitCheck, - transactionValidationRepo: validTransactionValidationRepo, - auditWriter: validAuditWriter, - expectedErr: nil, + name: "nil audit writer", + ruleEval: validRuleEval, + limitCheck: validLimitCheck, + transactionValidationRepo: validTransactionValidationRepo, + transactionValidationQueryRepo: validTransactionValidationQueryRepo, + auditWriter: nil, + expectedErr: ErrNilAuditWriter, + }, + { + name: "all valid dependencies", + ruleEval: validRuleEval, + limitCheck: validLimitCheck, + transactionValidationRepo: validTransactionValidationRepo, + transactionValidationQueryRepo: validTransactionValidationQueryRepo, + auditWriter: validAuditWriter, + expectedErr: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - service, err := NewValidationService(tt.ruleEval, tt.limitCheck, tt.transactionValidationRepo, tt.auditWriter, nil) + service, err := NewValidationService(tt.ruleEval, tt.limitCheck, tt.transactionValidationRepo, tt.transactionValidationQueryRepo, tt.auditWriter, nil) if tt.expectedErr != nil { require.Error(t, err) @@ -1148,6 +1238,13 @@ func TestValidate_TransactionValidationPersistenceSuccess(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -1181,7 +1278,7 @@ func TestValidate_TransactionValidationPersistenceSuccess(t *testing.T) { return nil }) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1190,7 +1287,7 @@ func TestValidate_TransactionValidationPersistenceSuccess(t *testing.T) { // Assert: Validation succeeds - the client gets the result regardless of internal logging require.NoError(t, err, "Validation should succeed") require.NotNil(t, result) - assert.Equal(t, model.DecisionAllow, result.Decision) + assert.Equal(t, model.DecisionAllow, result.Response.Decision) // Note: We can't easily verify the log output in this test without injecting a mock logger. // The important behavior is that the validation result is returned correctly. @@ -1221,6 +1318,13 @@ func TestValidate_AuditPersistFailure_LogsError(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -1255,7 +1359,7 @@ func TestValidate_AuditPersistFailure_LogsError(t *testing.T) { return errors.New("database connection failed") }) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1264,7 +1368,7 @@ func TestValidate_AuditPersistFailure_LogsError(t *testing.T) { // Assert: Validation succeeds despite audit failure require.NoError(t, err, "Validation should succeed even if audit fails") require.NotNil(t, result) - assert.Equal(t, model.DecisionAllow, result.Decision) + assert.Equal(t, model.DecisionAllow, result.Response.Decision) // Wait for async audit goroutine to complete select { @@ -1314,6 +1418,13 @@ func TestValidate_WithSegmentAndPortfolio(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) // AuditWriter mock - expects RecordValidationEvent call with segment and portfolio auditWriter := mocks.NewMockAuditWriter(ctrl) @@ -1380,7 +1491,7 @@ func TestValidate_WithSegmentAndPortfolio(t *testing.T) { return nil }) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1396,7 +1507,7 @@ func TestValidate_WithSegmentAndPortfolio(t *testing.T) { // Assert require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, model.DecisionAllow, result.Decision) + assert.Equal(t, model.DecisionAllow, result.Response.Decision) } // TestValidate_NilRequest verifies that Validate returns an error when called with nil request. @@ -1406,11 +1517,12 @@ func TestValidate_NilRequest(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) auditWriter := mocks.NewMockAuditWriter(ctrl) - // No mock expectations - function should return early + // No mock expectations - function should return early (nil check before FindByRequestID) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1442,6 +1554,13 @@ func TestValidate_RuleEvaluatorReturnsNil(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) auditWriter := mocks.NewMockAuditWriter(ctrl) // Rule evaluation returns nil result (but no error) @@ -1451,7 +1570,7 @@ func TestValidate_RuleEvaluatorReturnsNil(t *testing.T) { // No limit check or audit expected - should fail early - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1484,6 +1603,13 @@ func TestValidate_LimitCheckerReturnsNil(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) auditWriter := mocks.NewMockAuditWriter(ctrl) // Rule evaluation returns ALLOW @@ -1505,7 +1631,7 @@ func TestValidate_LimitCheckerReturnsNil(t *testing.T) { // No audit expected - should fail early - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1539,6 +1665,13 @@ func TestValidate_AuditEventWriterFailure(t *testing.T) { ruleEval := mocks.NewMockRuleEvaluator(ctrl) limitCheck := mocks.NewMockLimitChecker(ctrl) transactionValidationRepo := commandMocks.NewMockTransactionValidationRepository(ctrl) + transactionValidationQueryRepo := queryMocks.NewMockTransactionValidationRepository(ctrl) + + // FindByRequestID returns nil (no existing record) - new request + transactionValidationQueryRepo.EXPECT(). + FindByRequestID(gomock.Any(), gomock.Any()). + Return(nil, nil). + Times(1) auditWriter := mocks.NewMockAuditWriter(ctrl) // AuditWriter returns error @@ -1574,7 +1707,7 @@ func TestValidate_AuditEventWriterFailure(t *testing.T) { return nil }) - service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, auditWriter, nil) + service, err := NewValidationService(ruleEval, limitCheck, transactionValidationRepo, transactionValidationQueryRepo, auditWriter, nil) require.NoError(t, err) // Act @@ -1590,5 +1723,5 @@ func TestValidate_AuditEventWriterFailure(t *testing.T) { // Assert: Validation succeeds despite audit writer failure (best-effort audit) require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, model.DecisionAllow, result.Decision) + assert.Equal(t, model.DecisionAllow, result.Response.Decision) } diff --git a/pkg/hash/hash.go b/pkg/hash/hash.go new file mode 100644 index 00000000..2aaffb22 --- /dev/null +++ b/pkg/hash/hash.go @@ -0,0 +1,30 @@ +// Copyright (c) 2026 Lerian Studio. All rights reserved. +// Use of this source code is governed by the Elastic License 2.0 +// that can be found in the LICENSE file. + +// Package hash provides utility functions for hashing UUIDs to integer values. +// Used for PostgreSQL advisory locks in the idempotency/deduplication flow. +package hash + +import ( + "hash/fnv" + + "github.com/google/uuid" +) + +// HashUUIDToInt32 converts a UUID to an int32 hash using FNV-1a algorithm. +// This is used for PostgreSQL advisory lock keys (pg_advisory_xact_lock). +// The hash is deterministic - the same UUID always produces the same int32. +// +// FNV-1a is chosen because: +// - Fast computation (critical for high-throughput validation) +// - Good distribution across the int32 range +// - Deterministic (no external state) +// - Built into Go standard library +func HashUUIDToInt32(id uuid.UUID) int32 { + h := fnv.New32a() + // hash.Hash.Write never returns an error per Go documentation + _, _ = h.Write(id[:]) + + return int32(h.Sum32()) // #nosec G115 - deliberate wrap-around: advisory locks use full int32 range +} diff --git a/pkg/hash/hash_test.go b/pkg/hash/hash_test.go new file mode 100644 index 00000000..c441291c --- /dev/null +++ b/pkg/hash/hash_test.go @@ -0,0 +1,199 @@ +// Copyright (c) 2026 Lerian Studio. All rights reserved. +// Use of this source code is governed by the Elastic License 2.0 +// that can be found in the LICENSE file. + +package hash + +import ( + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ============================================================================= +// HashUUIDToInt32 Utility for Advisory Locks Tests +// ============================================================================= +// These tests define the behavior for HashUUIDToInt32 which is used to generate +// PostgreSQL advisory lock keys from request_id UUIDs (DD-8). +// ============================================================================= + +// TestHashUUIDToInt32_Deterministic verifies that the same UUID always produces +// the same int32 hash value. This is critical for advisory locks - concurrent +// requests with the same request_id MUST acquire the same lock. +// +// This test will FAIL to compile because HashUUIDToInt32 doesn't exist yet. +func TestHashUUIDToInt32_Deterministic(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + uuid uuid.UUID + }{ + { + name: "nil UUID", + uuid: uuid.Nil, + }, + { + name: "random UUID 1", + uuid: uuid.MustParse("550e8400-e29b-41d4-a716-446655440000"), + }, + { + name: "random UUID 2", + uuid: uuid.MustParse("6ba7b810-9dad-11d1-80b4-00c04fd430c8"), + }, + { + name: "max UUID", + uuid: uuid.MustParse("ffffffff-ffff-ffff-ffff-ffffffffffff"), + }, + { + name: "sequential UUID pattern", + uuid: uuid.MustParse("00000000-0000-0000-0000-000000000001"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Call HashUUIDToInt32 multiple times with the same input + // All results should be identical + result1 := HashUUIDToInt32(tt.uuid) + result2 := HashUUIDToInt32(tt.uuid) + result3 := HashUUIDToInt32(tt.uuid) + + assert.Equal(t, result1, result2, "Hash should be deterministic (call 1 vs 2)") + assert.Equal(t, result2, result3, "Hash should be deterministic (call 2 vs 3)") + + // Additional validation: result should be in valid int32 range + // (Go's int32 is already bounded, but explicit check for documentation) + require.True(t, result1 >= -2147483648 && result1 <= 2147483647, + "Result should be valid int32") + }) + } +} + +// TestHashUUIDToInt32_DifferentInputs verifies that different UUIDs produce +// different hash values (with high probability). This ensures that advisory +// locks for different request_ids don't collide. +// +// This test will FAIL to compile because HashUUIDToInt32 doesn't exist yet. +func TestHashUUIDToInt32_DifferentInputs(t *testing.T) { + t.Parallel() + + // Generate a set of unique UUIDs + uuids := []uuid.UUID{ + uuid.MustParse("00000000-0000-0000-0000-000000000001"), + uuid.MustParse("00000000-0000-0000-0000-000000000002"), + uuid.MustParse("550e8400-e29b-41d4-a716-446655440000"), + uuid.MustParse("6ba7b810-9dad-11d1-80b4-00c04fd430c8"), + uuid.MustParse("f47ac10b-58cc-4372-a567-0e02b2c3d479"), + uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"), + uuid.MustParse("11111111-2222-3333-4444-555555555555"), + uuid.MustParse("deadbeef-cafe-babe-dead-beefcafebabe"), + } + + // Hash all UUIDs + hashes := make(map[int32]uuid.UUID) + + for _, id := range uuids { + hash := HashUUIDToInt32(id) + + // Check for collision + if existing, found := hashes[hash]; found { + // In real-world, FNV-1a should not collide for these test cases + // If it does, the test will fail and we need to investigate + t.Errorf("Hash collision detected: UUID %s and UUID %s both hash to %d", + existing.String(), id.String(), hash) + } + + hashes[hash] = id + } + + // Verify we got unique hashes for all inputs + assert.Len(t, hashes, len(uuids), "All UUIDs should produce unique hashes (no collisions)") +} + +// TestHashUUIDToInt32_BoundaryValues tests edge cases and boundary values +// to ensure the hash function handles them correctly. +// +// This test will FAIL to compile because HashUUIDToInt32 doesn't exist yet. +func TestHashUUIDToInt32_BoundaryValues(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + uuid uuid.UUID + }{ + { + name: "nil UUID (all zeros)", + uuid: uuid.Nil, + }, + { + name: "max UUID (all ones)", + uuid: uuid.MustParse("ffffffff-ffff-ffff-ffff-ffffffffffff"), + }, + { + name: "alternating bits pattern 1", + uuid: uuid.MustParse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"), + }, + { + name: "alternating bits pattern 2", + uuid: uuid.MustParse("55555555-5555-5555-5555-555555555555"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Should not panic for any valid UUID + result := HashUUIDToInt32(tt.uuid) + + // Result should be deterministic + assert.Equal(t, result, HashUUIDToInt32(tt.uuid), + "Hash should be deterministic for boundary values") + }) + } +} + +// TestHashUUIDToInt32_Distribution tests that the hash function produces +// a reasonable distribution of values across the int32 range. +// This is important for advisory lock performance - we don't want all locks +// to cluster in a small range. +// +// This test will FAIL to compile because HashUUIDToInt32 doesn't exist yet. +func TestHashUUIDToInt32_Distribution(t *testing.T) { + t.Parallel() + + // Generate many random UUIDs and check distribution + const sampleSize = 1000 + positiveCount := 0 + negativeCount := 0 + + for i := 0; i < sampleSize; i++ { + id := uuid.New() + hash := HashUUIDToInt32(id) + + if hash >= 0 { + positiveCount++ + } else { + negativeCount++ + } + } + + // FNV-1a should produce a roughly even distribution across positive/negative + // Allow for some variance (40%-60% split is reasonable) + minExpected := sampleSize * 40 / 100 + maxExpected := sampleSize * 60 / 100 + + assert.GreaterOrEqual(t, positiveCount, minExpected, + "Distribution should have at least 40%% positive values") + assert.LessOrEqual(t, positiveCount, maxExpected, + "Distribution should have at most 60%% positive values") + assert.GreaterOrEqual(t, negativeCount, minExpected, + "Distribution should have at least 40%% negative values") + assert.LessOrEqual(t, negativeCount, maxExpected, + "Distribution should have at most 60%% negative values") +} diff --git a/pkg/model/transaction_validation.go b/pkg/model/transaction_validation.go index e348cdd4..1bf28c4c 100644 --- a/pkg/model/transaction_validation.go +++ b/pkg/model/transaction_validation.go @@ -81,9 +81,18 @@ func (tv *TransactionValidation) ToValidationResponse() *ValidationResponse { return nil } - // Defensive copy for LimitUsageDetails slice to prevent external mutation + // Defensive copy for LimitUsageDetails slice to prevent external mutation. + // Deep copies nested Scopes slice; pointer fields within Scope (*uuid.UUID, *string) + // are shallow-copied since the pointed-to values are immutable by convention. limitDetailsCopy := make([]LimitUsageDetail, len(tv.LimitUsageDetails)) - copy(limitDetailsCopy, tv.LimitUsageDetails) + for i, detail := range tv.LimitUsageDetails { + limitDetailsCopy[i] = detail + if detail.Scopes != nil { + scopesCopy := make([]Scope, len(detail.Scopes)) + copy(scopesCopy, detail.Scopes) + limitDetailsCopy[i].Scopes = scopesCopy + } + } matchedCopy := make([]uuid.UUID, len(tv.MatchedRuleIDs)) copy(matchedCopy, tv.MatchedRuleIDs) diff --git a/pkg/model/transaction_validation_response_test.go b/pkg/model/transaction_validation_response_test.go index 8cfe9e77..9dd9c262 100644 --- a/pkg/model/transaction_validation_response_test.go +++ b/pkg/model/transaction_validation_response_test.go @@ -220,6 +220,43 @@ func TestTransactionValidation_ToValidationResponse_DefensiveCopy(t *testing.T) assert.False(t, tv.LimitUsageDetails[0].Exceeded, "original LimitUsageDetails mutated via response") } +// TestTransactionValidation_ToValidationResponse_DefensiveCopy_Scopes verifies that +// mutating the nested Scopes slice in the response does not affect the original entity. +func TestTransactionValidation_ToValidationResponse_DefensiveCopy_Scopes(t *testing.T) { + t.Parallel() + + originalAccountID := uuid.MustParse("550e8400-e29b-41d4-a716-446655440001") + originalLimitID := uuid.MustParse("550e8400-e29b-41d4-a716-446655440030") + + tv := &TransactionValidation{ + ID: uuid.MustParse("550e8400-e29b-41d4-a716-446655440010"), + RequestID: uuid.MustParse("550e8400-e29b-41d4-a716-446655440020"), + EvaluationResult: EvaluationResult{ + Decision: DecisionAllow, + }, + LimitUsageDetails: []LimitUsageDetail{ + { + LimitID: originalLimitID, + Scopes: []Scope{ + {AccountID: &originalAccountID}, + }, + }, + }, + CreatedAt: time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC), + } + + resp := tv.ToValidationResponse() + + // Mutate response's Scopes slice (append) + resp.LimitUsageDetails[0].Scopes = append(resp.LimitUsageDetails[0].Scopes, Scope{}) + + // Original must remain unchanged + require.Len(t, tv.LimitUsageDetails[0].Scopes, 1, + "original Scopes length mutated via response append") + assert.Equal(t, originalAccountID, *tv.LimitUsageDetails[0].Scopes[0].AccountID, + "original Scopes content mutated via response") +} + // TestTransactionValidation_ToValidationResponse_NilReceiver verifies nil-safety. // This is critical because FindByRequestID returns (nil, nil) for not-found cases, // and callers might chain .ToValidationResponse() on the result. diff --git a/pkg/model/validation.go b/pkg/model/validation.go index f0135b7f..a7951ad3 100644 --- a/pkg/model/validation.go +++ b/pkg/model/validation.go @@ -213,13 +213,11 @@ func (r *ValidationRequest) NormalizeAndValidate(now time.Time) error { // LimitUsageDetail contains usage information for a checked limit. // Amounts are expressed as decimal values. // Note: RemainingAmount is calculated as (LimitAmount - CurrentUsage), not stored. -// Aligned with section 4.1.1 LimitUsage structure. type LimitUsageDetail struct { LimitID uuid.UUID `json:"limitId" swaggertype:"string" format:"uuid"` LimitAmount decimal.Decimal `json:"limitAmount" swaggertype:"string" example:"1000.00"` // Scope is a human-readable string representation of the limit's scope // (e.g., "account:uuid" or "segment:uuid" or "global"). - // Per section 4.1.1. Scope string `json:"scope"` // Period indicates the type of limit (DAILY, WEEKLY, MONTHLY, CUSTOM, PER_TRANSACTION). Period LimitType `json:"period" swaggertype:"string"` diff --git a/tests/integration/01_validation_test.go b/tests/integration/01_validation_test.go index 66a238a9..95092d4b 100644 --- a/tests/integration/01_validation_test.go +++ b/tests/integration/01_validation_test.go @@ -2447,7 +2447,7 @@ func TestValidation_1_1_55_DeactivatedRuleNotEvaluated(t *testing.T) { // Test 1.2.1: Retrieves validation by ID func TestValidation_1_2_1_RetrievesValidationByID(t *testing.T) { accountID := testutil.MustDeterministicUUID(400).String() - // Seed 12001 avoids collision with seed 401 used in TestValidation_CompletePayload, + // Seed 12001 avoids collision with seed 104 used in TestValidation_CompletePayload, // which shares the same request_id column now enforced as UNIQUE by migration 000009. requestID := testutil.MustDeterministicUUID(12001).String()