From e62bbc913a9d6075b613ebbc414ee3ed1b4ed753 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Sun, 26 Oct 2014 19:57:34 +0100 Subject: [PATCH 1/6] Add exception conversion to ConverterService- Issue #161 Add content negotiation in StatusFilter --- .../resource/AnnotatedResource20TestCase.java | 19 ++- .../{MyException.java => MyException01.java} | 6 +- .../restlet/test/resource/MyException02.java | 25 ++++ .../restlet/test/resource/MyResource20.java | 5 +- .../test/resource/MyServerResource20.java | 11 +- .../test/service/StatusServiceTestCase.java | 71 +++++++++++- .../engine/application/StatusFilter.java | 30 ++++- .../engine/resource/AnnotationUtils.java | 6 +- .../engine/resource/StatusAnnotationInfo.java | 34 ++++-- .../engine/util/ThrowableSerializer.java | 108 ++++++++++++++++++ .../src/org/restlet/resource/Status.java | 27 ++++- .../org/restlet/service/StatusService.java | 107 ++++++++++++++++- 12 files changed, 408 insertions(+), 41 deletions(-) rename modules/org.restlet.test/src/org/restlet/test/resource/{MyException.java => MyException01.java} (77%) create mode 100644 modules/org.restlet.test/src/org/restlet/test/resource/MyException02.java create mode 100644 modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java b/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java index 93c4c16088..41f742a575 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java @@ -77,9 +77,24 @@ protected void tearDown() throws Exception { public void testGet() throws IOException, ResourceException { try { myResource.represent(); - } catch (MyException e) { - assertNotNull(e.getDate()); + fail("should fail"); + } catch (MyException01 e) { + fail("exception should be catch by client resource"); + } catch (ResourceException e) { + assertEquals(400, e.getStatus().getCode()); } } + + public void testGetAndSerializeException() throws IOException, ResourceException { + try { + myResource.representAndSerializeException(); + fail("should fail"); + } catch (MyException02 e) { + fail("exception should be catch by client resource"); + } catch (ResourceException e) { + assertEquals(400, e.getStatus().getCode()); + //TODO How to retrieve the response entity with the error representation ? + } + } } diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyException.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyException01.java similarity index 77% rename from modules/org.restlet.test/src/org/restlet/test/resource/MyException.java rename to modules/org.restlet.test/src/org/restlet/test/resource/MyException01.java index f37ef9617d..7f3a10d18f 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/MyException.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyException01.java @@ -4,14 +4,14 @@ import org.restlet.resource.Status; -@Status("401") -public class MyException extends Throwable { +@Status(400) +public class MyException01 extends Throwable { private static final long serialVersionUID = 1L; private Date date; - public MyException(Date date) { + public MyException01(Date date) { this.date = date; } diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyException02.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyException02.java new file mode 100644 index 0000000000..8205f13f25 --- /dev/null +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyException02.java @@ -0,0 +1,25 @@ +package org.restlet.test.resource; + +import org.restlet.resource.Status; + +import java.util.Date; + +@Status(value = 400, serializeProperties = true) +public class MyException02 extends Throwable { + + private static final long serialVersionUID = 1L; + + private String customProperty; + + public MyException02(String customProperty) { + this.customProperty = customProperty ; + } + + public String getCustomProperty() { + return customProperty; + } + + public void setCustomProperty(String customProperty) { + this.customProperty = customProperty; + } +} diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java index b92e7b81ad..1488a8027a 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java @@ -43,6 +43,9 @@ public interface MyResource20 { @Get - MyBean represent() throws MyException; + MyBean represent() throws MyException01; + + @Get + MyBean representAndSerializeException() throws MyException02; } diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java index aaa54ebde5..6a434d3522 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java @@ -55,12 +55,13 @@ public static void main(String[] args) throws Exception { private volatile MyBean myBean = new MyBean("myName", "myDescription"); @SuppressWarnings("unused") - public MyBean represent() throws MyException { - if (true) { - throw new MyException(new Date()); - } + public MyBean represent() throws MyException01 { + throw new MyException01(new Date()); + } - return myBean; + @Override + public MyBean representAndSerializeException() throws MyException02 { + throw new MyException02("representAndSerializeException error"); } } diff --git a/modules/org.restlet.test/src/org/restlet/test/service/StatusServiceTestCase.java b/modules/org.restlet.test/src/org/restlet/test/service/StatusServiceTestCase.java index 2cd542545e..8563b756b6 100644 --- a/modules/org.restlet.test/src/org/restlet/test/service/StatusServiceTestCase.java +++ b/modules/org.restlet.test/src/org/restlet/test/service/StatusServiceTestCase.java @@ -33,12 +33,24 @@ package org.restlet.test.service; +import java.io.IOException; import java.util.Date; +import java.util.HashMap; +import java.util.LinkedHashMap; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.data.MediaType; import org.restlet.data.Status; +import org.restlet.ext.jackson.JacksonRepresentation; +import org.restlet.representation.Representation; +import org.restlet.service.ConnegService; +import org.restlet.service.ConverterService; +import org.restlet.service.MetadataService; import org.restlet.service.StatusService; import org.restlet.test.RestletTestCase; -import org.restlet.test.resource.MyException; +import org.restlet.test.resource.MyException01; +import org.restlet.test.resource.MyException02; /** * Unit tests for the status service. @@ -49,8 +61,61 @@ public class StatusServiceTestCase extends RestletTestCase { public void testAnnotation() { StatusService ss = new StatusService(); - Status status = ss.toStatus(new MyException(new Date()), null, null); - assertEquals(401, status.getCode()); + Status status = ss.toStatus(new MyException01(new Date()), null, null); + assertEquals(400, status.getCode()); + } + + public void testRepresentation() throws IOException { + StatusService ss = new StatusService(); + Status status = new Status(400, new MyException01(new Date())); + + ConverterService converterService = new ConverterService(); + ConnegService connegService = new ConnegService(); + MetadataService metadataService = new MetadataService(); + + Request request = new Request(); + Representation representation = ss.toRepresentation(status, null, + request, new Response(request), + converterService, connegService, metadataService); + + //verify + Status expectedStatus = Status.CLIENT_ERROR_BAD_REQUEST; + HashMap expectedRepresentationMap = new LinkedHashMap(); + expectedRepresentationMap.put("code", expectedStatus.getCode()); + expectedRepresentationMap.put("reasonPhrase", expectedStatus.getReasonPhrase()); + expectedRepresentationMap.put("description", expectedStatus.getDescription()); + String expectedJsonRepresentation = + new JacksonRepresentation>(expectedRepresentationMap) + .getText(); + + Status.CLIENT_ERROR_BAD_REQUEST.getCode(); + assertEquals(MediaType.APPLICATION_JSON, representation.getMediaType()); + assertEquals(expectedJsonRepresentation, representation.getText()); + } + + public void testRepresentationWithExceptionPropertiesSerialized() throws IOException { + StatusService ss = new StatusService(); + Status status = new Status(400, new MyException02("test message")); + + ConverterService converterService = new ConverterService(); + ConnegService connegService = new ConnegService(); + MetadataService metadataService = new MetadataService(); + + Request request = new Request(); + Representation representation = ss.toRepresentation(status, null, + request, new Response(request), + converterService, connegService, metadataService); + + //verify + HashMap expectedRepresentationMap = new LinkedHashMap(); + expectedRepresentationMap.put("customProperty", "test message"); + String expectedJsonRepresentation = + new JacksonRepresentation>(expectedRepresentationMap) + .getText(); + + Status.CLIENT_ERROR_BAD_REQUEST.getCode(); + assertEquals(MediaType.APPLICATION_JSON, representation.getMediaType()); + assertEquals(expectedJsonRepresentation, representation.getText()); } } diff --git a/modules/org.restlet/src/org/restlet/engine/application/StatusFilter.java b/modules/org.restlet/src/org/restlet/engine/application/StatusFilter.java index 334ccf820a..13cff1b665 100644 --- a/modules/org.restlet/src/org/restlet/engine/application/StatusFilter.java +++ b/modules/org.restlet/src/org/restlet/engine/application/StatusFilter.java @@ -45,7 +45,9 @@ import org.restlet.representation.Representation; import org.restlet.representation.StringRepresentation; import org.restlet.routing.Filter; +import org.restlet.service.ConnegService; import org.restlet.service.ConverterService; +import org.restlet.service.MetadataService; import org.restlet.service.StatusService; // [excludes gwt] @@ -176,7 +178,9 @@ protected int doHandle(Request request, Response response) { if ((response.getEntity() == null) || isOverwriting()) { response.setEntity(getStatusService().toRepresentation( status, throwable, request, response, - getApplication().getConverterService())); + getConverterService(), + getConnegService(), + getMetadataService())); } } } @@ -204,6 +208,26 @@ public ConverterService getConverterService() { .getConverterService(); } + /** + * Returns the content negotiation service of the application if available or null. + * + * @return The content negotiation service of the application if available or null. + */ + public ConnegService getConnegService() { + return (getApplication() == null) ? null : getApplication() + .getConnegService(); + } + + /** + * Returns the metadata service of the application if available or null. + * + * @return The metadata service of the application if available or null. + */ + public MetadataService getMetadataService() { + return (getApplication() == null) ? null : getApplication() + .getMetadataService(); + } + /** * Returns a representation for the given status.
* In order to customize the default representation, this method can be @@ -279,7 +303,7 @@ public Reference getHomeRef() { * @param response * The response updated. * @return The representation of the given status. - * @deprecated Use {@link #toRepresentation(Status, Request, Response)} + * @deprecated Use {@link #toRepresentation(Status, Throwable, Request, Response)} * instead. */ @Deprecated @@ -403,7 +427,7 @@ protected Representation toRepresentation(Status status, try { result = getStatusService().toRepresentation(status, throwable, - request, response, getConverterService()); + request, response, getConverterService(), getConnegService(), getMetadataService()); } catch (Exception e) { getLogger().log(Level.WARNING, "Unable to get the custom status representation", e); diff --git a/modules/org.restlet/src/org/restlet/engine/resource/AnnotationUtils.java b/modules/org.restlet/src/org/restlet/engine/resource/AnnotationUtils.java index cf5c58db59..d80f9ee01b 100644 --- a/modules/org.restlet/src/org/restlet/engine/resource/AnnotationUtils.java +++ b/modules/org.restlet/src/org/restlet/engine/resource/AnnotationUtils.java @@ -27,7 +27,7 @@ * Alternatively, you can obtain a royalty free commercial license with less * limitations, transferable or non-transferable, directly at * http://restlet.com/products/restlet-framework - * + * * Restlet is a registered trademark of Restlet S.A.S. */ @@ -44,6 +44,7 @@ import org.restlet.data.Method; import org.restlet.representation.Representation; import org.restlet.resource.ServerResource; +import org.restlet.resource.Status; import org.restlet.service.MetadataService; // [excludes gwt] @@ -147,8 +148,9 @@ private List addStatusAnnotationDescriptors( .getAnnotation(org.restlet.resource.Status.class); if (annotation != null) { + Status status = (Status) annotation; result.add(new StatusAnnotationInfo(initialClass, - ((org.restlet.resource.Status) annotation).value())); + status.value(), status.serializeProperties())); } return result; diff --git a/modules/org.restlet/src/org/restlet/engine/resource/StatusAnnotationInfo.java b/modules/org.restlet/src/org/restlet/engine/resource/StatusAnnotationInfo.java index 5355c9cf88..ea5d0ed492 100644 --- a/modules/org.restlet/src/org/restlet/engine/resource/StatusAnnotationInfo.java +++ b/modules/org.restlet/src/org/restlet/engine/resource/StatusAnnotationInfo.java @@ -46,24 +46,24 @@ public class StatusAnnotationInfo extends AnnotationInfo { /** The status parsed from the annotation value. */ private final Status status; + /** The serializeProperties indicator parsed from the annotation value. */ + private final boolean serializeProperties; + /** * Constructor. - * - * @param javaClass + * @param javaClass * The class or interface that hosts the annotated Java method. - * @param value - * The annotation value. + * @param code + * The status code + * @param serializeProperties + * The indicator for serialize properties attribute. */ - public StatusAnnotationInfo(Class javaClass, String value) { - super(javaClass, null, value); + public StatusAnnotationInfo(Class javaClass, int code, boolean serializeProperties) { + super(javaClass, null, Integer.toString(code)); // Parse the main components of the annotation value - if ((value != null) && !value.equals("")) { - Integer code = Integer.parseInt(value); - this.status = Status.valueOf(code); - } else { - this.status = Status.SERVER_ERROR_INTERNAL; - } + this.status = Status.valueOf(code); + this.serializeProperties = serializeProperties; } /** @@ -101,10 +101,20 @@ public Status getStatus() { return status; } + /** + * Returns the serializeProperties indicator parsed from the annotation value. + * + * @return the serializeProperties indicator parsed from the annotation value. + */ + public boolean isSerializeProperties() { + return serializeProperties; + } + @Override public String toString() { return "StatusAnnotationInfo [javaMethod: " + javaMethod + ", javaClass: " + getJavaClass() + ", status: " + status + + ", serializeProperties: " + serializeProperties + "]"; } diff --git a/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java b/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java new file mode 100644 index 0000000000..3ee58a6626 --- /dev/null +++ b/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java @@ -0,0 +1,108 @@ +/** + * Copyright 2005-2014 Restlet + * + * The contents of this file are subject to the terms of one of the following + * open source licenses: Apache 2.0 or LGPL 3.0 or LGPL 2.1 or CDDL 1.0 or EPL + * 1.0 (the "Licenses"). You can select the license that you prefer but you may + * not use this file except in compliance with one of these Licenses. + * + * You can obtain a copy of the Apache 2.0 license at + * http://www.opensource.org/licenses/apache-2.0 + * + * You can obtain a copy of the LGPL 3.0 license at + * http://www.opensource.org/licenses/lgpl-3.0 + * + * You can obtain a copy of the LGPL 2.1 license at + * http://www.opensource.org/licenses/lgpl-2.1 + * + * You can obtain a copy of the CDDL 1.0 license at + * http://www.opensource.org/licenses/cddl1 + * + * You can obtain a copy of the EPL 1.0 license at + * http://www.opensource.org/licenses/eclipse-1.0 + * + * See the Licenses for the specific language governing permissions and + * limitations under the Licenses. + * + * Alternatively, you can obtain a royalty free commercial license with less + * limitations, transferable or non-transferable, directly at + * http://restlet.com/products/restlet-framework + * + * Restlet is a registered trademark of Restlet S.A.S. + */ + +package org.restlet.engine.util; + +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * Utilities to serialize {@link Throwable}. + * + * @author Manuel Boillod + */ +public class ThrowableSerializer { + + /** BeanInfo cache. */ + private static final ConcurrentMap, BeanInfo> cache = new ConcurrentHashMap, BeanInfo>(); + + /** + * Serialize {@link Throwable} properties to a Map using reflection. + * The properties of {@link Throwable} class are ignored except if + * they are overriden. + * + * @param throwable + * {@link Throwable} instance to serialize. + * + * @return A map with the @link Throwable} subclasses properties. + */ + public static Map serializeToMap(Throwable throwable) { + try { + BeanInfo beanInfo = getBeanInfo(throwable); + Map properties = new HashMap(); + + PropertyDescriptor[] propertyDescriptors = beanInfo.getPropertyDescriptors(); + for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { + properties.put( + propertyDescriptor.getName(), + propertyDescriptor.getReadMethod().invoke(throwable)); + } + return properties; + } catch (Exception e) { + throw new RuntimeException("Could not serialize properties of class " + throwable.getCause(), e); + } + } + + /** + * Get a BeanInfo from cache or create it. + * @param throwable + * Throwable instance + * @return BeanInfo of throwable class + */ + private static BeanInfo getBeanInfo(Throwable throwable) throws IntrospectionException { + BeanInfo result = cache.get(throwable.getClass()); + + if (result == null) { + // Inspect the class itself for annotations + result = Introspector.getBeanInfo(throwable.getClass(), Throwable.class, Introspector.IGNORE_ALL_BEANINFO); + + // Put the list in the cache if no one was previously present + BeanInfo prev = cache.putIfAbsent(throwable.getClass(), result); + + if (prev != null) { + // Reuse the previous entry + result = prev; + } + } + + return result; + } + + ; +} diff --git a/modules/org.restlet/src/org/restlet/resource/Status.java b/modules/org.restlet/src/org/restlet/resource/Status.java index e55d3577e3..38327ff208 100644 --- a/modules/org.restlet/src/org/restlet/resource/Status.java +++ b/modules/org.restlet/src/org/restlet/resource/Status.java @@ -50,15 +50,23 @@ * @Get * public MyBean represent() throws MyServerError, MyNotFoundError; * - * @Status("500") + * @Status(500) * public class MyServerError implements Throwable{ * ... * } * - * @Status("404") + * @Status(404) * public class MyNotFoundError extends RuntimeException{ * ... * } + * + * @Status(value = 400, serializeProperties = true) + * public class MyBadParameterError extends RuntimeException{ + * public String getParameterName() { + * ... + * }; + * ... + * } * * * @author Jerome Louvel @@ -70,10 +78,21 @@ /** * Specifies the HTTP status code associated to the annotated - * {@link Throwable}. + * {@link Throwable}. Default is 500. * * @return The result HTTP status code. */ - String value() default ""; + int value() default 500; + + /** + * Indicates if the properties of the annotated {@link Throwable} + * should be serialized in the HTTP response. + * The properties of the class {@link Throwable} + * are not serialized. + * + * @return True if properties of the annotated {@link Throwable} + * should be serialized in the HTTP response. + */ + boolean serializeProperties() default false; } diff --git a/modules/org.restlet/src/org/restlet/service/StatusService.java b/modules/org.restlet/src/org/restlet/service/StatusService.java index 56c7af938c..8d63c34d23 100644 --- a/modules/org.restlet/src/org/restlet/service/StatusService.java +++ b/modules/org.restlet/src/org/restlet/service/StatusService.java @@ -36,12 +36,23 @@ import org.restlet.Context; import org.restlet.Request; import org.restlet.Response; +import org.restlet.data.MediaType; import org.restlet.data.Reference; import org.restlet.data.Status; +import org.restlet.engine.converter.ConverterUtils; +import org.restlet.engine.resource.VariantInfo; +import org.restlet.engine.util.ThrowableSerializer; import org.restlet.representation.Representation; +import org.restlet.representation.Variant; import org.restlet.resource.Resource; import org.restlet.resource.ResourceException; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.logging.Level; + /** * Service to handle error statuses. If an exception is thrown within your * application or Restlet code, it will be intercepted by this service if it is @@ -68,6 +79,10 @@ * @author Jerome Louvel */ public class StatusService extends Service { + + /** HTML Variant */ + private static final VariantInfo VARIANT_HTML = new VariantInfo(MediaType.TEXT_HTML); + /** The email address to contact in case of error. */ private volatile String contactEmail; @@ -134,14 +149,16 @@ public Reference getHomeRef() { * @param response * The response updated. * @return The representation of the given status. - * @deprecated Use {@link #toRepresentation(Status, Request, Response)} + * @deprecated Use {@link #toRepresentation(Status, Throwable, Request, + * Response, ConverterService, ConnegService, MetadataService)} * instead. */ @Deprecated public Representation getRepresentation(Status status, Request request, Response response) { // [ifndef gwt] instruction - return toRepresentation(status, null, request, response, null); + return toRepresentation(status, null, + request, response, null, null, null); // [ifdef gwt] instruction uncomment // return toRepresentation(status, null, request, response); } @@ -231,7 +248,8 @@ public void setOverwriting(boolean overwriting) { * Returns a representation for the given status.
* In order to customize the default representation, this method can be * overridden. By default it invokes - * {@link #toRepresentation(Status, Request, Response)}. + * {@link #toRepresentation(Status, Throwable, Request, Response, + * ConverterService, ConnegService, MetadataService)} * * @param status * The status to represent. @@ -245,7 +263,8 @@ public Representation toRepresentation(Status status, Throwable throwable, Resource resource) { // [ifndef gwt] instruction return toRepresentation(status, throwable, resource.getRequest(), - resource.getResponse(), resource.getConverterService()); + resource.getResponse(), resource.getConverterService(), + resource.getConnegService(), resource.getMetadataService()); // [ifdef gwt] instruction uncomment // return null; } @@ -255,7 +274,7 @@ public Representation toRepresentation(Status status, Throwable throwable, * Returns a representation for the given status. In order to customize the * default representation, this method can be overridden. It returns null by * default. - * + * * @param status * The status to represent. * @param throwable @@ -267,11 +286,87 @@ public Representation toRepresentation(Status status, Throwable throwable, * @param converterService * The converter service. * @return The representation of the given status. + * + * @deprecated Use {@link #toRepresentation(org.restlet.data.Status, Throwable, org.restlet.Request, org.restlet.Response, ConverterService, ConnegService, MetadataService)} instead. */ public Representation toRepresentation(Status status, Throwable throwable, Request request, Response response, ConverterService converterService) { - return null; + return toRepresentation(status, throwable, + request, response, converterService, null, null); + } + + /** + * Returns a representation for the given status. In order to customize the + * default representation, this method can be overridden. It returns a + * {@link org.restlet.data.Status} representation by default or a {@link java.lang.Throwable} + * representation if the throwable is annotated with {@link org.restlet.resource.Status}. + * + * @param status + * The status to represent. + * @param throwable + * The exception or error caught. If null, use {@link org.restlet.data.Status#getThrowable()}. + * @param request + * The request handled. + * @param response + * The response updated. + * @param converterService + * The converter service. + * @param connegService + * The content negotiation service. + * @param metadataService + * The metadata service. + * @return The representation of the given status. + */ + public Representation toRepresentation(Status status, Throwable throwable, + Request request, Response response, + ConverterService converterService, + ConnegService connegService, + MetadataService metadataService) { + Representation result = null; + + //do content negotiation for status + if (converterService != null && connegService != null && metadataService != null) { + Map representationMap = null; + + //serialize exception if any and if {@link org.restlet.resource.Status} annotation ask for it + // [ifndef gwt] + Throwable cause = throwable != null ? throwable : status.getThrowable(); + if (cause != null) { + org.restlet.engine.resource.StatusAnnotationInfo sai = org.restlet.engine.resource.AnnotationUtils + .getInstance() + .getStatusAnnotationInfo(cause.getClass()); + if (sai != null && sai.isSerializeProperties()) { + try { + representationMap = ThrowableSerializer.serializeToMap(cause); + } catch (Exception e) { + Context.getCurrentLogger().log( + Level.WARNING, "Could not serialize throwable class " + cause.getClass(), e); + } + } + } + // [enddef] + + //default representation match with the status properties + if (representationMap == null) { + representationMap = new LinkedHashMap(); + representationMap.put("code", status.getCode()); + representationMap.put("reasonPhrase", status.getReasonPhrase()); + representationMap.put("description", status.getDescription()); + } + + List variants = ConverterUtils.getVariants(representationMap.getClass(), null); + if (!variants.contains(VARIANT_HTML)) { + variants.add(VARIANT_HTML); + } + Variant variant = connegService.getPreferredVariant(variants, request, metadataService); + try { + result = converterService.toRepresentation(representationMap, variant); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return result; } // [ifdef gwt] method uncomment From 4368bad00a637c101c2a9c6cd1022dfc5c774af5 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Sun, 26 Oct 2014 19:38:32 +0100 Subject: [PATCH 2/6] Remove code which is already done in StatusFilter.java --- .../src/org/restlet/resource/ServerResource.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modules/org.restlet/src/org/restlet/resource/ServerResource.java b/modules/org.restlet/src/org/restlet/resource/ServerResource.java index 963b6ead8f..d1cee60736 100644 --- a/modules/org.restlet/src/org/restlet/resource/ServerResource.java +++ b/modules/org.restlet/src/org/restlet/resource/ServerResource.java @@ -270,12 +270,6 @@ protected void doCatch(Throwable throwable) { if (getResponse() != null) { getResponse().setStatus(status); - - if (getResponseEntity() == null) { - getResponse().setEntity( - getStatusService().toRepresentation(status, throwable, - this)); - } } } From fb50102609101c4ef0145f0aa5ca9e43f52c9eb5 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Mon, 27 Oct 2014 10:57:18 +0100 Subject: [PATCH 3/6] Extract BeanInfoutils from ThrowableSerializer --- .../restlet/engine/util/BeanInfoUtils.java | 52 +++++++++++++++++++ .../engine/util/ThrowableSerializer.java | 36 +------------ 2 files changed, 53 insertions(+), 35 deletions(-) create mode 100644 modules/org.restlet/src/org/restlet/engine/util/BeanInfoUtils.java diff --git a/modules/org.restlet/src/org/restlet/engine/util/BeanInfoUtils.java b/modules/org.restlet/src/org/restlet/engine/util/BeanInfoUtils.java new file mode 100644 index 0000000000..716bdb5333 --- /dev/null +++ b/modules/org.restlet/src/org/restlet/engine/util/BeanInfoUtils.java @@ -0,0 +1,52 @@ +package org.restlet.engine.util; + +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.Introspector; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * Utilities to get the {@link BeanInfo} of a class. + * + * @author Manuel Boillod + */ +public class BeanInfoUtils { + + /** BeanInfo cache. */ + private static final ConcurrentMap, BeanInfo> cache = new ConcurrentHashMap, BeanInfo>(); + + /** + * Get a BeanInfo from cache or create it. + * Stop introspection to {@link Object} or {@link Throwable} if the class + * is a subtype of {@link Throwable} + * + * @param clazz + * The class + * @return BeanInfo of the class + */ + public static BeanInfo getBeanInfo(Class clazz) { + BeanInfo result = cache.get(clazz); + + if (result == null) { + // Inspect the class itself for annotations + + Class stopClass = Throwable.class.isAssignableFrom(clazz) ? Throwable.class : Object.class; + try { + result = Introspector.getBeanInfo(clazz, stopClass, Introspector.IGNORE_ALL_BEANINFO); + } catch (IntrospectionException e) { + throw new RuntimeException("Could not get BeanInfo of class " + clazz.getName(), e); + } + + // Put the list in the cache if no one was previously present + BeanInfo prev = cache.putIfAbsent(clazz, result); + + if (prev != null) { + // Reuse the previous entry + result = prev; + } + } + + return result; + } +} diff --git a/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java b/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java index 3ee58a6626..dfd1f425e0 100644 --- a/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java +++ b/modules/org.restlet/src/org/restlet/engine/util/ThrowableSerializer.java @@ -34,13 +34,9 @@ package org.restlet.engine.util; import java.beans.BeanInfo; -import java.beans.IntrospectionException; -import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; /** * Utilities to serialize {@link Throwable}. @@ -49,9 +45,6 @@ */ public class ThrowableSerializer { - /** BeanInfo cache. */ - private static final ConcurrentMap, BeanInfo> cache = new ConcurrentHashMap, BeanInfo>(); - /** * Serialize {@link Throwable} properties to a Map using reflection. * The properties of {@link Throwable} class are ignored except if @@ -64,7 +57,7 @@ public class ThrowableSerializer { */ public static Map serializeToMap(Throwable throwable) { try { - BeanInfo beanInfo = getBeanInfo(throwable); + BeanInfo beanInfo = BeanInfoUtils.getBeanInfo(throwable.getClass()); Map properties = new HashMap(); PropertyDescriptor[] propertyDescriptors = beanInfo.getPropertyDescriptors(); @@ -78,31 +71,4 @@ public static Map serializeToMap(Throwable throwable) { throw new RuntimeException("Could not serialize properties of class " + throwable.getCause(), e); } } - - /** - * Get a BeanInfo from cache or create it. - * @param throwable - * Throwable instance - * @return BeanInfo of throwable class - */ - private static BeanInfo getBeanInfo(Throwable throwable) throws IntrospectionException { - BeanInfo result = cache.get(throwable.getClass()); - - if (result == null) { - // Inspect the class itself for annotations - result = Introspector.getBeanInfo(throwable.getClass(), Throwable.class, Introspector.IGNORE_ALL_BEANINFO); - - // Put the list in the cache if no one was previously present - BeanInfo prev = cache.putIfAbsent(throwable.getClass(), result); - - if (prev != null) { - // Reuse the previous entry - result = prev; - } - } - - return result; - } - - ; } From 507319bb956e5f4863dd668e973723290b333df0 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Mon, 27 Oct 2014 11:06:22 +0100 Subject: [PATCH 4/6] Use StatusRepresentation instead of a map (better for documentation) --- .../representation/StatusRepresentation.java | 49 +++++++++++++++++++ .../org/restlet/service/StatusService.java | 16 +++--- 2 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 modules/org.restlet/src/org/restlet/representation/StatusRepresentation.java diff --git a/modules/org.restlet/src/org/restlet/representation/StatusRepresentation.java b/modules/org.restlet/src/org/restlet/representation/StatusRepresentation.java new file mode 100644 index 0000000000..8d28702f57 --- /dev/null +++ b/modules/org.restlet/src/org/restlet/representation/StatusRepresentation.java @@ -0,0 +1,49 @@ +package org.restlet.representation; + +import org.restlet.data.Status; + +/** + * + * Representation of a {@link Status} + * + * @author Manuel Boillod + */ +public class StatusRepresentation { + int code; + String reasonPhrase; + String description; + + public StatusRepresentation() { + } + + public StatusRepresentation(Status status) { + this.code = status.getCode(); + this.reasonPhrase = status.getReasonPhrase(); + this.description = status.getDescription(); + + } + + public int getCode() { + return code; + } + + public void setCode(int code) { + this.code = code; + } + + public String getReasonPhrase() { + return reasonPhrase; + } + + public void setReasonPhrase(String reasonPhrase) { + this.reasonPhrase = reasonPhrase; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } +} \ No newline at end of file diff --git a/modules/org.restlet/src/org/restlet/service/StatusService.java b/modules/org.restlet/src/org/restlet/service/StatusService.java index 8d63c34d23..86a007eb9d 100644 --- a/modules/org.restlet/src/org/restlet/service/StatusService.java +++ b/modules/org.restlet/src/org/restlet/service/StatusService.java @@ -43,6 +43,7 @@ import org.restlet.engine.resource.VariantInfo; import org.restlet.engine.util.ThrowableSerializer; import org.restlet.representation.Representation; +import org.restlet.representation.StatusRepresentation; import org.restlet.representation.Variant; import org.restlet.resource.Resource; import org.restlet.resource.ResourceException; @@ -327,7 +328,7 @@ public Representation toRepresentation(Status status, Throwable throwable, //do content negotiation for status if (converterService != null && connegService != null && metadataService != null) { - Map representationMap = null; + Object representationObject = null; //serialize exception if any and if {@link org.restlet.resource.Status} annotation ask for it // [ifndef gwt] @@ -338,7 +339,7 @@ public Representation toRepresentation(Status status, Throwable throwable, .getStatusAnnotationInfo(cause.getClass()); if (sai != null && sai.isSerializeProperties()) { try { - representationMap = ThrowableSerializer.serializeToMap(cause); + representationObject = ThrowableSerializer.serializeToMap(cause); } catch (Exception e) { Context.getCurrentLogger().log( Level.WARNING, "Could not serialize throwable class " + cause.getClass(), e); @@ -348,20 +349,17 @@ public Representation toRepresentation(Status status, Throwable throwable, // [enddef] //default representation match with the status properties - if (representationMap == null) { - representationMap = new LinkedHashMap(); - representationMap.put("code", status.getCode()); - representationMap.put("reasonPhrase", status.getReasonPhrase()); - representationMap.put("description", status.getDescription()); + if (representationObject == null) { + representationObject = new StatusRepresentation(status); } - List variants = ConverterUtils.getVariants(representationMap.getClass(), null); + List variants = ConverterUtils.getVariants(representationObject.getClass(), null); if (!variants.contains(VARIANT_HTML)) { variants.add(VARIANT_HTML); } Variant variant = connegService.getPreferredVariant(variants, request, metadataService); try { - result = converterService.toRepresentation(representationMap, variant); + result = converterService.toRepresentation(representationObject, variant); } catch (IOException e) { throw new RuntimeException(e); } From 5eb53799f45b64ae5b9603eb2b131fc942ee3d66 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Mon, 27 Oct 2014 13:36:58 +0100 Subject: [PATCH 5/6] Verify response entity in tests --- .../resource/AnnotatedResource20TestCase.java | 31 ++++++++++++++++--- .../restlet/test/resource/MyResource20.java | 3 +- .../test/resource/MyServerResource20.java | 4 +-- .../org/restlet/service/StatusService.java | 2 -- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java b/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java index 41f742a575..c76a52258d 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/AnnotatedResource20TestCase.java @@ -34,11 +34,15 @@ package org.restlet.test.resource; import java.io.IOException; +import java.util.Map; +import org.restlet.Application; import org.restlet.engine.Engine; import org.restlet.ext.jackson.JacksonConverter; +import org.restlet.ext.jackson.JacksonRepresentation; +import org.restlet.representation.Representation; +import org.restlet.representation.StatusRepresentation; import org.restlet.resource.ClientResource; -import org.restlet.resource.Finder; import org.restlet.resource.ResourceException; import org.restlet.test.RestletTestCase; @@ -59,11 +63,13 @@ protected void setUp() throws Exception { Engine.getInstance().getRegisteredConverters() .add(new JacksonConverter()); Engine.getInstance().registerDefaultConverters(); - Finder finder = new Finder(); - finder.setTargetClass(MyServerResource20.class); + + //Use Application because we need StatusService, ConverterService, ... + Application application = new Application(); + application.setInboundRoot(MyServerResource20.class); this.clientResource = new ClientResource("http://local"); - this.clientResource.setNext(finder); + this.clientResource.setNext(application); this.myResource = clientResource.wrap(MyResource20.class); } @@ -82,6 +88,15 @@ public void testGet() throws IOException, ResourceException { fail("exception should be catch by client resource"); } catch (ResourceException e) { assertEquals(400, e.getStatus().getCode()); + Representation responseEntity = clientResource.getResponseEntity(); + if (responseEntity instanceof JacksonRepresentation) { + assertTrue(JacksonRepresentation.class.isAssignableFrom(responseEntity.getClass())); + JacksonRepresentation jacksonRepresentation = (JacksonRepresentation) responseEntity; + Object entity = jacksonRepresentation.getObject(); + assertTrue(StatusRepresentation.class.isAssignableFrom(entity.getClass())); + StatusRepresentation statusRepresentation = (StatusRepresentation) entity; + assertEquals(400, statusRepresentation.getCode()); + } } } @@ -94,7 +109,13 @@ public void testGetAndSerializeException() throws IOException, ResourceException fail("exception should be catch by client resource"); } catch (ResourceException e) { assertEquals(400, e.getStatus().getCode()); - //TODO How to retrieve the response entity with the error representation ? + Representation responseEntity = clientResource.getResponseEntity(); + assertTrue(JacksonRepresentation.class.isAssignableFrom(responseEntity.getClass())); + JacksonRepresentation jacksonRepresentation = (JacksonRepresentation) responseEntity; + Object entity = jacksonRepresentation.getObject(); + assertTrue(Map.class.isAssignableFrom(entity.getClass())); + Map map = (Map) entity; + assertEquals("my custom error", map.get("customProperty")); } } } diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java index 1488a8027a..dee35b114a 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyResource20.java @@ -34,6 +34,7 @@ package org.restlet.test.resource; import org.restlet.resource.Get; +import org.restlet.resource.Put; /** * Sample annotated interface. @@ -45,7 +46,7 @@ public interface MyResource20 { @Get MyBean represent() throws MyException01; - @Get + @Put MyBean representAndSerializeException() throws MyException02; } diff --git a/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java b/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java index 6a434d3522..822fc592ad 100644 --- a/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java +++ b/modules/org.restlet.test/src/org/restlet/test/resource/MyServerResource20.java @@ -52,8 +52,6 @@ public static void main(String[] args) throws Exception { server.start(); } - private volatile MyBean myBean = new MyBean("myName", "myDescription"); - @SuppressWarnings("unused") public MyBean represent() throws MyException01 { throw new MyException01(new Date()); @@ -61,7 +59,7 @@ public MyBean represent() throws MyException01 { @Override public MyBean representAndSerializeException() throws MyException02 { - throw new MyException02("representAndSerializeException error"); + throw new MyException02("my custom error"); } } diff --git a/modules/org.restlet/src/org/restlet/service/StatusService.java b/modules/org.restlet/src/org/restlet/service/StatusService.java index 86a007eb9d..1e83dbdfb3 100644 --- a/modules/org.restlet/src/org/restlet/service/StatusService.java +++ b/modules/org.restlet/src/org/restlet/service/StatusService.java @@ -49,9 +49,7 @@ import org.restlet.resource.ResourceException; import java.io.IOException; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.logging.Level; /** From dfb6e76b82760b20cbf215cebebb41dfcdcf4ee6 Mon Sep 17 00:00:00 2001 From: Manuel Boillod Date: Mon, 27 Oct 2014 13:40:24 +0100 Subject: [PATCH 6/6] Fix GWT Edition WARNING: StatusService has been removed from GWT Edition --- modules/org.restlet/module.xml | 4 ++ .../src/org/restlet/data/Status.java | 3 +- .../src/org/restlet/resource/Resource.java | 10 ++--- .../org/restlet/service/StatusService.java | 39 +------------------ 4 files changed, 11 insertions(+), 45 deletions(-) diff --git a/modules/org.restlet/module.xml b/modules/org.restlet/module.xml index 9fa01441da..57b7bb4480 100644 --- a/modules/org.restlet/module.xml +++ b/modules/org.restlet/module.xml @@ -84,6 +84,7 @@ + @@ -118,6 +119,7 @@ + @@ -134,6 +136,7 @@ + @@ -164,6 +167,7 @@ + diff --git a/modules/org.restlet/src/org/restlet/data/Status.java b/modules/org.restlet/src/org/restlet/data/Status.java index 5576149fb3..e7becd7d8a 100644 --- a/modules/org.restlet/src/org/restlet/data/Status.java +++ b/modules/org.restlet/src/org/restlet/data/Status.java @@ -35,7 +35,6 @@ import org.restlet.engine.Edition; import org.restlet.engine.Engine; -import org.restlet.service.StatusService; /** * Status to return after handling a call. @@ -1092,7 +1091,7 @@ public int getCode() { /** * Returns the description. This value is typically used by the - * {@link StatusService} to build a meaningful description of an error via a + * {@link org.restlet.service.StatusService} to build a meaningful description of an error via a * response entity. * * @return The description. diff --git a/modules/org.restlet/src/org/restlet/resource/Resource.java b/modules/org.restlet/src/org/restlet/resource/Resource.java index 5cfe7f4157..78475c3008 100644 --- a/modules/org.restlet/src/org/restlet/resource/Resource.java +++ b/modules/org.restlet/src/org/restlet/resource/Resource.java @@ -63,7 +63,6 @@ import org.restlet.representation.Representation; import org.restlet.representation.Variant; import org.restlet.service.MetadataService; -import org.restlet.service.StatusService; import org.restlet.util.Series; /** @@ -743,19 +742,20 @@ public Status getStatus() { return getResponse() == null ? null : getResponse().getStatus(); } + // [ifndef gwt] method /** * Returns the application's status service or create a new one. - * + * * @return The status service. */ - public StatusService getStatusService() { - StatusService result = null; + public org.restlet.service.StatusService getStatusService() { + org.restlet.service.StatusService result = null; // [ifndef gwt] instruction result = getApplication().getStatusService(); if (result == null) { - result = new StatusService(); + result = new org.restlet.service.StatusService(); } return result; diff --git a/modules/org.restlet/src/org/restlet/service/StatusService.java b/modules/org.restlet/src/org/restlet/service/StatusService.java index 1e83dbdfb3..a8efa79316 100644 --- a/modules/org.restlet/src/org/restlet/service/StatusService.java +++ b/modules/org.restlet/src/org/restlet/service/StatusService.java @@ -111,7 +111,6 @@ public StatusService(boolean enabled) { this.overwriting = false; } - // [ifndef gwt] method @Override public org.restlet.routing.Filter createInboundFilter(Context context) { return new org.restlet.engine.application.StatusFilter(context, this); @@ -155,11 +154,8 @@ public Reference getHomeRef() { @Deprecated public Representation getRepresentation(Status status, Request request, Response response) { - // [ifndef gwt] instruction return toRepresentation(status, null, request, response, null, null, null); - // [ifdef gwt] instruction uncomment - // return toRepresentation(status, null, request, response); } /** @@ -260,15 +256,11 @@ public void setOverwriting(boolean overwriting) { */ public Representation toRepresentation(Status status, Throwable throwable, Resource resource) { - // [ifndef gwt] instruction return toRepresentation(status, throwable, resource.getRequest(), resource.getResponse(), resource.getConverterService(), resource.getConnegService(), resource.getMetadataService()); - // [ifdef gwt] instruction uncomment - // return null; } - // [ifndef gwt] method /** * Returns a representation for the given status. In order to customize the * default representation, this method can be overridden. It returns null by @@ -329,7 +321,6 @@ public Representation toRepresentation(Status status, Throwable throwable, Object representationObject = null; //serialize exception if any and if {@link org.restlet.resource.Status} annotation ask for it - // [ifndef gwt] Throwable cause = throwable != null ? throwable : status.getThrowable(); if (cause != null) { org.restlet.engine.resource.StatusAnnotationInfo sai = org.restlet.engine.resource.AnnotationUtils @@ -344,7 +335,6 @@ public Representation toRepresentation(Status status, Throwable throwable, } } } - // [enddef] //default representation match with the status properties if (representationObject == null) { @@ -362,33 +352,10 @@ public Representation toRepresentation(Status status, Throwable throwable, throw new RuntimeException(e); } } + // [enddef] return result; } - // [ifdef gwt] method uncomment - // /** - // * Returns a representation for the given status. In order to customize - // the - // * default representation, this method can be overridden. It returns null - // by - // * default. - // * - // * @param status - // * The status to represent. - // * @param throwable - // * The exception or error caught. - // * @param request - // * The request handled. - // * @param response - // * The response updated. - // * @return The representation of the given status. - // */ - // public Representation toRepresentation(Status status, Throwable - // throwable, - // Request request, Response response) { - // return null; - // } - /** * Returns a status for a given exception or error. By default it unwraps * the status of {@link ResourceException}. For other exceptions or errors, @@ -419,7 +386,6 @@ public Status toStatus(Throwable throwable, Request request, result = re.getStatus(); } } else { - // [ifndef gwt] org.restlet.engine.resource.StatusAnnotationInfo sai = org.restlet.engine.resource.AnnotationUtils .getInstance() .getStatusAnnotationInfo(throwable.getClass()); @@ -429,9 +395,6 @@ public Status toStatus(Throwable throwable, Request request, } else { result = new Status(Status.SERVER_ERROR_INTERNAL, throwable); } - // [enddef] - // [ifdef gwt] instruction uncomment - // result = new Status(Status.SERVER_ERROR_INTERNAL, throwable); } return result;