address code quality issues

europeana · Jun 17, 2024 · 67dfaff · 67dfaff
1 parent 01ef3a4
commit 67dfaff
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 36 deletions.
diff --git a/...ava/eu/europeana/api/translation/service/etranslation/ETranslationTranslationService.java b/...ava/eu/europeana/api/translation/service/etranslation/ETranslationTranslationService.java
@@ -48,7 +48,9 @@ public class ETranslationTranslationService extends AbstractTranslationService {
   public static final String PATH_ERROR_CALLBACK = "/etranslation/error-callback";
   public static final int ETRANSLATION_SNIPPET_LIMIT = 4990;
   public static final int ETRANSLATION_SNIPPET_LIMIT_TESTS = 200;
-
+  private static final int SECOND_MILIS = 1000;
+
+
   private String serviceId;
   private final String baseUrl;
   private final String domain;
@@ -58,6 +60,17 @@ public class ETranslationTranslationService extends AbstractTranslationService {
   private final int maxWaitMillisec;
   private final RedisMessageListenerContainer redisMessageListenerContainer;
 
+  /**
+   * Contructor for etranslation service using dependency injection
+   * @param etranslationServiceBaseUrl base uRL of eTranslation service
+   * @param domain eTranslation domain
+   * @param translationApiBaseUrl the base URL of the translation API deployment
+   * @param maxWaitMillisec timeout for eTranslation callback 
+   * @param username eTranslation credential
+   * @param password eTranslation credential
+   * @param redisMessageListenerContainer container for PUB/SUB redis message listeners
+   * @throws TranslationException thrown in case that the translation cannot be performed/retrieved 
+   */
   public ETranslationTranslationService(String etranslationServiceBaseUrl, String domain,
       String translationApiBaseUrl, int maxWaitMillisec, String username, String password,
       RedisMessageListenerContainer redisMessageListenerContainer) throws TranslationException {
@@ -145,11 +158,8 @@ public void translate(List<TranslationObj> translationObjs) throws TranslationEx
       if (!FAKE_BASE_URL_FOR_TESTING.equals(baseUrl)) {
         sendTranslationRequest(body);
       }
-      // create a redis message listener obj, and wait on that obj until it get notified from the
-      // redis publisher
-      // createRedisMessageListenerAndWaitForResults(translationObjs, eTranslExtRef,
-      // eTranslJointStr.length());
-      readTranslationResponseFromRedis(redisMessageListenerAdapter, translationObjs, eTranslExtRef, eTranslJointStr.length());
+      // read translation response
+      readTranslationResponseFromRedis(redisMessageListenerAdapter, translationObjs, eTranslJointStr.length());
 
     } catch (JSONException | UnsupportedEncodingException e) {
       throw new TranslationException(
@@ -170,7 +180,7 @@ public void translate(List<TranslationObj> translationObjs) throws TranslationEx
 
 
   private void readTranslationResponseFromRedis(MessageListenerAdapter redisMessageListenerAdapter, List<TranslationObj> translationObjs,
-      String eTranslExtRef, int textSize) throws TranslationException {
+      int textSize) throws TranslationException {
 
     if(redisMessageListenerAdapter == null) {
       //if channel not registered, cannot read results
@@ -202,22 +212,27 @@ private void readTranslationResponseFromRedis(MessageListenerAdapter redisMessag
   }
 
   private int toSeconds(int maxWaitMillisec) {
-    return maxWaitMillisec / 1000;
+    return maxWaitMillisec / SECOND_MILIS;
   }
 
   private boolean isSnippetLimitExceeded(int textSize) {
     // use smaller limit for the tests (e.g. 200)
     if (FAKE_BASE_URL_FOR_TESTING.equals(baseUrl)) {
-      return textSize > ETRANSLATION_SNIPPET_LIMIT_TESTS ? true : false;
+      return (textSize > ETRANSLATION_SNIPPET_LIMIT_TESTS);
     } else {
-      return textSize > ETRANSLATION_SNIPPET_LIMIT ? true : false;
+      return (textSize > ETRANSLATION_SNIPPET_LIMIT);
     }
   }
 
-  private String readMessageFromChannel(MessageListenerAdapter redisMessageListenerAdapter) throws TranslationException {
+  private String readMessageFromChannel(MessageListenerAdapter redisMessageListenerAdapter){
 
     RedisMessageListener redisMessageListener =
         (RedisMessageListener) redisMessageListenerAdapter.getDelegate();
+
+    if(redisMessageListener == null) {
+      throw new IllegalArgumentException("ETranslation callback handling was not propetly initilized, the message listener must not be null!");
+    }
+
 
     synchronized (redisMessageListener) {
       /*
@@ -459,7 +474,9 @@ public boolean isSupported(String srcLang, String trgLang) {
   }
 
   @Override
-  public void close() {}
+  public void close() {
+    //nothing to do here
+  }
 
   @Override
   public String getExternalServiceEndPoint() {

diff --git a/...src/main/java/eu/europeana/api/translation/service/etranslation/RedisMessageListener.java b/...src/main/java/eu/europeana/api/translation/service/etranslation/RedisMessageListener.java
@@ -14,8 +14,12 @@ public class RedisMessageListener implements MessageListener {
     private static final Logger LOGGER = LogManager.getLogger(RedisMessageListener.class);
     private String message;
     //if true, the message received will be a document (e.g. from the eTranslation), otherwise a text-snippet
-    private boolean messageAsDocument; 
+    private final boolean messageAsDocument; 
 
+    /**
+     * Constructur, indicating if the expected message is for document translation or not
+     * @param messageAsDocument indicate if the expected response is from document translation
+     */
     public RedisMessageListener(boolean messageAsDocument) {
       super();
       this.messageAsDocument = messageAsDocument;

diff --git a/...s/src/integration-test/java/eu/europeana/api/translation/tests/web/TranslationRestIT.java b/...s/src/integration-test/java/eu/europeana/api/translation/tests/web/TranslationRestIT.java
@@ -132,9 +132,10 @@ public eTranslationSimulatorThreadForTextSnippetTranslation(MockMvc mockMvc) {
     }
     @Override
     public void run() {
+      String result;
       try {
         String requestJson = getJsonStringInput(TRANSLATION_REQUEST_E_TRANSLATION);
-        String result = mockMvc
+        result = mockMvc
             .perform(
                 post(BASE_URL_TRANSLATE)
                   .header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
@@ -143,16 +144,20 @@ public void run() {
             .andExpect(status().isOk())
             .andReturn().getResponse().getContentAsString();
 
-        assertNotNull(result);
         JSONObject json = new JSONObject(result);
         String langFieldValue = json.getString(TranslationAppConstants.LANG);
-        assertEquals(LANGUAGE_EN, langFieldValue);
+        if(langFieldValue == null) {
+          throw new IllegalArgumentException("langFieldValue must not be null");
+        }
 
         List<String> translations = Collections.singletonList(json.getString(TranslationAppConstants.TRANSLATIONS));
         assertTrue(translations.contains("That is my dog.") && translations.contains("That is my tree."));
         String serviceFieldValue = json.getString(TranslationAppConstants.SERVICE);
-        assertNotNull(serviceFieldValue);
+        if(serviceFieldValue == null) {
+          throw new IllegalArgumentException("serviceFieldValue must not be null");
+        }
       } catch (Exception e) {
+        throw new RuntimeException("cannot run simulator for eTranslation callback!", e);
       }
     }
   }

diff --git a/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationConfig.java b/translation-web/src/main/java/eu/europeana/api/translation/config/TranslationConfig.java
@@ -185,7 +185,7 @@ public String getEtranslationBaseUrl() {
     return etranslationBaseUrl;
   }
 
-  public boolean getEtranslationTruncate() {
+  public boolean isEtranslationTruncate() {
     return etranslationTruncate;
   }
 

diff --git a/...on-web/src/main/java/eu/europeana/api/translation/web/ETranslationCallbackController.java b/...on-web/src/main/java/eu/europeana/api/translation/web/ETranslationCallbackController.java
@@ -31,6 +31,9 @@ public ETranslationCallbackController(RedisTemplate<String, CachedTranslation> r
 
   @Tag(description = "ETranslation callback endpoint", name = "eTranslationCallback")
   @PostMapping(value = ETranslationTranslationService.PATH_CALLBACK)
+  /**
+   * callback endpoint for eTranslation - post request
+   */
   public void eTranslationCallbackPost(
       @RequestParam(value = "target-language", required = false) String targetLanguage,
       @RequestParam(value = "translated-text", required = false) String translatedTextSnippet,
@@ -50,18 +53,19 @@ public void eTranslationCallbackPost(
      * if we send a text snippet in the text-to-translate field, we ge the output in the translated-text parameter 
      * (although also extracted from the body)
      */
-    String translations = translatedTextSnippet!=null ? translatedTextSnippet : body;
+    String translations = (translatedTextSnippet == null) ? body : translatedTextSnippet ;
     if(externalReference!=null && translations!=null) {
       redisTemplate.convertAndSend(externalReference, translations);
     }
   }
 
   /**
    * This method is deprecated, it is used for manual simulations only, as the eTranslation send post callbacks
+   * @deprecated for simulation purposes only
    */
   @Tag(description = "ETranslation callback endpoint", name = "eTranslationCallback")
   @GetMapping(value = ETranslationTranslationService.PATH_CALLBACK)
-  @Deprecated
+  @Deprecated(since = "begiging ...")
   public ResponseEntity<String> eTranslationCallbackGet(
       @RequestParam(value = "target-language", required = false) String targetLanguage,
       @RequestParam(value = "translated-text", required = false) String translatedTextSnippet,
@@ -71,7 +75,8 @@ public ResponseEntity<String> eTranslationCallbackGet(
 
     if (timeout != null && timeout > 0) {
       // for simulation purposes, wait for $timeout seconds
-      Thread.sleep(timeout * 1000);
+      final long SECONDS_MILIS = 1000;
+      Thread.sleep(timeout * SECONDS_MILIS);
       return ResponseEntity.status(HttpStatus.ACCEPTED).build();
     }
 
@@ -92,13 +97,21 @@ public ResponseEntity<String> eTranslationCallbackGet(
 
   @Tag(description = "ETranslation error callback endpoint", name = "eTranslationErrorCallback")
   @PostMapping(value = ETranslationTranslationService.PATH_ERROR_CALLBACK)
+  /**
+   * callback endpoint for eTranslation errors - post request
+   */
   public void eTranslationErrorCallbackPost(
       @RequestParam(value = "error-code", required = false) String errorCode,
       @RequestParam(value = "error-message", required = false) String errorMessage,
       @RequestParam(value = "target-languages", required = false) String targetLanguages,
       @RequestParam(value = "request-id", required = false) String requestId,
       @RequestParam(value = "external-reference", required = false) String externalReference,
       @RequestBody(required = false) String body) {
+    handleErroCallback(errorCode, errorMessage, requestId, externalReference);
+  }
+
+  private void handleErroCallback(String errorCode, String errorMessage, String requestId,
+      String externalReference) {
     if (LOGGER.isDebugEnabled()) {
       LOGGER.debug(
           "eTranslation error callback has been received with the following parameters: error-code: {},"
@@ -117,27 +130,17 @@ public void eTranslationErrorCallbackPost(
 
   @Tag(description = "ETranslation error callback endpoint", name = "eTranslationErrorCallback")
   @GetMapping(value = ETranslationTranslationService.PATH_ERROR_CALLBACK)
+  /**
+   * callback endpoint for eTranslation - get request
+   */
   public void eTranslationErrorCallbackGet(
       @RequestParam(value = "error-code", required = false) String errorCode,
       @RequestParam(value = "error-message", required = false) String errorMessage,
       @RequestParam(value = "target-languages", required = false) String targetLanguages,
       @RequestParam(value = "request-id", required = false) String requestId,
       @RequestParam(value = "external-reference", required = false) String externalReference,
       @RequestBody(required = false) String body) {
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug(
-          "eTranslation error callback has been received with the following parameters: error-code: {},"
-              + "error-message: {}, request-id: {}, external-reference: {}",
-          LoggingUtils.sanitizeUserInput(errorCode), LoggingUtils.sanitizeUserInput(errorMessage),
-          LoggingUtils.sanitizeUserInput(requestId),
-          LoggingUtils.sanitizeUserInput(externalReference));
-    }
-    if (externalReference != null) {
-      redisTemplate.convertAndSend(externalReference,
-          String.format("%s: error-code=%s, error-message=%s",
-              ETranslationTranslationService.ERROR_CALLBACK_MARKUP, errorCode,
-              errorMessage));
-    }
+    handleErroCallback(errorCode, errorMessage, requestId, externalReference);
   }
 
 

diff --git a/...ion-web/src/main/java/eu/europeana/api/translation/web/service/TranslationWebService.java b/...ion-web/src/main/java/eu/europeana/api/translation/web/service/TranslationWebService.java
@@ -222,7 +222,7 @@ private boolean exceedesSnippetLimit(StringBuilder translJointString, String tex
   }
 
   private boolean shouldTruncateText(TranslationRequest translationRequest) {
-    return translationConfig.getEtranslationTruncate()
+    return translationConfig.isEtranslationTruncate()
         && ETranslationTranslationService.DEFAULT_SERVICE_ID.equals(translationRequest.getService());
   }