From df5a4ff0dee8794cb1d0c67afed5f824125a828a Mon Sep 17 00:00:00 2001 From: Michael Bolz Date: Fri, 14 Aug 2015 09:51:12 +0200 Subject: [PATCH] [OLINGO-659] API review for ODataRequest/Response --- .../olingo/server/api/ODataRequest.java | 30 +++++++- .../olingo/server/api/ODataResponse.java | 70 +++++++++++++++++-- .../core/responses/ServiceResponse.java | 4 +- .../olingo/server/core/ODataHandler.java | 8 +-- .../server/core/ODataHttpHandlerImpl.java | 6 +- .../BatchReferenceRewriter.java | 2 +- .../core/debug/DebugResponseHelperImpl.java | 2 +- .../server/core/debug/DebugTabBody.java | 4 +- .../server/core/debug/DebugTabResponse.java | 22 ++++-- .../serializer/AsyncResponseSerializer.java | 11 +-- .../serializer/BatchResponseSerializer.java | 10 +-- .../server/tecsvc/async/AsyncProcessor.java | 4 +- .../olingo/server/core/ODataHandlerTest.java | 12 ++-- 13 files changed, 142 insertions(+), 43 deletions(-) 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 5b734ecb8..4f364e186 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 @@ -60,6 +60,30 @@ public class ODataRequest { this.method = method; } + /** + *

Set a header to the response.

+ *

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 + * @param value value for the given header name + * @see RFC 7230, section 3.2.2 + */ + public void setHeader(final String name, final String value) { + headers.setHeader(name, value); + } + + /** + *

Adds a header to the request.

+ *

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

+ *

If a header already exists then the list of values will just be extended.

+ * @param name case-insensitive header name + * @param value value for the given header name + * @see RFC 7230, section 3.2.2 + */ + public void addHeader(final String name, final String value) { + headers.addHeader(name, value); + } + /** *

Adds a header to the request.

*

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

@@ -73,20 +97,20 @@ public class ODataRequest { } /** - * Gets header value for a given name. + * Get header values for a given name. * @param name the header name as a case-insensitive key * @return the header value(s) or null if not found */ public List getHeaders(final String name) { HttpHeader h = headers.getHeader(name); if(h == null) { - return null;//Collections.emptyList(); + return null; } return new ArrayList(h.getValues()); } /** - * Gets first header value for a given name. + * Get first header value for a given name. * @param name the header name as a case-insensitive key * @return the first header value or null if not found */ diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataResponse.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataResponse.java index b104ff7ca..367ce1696 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataResponse.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/ODataResponse.java @@ -19,9 +19,11 @@ package org.apache.olingo.server.api; import java.io.InputStream; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.olingo.commons.api.http.HttpHeader; @@ -54,27 +56,81 @@ public class ODataResponse { } /** - * Sets a header. - * @param name the name - * @param value the value + *

Set a header to the response.

+ *

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 + * @param value value for the given header name + * @see RFC 7230, section 3.2.2 */ public void setHeader(final String name, final String value) { headers.setHeader(name, value); } /** - * Gets all headers. + *

Adds a header to the response.

+ *

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

+ *

If a header already exists then the list of values will just be extended.

+ * @param name case-insensitive header name + * @param value value for the given header name + * @see RFC 7230, section 3.2.2 + */ + public void addHeader(final String name, final String value) { + headers.setHeader(name, value); + } + + /** + *

Adds a header to the response.

+ *

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

+ *

If a header already exists then the list of values will just be extended.

+ * @param name case-insensitive header name + * @param values list of values for the given header name + * @see RFC 7230, section 3.2.2 + */ + public void addHeader(final String name, final List values) { + headers.addHeader(name, values); + } + + /** + * Get all headers with the according values. + * * @return an unmodifiable Map of header names/values */ - public Map getHeaders() { - Map result = new HashMap(); + public Map> getAllHeaders() { + Map> result = new HashMap>(); Collection allHeaders = headers.getHeaders(); for (HttpHeader header : allHeaders) { - result.put(header.getName(), header.getValues().iterator().next()); + result.put(header.getName(), new ArrayList(header.getValues())); } return Collections.unmodifiableMap(result); } + /** + * Gets header value for a given name. + * @param name the header name as a case-insensitive key + * @return the header value(s) or null if not found + */ + public List getHeaders(final String name) { + HttpHeader h = headers.getHeader(name); + if(h == null) { + return null; + } + return new ArrayList(h.getValues()); + } + + /** + * Gets first header value for a given name. + * If header name is not known null is returned. + * + * @param name the header name as a case-insensitive key + * @return the first header value or null if not found + */ + public String getHeader(final String name) { + final List values = getHeaders(name); + return values == null ? null : values.get(0); + } + + /** * Sets the content (body). * @param content the content as {@link InputStream} diff --git a/lib/server-core-ext/src/main/java/org/apache/olingo/server/core/responses/ServiceResponse.java b/lib/server-core-ext/src/main/java/org/apache/olingo/server/core/responses/ServiceResponse.java index deb120630..56ae9f428 100644 --- a/lib/server-core-ext/src/main/java/org/apache/olingo/server/core/responses/ServiceResponse.java +++ b/lib/server-core-ext/src/main/java/org/apache/olingo/server/core/responses/ServiceResponse.java @@ -55,7 +55,7 @@ public abstract class ServiceResponse { if (!this.closed) { if (this.strictApplyPreferences) { if (!preferences.isEmpty()) { - assert(this.response.getHeaders().get("Preference-Applied") != null); + assert(this.response.getAllHeaders().get("Preference-Applied") != null); } } this.closed = true; @@ -97,7 +97,7 @@ public abstract class ServiceResponse { public void writeHeader(String key, String value) { if ("Preference-Applied".equals(key)) { - String previous = this.response.getHeaders().get(key); + String previous = this.response.getHeader(key); if (previous != null) { value = previous+";"+value; } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandler.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandler.java index bfbc5ac3d..80d548a86 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandler.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHandler.java @@ -117,7 +117,9 @@ public class ODataHandler { private void processInternal(final ODataRequest request, final ODataResponse response) throws ODataApplicationException, ODataLibraryException { - validateODataVersion(request, response); + response.setHeader(HttpHeader.ODATA_VERSION, ODataServiceVersion.V40.toString()); + + validateODataVersion(request); int measurementUriParser = debugger.startRuntimeMeasurement("UriParser", "parseUri"); uriInfo = new Parser().parseUri(request.getRawODataPath(), request.getRawQueryPath(), null, @@ -157,11 +159,9 @@ public class ODataHandler { debugger.stopRuntimeMeasurement(measurementHandle); } - private void validateODataVersion(final ODataRequest request, final ODataResponse response) + private void validateODataVersion(final ODataRequest request) throws ODataHandlerException { final String maxVersion = request.getHeader(HttpHeader.ODATA_MAX_VERSION); - response.setHeader(HttpHeader.ODATA_VERSION, ODataServiceVersion.V40.toString()); - if (maxVersion != null) { if (ODataServiceVersion.isBiggerThan(ODataServiceVersion.V40.toString(), maxVersion)) { throw new ODataHandlerException("ODataVersion not supported: " + maxVersion, diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHttpHandlerImpl.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHttpHandlerImpl.java index e4435917b..fbcc230b8 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHttpHandlerImpl.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/ODataHttpHandlerImpl.java @@ -147,8 +147,10 @@ public class ODataHttpHandlerImpl implements ODataHttpHandler { static void convertToHttp(final HttpServletResponse response, final ODataResponse odResponse) { response.setStatus(odResponse.getStatusCode()); - for (Entry entry : odResponse.getHeaders().entrySet()) { - response.setHeader(entry.getKey(), entry.getValue()); + for (Entry> entry : odResponse.getAllHeaders().entrySet()) { + for (String headerValue : entry.getValue()) { + response.addHeader(entry.getKey(), headerValue); + } } InputStream input = odResponse.getContent(); diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java index 31d201040..a941c336d 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java @@ -78,7 +78,7 @@ public class BatchReferenceRewriter { if (request.getMethod() == HttpMethod.POST) { // Create entity // The URI of the new resource will be generated by the server and published in the location header - final String locationHeader = response.getHeaders().get(HttpHeader.LOCATION); + final String locationHeader = response.getHeader(HttpHeader.LOCATION); resourceUri = locationHeader == null ? null : parseODataPath(locationHeader, request.getRawBaseUri()); } else { // Update, Upsert (PUT, PATCH, Delete) diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugResponseHelperImpl.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugResponseHelperImpl.java index fa07f94c7..3a1c2e0dc 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugResponseHelperImpl.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugResponseHelperImpl.java @@ -73,7 +73,7 @@ public class DebugResponseHelperImpl implements DebugResponseHelper { switch (requestedFormat) { case DOWNLOAD: response.setHeader("Content-Disposition", "attachment; filename=OData-Response." - + new Date().toString().replace(' ', '_').replace(':', '.') + ".html"); + + new Date().toString().replace(' ', '_').replace(':', '.') + ".html"); // Download is the same as html except for the above header case HTML: String title = debugInfo.getRequest() == null ? diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabBody.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabBody.java index 5608d584a..e0330b81e 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabBody.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabBody.java @@ -52,7 +52,7 @@ public class DebugTabBody implements DebugTab { this.response = response; this.serviceRoot = serviceRoot == null ? "/" : serviceRoot; if (response != null) { - final String contentType = response.getHeaders().get(HttpHeader.CONTENT_TYPE); + final String contentType = response.getHeader(HttpHeader.CONTENT_TYPE); // TODO: Differentiate better if (contentType != null) { responseContent = ResponseContent.JSON; @@ -125,7 +125,7 @@ public class DebugTabBody implements DebugTab { break; case IMAGE: // Make header query case insensitive - writer.append("\n"); break; diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabResponse.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabResponse.java index 528cf6160..11331a9ce 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabResponse.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/debug/DebugTabResponse.java @@ -21,6 +21,8 @@ package org.apache.olingo.server.core.debug; import java.io.IOException; import java.io.Writer; import java.util.Collections; +import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.olingo.commons.api.http.HttpStatusCode; @@ -36,13 +38,13 @@ public class DebugTabResponse implements DebugTab { private final ODataResponse response; private final String serviceRoot; private final HttpStatusCode status; - private final Map headers; + private final Map> headers; public DebugTabResponse(final ODataResponse applicationResponse, final String serviceRoot) { this.response = applicationResponse; if (response != null) { status = HttpStatusCode.fromStatusCode(response.getStatusCode()); - headers = response.getHeaders(); + headers = response.getAllHeaders(); } else { status = HttpStatusCode.INTERNAL_SERVER_ERROR; headers = Collections.emptyMap(); @@ -69,7 +71,7 @@ public class DebugTabResponse implements DebugTab { if (headers != null && !headers.isEmpty()) { gen.writeFieldName("headers"); - DebugResponseHelperImpl.appendJsonTable(gen, headers); + DebugResponseHelperImpl.appendJsonTable(gen, map(headers)); } gen.writeFieldName("body"); @@ -82,13 +84,25 @@ public class DebugTabResponse implements DebugTab { gen.writeEndObject(); } + private Map map(Map> headers) { + Map result = new HashMap(); + for (Map.Entry> entry : headers.entrySet()) { + if(entry.getValue().size() == 1) { + result.put(entry.getKey(), entry.getValue().get(0)); + } else { + result.put(entry.getKey(), entry.getValue().toString()); + } + } + return result; + } + @Override public void appendHtml(final Writer writer) throws IOException { writer.append("

Status Code

\n") .append("

").append(Integer.toString(status.getStatusCode())).append(' ') .append(status.getInfo()).append("

\n") .append("

Response Headers

\n"); - DebugResponseHelperImpl.appendHtmlTable(writer, headers); + DebugResponseHelperImpl.appendHtmlTable(writer, map(headers)); writer.append("

Response Body

\n"); if (response != null && response.getContent() != null) { new DebugTabBody(response, serviceRoot).appendHtml(writer); diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/AsyncResponseSerializer.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/AsyncResponseSerializer.java index 9c3060e5d..6f431d4af 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/AsyncResponseSerializer.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/AsyncResponseSerializer.java @@ -31,6 +31,7 @@ import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; +import java.util.List; import java.util.Map; public class AsyncResponseSerializer { @@ -59,16 +60,18 @@ public class AsyncResponseSerializer { private void appendResponseHeader(final ODataResponse response, final ByteArrayOutputStream buffer) throws IOException { - final Map header = response.getHeaders(); + final Map> header = response.getAllHeaders(); - for (final Map.Entry entry: header.entrySet()) { + for (final Map.Entry> entry: header.entrySet()) { appendHeader(entry.getKey(), entry.getValue(), buffer); } } - private void appendHeader(final String name, final String value, final ByteArrayOutputStream buffer) + private void appendHeader(final String name, final List values, final ByteArrayOutputStream buffer) throws IOException { - append(name + COLON + SP + value + CRLF, buffer); + for (String value : values) { + append(name + COLON + SP + value + CRLF, buffer); + } } private void appendStatusLine(final ODataResponse response, final ByteArrayOutputStream buffer) diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/BatchResponseSerializer.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/BatchResponseSerializer.java index d54f0280c..4af94f98b 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/BatchResponseSerializer.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/serializer/BatchResponseSerializer.java @@ -135,12 +135,12 @@ public class BatchResponseSerializer { private void appendResponseHeader(final ODataResponse response, final int contentLength, final BodyBuilder builder) { - final Map header = response.getHeaders(); + final Map> header = response.getAllHeaders(); - for (final Map.Entry entry : header.entrySet()) { + for (final Map.Entry> entry : header.entrySet()) { // Requests do never has a content id header if (!entry.getKey().equalsIgnoreCase(HttpHeader.CONTENT_ID)) { - appendHeader(entry.getKey(), entry.getValue(), builder); + appendHeader(entry.getKey(), entry.getValue().get(0), builder); } } @@ -153,8 +153,8 @@ public class BatchResponseSerializer { appendHeader(BatchParserCommon.CONTENT_TRANSFER_ENCODING, BatchParserCommon.BINARY_ENCODING, builder); if (isChangeSet) { - if (response.getHeaders().get(HttpHeader.CONTENT_ID) != null) { - appendHeader(HttpHeader.CONTENT_ID, response.getHeaders().get(HttpHeader.CONTENT_ID), builder); + if (response.getAllHeaders().get(HttpHeader.CONTENT_ID) != null) { + appendHeader(HttpHeader.CONTENT_ID, response.getHeader(HttpHeader.CONTENT_ID), builder); } else { throw new BatchSerializerException("Missing content id", MessageKeys.MISSING_CONTENT_ID); } diff --git a/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/async/AsyncProcessor.java b/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/async/AsyncProcessor.java index 9f36e11b9..da16f9b63 100644 --- a/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/async/AsyncProcessor.java +++ b/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/async/AsyncProcessor.java @@ -193,8 +193,8 @@ public class AsyncProcessor { private static ODataResponse createODataResponse(ODataResponse response) { ODataResponse created = new ODataResponse(); - for (Map.Entry header : response.getHeaders().entrySet()) { - created.setHeader(header.getKey(), header.getValue()); + for (Map.Entry> header : response.getAllHeaders().entrySet()) { + created.addHeader(header.getKey(), header.getValue()); } return created; } 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 5b347d15a..8cd470655 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 @@ -106,7 +106,7 @@ public class ODataHandlerTest { final ODataResponse response = dispatch(HttpMethod.GET, "/", null); assertEquals(HttpStatusCode.OK.getStatusCode(), response.getStatusCode()); - String ct = response.getHeaders().get(HttpHeader.CONTENT_TYPE); + String ct = response.getHeader(HttpHeader.CONTENT_TYPE); assertThat(ct, containsString("application/json")); assertThat(ct, containsString("odata.metadata=minimal")); @@ -121,7 +121,7 @@ public class ODataHandlerTest { public void serviceDocumentRedirect() throws Exception { final ODataResponse response = dispatch(HttpMethod.GET, "", null); assertEquals(HttpStatusCode.TEMPORARY_REDIRECT.getStatusCode(), response.getStatusCode()); - assertEquals(BASE_URI + "/", response.getHeaders().get(HttpHeader.LOCATION)); + assertEquals(BASE_URI + "/", response.getHeader(HttpHeader.LOCATION)); } @Test @@ -143,7 +143,7 @@ public class ODataHandlerTest { public void metadataDefault() throws Exception { final ODataResponse response = dispatch(HttpMethod.GET, "$metadata", null); assertEquals(HttpStatusCode.OK.getStatusCode(), response.getStatusCode()); - assertEquals(HttpContentType.APPLICATION_XML, response.getHeaders().get(HttpHeader.CONTENT_TYPE)); + assertEquals(HttpContentType.APPLICATION_XML, response.getHeader(HttpHeader.CONTENT_TYPE)); assertNotNull(response.getContent()); assertThat(IOUtils.toString(response.getContent()), @@ -153,14 +153,14 @@ public class ODataHandlerTest { @Test public void maxVersionNone() { final ODataResponse response = dispatch(HttpMethod.GET, "$metadata", null); - assertEquals(ODataServiceVersion.V40.toString(), response.getHeaders().get(HttpHeader.ODATA_VERSION)); + assertEquals(ODataServiceVersion.V40.toString(), response.getHeader(HttpHeader.ODATA_VERSION)); } @Test public void maxVersionSupported() { final ODataResponse response = dispatch(HttpMethod.GET, "$metadata", null, HttpHeader.ODATA_MAX_VERSION, ODataServiceVersion.V40.toString(), null); - assertEquals(ODataServiceVersion.V40.toString(), response.getHeaders().get(HttpHeader.ODATA_VERSION)); + assertEquals(ODataServiceVersion.V40.toString(), response.getHeader(HttpHeader.ODATA_VERSION)); } @Test @@ -168,7 +168,7 @@ public class ODataHandlerTest { final ODataResponse response = dispatch(HttpMethod.GET, "$metadata", null, HttpHeader.ODATA_MAX_VERSION, ODataServiceVersion.V30.toString(), null); - assertEquals(ODataServiceVersion.V40.toString(), response.getHeaders().get(HttpHeader.ODATA_VERSION)); + assertEquals(ODataServiceVersion.V40.toString(), response.getHeader(HttpHeader.ODATA_VERSION)); assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), response.getStatusCode()); }