[OLINGO-1006] Fix batch issue with additional segments
Project: http://git-wip-us.apache.org/repos/asf/olingo-odata2/repo Commit: http://git-wip-us.apache.org/repos/asf/olingo-odata2/commit/8d90a160 Tree: http://git-wip-us.apache.org/repos/asf/olingo-odata2/tree/8d90a160 Diff: http://git-wip-us.apache.org/repos/asf/olingo-odata2/diff/8d90a160 Branch: refs/heads/master Commit: 8d90a160634b6df41b011825bbbb9bc038d95bd7 Parents: 546466c Author: Christian Amend <[email protected]> Authored: Thu Aug 4 16:25:49 2016 +0200 Committer: Christian Amend <[email protected]> Committed: Wed Aug 17 14:32:20 2016 +0200 ---------------------------------------------------------------------- .../apache/olingo/odata2/api/uri/PathInfo.java | 2 +- .../odata2/core/batch/BatchHandlerImpl.java | 8 +- .../odata2/core/batch/v2/BatchParser.java | 13 +- .../odata2/core/batch/BatchHandlerTest.java | 248 +++++++++++++++++++ .../resources/batchContentIdReferencing.batch | 27 ++ .../odata2/fit/client/ClientBatchTest.java | 12 +- 6 files changed, 286 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-api/src/main/java/org/apache/olingo/odata2/api/uri/PathInfo.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-api/src/main/java/org/apache/olingo/odata2/api/uri/PathInfo.java b/odata2-lib/odata-api/src/main/java/org/apache/olingo/odata2/api/uri/PathInfo.java index 0e59a24..8df6ee2 100644 --- a/odata2-lib/odata-api/src/main/java/org/apache/olingo/odata2/api/uri/PathInfo.java +++ b/odata2-lib/odata-api/src/main/java/org/apache/olingo/odata2/api/uri/PathInfo.java @@ -41,7 +41,7 @@ public interface PathInfo { List<PathSegment> getODataSegments(); /** - * Gets the root URI of this service. + * Gets the root URI of this service. This includes any segments which can be found in the preceding segments list. * @return absolute base URI of the request */ URI getServiceRoot(); http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/BatchHandlerImpl.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/BatchHandlerImpl.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/BatchHandlerImpl.java index 75eb265..64f8e22 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/BatchHandlerImpl.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/BatchHandlerImpl.java @@ -109,7 +109,7 @@ public class BatchHandlerImpl implements BatchHandler { private void fillContentIdMap(final ODataResponse response, final String contentId, final String baseUri) { String location = response.getHeader(HttpHeaders.LOCATION); - if(location != null) { + if (location != null) { String relLocation = location.replace(baseUri + "/", ""); contentIdMap.put("$" + contentId, relLocation); } @@ -119,7 +119,7 @@ public class BatchHandlerImpl implements BatchHandler { throws ODataException { String contentId = contentIdMap.get(odataSegments.get(0).getPath()); if (contentId == null) { - //invalid content ID. But throwing an exception here is wrong so we use the base request and fail later + // invalid content ID. But throwing an exception here is wrong so we use the base request and fail later return request; } PathInfoImpl pathInfo = new PathInfoImpl(); @@ -171,13 +171,11 @@ public class BatchHandlerImpl implements BatchHandler { } private String getBaseUri(final ODataRequest request) { + // The service root already contains any additional path parameters String baseUri = request.getPathInfo().getServiceRoot().toASCIIString(); if (baseUri.endsWith("/")) { baseUri = baseUri.substring(0, baseUri.length() - 1); } - for (PathSegment segment : request.getPathInfo().getPrecedingSegments()) { - baseUri += "/" + segment.getPath(); - } return baseUri; } http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java index 7f53e0d..a79714b 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java @@ -29,7 +29,6 @@ import org.apache.olingo.odata2.api.batch.BatchRequestPart; import org.apache.olingo.odata2.api.client.batch.BatchSingleResponse; import org.apache.olingo.odata2.api.ep.EntityProviderBatchProperties; import org.apache.olingo.odata2.api.uri.PathInfo; -import org.apache.olingo.odata2.api.uri.PathSegment; import org.apache.olingo.odata2.core.exception.ODataRuntimeException; public class BatchParser { @@ -103,18 +102,10 @@ public class BatchParser { private String getBaseUri() throws BatchException { String baseUri = ""; + //The service root already contains any additional path parameters if (batchRequestPathInfo != null && batchRequestPathInfo.getServiceRoot() != null) { final String uri = batchRequestPathInfo.getServiceRoot().toASCIIString(); - - baseUri = addPathSegements(removeLastSlash(uri)); - } - - return baseUri; - } - - private String addPathSegements(String baseUri) { - for (PathSegment precedingPS : batchRequestPathInfo.getPrecedingSegments()) { - baseUri = baseUri + "/" + precedingPS.getPath(); + baseUri = removeLastSlash(uri); } return baseUri; http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchHandlerTest.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchHandlerTest.java b/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchHandlerTest.java new file mode 100644 index 0000000..218e163 --- /dev/null +++ b/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchHandlerTest.java @@ -0,0 +1,248 @@ +/******************************************************************************* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + ******************************************************************************/ +package org.apache.olingo.odata2.core.batch; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; + +import org.apache.olingo.odata2.api.ODataService; +import org.apache.olingo.odata2.api.ODataServiceFactory; +import org.apache.olingo.odata2.api.batch.BatchHandler; +import org.apache.olingo.odata2.api.batch.BatchRequestPart; +import org.apache.olingo.odata2.api.batch.BatchResponsePart; +import org.apache.olingo.odata2.api.commons.HttpStatusCodes; +import org.apache.olingo.odata2.api.edm.Edm; +import org.apache.olingo.odata2.api.ep.EntityProvider; +import org.apache.olingo.odata2.api.ep.EntityProviderBatchProperties; +import org.apache.olingo.odata2.api.exception.ODataException; +import org.apache.olingo.odata2.api.processor.ODataContext; +import org.apache.olingo.odata2.api.processor.ODataProcessor; +import org.apache.olingo.odata2.api.processor.ODataRequest; +import org.apache.olingo.odata2.api.processor.ODataResponse; +import org.apache.olingo.odata2.api.processor.part.BatchProcessor; +import org.apache.olingo.odata2.api.processor.part.EntityMediaProcessor; +import org.apache.olingo.odata2.api.processor.part.EntityProcessor; +import org.apache.olingo.odata2.api.processor.part.EntitySetProcessor; +import org.apache.olingo.odata2.api.processor.part.EntitySimplePropertyProcessor; +import org.apache.olingo.odata2.api.uri.PathInfo; +import org.apache.olingo.odata2.api.uri.PathSegment; +import org.apache.olingo.odata2.api.uri.info.GetEntitySetCountUriInfo; +import org.apache.olingo.odata2.api.uri.info.GetEntitySetUriInfo; +import org.apache.olingo.odata2.api.uri.info.GetSimplePropertyUriInfo; +import org.apache.olingo.odata2.api.uri.info.PostUriInfo; +import org.apache.olingo.odata2.api.uri.info.PutMergePatchUriInfo; +import org.apache.olingo.odata2.core.ODataPathSegmentImpl; +import org.apache.olingo.odata2.core.PathInfoImpl; +import org.apache.olingo.odata2.testutil.helper.StringHelper; +import org.apache.olingo.odata2.testutil.mock.MockFacade; +import org.junit.Before; +import org.junit.Test; + +public class BatchHandlerTest { + + private BatchHandler handler; + private static final String CONTENT_TYPE = "multipart/mixed; boundary=batch_123"; + private static final String CRLF = "\r\n"; + private static String SERVICE_BASE = "http://localhost/odata/";; + private static String SERVICE_ROOT = null; + + @Before + public void setupBatchHandler() throws Exception { + ODataProcessor processor = new LocalProcessor(); + ODataService serviceMock = mock(ODataService.class); + when(serviceMock.getBatchProcessor()).thenReturn((BatchProcessor) processor); + when(serviceMock.getEntitySetProcessor()).thenReturn((EntitySetProcessor) processor); + when(serviceMock.getEntitySimplePropertyProcessor()).thenReturn((EntitySimplePropertyProcessor) processor); + when(serviceMock.getProcessor()).thenReturn(processor); + Edm mockEdm = MockFacade.getMockEdm(); + when(serviceMock.getEntityDataModel()).thenReturn(mockEdm); + List<String> supportedContentTypes = Arrays.asList("application/json;charset=utf-8", "application/json"); + when(serviceMock.getSupportedContentTypes(EntityMediaProcessor.class)).thenReturn(supportedContentTypes); + when(serviceMock.getSupportedContentTypes(EntityProcessor.class)).thenReturn(supportedContentTypes); + when(serviceMock.getSupportedContentTypes(EntitySimplePropertyProcessor.class)).thenReturn(supportedContentTypes); + handler = new BatchHandlerImpl(mock(ODataServiceFactory.class), serviceMock); + } + + @Test + public void contentIdReferencing() throws Exception { + SERVICE_ROOT = SERVICE_BASE; + PathInfoImpl pathInfo = new PathInfoImpl(); + pathInfo.setServiceRoot(new URI(SERVICE_ROOT)); + List<PathSegment> odataPathSegements = new ArrayList<PathSegment>(); + odataPathSegements.add(new ODataPathSegmentImpl("$batch", null)); + pathInfo.setODataPathSegment(odataPathSegements); + EntityProviderBatchProperties properties = EntityProviderBatchProperties.init().pathInfo(pathInfo).build(); + InputStream content = readFile("/batchContentIdReferencing.batch"); + List<BatchRequestPart> parsedRequest = EntityProvider.parseBatchRequest(CONTENT_TYPE, content, properties); + + PathInfo firstPathInfo = parsedRequest.get(0).getRequests().get(0).getPathInfo(); + assertFirst(firstPathInfo); + + handler.handleBatchPart(parsedRequest.get(0)); + } + + @Test + public void contentIdReferencingWithAdditionalSegments() throws Exception { + SERVICE_ROOT = SERVICE_BASE + "seg1/seg2/"; + PathInfoImpl pathInfo = new PathInfoImpl(); + List<PathSegment> precedingPathSegements = new ArrayList<PathSegment>(); + precedingPathSegements.add(new ODataPathSegmentImpl("seg1", null)); + precedingPathSegements.add(new ODataPathSegmentImpl("seg2", null)); + pathInfo.setPrecedingPathSegment(precedingPathSegements); + pathInfo.setServiceRoot(new URI(SERVICE_ROOT)); + List<PathSegment> odataPathSegements = new ArrayList<PathSegment>(); + odataPathSegements.add(new ODataPathSegmentImpl("$batch", null)); + pathInfo.setODataPathSegment(odataPathSegements); + EntityProviderBatchProperties properties = EntityProviderBatchProperties.init().pathInfo(pathInfo).build(); + InputStream content = readFile("/batchContentIdReferencing.batch"); + List<BatchRequestPart> parsedRequest = EntityProvider.parseBatchRequest(CONTENT_TYPE, content, properties); + + PathInfo firstPathInfo = parsedRequest.get(0).getRequests().get(0).getPathInfo(); + assertFirst(firstPathInfo); + + handler.handleBatchPart(parsedRequest.get(0)); + } + + @Test + public void contentIdReferencingWithAdditionalSegmentsAndMatrixParameter() throws Exception { + SERVICE_ROOT = SERVICE_BASE + "seg1;v=1/seg2;v=2/"; + PathInfoImpl pathInfo = new PathInfoImpl(); + List<PathSegment> precedingPathSegements = new ArrayList<PathSegment>(); + HashMap<String, List<String>> matrixParameters1 = new HashMap<String, List<String>>(); + matrixParameters1.put("v", Arrays.asList("1")); + HashMap<String, List<String>> matrixParameters2 = new HashMap<String, List<String>>(); + matrixParameters1.put("v", Arrays.asList("2")); + precedingPathSegements.add(new ODataPathSegmentImpl("seg1", matrixParameters1)); + precedingPathSegements.add(new ODataPathSegmentImpl("seg2", matrixParameters2)); + pathInfo.setPrecedingPathSegment(precedingPathSegements); + pathInfo.setServiceRoot(new URI(SERVICE_ROOT)); + List<PathSegment> odataPathSegements = new ArrayList<PathSegment>(); + odataPathSegements.add(new ODataPathSegmentImpl("$batch", null)); + pathInfo.setODataPathSegment(odataPathSegements); + EntityProviderBatchProperties properties = EntityProviderBatchProperties.init().pathInfo(pathInfo).build(); + InputStream content = readFile("/batchContentIdReferencing.batch"); + List<BatchRequestPart> parsedRequest = EntityProvider.parseBatchRequest(CONTENT_TYPE, content, properties); + + PathInfo firstPathInfo = parsedRequest.get(0).getRequests().get(0).getPathInfo(); + assertFirst(firstPathInfo); + + handler.handleBatchPart(parsedRequest.get(0)); + } + + private void assertFirst(PathInfo pathInfo) { + assertEquals(SERVICE_ROOT + "Employees", pathInfo.getRequestUri().toString()); + assertEquals(SERVICE_ROOT, pathInfo.getServiceRoot().toString()); + } + + private InputStream readFile(String fileName) throws IOException { + InputStream in = ClassLoader.class.getResourceAsStream(fileName); + if (in == null) { + throw new IOException("Requested file '" + fileName + "' was not found."); + } + + return StringHelper.toStream(in).asStreamWithLineSeparation(CRLF); + } + + public class LocalProcessor implements BatchProcessor, EntitySetProcessor, EntitySimplePropertyProcessor { + + private ODataContext context; + + @Override + public void setContext(ODataContext context) throws ODataException { + this.context = context; + } + + @Override + public ODataContext getContext() throws ODataException { + return context; + } + + @Override + public BatchResponsePart executeChangeSet(BatchHandler handler, List<ODataRequest> requests) throws ODataException { + List<ODataResponse> responses = new ArrayList<ODataResponse>(); + + // handle create + ODataResponse response = handler.handleRequest(requests.get(0)); + assertEquals(HttpStatusCodes.OK, response.getStatus()); + assertEquals(SERVICE_ROOT + "Employees('1')", response.getIdLiteral()); + responses.add(response); + + // handle update + response = handler.handleRequest(requests.get(1)); + assertEquals(HttpStatusCodes.OK, response.getStatus()); + responses.add(response); + + return BatchResponsePart.responses(responses).changeSet(true).build(); + } + + @Override + public ODataResponse createEntity(PostUriInfo uriInfo, InputStream content, String requestContentType, + String contentType) throws ODataException { + PathInfo pathInfo = getContext().getPathInfo(); + assertFirst(pathInfo); + assertEquals("Employees", uriInfo.getTargetEntitySet().getName()); + return ODataResponse.newBuilder().status(HttpStatusCodes.OK).idLiteral(SERVICE_ROOT + "Employees('1')").build(); + } + + @Override + public ODataResponse updateEntitySimpleProperty(PutMergePatchUriInfo uriInfo, InputStream content, + String requestContentType, String contentType) throws ODataException { + PathInfo pathInfo = getContext().getPathInfo(); + assertEquals(SERVICE_ROOT + "Employees('1')/EmployeeName", pathInfo.getRequestUri().toString()); + assertEquals(SERVICE_ROOT, pathInfo.getServiceRoot().toString()); + assertEquals("Employees", uriInfo.getTargetEntitySet().getName()); + return ODataResponse.newBuilder().status(HttpStatusCodes.OK).build(); + } + + @Override + public ODataResponse readEntitySimpleProperty(GetSimplePropertyUriInfo uriInfo, String contentType) + throws ODataException { + // this method is not needed. + return null; + } + + @Override + public ODataResponse readEntitySet(GetEntitySetUriInfo uriInfo, String contentType) throws ODataException { + // this method is not needed. + return null; + } + + @Override + public ODataResponse countEntitySet(GetEntitySetCountUriInfo uriInfo, String contentType) throws ODataException { + // this method is not needed. + return null; + } + + @Override + public ODataResponse executeBatch(BatchHandler handler, String contentType, InputStream content) + throws ODataException { + // this method is not needed. + return null; + } + } +} http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-core/src/test/resources/batchContentIdReferencing.batch ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/test/resources/batchContentIdReferencing.batch b/odata2-lib/odata-core/src/test/resources/batchContentIdReferencing.batch new file mode 100644 index 0000000..087f662 --- /dev/null +++ b/odata2-lib/odata-core/src/test/resources/batchContentIdReferencing.batch @@ -0,0 +1,27 @@ +--batch_123 +Content-Type: multipart/mixed; boundary=changeset_b4d2651f-4c8e-4707-8ac3-5bdde1e25760 + +--changeset_b4d2651f-4c8e-4707-8ac3-5bdde1e25760 +Content-Type: application/http +Content-Transfer-Encoding: binary +Content-Id: 1 + +POST Employees HTTP/1.1 +Content-Length: 23 +Accept: application/json +content-type: application/json + +gAAAAgABwESAAMAAAABAAEA +--changeset_b4d2651f-4c8e-4707-8ac3-5bdde1e25760 +Content-Type: application/http +Content-Transfer-Encoding: binary +Content-Id: 2 + +PUT $1/EmployeeName HTTP/1.1 +Content-Length: 41 +Accept: application/json +content-type: application/json + +{"EmployeeName":"Frederic Fall MODIFIED"} +--changeset_b4d2651f-4c8e-4707-8ac3-5bdde1e25760-- +--batch_123-- \ No newline at end of file http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/8d90a160/odata2-lib/odata-fit/src/test/java/org/apache/olingo/odata2/fit/client/ClientBatchTest.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-fit/src/test/java/org/apache/olingo/odata2/fit/client/ClientBatchTest.java b/odata2-lib/odata-fit/src/test/java/org/apache/olingo/odata2/fit/client/ClientBatchTest.java index 3ab22f6..83dff6f 100644 --- a/odata2-lib/odata-fit/src/test/java/org/apache/olingo/odata2/fit/client/ClientBatchTest.java +++ b/odata2-lib/odata-fit/src/test/java/org/apache/olingo/odata2/fit/client/ClientBatchTest.java @@ -45,10 +45,8 @@ import org.apache.olingo.odata2.api.ep.EntityProvider; import org.apache.olingo.odata2.fit.ref.AbstractRefTest; import org.apache.olingo.odata2.testutil.helper.StringHelper; import org.apache.olingo.odata2.testutil.server.ServletType; -import org.junit.Ignore; import org.junit.Test; -@Ignore public class ClientBatchTest extends AbstractRefTest { public ClientBatchTest(final ServletType servletType) { super(servletType); @@ -66,7 +64,7 @@ public class ClientBatchTest extends AbstractRefTest { batch.add(request); InputStream body = EntityProvider.writeBatchRequest(batch, BOUNDARY); - String batchRequestBody = StringHelper.inputStreamToString(body, true); + String batchRequestBody = StringHelper.inputStreamToStringCRLFLineBreaks(body); checkMimeHeaders(batchRequestBody); checkBoundaryDelimiters(batchRequestBody); assertTrue(batchRequestBody.contains("GET $metadata HTTP/1.1")); @@ -78,7 +76,7 @@ public class ClientBatchTest extends AbstractRefTest { for (BatchSingleResponse response : responses) { assertEquals("200", response.getStatusCode()); assertEquals("OK", response.getStatusInfo()); - assertTrue(response.getBody().contains("<edmx:Edmx Version=\"1.0\"")); + assertTrue(response.getBody().contains("<edmx:Edmx")); assertEquals("application/xml;charset=utf-8", response.getHeader(HttpHeaders.CONTENT_TYPE)); assertNotNull(response.getHeader(HttpHeaders.CONTENT_LENGTH)); } @@ -106,7 +104,7 @@ public class ClientBatchTest extends AbstractRefTest { batch.add(request); InputStream body = EntityProvider.writeBatchRequest(batch, BOUNDARY); - String bodyAsString = StringHelper.inputStreamToString(body, true); + String bodyAsString = StringHelper.inputStreamToStringCRLFLineBreaks(body); checkMimeHeaders(bodyAsString); checkBoundaryDelimiters(bodyAsString); @@ -214,7 +212,7 @@ public class ClientBatchTest extends AbstractRefTest { batch.add(request); InputStream body = EntityProvider.writeBatchRequest(batch, BOUNDARY); - String bodyAsString = StringHelper.inputStreamToString(body, true); + String bodyAsString = StringHelper.inputStreamToStringCRLFLineBreaks(body); checkMimeHeaders(bodyAsString); checkBoundaryDelimiters(bodyAsString); assertTrue(bodyAsString.contains("POST Employees HTTP/1.1")); @@ -250,7 +248,7 @@ public class ClientBatchTest extends AbstractRefTest { batch.add(request); InputStream body = EntityProvider.writeBatchRequest(batch, BOUNDARY); - String bodyAsString = StringHelper.inputStreamToString(body, true); + String bodyAsString = StringHelper.inputStreamToStringCRLFLineBreaks(body); checkMimeHeaders(bodyAsString); checkBoundaryDelimiters(bodyAsString);
