diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/config/EC2RestClientModuleExpectTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/config/EC2RestClientModuleExpectTest.java index 97ae9ea433..cee0074d32 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/config/EC2RestClientModuleExpectTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/config/EC2RestClientModuleExpectTest.java @@ -26,7 +26,6 @@ import java.util.Properties; import java.util.Set; import org.jclouds.ec2.internal.BaseEC2ExpectTest; -import org.jclouds.ec2.services.BaseEC2AsyncClientTest.StubEC2RestClientModule; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; import org.jclouds.location.Region; @@ -140,9 +139,5 @@ public class EC2RestClientModuleExpectTest extends BaseEC2ExpectTest { public Injector createClient(Function fn, Module module, Properties props) { return createInjector(fn, module, props); } - - @Override - protected Module createModule() { - return new StubEC2RestClientModule(); - } + } diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ExpectTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ExpectTest.java index 3a07855a1d..0397f2b85c 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ExpectTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/internal/BaseEC2ExpectTest.java @@ -28,11 +28,16 @@ import org.jclouds.date.DateService; import org.jclouds.date.internal.SimpleDateFormatDateService; import org.jclouds.ec2.EC2AsyncClient; import org.jclouds.ec2.EC2Client; +import org.jclouds.ec2.EC2ContextBuilder; +import org.jclouds.ec2.EC2PropertiesBuilder; import org.jclouds.ec2.config.EC2RestClientModule; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; +import org.jclouds.http.RequiresHttp; import org.jclouds.rest.BaseRestClientExpectTest; import org.jclouds.rest.ConfiguresRestClient; +import org.jclouds.rest.RestContextFactory; +import org.jclouds.rest.RestContextSpec; import com.google.common.base.Functions; import com.google.common.collect.ImmutableMap; @@ -40,7 +45,6 @@ import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.google.inject.Module; -import com.google.inject.Provides; public abstract class BaseEC2ExpectTest extends BaseRestClientExpectTest { protected static final String CONSTANT_DATE = "2012-04-16T15:54:08.897Z"; @@ -82,18 +86,36 @@ public abstract class BaseEC2ExpectTest extends BaseRestClientExpectTest { } describeAvailabilityZonesRequestResponse = builder.build(); } - + + @RequiresHttp @ConfiguresRestClient - private static final class TestEC2RestClientModule extends EC2RestClientModule { + public static class StubEC2RestClientModule extends EC2RestClientModule { + + public StubEC2RestClientModule() { + super(EC2Client.class, EC2AsyncClient.class, DELEGATE_MAP); + } + @Override - @Provides protected String provideTimeStamp(DateService dateService) { return CONSTANT_DATE; } } - + @Override protected Module createModule() { - return new TestEC2RestClientModule(); + return new StubEC2RestClientModule(); + } + + protected String provider = "ec2"; + + /** + * this is only here as "ec2" is not in rest.properties + */ + @SuppressWarnings({ "unchecked", "rawtypes" }) + @Override + public RestContextSpec makeContextSpec() { + return RestContextFactory.contextSpec(provider, "https://ec2.us-east-1.amazonaws.com", EC2AsyncClient.VERSION, + "", "", "identity", "credential", EC2Client.class, EC2AsyncClient.class, + (Class) EC2PropertiesBuilder.class, (Class) EC2ContextBuilder.class, ImmutableSet. of()); } } diff --git a/common/aws/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java b/common/aws/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java index 8c39c3b43f..615801b439 100644 --- a/common/aws/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java +++ b/common/aws/src/main/java/org/jclouds/aws/handlers/ParseAWSErrorFromXmlContent.java @@ -18,6 +18,7 @@ */ package org.jclouds.aws.handlers; +import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; import static org.jclouds.http.HttpUtils.releasePayload; import java.io.IOException; @@ -65,7 +66,9 @@ public class ParseAWSErrorFromXmlContent implements HttpErrorHandler { Exception exception = new HttpResponseException(command, response); try { AWSError error = null; - String message = null; + // it is important to always read fully and close streams + byte[] data = closeClientButKeepContentStream(response); + String message = data != null ? new String(data) : null; if (response.getPayload() != null) { String contentType = response.getPayload().getContentMetadata().getContentType(); if (contentType != null && (contentType.indexOf("xml") != -1 || contentType.indexOf("unknown") != -1)) { @@ -102,14 +105,14 @@ public class ParseAWSErrorFromXmlContent implements HttpErrorHandler { exception = new UnsupportedOperationException(message, exception); else if ("AddressLimitExceeded".equals(errorCode)) exception = new InsufficientResourcesException(message, exception); - else if (errorCode != null && (errorCode.endsWith("NotFound") || errorCode.endsWith(".Unknown"))) + else if (errorCode != null && (errorCode.indexOf("NotFound") != -1 || errorCode.endsWith(".Unknown"))) exception = new ResourceNotFoundException(message, exception); else if ("IncorrectState".equals(errorCode) || (errorCode != null && (error.getCode().endsWith(".Duplicate") | error.getCode().endsWith( ".InUse"))) || (message != null && (message.indexOf("already exists") != -1 || message.indexOf("is in use") != -1))) exception = new IllegalStateException(message, exception); - else if ("AuthFailure".equals(errorCode)) + else if (errorCode != null && errorCode.indexOf("AuthFailure") != -1) exception = new AuthorizationException(message, exception); else if (message != null && (message.indexOf("Invalid id") != -1 || message.indexOf("Failed to bind") != -1)) diff --git a/core/src/test/java/org/jclouds/rest/BaseRestClientExpectTest.java b/core/src/test/java/org/jclouds/rest/BaseRestClientExpectTest.java index f689f78fc5..ea518cda72 100644 --- a/core/src/test/java/org/jclouds/rest/BaseRestClientExpectTest.java +++ b/core/src/test/java/org/jclouds/rest/BaseRestClientExpectTest.java @@ -17,7 +17,6 @@ * under the License. */ package org.jclouds.rest; - import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.annotation.ElementType.TYPE; import static java.lang.annotation.RetentionPolicy.RUNTIME; @@ -40,6 +39,12 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.custommonkey.xmlunit.Diff; +import org.custommonkey.xmlunit.Difference; +import org.custommonkey.xmlunit.DifferenceConstants; +import org.custommonkey.xmlunit.DifferenceListener; +import org.custommonkey.xmlunit.NodeDetail; +import org.custommonkey.xmlunit.XMLUnit; import org.jclouds.Constants; import org.jclouds.concurrent.MoreExecutors; import org.jclouds.concurrent.SingleThreaded; @@ -59,14 +64,18 @@ import org.jclouds.io.Payloads; import org.jclouds.logging.config.NullLoggingModule; import org.jclouds.util.Strings2; import org.testng.annotations.Test; +import org.w3c.dom.Node; import com.google.common.annotations.Beta; import com.google.common.base.Function; +import com.google.common.base.Objects; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.gson.JsonElement; +import com.google.gson.JsonParser; import com.google.inject.AbstractModule; import com.google.inject.Binder; import com.google.inject.Injector; @@ -111,6 +120,7 @@ public abstract class BaseRestClientExpectTest { Class async(); } + protected String provider = "mock"; @@ -326,7 +336,9 @@ public abstract class BaseRestClientExpectTest { return HttpResponse.builder().statusCode(500).message( String.format("request %s is out of range (%s)", index, requests.size())).payload( Payloads.newStringPayload(renderRequest(input))).build(); - assertEquals(renderRequest(input), renderRequest(requests.get(index))); + if (!httpRequestsAreEqual(input, requests.get(index))) { + assertEquals(renderRequest(input), renderRequest(requests.get(index))); + } return responses.get(index); } }); @@ -344,14 +356,98 @@ public abstract class BaseRestClientExpectTest { return requestsSendResponses(requestToResponse, createModule()); } + protected enum HttpRequestComparisonType { + XML, JSON, DEFAULT; + } + + /** + * How should this HttpRequest be compared with others? + */ + protected HttpRequestComparisonType compareHttpRequestAsType(HttpRequest input) { + return HttpRequestComparisonType.DEFAULT; + } + + /** + * Compare two requests as instructed by {@link #compareHttpRequestAsType(HttpRequest)} - default + * is to compare using Objects.equal + */ + public boolean httpRequestsAreEqual(HttpRequest a, HttpRequest b) { + try { + if (a == null || b == null || !Objects.equal(a.getRequestLine(), b.getRequestLine()) + || !Objects.equal(a.getHeaders(), b.getHeaders())) { + return false; + } + if (a.getPayload() == null || b.getPayload() == null) { + return Objects.equal(a, b); + } + + switch (compareHttpRequestAsType(a)) { + case XML: { + Diff diff = XMLUnit.compareXML(Strings2.toStringAndClose(a.getPayload().getInput()), Strings2 + .toStringAndClose(b.getPayload().getInput())); + + // Ignoring whitespace in elements that have other children, xsi:schemaLocation and + // differences in namespace prefixes + diff.overrideDifferenceListener(new DifferenceListener() { + @Override + public int differenceFound(Difference diff) { + if (diff.getId() == DifferenceConstants.SCHEMA_LOCATION_ID + || diff.getId() == DifferenceConstants.NAMESPACE_PREFIX_ID) { + return RETURN_IGNORE_DIFFERENCE_NODES_IDENTICAL; + } + if (diff.getId() == DifferenceConstants.TEXT_VALUE_ID) { + for (NodeDetail detail : ImmutableSet.of(diff.getControlNodeDetail(), diff.getTestNodeDetail())) { + if (detail.getNode().getParentNode().getChildNodes().getLength() < 2 + || !detail.getValue().trim().isEmpty()) { + return RETURN_ACCEPT_DIFFERENCE; + } + } + return RETURN_IGNORE_DIFFERENCE_NODES_IDENTICAL; + } + return RETURN_ACCEPT_DIFFERENCE; + } + + @Override + public void skippedComparison(Node node, Node node1) { + } + }); + + return diff.identical(); + } + case JSON: { + JsonParser parser = new JsonParser(); + JsonElement payloadA = parser.parse(Strings2.toStringAndClose(a.getPayload().getInput())); + JsonElement payloadB = parser.parse(Strings2.toStringAndClose(b.getPayload().getInput())); + return Objects.equal(payloadA, payloadB); + } + default: { + return Objects.equal(a, b); + } + } + } catch (Exception e) { + throw Throwables.propagate(e); + } + } + public S requestsSendResponses(final Map requestToResponse, Module module) { + return requestsSendResponses(requestToResponse, module, setupProperties()); + } + + public S requestsSendResponses(final Map requestToResponse, Module module, + Properties props) { return createClient(new Function() { ImmutableBiMap bimap = ImmutableBiMap.copyOf(requestToResponse); @Override public HttpResponse apply(HttpRequest input) { - if (!(requestToResponse.containsKey(input))) { + HttpResponse response = null; + for (HttpRequest request : requestToResponse.keySet()) { + if (httpRequestsAreEqual(input, request)) { + response = requestToResponse.get(request); + } + } + if (response == null) { StringBuilder payload = new StringBuilder("\n"); payload.append("the following request is not configured:\n"); payload.append("----------------------------------------\n"); @@ -362,16 +458,17 @@ public abstract class BaseRestClientExpectTest { payload.append("----------------------------------------\n"); payload.append(renderRequest(request)); } - return HttpResponse.builder().statusCode(500).message("no response configured for request").payload( + response = HttpResponse.builder().statusCode(500).message("no response configured for request").payload( Payloads.newStringPayload(payload.toString())).build(); + } else if (compareHttpRequestAsType(input) == HttpRequestComparisonType.DEFAULT) { + // in case hashCode/equals doesn't do a full content check + assertEquals(renderRequest(input), renderRequest(bimap.inverse().get(response))); } - HttpResponse response = requestToResponse.get(input); - // in case hashCode/equals doesn't do a full content check - assertEquals(renderRequest(input), renderRequest(bimap.inverse().get(response))); + return response; } - }, module); + }, module, props); } public String renderRequest(HttpRequest request) { @@ -381,7 +478,7 @@ public abstract class BaseRestClientExpectTest { } if (request.getPayload() != null) { for (Entry header : HttpUtils.getContentHeadersFromMetadata( - request.getPayload().getContentMetadata()).entries()) { + request.getPayload().getContentMetadata()).entries()) { builder.append(header.getKey()).append(": ").append(header.getValue()).append('\n'); } try { @@ -402,7 +499,6 @@ public abstract class BaseRestClientExpectTest { public S createClient(Function fn, Module module) { return createClient(fn, module, setupProperties()); - } public S createClient(Function fn, Properties props) { @@ -428,7 +524,7 @@ public abstract class BaseRestClientExpectTest { protected String credential = "credential"; @SuppressWarnings("unchecked") - private RestContextSpec makeContextSpec() { + protected RestContextSpec makeContextSpec() { if (getClass().isAnnotationPresent(RegisterContext.class)) return (RestContextSpec) contextSpec(provider, "http://mock", "1", "", "", "userfoo", null, getClass() .getAnnotation(RegisterContext.class).sync(), diff --git a/project/pom.xml b/project/pom.xml index a615b8fd4f..2973f79618 100644 --- a/project/pom.xml +++ b/project/pom.xml @@ -258,6 +258,12 @@ 1.2.1 test + + xmlunit + xmlunit + 1.3 + test +