diff --git a/internal/sync/bootstrap/bootstrap.go b/internal/sync/bootstrap/bootstrap.go index 3a76eb84..3eefd8d7 100644 --- a/internal/sync/bootstrap/bootstrap.go +++ b/internal/sync/bootstrap/bootstrap.go @@ -31,18 +31,12 @@ type AttachmentReader interface { ReadAttachment(projectKey string, kind syncdomain.AttachmentKind, key string) (syncdomain.Attachment, error) } -// ManifestStore reads and writes the sync baseline. -type ManifestStore interface { - Load(projectKeys []string) (syncmanifest.Manifest, error) - Update(previous, next syncmanifest.Manifest) (syncmanifest.Manifest, error) -} - // Options are the dependencies and the input of one add. type Options struct { Catalog Catalog Attachments AttachmentReader Store synclocal.Store - Manifest ManifestStore + Baselines syncmanifest.Baselines Input io.Reader Output io.Writer // Initial is true when the workspace has no .launchdarkly directory. @@ -242,7 +236,7 @@ func finishSelection(options Options, files []synclocal.VariationFile) error { for _, file := range files { projectKeys = append(projectKeys, file.ProjectKey) } - manifest, err := options.Manifest.Load(projectKeys) + baseline, err := options.Baselines.Load(projectKeys) if err != nil { return err } @@ -256,9 +250,9 @@ func finishSelection(options Options, files []synclocal.VariationFile) error { if err != nil { return err } - next, err := recordCreatedVariations(manifest, options.Store, files) + next, err := recordCreatedVariations(baseline.Lock, options.Store, files) if err == nil { - _, err = options.Manifest.Update(manifest, next) + _, err = options.Baselines.Save(baseline, next) } if err != nil { return errors.Join(err, options.Store.RollbackCreation(creation)) diff --git a/internal/sync/bootstrap/bootstrap_test.go b/internal/sync/bootstrap/bootstrap_test.go index 357e15ea..0dd06807 100644 --- a/internal/sync/bootstrap/bootstrap_test.go +++ b/internal/sync/bootstrap/bootstrap_test.go @@ -176,17 +176,18 @@ func TestFinishSelectionWritesInitialManifest(t *testing.T) { secondVariation.Name = "Variation 2" err := finishSelection(Options{ - Store: synclocal.NewStore(root), - Manifest: manifestStore, - Output: &output, - Initial: true, + Store: synclocal.NewStore(root), + Baselines: manifestStore, + Output: &output, + Initial: true, }, []synclocal.VariationFile{ {ProjectKey: "project", ConfigKey: "config", Upsert: true, Variation: variation}, {ProjectKey: "project", ConfigKey: "config", Upsert: true, Variation: secondVariation}, }) require.NoError(t, err) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) require.Len(t, manifest.Resources, 3) attachmentFingerprint, err := syncdomain.FingerprintAttachment("project", variation.Attachments[0]) @@ -221,7 +222,7 @@ func TestFinishSelectionFingerprintsExistingAttachmentContent(t *testing.T) { Tools: []syncdomain.AttachmentRef{{Key: "search"}}, Attachments: []syncdomain.Attachment{localAttachment}, } require.NoError(t, finishSelection(Options{ - Store: store, Manifest: manifestStore, Output: io.Discard, Initial: true, + Store: store, Baselines: manifestStore, Output: io.Discard, Initial: true, }, []synclocal.VariationFile{{ ProjectKey: "project", ConfigKey: "config", Variation: first, }})) @@ -241,7 +242,7 @@ func TestFinishSelectionFingerprintsExistingAttachmentContent(t *testing.T) { Tools: []syncdomain.AttachmentRef{{Key: "search"}}, Attachments: []syncdomain.Attachment{serverAttachment}, } require.NoError(t, finishSelection(Options{ - Store: store, Manifest: manifestStore, Output: io.Discard, + Store: store, Baselines: manifestStore, Output: io.Discard, }, []synclocal.VariationFile{{ ProjectKey: "project", ConfigKey: "config", Variation: second, }})) @@ -258,7 +259,8 @@ func TestFinishSelectionFingerprintsExistingAttachmentContent(t *testing.T) { } require.NotEmpty(t, expectedFingerprint) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) for _, resource := range manifest.Resources { if resource.ResourceKind == syncdomain.KindVariation && resource.LookupKey == "config/second" { @@ -285,19 +287,20 @@ func TestFinishSelectionAddsMultipleVersionedVariationsToExistingManifest(t *tes ModelConfigKey: "model", ModelConfigVersion: 3, } require.NoError(t, finishSelection(Options{ - Store: store, Manifest: manifestStore, Output: io.Discard, Initial: true, + Store: store, Baselines: manifestStore, Output: io.Discard, Initial: true, }, []synclocal.VariationFile{{ ProjectKey: "project", ConfigKey: "config", Variation: first, }})) require.NoError(t, finishSelection(Options{ - Store: store, Manifest: manifestStore, Output: io.Discard, + Store: store, Baselines: manifestStore, Output: io.Discard, }, []synclocal.VariationFile{ {ProjectKey: "project", ConfigKey: "config", Variation: second}, {ProjectKey: "project", ConfigKey: "config", Variation: third}, })) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) require.Len(t, manifest.Resources, 3) assert.Equal(t, "config/first", manifest.Resources[0].LookupKey) @@ -317,10 +320,10 @@ func TestFinishSelectionRollsBackFilesWhenManifestWriteFails(t *testing.T) { } err := finishSelection(Options{ - Store: synclocal.NewStore(root), - Manifest: failingManifestStore{}, - Output: io.Discard, - Initial: true, + Store: synclocal.NewStore(root), + Baselines: failingManifestStore{}, + Output: io.Discard, + Initial: true, }, []synclocal.VariationFile{{ ProjectKey: "project", ConfigKey: "config", Variation: variation, }}) @@ -350,7 +353,7 @@ func TestFinishSelectionRollsBackNewAttachmentWithoutRemovingExistingWorkspace(t } err = finishSelection(Options{ - Store: store, Manifest: failingManifestStore{}, Output: io.Discard, + Store: store, Baselines: failingManifestStore{}, Output: io.Discard, }, []synclocal.VariationFile{{ ProjectKey: "project", ConfigKey: "config", Variation: variation, }}) @@ -429,27 +432,24 @@ func (catalog *fakeCatalog) Config(string, string) (syncapi.Config, error) { type failingManifestStore struct{} -func (failingManifestStore) Load([]string) (syncmanifest.Manifest, error) { - return syncmanifest.New(), nil +func (failingManifestStore) Load([]string) (syncmanifest.Baseline, error) { + return syncmanifest.Baseline{Lock: syncmanifest.New()}, nil } -func (failingManifestStore) Update(syncmanifest.Manifest, syncmanifest.Manifest) (syncmanifest.Manifest, error) { - return syncmanifest.Manifest{}, errors.New("write manifest") +func (failingManifestStore) Save(syncmanifest.Baseline, syncmanifest.Manifest) (syncmanifest.Baseline, error) { + return syncmanifest.Baseline{}, errors.New("write manifest") } type memoryManifestStore struct { manifest syncmanifest.Manifest } -func (store *memoryManifestStore) Load([]string) (syncmanifest.Manifest, error) { - return store.manifest, nil +func (store *memoryManifestStore) Load([]string) (syncmanifest.Baseline, error) { + return syncmanifest.Baseline{Lock: store.manifest}, nil } -func (store *memoryManifestStore) Update( - _ syncmanifest.Manifest, - next syncmanifest.Manifest, -) (syncmanifest.Manifest, error) { +func (store *memoryManifestStore) Save(_ syncmanifest.Baseline, next syncmanifest.Manifest) (syncmanifest.Baseline, error) { next.Sort() store.manifest = next - return next, nil + return syncmanifest.Baseline{Lock: next}, nil } diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index 59e8c6a5..c4ed1d82 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -15,17 +15,11 @@ import ( syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" ) -// ManifestStore reads and writes the sync baseline. -type ManifestStore interface { - Load(projectKeys []string) (syncmanifest.Manifest, error) - Update(previous, next syncmanifest.Manifest) (syncmanifest.Manifest, error) -} - // Options are the dependencies and the input of one detach. type Options struct { RepositoryRoot string Store synclocal.Store - Manifest ManifestStore + Baselines syncmanifest.Baselines // ProjectKeys are the projects that have local files. ProjectKeys []string Input io.Reader @@ -44,7 +38,7 @@ func Run(options Options) error { slices.Sort(projectKeys) projectKeys = slices.Compact(projectKeys) - synced, manifest, err := loadResources(options.RepositoryRoot, options.Manifest, projectKeys) + synced, baseline, err := loadResources(options.RepositoryRoot, options.Baselines, projectKeys) if err != nil { return err } @@ -66,7 +60,7 @@ func Run(options Options) error { return err } } - if err := detachResources(options, manifest, selected); err != nil { + if err := detachResources(options, baseline, selected); err != nil { return err } @@ -110,24 +104,24 @@ func validateSelections(synced, selected []syncdomain.ResourceID) error { return nil } -// loadResources returns each variation that the manifest tracks or that has -// a local file, in identity order. +// loadResources returns each variation that the lock tracks or that has a +// local file, in identity order. func loadResources( repositoryRoot string, - manifestStore ManifestStore, + baselines syncmanifest.Baselines, projectKeys []string, -) ([]syncdomain.ResourceID, syncmanifest.Manifest, error) { - manifest, err := manifestStore.Load(projectKeys) +) ([]syncdomain.ResourceID, syncmanifest.Baseline, error) { + baseline, err := baselines.Load(projectKeys) if err != nil { - return nil, syncmanifest.Manifest{}, err + return nil, syncmanifest.Baseline{}, err } files, err := synclocal.SourceFiles(repositoryRoot) if err != nil { - return nil, syncmanifest.Manifest{}, err + return nil, syncmanifest.Baseline{}, err } var synced []syncdomain.ResourceID - for _, resource := range manifest.Resources { + for _, resource := range baseline.Lock.Resources { if resource.ResourceKind == syncdomain.KindVariation { synced = append(synced, resource.ID()) } @@ -138,16 +132,16 @@ func loadResources( } } slices.SortFunc(synced, syncdomain.CompareResourceIDs) - return slices.Compact(synced), manifest, nil + return slices.Compact(synced), baseline, nil } -// detachResources removes the selected variations from the manifest, and then -// deletes their local files. If the delete fails, it restores the manifest. -func detachResources(options Options, original syncmanifest.Manifest, selected []syncdomain.ResourceID) error { +// detachResources removes the selected variations from the baseline, and then +// deletes their local files. If the delete fails, it restores the baseline. +func detachResources(options Options, original syncmanifest.Baseline, selected []syncdomain.ResourceID) error { isSelected := func(id syncdomain.ResourceID) bool { return slices.Contains(selected, id) } next := syncmanifest.New() - for _, resource := range original.Resources { + for _, resource := range original.Lock.Resources { if !isSelected(resource.ID()) { next.Resources = append(next.Resources, resource) } @@ -160,12 +154,12 @@ func detachResources(options Options, original syncmanifest.Manifest, selected [ }) next.RemoveUnusedAttachments(remaining) } - persisted, err := options.Manifest.Update(original, next) + saved, err := options.Baselines.Save(original, next) if err != nil { return err } - restoreManifest := func(cause error) error { - _, restoreErr := options.Manifest.Update(persisted, original) + restoreBaseline := func(cause error) error { + _, restoreErr := options.Baselines.Save(saved, original.Lock) return errors.Join(cause, restoreErr) } @@ -177,7 +171,7 @@ func detachResources(options Options, original syncmanifest.Manifest, selected [ } exists, err := options.Store.VariationExists(resource.ProjectKey, configKey, variationKey) if err != nil { - return restoreManifest(err) + return restoreBaseline(err) } if exists { deletions = append(deletions, synclocal.VariationDeletion{ @@ -186,7 +180,7 @@ func detachResources(options Options, original syncmanifest.Manifest, selected [ } } if _, err := options.Store.DeleteVariations(deletions); err != nil { - return restoreManifest(err) + return restoreBaseline(err) } return nil } diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index 2d739bc0..7e6a27cb 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -77,13 +77,14 @@ func TestDetachResourcesPrunesUnreferencedAttachmentManifestEntries(t *testing.T resource := syncdomain.ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/prompt"} err = detachResources( - Options{RepositoryRoot: root, Store: store, Manifest: manifestStore}, - original, + Options{RepositoryRoot: root, Store: store, Baselines: manifestStore}, + syncmanifest.Baseline{Lock: original}, []syncdomain.ResourceID{resource}, ) require.NoError(t, err) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) assert.Empty(t, manifest.Resources) } @@ -112,7 +113,7 @@ func TestDetachResourcesRemovesWrapperAndManifestButKeepsReferencedFile(t *testi require.NoError(t, manifestStore.Write(original)) resource := syncdomain.ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/prompt"} - err = detachResources(Options{Store: store, Manifest: manifestStore}, original, []syncdomain.ResourceID{resource}) + err = detachResources(Options{Store: store, Baselines: manifestStore}, syncmanifest.Baseline{Lock: original}, []syncdomain.ResourceID{resource}) require.NoError(t, err) exists, err := store.VariationExists("project", "config", "prompt") @@ -120,7 +121,8 @@ func TestDetachResourcesRemovesWrapperAndManifestButKeepsReferencedFile(t *testi assert.False(t, exists) _, err = os.Stat(referencePath) require.NoError(t, err) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) assert.Empty(t, manifest.Resources) } @@ -138,10 +140,11 @@ func TestDetachResourcesRemovesManifestEntryWhenWrapperWasAlreadyDeleted(t *test require.NoError(t, manifestStore.Write(original)) resource := syncdomain.ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/deleted"} - err := detachResources(Options{Store: store, Manifest: manifestStore}, original, []syncdomain.ResourceID{resource}) + err := detachResources(Options{Store: store, Baselines: manifestStore}, syncmanifest.Baseline{Lock: original}, []syncdomain.ResourceID{resource}) require.NoError(t, err) - manifest, err := manifestStore.Load([]string{"project"}) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, err) assert.Empty(t, manifest.Resources) } @@ -157,13 +160,14 @@ func TestDetachResourcesDeletesUnreadableWrapper(t *testing.T) { resource := syncdomain.ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/broken"} err := detachResources( - Options{Store: store, Manifest: manifestStore}, - syncmanifest.New(), + Options{Store: store, Baselines: manifestStore}, + syncmanifest.Baseline{Lock: syncmanifest.New()}, []syncdomain.ResourceID{resource}, ) require.NoError(t, err) - manifest, loadErr := manifestStore.Load([]string{"project"}) + baseline, loadErr := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock require.NoError(t, loadErr) assert.Empty(t, manifest.Resources) _, statErr := os.Stat(wrapper) @@ -181,7 +185,7 @@ func TestRunRequiresTerminalWhenResourcesExist(t *testing.T) { err = Run(Options{ RepositoryRoot: root, Store: store, - Manifest: newMemoryManifestStore(), + Baselines: newMemoryManifestStore(), Input: bytes.NewBuffer(nil), Output: bytes.NewBuffer(nil), }) @@ -204,7 +208,7 @@ func TestRunUsesExplicitSelectionsWithoutTerminal(t *testing.T) { err = Run(Options{ RepositoryRoot: root, Store: store, - Manifest: manifestStore, + Baselines: manifestStore, Input: bytes.NewBuffer(nil), Output: bytes.NewBuffer(nil), Selections: []syncdomain.ResourceID{selection}, @@ -225,7 +229,7 @@ func TestRunReportsWhenNoResourcesAreSynced(t *testing.T) { err := Run(Options{ RepositoryRoot: root, Store: synclocal.NewStore(root), - Manifest: newMemoryManifestStore(), + Baselines: newMemoryManifestStore(), Input: bytes.NewBuffer(nil), Output: &output, }) @@ -241,7 +245,7 @@ func TestRunRejectsExplicitSelectionWhenNoResourcesAreSynced(t *testing.T) { err := Run(Options{ RepositoryRoot: root, Store: synclocal.NewStore(root), - Manifest: newMemoryManifestStore(), + Baselines: newMemoryManifestStore(), Input: bytes.NewBuffer(nil), Output: &output, Selections: []syncdomain.ResourceID{syncdomain.VariationID("production", "support", "default")}, @@ -261,17 +265,14 @@ func newMemoryManifestStore() *memoryManifestStore { return &memoryManifestStore{manifest: syncmanifest.New()} } -func (store *memoryManifestStore) Load(projectKeys []string) (syncmanifest.Manifest, error) { +func (store *memoryManifestStore) Load(projectKeys []string) (syncmanifest.Baseline, error) { store.loadedProjectKeys = append([]string(nil), projectKeys...) - return store.manifest, nil + return syncmanifest.Baseline{Lock: store.manifest}, nil } -func (store *memoryManifestStore) Update( - _ syncmanifest.Manifest, - next syncmanifest.Manifest, -) (syncmanifest.Manifest, error) { +func (store *memoryManifestStore) Save(_ syncmanifest.Baseline, next syncmanifest.Manifest) (syncmanifest.Baseline, error) { store.manifest = next - return next, nil + return syncmanifest.Baseline{Lock: next}, nil } func (store *memoryManifestStore) Write(manifest syncmanifest.Manifest) error { diff --git a/internal/sync/local/layout.go b/internal/sync/local/layout.go index 8e26ce4d..f992d7d3 100644 --- a/internal/sync/local/layout.go +++ b/internal/sync/local/layout.go @@ -15,6 +15,9 @@ import ( // .launchdarkly//configs//.prompt.md // .launchdarkly//tools/.json // .launchdarkly//skills/.md +// .launchdarkly/sync.lock +// +// A file directly in .launchdarkly, such as sync.lock, is not a resource. const ( configsDir = "configs" toolsDir = "tools" diff --git a/internal/sync/local/lock.go b/internal/sync/local/lock.go new file mode 100644 index 00000000..22abdc8f --- /dev/null +++ b/internal/sync/local/lock.go @@ -0,0 +1,50 @@ +package local + +import ( + "bytes" + "errors" + "os" + "path/filepath" +) + +// lockFileName is the file that stores the sync baseline of the working copy. +// The manifest package owns its format. +const lockFileName = "sync.lock" + +// ReadLock returns the content of the sync.lock file. The error matches +// fs.ErrNotExist when the file does not exist. +func (store Store) ReadLock() ([]byte, error) { + path := filepath.Join(store.root, lockFileName) + if err := rejectSymlinkedPath(store.root, path); err != nil { + return nil, err + } + return os.ReadFile(path) +} + +// WriteLock replaces the sync.lock file atomically. It does not write a file +// whose content is the same. Empty content removes the file. +func (store Store) WriteLock(content []byte) error { + path := filepath.Join(store.root, lockFileName) + if err := rejectSymlinkedPath(store.root, path); err != nil { + return err + } + if len(content) == 0 { + if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + return nil + } + if current, err := os.ReadFile(path); err == nil && bytes.Equal(current, content) { + return nil + } + + tempPath, err := writeTempFile(path, content, 0o644) + if err != nil { + return err + } + if err := os.Rename(tempPath, path); err != nil { + _ = os.Remove(tempPath) + return err + } + return nil +} diff --git a/internal/sync/local/lock_test.go b/internal/sync/local/lock_test.go new file mode 100644 index 00000000..6e1e9caa --- /dev/null +++ b/internal/sync/local/lock_test.go @@ -0,0 +1,47 @@ +package local + +import ( + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" +) + +func TestStoreReadsWritesAndRemovesLock(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(root, syncdomain.RootDir), 0o755)) + store := NewStore(root) + + _, err := store.ReadLock() + require.ErrorIs(t, err, fs.ErrNotExist) + + require.NoError(t, store.WriteLock([]byte("formatVersion: 1\n"))) + content, err := store.ReadLock() + require.NoError(t, err) + assert.Equal(t, "formatVersion: 1\n", string(content)) + + require.NoError(t, store.WriteLock(nil)) + _, err = store.ReadLock() + require.ErrorIs(t, err, fs.ErrNotExist) + require.NoError(t, store.WriteLock(nil)) +} + +func TestStoreRejectsSymlinkedLock(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(root, syncdomain.RootDir), 0o755)) + target := filepath.Join(t.TempDir(), "outside.lock") + require.NoError(t, os.WriteFile(target, []byte("outside"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(root, syncdomain.RootDir, lockFileName))) + + err := NewStore(root).WriteLock([]byte("formatVersion: 1\n")) + + require.ErrorContains(t, err, "symbolic links are not supported") + content, err := os.ReadFile(target) + require.NoError(t, err) + assert.Equal(t, "outside", string(content)) +} diff --git a/internal/sync/manifest/baseline.go b/internal/sync/manifest/baseline.go new file mode 100644 index 00000000..c0343833 --- /dev/null +++ b/internal/sync/manifest/baseline.go @@ -0,0 +1,97 @@ +package manifest + +import ( + "slices" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" +) + +// Baseline is the sync baseline of one working copy. +// +// Lock is the state that this working copy last agreed on with LaunchDarkly. +// Sync compares the local files and LaunchDarkly with Lock to find which side +// changed. The remote manifest is shared by every working copy of the +// repository. It supplies the version of each entry, which LaunchDarkly uses +// to reject two saves of one entry at the same time. +type Baseline struct { + Lock Manifest + remote Manifest + lockFileExists bool +} + +// HasLockFile reports whether the working copy has a sync.lock file. A +// working copy without one uses the remote manifest as its lock. +func (baseline Baseline) HasLockFile() bool { + return baseline.lockFileExists +} + +// Stale reports whether another working copy synced a different state of the +// resource after this working copy wrote its lock. +// +// The fingerprints decide, not the versions. A newer version with the same +// fingerprint means that LaunchDarkly came back to the state of the lock, so +// there is nothing to pull. A missing remote entry means that another working +// copy stopped syncing the resource, which does not change this working copy. +func (baseline Baseline) Stale(id syncdomain.ResourceID) bool { + lockIndex, remoteIndex := baseline.Lock.index(id), baseline.remote.index(id) + if lockIndex < 0 || remoteIndex < 0 { + return false + } + return baseline.Lock.Resources[lockIndex].Fingerprint != baseline.remote.Resources[remoteIndex].Fingerprint +} + +// Baselines loads and saves the baseline of a working copy. BaselineStore +// implements it. +type Baselines interface { + Load(projectKeys []string) (Baseline, error) + Save(current Baseline, next Manifest) (Baseline, error) +} + +// BaselineStore keeps the baseline in the sync.lock file and the entry +// versions in the remote manifest. +type BaselineStore struct { + remote Store + lock LockFile +} + +var _ Baselines = BaselineStore{} + +// NewBaselineStore creates a store for one working copy. +func NewBaselineStore(remote Store, lock LockFile) BaselineStore { + return BaselineStore{remote: remote, lock: lock} +} + +// Load reads the sync.lock file and the remote manifest of each project that +// projectKeys or the lock names. A working copy without a sync.lock file uses +// the remote manifest as its lock. Its next save then writes the file. +func (store BaselineStore) Load(projectKeys []string) (Baseline, error) { + lock, hasLock, err := ReadLock(store.lock) + if err != nil { + return Baseline{}, err + } + remote, err := store.remote.Load(append(slices.Clone(projectKeys), lock.ProjectKeys()...)) + if err != nil { + return Baseline{}, err + } + if !hasLock { + lock = remote.Clone() + } + return Baseline{Lock: lock, remote: remote, lockFileExists: hasLock}, nil +} + +// Save records next as the new baseline. It sends the entries that changed +// from current.Lock to next to the remote manifest, and then writes next to +// the sync.lock file with the versions that LaunchDarkly returned. If the +// remote save fails, the lock file does not change. +func (store BaselineStore) Save(current Baseline, next Manifest) (Baseline, error) { + remote, err := store.remote.Update(current.remote, current.remote.WithChanges(current.Lock, next)) + if err != nil { + return Baseline{}, err + } + saved := Baseline{Lock: next.WithVersionsFrom(remote), remote: remote, lockFileExists: true} + saved.Lock.Sort() + if err := writeLock(store.lock, saved.Lock); err != nil { + return Baseline{}, err + } + return saved, nil +} diff --git a/internal/sync/manifest/baseline_test.go b/internal/sync/manifest/baseline_test.go new file mode 100644 index 00000000..c84b5199 --- /dev/null +++ b/internal/sync/manifest/baseline_test.go @@ -0,0 +1,222 @@ +package manifest + +import ( + "io/fs" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncapi "github.com/launchdarkly/ldcli/internal/sync/api" +) + +const testSource = "git:example/repo" + +func TestLockFormatIsStable(t *testing.T) { + content, err := encodeLock(Manifest{Resources: []Resource{ + {ResourceKind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default", Fingerprint: fingerprint("a"), Version: 6}, + {ResourceKind: syncdomain.KindTool, ProjectKey: "production", LookupKey: "search", Fingerprint: fingerprint("b"), Version: 2}, + }}) + + require.NoError(t, err) + assert.Equal(t, `# Written by ldcli sync. Commit this file with the .launchdarkly files. +formatVersion: 1 +resources: + - kind: tool + project: production + key: search + fingerprint: `+fingerprint("b")+` + version: 2 + - kind: variation + project: production + key: support/default + fingerprint: `+fingerprint("a")+` + version: 6 +`, string(content)) + + decoded, err := decodeLock(content) + require.NoError(t, err) + assert.Len(t, decoded.Resources, 2) +} + +func TestDecodeLockRejectsInvalidContent(t *testing.T) { + for name, content := range map[string]string{ + "unknown field": "formatVersion: 1\nresources: []\nextra: true\n", + "unknown version": "formatVersion: 2\nresources: []\n", + "bad fingerprint": "formatVersion: 1\nresources:\n - kind: tool\n project: p\n key: k\n fingerprint: x\n", + } { + _, err := decodeLock([]byte(content)) + assert.Error(t, err, name) + } +} + +func TestBaselineStoreUsesRemoteManifestWithoutLockFile(t *testing.T) { + client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 3)}} + lock := &memoryLock{} + + baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load([]string{"production"}) + + require.NoError(t, err) + assert.False(t, baseline.HasLockFile()) + require.Len(t, baseline.Lock.Resources, 1) + assert.Equal(t, fingerprint("a"), baseline.Lock.Resources[0].Fingerprint) + assert.False(t, baseline.Stale(variationID())) +} + +func TestBaselineStoreUsesLockFileAsBaselineAndReportsStaleness(t *testing.T) { + client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}} + lock := lockWith(t, fingerprint("a"), 4) + + baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load(nil) + + require.NoError(t, err) + assert.True(t, baseline.HasLockFile()) + assert.Equal(t, fingerprint("a"), baseline.Lock.Resources[0].Fingerprint) + assert.True(t, baseline.Stale(variationID())) +} + +func TestBaselineStaleComparesFingerprints(t *testing.T) { + tests := map[string]struct { + remote syncapi.SyncManifest + stale bool + }{ + "newer version with a different fingerprint": {remote: remoteManifest(fingerprint("b"), 5), stale: true}, + "newer version with the same fingerprint": {remote: remoteManifest(fingerprint("a"), 5)}, + "same version with a different fingerprint": {remote: remoteManifest(fingerprint("b"), 4), stale: true}, + "remote entry removed": {remote: syncapi.SyncManifest{Source: testSource}}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": test.remote}} + baseline, err := NewBaselineStore(NewStore(client, testSource), lockWith(t, fingerprint("a"), 4)).Load(nil) + + require.NoError(t, err) + assert.Equal(t, test.stale, baseline.Stale(variationID())) + }) + } +} + +// The version in each request must be the version that sync read from +// LaunchDarkly, so that LaunchDarkly rejects a save that races another save. +func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLock(t *testing.T) { + client := &manifestClient{ + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}, + patchResponses: []syncapi.SyncManifest{{ + Source: testSource, + Items: []syncapi.SyncManifestResource{ + {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint("c"), Version: 6}, + {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 1}, + }, + }}, + } + lock := lockWith(t, fingerprint("a"), 4) + store := NewBaselineStore(NewStore(client, testSource), lock) + baseline, err := store.Load(nil) + require.NoError(t, err) + + next := baseline.Lock.Clone() + next.SetFingerprint(variationID(), fingerprint("c")) + next.SetFingerprint(syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: "production", LookupKey: "search"}, fingerprint("d")) + saved, err := store.Save(baseline, next) + + require.NoError(t, err) + require.Len(t, client.patches, 1) + assert.Equal(t, []syncapi.SyncManifestUpsert{ + {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 0}, + {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint("c"), Version: 5}, + }, client.patches[0].upserts) + written, _, err := ReadLock(lock) + require.NoError(t, err) + assert.Equal(t, saved.Lock, written) + assert.Equal(t, 6, written.Resources[written.index(variationID())].Version) + assert.False(t, saved.Stale(variationID())) +} + +func TestBaselineStoreSaveKeepsRemoteEntryThatThisCopyDidNotChange(t *testing.T) { + client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}} + lock := lockWith(t, fingerprint("a"), 4) + store := NewBaselineStore(NewStore(client, testSource), lock) + baseline, err := store.Load(nil) + require.NoError(t, err) + + _, err = store.Save(baseline, baseline.Lock) + + require.NoError(t, err) + assert.Empty(t, client.patches) +} + +func TestBaselineStoreSaveDoesNotWriteLockWhenRemoteSaveFails(t *testing.T) { + client := &manifestClient{ + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 4)}, + patchErr: uncertainPatchError(t), + } + lock := lockWith(t, fingerprint("a"), 4) + before := string(lock.content) + store := NewBaselineStore(NewStore(client, testSource), lock) + baseline, err := store.Load(nil) + require.NoError(t, err) + + next := baseline.Lock.Clone() + next.SetFingerprint(variationID(), fingerprint("c")) + _, err = store.Save(baseline, next) + + require.Error(t, err) + assert.Equal(t, before, string(lock.content)) +} + +func TestBaselineStoreSaveRemovesEmptyLock(t *testing.T) { + client := &manifestClient{ + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 4)}, + patchResponses: []syncapi.SyncManifest{{Source: testSource}}, + } + lock := lockWith(t, fingerprint("a"), 4) + store := NewBaselineStore(NewStore(client, testSource), lock) + baseline, err := store.Load(nil) + require.NoError(t, err) + + _, err = store.Save(baseline, New()) + + require.NoError(t, err) + assert.Nil(t, lock.content) + assert.Equal(t, []syncapi.SyncManifestDeletion{{ + ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Version: 4, + }}, client.patches[0].deletions) +} + +func variationID() syncdomain.ResourceID { + return syncdomain.VariationID("production", "support", "default") +} + +func remoteManifest(fingerprint string, version int) syncapi.SyncManifest { + return syncapi.SyncManifest{Source: testSource, Items: []syncapi.SyncManifestResource{{ + ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint, Version: version, + }}} +} + +func lockWith(t *testing.T, fingerprint string, version int) *memoryLock { + t.Helper() + content, err := encodeLock(Manifest{Resources: []Resource{{ + ResourceKind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default", + Fingerprint: fingerprint, Version: version, + }}}) + require.NoError(t, err) + return &memoryLock{content: content} +} + +type memoryLock struct { + content []byte +} + +func (lock *memoryLock) ReadLock() ([]byte, error) { + if lock.content == nil { + return nil, fs.ErrNotExist + } + return lock.content, nil +} + +func (lock *memoryLock) WriteLock(content []byte) error { + lock.content = content + return nil +} diff --git a/internal/sync/manifest/lock.go b/internal/sync/manifest/lock.go new file mode 100644 index 00000000..800671c1 --- /dev/null +++ b/internal/sync/manifest/lock.go @@ -0,0 +1,105 @@ +package manifest + +import ( + "bytes" + "errors" + "fmt" + "io/fs" + + "gopkg.in/yaml.v3" +) + +// The sync.lock file stores the baseline of one working copy. Engineers commit +// it with the .launchdarkly files, so that the baseline moves with the files. +const ( + lockFormatVersion = 1 + lockHeader = "# Written by ldcli sync. Commit this file with the .launchdarkly files.\n" +) + +// LockFile reads and writes the sync.lock file of a working copy. ReadLock +// returns an error that matches fs.ErrNotExist when the file does not exist. +// WriteLock with empty content removes the file. +type LockFile interface { + ReadLock() ([]byte, error) + WriteLock(content []byte) error +} + +type lockDocument struct { + FormatVersion int `yaml:"formatVersion"` + Resources []Resource `yaml:"resources"` +} + +// ReadLock decodes the sync.lock file. The bool result is false when the file +// does not exist. +func ReadLock(lock LockFile) (Manifest, bool, error) { + content, err := lock.ReadLock() + if errors.Is(err, fs.ErrNotExist) { + return New(), false, nil + } + if err != nil { + return Manifest{}, false, fmt.Errorf("read sync.lock: %w", err) + } + manifest, err := decodeLock(content) + if err != nil { + return Manifest{}, false, fmt.Errorf("read sync.lock: %w", err) + } + return manifest, true, nil +} + +// writeLock encodes the manifest to the sync.lock file. A manifest without +// entries removes the file, so that a workspace without resources has no +// .launchdarkly directory. +func writeLock(lock LockFile, manifest Manifest) error { + content, err := encodeLock(manifest) + if err != nil { + return err + } + if err := lock.WriteLock(content); err != nil { + return fmt.Errorf("write sync.lock: %w", err) + } + return nil +} + +// encodeLock returns the lock content, with the entries in identity order so +// that a Git diff shows only the entries that changed. +func encodeLock(manifest Manifest) ([]byte, error) { + if len(manifest.Resources) == 0 { + return nil, nil + } + manifest = manifest.Clone() + manifest.Sort() + + var content bytes.Buffer + content.WriteString(lockHeader) + encoder := yaml.NewEncoder(&content) + encoder.SetIndent(2) + if err := encoder.Encode(lockDocument{FormatVersion: lockFormatVersion, Resources: manifest.Resources}); err != nil { + return nil, fmt.Errorf("encode sync.lock: %w", err) + } + if err := encoder.Close(); err != nil { + return nil, fmt.Errorf("encode sync.lock: %w", err) + } + return content.Bytes(), nil +} + +func decodeLock(content []byte) (Manifest, error) { + var document lockDocument + decoder := yaml.NewDecoder(bytes.NewReader(content)) + decoder.KnownFields(true) + if err := decoder.Decode(&document); err != nil { + return Manifest{}, fmt.Errorf("decode: %w", err) + } + if document.FormatVersion != lockFormatVersion { + return Manifest{}, fmt.Errorf("unsupported formatVersion %d", document.FormatVersion) + } + + manifest := Manifest{Resources: document.Resources} + if manifest.Resources == nil { + manifest = New() + } + if err := manifest.Validate(); err != nil { + return Manifest{}, err + } + manifest.Sort() + return manifest, nil +} diff --git a/internal/sync/manifest/model.go b/internal/sync/manifest/model.go index 95cddf4e..92dd0ef6 100644 --- a/internal/sync/manifest/model.go +++ b/internal/sync/manifest/model.go @@ -20,13 +20,14 @@ type Manifest struct { } // Resource is the baseline of one resource. Version is the version of the -// remote manifest entry, which LaunchDarkly uses for optimistic locking. +// remote manifest entry, which LaunchDarkly uses for optimistic locking. The +// YAML tags are the format of the sync.lock file. type Resource struct { - ResourceKind syncdomain.Kind - ProjectKey string - LookupKey string - Fingerprint string - Version int + ResourceKind syncdomain.Kind `yaml:"kind"` + ProjectKey string `yaml:"project"` + LookupKey string `yaml:"key"` + Fingerprint string `yaml:"fingerprint"` + Version int `yaml:"version"` } // ID returns the identity of the resource. @@ -110,6 +111,47 @@ func (manifest *Manifest) RemoveUnusedAttachments(variations []syncdomain.Synced }) } +// WithChanges returns a copy of the manifest with the entry changes from +// before to after. An entry that is the same in before and after keeps the +// value that the manifest has, which can come from another working copy. +func (manifest Manifest) WithChanges(before, after Manifest) Manifest { + result := manifest.Clone() + for _, resource := range after.Resources { + if index := before.index(resource.ID()); index < 0 || before.Resources[index].Fingerprint != resource.Fingerprint { + result.SetFingerprint(resource.ID(), resource.Fingerprint) + } + } + for _, resource := range before.Resources { + if after.index(resource.ID()) < 0 { + result.Remove(resource.ID()) + } + } + return result +} + +// WithVersionsFrom returns a copy of the manifest in which each entry has the +// version of the same entry in source, or 0 if source does not have it. +func (manifest Manifest) WithVersionsFrom(source Manifest) Manifest { + versions := make(map[syncdomain.ResourceID]int, len(source.Resources)) + for _, resource := range source.Resources { + versions[resource.ID()] = resource.Version + } + result := manifest.Clone() + for index := range result.Resources { + result.Resources[index].Version = versions[result.Resources[index].ID()] + } + return result +} + +// ProjectKeys returns each project that has an entry, in order. +func (manifest Manifest) ProjectKeys() []string { + keys := make([]string, 0, len(manifest.Resources)) + for _, resource := range manifest.Resources { + keys = append(keys, resource.ProjectKey) + } + return uniqueSorted(keys) +} + // Validate makes sure that each entry has a safe identity, a valid // fingerprint, and a unique identity. func (manifest Manifest) Validate() error { diff --git a/internal/sync/manifest/store.go b/internal/sync/manifest/store.go index 23fccf08..66c8d9f6 100644 --- a/internal/sync/manifest/store.go +++ b/internal/sync/manifest/store.go @@ -77,7 +77,8 @@ func (store Store) Update(previous, next Manifest) (Manifest, error) { after := next.project(projectKey) upserts, deletions := changes(before, after) if len(upserts) == 0 && len(deletions) == 0 { - result.Resources = append(result.Resources, withVersions(after, before)...) + unchanged := Manifest{Resources: after}.WithVersionsFrom(Manifest{Resources: before}) + result.Resources = append(result.Resources, unchanged.Resources...) continue } @@ -220,20 +221,6 @@ func changesApplied( return true } -// withVersions copies the remote version of each entry in before to the -// matching entry in after. -func withVersions(after, before []Resource) []Resource { - versions := make(map[syncdomain.ResourceID]int, len(before)) - for _, resource := range before { - versions[resource.ID()] = resource.Version - } - result := slices.Clone(after) - for index := range result { - result[index].Version = versions[result[index].ID()] - } - return result -} - // project returns the entries of one project. func (manifest Manifest) project(projectKey string) []Resource { var resources []Resource @@ -251,14 +238,8 @@ func sorted(resources []Resource) []Resource { return manifest.Resources } -func projectKeys(manifests ...Manifest) []string { - var keys []string - for _, manifest := range manifests { - for _, resource := range manifest.Resources { - keys = append(keys, resource.ProjectKey) - } - } - return uniqueSorted(keys) +func projectKeys(previous, next Manifest) []string { + return uniqueSorted(append(previous.ProjectKeys(), next.ProjectKeys()...)) } func uniqueSorted(values []string) []string { diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 6a3f923f..81bd5c89 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -758,20 +758,52 @@ func TestPromptServerDeletionLeavesReferencedFile(t *testing.T) { assert.Empty(t, resources) } +// A working copy that is behind another working copy must pull the newer +// LaunchDarkly state. It must not write its older file back to LaunchDarkly. +func TestPromptStaleWorkingCopyPullsInsteadOfReverting(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, false) + writeManifest(t, root, baseline) + api := &directAPI{variation: pointer(baseline)} + _, _, err := runPrompt(t, root, api, "--yes") + require.NoError(t, err) + + // Another working copy syncs a change. The shared remote manifest moves. + other := variation("Other working copy") + api.variation = pointer(other) + writeManifest(t, root, other) + manifestsByRoot[root].Items[0].Version = 2 + + _, review, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Equal(t, other.Name, api.variation.Name) + assert.False(t, slices.ContainsFunc(api.requests, func(request string) bool { + return strings.HasPrefix(request, "PATCH ") && strings.Contains(request, "/variations/") + }), "sync wrote the older local file back to LaunchDarkly") + assert.Contains(t, review, "Another working copy synced this variation after your sync.lock.") + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + assert.Equal(t, other.Name, resources[0].Variation.Name) +} + func TestPromptPropagatesTrackedLocalDeletion(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") writeVariation(t, root, baseline, false) writeManifest(t, root, baseline) - command := exec.Command("git", "add", ".launchdarkly") - command.Dir = root - output, err := command.CombinedOutput() - require.NoError(t, err, "%s", output) + api := &directAPI{variation: pointer(baseline)} + + // The first sync writes sync.lock, which tracks the project after its + // files are gone. + _, _, err := runPrompt(t, root, api, "--yes") + require.NoError(t, err) _, err = synclocal.NewStore(root).DeleteVariations([]synclocal.VariationDeletion{{ ProjectKey: "production", ConfigKey: "support", VariationKey: "default", }}) require.NoError(t, err) - api := &directAPI{variation: pointer(baseline)} _, _, err = runPrompt(t, root, api, "--yes") diff --git a/internal/sync/prompt/conflict_test.go b/internal/sync/prompt/conflict_test.go index f87b2f76..26089b55 100644 --- a/internal/sync/prompt/conflict_test.go +++ b/internal/sync/prompt/conflict_test.go @@ -267,7 +267,7 @@ func TestRunWorkspaceSyncAppliesConflictChoiceAfterRevalidation(t *testing.T) { AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, Output: &output, ErrorOutput: &output, }, - syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + syncWorkspace{root: root, local: localStore, baselines: manifestStore}, nil, ) @@ -292,7 +292,7 @@ func TestRunWorkspaceSyncAbortsConflictWithoutWriting(t *testing.T) { AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, Output: &output, ErrorOutput: &output, }, - syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + syncWorkspace{root: root, local: localStore, baselines: manifestStore}, nil, ) @@ -318,7 +318,7 @@ func TestRunWorkspaceSyncAbortsAttachmentConflictWithoutWriting(t *testing.T) { AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, Output: &output, ErrorOutput: &output, }, - syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + syncWorkspace{root: root, local: localStore, baselines: manifestStore}, nil, ) @@ -349,7 +349,7 @@ func TestRunWorkspaceSyncUsesLaunchDarklyForAttachmentConflict(t *testing.T) { AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, Output: &output, ErrorOutput: &output, }, - syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + syncWorkspace{root: root, local: localStore, baselines: manifestStore}, nil, ) @@ -416,7 +416,7 @@ func divergentPlan(t *testing.T) Plan { }) } -func writeConflictWorkspace(t *testing.T, root string, baseline, local syncdomain.Variation) (synclocal.Store, manifestStore) { +func writeConflictWorkspace(t *testing.T, root string, baseline, local syncdomain.Variation) (synclocal.Store, syncmanifest.Baselines) { t.Helper() command := exec.Command("git", "init", "--quiet") @@ -445,16 +445,13 @@ type memoryManifestStore struct { manifest syncmanifest.Manifest } -func (store *memoryManifestStore) Load([]string) (syncmanifest.Manifest, error) { - return store.manifest, nil +func (store *memoryManifestStore) Load([]string) (syncmanifest.Baseline, error) { + return syncmanifest.Baseline{Lock: store.manifest}, nil } -func (store *memoryManifestStore) Update( - _ syncmanifest.Manifest, - next syncmanifest.Manifest, -) (syncmanifest.Manifest, error) { +func (store *memoryManifestStore) Save(_ syncmanifest.Baseline, next syncmanifest.Manifest) (syncmanifest.Baseline, error) { store.manifest = next - return next, nil + return syncmanifest.Baseline{Lock: next}, nil } type conflictAPI struct { diff --git a/internal/sync/prompt/output.go b/internal/sync/prompt/output.go index 7b1fff26..8f22c2d2 100644 --- a/internal/sync/prompt/output.go +++ b/internal/sync/prompt/output.go @@ -57,17 +57,21 @@ func writePlanOutput(out io.Writer, outputKind string, plan Plan) error { type planResourceOutput struct { resourceOutput - Action Action `json:"action"` - Error string `json:"error,omitempty"` - Diff variationDiffFields `json:"diff,omitempty"` + Action Action `json:"action"` + SyncedElsewhere bool `json:"syncedElsewhere,omitempty"` + Suggestion string `json:"suggestion,omitempty"` + Error string `json:"error,omitempty"` + Diff variationDiffFields `json:"diff,omitempty"` } resources := make([]planResourceOutput, 0, len(plan.Resources)) for _, resource := range plan.Resources { resources = append(resources, planResourceOutput{ - resourceOutput: newResourceOutput(resource.ID), - Action: resource.Action, - Error: resource.Error, - Diff: resource.Diff, + resourceOutput: newResourceOutput(resource.ID), + Action: resource.Action, + SyncedElsewhere: resource.SyncedElsewhere, + Suggestion: staleSuggestion(resource), + Error: resource.Error, + Diff: resource.Diff, }) } return writeJSON(out, map[string]any{"resources": resources}) @@ -127,6 +131,13 @@ func writePlanReview(out io.Writer, outputKind string, plan Plan, width int) err return nil } markdown := outputKind == outputMarkdown + detail := func(label, text string) { + if markdown { + _ = console.Printf("%s: %s\n", label, text) + } else { + _ = console.Printf(" %s: %s\n", label, text) + } + } currentProject, currentConfig := "", "" for _, resource := range plan.Resources { @@ -157,12 +168,12 @@ func writePlanReview(out io.Writer, outputKind string, plan Plan, width int) err _ = console.Printf("\n %s\n Action: %s\n", reviewHeading("Variation: "+variationKey, width), action) } + if resource.SyncedElsewhere { + detail("Note", "Another working copy synced this variation after your sync.lock.") + detail("Suggestion", staleSuggestion(resource)) + } if resource.Error != "" { - if markdown { - _ = console.Printf("Error: %s\n", resource.Error) - } else { - _ = console.Printf(" Error: %s\n", resource.Error) - } + detail("Error", resource.Error) } if len(resource.Diff) != 0 { rendered, err := renderVariationDiff(resource.Diff, outputKind, width, diffPresentation(resource.Action)) @@ -175,6 +186,23 @@ func writePlanReview(out io.Writer, outputKind string, plan Plan, width int) err return nil } +// staleSuggestion tells the user what to do when another working copy synced +// the resource. It returns an empty string for a resource that is not stale. +func staleSuggestion(resource PlannedResource) string { + switch { + case !resource.SyncedElsewhere: + return "" + case resource.Action == ActionUpdateLocal: + return "If the other working copy pushed its change to Git, run git pull before you sync. " + + "Then your sync.lock does not get a merge conflict." + case resource.Action == ActionConflict: + return "Run git pull to get the other change, and then run sync again. " + + "If the conflict remains, choose a side with --conflict or --resolve." + default: + return "Run git pull to get the latest .launchdarkly files and sync.lock, and then run sync again." + } +} + // reviewHeading colors a heading when the output is a terminal. func reviewHeading(value string, width int) string { if width <= 0 { diff --git a/internal/sync/prompt/output_test.go b/internal/sync/prompt/output_test.go index ff170880..d09b661a 100644 --- a/internal/sync/prompt/output_test.go +++ b/internal/sync/prompt/output_test.go @@ -57,6 +57,26 @@ func TestWritePlanDescribesArchivedLaunchDarklyVariation(t *testing.T) { assert.Contains(t, output.String(), "(archived in LaunchDarkly)") } +func TestWritePlanNotesResourceSyncedByAnotherWorkingCopy(t *testing.T) { + plan := Plan{Resources: []PlannedResource{{ID: testResourceID(), Action: ActionUpdateLocal, SyncedElsewhere: true}}} + + var text, data bytes.Buffer + require.NoError(t, writePlanOutput(&text, "plaintext", plan)) + require.NoError(t, writePlanOutput(&data, "json", plan)) + + assert.Contains(t, text.String(), "Note: Another working copy synced this variation after your sync.lock.") + assert.Contains(t, text.String(), "Suggestion: If the other working copy pushed its change to Git, run git pull before you sync.") + assert.Contains(t, data.String(), `"syncedElsewhere": true`) + assert.Contains(t, data.String(), `"suggestion": "If the other working copy pushed its change to Git`) +} + +func TestStaleSuggestionMatchesTheAction(t *testing.T) { + assert.Empty(t, staleSuggestion(PlannedResource{Action: ActionUpdateLocal})) + assert.Contains(t, staleSuggestion(PlannedResource{Action: ActionUpdateLocal, SyncedElsewhere: true}), "run git pull before you sync") + assert.Contains(t, staleSuggestion(PlannedResource{Action: ActionConflict, SyncedElsewhere: true}), "--conflict or --resolve") + assert.Contains(t, staleSuggestion(PlannedResource{Action: ActionInSync, SyncedElsewhere: true}), "run sync again") +} + func TestWritePlanRendersHumanFriendlyAttachmentDiff(t *testing.T) { server := testVariation("Support") local := server diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 79c47f83..f58cfef1 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -57,6 +57,10 @@ type PlannedResource struct { ServerMode syncdomain.VariationMode Local *syncdomain.Variation Server *syncdomain.Variation + // SyncedElsewhere is true when another working copy synced the resource + // after this working copy wrote its sync.lock file. The plan is still + // correct, because it compares against this working copy's lock. + SyncedElsewhere bool // ServerHasStaleAttachmentPins is true when LaunchDarkly pins an older // version of a tool or skill than its latest version. ServerHasStaleAttachmentPins bool diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 8b770ce1..f372147b 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -24,16 +24,12 @@ import ( syncsource "github.com/launchdarkly/ldcli/internal/sync/source" ) -type manifestStore interface { - Load(projectKeys []string) (syncmanifest.Manifest, error) - Update(previous, next syncmanifest.Manifest) (syncmanifest.Manifest, error) -} - // syncWorkspace is the Git repository that one command syncs. type syncWorkspace struct { - root string - local synclocal.Store - manifest manifestStore + root string + local synclocal.Store + // baselines keeps the sync.lock file and the remote manifest. + baselines syncmanifest.Baselines } // Runner runs the prompt sync commands. Tests replace its function fields. @@ -71,10 +67,11 @@ func (runner Runner) Run(options Options) error { if err != nil { return err } + local := synclocal.NewStore(resolved.Root) workspace := syncWorkspace{ - root: resolved.Root, - local: synclocal.NewStore(resolved.Root), - manifest: syncmanifest.NewStore(runner.api(options), resolved.Source), + root: resolved.Root, + local: local, + baselines: syncmanifest.NewBaselineStore(syncmanifest.NewStore(runner.api(options), resolved.Source), local), } switch action := options.Action.(type) { @@ -109,7 +106,7 @@ func (runner Runner) runSync(options Options, workspace syncWorkspace, action Sy if err != nil { return err } - projectKeys, err := discoverProjectKeys(workspace.root) + projectKeys, err := workspace.projectKeys() if err != nil { return err } @@ -145,14 +142,14 @@ func (runner Runner) runAdd(options Options, workspace syncWorkspace, action Add } func (runner Runner) runDetach(options Options, workspace syncWorkspace, action DetachAction) error { - projectKeys, err := discoverProjectKeys(workspace.root) + projectKeys, err := workspace.projectKeys() if err != nil { return err } return runner.detach(syncdetach.Options{ RepositoryRoot: workspace.root, Store: workspace.local, - Manifest: workspace.manifest, + Baselines: workspace.baselines, ProjectKeys: projectKeys, Input: options.Input, Output: options.Output, @@ -224,7 +221,7 @@ func (runner Runner) bootstrapOptions( Catalog: api, Attachments: api, Store: workspace.local, - Manifest: workspace.manifest, + Baselines: workspace.baselines, Input: options.Input, Output: options.Output, Initial: initial, diff --git a/internal/sync/prompt/runner_test.go b/internal/sync/prompt/runner_test.go index 55aef41a..ec0a45da 100644 --- a/internal/sync/prompt/runner_test.go +++ b/internal/sync/prompt/runner_test.go @@ -164,7 +164,7 @@ func TestRunnerDetachesWithoutCallingTheAPI(t *testing.T) { runner.detach = func(options syncdetach.Options) error { called = true assert.NotZero(t, options.Store) - assert.NotZero(t, options.Manifest) + assert.NotZero(t, options.Baselines) assert.Equal(t, os.Stdin, options.Input) assert.Equal(t, io.Discard, options.Output) return nil diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 7c7a0c98..083d0981 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -8,7 +8,6 @@ import ( syncapi "github.com/launchdarkly/ldcli/internal/sync/api" synclocal "github.com/launchdarkly/ldcli/internal/sync/local" syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" - syncrepository "github.com/launchdarkly/ldcli/internal/sync/repository" ) // workspaceState is everything that one plan depends on. The plan compares @@ -16,37 +15,37 @@ import ( // stores it, so that a local write keeps the form of the file. type workspaceState struct { projectKeys []string - manifest syncmanifest.Manifest + baseline syncmanifest.Baseline plan Plan localFiles map[ResourceID]syncdomain.SyncedResource } -// loadState reads the manifest, the local files, and LaunchDarkly, and -// builds the plan. +// loadState reads the baseline, the local files, and LaunchDarkly, and builds +// the plan. func (workspace syncWorkspace) loadState(client syncapi.Client) (workspaceState, error) { - projectKeys, err := discoverProjectKeys(workspace.root) + projectKeys, err := workspace.projectKeys() if err != nil { return workspaceState{}, err } - // The manifest is the common ancestor in a three-way comparison of the - // local files and LaunchDarkly. - manifest, err := workspace.manifest.Load(projectKeys) + // The lock is the common ancestor in a three-way comparison of the local + // files and LaunchDarkly. + baseline, err := workspace.baselines.Load(projectKeys) if err != nil { return workspaceState{}, err } - plan, localFiles, err := loadWorkspacePlan(workspace.root, manifest, client) + plan, localFiles, err := loadWorkspacePlan(workspace.root, baseline, client) if err != nil { return workspaceState{}, err } - return workspaceState{projectKeys: projectKeys, manifest: manifest, plan: plan, localFiles: localFiles}, nil + return workspaceState{projectKeys: projectKeys, baseline: baseline, plan: plan, localFiles: localFiles}, nil } // loadWorkspacePlan reads the local variations and their LaunchDarkly -// versions, and compares both with the baseline. It also returns the local +// versions, and compares both with the lock. It also returns the local // variations as their files store them. func loadWorkspacePlan( repositoryRoot string, - baseline syncmanifest.Manifest, + baseline syncmanifest.Baseline, client syncapi.Client, ) (Plan, map[ResourceID]syncdomain.SyncedResource, error) { localFiles, err := compileWorkspace(repositoryRoot) @@ -59,12 +58,12 @@ func loadWorkspacePlan( } localFilesByID := make(map[ResourceID]syncdomain.SyncedResource, len(localFiles)) - ids := make(map[ResourceID]struct{}, len(localFiles)+len(baseline.Resources)) + ids := make(map[ResourceID]struct{}, len(localFiles)+len(baseline.Lock.Resources)) for _, resource := range localFiles { localFilesByID[resource.ID()] = resource ids[resource.ID()] = struct{}{} } - for _, resource := range baseline.Resources { + for _, resource := range baseline.Lock.Resources { if resource.ResourceKind == syncdomain.KindVariation { ids[resource.ID()] = struct{}{} } @@ -79,7 +78,11 @@ func loadWorkspacePlan( } server[id] = resource } - return BuildPlan(baseline, local, server), localFilesByID, nil + plan := BuildPlan(baseline.Lock, local, server) + for index := range plan.Resources { + plan.Resources[index].SyncedElsewhere = baseline.Stale(plan.Resources[index].ID) + } + return plan, localFilesByID, nil } // readServerResource reads one variation from LaunchDarkly with the content @@ -104,20 +107,21 @@ func readServerResource(client syncapi.Client, attachments *attachmentCache, id return resource, nil } -// discoverProjectKeys returns each project that has a local sync file, or -// that had one before a deletion that Git reports. -func discoverProjectKeys(repositoryRoot string) ([]string, error) { - files, err := synclocal.SourceFiles(repositoryRoot) +// projectKeys returns each project that has a local sync file or an entry in +// the sync.lock file. A project whose files are all missing is still in the +// lock, so sync can restore its files. +func (workspace syncWorkspace) projectKeys() ([]string, error) { + files, err := synclocal.SourceFiles(workspace.root) if err != nil { return nil, err } - deleted, err := syncrepository.DeletedPaths(repositoryRoot) + lock, _, err := syncmanifest.ReadLock(workspace.local) if err != nil { return nil, err } - var projectKeys []string - for _, file := range append(files, deleted...) { + projectKeys := lock.ProjectKeys() + for _, file := range files { if id, ok := synclocal.ParseManagedPath(file); ok { projectKeys = append(projectKeys, id.ProjectKey) } diff --git a/internal/sync/prompt/sync.go b/internal/sync/prompt/sync.go index f70b2733..77e35535 100644 --- a/internal/sync/prompt/sync.go +++ b/internal/sync/prompt/sync.go @@ -10,7 +10,7 @@ import ( // runWorkspaceSync runs one sync in five steps: // -// 1. Read the manifest, the local files, and LaunchDarkly, and build a plan. +// 1. Read the baseline, the local files, and LaunchDarkly, and build a plan. // 2. Ask the user to resolve each conflict. // 3. Show the plan and ask the user to apply it. // 4. Read the state again, and stop if it changed after the review. @@ -60,6 +60,12 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, } if !proceed { if !resolved.HasChanges() { + // A workspace from before sync.lock gets the file on its first sync. + if !reviewed.baseline.HasLockFile() { + if _, err := workspace.baselines.Save(reviewed.baseline, reviewed.baseline.Lock); err != nil { + return err + } + } return cleanupOrphanedAttachments(options, workspace.local, interactive) } return nil @@ -102,10 +108,10 @@ func (runner Runner) applyPlan( interactive bool, ) error { plan := applyConflictResolutions(current.plan, resolutions) - outcomes, next, err := executePlan(workspace.root, workspace.local, client, current.manifest, plan, current.localFiles) + outcomes, next, err := executePlan(workspace.root, workspace.local, client, current.baseline.Lock, plan, current.localFiles) failures := []error{err} - if _, err := workspace.manifest.Update(current.manifest, next); err != nil { + if _, err := workspace.baselines.Save(current.baseline, next); err != nil { failures = append(failures, err) } if err := workspace.local.RemoveEmptyDirectories(); err != nil { diff --git a/internal/sync/prompt/watch_test.go b/internal/sync/prompt/watch_test.go index 7b4ea549..4b60bdba 100644 --- a/internal/sync/prompt/watch_test.go +++ b/internal/sync/prompt/watch_test.go @@ -457,3 +457,16 @@ func TestSourceWatcherRecognizesNewManagedResourceKinds(t *testing.T) { Op: fsnotify.Create, })) } + +func TestSourceWatcherIgnoresSyncLock(t *testing.T) { + root := t.TempDir() + managedRoot := filepath.Join(root, syncdomain.RootDir) + lockFile := filepath.Join(managedRoot, "sync.lock") + require.NoError(t, os.MkdirAll(managedRoot, 0o755)) + require.NoError(t, os.WriteFile(lockFile, []byte("formatVersion: 1\n"), 0o644)) + watcher := sourceWatcher{managedRoot: managedRoot, files: make(map[string]struct{})} + + for _, op := range []fsnotify.Op{fsnotify.Create, fsnotify.Write, fsnotify.Rename} { + require.False(t, watcher.relevant(fsnotify.Event{Name: lockFile, Op: op}), op.String()) + } +} diff --git a/internal/sync/repository/git.go b/internal/sync/repository/git.go index 076a837a..4cdd5ea3 100644 --- a/internal/sync/repository/git.go +++ b/internal/sync/repository/git.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "os/exec" - "slices" "strings" ) @@ -55,11 +54,6 @@ func FindGitRepository(dir string) (GitRepository, bool, error) { return findGitRepository(execGit{}, dir) } -// DeletedPaths returns staged and unstaged deleted paths relative to the repository. -func DeletedPaths(repositoryRoot string) ([]string, error) { - return deletedPaths(execGit{}, repositoryRoot) -} - // findGitRepository contains the injectable repository-discovery workflow used // by the real command and focused tests. func findGitRepository(git gitRunner, dir string) (GitRepository, bool, error) { @@ -87,33 +81,3 @@ func findGitRepository(git gitRunner, dir string) (GitRepository, bool, error) { } return GitRepository{Root: root, Origin: origin}, true, nil } - -func deletedPaths(git gitRunner, repositoryRoot string) ([]string, error) { - var paths []string - commands := [][]string{ - {"diff", "--name-only", "--diff-filter=D", "-z", "--", ".launchdarkly"}, - {"diff", "--cached", "--name-only", "--diff-filter=D", "-z", "--", ".launchdarkly"}, - } - for _, command := range commands { - output, stderr, err := git.output(repositoryRoot, command...) - if err != nil { - if stderr != "" { - return nil, fmt.Errorf("find deleted sync files: %s: %w", stderr, err) - } - return nil, fmt.Errorf("find deleted sync files: %w", err) - } - paths = append(paths, splitNullTerminated(output)...) - } - slices.Sort(paths) - return slices.Compact(paths), nil -} - -func splitNullTerminated(value string) []string { - var values []string - for _, item := range strings.Split(value, "\x00") { - if item != "" { - values = append(values, item) - } - } - return values -} diff --git a/internal/sync/repository/git_test.go b/internal/sync/repository/git_test.go index f3b829d9..3c57b317 100644 --- a/internal/sync/repository/git_test.go +++ b/internal/sync/repository/git_test.go @@ -99,25 +99,6 @@ func TestFindGitRepositoryReturnsOperationalError(t *testing.T) { }) } -func TestDeletedPathsCombinesStagedAndUnstagedChanges(t *testing.T) { - git := &fakeGit{outputs: map[string]gitResult{ - "diff --name-only --diff-filter=D -z -- .launchdarkly": { - output: ".launchdarkly/project/configs/config/unstaged.prompt.md\x00", - }, - "diff --cached --name-only --diff-filter=D -z -- .launchdarkly": { - output: ".launchdarkly/project/configs/config/staged.prompt.md\x00", - }, - }} - - paths, err := deletedPaths(git, "/tmp/example") - - require.NoError(t, err) - require.Equal(t, []string{ - ".launchdarkly/project/configs/config/staged.prompt.md", - ".launchdarkly/project/configs/config/unstaged.prompt.md", - }, paths) -} - type gitResult struct { output string stderr string