Updated Branches: refs/heads/master 1178f47cd -> d52f46056
Introduce StripExpectHeader filter and a property to control it. Some providers (specifically HP Cloud and Google Cloud Storage) do not properly support Expect: 100-continue headers. JDK7 is stricter in its handling of the Expect header than JDK6 -- in particular, it expects servers to properly respond to an expect header and times out only if a prior timeout did not exist on the underlying HTTP connection. As a result, JDK7 tests against these providers hang and fail. This commit introduces a new filter -- appropriate called StripExpectHeader -- that is controlled by the property jclouds.strip-expect-header. The property defaults to false to preserve existing behavior but allows applications to tweak Expect header handling. Tested by running HPCS live tests with JDK7 -- previously most of these tests would fail with timeouts. Closes JCLOUDS-181 Project: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/commit/d52f4605 Tree: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/tree/d52f4605 Diff: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/diff/d52f4605 Branch: refs/heads/master Commit: d52f4605626a34dc8c488dfa3e62eed605053382 Parents: 1178f47 Author: Diwaker Gupta <[email protected]> Authored: Wed Jul 10 14:30:51 2013 -0700 Committer: Andrew Gaul <[email protected]> Committed: Thu Jul 11 15:04:05 2013 -0700 ---------------------------------------------------------------------- core/src/main/java/org/jclouds/Constants.java | 5 +++ .../jclouds/apis/internal/BaseApiMetadata.java | 3 +- .../jclouds/http/filters/StripExpectHeader.java | 36 ++++++++++++++++ .../rest/internal/RestAnnotationProcessor.java | 10 ++++- .../http/filters/StripExpectHeaderTest.java | 38 +++++++++++++++++ .../internal/RestAnnotationProcessorTest.java | 43 ++++++++++++++++++-- 6 files changed, 129 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/main/java/org/jclouds/Constants.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/Constants.java b/core/src/main/java/org/jclouds/Constants.java index 1185edb..2c4304d 100644 --- a/core/src/main/java/org/jclouds/Constants.java +++ b/core/src/main/java/org/jclouds/Constants.java @@ -296,4 +296,9 @@ public interface Constants { */ public static final String PROPERTY_PRETTY_PRINT_PAYLOADS = "jclouds.payloads.pretty-print"; + /** + * When true, strip the Expect: 100-continue header. Useful when interacting with + * providers that don't properly support Expect headers. Defaults to false. + */ + public static final String PROPERTY_STRIP_EXPECT_HEADER = "jclouds.strip-expect-header"; } http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java b/core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java index 42b159a..66a9110 100644 --- a/core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java +++ b/core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java @@ -29,6 +29,7 @@ import static org.jclouds.Constants.PROPERTY_PRETTY_PRINT_PAYLOADS; import static org.jclouds.Constants.PROPERTY_SCHEDULER_THREADS; import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL; import static org.jclouds.Constants.PROPERTY_SO_TIMEOUT; +import static org.jclouds.Constants.PROPERTY_STRIP_EXPECT_HEADER; import static org.jclouds.Constants.PROPERTY_USER_THREADS; import static org.jclouds.reflect.Reflection2.typeToken; @@ -46,7 +47,6 @@ import com.google.common.base.Optional; import com.google.common.collect.ImmutableSet; import com.google.common.reflect.TypeToken; import com.google.inject.Module; - /** * The BaseApiMetadata class is an abstraction of {@link ApiMetadata} to be extended by those * implementing ApiMetadata. @@ -72,6 +72,7 @@ public abstract class BaseApiMetadata implements ApiMetadata { props.setProperty(PROPERTY_MAX_SESSION_FAILURES, 2 + ""); props.setProperty(PROPERTY_SESSION_INTERVAL, 60 + ""); props.setProperty(PROPERTY_PRETTY_PRINT_PAYLOADS, "true"); + props.setProperty(PROPERTY_STRIP_EXPECT_HEADER, "false"); return props; } http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/main/java/org/jclouds/http/filters/StripExpectHeader.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/http/filters/StripExpectHeader.java b/core/src/main/java/org/jclouds/http/filters/StripExpectHeader.java new file mode 100644 index 0000000..fa8c7d7 --- /dev/null +++ b/core/src/main/java/org/jclouds/http/filters/StripExpectHeader.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jclouds.http.filters; + +import org.jclouds.http.HttpException; +import org.jclouds.http.HttpRequest; +import org.jclouds.http.HttpRequestFilter; + +import com.google.common.net.HttpHeaders; +import com.google.inject.Singleton; + +/** + * @author Diwaker Gupta + */ +@Singleton +public class StripExpectHeader implements HttpRequestFilter { + @Override + public HttpRequest filter(HttpRequest request) throws HttpException { + return request.toBuilder().removeHeader(HttpHeaders.EXPECT).build(); + } +} http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java index 2a32ad1..7fd789d 100644 --- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java +++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java @@ -50,7 +50,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.NoSuchElementException; import java.util.Set; - import javax.annotation.Resource; import javax.inject.Named; import javax.ws.rs.FormParam; @@ -65,6 +64,7 @@ import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpRequestFilter; import org.jclouds.http.HttpUtils; import org.jclouds.http.Uris.UriBuilder; +import org.jclouds.http.filters.StripExpectHeader; import org.jclouds.http.options.HttpRequestOptions; import org.jclouds.io.ContentMetadataCodec; import org.jclouds.io.Payload; @@ -149,11 +149,13 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest private final InputParamValidator inputParamValidator; private final GetAcceptHeaders getAcceptHeaders; private final Invocation caller; + private final boolean stripExpectHeader; @Inject private RestAnnotationProcessor(Injector injector, @ApiVersion String apiVersion, @BuildVersion String buildVersion, HttpUtils utils, ContentMetadataCodec contentMetadataCodec, InputParamValidator inputParamValidator, - GetAcceptHeaders getAcceptHeaders, @Nullable @Named("caller") Invocation caller) { + GetAcceptHeaders getAcceptHeaders, @Nullable @Named("caller") Invocation caller, + @Named(Constants.PROPERTY_STRIP_EXPECT_HEADER) boolean stripExpectHeader) { this.injector = injector; this.utils = utils; this.contentMetadataCodec = contentMetadataCodec; @@ -162,6 +164,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest this.inputParamValidator = inputParamValidator; this.getAcceptHeaders = getAcceptHeaders; this.caller = caller; + this.stripExpectHeader = stripExpectHeader; } /** @@ -208,6 +211,9 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest } requestBuilder.filters(getFiltersIfAnnotated(invocation)); + if (stripExpectHeader) { + requestBuilder.filter(new StripExpectHeader()); + } Multimap<String, Object> tokenValues = LinkedHashMultimap.create(); http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/test/java/org/jclouds/http/filters/StripExpectHeaderTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/http/filters/StripExpectHeaderTest.java b/core/src/test/java/org/jclouds/http/filters/StripExpectHeaderTest.java new file mode 100644 index 0000000..c38002c --- /dev/null +++ b/core/src/test/java/org/jclouds/http/filters/StripExpectHeaderTest.java @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jclouds.http.filters; + +import static org.testng.Assert.assertFalse; + +import org.jclouds.http.HttpRequest; +import org.testng.annotations.Test; + +import com.google.common.net.HttpHeaders; + +/** + * @author Diwaker Gupta + */ +@Test(groups = "unit") +public class StripExpectHeaderTest { + public void testExpectHeaderIsStripped() { + HttpRequest request = HttpRequest.builder().method("POST").addHeader(HttpHeaders.EXPECT, "100-Continue") + .endpoint("http://localhost";).build(); + request = new StripExpectHeader().filter(request); + assertFalse(request.getHeaders().containsKey(HttpHeaders.EXPECT)); + } +} http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/d52f4605/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java index 1a42f57..4e07666 100644 --- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java +++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java @@ -24,6 +24,7 @@ import static org.jclouds.io.Payloads.newStringPayload; import static org.jclouds.reflect.Reflection2.method; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; import java.io.Closeable; @@ -42,9 +43,9 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.Map; +import java.util.Properties; import java.util.Set; import java.util.concurrent.ExecutionException; - import javax.inject.Named; import javax.inject.Qualifier; import javax.inject.Singleton; @@ -62,6 +63,7 @@ import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; import org.eclipse.jetty.http.HttpHeaders; +import org.jclouds.Constants; import org.jclouds.ContextBuilder; import org.jclouds.date.DateService; import org.jclouds.date.internal.SimpleDateFormatDateService; @@ -72,6 +74,7 @@ import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpRequestFilter; import org.jclouds.http.HttpResponse; import org.jclouds.http.IOExceptionRetryHandler; +import org.jclouds.http.filters.StripExpectHeader; import org.jclouds.http.functions.ParseFirstJsonValueNamed; import org.jclouds.http.functions.ParseJson; import org.jclouds.http.functions.ParseSax; @@ -851,8 +854,8 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { Lists.<Object> newArrayList((String) null))); Assert.fail("call should have failed with illegal null parameter, not permitted " + request + " to be created"); } catch (NullPointerException e) { - Assert.assertTrue(e.toString().indexOf("postNonnull parameter 1") >= 0, - "Error message should have referred to 'parameter 1': " + e); + assertTrue(e.toString().indexOf("postNonnull parameter 1") >= 0, + "Error message should have referred to 'parameter 1': " + e); } } @@ -1367,6 +1370,9 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { @OverrideRequestFilters @RequestFilters(TestRequestFilter2.class) public void getOverride(HttpRequest request); + + @POST + public void post(); } @Test @@ -1396,6 +1402,37 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { assertEquals(request.getFilters().get(0).getClass(), TestRequestFilter2.class); } + @Test + public void testRequestFilterStripExpect() { + // First, verify that by default, the StripExpectHeader filter is not applied + Invokable<?, ?> method = method(TestRequestFilter.class, "post"); + Invocation invocation = Invocation.create(method, + ImmutableList.<Object>of(HttpRequest.builder().method("POST").endpoint("http://localhost";) + .addHeader(HttpHeaders.EXPECT, "100-Continue").build())); + GeneratedHttpRequest request = processor.apply(invocation); + assertEquals(request.getFilters().size(), 1); + assertEquals(request.getFilters().get(0).getClass(), TestRequestFilter1.class); + + // Now let's create a new injector with the property set. Use that to create the annotation processor. + Properties overrides = new Properties(); + overrides.setProperty(Constants.PROPERTY_STRIP_EXPECT_HEADER, "true"); + Injector injector = ContextBuilder.newBuilder( + AnonymousProviderMetadata.forClientMappedToAsyncClientOnEndpoint(Callee.class, AsyncCallee.class, + "http://localhost:9999";)) + .modules(ImmutableSet.<Module> of(new MockModule(), new NullLoggingModule(), new AbstractModule() { + protected void configure() { + bind(new TypeLiteral<Supplier<URI>>() { + }).annotatedWith(Localhost2.class).toInstance( + Suppliers.ofInstance(URI.create("http://localhost:1111";))); + }})) + .overrides(overrides).buildInjector(); + RestAnnotationProcessor newProcessor = injector.getInstance(RestAnnotationProcessor.class); + // Verify that this time the filter is indeed applied as expected. + request = newProcessor.apply(invocation); + assertEquals(request.getFilters().size(), 2); + assertEquals(request.getFilters().get(1).getClass(), StripExpectHeader.class); + } + public class TestEncoding { @GET @Path("/{path1}/{path2}")
