You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@olingo.apache.org by mi...@apache.org on 2022/11/12 21:33:01 UTC

[olingo-odata4] branch master updated: OLINGO-1577 Determine content type from HTTP response

This is an automated email from the ASF dual-hosted git repository.

mibo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/olingo-odata4.git


The following commit(s) were added to refs/heads/master by this push:
     new 532719421 OLINGO-1577 Determine content type from HTTP response
532719421 is described below

commit 532719421a2b32476c69eea99dcb355384082d08
Author: Daniel Heid <da...@extern.stromnetz-hamburg.de>
AuthorDate: Fri Nov 11 12:03:14 2022 +0100

    OLINGO-1577 Determine content type from HTTP response
---
 .../TransactionalPersistenceManagerImpl.java       | 16 +++--
 .../header/ODataErrorResponseChecker.java          | 24 ++++---
 .../communication/request/AbstractRequest.java     | 27 ++++++--
 .../org/apache/olingo/client/core/ErrorTest.java   | 52 +++++++--------
 .../olingo/commons/api/format/ContentType.java     | 37 ++++++++++-
 .../olingo/commons/api/format/ContentTypeTest.java | 77 +++++++++++++++++++++-
 6 files changed, 177 insertions(+), 56 deletions(-)

diff --git a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java
index 5b29506ae..a13a968bf 100644
--- a/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java
+++ b/ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/TransactionalPersistenceManagerImpl.java
@@ -18,11 +18,6 @@
  */
 package org.apache.olingo.ext.proxy.commons;
 
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
 import org.apache.olingo.client.api.communication.ODataServerErrorException;
 import org.apache.olingo.client.api.communication.request.ODataBatchableRequest;
 import org.apache.olingo.client.api.communication.request.ODataRequest;
@@ -37,10 +32,16 @@ import org.apache.olingo.client.api.communication.response.ODataEntityUpdateResp
 import org.apache.olingo.client.api.communication.response.ODataResponse;
 import org.apache.olingo.client.core.communication.header.ODataErrorResponseChecker;
 import org.apache.olingo.client.core.communication.request.batch.ODataChangesetResponseItem;
+import org.apache.olingo.commons.api.format.ContentType;
 import org.apache.olingo.ext.proxy.AbstractService;
 import org.apache.olingo.ext.proxy.api.ODataFlushException;
 import org.apache.olingo.ext.proxy.api.ODataResponseError;
 
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
 /**
  * {@link org.apache.olingo.ext.proxy.api.PersistenceManager} implementation using OData batch requests to implement
  * high-level user transactions: all read-write operations will be packed in a batch request to the OData service when
@@ -50,6 +51,8 @@ public class TransactionalPersistenceManagerImpl extends AbstractPersistenceMana
 
   private static final long serialVersionUID = -3320312269235907501L;
 
+  private static final ContentType DEFAULT_CONTENT_TYPE = ContentType.JSON;
+
   public TransactionalPersistenceManagerImpl(final AbstractService<?> factory) {
     super(factory);
   }
@@ -103,11 +106,12 @@ public class TransactionalPersistenceManagerImpl extends AbstractPersistenceMana
 
         final ODataResponse res = chgres.next();
         if (res.getStatusCode() >= 400) {
+          ContentType contentType = ContentType.fromAcceptHeader(request.getAccept());
           errors.add(new ODataResponseError(ODataErrorResponseChecker.checkResponse(
                   service.getClient(),
                   new ResponseStatusLine(res),
                   res.getRawResponse(),
-                  ((ODataRequest) request).getAccept()), index, requests.get(index)));
+                  contentType), index, requests.get(index)));
           if (!service.getClient().getConfiguration().isContinueOnError()) {
             throw new ODataFlushException(response.getStatusCode(), errors);
           }
diff --git a/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/header/ODataErrorResponseChecker.java b/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/header/ODataErrorResponseChecker.java
index 0c05af46c..5a78012f3 100644
--- a/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/header/ODataErrorResponseChecker.java
+++ b/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/header/ODataErrorResponseChecker.java
@@ -6,9 +6,9 @@
  * 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
@@ -18,11 +18,6 @@
  */
 package org.apache.olingo.client.core.communication.header;
 
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.Map;
-
 import org.apache.commons.io.IOUtils;
 import org.apache.http.StatusLine;
 import org.apache.olingo.client.api.ODataClient;
@@ -32,10 +27,14 @@ import org.apache.olingo.client.api.serialization.ODataDeserializerException;
 import org.apache.olingo.commons.api.ex.ODataError;
 import org.apache.olingo.commons.api.ex.ODataRuntimeException;
 import org.apache.olingo.commons.api.format.ContentType;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Map;
