Fix code smells

folio-org · Sep 3, 2024 · affe9d2 · affe9d2
1 parent 9e430c9
commit affe9d2
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 59 deletions.
diff --git a/okapi-core/src/main/java/org/folio/okapi/managers/TimerManager.java b/okapi-core/src/main/java/org/folio/okapi/managers/TimerManager.java
@@ -31,7 +31,7 @@
 import org.folio.okapi.util.TenantProductSeq;
 
 public class TimerManager {
-  private static final Logger logger = OkapiLogger.get();
+  private static final Logger LOGGER = OkapiLogger.get();
   private static final String MAP_NAME = "org.folio.okapi.timer.map";
   private static final String EVENT_NAME = "org.folio.okapi.timer.event";
   /**
@@ -204,14 +204,14 @@ private void fireTimer(String tenantId, ModuleDescriptor md, TimerDescriptor tim
     HttpMethod httpMethod = routingEntry.getDefaultMethod(HttpMethod.POST);
     ModuleInstance inst = new ModuleInstance(md, routingEntry, path, httpMethod, true);
     MultiMap headers = MultiMap.caseInsensitiveMultiMap();
-    logger.info("timer {} call start module {} for tenant {}",
+    LOGGER.info("timer {} call start module {} for tenant {}",
         timerDescriptor.getId(), md.getId(), tenantId);
     proxyService.callSystemInterface(headers, tenantId, inst, "")
         .onFailure(cause ->
-            logger.info("timer {} call failed to module {} for tenant {} : {}",
+            LOGGER.info("timer {} call failed to module {} for tenant {} : {}",
                 timerDescriptor.getId(), md.getId(), tenantId, cause.getMessage()))
         .onSuccess(res ->
-            logger.info("timer {} call succeeded to module {} for tenant {}",
+            LOGGER.info("timer {} call succeeded to module {} for tenant {}",
                 timerDescriptor.getId(), md.getId(), tenantId));
   }
 
@@ -226,7 +226,7 @@ ModuleDescriptor getModuleForTimer(List<ModuleDescriptor> list, String tenantPro
     try {
       tenantProductSeq = new TenantProductSeq(tenantProductSeqString);
     } catch (RuntimeException e) {
-      logger.error("Invalid id of timer: {}", tenantProductSeqString, e);
+      LOGGER.error("Invalid id of timer: {}", tenantProductSeqString, e);
       return null;
     }
     for (ModuleDescriptor md : list) {
@@ -252,7 +252,7 @@ ModuleDescriptor getModuleForTimer(List<ModuleDescriptor> list, String tenantPro
    * @param timerId timer identifier
    */
   private void handleTimer(String tenantId, String tenantProductSeq) {
-    logger.info("timer {} handle for tenant {}", tenantProductSeq, tenantId);
+    LOGGER.info("timer {} handle for tenant {}", tenantProductSeq, tenantId);
     tenantTimers.get(tenantId).get(tenantProductSeq)
         .compose(timerDescriptor ->
             // this timer is latest and current ... do the work ...
@@ -271,7 +271,7 @@ private void handleTimer(String tenantId, String tenantProductSeq) {
               waitTimer(tenantId, timerDescriptor);
             })
         )
-        .onFailure(cause -> logger.warn("handleTimer id={} {}", tenantProductSeq,
+        .onFailure(cause -> LOGGER.warn("handleTimer id={} {}", tenantProductSeq,
             cause.getMessage(), cause));
   }
 
@@ -288,7 +288,7 @@ private void waitTimer(String tenantId, TimerDescriptor timerDescriptor) {
     RoutingEntry routingEntry = timerDescriptor.getRoutingEntry();
     final long delay = routingEntry.getDelayMilliSeconds();
     final String tenantProductSeq = timerDescriptor.getId();
-    logger.info("waitTimer {} delay {} for tenant {}", tenantProductSeq, delay, tenantId);
+    LOGGER.info("waitTimer {} delay {} for tenant {}", tenantProductSeq, delay, tenantId);
     if (delay > 0) {
       long timer = vertx.setTimer(delay, res -> handleTimer(tenantId, tenantProductSeq));
       timerRunning.put(tenantProductSeq, timer);
@@ -315,7 +315,7 @@ public Future<TimerDescriptor> getTimer(String tenantId, String productSeq) {
     try {
       tenantProductSeq = new TenantProductSeq(tenantId, productSeq).toString();
     } catch (RuntimeException e) {
-      logger.warn("Timer lookup with invalid productSeq: {}", productSeq);
+      LOGGER.warn("Timer lookup with invalid productSeq {}: {}", productSeq, e.getMessage());
       return Future.failedFuture(new OkapiError(ErrorType.NOT_FOUND, tenantId));
     }
     return timerMap.getNotFound(tenantProductSeq)
@@ -440,13 +440,15 @@ private void consumePatchTimer() {
     });
   }
 
+  @SuppressWarnings("java:S2583")  // false positive as timerDescriptor can be null
+  // therefore suppress 'Change this condition so that it does not always evaluate to "false"'
   static boolean belongs(TimerDescriptor timerDescriptor, String tenantId) {
     try {
       var tenantProductSeq = new TenantProductSeq(timerDescriptor.getId());
       return Objects.equals(tenantProductSeq.getTenantId(), tenantId);
     } catch (RuntimeException e) {
       var id = timerDescriptor == null ? "null" : timerDescriptor.getId();
-      logger.error("Comparing TimerDescriptor fails: id={}, tenantId={}", id, tenantId, e);
+      LOGGER.error("Comparing TimerDescriptor fails: id={}, tenantId={}", id, tenantId, e);
       return false;
     }
   }

diff --git a/okapi-core/src/test/java/org/folio/okapi/managers/TimerManagerTest.java b/okapi-core/src/test/java/org/folio/okapi/managers/TimerManagerTest.java
@@ -1,18 +1,16 @@
 package org.folio.okapi.managers;
 
 import static io.vertx.core.Future.succeededFuture;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import io.vertx.core.Future;
 import io.vertx.core.Vertx;
 import io.vertx.junit5.Timeout;
 import io.vertx.junit5.VertxExtension;
 import io.vertx.junit5.VertxTestContext;
 import java.util.List;
-import java.util.stream.Stream;
 import org.assertj.core.api.Assertions;
 import org.folio.okapi.bean.InterfaceDescriptor;
 import org.folio.okapi.bean.ModuleDescriptor;
@@ -24,8 +22,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.Arguments;
-import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 @Timeout(5000)
@@ -104,6 +101,20 @@ TimerDescriptor timerDescriptor(String id, Integer seconds) {
     return timerDescriptor;
   }
 
+  Future<Void> testPatchTimer(TimerManager timerManager, String productSeq, Integer seconds) {
+    return timerManager.patchTimer("test_tenant", timerDescriptor(productSeq, seconds))
+        .compose(x -> timerManager.getTimer("test_tenant", productSeq))
+        .map(timerDescriptor -> {
+          var delay = timerDescriptor.getRoutingEntry().getDelay();
+          if (seconds == null) {
+            Assertions.assertThat(delay).isEqualTo("" + 1);
+          } else {
+            Assertions.assertThat(delay).isEqualTo("" + seconds);
+          }
+          return null;
+        });
+  }
+
   @Test
   void init(Vertx vertx, VertxTestContext vtc) {
     var timerStore = new TimerStoreMemory(timerDescriptor("mod-y_0", 2));
@@ -115,52 +126,26 @@ void init(Vertx vertx, VertxTestContext vtc) {
     var proxyService = mock(ProxyService.class);
     var timerManager = new TimerManager(timerStore, true);
     timerManager.init(vertx, tenantManager, discoveryManager, proxyService)
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-y_0", 0)))
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-y_0", null)))
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-y_0", 1)))
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-y_0", 2)))
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-y_0", 2)))
-    .compose(x -> timerManager.patchTimer("test_tenant", timerDescriptor("mod-x_0", 2)))
-    .onComplete(vtc.succeedingThenComplete());
-  }
-
-  static Stream<Arguments> tenantProductSeq() {
-    return Stream.of(
-        Arguments.of(null, "mod-foo", 9, "mod-foo_9"),
-        Arguments.of(null, "mod-foo-bar", 999, "mod-foo-bar_999"),
-        Arguments.of("tenant", "mod-foo", 0, "tenant_mod-foo_0"),
-        Arguments.of("test_tenant", "mod-foo-bar", 123, "test_tenant_mod-foo-bar_123")
-        );
-  }
-
-  @ParameterizedTest
-  @MethodSource
-  void tenantProductSeq(String tenantId, String product, int seq, String tenantProductSeq) {
-    var actual = new TenantProductSeq(tenantId, product, seq).toString();
-    assertThat(actual, is(tenantProductSeq));
-
-    var actual2 = new TenantProductSeq(tenantProductSeq);
-    assertThat(actual2.getTenantId(), is(tenantId));
-    assertThat(actual2.getProduct(), is(product));
-    assertThat(actual2.getSeq(), is(seq));
-
-    var actual3 = new TenantProductSeq("diku", tenantProductSeq);
-    assertThat(actual3.getTenantId(), is("diku"));
-    assertThat(actual3.getProduct(), is(product));
-    assertThat(actual3.getSeq(), is(seq));
+    .compose(x -> testPatchTimer(timerManager, "mod-y_0", 0))
+    .compose(x -> testPatchTimer(timerManager, "mod-y_0", null))
+    .compose(x -> testPatchTimer(timerManager, "mod-y_0", 1))
+    .compose(x -> testPatchTimer(timerManager, "mod-y_0", 2))
+    .compose(x -> testPatchTimer(timerManager, "mod-y_0", 2))
+    .compose(x -> testPatchTimer(timerManager, "mod-x_0", 2))
+    .andThen(vtc.succeedingThenComplete());
   }
 
   @ParameterizedTest
-  @ValueSource(strings = {
-      "",
-      "_",
-      "-",
-      "mod-foo",
-      "mod-foo_",
-      "tenant_mod-foo",
-      "tenant_mod-foo_",
-  })
-  void tenantProductSeqFail(String tenantProductSeq) {
-    assertThrows(NumberFormatException.class, () -> new TenantProductSeq(tenantProductSeq));
+  @CsvSource(textBlock = """
+                  , , false
+                  foo, foo, false
+                  foo_0, foo, false
+                  t_foo_0, t, true
+                  t_foo, t, false
+  """)
+  void belongs(String timerId, String tenantId, boolean expected) {
+    var timerDescriptor = new TimerDescriptor();
+    timerDescriptor.setId(timerId);
+    Assertions.assertThat(TimerManager.belongs(timerDescriptor, tenantId)).isEqualTo(expected);
   }
 }
diff --git a/okapi-core/src/test/java/org/folio/okapi/util/TenantProductSeqTest.java b/okapi-core/src/test/java/org/folio/okapi/util/TenantProductSeqTest.java
@@ -0,0 +1,56 @@
+package org.folio.okapi.util;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class TenantProductSeqTest {
+
+  static Stream<Arguments> tenantProductSeq() {
+    return Stream.of(
+        Arguments.of(null, "mod-foo", 9, "mod-foo_9"),
+        Arguments.of(null, "mod-foo-bar", 999, "mod-foo-bar_999"),
+        Arguments.of("tenant", "mod-foo", 0, "tenant_mod-foo_0"),
+        Arguments.of("test_tenant", "mod-foo-bar", 123, "test_tenant_mod-foo-bar_123")
+        );
+  }
+
+  @ParameterizedTest
+  @MethodSource
+  void tenantProductSeq(String tenantId, String product, int seq, String tenantProductSeq) {
+    var actual = new TenantProductSeq(tenantId, product, seq).toString();
+    assertThat(actual, is(tenantProductSeq));
+
+    var actual2 = new TenantProductSeq(tenantProductSeq);
+    assertThat(actual2.getTenantId(), is(tenantId));
+    assertThat(actual2.getProduct(), is(product));
+    assertThat(actual2.getSeq(), is(seq));
+
+    var actual3 = new TenantProductSeq("diku", tenantProductSeq);
+    assertThat(actual3.getTenantId(), is("diku"));
+    assertThat(actual3.getProduct(), is(product));
+    assertThat(actual3.getSeq(), is(seq));
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {
+      "",
+      "_",
+      "-",
+      "mod-foo",
+      "mod-foo_",
+      "tenant_mod-foo",
+      "tenant_mod-foo_",
+  })
+  void tenantProductSeqFail(String tenantProductSeq) {
+    assertThrows(NumberFormatException.class, () -> new TenantProductSeq(tenantProductSeq));
+  }
+}