From b11d1a44b41bc1df9d756521a5641af5303a721d Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 01:36:48 -0300 Subject: [PATCH 1/7] Refactor: Renamed iterator variables for greater clarity --- .../icedteaweb/validator/DirectoryCheckResults.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/validator/DirectoryCheckResults.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/validator/DirectoryCheckResults.java index 22bf1385b..116bc0fa2 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/validator/DirectoryCheckResults.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/validator/DirectoryCheckResults.java @@ -27,14 +27,14 @@ public DirectoryCheckResults(final List results) { * @return sum of passed checks, 0-3 per result */ public int getPasses() { - return results.stream().mapToInt(r -> r.getPasses()).sum(); + return results.stream().mapToInt(result -> result.getPasses()).sum(); } /** * @return sum of failed checks, 0-3 per results */ public int getFailures() { - return results.stream().mapToInt(r -> r.getFailures()).sum(); + return results.stream().mapToInt(result -> result.getFailures()).sum(); } /** @@ -60,9 +60,9 @@ public String toString() { private static String resultsToString(final List results) { final StringBuilder sb = new StringBuilder(); - for (final DirectoryCheckResult r : results) { - if (r.getFailures() > 0) { - sb.append(r.getMessage()); + for (final DirectoryCheckResult result : results) { + if (result.getFailures() > 0) { + sb.append(result.getMessage()); } } return sb.toString(); From 8aa11c261b1feb0b332fbf86133927e083f187e7 Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 04:42:16 -0300 Subject: [PATCH 2/7] Refactor: Extracted StringFormatter from BaseLogger for better code organization and maintainability --- .../net/adoptopenjdk/icedteaweb/logging/BaseLogger.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java index 481abf763..6c9ac4804 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java @@ -9,10 +9,12 @@ public abstract class BaseLogger implements Logger { protected String expand(final String msg, final Object[] args) { - return doExpand(msg, args); + return new StringFormatter().expand(msg, args); } +} - static String doExpand(final String msg, final Object[] args) { +class StringFormatter { + String expand(final String msg, final Object[] args) { if (msg == null) { return "null"; } From 55275aa4d2d1ca140b118bb3c33c4494f0d03226 Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 05:32:10 -0300 Subject: [PATCH 3/7] Refactor: Introducing Explaining Variable - const REGEXP_MODIFIER pattern, which is used in the extractVersionId method --- .../net/adoptopenjdk/icedteaweb/jnlp/version/SimpleRange.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/jnlp/version/SimpleRange.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/jnlp/version/SimpleRange.java index ccb4c3d84..885e3af1e 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/jnlp/version/SimpleRange.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/jnlp/version/SimpleRange.java @@ -44,6 +44,7 @@ */ class SimpleRange { + private static final String REGEXP_MODIFIER_END = REGEXP_MODIFIER + "$"; private final VersionId versionId; private final VersionModifier modifier; @@ -109,7 +110,7 @@ private static VersionModifier extractModifier(final String simpleRange) { } private static VersionId extractVersionId(final String simpleRange) { - final String exactId = simpleRange.replaceAll(REGEXP_MODIFIER + "$", ""); + final String exactId = simpleRange.replaceAll(REGEXP_MODIFIER_END, ""); return VersionId.fromString(exactId); } From 48d7374d0e94e81f5caa6e80e498d4771c960ad8 Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 06:05:09 -0300 Subject: [PATCH 4/7] Fixing test case as change in method because Class extended --- .../icedteaweb/logging/BaseLogger.java | 2 +- .../icedteaweb/logging/BaseLoggerTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java index 6c9ac4804..89883d6cb 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java @@ -8,7 +8,7 @@ */ public abstract class BaseLogger implements Logger { - protected String expand(final String msg, final Object[] args) { + protected static String expand(final String msg, final Object[] args) { return new StringFormatter().expand(msg, args); } } diff --git a/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java b/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java index c718ea4f1..2897164e9 100644 --- a/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java +++ b/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java @@ -11,7 +11,7 @@ public void testNullMessageHandledCorrectly() { final String message = null; final Object[] args = {"ONE"}; - assertThat(BaseLogger.doExpand(message, args), is("null")); + assertThat(BaseLogger.expand(message, args), is("null")); } @Test @@ -19,7 +19,7 @@ public void testMessageWithoutArgumentsStaysUnchanged() { final String message = "Message without arguments stay unchanged."; final Object[] args = {}; - assertThat(BaseLogger.doExpand(message, args), is("Message without arguments stay unchanged.")); + assertThat(BaseLogger.expand(message, args), is("Message without arguments stay unchanged.")); } @Test @@ -27,7 +27,7 @@ public void testMessageWitNullArgumentsStaysUnchanged() { final String message = "Message with null arguments stay unchanged."; final Object[] args = null; - assertThat(BaseLogger.doExpand(message, args), is("Message with null arguments stay unchanged.")); + assertThat(BaseLogger.expand(message, args), is("Message with null arguments stay unchanged.")); } @Test @@ -35,7 +35,7 @@ public void testMessageWithOneArgumentExpandsCorrectly() { final String message = "This is a message with {} argument."; final Object[] args = {"ONE"}; - assertThat(BaseLogger.doExpand(message, args), is("This is a message with ONE argument.")); + assertThat(BaseLogger.expand(message, args), is("This is a message with ONE argument.")); } @Test @@ -43,7 +43,7 @@ public void testMessageWithTwoArgumentExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE", 2}; - assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and 2.")); + assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and 2.")); } @Test @@ -51,7 +51,7 @@ public void testMessageWithTooManyArgumentExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE", 2, "abc"}; - assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and 2.")); + assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and 2.")); } @Test @@ -59,6 +59,6 @@ public void testMessageWithTooFewArgumentsExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE"}; - assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and {}.")); + assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and {}.")); } } From a6b4efd716e40248a29ffe894699cb73f7a12aa7 Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 21:23:51 -0300 Subject: [PATCH 5/7] Refactor: Extracting method for clarity and reusability --- .../adoptopenjdk/icedteaweb/ProcessUtils.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/ProcessUtils.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/ProcessUtils.java index f62e36dcc..5cc6518e7 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/ProcessUtils.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/ProcessUtils.java @@ -55,15 +55,21 @@ public static void waitForSafely(final Process process) { while (!pTerminated) { try { process.waitFor(); - } catch (final InterruptedException e) { - LOG.error(IcedTeaWebConstants.DEFAULT_ERROR_MESSAGE, e); - } - try { - process.exitValue(); pTerminated = true; - } catch (final IllegalThreadStateException e) { - LOG.error(IcedTeaWebConstants.DEFAULT_ERROR_MESSAGE, e); + } catch (final InterruptedException e) { + logInterruptedException(e); } } } + + /** + * Logs the given InterruptedException to the error log using the default error message. + * + * @param e the InterruptedException object that occurred and will be logged + * @return This method does not have a return value as it logs the InterruptedException. + */ + private static void logInterruptedException(InterruptedException e) { + LOG.error(IcedTeaWebConstants.DEFAULT_ERROR_MESSAGE, e); + } + } From e305bef4fe4fd1fdcf13e5c9d3f593929ab29633 Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 22:05:13 -0300 Subject: [PATCH 6/7] Revert "Fixing test case as change in method because Class extended" because test cases should not be messed wit  This reverts commit 48d7374d0e94e81f5caa6e80e498d4771c960ad8. --- .../icedteaweb/logging/BaseLogger.java | 2 +- .../icedteaweb/logging/BaseLoggerTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java index 89883d6cb..6c9ac4804 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java @@ -8,7 +8,7 @@ */ public abstract class BaseLogger implements Logger { - protected static String expand(final String msg, final Object[] args) { + protected String expand(final String msg, final Object[] args) { return new StringFormatter().expand(msg, args); } } diff --git a/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java b/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java index 2897164e9..c718ea4f1 100644 --- a/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java +++ b/common/src/test/java/net/adoptopenjdk/icedteaweb/logging/BaseLoggerTest.java @@ -11,7 +11,7 @@ public void testNullMessageHandledCorrectly() { final String message = null; final Object[] args = {"ONE"}; - assertThat(BaseLogger.expand(message, args), is("null")); + assertThat(BaseLogger.doExpand(message, args), is("null")); } @Test @@ -19,7 +19,7 @@ public void testMessageWithoutArgumentsStaysUnchanged() { final String message = "Message without arguments stay unchanged."; final Object[] args = {}; - assertThat(BaseLogger.expand(message, args), is("Message without arguments stay unchanged.")); + assertThat(BaseLogger.doExpand(message, args), is("Message without arguments stay unchanged.")); } @Test @@ -27,7 +27,7 @@ public void testMessageWitNullArgumentsStaysUnchanged() { final String message = "Message with null arguments stay unchanged."; final Object[] args = null; - assertThat(BaseLogger.expand(message, args), is("Message with null arguments stay unchanged.")); + assertThat(BaseLogger.doExpand(message, args), is("Message with null arguments stay unchanged.")); } @Test @@ -35,7 +35,7 @@ public void testMessageWithOneArgumentExpandsCorrectly() { final String message = "This is a message with {} argument."; final Object[] args = {"ONE"}; - assertThat(BaseLogger.expand(message, args), is("This is a message with ONE argument.")); + assertThat(BaseLogger.doExpand(message, args), is("This is a message with ONE argument.")); } @Test @@ -43,7 +43,7 @@ public void testMessageWithTwoArgumentExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE", 2}; - assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and 2.")); + assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and 2.")); } @Test @@ -51,7 +51,7 @@ public void testMessageWithTooManyArgumentExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE", 2, "abc"}; - assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and 2.")); + assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and 2.")); } @Test @@ -59,6 +59,6 @@ public void testMessageWithTooFewArgumentsExpandsCorrectly() { final String message = "This is a message with the arguments {} and {}."; final Object[] args = {"ONE"}; - assertThat(BaseLogger.expand(message, args), is("This is a message with the arguments ONE and {}.")); + assertThat(BaseLogger.doExpand(message, args), is("This is a message with the arguments ONE and {}.")); } } From 3bda7238d76c85336ab252f8b08a463b07c715dd Mon Sep 17 00:00:00 2001 From: Sarvang Dave Date: Tue, 4 Apr 2023 22:12:50 -0300 Subject: [PATCH 7/7] Refactored BaseLogger to match test cases --- .../net/adoptopenjdk/icedteaweb/logging/BaseLogger.java | 6 +++--- .../icedteaweb/logging/SystemOutLoggerFactory.java | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java index 6c9ac4804..f31f4ca03 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/BaseLogger.java @@ -8,13 +8,13 @@ */ public abstract class BaseLogger implements Logger { - protected String expand(final String msg, final Object[] args) { - return new StringFormatter().expand(msg, args); + protected static String doExpand(final String msg, final Object[] args) { + return new StringFormatter().doExpand(msg, args); } } class StringFormatter { - String expand(final String msg, final Object[] args) { + String doExpand(final String msg, final Object[] args) { if (msg == null) { return "null"; } diff --git a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/SystemOutLoggerFactory.java b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/SystemOutLoggerFactory.java index 15de0144c..b027d85e4 100644 --- a/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/SystemOutLoggerFactory.java +++ b/common/src/main/java/net/adoptopenjdk/icedteaweb/logging/SystemOutLoggerFactory.java @@ -47,7 +47,7 @@ public void debug(final String msg) { @Override public void debug(final String msg, final Object... arguments) { - log(DEBUG, expand(msg, arguments), null); + log(DEBUG, doExpand(msg, arguments), null); } @Override @@ -62,7 +62,7 @@ public void info(final String msg) { @Override public void info(final String msg, final Object... arguments) { - log(INFO, expand(msg, arguments), null); + log(INFO, doExpand(msg, arguments), null); } @Override @@ -77,7 +77,7 @@ public void warn(final String msg) { @Override public void warn(final String msg, final Object... arguments) { - log(WARNING, expand(msg, arguments), null); + log(WARNING, doExpand(msg, arguments), null); } @Override @@ -92,7 +92,7 @@ public void error(final String msg) { @Override public void error(final String msg, final Object... arguments) { - log(ERROR, expand(msg, arguments), null); + log(ERROR, doExpand(msg, arguments), null); } @Override