+
 public final class ODataErrorResponseChecker {
 
   protected static final Logger LOG = LoggerFactory.getLogger(ODataErrorResponseChecker.class);
@@ -49,7 +48,7 @@ public final class ODataErrorResponseChecker {
 
   public static ODataRuntimeException checkResponse(
       final ODataClient odataClient, final StatusLine statusLine, final InputStream entity,
-      final String accept) {
+      final ContentType contentType) {
 
     ODataRuntimeException result;
     InputStream entityForException = null;
@@ -57,10 +56,9 @@ public final class ODataErrorResponseChecker {
     if (entity == null) {
       result = new ODataClientErrorException(statusLine);
     } else {
-      final ContentType contentType = accept.contains("xml") ? ContentType.APPLICATION_ATOM_XML : ContentType.JSON;
 
       ODataError error = new ODataError();
-      if (!accept.contains("text/plain")) {
+      if (!contentType.isCompatible(ContentType.TEXT_PLAIN)) {
         try {
           byte[] bytes = IOUtils.toByteArray(entity);
           entityForException = new ByteArrayInputStream(bytes);
@@ -94,8 +92,8 @@ public final class ODataErrorResponseChecker {
         }
       }
 
-      if (statusLine.getStatusCode() >= 500 && error!= null && 
-          (error.getDetails() == null || error.getDetails().isEmpty()) && 
+      if (statusLine.getStatusCode() >= 500 && error!= null &&
+          (error.getDetails() == null || error.getDetails().isEmpty()) &&
           (error.getInnerError() == null || error.getInnerError().size() == 0)) {
         result = new ODataServerErrorException(statusLine, entityForException);
       } else {
diff --git a/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AbstractRequest.java b/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AbstractRequest.java
index 01631566a..03ef3ac9c 100644
--- a/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AbstractRequest.java
+++ b/lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AbstractRequest.java
@@ -15,9 +15,7 @@
  */
 package org.apache.olingo.client.core.communication.request;
 
-import java.io.IOException;
-
-import org.apache.http.Header;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.olingo.client.api.EdmEnabledODataClient;
@@ -25,16 +23,18 @@ import org.apache.olingo.client.api.ODataClient;
 import org.apache.olingo.client.api.communication.ODataClientErrorException;
 import org.apache.olingo.client.core.communication.header.ODataErrorResponseChecker;
 import org.apache.olingo.commons.api.ex.ODataRuntimeException;
+import org.apache.olingo.commons.api.format.ContentType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
+
 public abstract class AbstractRequest {
 
   /**
    * Logger.
    */
   protected static final Logger LOG = LoggerFactory.getLogger(AbstractRequest.class);
-  private static final String TEXT_CONTENT_TYPE = "text/plain";
 
   protected void checkRequest(final ODataClient odataClient, final HttpUriRequest request) {
     // If using and Edm enabled client, checks that the cached service root matches the request URI
@@ -53,14 +53,13 @@ public abstract class AbstractRequest {
           final ODataClient odataClient, final HttpResponse response, final String accept) {
 
     if (response.getStatusLine().getStatusCode() >= 400) {
-      Header contentTypeHeader = response.getEntity() != null ? response.getEntity().getContentType() : null;
+      final ContentType contentType = determineContentType(response, accept);
       try {
         final ODataRuntimeException exception = ODataErrorResponseChecker.checkResponse(
                 odataClient,
                 response.getStatusLine(),
                 response.getEntity() == null ? null : response.getEntity().getContent(),
-                    (contentTypeHeader != null && 
-                    contentTypeHeader.getValue().contains(TEXT_CONTENT_TYPE)) ? TEXT_CONTENT_TYPE : accept);
+                contentType);
         if (exception != null) {
           if (exception instanceof ODataClientErrorException) {
             ((ODataClientErrorException)exception).setHeaderInfo(response.getAllHeaders());
@@ -73,4 +72,18 @@ public abstract class AbstractRequest {
       }
     }
   }
+
+  private static ContentType determineContentType(HttpResponse response, String accept) {
+    if (response.getEntity() == null
+            || response.getEntity().getContentType() == null
+            || StringUtils.isBlank(response.getEntity().getContentType().getValue())) {
+      return ContentType.fromAcceptHeader(accept);
+    }
+    try {
+      return ContentType.create(response.getEntity().getContentType().getValue());
+    } catch (Exception exception) {
+      return ContentType.JSON;
+    }
+  }
+
 }
diff --git a/lib/client-core/src/test/java/org/apache/olingo/client/core/ErrorTest.java b/lib/client-core/src/test/java/org/apache/olingo/client/core/ErrorTest.java
index 05e91a0ce..efe10ae6e 100644
--- a/lib/client-core/src/test/java/org/apache/olingo/client/core/ErrorTest.java
+++ b/lib/client-core/src/test/java/org/apache/olingo/client/core/ErrorTest.java
@@ -18,17 +18,6 @@
  */
 package org.apache.olingo.client.core;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.io.ByteArrayInputStream;
-import java.io.InputStream;
-import java.util.Map;
-
 import org.apache.http.StatusLine;
 import org.apache.olingo.client.api.ODataClient;
 import org.apache.olingo.client.api.communication.ODataClientErrorException;
@@ -41,6 +30,17 @@ import org.apache.olingo.commons.api.ex.ODataRuntimeException;
 import org.apache.olingo.commons.api.format.ContentType;
 import org.junit.Test;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class ErrorTest extends AbstractTest {
 
   private ODataError error(final String name, final ContentType contentType) throws ODataDeserializerException {
@@ -55,7 +55,7 @@ public class ErrorTest extends AbstractTest {
     assertEquals("501", error.getCode());
     assertEquals("Unsupported functionality", error.getMessage());
     assertEquals("query", error.getTarget());
-    
+
     // verify details
     final ODataErrorDetail detail = error.getDetails().get(0);
     assertEquals("Code should be correct", "301", detail.getCode());
@@ -74,7 +74,7 @@ public class ErrorTest extends AbstractTest {
     assertEquals("innerError['context'] should be correct",
         "{\"key1\":\"for debug deployment only\"}", innerErr.get("context"));
     assertEquals("innerError['trace'] should be correct",
-        "[\"callmethod1 etc\",\"callmethod2 etc\"]", innerErr.get("trace"));    
+        "[\"callmethod1 etc\",\"callmethod2 etc\"]", innerErr.get("trace"));
   }
 
   @Test
@@ -89,9 +89,9 @@ public class ErrorTest extends AbstractTest {
     StatusLine statusLine = mock(StatusLine.class);
     when(statusLine.getStatusCode()).thenReturn(500);
     when(statusLine.toString()).thenReturn("Internal Server Error");
-    
+
     ODataClientErrorException exp = (ODataClientErrorException) ODataErrorResponseChecker.
-        checkResponse(odataClient, statusLine, entity, "Json");
+        checkResponse(odataClient, statusLine, entity, ContentType.JSON);
     assertTrue(exp.getMessage().contains("(500) Internal Server Error"));
     ODataError error = exp.getODataError();
     assertTrue(error.getMessage().startsWith("Internal Server Error"));
@@ -100,9 +100,9 @@ public class ErrorTest extends AbstractTest {
     assertEquals("\"Method does not support entities of specific type\"", error.getInnerError().get("message"));
     assertEquals("\"FaultException\"", error.getInnerError().get("type"));
     assertNull(error.getDetails());
-        
+
   }
-  
+
   @Test
   public void test2OLINGO1102() throws Exception {
     ODataClient odataClient = ODataClientFactory.getClient();
@@ -110,35 +110,35 @@ public class ErrorTest extends AbstractTest {
     StatusLine statusLine = mock(StatusLine.class);
     when(statusLine.getStatusCode()).thenReturn(500);
     when(statusLine.toString()).thenReturn("Internal Server Error");
-        
+
     ODataServerErrorException exp = (ODataServerErrorException) ODataErrorResponseChecker.
-        checkResponse(odataClient, statusLine, entity, "Json");
+        checkResponse(odataClient, statusLine, entity, ContentType.JSON);
     assertTrue(exp.getMessage().startsWith("Internal Server Error"));
   }
-  
+
   @Test
   public void testWithNull() throws Exception {
     ODataClient odataClient = ODataClientFactory.getClient();
     StatusLine statusLine = mock(StatusLine.class);
     when(statusLine.getStatusCode()).thenReturn(500);
     when(statusLine.toString()).thenReturn("Internal Server Error");
-        
+
     ODataRuntimeException exp = ODataErrorResponseChecker.
-        checkResponse(odataClient, statusLine, null, "Json");
+        checkResponse(odataClient, statusLine, null, ContentType.JSON);
     assertTrue(exp.getMessage().startsWith("Internal Server Error"));
   }
-  
+
   @Test
   public void testExpTextMsg403() throws Exception {
     ODataClient odataClient = ODataClientFactory.getClient();
-    InputStream entity = new ByteArrayInputStream("CSRF Validation Exception".getBytes()); 
+    InputStream entity = new ByteArrayInputStream("CSRF Validation Exception".getBytes());
     StatusLine statusLine = mock(StatusLine.class);
     when(statusLine.getStatusCode()).thenReturn(403);
     when(statusLine.toString()).thenReturn("Validation Exception");
     when(statusLine.getReasonPhrase()).thenReturn("Forbidden");
-    
+
     ODataClientErrorException exp = (ODataClientErrorException) ODataErrorResponseChecker.
-        checkResponse(odataClient, statusLine, entity, "text/plain");
+        checkResponse(odataClient, statusLine, entity, ContentType.TEXT_PLAIN);
     assertEquals(exp.getStatusLine().getStatusCode(), 403);
     ODataError error = exp.getODataError();
     assertTrue(error.getMessage().equals("CSRF Validation Exception"));
diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java
index 892c47452..89e43f415 100644
--- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java
+++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java
@@ -6,9 +6,9 @@
  * 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
@@ -204,6 +204,39 @@ public final class ContentType {
     TypeUtil.parseParameters(params, parameters);
   }
 
+  /**
+   * Uses the first MIME type from the accept header to determine the content type.
+   *
+   * @param accept The accept header content, e.g. text/html,application/xhtml+xml,application/xml, may be null
+   * @return The content type according to the accept header's first MIME type. Defaults to application/json if the
+   * accept header does not contain valid information. Never null.
+   */
+  public static ContentType fromAcceptHeader(String accept) {
+    if (accept == null || accept.trim().isEmpty()) {
+      return JSON;
+    }
+    String acceptType = accept.split(",")[0];
+    if (acceptType == null || acceptType.trim().isEmpty()) {
+      return JSON;
+    }
+    int semicolonIndex = acceptType.indexOf(';');
+    String cleanedAcceptType;
+    if (semicolonIndex == -1) {
+      cleanedAcceptType = acceptType.trim();
+    } else {
+      cleanedAcceptType = acceptType.trim().substring(0, semicolonIndex).trim();
+      if (cleanedAcceptType.trim().isEmpty()) {
+        return JSON;
+      }
+    }
+
+    try {
+      return create(cleanedAcceptType);
+    } catch (Exception exception) {
+      return accept.contains("xml") ? APPLICATION_ATOM_XML : JSON;
+    }
+  }
+
   /** Gets the type of this content type. */
   public String getType() {
     return type;
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 65750e1b7..6040a9279 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
@@ -18,6 +18,8 @@
  */
 package org.apache.olingo.commons.api.format;
 
+import org.junit.Test;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -26,8 +28,6 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import org.junit.Test;
-
 public class ContentTypeTest {
 
   @Test
@@ -120,4 +120,77 @@ public class ContentTypeTest {
       assertNotNull(e);
     }
   }
+
+  @Test
+  public void firstFromValidMultiAcceptHeader() {
+
+    ContentType contentType = ContentType.fromAcceptHeader("application/xml ; q=0.9, application/xhtml+xml,*/*;q=0.8 ");
+
+    assertEquals(ContentType.APPLICATION_XML, contentType);
+
+  }
+
+  @Test
+  public void missingMimeTypefromAcceptHeaderDefaultsToJson() {
+
+    ContentType contentType = ContentType.fromAcceptHeader(";q=0.9");
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
+  @Test
+  public void fromValidSingleAcceptHeader() {
+
+    ContentType contentType = ContentType.fromAcceptHeader("application/xml");
+
+    assertEquals(ContentType.APPLICATION_XML, contentType);
+
+  }
+
+  @Test
+  public void fromAcceptHeaderDefaultsToJsonIfNull() {
+
+    ContentType contentType = ContentType.fromAcceptHeader(null);
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
+  @Test
+  public void fromAcceptHeaderDefaultsToJsonIfEmpty() {
+
+    ContentType contentType = ContentType.fromAcceptHeader("");
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
+  @Test
+  public void fromAcceptHeaderDefaultsToJsonIfBlank() {
+
+    ContentType contentType = ContentType.fromAcceptHeader(" ");
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
+  @Test
+  public void fromAcceptHeaderDefaultsToJsonIfInvalid() {
+
+    ContentType contentType = ContentType.fromAcceptHeader("invalid");
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
+  @Test
+  public void fromAcceptHeaderDefaultsToJsonIfFirstValueBlank() {
+
+    ContentType contentType = ContentType.fromAcceptHeader("  ,text/plain ");
+
+    assertEquals(ContentType.JSON, contentType);
+
+  }
+
 }