Sonar reported issues (#816)

csviri · web-flow · commit d42942b03485 · 2022-01-13T11:04:47.000+01:00
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java
@@ -20,7 +20,7 @@
 import io.javaoperatorsdk.operator.processing.LifecycleAware;
 
 @SuppressWarnings("rawtypes")
-public class Operator implements AutoCloseable, LifecycleAware {
+public class Operator implements LifecycleAware {
   private static final Logger log = LoggerFactory.getLogger(Operator.class);
   private final KubernetesClient kubernetesClient;
   private final ConfigurationService configurationService;
@@ -43,9 +43,9 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur
     this.configurationService = configurationService;
   }
 
-  /** Adds a shutdown hook that automatically calls {@link #close()} when the app shuts down. */
+  /** Adds a shutdown hook that automatically calls {@link #stop()} ()} when the app shuts down. */
   public void installShutdownHook() {
-    Runtime.getRuntime().addShutdownHook(new Thread(this::close));
+    Runtime.getRuntime().addShutdownHook(new Thread(this::stop));
   }
 
   public KubernetesClient getKubernetesClient() {
@@ -95,12 +95,6 @@ public void stop() throws OperatorException {
     }
   }
 
-  /** Stop the operator. */
-  @Override
-  public void close() {
-    stop();
-  }
-
   /**
    * Add a registration requests for the specified reconciler with this operator. The effective
    * registration of the reconciler is delayed till the operator is started.
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java
@@ -6,16 +6,17 @@ public class GenericRetryExecution implements RetryExecution {
 
   private final GenericRetry genericRetry;
 
-  private volatile int lastAttemptIndex = 0;
-  private volatile long currentInterval;
+  private int lastAttemptIndex = 0;
+  private long currentInterval;
 
   public GenericRetryExecution(GenericRetry genericRetry) {
     this.genericRetry = genericRetry;
     this.currentInterval = genericRetry.getInitialInterval();
   }
 
   public Optional<Long> nextDelay() {
-    if (genericRetry.getMaxAttempts() > -1 && lastAttemptIndex >= genericRetry.getMaxAttempts()) {
+    if (genericRetry.getMaxAttempts() > -1
+        && lastAttemptIndex >= genericRetry.getMaxAttempts()) {
       return Optional.empty();
     }
     if (lastAttemptIndex > 1) {
@@ -30,7 +31,8 @@ public Optional<Long> nextDelay() {
 
   @Override
   public boolean isLastAttempt() {
-    return genericRetry.getMaxAttempts() > -1 && lastAttemptIndex >= genericRetry.getMaxAttempts();
+    return genericRetry.getMaxAttempts() > -1
+        && lastAttemptIndex >= genericRetry.getMaxAttempts();
   }
 
   @Override
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
@@ -103,7 +103,7 @@ void addFinalizerOnNewResource() {
     verify(customResourceFacade, times(1))
         .replaceWithLock(
             argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)));
-    assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));
+    assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue();
   }
 
   @Test
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java
@@ -6,7 +6,7 @@
 
 import static org.mockito.Mockito.mock;
 
-public class AbstractEventSourceTest<S extends EventSource, T extends EventHandler> {
+public class AbstractEventSourceTestBase<S extends EventSource, T extends EventHandler> {
   protected T eventHandler;
   protected S source;
 
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CachingEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CachingEventSourceTest.java
@@ -12,7 +12,7 @@
 import static org.mockito.Mockito.*;
 
 class CachingEventSourceTest extends
-    AbstractEventSourceTest<CachingEventSource<SampleExternalResource, HasMetadata>, EventHandler> {
+    AbstractEventSourceTestBase<CachingEventSource<SampleExternalResource, HasMetadata>, EventHandler> {
 
   @BeforeEach
   public void setup() {
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java
@@ -60,9 +60,9 @@ void resourceWatchedByLabel() {
     assertThat(server).isNotNull();
     assertThat(client).isNotNull();
 
-    try (Operator o1 = new Operator(client, configurationService);
-        Operator o2 = new Operator(client, configurationService)) {
-
+    Operator o1 = new Operator(client, configurationService);
+    Operator o2 = new Operator(client, configurationService);
+    try {
       AtomicInteger c1 = new AtomicInteger();
       AtomicInteger c1err = new AtomicInteger();
       AtomicInteger c2 = new AtomicInteger();
@@ -113,7 +113,11 @@ void resourceWatchedByLabel() {
       assertThrows(
           ConditionTimeoutException.class,
           () -> await().atMost(2, TimeUnit.SECONDS).untilAtomic(c2err, is(greaterThan(0))));
+    } finally {
+      o1.stop();
+      o2.stop();
     }
+
   }
 
   public TestCustomResource newMyResource(String app, String namespace) {
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java
@@ -15,7 +15,7 @@
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
-import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTest;
+import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTestBase;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
 import static org.mockito.ArgumentMatchers.eq;
@@ -25,7 +25,7 @@
 import static org.mockito.Mockito.verify;
 
 class ControllerResourceEventSourceTest extends
-    AbstractEventSourceTest<ControllerResourceEventSource<TestCustomResource>, EventHandler> {
+    AbstractEventSourceTestBase<ControllerResourceEventSource<TestCustomResource>, EventHandler> {
 
   public static final String FINALIZER = "finalizer";
   private static final MixedOperation<TestCustomResource, KubernetesResourceList<TestCustomResource>, Resource<TestCustomResource>> client =
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSourceTest.java
@@ -8,7 +8,7 @@
 import io.javaoperatorsdk.operator.TestUtils;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
-import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTest;
+import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTestBase;
 import io.javaoperatorsdk.operator.processing.event.source.Cache;
 import io.javaoperatorsdk.operator.processing.event.source.SampleExternalResource;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
@@ -24,7 +24,7 @@
 import static org.mockito.Mockito.when;
 
 class PerResourcePollingEventSourceTest extends
-    AbstractEventSourceTest<PerResourcePollingEventSource<SampleExternalResource, TestCustomResource>, EventHandler> {
+    AbstractEventSourceTestBase<PerResourcePollingEventSource<SampleExternalResource, TestCustomResource>, EventHandler> {
 
   public static final int PERIOD = 80;
   private PerResourcePollingEventSource.ResourceSupplier<SampleExternalResource, TestCustomResource> supplier =
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PollingEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/polling/PollingEventSourceTest.java
@@ -10,15 +10,15 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
-import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTest;
+import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTestBase;
 import io.javaoperatorsdk.operator.processing.event.source.SampleExternalResource;
 
 import static io.javaoperatorsdk.operator.processing.event.source.SampleExternalResource.*;
 import static org.mockito.Mockito.*;
 
 class PollingEventSourceTest
     extends
-    AbstractEventSourceTest<PollingEventSource<SampleExternalResource, HasMetadata>, EventHandler> {
+    AbstractEventSourceTestBase<PollingEventSource<SampleExternalResource, HasMetadata>, EventHandler> {
 
   private Supplier<Map<ResourceID, SampleExternalResource>> supplier = mock(Supplier.class);
 
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java
@@ -15,15 +15,16 @@
 import io.javaoperatorsdk.operator.processing.event.Event;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
-import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTest;
+import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSourceTestBase;
 import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSourceTest.CapturingEventHandler;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 class TimerEventSourceTest
-    extends AbstractEventSourceTest<TimerEventSource<TestCustomResource>, CapturingEventHandler> {
+    extends
+    AbstractEventSourceTestBase<TimerEventSource<TestCustomResource>, CapturingEventHandler> {
 
   public static final int INITIAL_DELAY = 50;
   public static final int PERIOD = 50;
diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java
@@ -206,7 +206,7 @@ protected void after(ExtensionContext context) {
     }
 
     try {
-      this.operator.close();
+      this.operator.stop();
     } catch (Exception e) {
       // ignored
     }
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java
@@ -35,14 +35,15 @@ public AccumulativeMappingWriter loadExistingMappings() {
               .getFiler()
               .getResource(StandardLocation.CLASS_OUTPUT, "", resourcePath);
 
-      final var bufferedReader =
-          new BufferedReader(new InputStreamReader(readonlyResource.openInputStream()));
-      final var existingLines =
-          bufferedReader
-              .lines()
-              .map(l -> l.split(","))
-              .collect(Collectors.toMap(parts -> parts[0], parts -> parts[1]));
-      mappings.putAll(existingLines);
+      try (BufferedReader bufferedReader =
+          new BufferedReader(new InputStreamReader(readonlyResource.openInputStream()))) {
+        final var existingLines =
+            bufferedReader
+                .lines()
+                .map(l -> l.split(","))
+                .collect(Collectors.toMap(parts -> parts[0], parts -> parts[1]));
+        mappings.putAll(existingLines);
+      }
     } catch (IOException e) {
     }
     return this;
@@ -71,8 +72,7 @@ public void flush() {
         printWriter.println(entry.getKey() + "," + entry.getValue());
       }
     } catch (IOException e) {
-      e.printStackTrace();
-      throw new RuntimeException(e);
+      throw new IllegalStateException(e);
     } finally {
       if (printWriter != null) {
         printWriter.close();
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java
@@ -55,8 +55,9 @@ static <T, V> Map<T, V> provide(final String resourcePath, T key, V value) {
   }
 
   private static List<String> retrieveClassNamePairs(URL url) throws IOException {
-    return new BufferedReader(new InputStreamReader(url.openStream()))
-        .lines()
-        .collect(Collectors.toList());
+    try (BufferedReader br = new BufferedReader(new InputStreamReader(url.openStream()))) {
+      return br.lines()
+          .collect(Collectors.toList());
+    }
   }
 }
diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ControllerConfigurationAnnotationProcessor.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ControllerConfigurationAnnotationProcessor.java
@@ -15,6 +15,9 @@
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeMirror;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import io.fabric8.kubernetes.client.CustomResource;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 
@@ -28,6 +31,9 @@
 @AutoService(Processor.class)
 public class ControllerConfigurationAnnotationProcessor extends AbstractProcessor {
 
+  private static final Logger log =
+      LoggerFactory.getLogger(ControllerConfigurationAnnotationProcessor.class);
+
   private AccumulativeMappingWriter controllersResourceWriter;
   private TypeParameterResolver typeParameterResolver;
 
@@ -89,7 +95,7 @@ private void recordCRType(TypeElement controllerClassSymbol) {
           controllerClassSymbol.getQualifiedName().toString(), customResourceType.toString());
 
     } catch (Exception ioException) {
-      ioException.printStackTrace();
+      log.error("Error", ioException);
     }
   }
 
@@ -99,7 +105,7 @@ private TypeMirror findResourceType(TypeElement controllerClassSymbol) {
       return typeParameterResolver.resolve(
           processingEnv.getTypeUtils(), (DeclaredType) controllerClassSymbol.asType());
     } catch (Exception e) {
-      e.printStackTrace();
+      log.error("Error", e);
       return null;
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ void addFinalizerOnNewResource() {`
`103`	`103`	`verify(customResourceFacade, times(1))`
`104`	`104`	`.replaceWithLock(`
`105`	`105`	`argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)));`
`106`		`- assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER));`
	`106`	`+ assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue();`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`@Test`