From 79884d7c2607354a642f506b68a6a7d0e849c947 Mon Sep 17 00:00:00 2001 From: Michael Bolz Date: Wed, 16 Sep 2015 14:23:01 +0200 Subject: [PATCH 1/3] [OLINGO-775] Update Maven invoker plugin to newest version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 62ad9e3ec..745042c1a 100644 --- a/pom.xml +++ b/pom.xml @@ -363,7 +363,7 @@ org.apache.maven.plugins maven-invoker-plugin - 1.8 + 2.0.0 org.apache.maven.plugins From fbca4ef33e8c7dd151b81dced0fd24d3800aabd2 Mon Sep 17 00:00:00 2001 From: Klaus Straubinger Date: Wed, 16 Sep 2015 09:53:34 +0200 Subject: [PATCH 2/3] [OLINGO-659] more robust Accept and Content-Type handling Change-Id: I34e25df981650a2155729a6cf92806dbe36546b3 Signed-off-by: Michael Bolz --- .../olingo/commons/api/format/AcceptType.java | 11 ++- .../commons/api/format/AcceptTypeTest.java | 99 +++++++++++-------- .../commons/api/format/ContentTypeTest.java | 76 +++++++------- .../olingo/server/core/ContentNegotiator.java | 24 +++-- .../core/ContentNegotiatorException.java | 4 +- .../olingo/server/core/ODataDispatcher.java | 41 ++++---- .../server/core/ODataExceptionHelper.java | 3 +- .../server/core/ODataHandlerException.java | 2 + .../server-core-exceptions-i18n.properties | 4 +- .../server/core/ContentNegotiatorTest.java | 15 ++- .../olingo/server/core/ODataHandlerTest.java | 9 ++ 11 files changed, 153 insertions(+), 135 deletions(-) diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java index c1e50a782..f8640080c 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java @@ -24,6 +24,7 @@ import java.util.Comparator; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.regex.Pattern; /** * Internally used {@link AcceptType} for OData library. @@ -45,6 +46,8 @@ import java.util.Map; */ public final class AcceptType { + private static final Pattern Q_PATTERN = Pattern.compile("\\A(?:0(?:\\.\\d{0,3})?)|(?:1(?:\\.0{0,3})?)\\Z"); + private final String type; private final String subtype; private final Map parameters; @@ -77,12 +80,10 @@ public final class AcceptType { final String q = parameters.get(TypeUtil.PARAMETER_Q); if (q == null) { quality = 1F; - } else { - try { + } else if (Q_PATTERN.matcher(q).matches()) { quality = Float.valueOf(q); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Illegal quality parameter.", e); - } + } else { + throw new IllegalArgumentException("Illegal quality parameter '" + q + "'."); } } diff --git a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java index 589fae24d..890297556 100644 --- a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java +++ b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/AcceptTypeTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.List; @@ -31,7 +32,7 @@ import org.junit.Test; public class AcceptTypeTest { @Test - public void testWildcard() { + public void wildcard() { List atl = AcceptType.create("*/*"); assertEquals(1, atl.size()); @@ -42,7 +43,7 @@ public class AcceptTypeTest { } @Test - public void testWildcardSubtype() { + public void wildcardSubtype() { List atl = AcceptType.create("a/*"); assertEquals(1, atl.size()); @@ -53,7 +54,7 @@ public class AcceptTypeTest { } @Test - public void testSingleAcceptType() { + public void singleAcceptType() { assertTrue(AcceptType.create("a/a").get(0).matches(ContentType.create("a/a"))); assertTrue(AcceptType.create("a/a;q=0.2").get(0).matches(ContentType.create("a/a"))); assertFalse(AcceptType.create("a/a;x=y;q=0.2").get(0).matches(ContentType.create("a/a"))); @@ -64,7 +65,7 @@ public class AcceptTypeTest { } @Test - public void testAcceptTypes() { + public void acceptTypes() { List atl; atl = AcceptType.create("b/b,*/*,a/a,c/*"); @@ -101,53 +102,73 @@ public class AcceptTypeTest { } @Test - public void testWithQParameter() { - List atl = AcceptType.create("application/json;q=0.2"); + public void withQParameter() { + List acceptTypes = AcceptType.create("application/json;q=0.2"); - assertEquals(1, atl.size()); - assertEquals("application", atl.get(0).getType()); - assertEquals("json", atl.get(0).getSubtype()); - assertEquals("0.2", atl.get(0).getParameters().get("q")); - assertEquals("application/json;q=0.2", atl.get(0).toString()); + assertEquals(1, acceptTypes.size()); + final AcceptType acceptType = acceptTypes.get(0); + assertEquals("application", acceptType.getType()); + assertEquals("json", acceptType.getSubtype()); + assertEquals("0.2", acceptType.getParameters().get(TypeUtil.PARAMETER_Q)); + assertEquals("0.2", acceptType.getParameter(TypeUtil.PARAMETER_Q)); + assertEquals(Float.valueOf(0.2F), acceptType.getQuality()); + assertEquals("application/json;q=0.2", acceptType.toString()); } - @Test(expected = IllegalArgumentException.class) + @Test + public void formatErrors() { + expectCreateError("/"); + expectCreateError("//"); + expectCreateError("///"); + expectCreateError("a/b/c"); + expectCreateError("a//b"); + } + + @Test public void abbreviationsNotAllowed() { - AcceptType.create("application"); + expectCreateError("application"); } - @Test(expected = IllegalArgumentException.class) - public void testWrongQParameter() { - AcceptType.create(" a/a;q=z "); + @Test + public void wildcardError() { + expectCreateError("*/json"); } - @Test(expected = IllegalArgumentException.class) - public void incompleteParameter() { - AcceptType.create("a/b;parameter"); + @Test + public void wrongQParameter() { + expectCreateError(" a/a;q=z "); + expectCreateError("a/a;q=42"); + expectCreateError("a/a;q=0.0001"); } - @Test(expected = IllegalArgumentException.class) - public void missingParameterValue() { - AcceptType.create("a/b;parameter="); + @Test + public void parameterErrors() { + expectCreateError("a/b;parameter"); + expectCreateError("a/b;parameter="); + expectCreateError("a/b;name= value"); + expectCreateError("a/b;=value"); + expectCreateError("a/b;the name=value"); } - @Test(expected = IllegalArgumentException.class) - public void parameterValueStartingWithWhitespace() { - AcceptType.create("a/b;name= value"); - } - - @Test(expected = IllegalArgumentException.class) - public void missingParameterName() { - AcceptType.create("a/b;=value"); - } - - @Test(expected = IllegalArgumentException.class) - public void parameterNameWithWhitespace() { - AcceptType.create("a/b;the name=value"); - } - - @Test(expected = IllegalArgumentException.class) + @Test public void trailingSemicolon() { - AcceptType.create("a/b;"); + expectCreateError("a/b;"); + } + + @Test + public void fromContentType() { + final List acceptType = AcceptType.fromContentType(ContentType.APPLICATION_JSON); + assertNotNull(acceptType); + assertEquals(1, acceptType.size()); + assertEquals(ContentType.APPLICATION_JSON.toContentTypeString(), acceptType.get(0).toString()); + } + + private void expectCreateError(final String value) { + try { + AcceptType.create(value); + fail("Expected exception not thrown."); + } catch (final IllegalArgumentException e) { + assertNotNull(e); + } } } diff --git a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java index 753169eb5..65750e1b7 100644 --- a/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java +++ b/lib/commons-api/src/test/java/org/apache/olingo/commons/api/format/ContentTypeTest.java @@ -21,8 +21,10 @@ package org.apache.olingo.commons.api.format; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.Test; @@ -36,44 +38,25 @@ public class ContentTypeTest { assertEquals(ContentType.create("A/B"), ContentType.create("a/b")); } - @Test(expected = IllegalArgumentException.class) - public void createFail1() { - ContentType.create("a"); - } + @Test + public void createFail() { + createWrong("a"); + createWrong(" a / b "); + createWrong("a/b;"); + createWrong("a/b;parameter"); + createWrong("a/b;parameter="); + createWrong("a/b;=value"); + createWrong("a/b;the name=value"); + createWrong("a/b;name= value"); - @Test(expected = IllegalArgumentException.class) - public void createFail2() { - ContentType.create(" a / b "); - } + createWrong("*/*"); + createWrong("*"); + createWrong("a//b"); + createWrong("///"); + createWrong("a/*"); + createWrong("*/b"); - @Test(expected = IllegalArgumentException.class) - public void createFail3() { - ContentType.create("a/b;"); - } - - @Test(expected = IllegalArgumentException.class) - public void createFail4() { - ContentType.create("a/b;parameter"); - } - - @Test(expected = IllegalArgumentException.class) - public void createFail5() { - ContentType.create("a/b;parameter="); - } - - @Test(expected = IllegalArgumentException.class) - public void createFail6() { - ContentType.create("a/b;=value"); - } - - @Test(expected = IllegalArgumentException.class) - public void createFail7() { - ContentType.create("a/b;the name=value"); - } - - @Test(expected = IllegalArgumentException.class) - public void createFail8() { - ContentType.create("a/b;name= value"); + createWrong(null); } @Test @@ -99,17 +82,14 @@ public class ContentTypeTest { @Test public void parse() { + assertEquals(ContentType.APPLICATION_OCTET_STREAM, ContentType.parse("application/octet-stream")); + assertNull(ContentType.parse("a")); assertNull(ContentType.parse("a/b;c")); assertNull(ContentType.parse("a/b;c=")); assertNull(ContentType.parse("a/b;c= ")); } - @Test(expected = IllegalArgumentException.class) - public void wildcardFail() { - ContentType.create("*/*"); - } - @Test public void charsetUtf8() { ContentType ct1 = ContentType.create("a/b;charset=utf8"); @@ -118,8 +98,9 @@ public class ContentTypeTest { assertNotEquals(ct1, ct2); assertEquals(ct1.getType(), ct2.getType()); assertEquals(ct1.getSubtype(), ct2.getSubtype()); - assertEquals("utf8", ct1.getParameters().get("charset")); - assertEquals("utf-8", ct2.getParameters().get("charset")); + assertEquals("utf8", ct1.getParameters().get(ContentType.PARAMETER_CHARSET)); + assertEquals("utf-8", ct2.getParameters().get(ContentType.PARAMETER_CHARSET)); + assertEquals("utf-8", ct2.getParameter(ContentType.PARAMETER_CHARSET)); assertTrue(ct1.isCompatible(ct2)); } @@ -130,4 +111,13 @@ public class ContentTypeTest { ContentType.create(ContentType.create(ContentType.APPLICATION_JSON, "a", "b"), "c", "d") .toContentTypeString()); } + + private void createWrong(final String value) { + try { + ContentType.create(value); + fail("Expected exception not thrown."); + } catch (final IllegalArgumentException e) { + assertNotNull(e); + } + } } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java index 8da326dd8..e47d864ee 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiator.java @@ -37,12 +37,12 @@ public final class ContentNegotiator { private static final String XML = "xml"; private static final List DEFAULT_SUPPORTED_CONTENT_TYPES = - Collections.unmodifiableList(Arrays.asList( - ContentType.JSON, - ContentType.JSON_NO_METADATA, - ContentType.APPLICATION_JSON, - ContentType.APPLICATION_ATOM_XML, - ContentType.APPLICATION_XML)); + Collections.unmodifiableList(Arrays.asList( + ContentType.JSON, + ContentType.JSON_NO_METADATA, + ContentType.APPLICATION_JSON, + ContentType.APPLICATION_ATOM_XML, + ContentType.APPLICATION_XML)); private ContentNegotiator() {} @@ -102,17 +102,15 @@ public final class ContentNegotiator { ContentNegotiatorException.MessageKeys.UNSUPPORTED_FORMAT_OPTION, formatString); } } else if (acceptHeaderValue != null) { - final List acceptedContentTypes = AcceptType.create(acceptHeaderValue); try { - result = getAcceptedType(acceptedContentTypes, supportedContentTypes); + result = getAcceptedType(AcceptType.create(acceptHeaderValue), supportedContentTypes); } catch (final IllegalArgumentException e) { - throw new ContentNegotiatorException("charset in accept header not supported: " + acceptHeaderValue, e, - ContentNegotiatorException.MessageKeys.WRONG_CHARSET_IN_HEADER, HttpHeader.ACCEPT, acceptHeaderValue); + result = null; } if (result == null) { throw new ContentNegotiatorException( - "unsupported accept content type: " + acceptedContentTypes + " != " + supportedContentTypes, - ContentNegotiatorException.MessageKeys.UNSUPPORTED_CONTENT_TYPES, acceptedContentTypes.toString()); + "Unsupported or illegal Accept header value: " + acceptHeaderValue + " != " + supportedContentTypes, + ContentNegotiatorException.MessageKeys.UNSUPPORTED_ACCEPT_TYPES, acceptHeaderValue); } } else { final ContentType requestedContentType = getDefaultSupportedContentTypes(representationType).get(0); @@ -127,7 +125,7 @@ public final class ContentNegotiator { return result; } - private static ContentType mapContentType(String formatString) { + private static ContentType mapContentType(final String formatString) { return JSON.equalsIgnoreCase(formatString) ? ContentType.JSON : XML.equalsIgnoreCase(formatString) ? ContentType.APPLICATION_XML : ATOM.equalsIgnoreCase(formatString) ? ContentType.APPLICATION_ATOM_XML : null; diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiatorException.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiatorException.java index 5ecb1189c..a76c5495b 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiatorException.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ContentNegotiatorException.java @@ -24,10 +24,8 @@ public class ContentNegotiatorException extends ODataLibraryException { private static final long serialVersionUID = -8112658467394158700L; public static enum MessageKeys implements MessageKey { - /** parameters: HTTP header name, HTTP header value */ - WRONG_CHARSET_IN_HEADER, /** parameter: list of content-type ranges */ - UNSUPPORTED_CONTENT_TYPES, + UNSUPPORTED_ACCEPT_TYPES, /** parameter: content type */ UNSUPPORTED_CONTENT_TYPE, /** no parameter */ diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataDispatcher.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataDispatcher.java index 90ec8c203..bbe73e073 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataDispatcher.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataDispatcher.java @@ -297,15 +297,10 @@ public class ODataDispatcher { .deleteReference(request, response, uriInfo); } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(httpMethod), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, httpMethod.toString()); + throwMethodNotAllowed(httpMethod); } } - private String getMethodNotAllowedStringMessage(final HttpMethod httpMethod) { - return "HTTP method " + httpMethod + " is not allowed."; - } - private void handleValueDispatching(final ODataRequest request, final ODataResponse response, final int lastPathSegmentIndex) throws ODataApplicationException, ODataLibraryException { final HttpMethod method = request.getMethod(); @@ -337,8 +332,7 @@ public class ODataDispatcher { handler.selectProcessor(PrimitiveValueProcessor.class) .deletePrimitiveValue(request, response, uriInfo); } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } else { if (method == HttpMethod.GET) { @@ -358,8 +352,7 @@ public class ODataDispatcher { handler.selectProcessor(MediaEntityProcessor.class) .deleteMediaEntity(request, response, uriInfo); } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } } @@ -402,8 +395,7 @@ public class ODataDispatcher { .deleteComplex(request, response, uriInfo); } } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } @@ -445,8 +437,7 @@ public class ODataDispatcher { .deletePrimitive(request, response, uriInfo); } } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } @@ -493,8 +484,7 @@ public class ODataDispatcher { .createEntity(request, response, uriInfo, requestFormat, responseFormat); } } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } else { if (method == HttpMethod.GET) { @@ -515,8 +505,7 @@ public class ODataDispatcher { handler.selectProcessor(isMedia ? MediaEntityProcessor.class : EntityProcessor.class) .deleteEntity(request, response, uriInfo); } else { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(method), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, method.toString()); + throwMethodNotAllowed(method); } } } @@ -537,11 +526,15 @@ public class ODataDispatcher { private void checkMethod(final HttpMethod requestMethod, final HttpMethod allowedMethod) throws ODataHandlerException { if (requestMethod != allowedMethod) { - throw new ODataHandlerException(getMethodNotAllowedStringMessage(requestMethod), - ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, requestMethod.toString()); + throwMethodNotAllowed(requestMethod); } } + private void throwMethodNotAllowed(final HttpMethod httpMethod) throws ODataHandlerException { + throw new ODataHandlerException("HTTP method " + httpMethod + " is not allowed.", + ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED, httpMethod.toString()); + } + private ContentType getSupportedContentType(final String contentTypeHeader, final RepresentationType representationType, final boolean mustNotBeNull) throws ODataHandlerException, ContentNegotiatorException { @@ -551,7 +544,13 @@ public class ODataDispatcher { } return null; } - final ContentType contentType = ContentType.parse(contentTypeHeader); + ContentType contentType; + try { + contentType = ContentType.create(contentTypeHeader); + } catch (final IllegalArgumentException e) { + throw new ODataHandlerException("Illegal content type.", e, + ODataHandlerException.MessageKeys.INVALID_CONTENT_TYPE, contentTypeHeader); + } ContentNegotiator.checkSupport(contentType, handler.getCustomContentTypeSupport(), representationType); return contentType; } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataExceptionHelper.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataExceptionHelper.java index cc039221b..2de59d509 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataExceptionHelper.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataExceptionHelper.java @@ -91,7 +91,8 @@ public class ODataExceptionHelper { } else if (ODataHandlerException.MessageKeys.ODATA_VERSION_NOT_SUPPORTED.equals(e.getMessageKey()) || ODataHandlerException.MessageKeys.INVALID_HTTP_METHOD.equals(e.getMessageKey()) || ODataHandlerException.MessageKeys.AMBIGUOUS_XHTTP_METHOD.equals(e.getMessageKey()) - || ODataHandlerException.MessageKeys.MISSING_CONTENT_TYPE.equals(e.getMessageKey())) { + || ODataHandlerException.MessageKeys.MISSING_CONTENT_TYPE.equals(e.getMessageKey()) + || ODataHandlerException.MessageKeys.INVALID_CONTENT_TYPE.equals(e.getMessageKey())) { serverError.setStatusCode(HttpStatusCode.BAD_REQUEST.getStatusCode()); } else if (ODataHandlerException.MessageKeys.HTTP_METHOD_NOT_ALLOWED.equals(e.getMessageKey())) { serverError.setStatusCode(HttpStatusCode.METHOD_NOT_ALLOWED.getStatusCode()); diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandlerException.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandlerException.java index 03c4f9f67..82afe97f2 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandlerException.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandlerException.java @@ -39,6 +39,8 @@ public class ODataHandlerException extends ODataLibraryException { MISSING_CONTENT_TYPE, /** parameter: content type */ UNSUPPORTED_CONTENT_TYPE, + /** parameter: content type */ + INVALID_CONTENT_TYPE, /** parameter: version */ ODATA_VERSION_NOT_SUPPORTED; diff --git a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties index 0a2edd7df..f7c289427 100644 --- a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties +++ b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties @@ -26,6 +26,7 @@ ODataHandlerException.FUNCTIONALITY_NOT_IMPLEMENTED=The requested functionality ODataHandlerException.ODATA_VERSION_NOT_SUPPORTED=OData version '%1$s' is not supported. ODataHandlerException.MISSING_CONTENT_TYPE=The Content-Type HTTP header must be specified for this request. ODataHandlerException.UNSUPPORTED_CONTENT_TYPE=The content type '%1$s' is not supported for this request. +ODataHandlerException.INVALID_CONTENT_TYPE=The content type '%1$s' is not valid. UriParserSyntaxException.MUST_BE_LAST_SEGMENT=The segment '%1$s' must be the last segment. UriParserSyntaxException.UNKNOWN_SYSTEM_QUERY_OPTION=The system query option '%1$s' is not defined. @@ -81,8 +82,7 @@ UriValidationException.SECOND_LAST_SEGMENT_NOT_TYPED=The second last segment '%1 UriValidationException.UNALLOWED_KIND_BEFORE_VALUE=The kind '%1$s' is not allowed before '$value'. UriValidationException.UNALLOWED_KIND_BEFORE_COUNT=The kind '%1$s' is not allowed before '$count'. -ContentNegotiatorException.WRONG_CHARSET_IN_HEADER=The HTTP header '%1$s' with value '%2$s' contains an invalid character-set specification. -ContentNegotiatorException.UNSUPPORTED_CONTENT_TYPES=The content-type range '%1$s' is not supported. +ContentNegotiatorException.UNSUPPORTED_ACCEPT_TYPES=The content-type range '%1$s' is not supported as value of the Accept header. ContentNegotiatorException.UNSUPPORTED_CONTENT_TYPE=The content type '%1$s' is not supported. ContentNegotiatorException.NO_CONTENT_TYPE_SUPPORTED=No content type has been specified as supported. ContentNegotiatorException.UNSUPPORTED_FORMAT_OPTION=The $format option '%1$s' is not supported. diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java index b0a2a09a9..999a20cde 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/ContentNegotiatorTest.java @@ -40,19 +40,18 @@ import org.junit.Test; public class ContentNegotiatorTest { - static final private String ACCEPT_CASE_MIN = "application/json;odata.metadata=minimal"; + static final private String ACCEPT_CASE_MIN = ContentType.JSON.toContentTypeString(); static final private String ACCEPT_CASE_MIN_UTF8 = "application/json;charset=UTF-8;odata.metadata=minimal"; - static final private String ACCEPT_CASE_FULL = "application/json;odata.metadata=full"; - static final private String ACCEPT_CASE_NONE = "application/json;odata.metadata=none"; + static final private String ACCEPT_CASE_FULL = ContentType.JSON_FULL_METADATA.toContentTypeString(); + static final private String ACCEPT_CASE_NONE = ContentType.JSON_NO_METADATA.toContentTypeString(); static final private String ACCEPT_CASE_MIN_UTF8_IEEE754 = "application/json;charset=UTF-8;odata.metadata=minimal;IEEE754Compatible=true"; - static final private String ACCEPT_CASE_MIN_IEEE754 = - "application/json;odata.metadata=minimal;IEEE754Compatible=true"; + static final private String ACCEPT_CASE_MIN_IEEE754 = ACCEPT_CASE_MIN + ";IEEE754Compatible=true"; static final private String ACCEPT_CASE_JSONQ = "application/json;q=0.2"; static final private String ACCEPT_CASE_XML = ContentType.APPLICATION_XML.toContentTypeString(); static final private String ACCEPT_CASE_WILDCARD1 = "*/*"; static final private String ACCEPT_CASE_WILDCARD2 = "application/*"; - + //@formatter:off (Eclipse formatter) //CHECKSTYLE:OFF (Maven checkstyle) @@ -101,6 +100,7 @@ public class ContentNegotiatorTest { { null, "a/a;x=y", null, "a/a;v=w" }, { null, null, "a/a;x=y", "a/a;v=w" }, { null, null, ACCEPT_CASE_FULL, null }, // not yet supported + { null, null, "*", null }, { null, "a/b;charset=ISO-8859-1", null, "a/b" }, { null, null, "a/b;charset=ISO-8859-1", "a/b" }, { null, null, null, "text/plain" } @@ -153,8 +153,7 @@ public class ContentNegotiatorTest { @Test public void checkSupport() throws Exception { - ContentNegotiator.checkSupport(ContentType.JSON, null, - RepresentationType.ENTITY); + ContentNegotiator.checkSupport(ContentType.JSON, null, RepresentationType.ENTITY); ContentNegotiator.checkSupport(ContentType.TEXT_PLAIN, null, RepresentationType.VALUE); try { ContentNegotiator.checkSupport(ContentType.APPLICATION_SVG_XML, null, RepresentationType.ENTITY); diff --git a/lib/server-test/src/test/java/org/apache/olingo/server/core/ODataHandlerTest.java b/lib/server-test/src/test/java/org/apache/olingo/server/core/ODataHandlerTest.java index a6ab6e8ec..76b7b508d 100644 --- a/lib/server-test/src/test/java/org/apache/olingo/server/core/ODataHandlerTest.java +++ b/lib/server-test/src/test/java/org/apache/olingo/server/core/ODataHandlerTest.java @@ -695,6 +695,15 @@ public class ODataHandlerTest { assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), response.getStatusCode()); } + @Test + public void illegalRequestContentType() throws Exception { + EntityProcessor processor = mock(EntityProcessor.class); + final ODataResponse response = dispatch(HttpMethod.POST, "ESAllPrim", null, + HttpHeader.CONTENT_TYPE, "*/*", processor); + verifyZeroInteractions(processor); + assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), response.getStatusCode()); + } + @Test public void unsupportedRequestContentType() throws Exception { EntityProcessor processor = mock(EntityProcessor.class); From 805028da2f291cbb05d73dbbecb798ab6f8b2231 Mon Sep 17 00:00:00 2001 From: Klaus Straubinger Date: Wed, 16 Sep 2015 10:53:17 +0200 Subject: [PATCH 3/3] [OLINGO-659] still more API clean-up Signed-off-by: Michael Bolz --- .../commons/api/data/DeletedEntity.java | 8 ++---- .../olingo/commons/api/data/Operation.java | 2 +- .../olingo/commons/api/data/package-info.java | 2 +- .../api/edm/constants/package-info.java | 3 +-- .../api/edm/geo/ComposedGeospatial.java | 3 --- .../olingo/commons/api/ex/package-info.java | 2 +- .../olingo/commons/api/format/TypeUtil.java | 20 +++++++++------ .../commons/api/format/package-info.java | 6 +++-- .../olingo/commons/api/http/package-info.java | 2 +- .../apache/olingo/server/api/HttpHeaders.java | 6 +++-- .../olingo/server/api/ODataRequest.java | 2 +- .../olingo/server/api/package-info.java | 25 ++++++++----------- .../server/api/processor/package-info.java | 24 +++++++++--------- 13 files changed, 51 insertions(+), 54 deletions(-) diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/DeletedEntity.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/DeletedEntity.java index 04f8055fd..a5a6a4345 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/DeletedEntity.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/DeletedEntity.java @@ -29,13 +29,9 @@ public class DeletedEntity { * Reason of the removal from the list */ public enum Reason { - /** - * The entity was deleted - */ + /** The entity was deleted. */ deleted, - /** - * The data of the entity as changed and not any longer part of the response - */ + /** The data of the entity has changed and is not any longer part of the response. */ changed } diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/Operation.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/Operation.java index 4b2580177..f7bf1facc 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/Operation.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/Operation.java @@ -21,7 +21,7 @@ package org.apache.olingo.commons.api.data; import java.net.URI; /** - * Data representation for a operation. + * Data representation for an operation. */ public class Operation { diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/package-info.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/package-info.java index 8a74ab6de..276a31d96 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/package-info.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/data/package-info.java @@ -17,6 +17,6 @@ * under the License. */ /** - * Contains all the data objects of an OData responses and OData requests + * Contains all the data objects of OData responses and OData requests */ package org.apache.olingo.commons.api.data; \ No newline at end of file diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/constants/package-info.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/constants/package-info.java index 4bcdf3626..5149056a9 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/constants/package-info.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/constants/package-info.java @@ -17,8 +17,7 @@ * under the License. */ /** - * Contains representations for all constants related - * EDM objects created during the URI parsing + * Contains representations for constants related to EDM objects. */ package org.apache.olingo.commons.api.edm.constants; diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/geo/ComposedGeospatial.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/geo/ComposedGeospatial.java index 8cfea8a79..f7e9f3d6d 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/geo/ComposedGeospatial.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/edm/geo/ComposedGeospatial.java @@ -43,9 +43,6 @@ public abstract class ComposedGeospatial extends Geospatia } } - /** - * {@inheritDoc } - */ @Override public Iterator iterator() { return this.geospatials.iterator(); diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/ex/package-info.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/ex/package-info.java index 04300d2b3..3a8fb90a9 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/ex/package-info.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/ex/package-info.java @@ -17,6 +17,6 @@ * under the License. */ /** - * Contains all OData errors and exceptions related classes. + * Contains all OData errors and exception-related classes. */ package org.apache.olingo.commons.api.ex; \ No newline at end of file diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java index 5fc7ce106..ffd6d6e95 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/TypeUtil.java @@ -24,7 +24,9 @@ import java.util.Map; import java.util.TreeMap; /** - * Helper class which is only used within this package (AcceptType and ContentType handling). + * Helper class which is only used within this package. + * @see AcceptType + * @see ContentType */ final class TypeUtil { @@ -102,13 +104,15 @@ final class TypeUtil { } /** - * Validate that parameter name and parameter value are valid . - * - * @param parameterName must be not null, not empty and contains no whitespace - * characters - * @param parameterValue must be not null, not empty and not start with a whitespace - * character - * @throws IllegalArgumentException if one of the above requirements are not met + * Validates that parameter name and parameter value are valid: + *
    + *
  • not null
  • + *
  • not empty
  • + *
  • does not contain whitespace characters (name), or does not start with whitespace (value), respectively
  • + *
+ * @param parameterName name + * @param parameterValue value + * @throws IllegalArgumentException if one of the above requirements is not met */ static void validateParameterNameAndValue(final String parameterName, final String parameterValue) throws IllegalArgumentException { diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/package-info.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/package-info.java index ef5c906ad..489597df8 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/package-info.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/package-info.java @@ -17,7 +17,9 @@ * under the License. */ /** - * Contains all the format related objects (also related to http headers Prefer, Accepted - * and Content-Type) which are used for handling of an OData responses and OData requests. + * Contains all the format-related objects used for the handling of + * OData responses and OData requests. + * They are related to the HTTP headers Prefer, Accept, + * and Content-Type. */ package org.apache.olingo.commons.api.format; \ No newline at end of file diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/http/package-info.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/http/package-info.java index df854343d..236070d34 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/http/package-info.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/http/package-info.java @@ -17,6 +17,6 @@ * under the License. */ /** - * Contains all the http related objects which are used for handling of an OData responses and OData requests. + * Contains all the HTTP related objects which are used for handling of OData responses and OData requests. */ package org.apache.olingo.commons.api.http; \ No newline at end of file diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/HttpHeaders.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/HttpHeaders.java index eec22f045..cc0fd33af 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/HttpHeaders.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/HttpHeaders.java @@ -26,9 +26,11 @@ import java.util.List; import java.util.Map; /** - * HttpHeader container. + * HttpHeader container for internal use in this package. + * @see ODataRequest + * @see ODataResponse */ -public class HttpHeaders { +final class HttpHeaders { private final Map> headers = new LinkedHashMap>(); /** diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataRequest.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataRequest.java index bb0709668..21122b8ae 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataRequest.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataRequest.java @@ -55,7 +55,7 @@ public class ODataRequest { } /** - *

Set a header to the response.

+ *

Sets a header in the request.

*

The header name will be handled as case-insensitive key.

*

If a header already exists then the header will be replaced by this new value.

* @param name case-insensitive header name diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/package-info.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/package-info.java index d87d42ea4..fcac1f936 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/package-info.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/package-info.java @@ -18,20 +18,17 @@ */ /** * Olingo Server API - *

- * OData Library is a protocol implementation of the OData V4.0 standard. For details of this standard - * see odata.org. - *

- * This API is intended to help implement an OData service. An OData service consists of a metadata provider - * implementation and an OData processor implementation. - *

- * An OData service can be exposed by a web application using the standard java servlet API. See the Olingo tutorials - * section on how to implement a V4 service for further information: - * http://olingo.apache.org/doc/odata4/index.html - *

- * The main entry point is the org.apache.olingo.server.api.OData class. Use the newInstance() method to create a new - * object and start providing your service from there. - * + *

The OData Library is a protocol implementation of the OData V4.0 standard. + * For details of this standard see odata.org.

+ *

This API is intended to help implementing an OData service. + * An OData service consists of a metadata provider implementation and an + * OData processor implementation.

+ *

An OData service can be exposed by a web application using the standard java servlet API. + * For further information, see the Olingo + * tutorials on how to implement an OData service.

+ *

The main entry point is the org.apache.olingo.server.api.OData class. + * Use the newInstance() method to create a new object and + * start providing your service from there.

*/ package org.apache.olingo.server.api; diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/processor/package-info.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/processor/package-info.java index c1a1b0f45..db41fb22e 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/processor/package-info.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/processor/package-info.java @@ -18,17 +18,17 @@ */ /** * Olingo Processors - *

- * Processors are used to handle OData requests and send back the OData reponse. Before a specific processor is called - * the Olingo library will parse the URI and validate it. Afterwards the Processor which matches the return type is - * called. E.g.: If a primitive property is requested by the URI we will call the PrimitveProcessor.read method. - *

- * Processors can be registered at the {@link org.apache.olingo.server.api.ODataHttpHandler} object. Per default we will - * have the {@link org.apache.olingo.server.api.processor.DefaultProcessor} registered to perform basic functionality - * like delivering the metadata and service document as well as rendering an OData error. - *
In case an application would like to perform custom tasks for these cases a new - * {@link org.apache.olingo.server.api.processor.ServiceDocumentProcessor} can be registered in order to overwrite the - * default behaviour. + *

Processors are used to handle OData requests and send back the OData reponse. + * Before a specific processor is called the Olingo library will parse the URI and validate it. + * Afterwards the Processor which matches the return type is called. + * Example: If a primitive property is requested by the URI we will call the + * PrimitiveProcessor.readPrimitive method.

+ *

Processors can be registered at the {@link org.apache.olingo.server.api.ODataHttpHandler} object. + * Per default the {@link org.apache.olingo.server.api.processor.DefaultProcessor} is registered + * to perform basic functionality like delivering the metadata and service documents, respectively, + * as well as rendering an OData error. + * In case an application would like to perform custom tasks for these cases a new + * {@link org.apache.olingo.server.api.processor.ServiceDocumentProcessor} can be registered + * in order to overwrite the default behavior. */ package org.apache.olingo.server.api.processor; -