Dont duplicate subscriptions (#2116)

* We do not want to add duplicated subscriptiosn Signed-off-by: Matthias Wessendorf <[email protected]> * new unit test Signed-off-by: Matthias Wessendorf <[email protected]> --------- Signed-off-by: Matthias Wessendorf <[email protected]>
knative · Jan 16, 2024 · b29f702 · b29f702
1 parent 98e5b25
commit b29f702
Show file tree

Hide file tree

Showing 2 changed files with 161 additions and 5 deletions.
diff --git a/cmd/subscribe.go b/cmd/subscribe.go
@@ -61,14 +61,10 @@ func runSubscribe(cmd *cobra.Command, args []string) (err error) {
 	}
 
 	// add subscription	to function
-	f.Deploy.Subscriptions = append(f.Deploy.Subscriptions, fn.KnativeSubscription{
-		Source:  cfg.Source,
-		Filters: extractFilterMap(cfg.Filter),
-	})
+	f.Deploy.Subscriptions = updateOrAddSubscription(f.Deploy.Subscriptions, cfg)
 
 	// pump it
 	return f.Write()
-
 }
 
 func extractFilterMap(filters []string) map[string]string {
@@ -91,6 +87,33 @@ type subscibeConfig struct {
 	Source string
 }
 
+func updateOrAddSubscription(subscriptions []fn.KnativeSubscription, cfg subscibeConfig) []fn.KnativeSubscription {
+	found := false
+	newFilters := extractFilterMap(cfg.Filter)
+
+	// Iterate over subscriptions to find if one with the same source already exists
+	for i, subscription := range subscriptions {
+		if subscription.Source == cfg.Source {
+			found = true
+			// Update filters. Override if the key already exists.
+			for newKey, newValue := range newFilters {
+				subscription.Filters[newKey] = newValue
+			}
+			subscriptions[i] = subscription // Reassign the updated subscription
+			break
+		}
+	}
+
+	// If a subscription with the source was not found, add a new one
+	if !found {
+		subscriptions = append(subscriptions, fn.KnativeSubscription{
+			Source:  cfg.Source,
+			Filters: newFilters,
+		})
+	}
+	return subscriptions
+}
+
 func newSubscribeConfig(cmd *cobra.Command) (c subscibeConfig) {
 	c = subscibeConfig{
 		Filter: viper.GetStringSlice("filter"),

diff --git a/cmd/subscribe_test.go b/cmd/subscribe_test.go
@@ -39,6 +39,67 @@ func TestSubscribeWithAll(t *testing.T) {
 	}
 }
 
+func TestSubscribeWithMultiple(t *testing.T) {
+	root := fromTempDirectory(t)
+
+	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "foo=go"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now load the function and ensure that the subscription is set correctly.
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Deploy.Subscriptions == nil {
+		t.Fatal("Expected subscription to be present ")
+	}
+	if f.Deploy.Subscriptions[0].Source != "my-broker" {
+		t.Fatalf("Expected subscription for broker to be 'my-broker', but got '%v'", f.Deploy.Subscriptions[0].Source)
+	}
+
+	if f.Deploy.Subscriptions[0].Filters["foo"] != "go" {
+		t.Fatalf("Expected subscription filter for 'foo' to be 'go', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+
+	cmd = NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "bar=foo"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now load the function and ensure that the subscription is set correctly.
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Deploy.Subscriptions == nil {
+		t.Fatal("Expected subscription to be present ")
+	}
+	if f.Deploy.Subscriptions[0].Source != "my-broker" {
+		t.Fatalf("Expected subscription for broker to be 'my-broker', but got '%v'", f.Deploy.Subscriptions[0].Source)
+	}
+
+	if f.Deploy.Subscriptions[0].Filters["foo"] != "go" {
+		t.Fatalf("Expected subscription filter for 'foo' to be 'go', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+	if f.Deploy.Subscriptions[0].Filters["bar"] != "foo" {
+		t.Fatalf("Expected subscription filter for 'bar' to be 'foo', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+
+}
+
 func TestSubscribeWithNoExplicitSourceAll(t *testing.T) {
 	root := fromTempDirectory(t)
 
@@ -71,3 +132,75 @@ func TestSubscribeWithNoExplicitSourceAll(t *testing.T) {
 		t.Fatalf("Expected subscription filter for 'foo' to be 'go', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
 	}
 }
+
+func TestSubscribeWithDuplicated(t *testing.T) {
+	root := fromTempDirectory(t)
+
+	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "foo=go"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now load the function and ensure that the subscription is set correctly.
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Deploy.Subscriptions == nil {
+		t.Fatal("Expected subscription to be present ")
+	}
+	if f.Deploy.Subscriptions[0].Source != "my-broker" {
+		t.Fatalf("Expected subscription for broker to be 'my-broker', but got '%v'", f.Deploy.Subscriptions[0].Source)
+	}
+
+	if f.Deploy.Subscriptions[0].Filters["foo"] != "go" {
+		t.Fatalf("Expected subscription filter for 'foo' to be 'go', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+
+	// call it again with same
+	cmd = NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "foo=go"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	// Now load the function and ensure that the subscription is set correctly.
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(f.Deploy.Subscriptions) > 1 {
+		t.Fatal("Expected only one subscription to be present ")
+	}
+
+	// call it again and override
+	cmd = NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "foo=gogo"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Now load the function and ensure that the subscription is set correctly.
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(f.Deploy.Subscriptions) > 1 {
+		t.Fatal("Expected only one subscription to be present ")
+	}
+	if f.Deploy.Subscriptions[0].Filters["foo"] != "gogo" {
+		t.Fatalf("Expected subscription filter for 'foo' to be 'gogo', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+
+}