feat: reorder subscriptions

internal/model/subscription.go

@@ -50,6 +50,13 @@ func (s *Subscription) GetFilePath(configDir string) string {
 // SubscriptionsData is the full persisted collection of subscriptions.
 type SubscriptionsData struct {
 	Subscriptions map[string]*Subscription `yaml:"subscriptions"`
+	// Order records the user-controlled display/merge order of
+	// subscription names (see `subscription move`). Subscriptions absent
+	// from Order (new ones, or a subscriptions.yaml predating this field)
+	// are appended alphabetically after it by OrderedNames — Order need
+	// not be exhaustive or kept in sync by every mutation, it's reconciled
+	// against Subscriptions on read.
+	Order []string `yaml:"order,omitempty"`
 }
 
 // NewSubscriptionsData returns an empty, ready-to-use collection.
@@ -57,22 +64,90 @@ func NewSubscriptionsData() *SubscriptionsData {
 	return &SubscriptionsData{Subscriptions: make(map[string]*Subscription)}
 }
 
+// OrderedNames returns every subscription name in display/merge order:
+// Order's entries first (skipping any that no longer exist), then any
+// subscription missing from Order (new, or loaded from a file predating
+// this field), sorted alphabetically.
+func (d *SubscriptionsData) OrderedNames() []string {
+	seen := make(map[string]bool, len(d.Subscriptions))
+	result := make([]string, 0, len(d.Subscriptions))
+
+	for _, name := range d.Order {
+		if _, ok := d.Subscriptions[name]; ok && !seen[name] {
+			result = append(result, name)
+			seen[name] = true
+		}
+	}
+
+	var missing []string
+	for name := range d.Subscriptions {
+		if !seen[name] {
+			missing = append(missing, name)
+		}
+	}
+	sort.Strings(missing)
+
+	return append(result, missing...)
+}
+
 // GetActiveSubscriptions returns every subscription currently marked active,
-// sorted by name for deterministic ordering. Multiple subscriptions may be
+// in display/merge order (see OrderedNames). Multiple subscriptions may be
 // active simultaneously — config apply merges all of them (see
 // internal/corecfg), prefixing each proxy's name with its source
 // subscription's name to avoid collisions.
 func (d *SubscriptionsData) GetActiveSubscriptions() []*Subscription {
 	var actives []*Subscription
-	for _, sub := range d.Subscriptions {
-		if sub.Status == StatusActive {
+	for _, name := range d.OrderedNames() {
+		if sub := d.Subscriptions[name]; sub.Status == StatusActive {
 			actives = append(actives, sub)
 		}
 	}
-	sort.Slice(actives, func(i, j int) bool { return actives[i].Name < actives[j].Name })
 	return actives
 }
 
+// MoveSubscription repositions name immediately before or after target in
+// the display/merge order. Returns false if name or target don't exist, or
+// name == target.
+func (d *SubscriptionsData) MoveSubscription(name, target string, before bool) bool {
+	if name == target {
+		return false
+	}
+	if _, ok := d.Subscriptions[name]; !ok {
+		return false
+	}
+	if _, ok := d.Subscriptions[target]; !ok {
+		return false
+	}
+
+	current := d.OrderedNames()
+	filtered := make([]string, 0, len(current))
+	for _, n := range current {
+		if n != name {
+			filtered = append(filtered, n)
+		}
+	}
+
+	targetIdx := -1
+	for i, n := range filtered {
+		if n == target {
+			targetIdx = i
+			break
+		}
+	}
+	insertAt := targetIdx
+	if !before {
+		insertAt++
+	}
+
+	result := make([]string, 0, len(filtered)+1)
+	result = append(result, filtered[:insertAt]...)
+	result = append(result, name)
+	result = append(result, filtered[insertAt:]...)
+
+	d.Order = result
+	return true
+}
+
 // SetActive marks name as active, in addition to any other subscriptions
 // already active. Returns false if name does not exist.
 func (d *SubscriptionsData) SetActive(name string) bool {
@@ -94,13 +169,15 @@ func (d *SubscriptionsData) SetInactive(name string) bool {
 	return true
 }
 
-// AddSubscription adds a new subscription, activating it if it's the first one.
+// AddSubscription adds a new subscription, activating it if it's the first
+// one, and appends it to the end of the display/merge order.
 func (d *SubscriptionsData) AddSubscription(name, url string) *Subscription {
 	sub := NewSubscription(name, url)
 	if len(d.Subscriptions) == 0 {
 		sub.Status = StatusActive
 	}
 	d.Subscriptions[name] = sub
+	d.Order = append(d.Order, name)
 	return sub
 }
 
@@ -111,11 +188,13 @@ func (d *SubscriptionsData) RemoveSubscription(name string) bool {
 		return false
 	}
 	delete(d.Subscriptions, name)
+	d.Order = removeString(d.Order, name)
 	return true
 }
 
-// RenameSubscription renames oldName to newName. Returns false if oldName is
-// missing or newName is already taken.
+// RenameSubscription renames oldName to newName, preserving its position in
+// the display/merge order. Returns false if oldName is missing or newName is
+// already taken.
 func (d *SubscriptionsData) RenameSubscription(oldName, newName string) bool {
 	sub, ok := d.Subscriptions[oldName]
 	if !ok {
@@ -127,9 +206,24 @@ func (d *SubscriptionsData) RenameSubscription(oldName, newName string) bool {
 	delete(d.Subscriptions, oldName)
 	sub.Name = newName
 	d.Subscriptions[newName] = sub
+	for i, n := range d.Order {
+		if n == oldName {
+			d.Order[i] = newName
+		}
+	}
 	return true
 }
 
+func removeString(list []string, s string) []string {
+	out := list[:0]
+	for _, v := range list {
+		if v != s {
+			out = append(out, v)
+		}
+	}
+	return out
+}
+
 // SetSubscriptionURL updates the URL for an existing subscription.
 func (d *SubscriptionsData) SetSubscriptionURL(name, url string) bool {
 	sub, ok := d.Subscriptions[name]

internal/model/subscription_test.go

@@ -0,0 +1,167 @@
+package model
+
+import (
+	"reflect"
+	"testing"
+)
+
+func newTestData(names ...string) *SubscriptionsData {
+	d := NewSubscriptionsData()
+	for _, n := range names {
+		d.AddSubscription(n, "http://example.com/"+n)
+	}
+	return d
+}
+
+func TestOrderedNames_FollowsAdditionOrder(t *testing.T) {
+	d := newTestData("c", "a", "b")
+	want := []string{"c", "a", "b"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestOrderedNames_MissingFromOrderAppendedAlphabetically(t *testing.T) {
+	d := newTestData("b", "a")
+	// Simulate a subscriptions.yaml predating the Order field, or a
+	// subscription added by some other path that didn't touch Order.
+	d.Order = nil
+	d.Subscriptions["z"] = NewSubscription("z", "http://example.com/z")
+
+	got := d.OrderedNames()
+	want := []string{"a", "b", "z"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestOrderedNames_StaleOrderEntryIgnored(t *testing.T) {
+	d := newTestData("a", "b")
+	d.Order = []string{"a", "removed", "b"}
+	got := d.OrderedNames()
+	want := []string{"a", "b"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestMoveSubscription_Before(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.MoveSubscription("c", "a", true) {
+		t.Fatal("MoveSubscription() = false, want true")
+	}
+	want := []string{"c", "a", "b"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestMoveSubscription_After(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.MoveSubscription("a", "b", false) {
+		t.Fatal("MoveSubscription() = false, want true")
+	}
+	want := []string{"b", "a", "c"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestMoveSubscription_AfterLast(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.MoveSubscription("a", "c", false) {
+		t.Fatal("MoveSubscription() = false, want true")
+	}
+	want := []string{"b", "c", "a"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestMoveSubscription_BeforeFirst(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.MoveSubscription("c", "a", true) {
+		t.Fatal("MoveSubscription() = false, want true")
+	}
+	want := []string{"c", "a", "b"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestMoveSubscription_NoOpWhenSameName(t *testing.T) {
+	d := newTestData("a", "b")
+	if d.MoveSubscription("a", "a", true) {
+		t.Fatal("MoveSubscription(a, a) = true, want false")
+	}
+}
+
+func TestMoveSubscription_UnknownName(t *testing.T) {
+	d := newTestData("a", "b")
+	if d.MoveSubscription("nope", "a", true) {
+		t.Fatal("MoveSubscription() = true, want false for unknown name")
+	}
+}
+
+func TestMoveSubscription_UnknownTarget(t *testing.T) {
+	d := newTestData("a", "b")
+	if d.MoveSubscription("a", "nope", true) {
+		t.Fatal("MoveSubscription() = true, want false for unknown target")
+	}
+}
+
+func TestMoveSubscription_MaterializesFullOrderFromLegacyData(t *testing.T) {
+	// A subscriptions.yaml predating the Order field: Subscriptions
+	// populated directly, Order left nil, as if freshly loaded from an old
+	// file.
+	d := &SubscriptionsData{Subscriptions: map[string]*Subscription{
+		"a": NewSubscription("a", "http://example.com/a"),
+		"b": NewSubscription("b", "http://example.com/b"),
+		"c": NewSubscription("c", "http://example.com/c"),
+	}}
+
+	if !d.MoveSubscription("c", "a", true) {
+		t.Fatal("MoveSubscription() = false, want true")
+	}
+	// Legacy fallback order is alphabetical (a, b, c); moving c before a
+	// should yield c, a, b.
+	want := []string{"c", "a", "b"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestRenameSubscription_PreservesPosition(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.RenameSubscription("b", "bee") {
+		t.Fatal("RenameSubscription() = false")
+	}
+	want := []string{"a", "bee", "c"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestRemoveSubscription_RemovesFromOrder(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	if !d.RemoveSubscription("b") {
+		t.Fatal("RemoveSubscription() = false")
+	}
+	want := []string{"a", "c"}
+	if got := d.OrderedNames(); !reflect.DeepEqual(got, want) {
+		t.Fatalf("got %#v, want %#v", got, want)
+	}
+}
+
+func TestGetActiveSubscriptions_RespectsOrder(t *testing.T) {
+	d := newTestData("a", "b", "c")
+	d.SetInactive("a") // AddSubscription auto-activates the first one added
+	d.SetActive("b")
+	d.SetActive("c")
+	d.MoveSubscription("c", "a", true)
+
+	actives := d.GetActiveSubscriptions()
+	if len(actives) != 2 || actives[0].Name != "c" || actives[1].Name != "b" {
+		t.Fatalf("unexpected order: %#v", actives)
+	}
+}