Skip to content

Commit

Permalink
Refactor DataID to be a computed field
Browse files Browse the repository at this point in the history
  • Loading branch information
@drichelson
drichelson committed May 27, 2024
1 parent d41b4dc commit bd6cbf8
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 72 deletions.
29 changes: 12 additions & 17 deletions internal/dorkly/reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ func (r *Reconciler) Reconcile(ctx context.Context) error {
}

var reconciledArchive RelayArchive
err = runStep("Merge existing archive and local yaml project files into reconciled archive", func() error {
err = runStep("Reconcile existing archive and local yaml project files into updated archive", func() error {
var err error
reconciledArchive, err = reconcile(*existingArchive, *newArchive)
logger.Infof("Merged archive for upload: %v", reconciledArchive.String())
logger.Infof("Reconciled archive for upload: %v", reconciledArchive.String())
return err
})
if err != nil {
Expand All @@ -89,7 +89,7 @@ func reconcile(old, new RelayArchive) (RelayArchive, error) {
//set all versions to 1
newEnv := new.envs[envKey]
newEnv.metadata.EnvMetadata.Version = 1
newEnv.metadata.incrementDataId()
//newEnv.metadata.incrementDataId()
for flagKey, flag := range newEnv.data.Flags {
flag.Version = 1
newEnv.data.Flags[flagKey] = flag
Expand All @@ -99,27 +99,24 @@ func reconcile(old, new RelayArchive) (RelayArchive, error) {

// TODO: Process deleted envs.. wtf.
for _, envKey := range compareResult.deleted {
logger.With("env", envKey).Error("Deleted environment found. Doing nothing for now...")
logger.With("env", envKey).Error("Deleted environment. Doing nothing for now...")
}

// Process existing envs
for _, envKey := range compareResult.existing {
logger.With("env", envKey).Info("Existing environment found")
shouldChangeDataId := false
logger.With("env", envKey).Info("Existing environment")
// compare env metadata:
oldEnv := old.envs[envKey]
newEnv := new.envs[envKey]

// These fields are not tracked in the local project yaml, so we need to copy them over
newEnv.metadata.EnvMetadata.Version = oldEnv.metadata.EnvMetadata.Version
newEnv.metadata.DataId = oldEnv.metadata.DataId
newEnv.metadata.dataId = oldEnv.metadata.dataId

// compare env metadata ignoring versions
if !reflect.DeepEqual(oldEnv.metadata, newEnv.metadata) {
logger.With("env", envKey).Info("Environment metadata changed.")
newEnv.metadata.EnvMetadata.Version++
shouldChangeDataId = true
}

// compare flags
Expand All @@ -132,7 +129,6 @@ func reconcile(old, new RelayArchive) (RelayArchive, error) {
flag := newEnv.data.Flags[flagKey]
flag.Version = 1
newEnv.data.Flags[flagKey] = flag
shouldChangeDataId = true
}

// Process deleted flags
Expand All @@ -142,7 +138,6 @@ func reconcile(old, new RelayArchive) (RelayArchive, error) {
deletedFlag.Version++
deletedFlag.Deleted = true
newEnv.data.Flags[flagKey] = deletedFlag
shouldChangeDataId = true
}

// Process existing flags
Expand All @@ -155,21 +150,21 @@ func reconcile(old, new RelayArchive) (RelayArchive, error) {
newFlag.Version++
logger.With("env", envKey).With("flag", flagKey).
Infof("Found modified flag. Version %d->%d", oldFlag.Version, newFlag.Version)
shouldChangeDataId = true
} else {
logger.With("env", envKey).With("flag", flagKey).With("version", newFlag.Version).
Infof("Found existing unchanged flag")
}
newEnv.data.Flags[flagKey] = newFlag
}

if shouldChangeDataId {
logger.With("env", envKey).
Infof("Found changes in this environment. Incrementing dataId: %d->%d", newEnv.metadata.dataId, newEnv.metadata.dataId+1)
newEnv.metadata.incrementDataId()
}
new.envs[envKey] = newEnv
}

// Now that we're done reconciling let's recalculate the DataId for each environment:
for envKey, env := range new.envs {
env.computeDataId()
new.envs[envKey] = env
}

return new, nil
}

Expand Down
36 changes: 19 additions & 17 deletions internal/dorkly/reconcile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,41 @@ func Test_Reconcile(t *testing.T) {
{
name: "no change",
old: relayArchive().
env(env("staging").version(1).dataId(1).
env(env("staging").version(1).dataId("2").
flag(booleanFlag("flag1").variation(false).version(1))),
new: relayArchive().
env(env("staging").
flag(booleanFlag("flag1").variation(false))),
expected: relayArchive().
env(env("staging").version(1).dataId(1).
env(env("staging").version(1).dataId("2").
flag(booleanFlag("flag1").variation(false).version(1))),
wantErr: assert.NoError,
},
{
name: "toggle flag",
old: relayArchive().
env(env("staging").version(1).dataId(1).
env(env("staging").version(1).dataId("2").
flag(booleanFlag("flag1").variation(false).version(1))),
new: relayArchive().
env(env("staging").
flag(booleanFlag("flag1").variation(true))),
expected: relayArchive().
env(env("staging").version(1).dataId(2).
env(env("staging").version(1).dataId("3").
flag(booleanFlag("flag1").variation(true).version(2))),
wantErr: assert.NoError,
},
{
name: "new flag",
old: relayArchive().
env(env("staging").version(1).dataId(1).
env(env("staging").version(1).dataId("2").
flag(booleanFlag("flag1").variation(false).version(1))),
new: relayArchive().
env(env("staging").
flag(booleanFlag("flag1").variation(false)).
flag(booleanFlag("flag2").variation(true)),
),
expected: relayArchive().
env(env("staging").version(1).dataId(2).
env(env("staging").version(1).dataId("3").
flag(booleanFlag("flag1").variation(false).version(1)).
flag(booleanFlag("flag2").variation(true).version(1)),
),
Expand All @@ -61,12 +61,12 @@ func Test_Reconcile(t *testing.T) {
{
name: "deleted flag",
old: relayArchive().
env(env("staging").version(1).dataId(1).
env(env("staging").version(1).dataId("2").
flag(booleanFlag("flag1").variation(false).version(1))),
new: relayArchive().
env(env("staging").version(1)),
expected: relayArchive().
env(env("staging").version(1).dataId(2).
env(env("staging").version(1).dataId("3").
flag(booleanFlag("flag1").variation(false).version(2).deleted(true))),
wantErr: assert.NoError,
},
Expand Down Expand Up @@ -102,20 +102,22 @@ type envBuilder struct {
}

func env(key string) envBuilder {
return envBuilder{Env{
RelayArchiveEnv{
EnvMetadata: RelayArchiveEnvMetadata{
EnvID: key,
EnvKey: key,
EnvName: key,
},
archiveEnv := RelayArchiveEnv{
EnvMetadata: RelayArchiveEnvMetadata{
EnvID: key,
EnvKey: key,
EnvName: key,
},
}

return envBuilder{Env{
archiveEnv,
RelayArchiveData{
Flags: map[string]ldmodel.FeatureFlag{},
}}}
}
func (b envBuilder) dataId(dataId int) envBuilder {
b.Env.metadata.setDataId(dataId)
func (b envBuilder) dataId(dataId string) envBuilder {
b.Env.metadata.DataId = dataId
return b
}

Expand Down
21 changes: 8 additions & 13 deletions internal/dorkly/relay_archive.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,27 @@ func (e *Env) String() string {
e.metadata.EnvMetadata.EnvName, e.metadata.EnvMetadata.Version, e.metadata.DataId, e.data.String())
}

func (e *Env) computeDataId() {
dataId := e.metadata.EnvMetadata.Version
for _, flag := range e.data.Flags {
dataId += flag.Version
}
e.metadata.DataId = fmt.Sprintf("%d", dataId)
}

// RelayArchiveEnv is a representation of the <env>.json file in the relay archive
type RelayArchiveEnv struct {
EnvMetadata RelayArchiveEnvMetadata `json:"env"`

// this must be changed in order for flag changes to be picked up.
// the official relay archive uses a double-quoted string like this: "\"<value>\""
DataId string `json:"dataId"`

// internal representation of dataId
dataId int
}

func (a *RelayArchiveEnv) String() string {
return fmt.Sprintf("Env: %v, DataId: %v", a.EnvMetadata.String(), a.DataId)
}

func (a *RelayArchiveEnv) incrementDataId() {
a.dataId++
a.setDataId(a.dataId)
}

func (a *RelayArchiveEnv) setDataId(dataId int) {
a.dataId = dataId
a.DataId = fmt.Sprintf("\"%d\"", a.dataId)
}

type RelayArchiveEnvMetadata struct {
EnvID string `json:"envID"`
EnvKey string `json:"envKey"`
Expand Down
25 changes: 0 additions & 25 deletions internal/dorkly/relay_archive_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,6 @@ import (
"testing"
)

func Test_ArchiveEnvironmentRep_IncrementDataId(t *testing.T) {
tests := []struct {
name string
input RelayArchiveEnv
expected RelayArchiveEnv
}{
{
name: "zero",
input: RelayArchiveEnv{},
expected: RelayArchiveEnv{dataId: 1, DataId: "\"1\""},
},
{
name: "one",
input: RelayArchiveEnv{dataId: 1, DataId: "\"1\""},
expected: RelayArchiveEnv{dataId: 2, DataId: "\"2\""},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.input.incrementDataId()
assert.Equal(t, tt.expected, tt.input)
})
}
}

// Ensures the round trip of archive -> files -> unmarshaling -> marshaling -> files -> archive works as expected.
func Test_RelayArchive_RoundTrip(t *testing.T) {
var err error
Expand Down

0 comments on commit bd6cbf8

Please sign in to comment.