[OLINGO-1411] Better header parsing

* [OLINGO-1411] Better header parsing

lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AsyncBatchRequestWrapperImpl.java

@@ -1,18 +1,18 @@
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
+ * or more contributor license agreements. See the NOTICE file
  * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
+ * 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
+ * with the License. You may obtain a copy of the License at
  *
- *   http://www.apache.org/licenses/LICENSE-2.0
+ * 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
+ * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
  */
@@ -34,7 +34,7 @@
 import org.apache.olingo.commons.api.http.HttpHeader;
 
 public class AsyncBatchRequestWrapperImpl extends AsyncRequestWrapperImpl<ODataBatchResponse>
-        implements AsyncBatchRequestWrapper {
+    implements AsyncBatchRequestWrapper {
 
   private BatchManager batchManager;
 
@@ -73,7 +73,7 @@ public AsyncResponseWrapper<ODataBatchResponse> execute() {
   }
 
   public class AsyncResponseWrapperImpl
-          extends AsyncRequestWrapperImpl<ODataBatchResponse>.AsyncResponseWrapperImpl {
+      extends AsyncRequestWrapperImpl<ODataBatchResponse>.AsyncResponseWrapperImpl {
 
     /**
      * Constructor.
@@ -100,7 +100,7 @@ private void retrieveMonitorDetails(final ODataBatchResponse res) {
 
       headers = res.getHeader(HttpHeader.RETRY_AFTER);
       if (headers != null && !headers.isEmpty()) {
-        this.retryAfter = Integer.parseInt(headers.iterator().next());
+        this.retryAfter = parseReplyAfter(headers.iterator().next());
       }
 
       headers = res.getHeader(HttpHeader.PREFERENCE_APPLIED);

lib/client-core/src/main/java/org/apache/olingo/client/core/communication/request/AsyncRequestWrapperImpl.java

@@ -141,11 +141,14 @@ protected HttpResponse doExecute() {
 
   public class AsyncResponseWrapperImpl implements AsyncResponseWrapper<R> {
 
+    static final int DEFAULT_RETRY_AFTER = 5;
+    static final int MAX_RETRY_AFTER = 10;
+
     protected URI location = null;
 
     protected R response = null;
 
-    protected int retryAfter = 5;
+    protected int retryAfter = DEFAULT_RETRY_AFTER;
 
     protected boolean preferenceApplied = false;
 
@@ -196,12 +199,12 @@ public R getODataResponse() {
 
           final Header[] headers = res.getHeaders(HttpHeader.RETRY_AFTER);
           if (ArrayUtils.isNotEmpty(headers)) {
-            this.retryAfter = Integer.parseInt(headers[0].getValue());
+            this.retryAfter = parseReplyAfter(headers[0].getValue());
           }
 
           try {
             // wait for retry-after
-            Thread.sleep((long)retryAfter * 1000);
+            Thread.sleep((long) retryAfter * 1000);
           } catch (InterruptedException ignore) {
             // ignore
           }
@@ -219,6 +222,21 @@ public R getODataResponse() {
       return response;
     }
 
+    int parseReplyAfter(String value) {
+      if (value == null || value.isEmpty()) {
+        return DEFAULT_RETRY_AFTER;
+
+      try {
+        int n = Integer.parseInt(value);
+        if (n < 0) {
+          return DEFAULT_RETRY_AFTER;
+        }
+        return Math.min(n, MAX_RETRY_AFTER);
+      } catch (NumberFormatException e) {
+        return DEFAULT_RETRY_AFTER;
+      }
+    }
+
     @Override
     public ODataDeleteResponse delete() {
       final ODataDeleteRequest deleteRequest = odataClient.getCUDRequestFactory().getDeleteRequest(location);
@@ -264,7 +282,7 @@ private void retrieveMonitorDetails(final HttpResponse res) {
 
       headers = res.getHeaders(HttpHeader.RETRY_AFTER);
       if (ArrayUtils.isNotEmpty(headers)) {
-        this.retryAfter = Integer.parseInt(headers[0].getValue());
+        this.retryAfter = parseReplyAfter(headers[0].getValue());
       }
 
       headers = res.getHeaders(HttpHeader.PREFERENCE_APPLIED);

lib/client-core/src/test/java/org/apache/olingo/client/core/communication/request/AsyncRequestWrapperTest.java

@@ -1,36 +1,55 @@
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
+ * or more contributor license agreements. See the NOTICE file
  * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
+ * 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
+ * with the License. You may obtain a copy of the License at
  *
- *   http://www.apache.org/licenses/LICENSE-2.0
+ * 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
+ * KIND, either express or implied. See the License for the
  * specific language governing permissions and limitations
  * under the License.
  */
 package org.apache.olingo.client.core.communication.request;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpResponseFactory;
+import org.apache.http.HttpVersion;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.impl.DefaultHttpResponseFactory;
+import org.apache.http.message.BasicStatusLine;
+import org.apache.olingo.client.api.Configuration;
 import org.apache.olingo.client.api.ODataClient;
 import org.apache.olingo.client.api.communication.request.ODataBatchableRequest;
+import org.apache.olingo.client.api.communication.response.AsyncResponseWrapper;
+import org.apache.olingo.client.api.communication.response.ODataResponse;
 import org.apache.olingo.client.api.domain.ClientInvokeResult;
+import org.apache.olingo.client.api.http.HttpClientFactory;
+import org.apache.olingo.client.api.http.HttpUriRequestFactory;
 import org.apache.olingo.client.core.ODataClientFactory;
 import org.apache.olingo.client.core.communication.request.AsyncRequestWrapperImpl.AsyncResponseWrapperImpl;
 import org.apache.olingo.client.core.communication.request.batch.ODataBatchRequestImpl;
 import org.apache.olingo.client.core.communication.request.invoke.ODataInvokeRequestImpl;
+import org.apache.olingo.commons.api.http.HttpHeader;
 import org.apache.olingo.commons.api.http.HttpMethod;
 import org.junit.Test;
 
@@ -41,25 +60,25 @@ public void testBatchReq() throws URISyntaxException {
 
     ODataClient client = ODataClientFactory.getClient();
     URI uri = new URI("localhost:8080");
-    AsyncBatchRequestWrapperImpl req = new AsyncBatchRequestWrapperImpl(client, 
+    AsyncBatchRequestWrapperImpl req = new AsyncBatchRequestWrapperImpl(client,
         client.getBatchRequestFactory().getBatchRequest("root"));
     assertNotNull(req.addChangeset());
     ODataBatchableRequest request = new ODataInvokeRequestImpl<ClientInvokeResult>(
         client, ClientInvokeResult.class, HttpMethod.GET, uri);
-    req.addRetrieve(request );
+    req.addRetrieve(request);
     req.addOutsideUpdate(request);
     assertNotNull(client.getAsyncRequestFactory().getAsyncRequestWrapper(request));
     ODataBatchRequestImpl batchRequest = new ODataBatchRequestImpl(client, uri);
-    assertNotNull(client.getAsyncRequestFactory().getAsyncBatchRequestWrapper(batchRequest ));
+    assertNotNull(client.getAsyncRequestFactory().getAsyncBatchRequestWrapper(batchRequest));
     assertNotNull(req.wait(10));
   }
-  
+
   @Test
   public void testReq() throws URISyntaxException {
 
     ODataClient client = ODataClientFactory.getClient();
     URI uri = new URI("localhost:8080");
-    AsyncRequestWrapperImpl req = new AsyncRequestWrapperImpl(client, 
+    AsyncRequestWrapperImpl req = new AsyncRequestWrapperImpl(client,
         client.getBatchRequestFactory().getBatchRequest("root"));
     assertNotNull(req);
     ODataBatchableRequest request = new ODataInvokeRequestImpl<ClientInvokeResult>(
@@ -70,20 +89,93 @@ public void testReq() throws URISyntaxException {
     AsyncResponseWrapperImpl res = req.new AsyncResponseWrapperImpl();
     res.forceNextMonitorCheck(uri);
   }
-
+
+  private AsyncRequestWrapperImpl createAsyncRequestWrapperImplWithRetryAfter(int retryAfter)
+      throws IOException {
+
+    HttpClient httpClient = mock(HttpClient.class);
+    ODataClient oDataClient = mock(ODataClient.class);
+    Configuration configuration = mock(Configuration.class);
+    HttpClientFactory httpClientFactory = mock(HttpClientFactory.class);
+    HttpUriRequestFactory httpUriRequestFactory = mock(HttpUriRequestFactory.class);
+    HttpUriRequest httpUriRequest = mock(HttpUriRequest.class);
+
+    when(oDataClient.getConfiguration()).thenReturn(configuration);
+    when(configuration.getHttpClientFactory()).thenReturn(httpClientFactory);
+    when(configuration.getHttpUriRequestFactory()).thenReturn(httpUriRequestFactory);
+    when(httpClientFactory.create(any(), any())).thenReturn(httpClient);
+    when(httpUriRequestFactory.create(any(), any())).thenReturn(httpUriRequest);
+
+    HttpResponseFactory factory = new DefaultHttpResponseFactory();
+    HttpResponse firstResponse = factory.newHttpResponse(
+        new BasicStatusLine(HttpVersion.HTTP_1_1, 202, null), null);
+    firstResponse.addHeader(HttpHeader.LOCATION, "http://localhost/monitor");
+    firstResponse.addHeader(HttpHeader.RETRY_AFTER, String.valueOf(retryAfter));
+    when(httpClient.execute(any(HttpUriRequest.class))).thenReturn(firstResponse);
+
+    AbstractODataRequest oDataRequest = mock(AbstractODataRequest.class);
+    ODataResponse oDataResponse = mock(ODataResponse.class);
+    when(oDataRequest.getResponseTemplate()).thenReturn(oDataResponse);
+    when(oDataResponse.initFromHttpResponse(any(HttpResponse.class))).thenReturn(null);
+
+    return new AsyncRequestWrapperImpl(oDataClient, oDataRequest);
+  }
+
   @Test
-  public void testWrapper(){
+  public void testTooBigRetryAfter() throws IOException {
+
+    AsyncRequestWrapperImpl req = createAsyncRequestWrapperImplWithRetryAfter(Integer.MAX_VALUE);
+    AsyncResponseWrapper wrappedResponse = req.execute();
+    assertTrue(wrappedResponse instanceof AsyncResponseWrapperImpl);
+    AsyncResponseWrapperImpl wrappedResponseImpl = (AsyncResponseWrapperImpl) wrappedResponse;
+    assertEquals(AsyncResponseWrapperImpl.MAX_RETRY_AFTER, wrappedResponseImpl.retryAfter);
+  }
+
+  @Test
+  public void testZeroRetryAfter() throws IOException {
+
+    AsyncRequestWrapperImpl req = createAsyncRequestWrapperImplWithRetryAfter(0);
+    AsyncResponseWrapper wrappedResponse = req.execute();
+    assertTrue(wrappedResponse instanceof AsyncResponseWrapperImpl);
+    AsyncResponseWrapperImpl wrappedResponseImpl = (AsyncResponseWrapperImpl) wrappedResponse;
+    assertEquals(0, wrappedResponseImpl.retryAfter);
+  }
+
+  @Test
+  public void testNegativeRetryAfter() throws IOException {
+
+    AsyncRequestWrapperImpl req = createAsyncRequestWrapperImplWithRetryAfter(-1);
+    AsyncResponseWrapper wrappedResponse = req.execute();
+    assertTrue(wrappedResponse instanceof AsyncResponseWrapperImpl);
+    AsyncResponseWrapperImpl wrappedResponseImpl = (AsyncResponseWrapperImpl) wrappedResponse;
+    assertEquals(AsyncResponseWrapperImpl.DEFAULT_RETRY_AFTER, wrappedResponseImpl.retryAfter);
+  }
+
+  @Test
+  public void testRetryAfter() throws IOException {
+
+    int retryAfter = 7;
+    assertNotEquals(retryAfter, AsyncResponseWrapperImpl.DEFAULT_RETRY_AFTER);
+    AsyncRequestWrapperImpl req = createAsyncRequestWrapperImplWithRetryAfter(retryAfter);
+    AsyncResponseWrapper wrappedResponse = req.execute();
+    assertTrue(wrappedResponse instanceof AsyncResponseWrapperImpl);
+    AsyncResponseWrapperImpl wrappedResponseImpl = (AsyncResponseWrapperImpl) wrappedResponse;
+    assertEquals(retryAfter, wrappedResponseImpl.retryAfter);
+  }
+
+  @Test
+  public void testWrapper() {
 
     Wrapper wrap = new Wrapper();
     wrap.setWrapped("test");
     assertEquals("test", wrap.getWrapped());
   }
-  
+
   @Test
-  public void testException(){
+  public void testException() {
 
-    AsyncRequestException  ex = new AsyncRequestException ("Exception");
+    AsyncRequestException ex = new AsyncRequestException("Exception");
     assertEquals("Exception", ex.getMessage());
   }
-  
+
 }