diff --git a/.config/pmd/java/ruleset.xml b/.config/pmd/java/ruleset.xml index a0fff54..4570323 100644 --- a/.config/pmd/java/ruleset.xml +++ b/.config/pmd/java/ruleset.xml @@ -2,7 +2,7 @@ + xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.github.io/ruleset_2_0_0.xsd"> This ruleset checks the code for discouraged programming constructs. @@ -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 @@ - @@ -310,12 +341,14 @@ + + - Do not used native HTML! Use Vaadin layouts and components to create required structure. + 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 @@ -330,4 +363,709 @@ + + + + + + + + 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/broken-links.yml b/.github/workflows/broken-links.yml index 5921f76..3a1009c 100644 --- a/.github/workflows/broken-links.yml +++ b/.github/workflows/broken-links.yml @@ -26,7 +26,7 @@ jobs: - name: Find already existing issue id: find-issue run: | - echo "number=$(gh issue list -l 'bug' -l 'automated' -L 1 -S 'in:title \"Link Checker Report\"' -s 'open' --json 'number' --jq '.[].number')" >> $GITHUB_OUTPUT + echo "number=$(gh issue list -l 'bug' -l 'automated' -L 1 -S 'in:title "Link Checker Report"' -s 'open' --json 'number' --jq '.[].number')" >> $GITHUB_OUTPUT env: GH_TOKEN: ${{ github.token }} @@ -38,7 +38,7 @@ jobs: - name: Create Issue From File if: steps.lychee.outputs.exit_code != 0 - uses: peter-evans/create-issue-from-file@e8ef132d6df98ed982188e460ebb3b5d4ef3a9cd # v5 + uses: peter-evans/create-issue-from-file@fca9117c27cdc29c6c4db3b86c48e4115a786710 # v6 with: issue-number: ${{ steps.find-issue.outputs.number }} title: Link Checker Report diff --git a/.github/workflows/check-build.yml b/.github/workflows/check-build.yml index a68c375..8169416 100644 --- a/.github/workflows/check-build.yml +++ b/.github/workflows/check-build.yml @@ -28,7 +28,7 @@ jobs: timeout-minutes: 30 strategy: matrix: - java: [17, 21] + java: [17, 21, 25] distribution: [temurin] steps: - uses: actions/checkout@v5 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5b3e9b3..413ee07 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,7 +19,7 @@ We also encourage you to read the [contribution instructions by GitHub](https:// ### Software Requirements You should have the following things installed: * Git -* Java 21 - should be as unmodified as possible (Recommended: [Eclipse Adoptium](https://adoptium.net/temurin/releases/)) +* Java 25 - should be as unmodified as possible (Recommended: [Eclipse Adoptium](https://adoptium.net/temurin/releases/)) * Maven (Note that the [Maven Wrapper](https://maven.apache.org/wrapper/) is shipped with the repo) ### Recommended setup diff --git a/pom.xml b/pom.xml index 5b7a0c7..6127e2c 100644 --- a/pom.xml +++ b/pom.xml @@ -45,7 +45,7 @@ com.puppycrawl.tools checkstyle - 11.0.1 + 11.1.0 diff --git a/vaadin-grid-exporter-demo/pom.xml b/vaadin-grid-exporter-demo/pom.xml index a62e0c9..516696e 100644 --- a/vaadin-grid-exporter-demo/pom.xml +++ b/vaadin-grid-exporter-demo/pom.xml @@ -29,9 +29,9 @@ software.xdev.vaadin.Application - 24.8.8 + 24.9.2 - 3.5.5 + 3.5.6 @@ -146,7 +146,7 @@ org.codehaus.mojo exec-maven-plugin - 3.5.1 + 3.6.1 patch-package-json-overrides @@ -216,7 +216,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.14.0 + 3.14.1 ${maven.compiler.release} diff --git a/vaadin-grid-exporter/pom.xml b/vaadin-grid-exporter/pom.xml index 3d517a6..f865e63 100644 --- a/vaadin-grid-exporter/pom.xml +++ b/vaadin-grid-exporter/pom.xml @@ -50,7 +50,7 @@ false - 24.8.8 + 24.9.2 @@ -132,7 +132,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.14.0 + 3.14.1 ${maven.compiler.release} @@ -143,7 +143,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.11.3 + 3.12.0 attach-javadocs @@ -210,7 +210,7 @@ org.codehaus.mojo flatten-maven-plugin - 1.7.2 + 1.7.3 ossrh @@ -256,7 +256,7 @@ org.sonatype.central central-publishing-maven-plugin - 0.8.0 + 0.9.0 true sonatype-central-portal @@ -278,7 +278,7 @@ com.puppycrawl.tools checkstyle - 11.0.1 + 11.1.0