Skip to content

Commit e299769

Browse files
authored
Merge pull request #2580 from keboola/mvasko/PSGO-233/fix-naming-collision-force-pull
fix(registry): skip naming.Attach() for rows with unresolved parent path [PSGO-233]
2 parents 06ef7ae + 1dc2c3c commit e299769

2 files changed

Lines changed: 82 additions & 2 deletions

File tree

internal/pkg/state/registry/registry.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,16 @@ func (s *Registry) Set(objectState model.ObjectState) error {
346346
}
347347

348348
if objectState.GetRelativePath() != "" {
349-
if err := s.namingRegistry.Attach(key, objectState.GetAbsPath()); err != nil {
350-
return err
349+
absPath := objectState.GetAbsPath()
350+
// Only register in the naming registry when the parent path has been resolved.
351+
// Objects with an unresolved parent path (parentPathSet=false) have a partial
352+
// Path() that equals just their RelativePath, which causes false collisions
353+
// between rows from different configs that share the same row name.
354+
// Such objects are registered once their path is fully generated later.
355+
if absPath.IsParentPathSet() {
356+
if err := s.namingRegistry.Attach(key, absPath); err != nil {
357+
return err
358+
}
351359
}
352360
}
353361

internal/pkg/state/registry/registry_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,78 @@ func TestIgnoreBranch(t *testing.T) {
272272
}
273273
}
274274

275+
// TestRegistrySet_RowsWithSameRelativeNameUnresolvedParent is a regression test
276+
// for PSGO-233: kbc pull --force crashed with a false naming collision when two
277+
// config rows from different configs shared the same relative path name.
278+
//
279+
// Root cause: manifest.Load(ignoreErrors=true) swallows a SetRecords() error,
280+
// leaving some rows with parentPathSet=false. registry.Set() then called
281+
// naming.Attach() with the partial Path() ("rows/besc-json-export" instead of
282+
// the full path), causing a spurious collision between unrelated rows.
283+
func TestRegistrySet_RowsWithSameRelativeNameUnresolvedParent(t *testing.T) {
284+
t.Parallel()
285+
ctx := t.Context()
286+
reg := New(knownpaths.NewNop(ctx), naming.NewRegistry(), NewComponentsMap(nil), SortByPath)
287+
288+
// Neither row has its parent path set (parentPathSet=false, zero value of bool).
289+
// AbsPath{RelativePath: "..."} leaves parentPath="" and parentPathSet=false.
290+
row1 := &ConfigRowState{
291+
ConfigRowManifest: &ConfigRowManifest{
292+
ConfigRowKey: ConfigRowKey{
293+
BranchID: 80, ComponentID: "keboola.wr-azure-event-hub",
294+
ConfigID: "1783940", ID: "2052339",
295+
},
296+
Paths: Paths{AbsPath: AbsPath{RelativePath: "rows/besc-json-export"}},
297+
},
298+
Remote: &ConfigRow{Name: "besc-json-export"},
299+
}
300+
row2 := &ConfigRowState{
301+
ConfigRowManifest: &ConfigRowManifest{
302+
ConfigRowKey: ConfigRowKey{
303+
BranchID: 80, ComponentID: "keboola.wr-azure-event-hub",
304+
ConfigID: "1783960", ID: "2405343",
305+
},
306+
Paths: Paths{AbsPath: AbsPath{RelativePath: "rows/besc-json-export"}},
307+
},
308+
Remote: &ConfigRow{Name: "besc-json-export"},
309+
}
310+
311+
require.NoError(t, reg.Set(row1), "first row with unresolved parent path should not error")
312+
require.NoError(t, reg.Set(row2), "second row with same relative name but different config should not error")
313+
assert.Len(t, reg.ConfigRows(), 2)
314+
}
315+
316+
// TestRegistrySet_RowWithResolvedParentIsAttached verifies that rows whose
317+
// parent path IS resolved still get registered in the naming registry (no regression).
318+
func TestRegistrySet_RowWithResolvedParentIsAttached(t *testing.T) {
319+
t.Parallel()
320+
ctx := t.Context()
321+
reg := New(knownpaths.NewNop(ctx), naming.NewRegistry(), NewComponentsMap(nil), SortByPath)
322+
323+
row := &ConfigRowState{
324+
ConfigRowManifest: &ConfigRowManifest{
325+
ConfigRowKey: ConfigRowKey{
326+
BranchID: 80, ComponentID: "keboola.wr-azure-event-hub",
327+
ConfigID: "1783940", ID: "2052339",
328+
},
329+
Paths: Paths{
330+
AbsPath: NewAbsPath(
331+
"80-dev/keboola.wr-azure-event-hub/config-1783940",
332+
"rows/besc-json-export",
333+
),
334+
},
335+
},
336+
Remote: &ConfigRow{Name: "besc-json-export"},
337+
}
338+
339+
require.NoError(t, reg.Set(row))
340+
341+
// Row with a resolved parent must be reachable via GetByPath.
342+
found, ok := reg.GetByPath("80-dev/keboola.wr-azure-event-hub/config-1783940/rows/besc-json-export")
343+
require.True(t, ok)
344+
assert.Equal(t, row, found)
345+
}
346+
275347
func newTestState(t *testing.T, paths *knownpaths.Paths) *Registry {
276348
t.Helper()
277349
registry := New(paths, naming.NewRegistry(), NewComponentsMap(nil), SortByPath)

0 commit comments

Comments
 (0)