diff --git a/cli/config/credentials/file_store.go b/cli/config/credentials/file_store.go index 07918a2c34e9..c69312b01490 100644 --- a/cli/config/credentials/file_store.go +++ b/cli/config/credentials/file_store.go @@ -28,7 +28,8 @@ func NewFileStore(file store) Store { return &fileStore{file: file} } -// Erase removes the given credentials from the file store. +// Erase removes the given credentials from the file store.This function is +// idempotent and does not update the file if credentials did not change. func (c *fileStore) Erase(serverAddress string) error { if _, exists := c.file.GetAuthConfigs()[serverAddress]; !exists { // nothing to do; no credentials found for the given serverAddress diff --git a/cli/config/credentials/file_store_test.go b/cli/config/credentials/file_store_test.go index 077d89dc8e10..466fa054a03e 100644 --- a/cli/config/credentials/file_store_test.go +++ b/cli/config/credentials/file_store_test.go @@ -10,9 +10,15 @@ import ( type fakeStore struct { configs map[string]types.AuthConfig + saveFn func(*fakeStore) error } func (f *fakeStore) Save() error { + if f.saveFn != nil { + // Pass a reference to the fakeStore itself in case saveFn + // wants to access it. + return f.saveFn(f) + } return nil } @@ -21,7 +27,78 @@ func (f *fakeStore) GetAuthConfigs() map[string]types.AuthConfig { } func (f *fakeStore) GetFilename() string { - return "/tmp/docker-fakestore" + return "no-config.json" +} + +// TestFileStoreIdempotent verifies that the config-file isn't updated +// if nothing changed. +func TestFileStoreIdempotent(t *testing.T) { + var saveCount, expectedSaveCount int + + s := NewFileStore(&fakeStore{ + configs: map[string]types.AuthConfig{}, + saveFn: func(*fakeStore) error { + saveCount++ + return nil + }, + }) + authOne := types.AuthConfig{ + Auth: "super_secret_token", + Email: "foo@example.com", + ServerAddress: "https://example.com", + } + authTwo := types.AuthConfig{ + Auth: "also_super_secret_token", + Email: "bar@example.com", + ServerAddress: "https://other.example.com", + } + + expectedSaveCount = 1 + t.Run("store new credentials", func(t *testing.T) { + assert.NilError(t, s.Store(authOne)) + retrievedAuth, err := s.Get(authOne.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, authOne)) + assert.Check(t, is.Equal(saveCount, expectedSaveCount)) + }) + t.Run("store same credentials is a no-op", func(t *testing.T) { + assert.NilError(t, s.Store(authOne)) + retrievedAuth, err := s.Get(authOne.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, authOne)) + assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed") + }) + t.Run("store other credentials", func(t *testing.T) { + expectedSaveCount++ + assert.NilError(t, s.Store(authTwo)) + retrievedAuth, err := s.Get(authTwo.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, authTwo)) + assert.Check(t, is.Equal(saveCount, expectedSaveCount)) + }) + t.Run("erase credentials", func(t *testing.T) { + expectedSaveCount++ + assert.NilError(t, s.Erase(authOne.ServerAddress)) + retrievedAuth, err := s.Get(authOne.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{})) + assert.Check(t, is.Equal(saveCount, expectedSaveCount)) + }) + t.Run("erase non-existing credentials is a no-op", func(t *testing.T) { + assert.NilError(t, s.Erase(authOne.ServerAddress)) + retrievedAuth, err := s.Get(authOne.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{})) + assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed") + }) + t.Run("erase other credentials", func(t *testing.T) { + expectedSaveCount++ + assert.NilError(t, s.Erase(authTwo.ServerAddress)) + retrievedAuth, err := s.Get(authTwo.ServerAddress) + assert.NilError(t, err) + assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{})) + assert.Check(t, is.Equal(saveCount, expectedSaveCount)) + }) } func TestFileStoreAddCredentials(t *testing.T) {