diff --git a/.config/pmd/java/ruleset.xml b/.config/pmd/java/ruleset.xml index 267fa5e..748c826 100644 --- a/.config/pmd/java/ruleset.xml +++ b/.config/pmd/java/ruleset.xml @@ -11,7 +11,6 @@ - @@ -153,6 +152,8 @@ + + @@ -184,6 +185,9 @@ + + + @@ -197,6 +201,33 @@ + + + + Usually all cases where `StringBuilder` (or the outdated `StringBuffer`) is used are either due to confusing (legacy) logic or may be replaced by a simpler string concatenation. + + Solution: + * Do not use `StringBuffer` because it's thread-safe and usually this is not needed + * If `StringBuilder` is only used in a simple method (like `toString`) and is effectively inlined: Use a simpler string concatenation (`"a" + x + "b"`). This will be optimized by the Java compiler internally. + * In all other cases: + * Check what is happening and if it makes ANY sense! If for example a CSV file is built here consider using a proper library instead! + * Abstract the Strings into a DTO, join them together using a collection (or `StringJoiner`) or use Java's Streaming API instead + + 3 + + + + + + + + + - @@ -234,7 +265,7 @@ - @@ -255,7 +286,7 @@ - @@ -277,7 +308,7 @@ - @@ -301,7 +332,7 @@ - @@ -309,4 +340,732 @@ + + + + + + Do not use native HTML! Use Vaadin layouts and components to create required structure. + If you are 100% sure that you escaped the value properly and you have no better options you can suppress this. + + 2 + + + + + + + + + + + + + + + + java.text.NumberFormat: DecimalFormat and ChoiceFormat are thread-unsafe. + + Solution: Create a new local one when needed in a method. + + 1 + + + + + + + + + + + + + + + A regular expression is compiled implicitly on every invocation. + Problem: This can be (CPU) expensive, depending on the length of the regular expression. + + Solution: Compile the regex pattern only once and assign it to a private static final Pattern field. + java.util.Pattern objects are thread-safe, so they can be shared among threads. + + 2 + + + + 5 and +(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))) +or +self::VariableAccess and @Name=ancestor::ClassBody[1]/FieldDeclaration/VariableDeclarator[StringLiteral[string-length(@Image) > 5 and +(matches(@Image, '[\.\$\|\(\)\[\]\{\}\^\?\*\+\\]+'))] or not(StringLiteral)]/VariableId/@Name] +]]> + + + + + + + + + + + + The default constructor of ByteArrayOutputStream creates a 32 bytes initial capacity and for StringWriter 16 chars. + Such a small buffer as capacity usually needs several expensive expansions. + + Solution: Explicitly declared the buffer size so that an expansion is not needed in most cases. + Typically much larger than 32, e.g. 4096. + + 2 + + + + + + + + + + + + + + + The time to find element is O(n); n = the number of enum values. + This identical processing is executed for every call. + Considered problematic when `n > 3`. + + Solution: Use a static field-to-enum-value Map. Access time is O(1), provided the hashCode is well-defined. + Implement a fromString method to provide the reverse conversion by using the map. + + 3 + + + + 3]//MethodDeclaration/Block + //MethodCall[pmd-java:matchesSig('java.util.stream.Stream#findFirst()') or pmd-java:matchesSig('java.util.stream.Stream#findAny()')] + [//MethodCall[pmd-java:matchesSig('java.util.stream.Stream#of(_)') or pmd-java:matchesSig('java.util.Arrays#stream(_)')] + [ArgumentList/MethodCall[pmd-java:matchesSig('_#values()')]]] +]]> + + + + + fromString(String name) { + return Stream.of(values()).filter(v -> v.toString().equals(name)).findAny(); // bad: iterates for every call, O(n) access time + } +} + +Usage: `Fruit f = Fruit.fromString("banana");` + +// GOOD +public enum Fruit { + APPLE("apple"), + ORANGE("orange"), + BANANA("banana"), + KIWI("kiwi"); + + private static final Map nameToValue = + Stream.of(values()).collect(toMap(Object::toString, v -> v)); + private final String name; + + Fruit(String name) { this.name = name; } + @Override public String toString() { return name; } + public static Optional fromString(String name) { + return Optional.ofNullable(nameToValue.get(name)); // good, get from Map, O(1) access time + } +} +]]> + + + + + + A regular expression is compiled on every invocation. + Problem: this can be expensive, depending on the length of the regular expression. + + Solution: Usually a pattern is a literal, not dynamic and can be compiled only once. Assign it to a private static field. + java.util.Pattern objects are thread-safe so they can be shared among threads. + + 2 + + + + + + + + + + + + + + + + Recreating a DateTimeFormatter is relatively expensive. + + Solution: Java 8+ java.time.DateTimeFormatter is thread-safe and can be shared among threads. + Create the formatter from a pattern only once, to initialize a static final field. + + 2 + + + + + + + + + + + + Creating a security provider is expensive because of loading of algorithms and other classes. + Additionally, it uses synchronized which leads to lock contention when used with multiple threads. + + Solution: This only needs to happen once in the JVM lifetime, because once loaded the provider is typically available from the Security class. + Create the security provider only once: Only in case when it's not yet available from the Security class. + + 2 + + + + + + + + + + + + + + + Reflection is relatively expensive. + + Solution: Avoid reflection. Use the non-reflective, explicit way like generation by IDE. + + 2 + + + + + + + + + + + + + + + java.util.SimpleDateFormat is thread-unsafe. + The usual solution is to create a new one when needed in a method. + Creating SimpleDateFormat is relatively expensive. + + Solution: Use java.time.DateTimeFormatter. These classes are immutable, thus thread-safe and can be made static. + + 2 + + + + + + + + + + + + + + + Blocking calls, for instance remote calls, may exhaust the common pool for some time thereby blocking all other use of the common pool. + In addition, nested use of the common pool can lead to deadlock. Do not use the common pool for blocking calls. + The parallelStream() call uses the common pool. + + Solution: Use a dedicated thread pool with enough threads to get proper parallelism. + The number of threads in the common pool is equal to the number of CPUs and meant to utilize all of them. + It assumes CPU-intensive non-blocking processing of in-memory data. + + See also: [_Be Aware of ForkJoinPool#commonPool()_](https://dzone.com/articles/be-aware-of-forkjoinpoolcommonpool) + + 2 + + + + + + + + + list = new ArrayList(); + final ForkJoinPool myFjPool = new ForkJoinPool(10); + final ExecutorService myExePool = Executors.newFixedThreadPool(10); + + void bad1() { + list.parallelStream().forEach(elem -> storeDataRemoteCall(elem)); // bad + } + + void good1() { + CompletableFuture[] futures = list.stream().map(elem -> CompletableFuture.supplyAsync(() -> storeDataRemoteCall(elem), myExePool)) + .toArray(CompletableFuture[]::new); + CompletableFuture.allOf(futures).get(10, TimeUnit.MILLISECONDS)); + } + + void good2() throws ExecutionException, InterruptedException { + myFjPool.submit(() -> + list.parallelStream().forEach(elem -> storeDataRemoteCall(elem)) + ).get(); + } + + String storeDataRemoteCall(String elem) { + // do remote call, blocking. We don't use the returned value. + RestTemplate tmpl; + return ""; + } +} +]]> + + + + + + CompletableFuture.supplyAsync/runAsync is typically used for remote calls. + By default it uses the common pool. + The number of threads in the common pool is equal to the number of CPU's, which is suitable for in-memory processing. + For I/O, however, this number is typically not suitable because most time is spent waiting for the response and not in CPU. + The common pool must not be used for blocking calls. + + Solution: A separate, properly sized pool of threads (an Executor) should be used for the async calls. + + See also: [_Be Aware of ForkJoinPool#commonPool()_](https://dzone.com/articles/be-aware-of-forkjoinpoolcommonpool) + + 2 + + + + + + + + +>[] futures = accounts.stream() + .map(account -> CompletableFuture.supplyAsync(() -> isAccountBlocked(account))) // bad + .toArray(CompletableFuture[]::new); + } + + void good() { + CompletableFuture>[] futures = accounts.stream() + .map(account -> CompletableFuture.supplyAsync(() -> isAccountBlocked(account), asyncPool)) // good + .toArray(CompletableFuture[]::new); + } +} +]]> + + + + + + `take()` stalls indefinitely in case of hanging threads and consumes a thread. + + Solution: use `poll()` with a timeout value and handle the timeout. + + 2 + + + + + + + + + void collectAllCollectionReplyFromThreads(CompletionService> completionService) { + try { + Future> futureLocal = completionService.take(); // bad + Future> futuresGood = completionService.poll(3, TimeUnit.SECONDS); // good + responseCollector.addAll(futuresGood.get(10, TimeUnit.SECONDS)); // good + } catch (InterruptedException | ExecutionException e) { + LOGGER.error("Error in Thread : {}", e); + } catch (TimeoutException e) { + LOGGER.error("Timeout in Thread : {}", e); + } +} +]]> + + + + + + Stalls indefinitely in case of stalled Callable(s) and consumes threads. + + Solution: Provide a timeout to the invokeAll/invokeAny method and handle the timeout. + + 2 + + + + + + + + +> executeTasksBad(Collection> tasks, ExecutorService executor) throws Exception { + return executor.invokeAll(tasks); // bad, no timeout + } + private List> executeTasksGood(Collection> tasks, ExecutorService executor) throws Exception { + return executor.invokeAll(tasks, OUR_TIMEOUT_IN_MILLIS, TimeUnit.MILLISECONDS); // good + } +} +]]> + + + + + + Stalls indefinitely in case of hanging threads and consumes a thread. + + Solution: Provide a timeout value and handle the timeout. + + 2 + + + + + + + + + complFuture) throws Exception { + return complFuture.get(); // bad +} + +public static String good(CompletableFuture complFuture) throws Exception { + return complFuture.get(10, TimeUnit.SECONDS); // good +} +]]> + + + + + + + Apache HttpClient with its connection pool and timeouts should be setup once and then used for many requests. + It is quite expensive to create and can only provide the benefits of pooling when reused in all requests for that connection. + + Solution: Create/build HttpClient with proper connection pooling and timeouts once, and then use it for requests. + + 3 + + + + + + + + + connectBad(Object req) { + HttpEntity requestEntity = new HttpEntity<>(req); + + HttpClient httpClient = HttpClientBuilder.create().setMaxConnPerRoute(10).build(); // bad + return remoteCall(httpClient, requestEntity); + } +} +]]> + + + + + + Problem: Gson creation is relatively expensive. A JMH benchmark shows a 24x improvement reusing one instance. + + Solution: Since Gson objects are thread-safe after creation, they can be shared between threads. + So reuse created instances from a static field. + Pay attention to use thread-safe (custom) adapters and serializers. + + 3 + + + + + + + + + + + diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0639fc6..6101566 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -91,7 +91,7 @@ jobs: - name: Create Release id: create-release - uses: shogo82148/actions-create-release@4661dc54f7b4b564074e9fbf73884d960de569a3 # v1 + uses: shogo82148/actions-create-release@7b89596097b26731bda0852f1504f813499079ee # v1 with: tag_name: v${{ steps.version.outputs.release }} release_name: v${{ steps.version.outputs.release }} diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties index 6a6b8b2..c0bcafe 100644 --- a/.mvn/wrapper/maven-wrapper.properties +++ b/.mvn/wrapper/maven-wrapper.properties @@ -1,17 +1,3 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. +wrapperVersion=3.3.4 +distributionType=only-script distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.11/apache-maven-3.9.11-bin.zip diff --git a/mvnw b/mvnw index 0830332..bd8896b 100755 --- a/mvnw +++ b/mvnw @@ -19,7 +19,7 @@ # ---------------------------------------------------------------------------- # ---------------------------------------------------------------------------- -# Apache Maven Wrapper startup batch script, version 3.3.0 +# Apache Maven Wrapper startup batch script, version 3.3.4 # # Optional ENV vars # ----------------- @@ -97,14 +97,25 @@ die() { exit 1 } +trim() { + # MWRAPPER-139: + # Trims trailing and leading whitespace, carriage returns, tabs, and linefeeds. + # Needed for removing poorly interpreted newline sequences when running in more + # exotic environments such as mingw bash on Windows. + printf "%s" "${1}" | tr -d '[:space:]' +} + +scriptDir="$(dirname "$0")" +scriptName="$(basename "$0")" + # parse distributionUrl and optional distributionSha256Sum, requires .mvn/wrapper/maven-wrapper.properties while IFS="=" read -r key value; do case "${key-}" in - distributionUrl) distributionUrl="${value-}" ;; - distributionSha256Sum) distributionSha256Sum="${value-}" ;; + distributionUrl) distributionUrl=$(trim "${value-}") ;; + distributionSha256Sum) distributionSha256Sum=$(trim "${value-}") ;; esac -done <"${0%/*}/.mvn/wrapper/maven-wrapper.properties" -[ -n "${distributionUrl-}" ] || die "cannot read distributionUrl property in ${0%/*}/.mvn/wrapper/maven-wrapper.properties" +done <"$scriptDir/.mvn/wrapper/maven-wrapper.properties" +[ -n "${distributionUrl-}" ] || die "cannot read distributionUrl property in $scriptDir/.mvn/wrapper/maven-wrapper.properties" case "${distributionUrl##*/}" in maven-mvnd-*bin.*) @@ -122,7 +133,7 @@ maven-mvnd-*bin.*) distributionUrl="${distributionUrl%-bin.*}-$distributionPlatform.zip" ;; maven-mvnd-*) MVN_CMD=mvnd.sh _MVNW_REPO_PATTERN=/maven/mvnd/ ;; -*) MVN_CMD="mvn${0##*/mvnw}" _MVNW_REPO_PATTERN=/org/apache/maven/ ;; +*) MVN_CMD="mvn${scriptName#mvnw}" _MVNW_REPO_PATTERN=/org/apache/maven/ ;; esac # apply MVNW_REPOURL and calculate MAVEN_HOME @@ -131,7 +142,8 @@ esac distributionUrlName="${distributionUrl##*/}" distributionUrlNameMain="${distributionUrlName%.*}" distributionUrlNameMain="${distributionUrlNameMain%-bin}" -MAVEN_HOME="$HOME/.m2/wrapper/dists/${distributionUrlNameMain-}/$(hash_string "$distributionUrl")" +MAVEN_USER_HOME="${MAVEN_USER_HOME:-${HOME}/.m2}" +MAVEN_HOME="${MAVEN_USER_HOME}/wrapper/dists/${distributionUrlNameMain-}/$(hash_string "$distributionUrl")" exec_maven() { unset MVNW_VERBOSE MVNW_USERNAME MVNW_PASSWORD MVNW_REPOURL || : @@ -199,7 +211,7 @@ elif set_java_home; then public static void main( String[] args ) throws Exception { setDefault( new Downloader() ); - java.nio.file.Files.copy( new java.net.URL( args[0] ).openStream(), java.nio.file.Paths.get( args[1] ).toAbsolutePath().normalize() ); + java.nio.file.Files.copy( java.net.URI.create( args[0] ).toURL().openStream(), java.nio.file.Paths.get( args[1] ).toAbsolutePath().normalize() ); } } END @@ -218,7 +230,7 @@ if [ -n "${distributionSha256Sum-}" ]; then echo "Please disable validation by removing 'distributionSha256Sum' from your maven-wrapper.properties." >&2 exit 1 elif command -v sha256sum >/dev/null; then - if echo "$distributionSha256Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" | sha256sum -c >/dev/null 2>&1; then + if echo "$distributionSha256Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" | sha256sum -c - >/dev/null 2>&1; then distributionSha256Result=true fi elif command -v shasum >/dev/null; then @@ -243,8 +255,41 @@ if command -v unzip >/dev/null; then else tar xzf${__MVNW_QUIET_TAR:+"$__MVNW_QUIET_TAR"} "$TMP_DOWNLOAD_DIR/$distributionUrlName" -C "$TMP_DOWNLOAD_DIR" || die "failed to untar" fi -printf %s\\n "$distributionUrl" >"$TMP_DOWNLOAD_DIR/$distributionUrlNameMain/mvnw.url" -mv -- "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain" "$MAVEN_HOME" || [ -d "$MAVEN_HOME" ] || die "fail to move MAVEN_HOME" + +# Find the actual extracted directory name (handles snapshots where filename != directory name) +actualDistributionDir="" + +# First try the expected directory name (for regular distributions) +if [ -d "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain" ]; then + if [ -f "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain/bin/$MVN_CMD" ]; then + actualDistributionDir="$distributionUrlNameMain" + fi +fi + +# If not found, search for any directory with the Maven executable (for snapshots) +if [ -z "$actualDistributionDir" ]; then + # enable globbing to iterate over items + set +f + for dir in "$TMP_DOWNLOAD_DIR"/*; do + if [ -d "$dir" ]; then + if [ -f "$dir/bin/$MVN_CMD" ]; then + actualDistributionDir="$(basename "$dir")" + break + fi + fi + done + set -f +fi + +if [ -z "$actualDistributionDir" ]; then + verbose "Contents of $TMP_DOWNLOAD_DIR:" + verbose "$(ls -la "$TMP_DOWNLOAD_DIR")" + die "Could not find Maven distribution directory in extracted archive" +fi + +verbose "Found extracted Maven distribution directory: $actualDistributionDir" +printf %s\\n "$distributionUrl" >"$TMP_DOWNLOAD_DIR/$actualDistributionDir/mvnw.url" +mv -- "$TMP_DOWNLOAD_DIR/$actualDistributionDir" "$MAVEN_HOME" || [ -d "$MAVEN_HOME" ] || die "fail to move MAVEN_HOME" clean || : exec_maven "$@" diff --git a/mvnw.cmd b/mvnw.cmd index 136e686..92450f9 100644 --- a/mvnw.cmd +++ b/mvnw.cmd @@ -19,7 +19,7 @@ @REM ---------------------------------------------------------------------------- @REM ---------------------------------------------------------------------------- -@REM Apache Maven Wrapper startup batch script, version 3.3.0 +@REM Apache Maven Wrapper startup batch script, version 3.3.4 @REM @REM Optional ENV vars @REM MVNW_REPOURL - repo url base for downloading maven distribution @@ -40,7 +40,7 @@ @SET __MVNW_ARG0_NAME__= @SET MVNW_USERNAME= @SET MVNW_PASSWORD= -@IF NOT "%__MVNW_CMD__%"=="" (%__MVNW_CMD__% %*) +@IF NOT "%__MVNW_CMD__%"=="" ("%__MVNW_CMD__%" %*) @echo Cannot start maven from wrapper >&2 && exit /b 1 @GOTO :EOF : end batch / begin powershell #> @@ -73,13 +73,30 @@ switch -wildcard -casesensitive ( $($distributionUrl -replace '^.*/','') ) { # apply MVNW_REPOURL and calculate MAVEN_HOME # maven home pattern: ~/.m2/wrapper/dists/{apache-maven-,maven-mvnd--}/ if ($env:MVNW_REPOURL) { - $MVNW_REPO_PATTERN = if ($USE_MVND) { "/org/apache/maven/" } else { "/maven/mvnd/" } - $distributionUrl = "$env:MVNW_REPOURL$MVNW_REPO_PATTERN$($distributionUrl -replace '^.*'+$MVNW_REPO_PATTERN,'')" + $MVNW_REPO_PATTERN = if ($USE_MVND -eq $False) { "/org/apache/maven/" } else { "/maven/mvnd/" } + $distributionUrl = "$env:MVNW_REPOURL$MVNW_REPO_PATTERN$($distributionUrl -replace "^.*$MVNW_REPO_PATTERN",'')" } $distributionUrlName = $distributionUrl -replace '^.*/','' $distributionUrlNameMain = $distributionUrlName -replace '\.[^.]*$','' -replace '-bin$','' -$MAVEN_HOME_PARENT = "$HOME/.m2/wrapper/dists/$distributionUrlNameMain" -$MAVEN_HOME_NAME = ([System.Security.Cryptography.MD5]::Create().ComputeHash([byte[]][char[]]$distributionUrl) | ForEach-Object {$_.ToString("x2")}) -join '' + +$MAVEN_M2_PATH = "$HOME/.m2" +if ($env:MAVEN_USER_HOME) { + $MAVEN_M2_PATH = "$env:MAVEN_USER_HOME" +} + +if (-not (Test-Path -Path $MAVEN_M2_PATH)) { + New-Item -Path $MAVEN_M2_PATH -ItemType Directory | Out-Null +} + +$MAVEN_WRAPPER_DISTS = $null +if ((Get-Item $MAVEN_M2_PATH).Target[0] -eq $null) { + $MAVEN_WRAPPER_DISTS = "$MAVEN_M2_PATH/wrapper/dists" +} else { + $MAVEN_WRAPPER_DISTS = (Get-Item $MAVEN_M2_PATH).Target[0] + "/wrapper/dists" +} + +$MAVEN_HOME_PARENT = "$MAVEN_WRAPPER_DISTS/$distributionUrlNameMain" +$MAVEN_HOME_NAME = ([System.Security.Cryptography.SHA256]::Create().ComputeHash([byte[]][char[]]$distributionUrl) | ForEach-Object {$_.ToString("x2")}) -join '' $MAVEN_HOME = "$MAVEN_HOME_PARENT/$MAVEN_HOME_NAME" if (Test-Path -Path "$MAVEN_HOME" -PathType Container) { @@ -131,7 +148,33 @@ if ($distributionSha256Sum) { # unzip and move Expand-Archive "$TMP_DOWNLOAD_DIR/$distributionUrlName" -DestinationPath "$TMP_DOWNLOAD_DIR" | Out-Null -Rename-Item -Path "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain" -NewName $MAVEN_HOME_NAME | Out-Null + +# Find the actual extracted directory name (handles snapshots where filename != directory name) +$actualDistributionDir = "" + +# First try the expected directory name (for regular distributions) +$expectedPath = Join-Path "$TMP_DOWNLOAD_DIR" "$distributionUrlNameMain" +$expectedMvnPath = Join-Path "$expectedPath" "bin/$MVN_CMD" +if ((Test-Path -Path $expectedPath -PathType Container) -and (Test-Path -Path $expectedMvnPath -PathType Leaf)) { + $actualDistributionDir = $distributionUrlNameMain +} + +# If not found, search for any directory with the Maven executable (for snapshots) +if (!$actualDistributionDir) { + Get-ChildItem -Path "$TMP_DOWNLOAD_DIR" -Directory | ForEach-Object { + $testPath = Join-Path $_.FullName "bin/$MVN_CMD" + if (Test-Path -Path $testPath -PathType Leaf) { + $actualDistributionDir = $_.Name + } + } +} + +if (!$actualDistributionDir) { + Write-Error "Could not find Maven distribution directory in extracted archive" +} + +Write-Verbose "Found extracted Maven distribution directory: $actualDistributionDir" +Rename-Item -Path "$TMP_DOWNLOAD_DIR/$actualDistributionDir" -NewName $MAVEN_HOME_NAME | Out-Null try { Move-Item -Path "$TMP_DOWNLOAD_DIR/$MAVEN_HOME_NAME" -Destination $MAVEN_HOME_PARENT | Out-Null } catch {