Skip to content

Commit 0e04217

Browse files
authored
refactor: Remove platform_ref inheritance from saga definitions (#2141)
* refactor: Remove platform_ref inheritance from saga definitions Delete the platform_ref inheritance system including override API, platform sync, similarity matching, version pinning, and fallback resolution. Simplify Definition struct by removing PlatformRef, OverrideReason, PlatformVersionAtOverride, ResolvedScript, and UsedPlatformFallback fields. Script is now required directly on all saga definitions. - Delete 12 files (override_api, similarity, migrate_platform_ref, fallback_resolution, platform_sync, version_pinning + tests) - Remove platform_ref columns from SQL queries and scan functions - Simplify CreateDraft INSERT and activation validation - Move ComputeScriptHash to registry.go, humanizeName/regex to seeder.go - Update orchestrators to use Script instead of ResolvedScript - Remove PlatformSagaVersionID from shared SagaInstance model - Add migration to drop platform_ref columns and set script NOT NULL - Update test DDL to match simplified schema * fix: Address CodeRabbit review feedback - Add NULL script backfill guard before SET NOT NULL in migration - Add empty script guard in ValidateSagaDraft handler - Add comment explaining why remove_platform_ref migration is excluded from setupPlatformTestDB (targets tenant tables, not public schema) * fix: Reject empty-script updates and include migration in test setup - Add handler-level guard rejecting explicit empty-script updates in UpdateSagaDefinition (InvalidArgument if script is set to "") - Create saga_definition with old schema in setupPlatformTestDB so the remove_platform_ref migration exercises the full migration chain --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent b3b0bd5 commit 0e04217

31 files changed

Lines changed: 250 additions & 4593 deletions

services/current-account/service/coverage_unit_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,12 +2026,12 @@ func TestResolveWithdrawalScript_SagaNotFound(t *testing.T) {
20262026
_ = script
20272027
}
20282028

2029-
func TestResolveDepositScript_ResolvedScriptEmpty(t *testing.T) {
2030-
// Test path where resolver returns a definition but with empty ResolvedScript
2029+
func TestResolveDepositScript_ScriptEmpty(t *testing.T) {
2030+
// Test path where resolver returns a definition but with empty Script
20312031
mockRegistry := &stubSagaRegistry{
20322032
getActiveResult: &refsaga.Definition{
2033-
Name: "deposit",
2034-
ResolvedScript: "", // Empty - should fall back
2033+
Name: "deposit",
2034+
Script: "", // Empty - should fall back
20352035
},
20362036
}
20372037

@@ -2061,11 +2061,11 @@ func TestResolveDepositScript_ResolvedScriptEmpty(t *testing.T) {
20612061
assert.Equal(t, "default_deposit()", script)
20622062
}
20632063

2064-
func TestResolveDepositScript_ResolvedScriptPresent(t *testing.T) {
2064+
func TestResolveDepositScript_ScriptPresent(t *testing.T) {
20652065
mockRegistry := &stubSagaRegistry{
20662066
getActiveResult: &refsaga.Definition{
2067-
Name: "deposit",
2068-
ResolvedScript: "custom_deposit()",
2067+
Name: "deposit",
2068+
Script: "custom_deposit()",
20692069
},
20702070
}
20712071

@@ -2095,11 +2095,11 @@ func TestResolveDepositScript_ResolvedScriptPresent(t *testing.T) {
20952095
assert.Equal(t, "custom_deposit()", script)
20962096
}
20972097

2098-
func TestResolveWithdrawalScript_ResolvedScriptEmpty(t *testing.T) {
2098+
func TestResolveWithdrawalScript_ScriptEmpty(t *testing.T) {
20992099
mockRegistry := &stubSagaRegistry{
21002100
getActiveResult: &refsaga.Definition{
2101-
Name: "withdrawal",
2102-
ResolvedScript: "",
2101+
Name: "withdrawal",
2102+
Script: "",
21032103
},
21042104
}
21052105
accountTypeCache := cache.NewLocalAccountTypeCache(
@@ -2128,11 +2128,11 @@ func TestResolveWithdrawalScript_ResolvedScriptEmpty(t *testing.T) {
21282128
assert.Equal(t, "default_withdrawal()", script)
21292129
}
21302130

2131-
func TestResolveWithdrawalScript_ResolvedScriptPresent(t *testing.T) {
2131+
func TestResolveWithdrawalScript_ScriptPresent(t *testing.T) {
21322132
mockRegistry := &stubSagaRegistry{
21332133
getActiveResult: &refsaga.Definition{
2134-
Name: "withdrawal",
2135-
ResolvedScript: "custom_withdrawal()",
2134+
Name: "withdrawal",
2135+
Script: "custom_withdrawal()",
21362136
},
21372137
}
21382138
accountTypeCache := cache.NewLocalAccountTypeCache(

services/current-account/service/deposit_orchestrator.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ func (o *DepositOrchestrator) resolveClearingAccountID(ctx context.Context, curr
323323
// Resolution order:
324324
// 1. If SagaResolver is configured and the account has a ProductTypeCode, resolve the
325325
// saga definition from the registry using ProductTypeSagaResolver. The definition's
326-
// ResolvedScript is used. If the resolver returns ErrSagaNotFound, the error is
326+
// Script is used. If the resolver returns ErrSagaNotFound, the error is
327327
// propagated (fail-fast: a prefixed product type must have a corresponding saga).
328328
// 2. Otherwise, fall back to the static default DepositScript.
329329
func (o *DepositOrchestrator) resolveDepositScript(ctx context.Context, account domain.CurrentAccount) (string, error) {
@@ -333,25 +333,25 @@ func (o *DepositOrchestrator) resolveDepositScript(ctx context.Context, account
333333

334334
tenantID, ok := tenant.FromContext(ctx)
335335
if !ok {
336-
// No tenant context fall back to static script
336+
// No tenant context - fall back to static script
337337
return o.depositScript, nil
338338
}
339339

340340
def, err := o.sagaResolver.ResolveForProductType(ctx, tenantID, account.ProductTypeCode(), "deposit")
341341
if err != nil {
342342
if errors.Is(err, refsaga.ErrSagaNotFound) {
343-
// Prefix is defined but no saga found fail fast per PRD convention
343+
// Prefix is defined but no saga found - fail fast per PRD convention
344344
return "", err
345345
}
346-
// ErrNotFound (product type or generic saga missing) fall back to static script
346+
// ErrNotFound (product type or generic saga missing) - fall back to static script
347347
o.logger.Warn("saga resolution fell through to default deposit script",
348348
"product_type_code", account.ProductTypeCode(),
349349
"error", err)
350350
return o.depositScript, nil
351351
}
352352

353-
if def.ResolvedScript == "" {
354-
// Definition has no resolved script fall back to static script
353+
if def.Script == "" {
354+
// Definition has no script - fall back to static script
355355
o.logger.Warn("resolved saga definition has no script, using default",
356356
"saga_name", def.Name,
357357
"product_type_code", account.ProductTypeCode())
@@ -361,5 +361,5 @@ func (o *DepositOrchestrator) resolveDepositScript(ctx context.Context, account
361361
o.logger.Debug("using product-type-specific deposit saga",
362362
"saga_name", def.Name,
363363
"product_type_code", account.ProductTypeCode())
364-
return def.ResolvedScript, nil
364+
return def.Script, nil
365365
}

services/current-account/service/withdrawal_orchestrator.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func (o *WithdrawalOrchestrator) resolveClearingAccountID(ctx context.Context, c
296296
// Resolution order:
297297
// 1. If SagaResolver is configured and the account has a ProductTypeCode, resolve the
298298
// saga definition from the registry using ProductTypeSagaResolver. The definition's
299-
// ResolvedScript is used. If the resolver returns ErrSagaNotFound, the error is
299+
// Script is used. If the resolver returns ErrSagaNotFound, the error is
300300
// propagated (fail-fast: a prefixed product type must have a corresponding saga).
301301
// 2. Otherwise, fall back to the static default WithdrawalScript.
302302
func (o *WithdrawalOrchestrator) resolveWithdrawalScript(ctx context.Context, account domain.CurrentAccount) (string, error) {
@@ -306,25 +306,25 @@ func (o *WithdrawalOrchestrator) resolveWithdrawalScript(ctx context.Context, ac
306306

307307
tenantID, ok := tenant.FromContext(ctx)
308308
if !ok {
309-
// No tenant context fall back to static script
309+
// No tenant context - fall back to static script
310310
return o.withdrawalScript, nil
311311
}
312312

313313
def, err := o.sagaResolver.ResolveForProductType(ctx, tenantID, account.ProductTypeCode(), "withdrawal")
314314
if err != nil {
315315
if errors.Is(err, refsaga.ErrSagaNotFound) {
316-
// Prefix is defined but no saga found fail fast per PRD convention
316+
// Prefix is defined but no saga found - fail fast per PRD convention
317317
return "", err
318318
}
319-
// ErrNotFound (product type or generic saga missing) fall back to static script
319+
// ErrNotFound (product type or generic saga missing) - fall back to static script
320320
o.logger.Warn("saga resolution fell through to default withdrawal script",
321321
"product_type_code", account.ProductTypeCode(),
322322
"error", err)
323323
return o.withdrawalScript, nil
324324
}
325325

326-
if def.ResolvedScript == "" {
327-
// Definition has no resolved script fall back to static script
326+
if def.Script == "" {
327+
// Definition has no script - fall back to static script
328328
o.logger.Warn("resolved saga definition has no script, using default",
329329
"saga_name", def.Name,
330330
"product_type_code", account.ProductTypeCode())
@@ -334,5 +334,5 @@ func (o *WithdrawalOrchestrator) resolveWithdrawalScript(ctx context.Context, ac
334334
o.logger.Debug("using product-type-specific withdrawal saga",
335335
"saga_name", def.Name,
336336
"product_type_code", account.ProductTypeCode())
337-
return def.ResolvedScript, nil
337+
return def.Script, nil
338338
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
-- Remove platform_ref inheritance infrastructure
2+
3+
-- Backfill any NULL scripts before adding NOT NULL constraint.
4+
-- platform_ref sagas that were never backfilled get a placeholder so the
5+
-- migration does not fail. In practice all rows already have scripts
6+
-- (seeder copies content directly), but this guards against edge cases.
7+
UPDATE saga_definition SET script = '' WHERE script IS NULL;
8+
9+
ALTER TABLE saga_definition DROP CONSTRAINT IF EXISTS chk_saga_definition_script_source;
10+
ALTER TABLE saga_definition DROP CONSTRAINT IF EXISTS fk_saga_definition_platform_ref;
11+
DROP INDEX IF EXISTS idx_saga_definition_platform_ref;
12+
ALTER TABLE saga_definition DROP COLUMN IF EXISTS platform_ref;
13+
ALTER TABLE saga_definition DROP COLUMN IF EXISTS override_reason;
14+
ALTER TABLE saga_definition DROP COLUMN IF EXISTS platform_version_at_override;
15+
ALTER TABLE saga_definition ALTER COLUMN script SET NOT NULL;

services/reference-data/migrations/atlas.sum

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
h1:EwBq+L017EYbTiGH2/bdEcRZzy3RR8fK/INabZpg6bw=
1+
h1:kgCY6RqEoKl5lYUcivK1Zy3sGgb0sjOcDwxaK2r5t44=
22
20260104000001_initial.sql h1:2h8BgK7z85efnTzrKZ23RRN3HPtRf9UOTy9FXhA33ik=
33
20260105000001_add_is_system.sql h1:eRDijg0+SC31BlTqSxDUO/5651nNGlvfHCRhP2W1xlk=
44
20260106000001_add_successor_id.sql h1:QasQ5t/OPJ8j138LFpSwy38qGwTyKWwWSlVg1EsPifc=
@@ -25,3 +25,4 @@ h1:EwBq+L017EYbTiGH2/bdEcRZzy3RR8fK/INabZpg6bw=
2525
20260221000002_mapping_definition_active_index.sql h1:I9S39ww++QM08RZfgV57LWoV+1iaNU4DoHP8oO/Yhm4=
2626
20260227000001_add_carbon_data_dimensions.sql h1:X8NuEfGkfnseEzqWHw6kQwVkoemxYuKPCqLD2mGIh+4=
2727
20260227000002_add_carbon_data_dimensions_constraint.sql h1:D5DK06dglc7zJBJWtWQfb83mVPEUmFZOq0z1bqenb3M=
28+
20260406000001_remove_platform_ref.sql h1:jlk8Tljyi49mk+e8s2gzkK2DNP5GciCFnH+hxlclhrQ=

0 commit comments

Comments
 (0)