PLAT-5219: Better retries with concurrent requests (#52)

* Better retries with concurrent requests

Previously, if two or more threads completed with 401/403 errors concurrently before
the `updateAccessToken` happened, only one of them would be retried.

Now, they'll all be retried. That multiple requests have to be retried due to the same
expired token is an unavoidable side-effect of the fact that we allow multiple
concurrent requests (and that we only know that the token is expired as a result from
the request itself), and we can live with that tradeoff.

[PLAT-5219](https://vertexvis.atlassian.net/browse/PLAT-5219)

* version update

* and another one, i guess

Author: sourada-e5

README.md

@@ -17,21 +17,21 @@ The client can be used with Java 1.8+ and pulled into Maven or Gradle projects.
 <dependency>
   <groupId>com.vertexvis</groupId>
   <artifactId>api-client-java</artifactId>
-  <version>0.8.2</version>
+  <version>0.8.3</version>
   <scope>compile</scope>
 </dependency>
 ```
 
 ### Gradle
 
 ```groovy
-compile "com.vertexvis:api-client-java:0.8.2"
+compile "com.vertexvis:api-client-java:0.8.3"
 ```
 
 ### Sbt
 
 ```sbt
-libraryDependencies += "com.vertexvis" % "api-client-java" % "0.8.2"
+libraryDependencies += "com.vertexvis" % "api-client-java" % "0.8.3"
 ```
 
 ### Others
@@ -44,7 +44,7 @@ mvn clean package
 
 Then manually install the following JARs.
 
-- `target/api-client-java-0.8.2.jar`
+- `target/api-client-java-0.8.3.jar`
 - `target/lib/*.jar`
 
 ## Usage

build.gradle

@@ -7,7 +7,7 @@ plugins {
 }
 
 group = 'com.vertexvis'
-version = '0.8.2'
+version = '0.8.3'
 
 repositories {
     mavenCentral()
@@ -76,19 +76,20 @@ javadoc {
 }
 
 dependencies {
-    implementation 'io.swagger:swagger-annotations:1.6.3'
+    implementation 'io.swagger:swagger-annotations:1.6.14'
     implementation "com.google.code.findbugs:jsr305:3.0.2"
-    implementation 'com.squareup.okhttp3:okhttp:4.9.3'
-    implementation 'com.squareup.okhttp3:logging-interceptor:4.9.3'
-    implementation 'com.google.code.gson:gson:2.8.9'
-    implementation 'io.gsonfire:gson-fire:1.8.5'
+    implementation 'com.squareup.okhttp3:okhttp:4.12.0'
+    implementation 'com.squareup.okhttp3:logging-interceptor:4.12.0'
+    implementation 'com.google.code.gson:gson:2.10'
+    implementation 'io.gsonfire:gson-fire:1.9.0'
     implementation 'org.apache.oltu.oauth2:org.apache.oltu.oauth2.client:1.0.1' // 1.0.2 doesn't work with okkhttp3
-    implementation 'org.apache.commons:commons-lang3:3.12.0'
-    implementation 'javax.annotation:javax.annotation-api:1.3.2'
-    implementation 'org.openapitools:jackson-databind-nullable:0.2.2'
-    implementation 'info.picocli:picocli:4.7.0'
-    testImplementation(platform('org.junit:junit-bom:5.8.2'))
-    testImplementation('org.junit.jupiter:junit-jupiter:5.8.2')
+    implementation 'org.apache.commons:commons-lang3:3.15.0'
+    implementation 'javax.annotation:javax.annotation-api:1.3'
+    implementation 'org.openapitools:jackson-databind-nullable:0.2.6'
+    implementation 'info.picocli:picocli:4.7.6'
+    testImplementation(platform('org.junit:junit-bom:5.10.3'))
+    testImplementation('org.junit.jupiter:junit-jupiter:5.10.3')
+    testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0")
 }
 
 test {

src/main/java/com/vertexvis/ApiClient.java

@@ -207,7 +207,7 @@ private void init() {
         json = new JSON();
 
         // Set default User-Agent.
-        setUserAgent("vertex-api-client-java/0.8.1");
+        setUserAgent("vertex-api-client-java/0.8.3");
 
         authentications = new HashMap<String, Authentication>();
     }

src/main/java/com/vertexvis/auth/OAuthOkHttpClient.java

@@ -18,7 +18,7 @@
 import java.util.Map.Entry;
 
 public class OAuthOkHttpClient implements HttpClient {
-    private OkHttpClient client;
+    private final OkHttpClient client;
 
     public OAuthOkHttpClient() {
         this.client = new OkHttpClient();

src/main/java/com/vertexvis/auth/RetryingOAuth.java

@@ -104,10 +104,9 @@ private Response retryingIntercept(Chain chain, boolean updateTokenAndRetryOnAut
               updateTokenAndRetryOnAuthorizationFailure
       ) {
         try {
-          if (updateAccessToken(requestAccessToken)) {
-            response.body().close();
-            return retryingIntercept(chain, false);
-          }
+          updateAccessToken(requestAccessToken);
+          response.body().close();
+          return retryingIntercept(chain, false);
         } catch (Exception e) {
           response.body().close();
           throw e;
@@ -119,24 +118,21 @@ private Response retryingIntercept(Chain chain, boolean updateTokenAndRetryOnAut
     }
   }
 
-  /*
-   * Returns true if the access token has been updated
-   */
-  public synchronized boolean updateAccessToken(String requestAccessToken) throws IOException {
+  private synchronized void updateAccessToken(String requestAccessToken) throws IOException {
+    // if we don't have a token at all, we go get one
+    // if we are here and requestAccessToken is not equal to getAccessToken(), that means
+    // some other thread has already updated the token and we can just continue on to retry with the new token.
     if (getAccessToken() == null || getAccessToken().equals(requestAccessToken)) {
       try {
         OAuthJSONAccessTokenResponse accessTokenResponse =
             oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage());
         if (accessTokenResponse != null && accessTokenResponse.getAccessToken() != null) {
           setAccessToken(accessTokenResponse.getAccessToken());
-          return !getAccessToken().equals(requestAccessToken);
         }
       } catch (OAuthSystemException | OAuthProblemException e) {
         throw new IOException(e);
       }
     }
-
-    return false;
   }
 
   public TokenRequestBuilder getTokenRequestBuilder() {

src/test/java/com/vertexvis/auth/RetryingOAuthTest.java

@@ -0,0 +1,79 @@
+package com.vertexvis.auth;
+
+import com.vertexvis.ApiClient;
+import com.vertexvis.ApiException;
+import com.vertexvis.api.PartRevisionsApi;
+import okhttp3.Call;
+import okhttp3.Request;
+import okhttp3.mockwebserver.Dispatcher;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class RetryingOAuthTest {
+    final static int numThreads = 10;
+    private static void startServer(MockWebServer server) throws IOException {
+        server.setDispatcher(new Dispatcher()
+        {
+            static final AtomicInteger numCalls = new AtomicInteger();
+
+            @NotNull
+            @Override
+            public MockResponse dispatch(@NotNull RecordedRequest recordedRequest) throws InterruptedException {
+                Thread.sleep(200);
+                var token = numCalls.incrementAndGet() > (numThreads / 2) ? "1" : "0";
+                if (Objects.equals(recordedRequest.getPath(), "/oauth2/token")) {
+                    return new MockResponse().addHeader("Content-Type", "application/json").setBody("{\"access_token\": \"" + token + "\"}");
+                }
+                if (!recordedRequest.getHeaders().get("Authorization").endsWith(token)) {
+                    return new MockResponse().setResponseCode(HttpURLConnection.HTTP_UNAUTHORIZED).setBody("{\"error\": \"invalid_token\"}");
+                }
+                return new MockResponse().setBody("{\"data\": {\"id\": \"" + UUID.randomUUID().toString() + "\"}}");
+            }
+        });
+        server.start();
+    }
+
+    @Test
+    public void multithreading() throws ApiException, IOException {
+        final AtomicInteger numFails = new AtomicInteger();
+        try (var server = new MockWebServer()) {
+            startServer(server);
+
+            final String baseUrl = server.url("/api").toString();
+            var client = new ApiClient(baseUrl, "clientid", "clientsecret", new HashMap<>());
+            var prs = new PartRevisionsApi(client);
+            Thread[] threads = new Thread[numThreads];
+            for (var i = 0; i < numThreads; i++) {
+                threads[i] = new Thread(() -> {
+                    try {
+                        prs.deletePartRevision(UUID.randomUUID());
+                    } catch (Exception e) {
+                        numFails.incrementAndGet();
+                    }
+                });
+               threads[i].start();
+            }
+
+            for (var i = 0; i < numThreads; i++) {
+                threads[i].join();
+            }
+
+            Assertions.assertEquals(0, numFails.get());
+        }
+        catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+
+    }
+}