Fix API compatability between Spring 5 and 6

Spring changed the API of ReponseEntity with 6.x so that code compiled with Spring 5.x will not work anymore. As a workaround we use getStatusCodeValue instead of getStatusCode to make the library work with both spring versions. Also cleanup error handling of CompletableFuture data loading Closes #59
porscheinformatik · Jan 14, 2023 · 51a7319 · 51a7319
1 parent 6dd4bae
commit 51a7319
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 65 deletions.
diff --git a/src/main/java/at/porscheinformatik/weblate/spring/WeblateMessageSource.java b/src/main/java/at/porscheinformatik/weblate/spring/WeblateMessageSource.java
@@ -1,5 +1,6 @@
 package at.porscheinformatik.weblate.spring;
 
+import static java.util.Collections.emptySet;
 import static java.util.Collections.singletonList;
 
 import java.net.URISyntaxException;
@@ -34,7 +35,6 @@
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.util.StringUtils;
-import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestTemplate;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -178,23 +178,24 @@ public void setInitialCacheTimestamp(long initialCacheTimestamp) {
    * @return if async enabled
    */
   public boolean isAsync() {
-      return async;
+    return async;
   }
 
   /**
-   * Use async loading - all operations will be performed in a single threaded {@link ExecutorService}.
+   * Use async loading - all operations will be performed in a single threaded
+   * {@link ExecutorService}.
    * 
    * @param async if true loading will be performed asynchronously
    */
   public void setAsync(boolean async) {
-      this.async = async;
+    this.async = async;
   }
 
   /**
    * @return all existing locales of the configured weblate component
    */
   public Set<Locale> getExistingLocales() {
-    return this.existingLocales.keySet();
+    return this.existingLocales == null ? emptySet() : this.existingLocales.keySet();
   }
 
   /**
@@ -203,7 +204,14 @@ public Set<Locale> getExistingLocales() {
    * This does not clear the cached translations.
    */
   public void reloadExistingLocales() {
-    CompletableFuture<Void> loadTask = CompletableFuture.runAsync(() -> loadCodes(), executor);
+    CompletableFuture<Void> loadTask = CompletableFuture
+        .runAsync(this::loadCodes, executor)
+        .handle((val, e) -> {
+          if (e != null) {
+            logger.warn("Error reloading locales", e);
+          }
+          return val;
+        });
     if (!async) {
       loadTask.join();
     }
@@ -359,14 +367,19 @@ private Properties loadTranslations(Locale locale, boolean reload) {
    *                   loaded (optional)
    */
   private void loadTranslation(Locale language, Properties properties, long timestamp) {
-    CompletableFuture<Map<Locale, String>> loadCodesTask = CompletableFuture.supplyAsync(() -> loadCodes());
+    CompletableFuture<Map<Locale, String>> loadCodesTask = CompletableFuture.supplyAsync(this::loadCodes);
 
     CompletableFuture<Void> loadTask = loadCodesTask.thenCompose(locales -> {
       String lang = existingLocales.get(language);
       if (lang != null) {
-        return loadTranslation(lang, properties, timestamp);
+        return CompletableFuture.runAsync(() -> loadTranslation(lang, properties, timestamp), executor);
       }
       return CompletableFuture.completedStage(null);
+    }).handle((val, e) -> {
+      if (e != null) {
+        logger.warn("Error loading translation for locale " + language, e);
+      }
+      return val;
     });
 
     if (!async) {
@@ -381,76 +394,70 @@ private static String formatTimestampIso(long timestamp) {
     return df.format(new Date(timestamp));
   }
 
-  private CompletableFuture<Void> loadTranslation(String code, Properties properties, long timestamp) {
-    return CompletableFuture.runAsync(() -> {
-      String currentQuery = query;
-      if (timestamp > 0L) {
-        String timestampStr = formatTimestampIso(timestamp);
-        currentQuery += " AND (added:>=" + timestampStr + " OR changed:>=" + timestampStr + ")";
-      }
+  private void loadTranslation(String code, Properties properties, long timestamp) {
+    String currentQuery = query;
+    if (timestamp > 0L) {
+      String timestampStr = formatTimestampIso(timestamp);
+      currentQuery += " AND (added:>=" + timestampStr + " OR changed:>=" + timestampStr + ")";
+    }
+    RequestEntity<Void> request = RequestEntity
+        .get(baseUrl + "/api/translations/{project}/{component}/{languageCode}/units/?q={query}",
+            project, component, code, currentQuery)
+        .accept(MediaType.APPLICATION_JSON)
+        .build();
 
-      try {
-        RequestEntity<Void> request = RequestEntity
-            .get(baseUrl + "/api/translations/{project}/{component}/{languageCode}/units/?q={query}",
-                project, component, code, currentQuery)
-            .accept(MediaType.APPLICATION_JSON)
-            .build();
-
-        if (restTemplate == null) {
-          restTemplate = createRestTemplate();
-        }
-
-        UnitsResponse responseBody;
-        while (true) {
-          ResponseEntity<UnitsResponse> response = restTemplate.exchange(request, UnitsResponse.class);
-          responseBody = response.getBody();
-          if (!response.getStatusCode().is2xxSuccessful() || responseBody == null) {
-            logger.warn(String.format("Got empty or non-200 response (status=%s, body=%s)", response.getStatusCode(),
-                response.getBody()));
-            break;
-          }
+    if (restTemplate == null) {
+      restTemplate = createRestTemplate();
+    }
 
-          for (Unit unit : responseBody.results) {
-            properties.put(unit.code, unit.target[0]);
-          }
+    UnitsResponse body;
+    while (true) {
+      ResponseEntity<UnitsResponse> response = restTemplate.exchange(request, UnitsResponse.class);
+      body = response.getBody();
+      if (response.getStatusCodeValue() < 200 || response.getStatusCodeValue() >= 300 || body == null) {
+        logger.warn(String.format("Got empty or non-200 response (status=%s, body=%s)", response.getStatusCode(),
+            response.getBody()));
+        break;
+      }
 
-          if (responseBody.next == null) {
-            break;
-          }
+      for (Unit unit : body.results) {
+        properties.put(unit.code, unit.target[0]);
+      }
+
+      if (body.next == null) {
+        break;
+      }
 
-          request = RequestEntity.get(responseBody.next.toURI()).accept(MediaType.APPLICATION_JSON).build();
-        }
-      } catch (RestClientException | URISyntaxException e) {
-        logger.warn("Could not load translations (code=" + code + ")", e);
+      try {
+        request = RequestEntity.get(body.next.toURI()).accept(MediaType.APPLICATION_JSON).build();
+      } catch (URISyntaxException e) {
+        throw new RuntimeException(e);
       }
-    }, executor);
+    }
   }
 
   private Map<Locale, String> loadCodes() {
     if (existingLocales != null) {
       return existingLocales;
     }
 
-    try {
-      RequestEntity<Void> request = RequestEntity.get(baseUrl + "/api/projects/{project}/languages/", project)
-          .accept(MediaType.APPLICATION_JSON).build();
+    RequestEntity<Void> request = RequestEntity.get(baseUrl + "/api/projects/{project}/languages/", project)
+        .accept(MediaType.APPLICATION_JSON).build();
 
-      if (restTemplate == null) {
-        restTemplate = createRestTemplate();
-      }
+    if (restTemplate == null) {
+      restTemplate = createRestTemplate();
+    }
 
-      ResponseEntity<List<Map<String, Object>>> response = restTemplate.exchange(request, LIST_MAP_STRING_OBJECT);
-      List<Map<String, Object>> body = response.getBody();
-      if (response.getStatusCode().is2xxSuccessful() && body != null) {
-        existingLocales = body.stream()
-            .filter(this::containsTranslations)
-            .map(this::extractCode)
-            .filter(Objects::nonNull)
-            .collect(Collectors.toMap(this::deriveLocaleFromCode, Function.identity()));
-      }
-    } catch (RestClientException e) {
-      logger.warn("Could not load languages", e);
+    ResponseEntity<List<Map<String, Object>>> response = restTemplate.exchange(request, LIST_MAP_STRING_OBJECT);
+    List<Map<String, Object>> body = response.getBody();
+    if (response.getStatusCodeValue() >= 200 && response.getStatusCodeValue() < 300 && body != null) {
+      existingLocales = body.stream()
+          .filter(this::containsTranslations)
+          .map(this::extractCode)
+          .filter(Objects::nonNull)
+          .collect(Collectors.toMap(this::deriveLocaleFromCode, Function.identity()));
     }
+
     return existingLocales;
   }
 

diff --git a/src/test/java/at/porscheinformatik/weblate/spring/WeblateMessageSourceTest.java b/src/test/java/at/porscheinformatik/weblate/spring/WeblateMessageSourceTest.java
@@ -3,6 +3,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 import static org.springframework.test.web.client.match.MockRestRequestMatchers.method;
@@ -18,6 +19,7 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 import org.springframework.context.MessageSource;
+import org.springframework.context.NoSuchMessageException;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
@@ -65,17 +67,25 @@ void tearDown() {
   }
 
   private void mockGetLocales() {
+    mockGetLocales(HttpStatus.OK);
+  }
+
+  private void mockGetLocales(HttpStatus status) {
     mockServer.expect(ExpectedCount.once(),
         requestTo("http://localhost:8080/api/projects/test-project/languages/")).andRespond(
-            withStatus(HttpStatus.OK)
+            withStatus(status)
                 .contentType(MediaType.APPLICATION_JSON)
                 .body("[{\"code\":\"en\", \"translated\":1},{\"code\":\"de\"}]"));
   }
 
   private void mockResponse(String body) {
+    mockResponse(body, HttpStatus.OK);
+  }
+
+  private void mockResponse(String body, HttpStatus status) {
     try {
       String url = "http://localhost:8080/api/translations/test-project/test-comp/en/units/";
-      DefaultResponseCreator response = withStatus(HttpStatus.OK).contentType(MediaType.APPLICATION_JSON);
+      DefaultResponseCreator response = withStatus(status).contentType(MediaType.APPLICATION_JSON);
       if (body != null) {
         response.body(body);
       }
@@ -96,6 +106,14 @@ void reloadExistingLocales() {
     assertFalse(messageSource.getExistingLocales().contains(Locale.GERMAN));
   }
 
+  @Test
+  void handleErrorInReloadExistingLocales() {
+    mockGetLocales(HttpStatus.INTERNAL_SERVER_ERROR);
+
+    messageSource.reloadExistingLocales();
+    assertTrue(messageSource.getExistingLocales().isEmpty());
+  }
+
   @Test
   void simpleCase() {
     mockGetLocales();
@@ -192,4 +210,19 @@ void asyncLoading() throws Exception {
     assertEquals(TEXT1, key1Value);
   }
 
+  @Test
+  void handlesHttpErrorLoadingTranslations() {
+    mockGetLocales();
+    mockResponse(null, HttpStatus.UNAUTHORIZED);
+
+    assertThrows(NoSuchMessageException.class, () -> messageSource.getMessage("key1", null, Locale.ENGLISH));
+  }
+
+  @Test
+  void handlesHttpErrorLoadingLanguages() {
+    mockGetLocales(HttpStatus.NOT_FOUND);
+
+    assertThrows(NoSuchMessageException.class, () -> messageSource.getMessage("key1", null, Locale.ENGLISH));
+  }
+
 }