From f095e8e0ceb6b65c6fc70f40a381416cea12496f Mon Sep 17 00:00:00 2001
From: Daniel Ferreira <danielfpferreira@users.noreply.github.com>
Date: Tue, 9 Aug 2022 20:26:47 +0100
Subject: [PATCH] fix: Call response modifier plugin when resp is not nil

Allows merged response from multible backends to be modified if at least
one of the backends replied, resp is not nil and the error would be for
instance a mergeErr.

Signed-off-by: Daniel Ferreira <danielfpferreira@users.noreply.github.com>
---
 proxy/plugin.go      | 25 +++++++++++----------
 proxy/plugin_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/proxy/plugin.go b/proxy/plugin.go
index 4910353d5..69b98b3f4 100644
--- a/proxy/plugin.go
+++ b/proxy/plugin.go
@@ -88,12 +88,7 @@ func newPluginMiddleware(logger logging.Logger, tag, pattern string, cfg map[str
 
 		if totReqModifiers == 0 {
 			return func(ctx context.Context, r *Request) (*Response, error) {
-				resp, err := next[0](ctx, r)
-				if err != nil {
-					return resp, err
-				}
-
-				return executeResponseModifiers(respModifiers, resp)
+				return nextProxyWithResponseModifiers(ctx, r, next[0], respModifiers)
 			}
 		}
 
@@ -116,16 +111,22 @@ func newPluginMiddleware(logger logging.Logger, tag, pattern string, cfg map[str
 				return nil, err
 			}
 
-			resp, err := next[0](ctx, r)
-			if err != nil {
-				return resp, err
-			}
-
-			return executeResponseModifiers(respModifiers, resp)
+			return nextProxyWithResponseModifiers(ctx, r, next[0], respModifiers)
 		}
 	}
 }
 
+func nextProxyWithResponseModifiers(ctx context.Context, r *Request, next Proxy, respModifiers []func(interface{}) (interface{}, error)) (*Response, error) {
+	resp, err := next(ctx, r)
+	// merged responses of multiple backends will have a non nil resp if at least one of the backends responds
+	// successfully
+	if err != nil && resp == nil {
+		return resp, err
+	}
+
+	return executeResponseModifiers(respModifiers, resp)
+}
+
 func executeRequestModifiers(reqModifiers []func(interface{}) (interface{}, error), r *Request) (*Request, error) {
 	var tmp RequestWrapper
 	tmp = &requestWrapper{
diff --git a/proxy/plugin_test.go b/proxy/plugin_test.go
index 7460ebfda..8e946059e 100644
--- a/proxy/plugin_test.go
+++ b/proxy/plugin_test.go
@@ -73,6 +73,59 @@ func TestNewPluginMiddleware_logger(t *testing.T) {
 	}
 }
 
+func TestNewPluginMiddleware_merge_error(t *testing.T) {
+	plugin.LoadWithLogger("./plugin/tests", ".so", plugin.RegisterModifier, logging.NoOp)
+
+	validator := func(ctx context.Context, r *Request) (*Response, error) {
+		return &Response{
+			Data:       map[string]interface{}{"foo": "bar"},
+			IsComplete: true,
+			Metadata: Metadata{
+				Headers:    map[string][]string{},
+				StatusCode: 0,
+			},
+		}, mergeError{errs: []error{fmt.Errorf("some backend error")}}
+	}
+
+	bknd := NewBackendPluginMiddleware(
+		logging.NoOp,
+		&config.Backend{
+			ExtraConfig: map[string]interface{}{
+				plugin.Namespace: map[string]interface{}{
+					"name": []interface{}{"lura-request-modifier-example-response"},
+				},
+			},
+		},
+	)(validator)
+
+	p := NewPluginMiddleware(
+		logging.NoOp,
+		&config.EndpointConfig{
+			ExtraConfig: map[string]interface{}{
+				plugin.Namespace: map[string]interface{}{
+					"name": []interface{}{
+						"lura-request-modifier-example-response",
+					},
+				},
+			},
+		},
+	)(bknd)
+
+	resp, err := p(context.Background(), &Request{Path: "/bar"})
+	if err != nil {
+		t.Error(err.Error())
+	}
+
+	if resp == nil {
+		t.Errorf("unexpected response: %v", resp)
+		return
+	}
+
+	if v, ok := resp.Data["foo"].(string); !ok || v != "bar" {
+		t.Errorf("unexpected foo value: %v", resp.Data["foo"])
+	}
+}
+
 func TestNewPluginMiddleware_error_request(t *testing.T) {
 	plugin.LoadWithLogger("./plugin/tests", ".so", plugin.RegisterModifier, logging.NoOp)