Merge branch 'sonar-cleanup' into 'main'

Minor cleanup from Sonar suggestions

See merge request weblogic-cloud/weblogic-image-tool!481

Author: ddsharpe

imagetool/src/main/java/com/oracle/weblogic/imagetool/cachestore/FileCacheStore.java

@@ -26,7 +26,7 @@
 
 public class FileCacheStore implements CacheStore {
 
-    public static final String CACHEDIR = "WLSIMG_CACHEDIR";
+    public static final String CACHE_DIR_ENV = "WLSIMG_CACHEDIR";
     private static final LoggingFacade logger = LoggingFactory.getLogger(FileCacheStore.class);
 
     private final Properties properties = new Properties();
@@ -144,7 +144,7 @@ private static String defaultCacheDir() {
      * @return cache directory
      */
     private static String initCacheDir() throws IOException {
-        String cacheDirStr = Utils.getEnvironmentProperty(CACHEDIR, FileCacheStore::defaultCacheDir);
+        String cacheDirStr = Utils.getEnvironmentProperty(CACHE_DIR_ENV, FileCacheStore::defaultCacheDir);
 
         Path cacheDir = Paths.get(cacheDirStr);
 

imagetool/src/main/java/com/oracle/weblogic/imagetool/inspect/InventoryPatch.java

@@ -5,8 +5,8 @@
 
 import java.util.ArrayList;
 import java.util.List;
-
-import com.oracle.weblogic.imagetool.util.Utils;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 public class InventoryPatch {
     private String bug;
@@ -27,26 +27,23 @@ public String description() {
 
     /**
      * Parse the provided string into patch objects, and return the list of patches.
-     * The fields should be separated by a semi-colon with three fields per patch.
+     * The fields should be separated by a semicolon with three fields per patch.
      * @param patchesString patch data from the inspected image
      * @return a list of patch objects
      */
     public static List<InventoryPatch> parseInventoryPatches(String patchesString) {
         List<InventoryPatch> patches = new ArrayList<>();
-        if (!Utils.isEmptyString(patchesString)) {
-            String[] tokens = patchesString.split(";");
-            for (int i = 0; i < tokens.length; i++) {
-                InventoryPatch patch = new InventoryPatch();
-                patch.bug = tokens[i];
-                if (i++ < tokens.length) {
-                    patch.uid = tokens[i];
-                }
-                if (i++ < tokens.length) {
-                    patch.description = tokens[i].replace("\"", "");
-                }
-                patches.add(patch);
-            }
+        // Pattern defines a tuple of three elements: patch ID, patch UID, and patch description.
+        Pattern tuplePattern = Pattern.compile("(\\d+);(\\d+);\\\"(.*?)\\\";");
+        Matcher patchMatcher = tuplePattern.matcher(patchesString);
+        while (patchMatcher.find()) {
+            InventoryPatch patch = new InventoryPatch();
+            patch.bug = patchMatcher.group(1);
+            patch.uid = patchMatcher.group(2);
+            patch.description = patchMatcher.group(3);
+            patches.add(patch);
         }
+
         return patches;
     }
 }

imagetool/src/test/java/com/oracle/weblogic/imagetool/cachestore/CacheStoreTestImpl.java

@@ -9,8 +9,8 @@
 
 public class CacheStoreTestImpl implements CacheStore {
 
-    private HashMap<String, String> cache = new HashMap<>();
-    private Path cacheDir;
+    private final HashMap<String, String> cache = new HashMap<>();
+    private final Path cacheDir;
 
     public CacheStoreTestImpl(Path cacheDir) {
         this.cacheDir = cacheDir;
@@ -46,6 +46,7 @@ public String deleteFromCache(String key) {
 
     @Override
     public void clearCache() {
+        cache.clear();
     }
 
     @Override

imagetool/src/test/java/com/oracle/weblogic/imagetool/cachestore/CachedFileTest.java

@@ -35,7 +35,7 @@ class CachedFileTest {
     static Path cacheDir;
     static CacheStore cacheStore;
     static final List<String> fileContents = Arrays.asList("A", "B", "C");
-    static final String ver12213 = "12.2.1.3.0";
+    static final String VER_12213 = "12.2.1.3.0";
 
     @BeforeAll
     static void setup(@TempDir Path tempDir, @TempDir Path cacheDir) throws IOException {
@@ -49,7 +49,7 @@ static void setup(@TempDir Path tempDir, @TempDir Path cacheDir) throws IOExcept
         CachedFileTest.cacheDir = cacheDir;
         cacheStore  = new CacheStoreTestImpl(cacheDir);
         // build a fake cache with several installers
-        cacheStore.addToCache("wls_" + ver12213, path12213.toString());
+        cacheStore.addToCache("wls_" + VER_12213, path12213.toString());
         cacheStore.addToCache("wls_12.2.1.4.0_" + BuildPlatform.getPlatformName(), path12214.toString());
         cacheStore.addToCache("wls_14.1.1.0.0_amd64", path1411.toString());
 
@@ -83,7 +83,7 @@ void userProvidedPatchVersionAsId() {
     }
 
     @Test
-    void resolveFileNotFound() throws Exception {
+    void resolveFileNotFound() {
         // resolve should fail for a CachedFile that is not in the store
         CachedFile fakeFile = new CachedFile(InstallerType.WLS, "10.3.6.0.0");
         assertThrows(FileNotFoundException.class, () -> fakeFile.resolve(cacheStore));
@@ -92,15 +92,15 @@ void resolveFileNotFound() throws Exception {
     @Test
     void resolveFileFindsFile() throws IOException {
         // Resolve a CachedFile stored in the cache (created in test setup above)
-        CachedFile wlsInstallerFile = new CachedFile(InstallerType.WLS, ver12213);
-        String expected = cacheStore.getValueFromCache("wls_" + ver12213);
+        CachedFile wlsInstallerFile = new CachedFile(InstallerType.WLS, VER_12213);
+        String expected = cacheStore.getValueFromCache("wls_" + VER_12213);
         assertEquals(expected, wlsInstallerFile.resolve(cacheStore), "CachedFile did not resolve file");
     }
 
     @Test
     void resolveNoArchFile() throws IOException {
         // Look for a cache entry where the user specified the architecture/platform amd64
-        CachedFile wlsNoArch = new CachedFile(InstallerType.WLS, ver12213, "amd64");
+        CachedFile wlsNoArch = new CachedFile(InstallerType.WLS, VER_12213, "amd64");
 
         // verify the cache is setup as expected.
         // wls_12.2.1.3.0 is in the cache, but wls_12.2.1.3.0_amd64 is NOT in the cache
@@ -138,7 +138,7 @@ void resolveFallbackToLocalArch() throws IOException {
 
     @Test
     void copyFile(@TempDir Path contextDir) throws Exception {
-        CachedFile wlsInstallerFile = new CachedFile(InstallerType.WLS, ver12213);
+        CachedFile wlsInstallerFile = new CachedFile(InstallerType.WLS, VER_12213);
         // copy the file from the cache store to the fake build context directory
         Path result = wlsInstallerFile.copyFile(cacheStore, contextDir.toString());
         // check to see if the file was copied correctly by examining the contents of the resulting file

imagetool/src/test/java/com/oracle/weblogic/imagetool/cachestore/FileCacheStoreTest.java

@@ -24,33 +24,33 @@
 @TestMethodOrder(MethodOrderer.OrderAnnotation.class)
 class FileCacheStoreTest {
 
-    private static final String testKey = "abc_xyz_123";
-    private static final String testVal = "this_is_a_test";
+    private static final String TEST_KEY = "abc_xyz_123";
+    private static final String TEST_VAL = "this_is_a_test";
 
     @BeforeAll
     static void init(@TempDir File tempDir) throws CacheStoreException {
-        System.setProperty(FileCacheStore.CACHEDIR, tempDir.getAbsolutePath());
+        System.setProperty(FileCacheStore.CACHE_DIR_ENV, tempDir.getAbsolutePath());
         cache().clearCache();
     }
 
     @Test
     @Order(1)
     void addingValueToCache() {
         // add value to cache
-        assertDoesNotThrow(() -> cache().addToCache(testKey, testVal), "Add to cache threw an exception");
+        assertDoesNotThrow(() -> cache().addToCache(TEST_KEY, TEST_VAL), "Add to cache threw an exception");
     }
 
     @Test
     @Order(2)
     void checkValueInCache() {
         // check to see if the key that was just added is there, and value matches expected value
         assertDoesNotThrow(() ->
-                assertTrue(cache().containsKey(testKey)),
+                assertTrue(cache().containsKey(TEST_KEY)),
             "containsKey failed to find key or value that was just added");
 
         // check (another way) that the value was just added
         assertDoesNotThrow(() ->
-                assertEquals(testVal, cache().getValueFromCache(testKey), "Found unexpected value in cache"),
+                assertEquals(TEST_VAL, cache().getValueFromCache(TEST_KEY), "Found unexpected value in cache"),
             "Get from cache threw an exception");
     }
 
@@ -63,7 +63,7 @@ void deleteValueFromCache() {
             "Delete from cache threw an exception");
 
         assertDoesNotThrow(() ->
-                assertEquals(testVal, cache().deleteFromCache(testKey), "Value from deleted key did not match"),
+                assertEquals(TEST_VAL, cache().deleteFromCache(TEST_KEY), "Value from deleted key did not match"),
             "Delete from cache threw an exception");
     }
 

imagetool/src/test/java/com/oracle/weblogic/imagetool/cachestore/PatchFileTest.java

@@ -209,8 +209,6 @@ void multiplePatchVersionsNoVersionSupplied() throws Exception {
         String filePathFromCache = cacheStore.getValueFromCache(patchId + "_12.2.1.3.0");
         assertNotNull(filePathFromCache, "Could not find new patch in cache");
         assertEquals(filePath, filePathFromCache, "Patch in cache does not match");
-
-        //assertEquals("600000000073715", patchFile.getReleaseNumber(), "Patch did not find release number");
     }
 
     @Test

imagetool/src/test/java/com/oracle/weblogic/imagetool/inspect/InspectTest.java

@@ -19,6 +19,17 @@
 
 @Tag("unit")
 class InspectTest {
+    @Test
+    void testPatchStringParser() {
+        assertEquals(0, InventoryPatch.parseInventoryPatches("").size());
+        assertEquals(1, InventoryPatch.parseInventoryPatches("30319071;23384603;\"One-off\";").size());
+        assertEquals(3, InventoryPatch.parseInventoryPatches(
+            "30319071;23384603;\"One-off\";26355633;21447583;\"One-off\";26287183;21447582;\"One-off\";").size());
+        // same as previous but with last semi-colon removed.  last patch should be discarded but shouldn't fail.
+        assertEquals(2, InventoryPatch.parseInventoryPatches(
+            "30319071;23384603;\"One-off\";26355633;21447583;\"One-off\";26287183;21447582;\"One-off\"").size());
+    }
+
     @Test
     void testJsonOutput() throws IOException {
         testPropertiesToJson("src/test/resources/inspect/image1.properties",

imagetool/src/test/java/com/oracle/weblogic/imagetool/util/DockerfileBuilderTest.java

@@ -44,7 +44,6 @@ void validateMustacheAliases() throws IOException {
 
         MustacheFactory mf = new DefaultMustacheFactory(new File("src/main/resources/docker-files"));
         Mustache mustache = mf.compile("Create_Image.mustache");
-        //mustache.execute(new PrintWriter(System.out), dockerfileOptions).flush();
         mustache.execute(new StringWriter(), dockerfileOptions).flush();
         assertTrue(true);
     }

imagetool/src/test/java/com/oracle/weblogic/imagetool/util/UtilsTest.java

@@ -42,7 +42,7 @@ class UtilsTest {
     private SystemProperties overrideProperties;
 
     @Test
-    void firstGreaterThanLast() throws Exception {
+    void firstGreaterThanLast() {
         String thisVersion = "12.2.1.3.0";
         String thatVersion = "12.2.1.2.0";
         assertTrue(Utils.compareVersions(thisVersion, thatVersion) > 0,
@@ -85,7 +85,7 @@ void firstGreaterThanLast() throws Exception {
     }
 
     @Test
-    void secondGreaterThanFirst() throws Exception {
+    void secondGreaterThanFirst() {
         String thisVersion = "12.2.1.3.0";
         String thatVersion = "12.2.1.4.0";
         assertTrue(Utils.compareVersions(thisVersion, thatVersion) < 0,
@@ -123,7 +123,7 @@ void secondGreaterThanFirst() throws Exception {
     }
 
     @Test
-    void versionsShouldBeEqual() throws Exception {
+    void versionsShouldBeEqual() {
         String thisVersion = "12.2.1.3.0";
         String thatVersion = "12.2.1.3.0";
         assertEquals(0, Utils.compareVersions(thisVersion, thatVersion),

pom.xml

@@ -154,7 +154,7 @@
                 <plugin>
                     <groupId>org.apache.maven.plugins</groupId>
                     <artifactId>maven-failsafe-plugin</artifactId>
-                    <version>3.1.2</version>
+                    <version>3.3.1</version>
                     <configuration>
                         <groups>${test.groups}</groups>
                         <excludedGroups>failing</excludedGroups>

tests/src/test/java/com/oracle/weblogic/imagetool/tests/extensions/TimingExtension.java

@@ -5,25 +5,21 @@
 
 import java.util.concurrent.TimeUnit;
 
-import com.oracle.weblogic.imagetool.logging.LoggingFacade;
-import com.oracle.weblogic.imagetool.logging.LoggingFactory;
 import org.junit.jupiter.api.extension.AfterTestExecutionCallback;
 import org.junit.jupiter.api.extension.BeforeTestExecutionCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
 
 public class TimingExtension implements BeforeTestExecutionCallback, AfterTestExecutionCallback {
-    private static final LoggingFacade logger = LoggingFactory.getLogger(TimingExtension.class);
-
     private static final String START_TIME = "start time";
     private static final String EM = LoggingExtension.EM;
 
     @Override
-    public void beforeTestExecution(ExtensionContext context) throws Exception {
+    public void beforeTestExecution(ExtensionContext context) {
         getStore(context).put(START_TIME, System.currentTimeMillis());
     }
 
     @Override
-    public void afterTestExecution(ExtensionContext context) throws Exception {
+    public void afterTestExecution(ExtensionContext context) {
         long startTime = getStore(context).remove(START_TIME, long.class);
         long duration = System.currentTimeMillis() - startTime;
         long minutes = TimeUnit.MILLISECONDS.toMinutes(duration);

tests/src/test/java/com/oracle/weblogic/imagetool/tests/utils/CacheCommand.java

@@ -90,6 +90,7 @@ public CacheCommand type(String value) {
      * Generate the command using the provided command line options.
      * @return the imagetool command as a string suitable for running in ProcessBuilder
      */
+    @Override
     public String build() {
         return super.build()
             + field("listItems", listItems)

tests/src/test/java/com/oracle/weblogic/imagetool/tests/utils/CreateAuxCommand.java

@@ -71,6 +71,7 @@ public CreateAuxCommand wdtModelOnly(boolean value) {
      * Generate the command using the provided command line options.
      * @return the imagetool command as a string suitable for running in ProcessBuilder
      */
+    @Override
     public String build() {
         return super.build()
             + field("--fromImage", fromImage)

tests/src/test/java/com/oracle/weblogic/imagetool/tests/utils/CreateCommand.java

@@ -146,6 +146,7 @@ public CreateCommand wdtModelOnly(boolean value) {
      * Generate the command using the provided command line options.
      * @return the imagetool command as a string suitable for running in ProcessBuilder
      */
+    @Override
     public String build() {
         return super.build()
             + field("--version", version)

tests/src/test/java/com/oracle/weblogic/imagetool/tests/utils/RebaseCommand.java

@@ -39,6 +39,7 @@ public RebaseCommand tag(String value) {
      * Generate the command using the provided command line options.
      * @return the imagetool command as a string suitable for running in ProcessBuilder
      */
+    @Override
     public String build() {
         return super.build()
             + field("--targetImage", targetImage)

tests/src/test/java/com/oracle/weblogic/imagetool/tests/utils/UpdateCommand.java

@@ -68,6 +68,7 @@ public UpdateCommand wdtModelOnly(boolean value) {
      * Generate the command using the provided command line options.
      * @return the imagetool command as a string suitable for running in ProcessBuilder
      */
+    @Override
     public String build() {
         return super.build()
             + field("--fromImage", fromImage)