From b2adfd542f973dc81570ede094c1e7c39231eaa8 Mon Sep 17 00:00:00 2001
From: Spencer Gibb <spencer@gibb.us>
Date: Mon, 12 Feb 2018 21:42:22 -0500
Subject: [PATCH] If filter is ordered already, don't wrap with ordered filter.

fixes gh-192
---
 .../route/builder/GatewayFilterSpec.java      | 15 +++++
 .../route/builder/GatewayFilterSpecTests.java | 66 +++++++++++++++++++
 2 files changed, 81 insertions(+)
 create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java

diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java
index 127d0c8b..5300cef2 100644
--- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java
+++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java
@@ -22,6 +22,8 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.cloud.gateway.filter.GatewayFilter;
 import org.springframework.cloud.gateway.filter.OrderedGatewayFilter;
@@ -45,6 +47,7 @@ import org.springframework.cloud.gateway.filter.factory.SetResponseHeaderGateway
 import org.springframework.cloud.gateway.filter.factory.SetStatusGatewayFilterFactory;
 import org.springframework.cloud.gateway.filter.ratelimit.KeyResolver;
 import org.springframework.cloud.gateway.route.Route;
+import org.springframework.core.Ordered;
 import org.springframework.http.HttpStatus;
 import org.springframework.tuple.Tuple;
 
@@ -54,6 +57,8 @@ import static org.springframework.tuple.TupleBuilder.tuple;
 
 public class GatewayFilterSpec extends UriSpec {
 
+	private static final Log log = LogFactory.getLog(GatewayFilterSpec.class);
+
 	static final Tuple EMPTY_TUPLE = tuple().build();
 
 	public GatewayFilterSpec(Route.Builder routeBuilder, RouteLocatorBuilder.Builder builder) {
@@ -74,10 +79,20 @@ public class GatewayFilterSpec extends UriSpec {
 	}
 
 	public GatewayFilterSpec filter(GatewayFilter gatewayFilter) {
+		if (gatewayFilter instanceof Ordered) {
+			this.routeBuilder.add(gatewayFilter);
+			return this;
+		}
 		return this.filter(gatewayFilter, 0);
 	}
 
 	public GatewayFilterSpec filter(GatewayFilter gatewayFilter, int order) {
+		if (gatewayFilter instanceof Ordered) {
+			this.routeBuilder.add(gatewayFilter);
+			log.warn("GatewayFilter already implements ordered "+gatewayFilter.getClass()
+					+ "ignoring order parameter: "+order);
+			return this;
+		}
 		this.routeBuilder.add(new OrderedGatewayFilter(gatewayFilter, order));
 		return this;
 	}
diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java
new file mode 100644
index 00000000..7b769588
--- /dev/null
+++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpecTests.java
@@ -0,0 +1,66 @@
+package org.springframework.cloud.gateway.route.builder;
+
+import org.junit.Test;
+import org.springframework.cloud.gateway.filter.OrderedGatewayFilter;
+import reactor.core.publisher.Mono;
+
+import org.springframework.cloud.gateway.filter.GatewayFilter;
+import org.springframework.cloud.gateway.filter.GatewayFilterChain;
+import org.springframework.cloud.gateway.route.Route;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.core.Ordered;
+import org.springframework.web.server.ServerWebExchange;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
+public class GatewayFilterSpecTests {
+
+	@Test
+	public void orderedInterfaceRespected() {
+		testFilter(MyOrderedFilter.class, new MyOrderedFilter(), 1000);
+	}
+
+	@Test
+	public void unorderedWithDefaultOrder() {
+		testFilter(OrderedGatewayFilter.class, new MyUnorderedFilter(), 0);
+	}
+
+	private void testFilter(Class<? extends GatewayFilter> type,
+							GatewayFilter gatewayFilter, int order) {
+		ConfigurableApplicationContext context = mock(ConfigurableApplicationContext.class);
+		Route.Builder routeBuilder = Route.builder()
+				.id("123")
+				.uri("abc:123")
+				.predicate(exchange -> true);
+		RouteLocatorBuilder.Builder routes = new RouteLocatorBuilder(context).routes();
+		GatewayFilterSpec spec = new GatewayFilterSpec(routeBuilder, routes);
+		spec.filter(gatewayFilter);
+
+		Route route = routeBuilder.build();
+		assertThat(route.getFilters()).hasSize(1);
+		GatewayFilter filter = route.getFilters().get(0);
+		assertThat(filter).isInstanceOf(type);
+		Ordered ordered = (Ordered) filter;
+		assertThat(ordered.getOrder()).isEqualTo(order);
+	}
+
+	protected static class MyOrderedFilter implements GatewayFilter, Ordered {
+		@Override
+		public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
+			return Mono.empty();
+		}
+
+		@Override
+		public int getOrder() {
+			return 1000;
+		}
+	}
+
+	protected static class MyUnorderedFilter implements GatewayFilter {
+		@Override
+		public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
+			return Mono.empty();
+		}
+	}
+}