diff --git a/cmd/import.go b/cmd/import.go index 1f270433..c9400feb 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -177,11 +177,13 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command f = filepath.Clean(f) // Upsert entry. - watchCfg.UpsertEntry(config.WatchEntry{ + if err := watchCfg.UpsertEntry(config.WatchEntry{ FilePath: f, Context: []string{globalClientOpts.Context}, MainArtifact: mainArtifact, - }) + }); err != nil { + return errors.Wrap(errors.KindEnvironment, err) + } // Write watch file. if err := config.WriteLocalWatchConfig(*watchCfg, watchFile); err != nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 1ea511fc..90ed91de 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -446,11 +446,11 @@ func TestWatchConfig(t *testing.T) { assert.Equal(t, "file1.yaml", loadedWCfg.Entries[0].FilePath) // 4. Upsert entry (new path) - loadedWCfg.UpsertEntry(WatchEntry{FilePath: "file2.yaml", Context: []string{"ctx2"}, MainArtifact: false}) + assert.NoError(t, loadedWCfg.UpsertEntry(WatchEntry{FilePath: "file2.yaml", Context: []string{"ctx2"}, MainArtifact: false})) assert.Len(t, loadedWCfg.Entries, 2) // 5. Upsert entry (existing path, append context) - loadedWCfg.UpsertEntry(WatchEntry{FilePath: "file1.yaml", Context: []string{"ctx3"}, MainArtifact: true}) + assert.NoError(t, loadedWCfg.UpsertEntry(WatchEntry{FilePath: "file1.yaml", Context: []string{"ctx3"}, MainArtifact: true})) assert.Len(t, loadedWCfg.Entries, 2) for _, e := range loadedWCfg.Entries { if e.FilePath == "file1.yaml" { @@ -458,6 +458,16 @@ func TestWatchConfig(t *testing.T) { assert.Contains(t, e.Context, "ctx3") } } + + // 6. Upsert entry again with existing context (must preserve existing contexts and not duplicate) + assert.NoError(t, loadedWCfg.UpsertEntry(WatchEntry{FilePath: "file1.yaml", Context: []string{"ctx3"}, MainArtifact: true})) + for _, e := range loadedWCfg.Entries { + if e.FilePath == "file1.yaml" { + assert.Contains(t, e.Context, "ctx1") + assert.Contains(t, e.Context, "ctx3") + assert.Len(t, e.Context, 2) + } + } } var secretMarkers = []string{ diff --git a/pkg/config/localconfig.go b/pkg/config/localconfig.go index ac7dd870..bedebbf6 100644 --- a/pkg/config/localconfig.go +++ b/pkg/config/localconfig.go @@ -21,6 +21,7 @@ import ( "os" "path/filepath" "slices" + "strings" configUtil "github.com/microcks/microcks-cli/pkg/util" ) @@ -386,18 +387,39 @@ func (l *LocalConfig) RemoveAuth(server string) bool { } // UpsertEntry upserts a watch entry. -func (w *WatchConfig) UpsertEntry(entry WatchEntry) { +func (w *WatchConfig) UpsertEntry(entry WatchEntry) error { + if len(entry.Context) == 0 { + return fmt.Errorf("watch entry context must not be empty") + } + for _, c := range entry.Context { + if strings.TrimSpace(c) == "" { + return fmt.Errorf("watch entry context must not contain blank context names") + } + } + for i, e := range w.Entries { if e.FilePath == entry.FilePath { - contexts := w.Entries[i].Context - if !slices.Contains(contexts, entry.Context[0]) { - entry.Context = append(entry.Context, contexts...) + merged := w.Entries[i].Context + for _, c := range entry.Context { + if !slices.Contains(merged, c) { + merged = append(merged, c) + } } + entry.Context = merged w.Entries[i] = entry - return + return nil } } + + var uniqueContexts []string + for _, c := range entry.Context { + if !slices.Contains(uniqueContexts, c) { + uniqueContexts = append(uniqueContexts, c) + } + } + entry.Context = uniqueContexts w.Entries = append(w.Entries, entry) + return nil } // ReadLocalWatchConfig loads up the local watch configuration file. Returns nil if config does not exist diff --git a/pkg/config/localconfig_test.go b/pkg/config/localconfig_test.go index 420432b3..a2dcf9a3 100644 --- a/pkg/config/localconfig_test.go +++ b/pkg/config/localconfig_test.go @@ -16,6 +16,7 @@ package config import ( + "slices" "testing" ) @@ -69,3 +70,113 @@ func TestRemoveInstance_NoDuplicatesAfterRecreate(t *testing.T) { } } } + +func TestWatchConfig_UpsertEntry_RejectsEmptyContext(t *testing.T) { + w := &WatchConfig{} + + err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: nil, + }) + if err == nil { + t.Fatal("expected error for nil context, got nil") + } + + err = w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{}, + }) + if err == nil { + t.Fatal("expected error for empty context slice, got nil") + } +} + +func TestWatchConfig_UpsertEntry_RejectsBlankContext(t *testing.T) { + w := &WatchConfig{} + + err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{""}, + }) + if err == nil { + t.Fatal("expected error for empty string context, got nil") + } + + err = w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{" "}, + }) + if err == nil { + t.Fatal("expected error for whitespace-only context, got nil") + } + + err = w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{"valid", " "}, + }) + if err == nil { + t.Fatal("expected error when one context is blank, got nil") + } +} + +func TestWatchConfig_UpsertEntry_MergesAndDeduplicatesContexts(t *testing.T) { + w := &WatchConfig{} + + // Add new entry with duplicate contexts in input + if err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{"dev", "dev"}, + MainArtifact: false, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(w.Entries) != 1 { + t.Fatalf("expected 1 entry, got %d", len(w.Entries)) + } + if len(w.Entries[0].Context) != 1 || w.Entries[0].Context[0] != "dev" { + t.Fatalf("expected [dev], got %v", w.Entries[0].Context) + } + + // Upsert with new context "staging" + if err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{"staging"}, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(w.Entries[0].Context) != 2 { + t.Fatalf("expected 2 contexts, got %v", w.Entries[0].Context) + } + + // Re-upsert with already existing context "staging" -> must preserve "dev" and not wipe out contexts + if err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{"staging"}, + MainArtifact: true, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(w.Entries[0].Context) != 2 { + t.Fatalf("expected contexts to be preserved, got %v", w.Entries[0].Context) + } + if !slices.Contains(w.Entries[0].Context, "dev") || !slices.Contains(w.Entries[0].Context, "staging") { + t.Fatalf("expected [dev, staging], got %v", w.Entries[0].Context) + } + if !w.Entries[0].MainArtifact { + t.Fatal("expected MainArtifact to be updated to true") + } + + // Upsert with multiple contexts including existing and new + if err := w.UpsertEntry(WatchEntry{ + FilePath: "api.yaml", + Context: []string{"prod", "dev", "prod"}, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(w.Entries[0].Context) != 3 { + t.Fatalf("expected 3 contexts [dev, staging, prod], got %v", w.Entries[0].Context) + } + if !slices.Contains(w.Entries[0].Context, "prod") { + t.Fatalf("expected prod to be added, got %v", w.Entries[0].Context) + } +}