From 7ebe1977d8a52dd3d738274952d05f2141bdf0ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibaud=20Lepr=C3=AAtre?= Date: Tue, 28 Jun 2016 18:53:25 +0200 Subject: [PATCH 01/11] Handle context-path on X-Forwarded-Prefix support Since revision 7b270c4b46c20d42233218082fae0075c3a5c481, Zuul add support of `X-Forwarded-Prefix` for route with `stripPrefix=true`. However it supposes that Zuul context-path is equals to `/`. By using a custom context-path on Zuul we can expect that `zuul.add-proxy-headers=true` will compute a prefix regarding that context-path. Indeed with following architecture: - Zuul with context-path equals to `/foo` - A random service (*bar-service*) without context-path Zuul configurations: ``` zuul: add-proxy-headers: true routes: bar-service: /bar/** ``` We expect that *bar-service* when targeting `http://blabla.com/foo/bar/1` will produce endpoint url equals to `http://blabla.com/foo/bar` and not only `http://blabla.com/bar` (that will not be resolvable). Furthermore context-path influence not only `stripPrefix=true` routes because with following configuration ``` zuul: add-proxy-headers: true routes: bar-service: path: /bar/** strip-prefix: false ``` We expect that *bar-service* when targeting `http://blabla.com/foo/bar/1` will also prodcues endpoint url equals to `http://blabla.com/foo/bar`... --- Moreover since *Spring 4.3* and new [`ForwardedHeaderFilter`](http://docs.spring.io/spring-framework/docs/4.3.0.BUILD-SNAPSHOT/javadoc-api/org/springframework/web/filter/ForwardedHeaderFilter.html) to handle `X-Forwarded-*` headers, when filter catches `X-Forwarded-Prefix` header it will override request context-path by `X-Forwarded-Prefix` header value and then **removes headers from request**. Thus the fact to support context-path will allow to use that filter with Zuul! --- Attention there is only one edge case, when `X-Forwarded-Prefix` header is present as same as custom `context-path`! In that case context-path will be ignored in favor to `X-Forwarded-Prefix` as same does `ForwardedHeaderFilter` --- .../zuul/filters/pre/PreDecorationFilter.java | 24 ++++-- .../filters/pre/PreDecorationFilterTests.java | 82 ++++++++++++++++++- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 39dc918b..d4449f64 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -121,23 +121,29 @@ public class PreDecorationFilter extends ZuulFilter { String.valueOf(ctx.getRequest().getServerPort())); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, ctx.getRequest().getScheme()); + String forwardedPrefix = + ctx.getRequest().getHeader("X-Forwarded-Prefix"); + String contextPath = ctx.getRequest().getContextPath(); + String prefix = StringUtils.hasLength(forwardedPrefix) + ? forwardedPrefix + : (StringUtils.hasLength(contextPath) ? contextPath : null); if (StringUtils.hasText(route.getPrefix())) { - String existingPrefix = ctx.getRequest() - .getHeader("X-Forwarded-Prefix"); StringBuilder newPrefixBuilder = new StringBuilder(); - if (StringUtils.hasLength(existingPrefix)) { - if (existingPrefix.endsWith("/") + if (prefix != null) { + if (prefix.endsWith("/") && route.getPrefix().startsWith("/")) { - newPrefixBuilder.append(existingPrefix, 0, - existingPrefix.length() - 1); + newPrefixBuilder.append(prefix, 0, + prefix.length() - 1); } else { - newPrefixBuilder.append(existingPrefix); + newPrefixBuilder.append(prefix); } } newPrefixBuilder.append(route.getPrefix()); - ctx.addZuulRequestHeader("X-Forwarded-Prefix", - newPrefixBuilder.toString()); + prefix = newPrefixBuilder.toString(); + } + if (prefix != null) { + ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); } String xforwardedfor = ctx.getRequest().getHeader("X-Forwarded-For"); String remoteAddr = ctx.getRequest().getRemoteAddr(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 9b521676..90d65f43 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -136,6 +136,86 @@ public class PreDecorationFilterTests { getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); } + @Test + public void routeWithContextPath() { + this.properties.setStripPrefix(false); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/api/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/api/foo/1", ctx.get("requestURI")); + assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + assertEquals("/context-path", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void prefixRouteWithContextPath() { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/foo/1", ctx.get("requestURI")); + assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + assertEquals("/context-path/api", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void routeIgnoreContextPathIfPrefixHeader() { + this.properties.setStripPrefix(false); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/api/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/api/foo/1", ctx.get("requestURI")); + assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + assertEquals("/prefix", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void prefixRouteIgnoreContextPathIfPrefixHeader() { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/foo/1", ctx.get("requestURI")); + assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + assertEquals("/prefix/api", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + @Test public void forwardRouteAddsLocation() throws Exception { this.properties.setPrefix("/api"); @@ -377,7 +457,7 @@ public class PreDecorationFilterTests { assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders, sensitiveHeaders.containsAll(Arrays.asList("x-bar", "x-foo"))); } - + @Test public void urlProperlyDecodedWhenCharacterEncodingIsSet() throws Exception { this.request.setCharacterEncoding("UTF-8"); From cf0c3bda92395407577e99e8476275c2530256e9 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 16 Aug 2016 14:57:22 +0200 Subject: [PATCH 02/11] Deploying documentation to proper folder What we're missing ATM is different documentation versions for different application versions. What this change does is that it's: - finding out what is the current branch (e.g. 1.0.x) - finding out out what is the name of the main adoc file (e.g. spring-cloud-sleuth) - pulling the changes from gh-pages after checkout - finding out what is the list of comma separated whitelisted branches (via the `docs.whitelisted.branches` prop) - in gh-pages creating a folder with name of the branch (e.g. /1.0.x) copying all the docs/target/generated-docs/ to that folder - if the branch from which we're calling the script is NOT master then we're changing the ${main.adoc}.html to index.html so that it's easier to access the docs (e.g. http://cloud.spring.io/spring-cloud-sleuth/1.0.x/) --- docs/pom.xml | 1 + docs/src/main/asciidoc/ghpages.sh | 119 +++++++++++++++++++++++------- 2 files changed, 95 insertions(+), 25 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index 201718f4..ddf45464 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -14,6 +14,7 @@ spring-cloud-netflix ${basedir}/.. + 1.0.x,1.1.x diff --git a/docs/src/main/asciidoc/ghpages.sh b/docs/src/main/asciidoc/ghpages.sh index e1063ce3..e83a3586 100755 --- a/docs/src/main/asciidoc/ghpages.sh +++ b/docs/src/main/asciidoc/ghpages.sh @@ -12,12 +12,46 @@ if ! [ -d docs/target/generated-docs ]; then exit 0 fi -# Find name of current branch +# The script should be executed from the root folder + +ROOT_FOLDER=`pwd` +echo "Current folder is ${ROOT_FOLDER}" + +if [[ ! -e "${ROOT_FOLDER}/.git" ]]; then + echo "You're not in the root folder of the project!" + exit 1 +fi + +# Retrieve properties ################################################################### -branch=$TRAVIS_BRANCH -[ "$branch" == "" ] && branch=`git rev-parse --abbrev-ref HEAD` -target=. -if [ "$branch" != "master" ]; then target=./$branch; mkdir -p $target; fi + +# Prop that will let commit the changes +COMMIT_CHANGES="no" + +# Get the name of the `docs.main` property +MAIN_ADOC_VALUE=$(mvn -q \ + -Dexec.executable="echo" \ + -Dexec.args='${docs.main}' \ + --non-recursive \ + org.codehaus.mojo:exec-maven-plugin:1.3.1:exec) +echo "Extracted 'main.adoc' from Maven build [${MAIN_ADOC_VALUE}]" + +# Get whitelisted branches - assumes that a `docs` module is available under `docs` profile +WHITELIST_PROPERTY="docs.whitelisted.branches" +WHITELISTED_BRANCHES_VALUE=$(mvn -q \ + -Dexec.executable="echo" \ + -Dexec.args="\${${WHITELIST_PROPERTY}}" \ + org.codehaus.mojo:exec-maven-plugin:1.3.1:exec \ + -P docs \ + -pl docs) +echo "Extracted '${WHITELIST_PROPERTY}' from Maven build [${WHITELISTED_BRANCHES_VALUE}]" + +# Code getting the name of the current branch. For master we want to publish as we did until now +# http://stackoverflow.com/questions/1593051/how-to-programmatically-determine-the-current-checked-out-git-branch +CURRENT_BRANCH=$(git symbolic-ref -q HEAD) +CURRENT_BRANCH=${CURRENT_BRANCH##refs/heads/} +CURRENT_BRANCH=${CURRENT_BRANCH:-HEAD} +echo "Current branch is [${CURRENT_BRANCH}]" # Stash any outstanding changes ################################################################### @@ -25,30 +59,65 @@ git diff-index --quiet HEAD dirty=$? if [ "$dirty" != "0" ]; then git stash; fi -# Switch to gh-pages branch to sync it with current branch +# Switch to gh-pages branch to sync it with master ################################################################### git checkout gh-pages +git pull origin gh-pages -for f in docs/target/generated-docs/*; do - file=${f#docs/target/generated-docs/*} - if ! git ls-files -i -o --exclude-standard --directory | grep -q ^$file$; then - # Not ignored... - cp -rf $f $target - git add -A $target/$file - fi -done - -git add -A README.adoc || echo "No change to README.adoc" -git commit -a -m "Sync docs from $branch to gh-pages" || echo "Nothing committed" - -# Uncomment the following push if you want to auto push to -# the gh-pages branch whenever you commit to branch locally. -# This is a little extreme. Use with care! +# Add git branches ################################################################### -git push origin gh-pages || echo "Cannot push gh-pages" +mkdir -p ${ROOT_FOLDER}/${CURRENT_BRANCH} +if [[ "${CURRENT_BRANCH}" == "master" ]] ; then + echo -e "Current branch is master - will copy the current docs only to the root folder" + for f in docs/target/generated-docs/*; do + file=${f#docs/target/generated-docs/*} + if ! git ls-files -i -o --exclude-standard --directory | grep -q ^$file$; then + # Not ignored... + cp -rf $f ${ROOT_FOLDER}/ + git add -A ${ROOT_FOLDER}/$file + fi + done + COMMIT_CHANGES="yes" +else + echo -e "Current branch is [${CURRENT_BRANCH}]" + # http://stackoverflow.com/questions/29300806/a-bash-script-to-check-if-a-string-is-present-in-a-comma-separated-list-of-strin + if [[ ",${WHITELISTED_BRANCHES_VALUE}," = *",${CURRENT_BRANCH},"* ]] ; then + echo -e "Branch [${CURRENT_BRANCH}] is whitelisted! Will copy the current docs to the [${CURRENT_BRANCH}] folder" + for f in docs/target/generated-docs/*; do + file=${f#docs/target/generated-docs/*} + if ! git ls-files -i -o --exclude-standard --directory | grep -q ^$file$; then + # Not ignored... + # We want users to access 1.0.0.RELEASE/ instead of 1.0.0.RELEASE/spring-cloud.sleuth.html + if [[ "${file}" == "${MAIN_ADOC_VALUE}.html" ]] ; then + # We don't want to copy the spring-cloud-sleuth.html + # we want it to be converted to index.html + cp -rf $f ${ROOT_FOLDER}/${CURRENT_BRANCH}/index.html + git add -A ${ROOT_FOLDER}/${CURRENT_BRANCH}/index.html + else + cp -rf $f ${ROOT_FOLDER}/${CURRENT_BRANCH} + git add -A ${ROOT_FOLDER}/${CURRENT_BRANCH}/$file + fi + fi + done + COMMIT_CHANGES="yes" + else + echo -e "Branch [${CURRENT_BRANCH}] is not on the white list! Check out the Maven [${WHITELIST_PROPERTY}] property in + [docs] module available under [docs] profile. Won't commit any changes to gh-pages for this branch." + fi +fi -# Finally, switch back to the current branch and exit block -git checkout $branch +if [[ "${COMMIT_CHANGES}" == "yes" ]] ; then + git commit -a -m "Sync docs from ${CURRENT_BRANCH} to gh-pages" + + # Uncomment the following push if you want to auto push to + # the gh-pages branch whenever you commit to master locally. + # This is a little extreme. Use with care! + ################################################################### + git push origin gh-pages +fi + +# Finally, switch back to the master branch and exit block +git checkout ${CURRENT_BRANCH} if [ "$dirty" != "0" ]; then git stash pop; fi -exit 0 +exit 0 \ No newline at end of file From c7a67205939667952d25967b55fdaa924eec4750 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 16 Aug 2016 15:27:29 +0200 Subject: [PATCH 03/11] Updated e2e tests with new brewery parameters --- scripts/runAcceptanceTests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/runAcceptanceTests.sh b/scripts/runAcceptanceTests.sh index 0fc9429a..97a9d6bf 100755 --- a/scripts/runAcceptanceTests.sh +++ b/scripts/runAcceptanceTests.sh @@ -14,9 +14,9 @@ curl "${SCRIPT_URL}" --output runAcceptanceTests.sh chmod +x runAcceptanceTests.sh echo "Killing all running apps" -./runAcceptanceTests.sh -t "${AT_WHAT_TO_TEST}" -n +./runAcceptanceTests.sh -t "${AT_WHAT_TO_TEST}" --killnow -./runAcceptanceTests.sh -t "${AT_WHAT_TO_TEST}" -k +./runAcceptanceTests.sh -t "${AT_WHAT_TO_TEST}" --killattheend SCRIPT_URL="https://raw.githubusercontent.com/spring-cloud-samples/tests/master/scripts/runTests.sh" From bc7b8367a90c85a41312045bf4c1731f481a5338 Mon Sep 17 00:00:00 2001 From: Jin Zhang Date: Wed, 17 Aug 2016 00:01:04 +0800 Subject: [PATCH 04/11] add *.swp *.swo to .gitignore (#1181) --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 67907698..81428507 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,5 @@ _site/ .factorypath *.log .shelf +*.swp +*.swo From 21df6e43a38b38817b940b564bb4db6dcf906c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Tue, 2 Aug 2016 13:39:04 +0200 Subject: [PATCH 05/11] Adds zuul property to customize Hystrix ExecutionIsolationStrategy Add a check to current context as contentLength is already in the context Get content length from Headers when RibbonCommandContext is built + add tests Use request content length RibbonRoutingFilter Add documentation for new zuul.ribbonIsolationStrategy property fixes gh-339 --- .../main/asciidoc/spring-cloud-netflix.adoc | 2 + .../netflix/ribbon/RibbonHttpRequest.java | 2 +- .../netflix/zuul/ZuulProxyConfiguration.java | 28 +++++++----- .../netflix/zuul/filters/ZuulProperties.java | 19 ++++++++ .../route/RestClientRibbonCommand.java | 17 ++----- .../route/RestClientRibbonCommandFactory.java | 16 ++++--- .../filters/route/RibbonCommandContext.java | 11 +---- .../filters/route/RibbonRoutingFilter.java | 11 ++--- .../route/apache/HttpClientRibbonCommand.java | 7 ++- .../HttpClientRibbonCommandFactory.java | 5 ++- .../route/okhttp/OkHttpRibbonCommand.java | 7 ++- .../okhttp/OkHttpRibbonCommandFactory.java | 5 ++- .../route/support/AbstractRibbonCommand.java | 45 ++++++++++--------- .../apache/RibbonApacheHttpRequestTests.java | 10 ++++- .../okhttp/OkHttpRibbonRequestTests.java | 33 +++++++++----- .../route/RestClientRibbonCommandTests.java | 18 ++++++-- ...tpClientRibbonCommandIntegrationTests.java | 3 +- .../OkHttpRibbonCommandIntegrationTests.java | 3 +- ...stClientRibbonCommandIntegrationTests.java | 4 +- 19 files changed, 154 insertions(+), 92 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 648b4084..ca921d35 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1060,6 +1060,8 @@ Zuul's rule engine allows rules and filters to be written in essentially any JVM NOTE: The configuration property `zuul.max.host.connections` has been replaced by two new properties, `zuul.host.maxTotalConnections` and `zuul.host.maxPerRouteConnections` which default to 200 and 20 respectively. +NOTE: Default Hystrix isolation pattern (ExecutionIsolationStrategy) for all routes is SEMAPHORE. `zuul.ribbonIsolationStrategy` can be changed to THREAD if this isolation pattern is preferred. + [[netflix-zuul-reverse-proxy]] === Embedded Zuul Reverse Proxy diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java index 2d68b12d..ec610c2c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java @@ -108,6 +108,6 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { } private boolean isDynamic(String name) { - return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding"); + return "Content-Length".equalsIgnoreCase(name) || "Transfer-Encoding".equalsIgnoreCase(name); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 4ac23b6d..b21d4d40 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -86,20 +86,24 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Configuration @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory); + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties); } } @Configuration @ConditionalOnProperty("zuul.ribbon.restclient.enabled") protected static class RestClientRibbonConfiguration { + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new RestClientRibbonCommandFactory(clientFactory); + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new RestClientRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -107,10 +111,12 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory); + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -118,18 +124,16 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Bean public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, - this.server.getServletPrefix(), - this.zuulProperties, - proxyRequestHelper); + return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), + this.zuulProperties, proxyRequestHelper); } // route filters @Bean public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper, RibbonCommandFactory ribbonCommandFactory) { - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, - ribbonCommandFactory, this.requestCustomizers); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, + this.requestCustomizers); return filter; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 0d49e12b..3bdb633b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -31,10 +31,14 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; +import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; + import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import static com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE; + /** * @author Spencer Gibb * @author Dave Syer @@ -131,6 +135,10 @@ public class ZuulProperties { */ private boolean sslHostnameValidationEnabled =true; + private ExecutionIsolationStrategy ribbonIsolationStrategy = SEMAPHORE; + + private HystrixSemaphore semaphore = new HystrixSemaphore(); + public Set getIgnoredHeaders() { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); if (ClassUtils.isPresent( @@ -298,6 +306,17 @@ public class ZuulProperties { */ private int maxPerRouteConnections = 20; } + + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class HystrixSemaphore { + /** + * The maximum number of total semaphores for Hystrix. + */ + private int maxSemaphores = 100; + + } public String getServletPattern() { String path = this.servletPath; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index 2631a3b2..39c850a4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.net.URI; import java.util.List; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; import org.springframework.util.MultiValueMap; @@ -33,23 +34,13 @@ import com.netflix.niws.client.http.RestClient; /** * Hystrix wrapper around Eureka Ribbon command * - * see original - * https://github.com/Netflix/zuul/blob/master/zuul-netflix/src/main/java/com/ - * netflix/zuul/dependency/ribbon/hystrix/RibbonCommand.java + * see original */ @SuppressWarnings("deprecation") public class RestClientRibbonCommand extends AbstractRibbonCommand { - @SuppressWarnings("unused") - @Deprecated - public RestClientRibbonCommand(String commandKey, RestClient restClient, HttpRequest.Verb verb, String uri, - Boolean retryable, MultiValueMap headers, - MultiValueMap params, InputStream requestEntity) { - this(commandKey, restClient, new RibbonCommandContext(commandKey, verb.verb(), uri, retryable, headers, params, requestEntity)); - } - - public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context) { - super(commandKey, client, context); + public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context, ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java index f5ab494e..9f8718f2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java @@ -17,28 +17,32 @@ package org.springframework.cloud.netflix.zuul.filters.route; -import com.netflix.client.http.HttpRequest; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import com.netflix.client.http.HttpRequest; import com.netflix.niws.client.http.RestClient; +import lombok.RequiredArgsConstructor; + /** * @author Spencer Gibb */ -public class RestClientRibbonCommandFactory implements RibbonCommandFactory { +@RequiredArgsConstructor +public class RestClientRibbonCommandFactory + implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - public RestClientRibbonCommandFactory(SpringClientFactory clientFactory) { - this.clientFactory = clientFactory; - } + private final ZuulProperties zuulProperties; @Override @SuppressWarnings("deprecation") public RestClientRibbonCommand create(RibbonCommandContext context) { RestClient restClient = this.clientFactory.getClient(context.getServiceId(), RestClient.class); - return new RestClientRibbonCommand(context.getServiceId(), restClient, context); + return new RestClientRibbonCommand(context.getServiceId(), restClient, context, + this.zuulProperties); } public SpringClientFactory getClientFactory() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index 173cd09d..a558c5fd 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -19,7 +19,6 @@ package org.springframework.cloud.netflix.zuul.filters.route; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.List; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; @@ -54,17 +53,11 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; - public RibbonCommandContext(String serviceId, String method, String uri, Boolean retryable, - MultiValueMap headers, MultiValueMap params, - InputStream requestEntity) { - this(serviceId, method, uri, retryable, headers, params, requestEntity, - new ArrayList(), null); - } - public URI uri() { try { return new URI(this.uri); - } catch (URISyntaxException e) { + } + catch (URISyntaxException e) { ReflectionUtils.rethrowRuntimeException(e); } return null; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index 8e355424..2df7c7bd 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -47,8 +47,8 @@ public class RibbonRoutingFilter extends ZuulFilter { protected List requestCustomizers; public RibbonRoutingFilter(ProxyRequestHelper helper, - RibbonCommandFactory ribbonCommandFactory, - List requestCustomizers) { + RibbonCommandFactory ribbonCommandFactory, + List requestCustomizers) { this.helper = helper; this.ribbonCommandFactory = ribbonCommandFactory; this.requestCustomizers = requestCustomizers; @@ -120,12 +120,13 @@ public class RibbonRoutingFilter extends ZuulFilter { uri = uri.replace("//", "/"); return new RibbonCommandContext(serviceId, verb, uri, retryable, headers, params, - requestEntity, this.requestCustomizers); + requestEntity, this.requestCustomizers, request.getContentLengthLong()); } protected ClientHttpResponse forward(RibbonCommandContext context) throws Exception { - Map info = this.helper.debug(context.getMethod(), context.getUri(), - context.getHeaders(), context.getParams(), context.getRequestEntity()); + Map info = this.helper.debug(context.getMethod(), + context.getUri(), context.getHeaders(), context.getParams(), + context.getRequestEntity()); RibbonCommand command = this.ribbonCommandFactory.create(context); try { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java index e9d17d9c..5061fb1f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java @@ -20,6 +20,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpRequest; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpResponse; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -29,8 +30,10 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class HttpClientRibbonCommand extends AbstractRibbonCommand { public HttpClientRibbonCommand(final String commandKey, - final RibbonLoadBalancingHttpClient client, RibbonCommandContext context) { - super(commandKey, client, context); + final RibbonLoadBalancingHttpClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java index 1f3063a6..ec9be369 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -31,6 +32,8 @@ public class HttpClientRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; + + private final ZuulProperties zuulProperties; @Override public HttpClientRibbonCommand create(final RibbonCommandContext context) { @@ -39,7 +42,7 @@ public class HttpClientRibbonCommandFactory implements serviceId, RibbonLoadBalancingHttpClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new HttpClientRibbonCommand(serviceId, client, context); + return new HttpClientRibbonCommand(serviceId, client, context, zuulProperties); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java index 9709ae16..f66f36b7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java @@ -20,6 +20,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonRequest; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonResponse; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -29,8 +30,10 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class OkHttpRibbonCommand extends AbstractRibbonCommand { public OkHttpRibbonCommand(final String commandKey, - final OkHttpLoadBalancingClient client, RibbonCommandContext context) { - super(commandKey, client, context); + final OkHttpLoadBalancingClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java index fc65e683..61001443 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -31,6 +32,8 @@ public class OkHttpRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; + + private final ZuulProperties zuulProperties; @Override public OkHttpRibbonCommand create(final RibbonCommandContext context) { @@ -39,7 +42,7 @@ public class OkHttpRibbonCommandFactory implements serviceId, OkHttpLoadBalancingClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new OkHttpRibbonCommand(serviceId, client, context); + return new OkHttpRibbonCommand(serviceId, client, context, zuulProperties); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index eb15b4ba..36beb774 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -18,10 +18,10 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.http.client.ClientHttpResponse; -import org.springframework.util.StringUtils; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; @@ -32,39 +32,48 @@ import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties; +import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; import com.netflix.zuul.constants.ZuulConstants; import com.netflix.zuul.context.RequestContext; /** * @author Spencer Gibb */ -public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> extends HystrixCommand implements - RibbonCommand { +public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> + extends HystrixCommand implements RibbonCommand { protected final LBC client; protected RibbonCommandContext context; - public AbstractRibbonCommand(LBC client, RibbonCommandContext context) { - this("default", client, context); + public AbstractRibbonCommand(LBC client, RibbonCommandContext context, + ZuulProperties zuulProperties) { + this("default", client, context, zuulProperties); } - public AbstractRibbonCommand(String commandKey, LBC client, RibbonCommandContext context) { - super(getSetter(commandKey)); + public AbstractRibbonCommand(String commandKey, LBC client, + RibbonCommandContext context, ZuulProperties zuulProperties) { + super(getSetter(commandKey, zuulProperties)); this.client = client; this.context = context; } - protected static Setter getSetter(final String commandKey) { + protected static Setter getSetter(final String commandKey, + ZuulProperties zuulProperties) { - // we want to default to semaphore-isolation since this wraps - // 2 others commands that are already thread isolated // @formatter:off - final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; - final DynamicIntProperty value = DynamicPropertyFactory.getInstance() - .getIntProperty(name, 100); - final HystrixCommandProperties.Setter setter = HystrixCommandProperties .Setter() - .withExecutionIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE) - .withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); + final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter() + .withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy()); + if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){ + final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; + // we want to default to semaphore-isolation since this wraps + // 2 others commands that are already thread isolated + final DynamicIntProperty value = DynamicPropertyFactory.getInstance() + .getIntProperty(name, 100); + setter.withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); + } else { + // TODO Find out is some parameters can be set here + } + return Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand")) .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey + "RibbonCommand")) .andCommandPropertiesDefaults(setter); @@ -74,10 +83,6 @@ public abstract class AbstractRibbonCommand headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); + headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonApacheHttpRequest httpRequest = new RibbonApacheHttpRequest(new RibbonCommandContext("example", "GET", uri, false, - headers, params, null)); + RibbonApacheHttpRequest httpRequest = + new RibbonApacheHttpRequest( + new RibbonCommandContext("example", "GET", uri, false, headers, params, null, new ArrayList())); HttpUriRequest request = httpRequest.toRequest(RequestConfig.custom().build()); @@ -63,7 +67,9 @@ public class RibbonApacheHttpRequestTests { assertThat("uri is wrong", request.getURI().toString(), startsWith(uri)); assertThat("my-header is missing", request.getFirstHeader("my-header"), is(notNullValue())); assertThat("my-header is wrong", request.getFirstHeader("my-header").getValue(), is(equalTo("my-value"))); + assertThat("Content-Length is wrong", request.getFirstHeader(HttpEncoding.CONTENT_LENGTH).getValue(), is(equalTo("5192"))); assertThat("myparam is missing", request.getURI().getQuery(), is(equalTo("myparam=myparamval"))); + } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java index 4c6c84fd..d35ea62d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java @@ -26,9 +26,11 @@ import static org.junit.Assert.assertThat; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import org.junit.Test; +import org.springframework.cloud.netflix.feign.encoding.HttpEncoding; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.util.LinkedMultiValueMap; @@ -47,33 +49,41 @@ public class OkHttpRibbonRequestTests { String uri = "http://example.com"; LinkedMultiValueMap headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); + // headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, false, headers, params, null); + RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, + false, headers, params, null, new ArrayList()); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); Request request = httpRequest.toRequest(); assertThat("body is not null", request.body(), is(nullValue())); assertThat("uri is wrong", request.url().toString(), startsWith(uri)); - assertThat("my-header is wrong", request.header("my-header"), is(equalTo("my-value"))); - assertThat("myparam is missing", request.url().queryParameter("myparam"), is(equalTo("myparamval"))); + assertThat("my-header is wrong", request.header("my-header"), + is(equalTo("my-value"))); + assertThat("myparam is missing", request.url().queryParameter("myparam"), + is(equalTo("myparamval"))); } @Test - // this situation happens, see https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 + // this situation happens, see + // https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 public void testEmptyEntityGet() throws Exception { String entityValue = ""; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, "GET"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, + "GET"); } @Test public void testNonEmptyEntityPost() throws Exception { String entityValue = "abcd"; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, "POST"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, + "POST"); } - void testEntity(String entityValue, ByteArrayInputStream requestEntity, boolean addContentLengthHeader, String method) throws IOException { + void testEntity(String entityValue, ByteArrayInputStream requestEntity, + boolean addContentLengthHeader, String method) throws IOException { String lengthString = String.valueOf(entityValue.length()); Long length = null; String uri = "http://example.com"; @@ -94,8 +104,9 @@ public class OkHttpRibbonRequestTests { builder.addHeader("from-customizer", "foo"); } }; - RibbonCommandContext context = new RibbonCommandContext("example", method, uri, false, - headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); + RibbonCommandContext context = new RibbonCommandContext("example", method, uri, + false, headers, new LinkedMultiValueMap(), requestEntity, + Collections.singletonList(requestCustomizer)); context.setContentLength(length); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); @@ -112,7 +123,8 @@ public class OkHttpRibbonRequestTests { if (!method.equalsIgnoreCase("get")) { assertThat("body is null", request.body(), is(notNullValue())); RequestBody body = request.body(); - assertThat("contentLength is wrong", body.contentLength(), is(equalTo((long) entityValue.length()))); + assertThat("contentLength is wrong", body.contentLength(), + is(equalTo((long) entityValue.length()))); Buffer content = new Buffer(); body.writeTo(content); String string = content.readByteString().utf8(); @@ -120,4 +132,3 @@ public class OkHttpRibbonRequestTests { } } } - diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java index f8d43345..5b4671d4 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java @@ -27,10 +27,13 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.net.URI; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Collections; +import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.StreamUtils; @@ -41,6 +44,13 @@ import com.netflix.client.http.HttpRequest; */ public class RestClientRibbonCommandTests { + private ZuulProperties zuulProperties; + + @Before + public void setUp() { + zuulProperties = new ZuulProperties(); + } + @Test public void testNullEntity() throws Exception { String uri = "http://example.com"; @@ -48,8 +58,10 @@ public class RestClientRibbonCommandTests { headers.add("my-header", "my-value"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, new RibbonCommandContext("example", "GET", uri, false, - headers, params, null)); + RestClientRibbonCommand command = + new RestClientRibbonCommand("cmd", null, + new RibbonCommandContext("example", "GET", uri, false, headers, params, null,new ArrayList()), + zuulProperties); HttpRequest request = command.createRequest(); @@ -96,7 +108,7 @@ public class RestClientRibbonCommandTests { uri.toString(), false, headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); context.setContentLength(length); - RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context); + RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context, zuulProperties); HttpRequest request = command.createRequest(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java index d4bcfa2c..e6765192 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java @@ -41,6 +41,7 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -179,7 +180,7 @@ public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory); + return new HttpClientRibbonCommandFactory(clientFactory, new ZuulProperties()); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java index 7c4086f8..023e7abe 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java @@ -32,6 +32,7 @@ import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.netflix.ribbon.RibbonClients; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -107,7 +108,7 @@ public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory); + return new OkHttpRibbonCommandFactory(clientFactory, new ZuulProperties()); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java index bb905634..3a88e5c5 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java @@ -330,7 +330,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { private SpringClientFactory clientFactory; public MyRibbonCommandFactory(SpringClientFactory clientFactory) { - super(clientFactory); + super(clientFactory, new ZuulProperties()); this.clientFactory = clientFactory; } @@ -356,7 +356,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { public MyCommand(int errorCode, String commandKey, RestClient restClient, RibbonCommandContext context) { - super(commandKey, restClient, context); + super(commandKey, restClient, context, new ZuulProperties()); this.errorCode = errorCode; } From d9768b537828662a6af8ba86babebc9db3c7f23c Mon Sep 17 00:00:00 2001 From: Stefan Fussenegger Date: Wed, 29 Jun 2016 15:23:48 +0200 Subject: [PATCH 06/11] Improved zuul proxy header support. - add zuul.add-host-header property to add Host header - add port to X-Forwarded-Host as defined in RFC 7239 - extract PreDecorationFilter.filterOrder() into a public constant fixes gh-1108 --- .../netflix/zuul/filters/ZuulProperties.java | 5 ++++ .../zuul/filters/pre/PreDecorationFilter.java | 21 ++++++++++++-- .../filters/pre/PreDecorationFilterTests.java | 28 +++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 3bdb633b..9d3278df 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -82,6 +82,11 @@ public class ZuulProperties { */ private boolean addProxyHeaders = true; + /** + * Flag to determine whether the proxy forwards the Host header. + */ + private boolean addHostHeader = false; + /** * Set of service names not to consider for proxying automatically. By default all * services in the discovery client will be proxied. diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 39dc918b..2c9d301e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -19,6 +19,8 @@ package org.springframework.cloud.netflix.zuul.filters.pre; import java.net.MalformedURLException; import java.net.URL; +import javax.servlet.http.HttpServletRequest; + import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; @@ -36,6 +38,8 @@ import lombok.extern.apachecommons.CommonsLog; @CommonsLog public class PreDecorationFilter extends ZuulFilter { + public static final int FILTER_ORDER = 5; + private RouteLocator routeLocator; private String dispatcherServletPath; @@ -58,7 +62,7 @@ public class PreDecorationFilter extends ZuulFilter { @Override public int filterOrder() { - return 5; + return FILTER_ORDER; } @Override @@ -115,8 +119,7 @@ public class PreDecorationFilter extends ZuulFilter { ctx.addOriginResponseHeader("X-Zuul-ServiceId", location); } if (this.properties.isAddProxyHeaders()) { - ctx.addZuulRequestHeader("X-Forwarded-Host", - ctx.getRequest().getServerName()); + ctx.addZuulRequestHeader("X-Forwarded-Host", toHostHeader(ctx.getRequest())); ctx.addZuulRequestHeader("X-Forwarded-Port", String.valueOf(ctx.getRequest().getServerPort())); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, @@ -149,6 +152,9 @@ public class PreDecorationFilter extends ZuulFilter { } ctx.addZuulRequestHeader("X-Forwarded-For", xforwardedfor); } + if (this.properties.isAddHostHeader()) { + ctx.addZuulRequestHeader("Host", toHostHeader(ctx.getRequest())); + } } } else { @@ -182,6 +188,15 @@ public class PreDecorationFilter extends ZuulFilter { return null; } + private String toHostHeader(HttpServletRequest request) { + int port = request.getServerPort(); + if ((port == 80 && "http".equals(request.getScheme())) || (port == 443 && "https".equals(request.getScheme()))) { + return request.getServerName(); + } else { + return request.getServerName() + ":" + port; + } + } + private URL getUrl(String target) { try { return new URL(target); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 9b521676..71a3a539 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -90,6 +90,34 @@ public class PreDecorationFilterTests { assertEquals(false, this.filter.shouldFilter()); } + @Test + public void xForwardedHostHasPort() throws Exception { + this.properties.setPrefix("/api"); + this.request.setRequestURI("/api/foo/1"); + this.request.setRemoteAddr("5.6.7.8"); + this.request.setServerPort(8080); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + } + + @Test + public void hostHeaderSet() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setAddHostHeader(true); + this.request.setRequestURI("/api/foo/1"); + this.request.setRemoteAddr("5.6.7.8"); + this.request.setServerPort(8080); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("localhost:8080", ctx.getZuulRequestHeaders().get("host")); + } + @Test public void prefixRouteAddsHeader() throws Exception { this.properties.setPrefix("/api"); From f76f294f7f14e7a29506b16f3ba2b059984d8302 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 17 Aug 2016 09:04:19 +0100 Subject: [PATCH 07/11] Rename test to conform to usual pattern --- ...yTest.java => RestTemplateRetryTests.java} | 578 +++++++++--------- 1 file changed, 288 insertions(+), 290 deletions(-) rename spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/{RestTemplateRetryTest.java => RestTemplateRetryTests.java} (93%) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java similarity index 93% rename from spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTest.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java index 8166bfb2..059de1ee 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTest.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java @@ -1,290 +1,288 @@ -package org.springframework.cloud.netflix.resttemplate; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import java.net.UnknownHostException; -import java.util.Arrays; -import java.util.concurrent.atomic.AtomicInteger; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; -import org.springframework.cloud.client.loadbalancer.LoadBalanced; -import org.springframework.cloud.netflix.ribbon.RibbonClient; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RestController; -import org.springframework.web.client.RestTemplate; - -import com.netflix.client.RetryHandler; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.AvailabilityFilteringRule; -import com.netflix.loadbalancer.BaseLoadBalancer; -import com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.IPing; -import com.netflix.loadbalancer.IRule; -import com.netflix.loadbalancer.LoadBalancerBuilder; -import com.netflix.loadbalancer.LoadBalancerStats; -import com.netflix.loadbalancer.Server; -import com.netflix.loadbalancer.ServerList; -import com.netflix.loadbalancer.ServerStats; -import com.netflix.niws.client.http.HttpClientLoadBalancerErrorHandler; - -@RunWith(SpringJUnit4ClassRunner.class) -@SpringBootTest(classes = RestTemplateRetryTest.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { - "spring.application.name=resttemplatetest", - "logging.level.org.springframework.cloud.netflix.resttemplate=DEBUG", - "badClients.ribbon.MaxAutoRetries=0", - "badClients.ribbon.OkToRetryOnAllOperations=true", "ribbon.http.client.enabled" }) -@DirtiesContext -public class RestTemplateRetryTest { - - final private static Log logger = LogFactory.getLog(RestTemplateRetryTest.class); - - @Value("${local.server.port}") - private int port = 0; - - @Autowired - private RestTemplate testClient; - - public RestTemplateRetryTest() { - } - - @Configuration - @EnableAutoConfiguration - @RestController - @RibbonClient(name = "badClients", configuration = LocalBadClientConfiguration.class) - public static class Application { - - private AtomicInteger hits = new AtomicInteger(1); - private AtomicInteger retryHits = new AtomicInteger(1); - - @RequestMapping(method = RequestMethod.GET, value = "/ping") - public int ping() { - return 0; - } - - @RequestMapping(method = RequestMethod.GET, value = "/good") - public int good() { - int lValue = this.hits.getAndIncrement(); - return lValue; - } - - @RequestMapping(method = RequestMethod.GET, value = "/timeout") - public int timeout() throws Exception { - int lValue = this.retryHits.getAndIncrement(); - - // Force the good server to have 2 consecutive errors a couple of times. - if (lValue == 2 || lValue == 3 || lValue == 5 || lValue == 6) { - Thread.sleep(500); - } - return lValue; - } - - @RequestMapping(method = RequestMethod.GET, value = "/null") - public int isNull() throws Exception { - throw new NullPointerException("Null"); - } - - @LoadBalanced - @Bean - RestTemplate restTemplate() { - return new RestTemplate(); - } - } - - @Before - public void setup() throws Exception { - // Force Ribbon configuration by making one call. - this.testClient.getForObject("http://badClients/ping", Integer.class); - } - - @Test - public void testNullPointer() throws Exception { - - LoadBalancerStats stats = LocalBadClientConfiguration.balancer - .getLoadBalancerStats(); - ServerStats badServer1Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer); - ServerStats badServer2Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer2); - ServerStats goodServerStats = stats - .getSingleServerStat(LocalBadClientConfiguration.goodServer); - - badServer1Stats.clearSuccessiveConnectionFailureCount(); - badServer2Stats.clearSuccessiveConnectionFailureCount(); - long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 10; - - // A null pointer should NOT trigger a circuit breaker. - for (int index = 0; index < 10; index++) { - try { - this.testClient.getForObject("http://badClients/null", Integer.class); - } - catch (Exception exception) { - } - } - logServerStats(LocalBadClientConfiguration.badServer); - logServerStats(LocalBadClientConfiguration.badServer2); - logServerStats(LocalBadClientConfiguration.goodServer); - - assertTrue(badServer1Stats.isCircuitBreakerTripped()); - assertTrue(badServer2Stats.isCircuitBreakerTripped()); - assertEquals(targetConnectionCount, goodServerStats.getTotalRequestsCount()); - - // Wait for any timeout thread to finish. - - } - - private void logServerStats(Server server) { - LoadBalancerStats stats = LocalBadClientConfiguration.balancer - .getLoadBalancerStats(); - ServerStats serverStats = stats.getSingleServerStat(server); - logger.debug("Server : " + server.toString() + " : Total Count == " - + serverStats.getTotalRequestsCount() + ", Failure Count == " - + serverStats.getFailureCount() + ", Successive Connection Failure == " - + serverStats.getSuccessiveConnectionFailureCount() - + ", Circuit Breaker ? == " + serverStats.isCircuitBreakerTripped()); - } - - @Test - public void testRestRetries() { - - LoadBalancerStats stats = LocalBadClientConfiguration.balancer - .getLoadBalancerStats(); - ServerStats badServer1Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer); - ServerStats badServer2Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer2); - ServerStats goodServerStats = stats - .getSingleServerStat(LocalBadClientConfiguration.goodServer); - - badServer1Stats.clearSuccessiveConnectionFailureCount(); - badServer2Stats.clearSuccessiveConnectionFailureCount(); - long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 20; - - int hits = 0; - - for (int index = 0; index < 20; index++) { - hits = this.testClient.getForObject("http://badClients/good", Integer.class); - } - - logServerStats(LocalBadClientConfiguration.badServer); - logServerStats(LocalBadClientConfiguration.badServer2); - logServerStats(LocalBadClientConfiguration.goodServer); - - assertTrue(badServer1Stats.isCircuitBreakerTripped()); - assertTrue(badServer2Stats.isCircuitBreakerTripped()); - assertEquals(targetConnectionCount, goodServerStats.getTotalRequestsCount()); - assertEquals(20, hits); - System.out.println("Retry Hits: " + hits); - } - - @Test - public void testRestRetriesWithReadTimeout() throws Exception { - - LoadBalancerStats stats = LocalBadClientConfiguration.balancer - .getLoadBalancerStats(); - ServerStats badServer1Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer); - ServerStats badServer2Stats = stats - .getSingleServerStat(LocalBadClientConfiguration.badServer2); - ServerStats goodServerStats = stats - .getSingleServerStat(LocalBadClientConfiguration.goodServer); - - badServer1Stats.clearSuccessiveConnectionFailureCount(); - badServer2Stats.clearSuccessiveConnectionFailureCount(); - assertTrue(!badServer1Stats.isCircuitBreakerTripped()); - assertTrue(!badServer2Stats.isCircuitBreakerTripped()); - - int hits = 0; - - for (int index = 0; index < 15; index++) { - hits = this.testClient.getForObject("http://badClients/timeout", - Integer.class); - } - logServerStats(LocalBadClientConfiguration.badServer); - logServerStats(LocalBadClientConfiguration.badServer2); - logServerStats(LocalBadClientConfiguration.goodServer); - - assertTrue(badServer1Stats.isCircuitBreakerTripped()); - assertTrue(badServer2Stats.isCircuitBreakerTripped()); - assertTrue(!goodServerStats.isCircuitBreakerTripped()); - - // 15 + 4 timeouts. See the endpoint for timeout conditions. - assertEquals(19, hits); - - // Wait for any timeout thread to finish. - Thread.sleep(600); - - } - -} - -// Load balancer with fixed server list for "local" pointing to localhost -// and some bogus servers are thrown in to test retry -@Configuration -class LocalBadClientConfiguration { - - static BaseLoadBalancer balancer; - static Server goodServer; - static Server badServer; - static Server badServer2; - - public LocalBadClientConfiguration() { - } - - @Value("${local.server.port}") - private int port = 0; - - @Bean - public IRule loadBalancerRule() { - // This is a good place to try different load balancing rules and how those rules - // behave in failure - // states: BestAvailableRule, WeightedResponseTimeRule, etc - - // This rule just uses a round robin and will skip servers that are in circuit - // breaker state. - return new AvailabilityFilteringRule(); - - } - - @Bean - public ILoadBalancer ribbonLoadBalancer(IClientConfig config, - ServerList serverList, IRule rule, IPing ping) { - - goodServer = new Server("localhost", this.port); - badServer = new Server("mybadhost", 10001); - badServer2 = new Server("localhost", -1); - - balancer = LoadBalancerBuilder.newBuilder().withClientConfig(config) - .withRule(rule).withPing(ping).buildFixedServerListLoadBalancer( - Arrays.asList(badServer, badServer2, goodServer)); - return balancer; - } - - @Bean - public RetryHandler retryHandler() { - return new OverrideRetryHandler(); - } - - static class OverrideRetryHandler extends HttpClientLoadBalancerErrorHandler { - public OverrideRetryHandler() { - this.circuitRelated.add(UnknownHostException.class); - this.retriable.add(UnknownHostException.class); - - } - } - -} +package org.springframework.cloud.netflix.resttemplate; + +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.cloud.client.loadbalancer.LoadBalanced; +import org.springframework.cloud.netflix.ribbon.RibbonClient; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestTemplate; + +import com.netflix.client.RetryHandler; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.AvailabilityFilteringRule; +import com.netflix.loadbalancer.BaseLoadBalancer; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.IPing; +import com.netflix.loadbalancer.IRule; +import com.netflix.loadbalancer.LoadBalancerBuilder; +import com.netflix.loadbalancer.LoadBalancerStats; +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerList; +import com.netflix.loadbalancer.ServerStats; +import com.netflix.niws.client.http.HttpClientLoadBalancerErrorHandler; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = RestTemplateRetryTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { + "spring.application.name=resttemplatetest", + "logging.level.org.springframework.cloud.netflix.resttemplate=DEBUG", + "badClients.ribbon.MaxAutoRetries=0", + "badClients.ribbon.OkToRetryOnAllOperations=true", "ribbon.http.client.enabled" }) +@DirtiesContext +public class RestTemplateRetryTests { + + final private static Log logger = LogFactory.getLog(RestTemplateRetryTests.class); + + @Value("${local.server.port}") + private int port = 0; + + @Autowired + private RestTemplate testClient; + + public RestTemplateRetryTests() { + } + + @Before + public void setup() throws Exception { + // Force Ribbon configuration by making one call. + this.testClient.getForObject("http://badClients/ping", Integer.class); + } + + @Test + public void testNullPointer() throws Exception { + + LoadBalancerStats stats = LocalBadClientConfiguration.balancer + .getLoadBalancerStats(); + ServerStats badServer1Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer); + ServerStats badServer2Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer2); + ServerStats goodServerStats = stats + .getSingleServerStat(LocalBadClientConfiguration.goodServer); + + badServer1Stats.clearSuccessiveConnectionFailureCount(); + badServer2Stats.clearSuccessiveConnectionFailureCount(); + long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 10; + + // A null pointer should NOT trigger a circuit breaker. + for (int index = 0; index < 10; index++) { + try { + this.testClient.getForObject("http://badClients/null", Integer.class); + } + catch (Exception exception) { + } + } + logServerStats(LocalBadClientConfiguration.badServer); + logServerStats(LocalBadClientConfiguration.badServer2); + logServerStats(LocalBadClientConfiguration.goodServer); + + assertTrue(badServer1Stats.isCircuitBreakerTripped()); + assertTrue(badServer2Stats.isCircuitBreakerTripped()); + assertEquals(targetConnectionCount, goodServerStats.getTotalRequestsCount()); + + // Wait for any timeout thread to finish. + + } + + private void logServerStats(Server server) { + LoadBalancerStats stats = LocalBadClientConfiguration.balancer + .getLoadBalancerStats(); + ServerStats serverStats = stats.getSingleServerStat(server); + logger.debug("Server : " + server.toString() + " : Total Count == " + + serverStats.getTotalRequestsCount() + ", Failure Count == " + + serverStats.getFailureCount() + ", Successive Connection Failure == " + + serverStats.getSuccessiveConnectionFailureCount() + + ", Circuit Breaker ? == " + serverStats.isCircuitBreakerTripped()); + } + + @Test + public void testRestRetries() { + + LoadBalancerStats stats = LocalBadClientConfiguration.balancer + .getLoadBalancerStats(); + ServerStats badServer1Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer); + ServerStats badServer2Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer2); + ServerStats goodServerStats = stats + .getSingleServerStat(LocalBadClientConfiguration.goodServer); + + badServer1Stats.clearSuccessiveConnectionFailureCount(); + badServer2Stats.clearSuccessiveConnectionFailureCount(); + long targetConnectionCount = goodServerStats.getTotalRequestsCount() + 20; + + int hits = 0; + + for (int index = 0; index < 20; index++) { + hits = this.testClient.getForObject("http://badClients/good", Integer.class); + } + + logServerStats(LocalBadClientConfiguration.badServer); + logServerStats(LocalBadClientConfiguration.badServer2); + logServerStats(LocalBadClientConfiguration.goodServer); + + assertTrue(badServer1Stats.isCircuitBreakerTripped()); + assertTrue(badServer2Stats.isCircuitBreakerTripped()); + assertEquals(targetConnectionCount, goodServerStats.getTotalRequestsCount()); + assertEquals(20, hits); + logger.debug("Retry Hits: " + hits); + } + + @Test + public void testRestRetriesWithReadTimeout() throws Exception { + + LoadBalancerStats stats = LocalBadClientConfiguration.balancer + .getLoadBalancerStats(); + ServerStats badServer1Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer); + ServerStats badServer2Stats = stats + .getSingleServerStat(LocalBadClientConfiguration.badServer2); + ServerStats goodServerStats = stats + .getSingleServerStat(LocalBadClientConfiguration.goodServer); + + badServer1Stats.clearSuccessiveConnectionFailureCount(); + badServer2Stats.clearSuccessiveConnectionFailureCount(); + assertTrue(!badServer1Stats.isCircuitBreakerTripped()); + assertTrue(!badServer2Stats.isCircuitBreakerTripped()); + + int hits = 0; + + for (int index = 0; index < 15; index++) { + hits = this.testClient.getForObject("http://badClients/timeout", + Integer.class); + } + logServerStats(LocalBadClientConfiguration.badServer); + logServerStats(LocalBadClientConfiguration.badServer2); + logServerStats(LocalBadClientConfiguration.goodServer); + + assertTrue(badServer1Stats.isCircuitBreakerTripped()); + assertTrue(badServer2Stats.isCircuitBreakerTripped()); + assertTrue(!goodServerStats.isCircuitBreakerTripped()); + + // 15 + 4 timeouts. See the endpoint for timeout conditions. + assertEquals(19, hits); + + // Wait for any timeout thread to finish. + Thread.sleep(600); + + } + + @Configuration + @EnableAutoConfiguration + @RestController + @RibbonClient(name = "badClients", configuration = LocalBadClientConfiguration.class) + public static class Application { + + private AtomicInteger hits = new AtomicInteger(1); + private AtomicInteger retryHits = new AtomicInteger(1); + + @RequestMapping(method = RequestMethod.GET, value = "/ping") + public int ping() { + return 0; + } + + @RequestMapping(method = RequestMethod.GET, value = "/good") + public int good() { + int lValue = this.hits.getAndIncrement(); + return lValue; + } + + @RequestMapping(method = RequestMethod.GET, value = "/timeout") + public int timeout() throws Exception { + int lValue = this.retryHits.getAndIncrement(); + + // Force the good server to have 2 consecutive errors a couple of times. + if (lValue == 2 || lValue == 3 || lValue == 5 || lValue == 6) { + Thread.sleep(500); + } + return lValue; + } + + @RequestMapping(method = RequestMethod.GET, value = "/null") + public int isNull() throws Exception { + throw new NullPointerException("Null"); + } + + @LoadBalanced + @Bean + RestTemplate restTemplate() { + return new RestTemplate(); + } + } + +} + +// Load balancer with fixed server list for "local" pointing to localhost +// and some bogus servers are thrown in to test retry +@Configuration +class LocalBadClientConfiguration { + + static BaseLoadBalancer balancer; + static Server goodServer; + static Server badServer; + static Server badServer2; + + public LocalBadClientConfiguration() { + } + + @Value("${local.server.port}") + private int port = 0; + + @Bean + public IRule loadBalancerRule() { + // This is a good place to try different load balancing rules and how those rules + // behave in failure states: BestAvailableRule, WeightedResponseTimeRule, etc + + // This rule just uses a round robin and will skip servers that are in circuit + // breaker state. + return new AvailabilityFilteringRule(); + + } + + @Bean + public ILoadBalancer ribbonLoadBalancer(IClientConfig config, + ServerList serverList, IRule rule, IPing ping) { + + goodServer = new Server("localhost", this.port); + badServer = new Server("mybadhost", 10001); + badServer2 = new Server("localhost", -1); + + balancer = LoadBalancerBuilder.newBuilder().withClientConfig(config) + .withRule(rule).withPing(ping).buildFixedServerListLoadBalancer( + Arrays.asList(badServer, badServer2, goodServer)); + return balancer; + } + + @Bean + public RetryHandler retryHandler() { + return new OverrideRetryHandler(); + } + + static class OverrideRetryHandler extends HttpClientLoadBalancerErrorHandler { + public OverrideRetryHandler() { + this.circuitRelated.add(UnknownHostException.class); + this.retriable.add(UnknownHostException.class); + } + } + +} From b9e2a1b21e56cfabad298d782de8e394585a50f1 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 17 Aug 2016 09:19:33 +0100 Subject: [PATCH 08/11] Fix test so it actually works if there is a server on localhost:80 --- .../cloud/netflix/resttemplate/RestTemplateRetryTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java index 059de1ee..c9c0285c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java @@ -20,6 +20,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.util.SocketUtils; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; @@ -46,7 +47,7 @@ import static org.junit.Assert.assertTrue; @SpringBootTest(classes = RestTemplateRetryTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "spring.application.name=resttemplatetest", "logging.level.org.springframework.cloud.netflix.resttemplate=DEBUG", - "badClients.ribbon.MaxAutoRetries=0", + "logging.level.com.netflix=DEBUG", "badClients.ribbon.MaxAutoRetries=0", "badClients.ribbon.OkToRetryOnAllOperations=true", "ribbon.http.client.enabled" }) @DirtiesContext public class RestTemplateRetryTests { @@ -265,7 +266,7 @@ class LocalBadClientConfiguration { goodServer = new Server("localhost", this.port); badServer = new Server("mybadhost", 10001); - badServer2 = new Server("localhost", -1); + badServer2 = new Server("localhost", SocketUtils.findAvailableTcpPort()); balancer = LoadBalancerBuilder.newBuilder().withClientConfig(config) .withRule(rule).withPing(ping).buildFixedServerListLoadBalancer( From 4602fc1105ddaeeda5e61411fd0280ad20031cbd Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 17 Aug 2016 12:45:28 +0200 Subject: [PATCH 09/11] Updating ghpages for all projects --- docs/src/main/asciidoc/ghpages.sh | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/docs/src/main/asciidoc/ghpages.sh b/docs/src/main/asciidoc/ghpages.sh index e83a3586..a5d1acd5 100755 --- a/docs/src/main/asciidoc/ghpages.sh +++ b/docs/src/main/asciidoc/ghpages.sh @@ -27,9 +27,23 @@ fi # Prop that will let commit the changes COMMIT_CHANGES="no" +MAVEN_PATH=${MAVEN_PATH:-} +echo "Path to Maven is [${MAVEN_PATH}]" + +# Code getting the name of the current branch. For master we want to publish as we did until now +# http://stackoverflow.com/questions/1593051/how-to-programmatically-determine-the-current-checked-out-git-branch +# If there is a branch already passed will reuse it - otherwise will try to find it +CURRENT_BRANCH=${BRANCH} +if [[ -z "${CURRENT_BRANCH}" ]] ; then + CURRENT_BRANCH=$(git symbolic-ref -q HEAD) + CURRENT_BRANCH=${CURRENT_BRANCH##refs/heads/} + CURRENT_BRANCH=${CURRENT_BRANCH:-HEAD} +fi +echo "Current branch is [${CURRENT_BRANCH}]" +git checkout ${CURRENT_BRANCH} # Get the name of the `docs.main` property -MAIN_ADOC_VALUE=$(mvn -q \ +MAIN_ADOC_VALUE=$("${MAVEN_PATH}"mvn -q \ -Dexec.executable="echo" \ -Dexec.args='${docs.main}' \ --non-recursive \ @@ -38,7 +52,7 @@ echo "Extracted 'main.adoc' from Maven build [${MAIN_ADOC_VALUE}]" # Get whitelisted branches - assumes that a `docs` module is available under `docs` profile WHITELIST_PROPERTY="docs.whitelisted.branches" -WHITELISTED_BRANCHES_VALUE=$(mvn -q \ +WHITELISTED_BRANCHES_VALUE=$("${MAVEN_PATH}"mvn -q \ -Dexec.executable="echo" \ -Dexec.args="\${${WHITELIST_PROPERTY}}" \ org.codehaus.mojo:exec-maven-plugin:1.3.1:exec \ @@ -46,17 +60,9 @@ WHITELISTED_BRANCHES_VALUE=$(mvn -q \ -pl docs) echo "Extracted '${WHITELIST_PROPERTY}' from Maven build [${WHITELISTED_BRANCHES_VALUE}]" -# Code getting the name of the current branch. For master we want to publish as we did until now -# http://stackoverflow.com/questions/1593051/how-to-programmatically-determine-the-current-checked-out-git-branch -CURRENT_BRANCH=$(git symbolic-ref -q HEAD) -CURRENT_BRANCH=${CURRENT_BRANCH##refs/heads/} -CURRENT_BRANCH=${CURRENT_BRANCH:-HEAD} -echo "Current branch is [${CURRENT_BRANCH}]" - # Stash any outstanding changes ################################################################### -git diff-index --quiet HEAD -dirty=$? +git diff-index --quiet HEAD && dirty=$? || (echo "Failed to check if the current repo is dirty. Assuming that it is." && dirty="1") if [ "$dirty" != "0" ]; then git stash; fi # Switch to gh-pages branch to sync it with master From 58cf52846308b1b1a6590ac0162a54214656278a Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 17 Aug 2016 14:01:58 +0100 Subject: [PATCH 10/11] Remove extra (redundant) condition A problem in Spring Boot means that the extra conditions are combined with OR not AND, so it's a bug to have more than one until that gets fixed (in an AnyNestedCondition). See https://github.com/spring-projects/spring-boot/pull/6672 --- .../eureka/EurekaClientAutoConfiguration.java | 18 ++---------------- .../EurekaClientAutoConfigurationTests.java | 4 ++-- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index 3557179f..8378efc8 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -26,7 +26,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.AutoConfigureBefore; -import org.springframework.boot.autoconfigure.condition.AllNestedConditions; import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -204,7 +203,8 @@ public class EurekaClientAutoConfiguration { @Target({ ElementType.TYPE, ElementType.METHOD }) @Retention(RetentionPolicy.RUNTIME) @Documented - @Conditional(OnRefreshScopeCondition.class) + @ConditionalOnClass(RefreshScope.class) + @ConditionalOnBean(RefreshAutoConfiguration.class) @interface ConditionalOnRefreshScope { } @@ -219,24 +219,10 @@ public class EurekaClientAutoConfiguration { static class MissingClass { } - @ConditionalOnClass(RefreshScope.class) @ConditionalOnMissingBean(RefreshAutoConfiguration.class) static class MissingScope { } } - private static class OnRefreshScopeCondition extends AllNestedConditions { - - public OnRefreshScopeCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @ConditionalOnClass(RefreshScope.class) - @ConditionalOnBean(RefreshAutoConfiguration.class) - static class FoundScope { - } - - } - } diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index a9240293..9363bec9 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -112,8 +112,8 @@ public class EurekaClientAutoConfigurationTests { EnvironmentTestUtils.addEnvironment(this.context, "server.port=8989", "eureka.client.serviceUrl.defaultZone=http://user:foo@example.com:80/eureka"); setupContext(MockClientConfiguration.class); - //ApacheHttpClient4 http = this.context.getBean(ApacheHttpClient4.class); - //Mockito.verify(http).addFilter(Matchers.any(HTTPBasicAuthFilter.class)); + // ApacheHttpClient4 http = this.context.getBean(ApacheHttpClient4.class); + // Mockito.verify(http).addFilter(Matchers.any(HTTPBasicAuthFilter.class)); } private void testNonSecurePort(String propName) { From 9ed5621a78c07a0fd84068273f9dc3bd8fd8febd Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 17 Aug 2016 09:57:35 -0600 Subject: [PATCH 11/11] Revert "Adds zuul property to customize Hystrix ExecutionIsolationStrategy" This reverts commit 21df6e43a38b38817b940b564bb4db6dcf906c40. --- .../main/asciidoc/spring-cloud-netflix.adoc | 2 - .../netflix/ribbon/RibbonHttpRequest.java | 2 +- .../netflix/zuul/ZuulProxyConfiguration.java | 28 +++++------- .../netflix/zuul/filters/ZuulProperties.java | 19 -------- .../route/RestClientRibbonCommand.java | 17 +++++-- .../route/RestClientRibbonCommandFactory.java | 18 +++----- .../filters/route/RibbonCommandContext.java | 11 ++++- .../filters/route/RibbonRoutingFilter.java | 11 +++-- .../route/apache/HttpClientRibbonCommand.java | 7 +-- .../HttpClientRibbonCommandFactory.java | 5 +-- .../route/okhttp/OkHttpRibbonCommand.java | 7 +-- .../okhttp/OkHttpRibbonCommandFactory.java | 5 +-- .../route/support/AbstractRibbonCommand.java | 45 +++++++++---------- .../apache/RibbonApacheHttpRequestTests.java | 10 +---- .../okhttp/OkHttpRibbonRequestTests.java | 33 +++++--------- .../route/RestClientRibbonCommandTests.java | 18 ++------ ...tpClientRibbonCommandIntegrationTests.java | 3 +- .../OkHttpRibbonCommandIntegrationTests.java | 3 +- ...stClientRibbonCommandIntegrationTests.java | 4 +- 19 files changed, 93 insertions(+), 155 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index ca921d35..648b4084 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1060,8 +1060,6 @@ Zuul's rule engine allows rules and filters to be written in essentially any JVM NOTE: The configuration property `zuul.max.host.connections` has been replaced by two new properties, `zuul.host.maxTotalConnections` and `zuul.host.maxPerRouteConnections` which default to 200 and 20 respectively. -NOTE: Default Hystrix isolation pattern (ExecutionIsolationStrategy) for all routes is SEMAPHORE. `zuul.ribbonIsolationStrategy` can be changed to THREAD if this isolation pattern is preferred. - [[netflix-zuul-reverse-proxy]] === Embedded Zuul Reverse Proxy diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java index ec610c2c..2d68b12d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java @@ -108,6 +108,6 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { } private boolean isDynamic(String name) { - return "Content-Length".equalsIgnoreCase(name) || "Transfer-Encoding".equalsIgnoreCase(name); + return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding"); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index b21d4d40..4ac23b6d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -86,24 +86,20 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Configuration @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { - @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties); + public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + return new HttpClientRibbonCommandFactory(clientFactory); } } @Configuration @ConditionalOnProperty("zuul.ribbon.restclient.enabled") protected static class RestClientRibbonConfiguration { - @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new RestClientRibbonCommandFactory(clientFactory, zuulProperties); + public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + return new RestClientRibbonCommandFactory(clientFactory); } } @@ -111,12 +107,10 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { - @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties); + public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + return new OkHttpRibbonCommandFactory(clientFactory); } } @@ -124,16 +118,18 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Bean public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), - this.zuulProperties, proxyRequestHelper); + return new PreDecorationFilter(routeLocator, + this.server.getServletPrefix(), + this.zuulProperties, + proxyRequestHelper); } // route filters @Bean public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper, RibbonCommandFactory ribbonCommandFactory) { - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, - this.requestCustomizers); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, + ribbonCommandFactory, this.requestCustomizers); return filter; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 9d3278df..58e22536 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -31,14 +31,10 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; -import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; - import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; -import static com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE; - /** * @author Spencer Gibb * @author Dave Syer @@ -140,10 +136,6 @@ public class ZuulProperties { */ private boolean sslHostnameValidationEnabled =true; - private ExecutionIsolationStrategy ribbonIsolationStrategy = SEMAPHORE; - - private HystrixSemaphore semaphore = new HystrixSemaphore(); - public Set getIgnoredHeaders() { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); if (ClassUtils.isPresent( @@ -311,17 +303,6 @@ public class ZuulProperties { */ private int maxPerRouteConnections = 20; } - - @Data - @AllArgsConstructor - @NoArgsConstructor - public static class HystrixSemaphore { - /** - * The maximum number of total semaphores for Hystrix. - */ - private int maxSemaphores = 100; - - } public String getServletPattern() { String path = this.servletPath; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index 39c850a4..2631a3b2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.net.URI; import java.util.List; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; import org.springframework.util.MultiValueMap; @@ -34,13 +33,23 @@ import com.netflix.niws.client.http.RestClient; /** * Hystrix wrapper around Eureka Ribbon command * - * see original + * see original + * https://github.com/Netflix/zuul/blob/master/zuul-netflix/src/main/java/com/ + * netflix/zuul/dependency/ribbon/hystrix/RibbonCommand.java */ @SuppressWarnings("deprecation") public class RestClientRibbonCommand extends AbstractRibbonCommand { - public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context, ZuulProperties zuulProperties) { - super(commandKey, client, context, zuulProperties); + @SuppressWarnings("unused") + @Deprecated + public RestClientRibbonCommand(String commandKey, RestClient restClient, HttpRequest.Verb verb, String uri, + Boolean retryable, MultiValueMap headers, + MultiValueMap params, InputStream requestEntity) { + this(commandKey, restClient, new RibbonCommandContext(commandKey, verb.verb(), uri, retryable, headers, params, requestEntity)); + } + + public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context) { + super(commandKey, client, context); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java index 9f8718f2..f5ab494e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java @@ -17,32 +17,28 @@ package org.springframework.cloud.netflix.zuul.filters.route; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; - import com.netflix.client.http.HttpRequest; -import com.netflix.niws.client.http.RestClient; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import lombok.RequiredArgsConstructor; +import com.netflix.niws.client.http.RestClient; /** * @author Spencer Gibb */ -@RequiredArgsConstructor -public class RestClientRibbonCommandFactory - implements RibbonCommandFactory { +public class RestClientRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - private final ZuulProperties zuulProperties; + public RestClientRibbonCommandFactory(SpringClientFactory clientFactory) { + this.clientFactory = clientFactory; + } @Override @SuppressWarnings("deprecation") public RestClientRibbonCommand create(RibbonCommandContext context) { RestClient restClient = this.clientFactory.getClient(context.getServiceId(), RestClient.class); - return new RestClientRibbonCommand(context.getServiceId(), restClient, context, - this.zuulProperties); + return new RestClientRibbonCommand(context.getServiceId(), restClient, context); } public SpringClientFactory getClientFactory() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index a558c5fd..173cd09d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.zuul.filters.route; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.List; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; @@ -53,11 +54,17 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; + public RibbonCommandContext(String serviceId, String method, String uri, Boolean retryable, + MultiValueMap headers, MultiValueMap params, + InputStream requestEntity) { + this(serviceId, method, uri, retryable, headers, params, requestEntity, + new ArrayList(), null); + } + public URI uri() { try { return new URI(this.uri); - } - catch (URISyntaxException e) { + } catch (URISyntaxException e) { ReflectionUtils.rethrowRuntimeException(e); } return null; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index 2df7c7bd..8e355424 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -47,8 +47,8 @@ public class RibbonRoutingFilter extends ZuulFilter { protected List requestCustomizers; public RibbonRoutingFilter(ProxyRequestHelper helper, - RibbonCommandFactory ribbonCommandFactory, - List requestCustomizers) { + RibbonCommandFactory ribbonCommandFactory, + List requestCustomizers) { this.helper = helper; this.ribbonCommandFactory = ribbonCommandFactory; this.requestCustomizers = requestCustomizers; @@ -120,13 +120,12 @@ public class RibbonRoutingFilter extends ZuulFilter { uri = uri.replace("//", "/"); return new RibbonCommandContext(serviceId, verb, uri, retryable, headers, params, - requestEntity, this.requestCustomizers, request.getContentLengthLong()); + requestEntity, this.requestCustomizers); } protected ClientHttpResponse forward(RibbonCommandContext context) throws Exception { - Map info = this.helper.debug(context.getMethod(), - context.getUri(), context.getHeaders(), context.getParams(), - context.getRequestEntity()); + Map info = this.helper.debug(context.getMethod(), context.getUri(), + context.getHeaders(), context.getParams(), context.getRequestEntity()); RibbonCommand command = this.ribbonCommandFactory.create(context); try { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java index 5061fb1f..e9d17d9c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java @@ -20,7 +20,6 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpRequest; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpResponse; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -30,10 +29,8 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class HttpClientRibbonCommand extends AbstractRibbonCommand { public HttpClientRibbonCommand(final String commandKey, - final RibbonLoadBalancingHttpClient client, - final RibbonCommandContext context, - final ZuulProperties zuulProperties) { - super(commandKey, client, context, zuulProperties); + final RibbonLoadBalancingHttpClient client, RibbonCommandContext context) { + super(commandKey, client, context); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java index ec9be369..1f3063a6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java @@ -18,7 +18,6 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -32,8 +31,6 @@ public class HttpClientRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - - private final ZuulProperties zuulProperties; @Override public HttpClientRibbonCommand create(final RibbonCommandContext context) { @@ -42,7 +39,7 @@ public class HttpClientRibbonCommandFactory implements serviceId, RibbonLoadBalancingHttpClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new HttpClientRibbonCommand(serviceId, client, context, zuulProperties); + return new HttpClientRibbonCommand(serviceId, client, context); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java index f66f36b7..9709ae16 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java @@ -20,7 +20,6 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonRequest; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonResponse; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -30,10 +29,8 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class OkHttpRibbonCommand extends AbstractRibbonCommand { public OkHttpRibbonCommand(final String commandKey, - final OkHttpLoadBalancingClient client, - final RibbonCommandContext context, - final ZuulProperties zuulProperties) { - super(commandKey, client, context, zuulProperties); + final OkHttpLoadBalancingClient client, RibbonCommandContext context) { + super(commandKey, client, context); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java index 61001443..fc65e683 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java @@ -18,7 +18,6 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -32,8 +31,6 @@ public class OkHttpRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - - private final ZuulProperties zuulProperties; @Override public OkHttpRibbonCommand create(final RibbonCommandContext context) { @@ -42,7 +39,7 @@ public class OkHttpRibbonCommandFactory implements serviceId, OkHttpLoadBalancingClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new OkHttpRibbonCommand(serviceId, client, context, zuulProperties); + return new OkHttpRibbonCommand(serviceId, client, context); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 36beb774..eb15b4ba 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -18,10 +18,10 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.util.StringUtils; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; @@ -32,48 +32,39 @@ import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties; -import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; import com.netflix.zuul.constants.ZuulConstants; import com.netflix.zuul.context.RequestContext; /** * @author Spencer Gibb */ -public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> - extends HystrixCommand implements RibbonCommand { +public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> extends HystrixCommand implements + RibbonCommand { protected final LBC client; protected RibbonCommandContext context; - public AbstractRibbonCommand(LBC client, RibbonCommandContext context, - ZuulProperties zuulProperties) { - this("default", client, context, zuulProperties); + public AbstractRibbonCommand(LBC client, RibbonCommandContext context) { + this("default", client, context); } - public AbstractRibbonCommand(String commandKey, LBC client, - RibbonCommandContext context, ZuulProperties zuulProperties) { - super(getSetter(commandKey, zuulProperties)); + public AbstractRibbonCommand(String commandKey, LBC client, RibbonCommandContext context) { + super(getSetter(commandKey)); this.client = client; this.context = context; } - protected static Setter getSetter(final String commandKey, - ZuulProperties zuulProperties) { + protected static Setter getSetter(final String commandKey) { + // we want to default to semaphore-isolation since this wraps + // 2 others commands that are already thread isolated // @formatter:off - final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter() - .withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy()); - if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){ - final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; - // we want to default to semaphore-isolation since this wraps - // 2 others commands that are already thread isolated - final DynamicIntProperty value = DynamicPropertyFactory.getInstance() - .getIntProperty(name, 100); - setter.withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); - } else { - // TODO Find out is some parameters can be set here - } - + final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; + final DynamicIntProperty value = DynamicPropertyFactory.getInstance() + .getIntProperty(name, 100); + final HystrixCommandProperties.Setter setter = HystrixCommandProperties .Setter() + .withExecutionIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE) + .withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); return Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand")) .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey + "RibbonCommand")) .andCommandPropertiesDefaults(setter); @@ -83,6 +74,10 @@ public abstract class AbstractRibbonCommand headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); - headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonApacheHttpRequest httpRequest = - new RibbonApacheHttpRequest( - new RibbonCommandContext("example", "GET", uri, false, headers, params, null, new ArrayList())); + RibbonApacheHttpRequest httpRequest = new RibbonApacheHttpRequest(new RibbonCommandContext("example", "GET", uri, false, + headers, params, null)); HttpUriRequest request = httpRequest.toRequest(RequestConfig.custom().build()); @@ -67,9 +63,7 @@ public class RibbonApacheHttpRequestTests { assertThat("uri is wrong", request.getURI().toString(), startsWith(uri)); assertThat("my-header is missing", request.getFirstHeader("my-header"), is(notNullValue())); assertThat("my-header is wrong", request.getFirstHeader("my-header").getValue(), is(equalTo("my-value"))); - assertThat("Content-Length is wrong", request.getFirstHeader(HttpEncoding.CONTENT_LENGTH).getValue(), is(equalTo("5192"))); assertThat("myparam is missing", request.getURI().getQuery(), is(equalTo("myparam=myparamval"))); - } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java index d35ea62d..4c6c84fd 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java @@ -26,11 +26,9 @@ import static org.junit.Assert.assertThat; import java.io.ByteArrayInputStream; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; import org.junit.Test; -import org.springframework.cloud.netflix.feign.encoding.HttpEncoding; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.util.LinkedMultiValueMap; @@ -49,41 +47,33 @@ public class OkHttpRibbonRequestTests { String uri = "http://example.com"; LinkedMultiValueMap headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); - // headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, - false, headers, params, null, new ArrayList()); + RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, false, headers, params, null); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); Request request = httpRequest.toRequest(); assertThat("body is not null", request.body(), is(nullValue())); assertThat("uri is wrong", request.url().toString(), startsWith(uri)); - assertThat("my-header is wrong", request.header("my-header"), - is(equalTo("my-value"))); - assertThat("myparam is missing", request.url().queryParameter("myparam"), - is(equalTo("myparamval"))); + assertThat("my-header is wrong", request.header("my-header"), is(equalTo("my-value"))); + assertThat("myparam is missing", request.url().queryParameter("myparam"), is(equalTo("myparamval"))); } @Test - // this situation happens, see - // https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 + // this situation happens, see https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 public void testEmptyEntityGet() throws Exception { String entityValue = ""; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, - "GET"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, "GET"); } @Test public void testNonEmptyEntityPost() throws Exception { String entityValue = "abcd"; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, - "POST"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, "POST"); } - void testEntity(String entityValue, ByteArrayInputStream requestEntity, - boolean addContentLengthHeader, String method) throws IOException { + void testEntity(String entityValue, ByteArrayInputStream requestEntity, boolean addContentLengthHeader, String method) throws IOException { String lengthString = String.valueOf(entityValue.length()); Long length = null; String uri = "http://example.com"; @@ -104,9 +94,8 @@ public class OkHttpRibbonRequestTests { builder.addHeader("from-customizer", "foo"); } }; - RibbonCommandContext context = new RibbonCommandContext("example", method, uri, - false, headers, new LinkedMultiValueMap(), requestEntity, - Collections.singletonList(requestCustomizer)); + RibbonCommandContext context = new RibbonCommandContext("example", method, uri, false, + headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); context.setContentLength(length); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); @@ -123,8 +112,7 @@ public class OkHttpRibbonRequestTests { if (!method.equalsIgnoreCase("get")) { assertThat("body is null", request.body(), is(notNullValue())); RequestBody body = request.body(); - assertThat("contentLength is wrong", body.contentLength(), - is(equalTo((long) entityValue.length()))); + assertThat("contentLength is wrong", body.contentLength(), is(equalTo((long) entityValue.length()))); Buffer content = new Buffer(); body.writeTo(content); String string = content.readByteString().utf8(); @@ -132,3 +120,4 @@ public class OkHttpRibbonRequestTests { } } } + diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java index 5b4671d4..f8d43345 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java @@ -27,13 +27,10 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.net.URI; import java.nio.charset.Charset; -import java.util.ArrayList; import java.util.Collections; -import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.StreamUtils; @@ -44,13 +41,6 @@ import com.netflix.client.http.HttpRequest; */ public class RestClientRibbonCommandTests { - private ZuulProperties zuulProperties; - - @Before - public void setUp() { - zuulProperties = new ZuulProperties(); - } - @Test public void testNullEntity() throws Exception { String uri = "http://example.com"; @@ -58,10 +48,8 @@ public class RestClientRibbonCommandTests { headers.add("my-header", "my-value"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RestClientRibbonCommand command = - new RestClientRibbonCommand("cmd", null, - new RibbonCommandContext("example", "GET", uri, false, headers, params, null,new ArrayList()), - zuulProperties); + RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, new RibbonCommandContext("example", "GET", uri, false, + headers, params, null)); HttpRequest request = command.createRequest(); @@ -108,7 +96,7 @@ public class RestClientRibbonCommandTests { uri.toString(), false, headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); context.setContentLength(length); - RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context, zuulProperties); + RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context); HttpRequest request = command.createRequest(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java index e6765192..d4bcfa2c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java @@ -41,7 +41,6 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -180,7 +179,7 @@ public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory, new ZuulProperties()); + return new HttpClientRibbonCommandFactory(clientFactory); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java index 023e7abe..7c4086f8 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java @@ -32,7 +32,6 @@ import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.netflix.ribbon.RibbonClients; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; -import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -108,7 +107,7 @@ public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory, new ZuulProperties()); + return new OkHttpRibbonCommandFactory(clientFactory); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java index 3a88e5c5..bb905634 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java @@ -330,7 +330,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { private SpringClientFactory clientFactory; public MyRibbonCommandFactory(SpringClientFactory clientFactory) { - super(clientFactory, new ZuulProperties()); + super(clientFactory); this.clientFactory = clientFactory; } @@ -356,7 +356,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { public MyCommand(int errorCode, String commandKey, RestClient restClient, RibbonCommandContext context) { - super(commandKey, restClient, context, new ZuulProperties()); + super(commandKey, restClient, context); this.errorCode = errorCode; }