diff --git a/config/versions/versions.go b/config/versions/versions.go index 8321beb6261..a5c2d4965f8 100644 --- a/config/versions/versions.go +++ b/config/versions/versions.go @@ -17,6 +17,7 @@ import ( "errors" "fmt" "log" + "slices" "strconv" "sync" @@ -26,8 +27,8 @@ import ( var ( errRegisteringVersion = errors.New("error registering config version") + errMissingVersion = errors.New("missing version") errVersionIncompatible = errors.New("version does not implement ConfigVersion or ExchangeVersion") - errVersionSequence = errors.New("version registered out of sequence") errModifyingExchange = errors.New("error modifying exchange config") errNoVersions = errors.New("error retrieving latest config version: No config versions are registered") errApplyingVersion = errors.New("error applying version") @@ -50,23 +51,22 @@ type ExchangeVersion interface { type manager struct { m sync.RWMutex versions []any - errors error } // Manager is a public instance of the config version manager var Manager = &manager{} // Deploy upgrades or downgrades the config between versions -// Will immediately return any errors encountered in registerVersion calls func (m *manager) Deploy(ctx context.Context, j []byte) ([]byte, error) { - if m.errors != nil { - return j, m.errors + if err := m.checkVersions(); err != nil { + return j, err } target, err := m.latest() if err != nil { return j, err } + m.m.RLock() defer m.m.RUnlock() @@ -163,22 +163,13 @@ func exchangeDeploy(ctx context.Context, patch ExchangeVersion, method func(Exch } // registerVersion takes instances of config versions and adds them to the registry -// Versions should be added sequentially without gaps, in import.go init -// Any errors will also added to the registry for reporting later func (m *manager) registerVersion(ver int, v any) { m.m.Lock() defer m.m.Unlock() - switch v.(type) { - case ExchangeVersion, ConfigVersion: - default: - m.errors = common.AppendError(m.errors, fmt.Errorf("%w: %v", errVersionIncompatible, ver)) - return - } - if len(m.versions) != ver { - m.errors = common.AppendError(m.errors, fmt.Errorf("%w: %v", errVersionSequence, ver)) - return + if ver >= len(m.versions) { + m.versions = slices.Grow(m.versions, ver+1)[:ver+1] } - m.versions = append(m.versions, v) + m.versions[ver] = v } // latest returns the highest version number @@ -190,3 +181,20 @@ func (m *manager) latest() (int, error) { } return len(m.versions) - 1, nil } + +// checkVersions ensures that registered versions are consistent +func (m *manager) checkVersions() error { + m.m.RLock() + defer m.m.RUnlock() + for ver, v := range m.versions { + switch v.(type) { + case ExchangeVersion, ConfigVersion: + default: + return fmt.Errorf("%w: %v", errVersionIncompatible, ver) + } + if v == nil { + return fmt.Errorf("%w: v%v", errMissingVersion, ver) + } + } + return nil +} diff --git a/config/versions/versions_test.go b/config/versions/versions_test.go index 643bffbfb06..4191269b37e 100644 --- a/config/versions/versions_test.go +++ b/config/versions/versions_test.go @@ -20,7 +20,8 @@ func TestDeploy(t *testing.T) { _, err = m.Deploy(context.Background(), []byte(``)) require.ErrorIs(t, err, errVersionIncompatible) - m.errors = nil + m = manager{} + m.registerVersion(0, &Version0{}) _, err = m.Deploy(context.Background(), []byte(`not an object`)) require.ErrorIs(t, err, jsonparser.KeyPathNotFoundError, "Should throw the correct error trying to add version to bad json") @@ -81,18 +82,16 @@ func TestRegisterVersion(t *testing.T) { m := manager{} m.registerVersion(0, &Version0{}) - require.NoError(t, m.errors) assert.NotEmpty(t, m.versions) - m.errors = nil - m.registerVersion(1, &TestVersion1{}) - require.ErrorIs(t, m.errors, errVersionIncompatible) - assert.ErrorContains(t, m.errors, ": 1") - - m.errors = nil m.registerVersion(2, &TestVersion2{}) - assert.ErrorIs(t, m.errors, errVersionSequence) - assert.ErrorContains(t, m.errors, ": 2") + require.Equal(t, 3, len(m.versions), "Must allocate a space for missing version 1") + require.NotNil(t, m.versions[2], "Must put Version 2 in the correct slot") + require.Nil(t, m.versions[1], "Must leave Version 1 alone") + + m.registerVersion(1, &TestVersion1{}) + require.Equal(t, 3, len(m.versions), "Must leave len alone when registering out-of-sequence") + require.NotNil(t, m.versions[1], "Should put Version 1 in the correct slot") } func TestLatest(t *testing.T) {