Skip to content

Commit 14986e9

Browse files
Matovidloclaude
andcommitted
fix(templates): introduce ErrMultipleParents sentinel; add unit test
- model/relation.go: declare ErrMultipleParents sentinel; wrap it into Relations.ParentKey error with %w so errors.Is can detect it - model/object.go: use errors.Is(err, ErrMultipleParents) in Config.ParentKey so only the shared-variables case falls back to the structural parent; all other relation errors are now propagated; update comment to cite the full VariablesForRelation invariant (NewOtherSideRelation hard-codes keboola.VariablesComponentID, making both checkDefinedOn guards unreachable) - mapper/ignore/remote.go: update comment to clarify the > 1 guard fires only when the variables config was absent from the loaded batch during relation linking, so the two-pass dedup never ran for it - mapper/ignore/remote_test.go: add TestIgnoreMapper_AfterRemoteOperation_Variables_SharedByMultipleConsumers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 06db3de commit 14986e9

4 files changed

Lines changed: 68 additions & 9 deletions

File tree

internal/pkg/mapper/ignore/remote.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,11 @@ func (m *ignoreMapper) isIgnoredConfig(ctx context.Context, config *model.Config
6363
if config.ComponentID == keboola.VariablesComponentID {
6464
variablesForRels := config.Relations.GetByType(model.VariablesForRelType)
6565

66-
// Ignore if shared across multiple consumers — cannot be embedded under a single
67-
// parent. The two-pass relation validator should remove these; this is a safety net
68-
// for the Templates API where object iteration order exposes an edge case.
66+
// Ignore if shared across multiple consumers — cannot be embedded under a single parent.
67+
// Normally the two-pass relations mapper removes all VariablesForRelType entries when
68+
// duplicates are detected (leaving len == 0, caught by the unattached check below).
69+
// This guard fires only when the variables config was absent from the loaded batch in
70+
// which consumer relations were linked, so the dedup pass never ran for it.
6971
if len(variablesForRels) > 1 {
7072
m.logger.Debugf(ctx, "Ignored %s shared by %d consumers", config.Desc(), len(variablesForRels))
7173
return true

internal/pkg/mapper/ignore/remote_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,55 @@ func TestIgnoreMapper_AfterRemoteOperation_Variables(t *testing.T) {
6262
}, state.All())
6363
}
6464

65+
func TestIgnoreMapper_AfterRemoteOperation_Variables_SharedByMultipleConsumers(t *testing.T) {
66+
t.Parallel()
67+
state, d := createStateWithMapper(t)
68+
logger := d.DebugLogger()
69+
70+
// Consumer 1
71+
consumer1Key := model.ConfigKey{BranchID: 1, ComponentID: "ex-generic-v2", ID: "1"}
72+
consumer1 := &model.ConfigState{
73+
ConfigManifest: &model.ConfigManifest{ConfigKey: consumer1Key},
74+
Remote: &model.Config{ConfigKey: consumer1Key},
75+
}
76+
require.NoError(t, state.Set(consumer1))
77+
78+
// Consumer 2
79+
consumer2Key := model.ConfigKey{BranchID: 1, ComponentID: "ex-generic-v2", ID: "2"}
80+
consumer2 := &model.ConfigState{
81+
ConfigManifest: &model.ConfigManifest{ConfigKey: consumer2Key},
82+
Remote: &model.Config{ConfigKey: consumer2Key},
83+
}
84+
require.NoError(t, state.Set(consumer2))
85+
86+
// Shared variables config with two VariablesForRelation entries — simulates the case
87+
// where the relations mapper's two-pass dedup did not run for this config (it was absent
88+
// from the loaded batch when consumer relations were linked).
89+
sharedVarsKey := model.ConfigKey{BranchID: 1, ComponentID: keboola.VariablesComponentID, ID: "3"}
90+
sharedVars := &model.ConfigState{
91+
ConfigManifest: &model.ConfigManifest{ConfigKey: sharedVarsKey},
92+
Remote: &model.Config{
93+
ConfigKey: sharedVarsKey,
94+
Relations: model.Relations{
95+
&model.VariablesForRelation{ComponentID: consumer1Key.ComponentID, ConfigID: consumer1Key.ID},
96+
&model.VariablesForRelation{ComponentID: consumer2Key.ComponentID, ConfigID: consumer2Key.ID},
97+
},
98+
},
99+
}
100+
require.NoError(t, state.Set(sharedVars))
101+
102+
// Invoke
103+
changes := model.NewRemoteChanges()
104+
changes.AddLoaded(consumer1)
105+
changes.AddLoaded(consumer2)
106+
changes.AddLoaded(sharedVars)
107+
require.NoError(t, state.Mapper().AfterRemoteOperation(t.Context(), changes))
108+
logger.AssertJSONMessages(t, `{"level":"debug","message":"Ignored config \"branch:1/component:keboola.variables/config:3\" shared by 2 consumers"}`)
109+
110+
// Shared variables config is removed; consumers remain
111+
assert.Equal(t, []model.ObjectState{consumer1, consumer2}, state.All())
112+
}
113+
65114
func TestIgnoreMapper_AfterRemoteOperation_Scheduler(t *testing.T) {
66115
t.Parallel()
67116
state, d := createStateWithMapper(t)

internal/pkg/model/object.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,16 +595,20 @@ func (r *ConfigRow) GetContent() *orderedmap.OrderedMap {
595595
}
596596

597597
// ParentKey - config parent can be modified via Relations, e.g. variables config embedded in another config.
598-
// When Relations.ParentKey returns an error it means multiple relation-defined parents exist (a variables
599-
// config shared by more than one consumer). In that case fall back to the structural parent (branch).
600-
// Other error kinds from Relations.ParentKey cannot occur here because individual relation errors require
601-
// a non-ConfigKey source, but c.Key() is always a ConfigKey.
598+
// When Relations.ParentKey returns ErrMultipleParents (a shared variables config), fall back to the
599+
// structural parent (branch). VariablesForRelation is only ever constructed by
600+
// VariablesFromRelation.NewOtherSideRelation, which hard-codes ComponentID: keboola.VariablesComponentID,
601+
// so neither checkDefinedOn guard can trigger in practice. All other errors are propagated.
602602
func (c *Config) ParentKey() (Key, error) {
603603
parentKey, err := c.Relations.ParentKey(c.Key())
604604
if err == nil && parentKey != nil {
605605
return parentKey, nil
606606
}
607-
// No parent defined via "Relations", or multiple parents — fall back to structural parent (branch).
607+
if err != nil && !errors.Is(err, ErrMultipleParents) {
608+
return nil, err
609+
}
610+
// No relation-defined parent, or multiple parents (shared variables config) —
611+
// fall back to the structural parent (branch).
608612
return c.ConfigKey.ParentKey()
609613
}
610614

internal/pkg/model/relation.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ const (
2424
UsedInRowInputMappingRelType = RelationType(`usedInRowInputMapping`)
2525
)
2626

27+
// ErrMultipleParents is returned by Relations.ParentKey when more than one relation
28+
// defines a parent key for the same object.
29+
var ErrMultipleParents = errors.New(`multiple parents defined by "relations"`)
30+
2731
// OneToXRelations gets relations that can be defined on an object only once.
2832
func OneToXRelations() []RelationType {
2933
return []RelationType{
@@ -80,7 +84,7 @@ func (v Relations) ParentKey(source Key) (Key, error) {
8084

8185
// Multiple parents are forbidden
8286
if len(parents) > 1 {
83-
return nil, errors.Errorf(`unexpected state: multiple parents defined by "relations" in %s`, source.Desc())
87+
return nil, errors.Errorf(`unexpected state in %s: %w`, source.Desc(), ErrMultipleParents)
8488
}
8589

8690
return nil, nil

0 commit comments

Comments
 (0)