From c09055b83afe5530e11ad3f7a61af0f758efa3a8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 8 May 2023 12:01:58 +0200 Subject: [PATCH 1/3] Consistent support for MultiValueMap and common Map implementations Closes gh-30440 --- .../core/CollectionFactory.java | 48 ++++++++--------- .../core/CollectionFactoryTests.java | 54 ++++++++++--------- 2 files changed, 53 insertions(+), 49 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java index cbda0925500..9d9332dbff3 100644 --- a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java +++ b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -73,11 +73,13 @@ public final class CollectionFactory { private static final Set> approximableMapTypes = Set.of( // Standard map interfaces Map.class, + MultiValueMap.class, SortedMap.class, NavigableMap.class, // Common concrete map classes HashMap.class, LinkedHashMap.class, + LinkedMultiValueMap.class, TreeMap.class, EnumMap.class); @@ -118,13 +120,7 @@ public final class CollectionFactory { */ @SuppressWarnings({"rawtypes", "unchecked"}) public static Collection createApproximateCollection(@Nullable Object collection, int capacity) { - if (collection instanceof LinkedList) { - return new LinkedList<>(); - } - else if (collection instanceof List) { - return new ArrayList<>(capacity); - } - else if (collection instanceof EnumSet enumSet) { + if (collection instanceof EnumSet enumSet) { Collection copy = EnumSet.copyOf(enumSet); copy.clear(); return copy; @@ -132,6 +128,12 @@ public final class CollectionFactory { else if (collection instanceof SortedSet sortedSet) { return new TreeSet<>(sortedSet.comparator()); } + else if (collection instanceof LinkedList) { + return new LinkedList<>(); + } + else if (collection instanceof List) { + return new ArrayList<>(capacity); + } else { return new LinkedHashSet<>(capacity); } @@ -187,8 +189,8 @@ public final class CollectionFactory { else if (LinkedList.class == collectionType) { return new LinkedList<>(); } - else if (TreeSet.class == collectionType || NavigableSet.class == collectionType - || SortedSet.class == collectionType) { + else if (TreeSet.class == collectionType || NavigableSet.class == collectionType || + SortedSet.class == collectionType) { return new TreeSet<>(); } else if (EnumSet.class.isAssignableFrom(collectionType)) { @@ -246,6 +248,9 @@ public final class CollectionFactory { else if (map instanceof SortedMap sortedMap) { return new TreeMap<>(sortedMap.comparator()); } + else if (map instanceof MultiValueMap) { + return new LinkedMultiValueMap(capacity); + } else { return new LinkedHashMap<>(capacity); } @@ -292,26 +297,21 @@ public final class CollectionFactory { @SuppressWarnings({"rawtypes", "unchecked"}) public static Map createMap(Class mapType, @Nullable Class keyType, int capacity) { Assert.notNull(mapType, "Map type must not be null"); - if (mapType.isInterface()) { - if (Map.class == mapType) { - return new LinkedHashMap<>(capacity); - } - else if (SortedMap.class == mapType || NavigableMap.class == mapType) { - return new TreeMap<>(); - } - else if (MultiValueMap.class == mapType) { - return new LinkedMultiValueMap(); - } - else { - throw new IllegalArgumentException("Unsupported Map interface: " + mapType.getName()); - } + if (LinkedHashMap.class == mapType || HashMap.class == mapType || Map.class == mapType) { + return new LinkedHashMap<>(capacity); + } + else if (LinkedMultiValueMap.class == mapType || MultiValueMap.class == mapType) { + return new LinkedMultiValueMap(); + } + else if (TreeMap.class == mapType || SortedMap.class == mapType || NavigableMap.class == mapType) { + return new TreeMap<>(); } else if (EnumMap.class == mapType) { Assert.notNull(keyType, "Cannot create EnumMap for unknown key type"); return new EnumMap(asEnumType(keyType)); } else { - if (!Map.class.isAssignableFrom(mapType)) { + if (mapType.isInterface() || !Map.class.isAssignableFrom(mapType)) { throw new IllegalArgumentException("Unsupported Map type: " + mapType.getName()); } try { diff --git a/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java b/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java index 570099d83af..32292b39778 100644 --- a/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java +++ b/spring-core/src/test/java/org/springframework/core/CollectionFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -209,21 +209,23 @@ class CollectionFactoryTests { @Test void createsCollectionsCorrectly() { // interfaces - assertThat(createCollection(List.class, 0)).isInstanceOf(ArrayList.class); - assertThat(createCollection(Set.class, 0)).isInstanceOf(LinkedHashSet.class); - assertThat(createCollection(Collection.class, 0)).isInstanceOf(LinkedHashSet.class); - assertThat(createCollection(SortedSet.class, 0)).isInstanceOf(TreeSet.class); - assertThat(createCollection(NavigableSet.class, 0)).isInstanceOf(TreeSet.class); - - assertThat(createCollection(List.class, String.class, 0)).isInstanceOf(ArrayList.class); - assertThat(createCollection(Set.class, String.class, 0)).isInstanceOf(LinkedHashSet.class); - assertThat(createCollection(Collection.class, String.class, 0)).isInstanceOf(LinkedHashSet.class); - assertThat(createCollection(SortedSet.class, String.class, 0)).isInstanceOf(TreeSet.class); - assertThat(createCollection(NavigableSet.class, String.class, 0)).isInstanceOf(TreeSet.class); + testCollection(List.class, ArrayList.class); + testCollection(Set.class, LinkedHashSet.class); + testCollection(Collection.class, LinkedHashSet.class); + testCollection(SortedSet.class, TreeSet.class); + testCollection(NavigableSet.class, TreeSet.class); // concrete types - assertThat(createCollection(HashSet.class, 0)).isInstanceOf(HashSet.class); - assertThat(createCollection(HashSet.class, String.class, 0)).isInstanceOf(HashSet.class); + testCollection(ArrayList.class, ArrayList.class); + testCollection(HashSet.class, LinkedHashSet.class); + testCollection(LinkedHashSet.class, LinkedHashSet.class); + testCollection(TreeSet.class, TreeSet.class); + } + + private void testCollection(Class collectionType, Class resultType) { + assertThat(CollectionFactory.isApproximableCollectionType(collectionType)).isTrue(); + assertThat(createCollection(collectionType, 0)).isInstanceOf(resultType); + assertThat(createCollection(collectionType, String.class, 0)).isInstanceOf(resultType); } @Test @@ -258,20 +260,22 @@ class CollectionFactoryTests { @Test void createsMapsCorrectly() { // interfaces - assertThat(createMap(Map.class, 0)).isInstanceOf(LinkedHashMap.class); - assertThat(createMap(SortedMap.class, 0)).isInstanceOf(TreeMap.class); - assertThat(createMap(NavigableMap.class, 0)).isInstanceOf(TreeMap.class); - assertThat(createMap(MultiValueMap.class, 0)).isInstanceOf(LinkedMultiValueMap.class); - - assertThat(createMap(Map.class, String.class, 0)).isInstanceOf(LinkedHashMap.class); - assertThat(createMap(SortedMap.class, String.class, 0)).isInstanceOf(TreeMap.class); - assertThat(createMap(NavigableMap.class, String.class, 0)).isInstanceOf(TreeMap.class); - assertThat(createMap(MultiValueMap.class, String.class, 0)).isInstanceOf(LinkedMultiValueMap.class); + testMap(Map.class, LinkedHashMap.class); + testMap(SortedMap.class, TreeMap.class); + testMap(NavigableMap.class, TreeMap.class); + testMap(MultiValueMap.class, LinkedMultiValueMap.class); // concrete types - assertThat(createMap(HashMap.class, 0)).isInstanceOf(HashMap.class); + testMap(HashMap.class, LinkedHashMap.class); + testMap(LinkedHashMap.class, LinkedHashMap.class); + testMap(TreeMap.class, TreeMap.class); + testMap(LinkedMultiValueMap.class, LinkedMultiValueMap.class); + } - assertThat(createMap(HashMap.class, String.class, 0)).isInstanceOf(HashMap.class); + private void testMap(Class mapType, Class resultType) { + assertThat(CollectionFactory.isApproximableMapType(mapType)).isTrue(); + assertThat(createMap(mapType, 0)).isInstanceOf(resultType); + assertThat(createMap(mapType, String.class, 0)).isInstanceOf(resultType); } @Test From 2c7e8661cfa88afcf75a9d0460ec867e7318d019 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 8 May 2023 12:02:25 +0200 Subject: [PATCH 2/3] Respect TaskDecorator configuration on DefaultManagedTaskExecutor Closes gh-30442 --- .../concurrent/ConcurrentTaskExecutor.java | 12 +++++- .../concurrent/ConcurrentTaskScheduler.java | 9 ++-- .../ConcurrentTaskExecutorTests.java | 41 +++++++++++++++++-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java index 17301854989..f0fd31a8abd 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutor.java @@ -84,6 +84,9 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche private TaskExecutorAdapter adaptedExecutor; + @Nullable + private TaskDecorator taskDecorator; + /** * Create a new ConcurrentTaskExecutor, using a single thread executor as default. @@ -139,6 +142,7 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche * @since 4.3 */ public final void setTaskDecorator(TaskDecorator taskDecorator) { + this.taskDecorator = taskDecorator; this.adaptedExecutor.setTaskDecorator(taskDecorator); } @@ -175,11 +179,15 @@ public class ConcurrentTaskExecutor implements AsyncListenableTaskExecutor, Sche } - private static TaskExecutorAdapter getAdaptedExecutor(Executor concurrentExecutor) { + private TaskExecutorAdapter getAdaptedExecutor(Executor concurrentExecutor) { if (managedExecutorServiceClass != null && managedExecutorServiceClass.isInstance(concurrentExecutor)) { return new ManagedTaskExecutorAdapter(concurrentExecutor); } - return new TaskExecutorAdapter(concurrentExecutor); + TaskExecutorAdapter adapter = new TaskExecutorAdapter(concurrentExecutor); + if (this.taskDecorator != null) { + adapter.setTaskDecorator(this.taskDecorator); + } + return adapter; } diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java index eddc2f58a9a..32e4592b99c 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java @@ -103,7 +103,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler() { super(); - this.scheduledExecutor = initScheduledExecutor(null); + initScheduledExecutor(null); } /** @@ -118,7 +118,7 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler(ScheduledExecutorService scheduledExecutor) { super(scheduledExecutor); - this.scheduledExecutor = initScheduledExecutor(scheduledExecutor); + initScheduledExecutor(scheduledExecutor); } /** @@ -134,11 +134,11 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T */ public ConcurrentTaskScheduler(Executor concurrentExecutor, ScheduledExecutorService scheduledExecutor) { super(concurrentExecutor); - this.scheduledExecutor = initScheduledExecutor(scheduledExecutor); + initScheduledExecutor(scheduledExecutor); } - private ScheduledExecutorService initScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) { + private void initScheduledExecutor(@Nullable ScheduledExecutorService scheduledExecutor) { if (scheduledExecutor != null) { this.scheduledExecutor = scheduledExecutor; this.enterpriseConcurrentScheduler = (managedScheduledExecutorServiceClass != null && @@ -148,7 +148,6 @@ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements T this.scheduledExecutor = Executors.newSingleThreadScheduledExecutor(); this.enterpriseConcurrentScheduler = false; } - return this.scheduledExecutor; } /** diff --git a/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java b/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java index 70830ce0494..bc8ac2ab55a 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/concurrent/ConcurrentTaskExecutorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.scheduling.concurrent; +import java.util.concurrent.Executor; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.RunnableFuture; import java.util.concurrent.ThreadPoolExecutor; @@ -25,6 +26,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.core.task.NoOpRunnable; +import org.springframework.core.task.TaskDecorator; +import org.springframework.util.Assert; import static org.assertj.core.api.Assertions.assertThatCode; @@ -38,8 +41,8 @@ class ConcurrentTaskExecutorTests extends AbstractSchedulingTaskExecutorTests { new ThreadPoolExecutor(1, 1, 60, TimeUnit.SECONDS, new LinkedBlockingQueue<>()); - @Override @SuppressWarnings("deprecation") + @Override protected org.springframework.core.task.AsyncListenableTaskExecutor buildExecutor() { concurrentExecutor.setThreadFactory(new CustomizableThreadFactory(this.threadNamePrefix)); return new ConcurrentTaskExecutor(concurrentExecutor); @@ -69,10 +72,40 @@ class ConcurrentTaskExecutorTests extends AbstractSchedulingTaskExecutorTests { } @Test - void passingNullExecutorToSetterResultsInDefaultTaskExecutorBeingUsed() { + void earlySetConcurrentExecutorCallRespectsConfiguredTaskDecorator() { ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor(); - executor.setConcurrentExecutor(null); + executor.setConcurrentExecutor(new DecoratedExecutor()); + executor.setTaskDecorator(new RunnableDecorator()); assertThatCode(() -> executor.execute(new NoOpRunnable())).doesNotThrowAnyException(); } + @Test + void lateSetConcurrentExecutorCallRespectsConfiguredTaskDecorator() { + ConcurrentTaskExecutor executor = new ConcurrentTaskExecutor(); + executor.setTaskDecorator(new RunnableDecorator()); + executor.setConcurrentExecutor(new DecoratedExecutor()); + assertThatCode(() -> executor.execute(new NoOpRunnable())).doesNotThrowAnyException(); + } + + + private static class DecoratedRunnable implements Runnable { + @Override + public void run() { + } + } + + private static class RunnableDecorator implements TaskDecorator { + @Override + public Runnable decorate(Runnable runnable) { + return new DecoratedRunnable(); + } + } + + private static class DecoratedExecutor implements Executor { + @Override + public void execute(Runnable command) { + Assert.state(command instanceof DecoratedRunnable, "TaskDecorator not applied"); + } + } + } From 4aa8619ac28cc0f5bab8b137deaa6afe1ffa94b2 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 8 May 2023 13:06:19 +0200 Subject: [PATCH 3/3] Upgrade to Tomcat 10.1.8, Jetty 11.0.15 / 12.0.0.beta1, Netty 4.1.92, Jackson 2.14.3, Mockito 5.3.1, Checkstyle 10.10 --- build.gradle | 2 +- framework-platform/framework-platform.gradle | 16 ++++++++-------- spring-web/spring-web.gradle | 4 +++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index 2bd350ca85f..534bcc8f77d 100644 --- a/build.gradle +++ b/build.gradle @@ -78,7 +78,7 @@ configure([rootProject] + javaProjects) { project -> } checkstyle { - toolVersion = "10.9.3" + toolVersion = "10.10.0" configDirectory.set(rootProject.file("src/checkstyle")) } diff --git a/framework-platform/framework-platform.gradle b/framework-platform/framework-platform.gradle index 25356a6d867..cd868155788 100644 --- a/framework-platform/framework-platform.gradle +++ b/framework-platform/framework-platform.gradle @@ -7,19 +7,19 @@ javaPlatform { } dependencies { - api(platform("com.fasterxml.jackson:jackson-bom:2.14.2")) + api(platform("com.fasterxml.jackson:jackson-bom:2.14.3")) api(platform("io.micrometer:micrometer-bom:1.10.6")) - api(platform("io.netty:netty-bom:4.1.91.Final")) + api(platform("io.netty:netty-bom:4.1.92.Final")) api(platform("io.netty:netty5-bom:5.0.0.Alpha5")) api(platform("io.projectreactor:reactor-bom:2022.0.6")) api(platform("io.rsocket:rsocket-bom:1.1.3")) api(platform("org.apache.groovy:groovy-bom:4.0.11")) api(platform("org.apache.logging.log4j:log4j-bom:2.20.0")) - api(platform("org.eclipse.jetty:jetty-bom:11.0.14")) + api(platform("org.eclipse.jetty:jetty-bom:11.0.15")) api(platform("org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.6.4")) api(platform("org.jetbrains.kotlinx:kotlinx-serialization-bom:1.4.0")) api(platform("org.junit:junit-bom:5.9.3")) - api(platform("org.mockito:mockito-bom:5.3.0")) + api(platform("org.mockito:mockito-bom:5.3.1")) constraints { api("com.fasterxml:aalto-xml:1.3.2") @@ -99,10 +99,10 @@ dependencies { api("org.apache.httpcomponents.client5:httpclient5:5.2.1") api("org.apache.httpcomponents.core5:httpcore5-reactive:5.2.1") api("org.apache.poi:poi-ooxml:5.2.3") - api("org.apache.tomcat.embed:tomcat-embed-core:10.1.7") - api("org.apache.tomcat.embed:tomcat-embed-websocket:10.1.7") - api("org.apache.tomcat:tomcat-util:10.1.7") - api("org.apache.tomcat:tomcat-websocket:10.1.7") + api("org.apache.tomcat.embed:tomcat-embed-core:10.1.8") + api("org.apache.tomcat.embed:tomcat-embed-websocket:10.1.8") + api("org.apache.tomcat:tomcat-util:10.1.8") + api("org.apache.tomcat:tomcat-websocket:10.1.8") api("org.aspectj:aspectjrt:1.9.19") api("org.aspectj:aspectjtools:1.9.19") api("org.aspectj:aspectjweaver:1.9.19") diff --git a/spring-web/spring-web.gradle b/spring-web/spring-web.gradle index 0da919218dd..a6289ac8d9d 100644 --- a/spring-web/spring-web.gradle +++ b/spring-web/spring-web.gradle @@ -37,8 +37,10 @@ dependencies { optional("org.eclipse.jetty:jetty-servlet") { exclude group: "jakarta.servlet", module: "jakarta.servlet-api" } - optional("org.eclipse.jetty.ee10:jetty-ee10-servlet:12.0.0.beta0") { + optional("org.eclipse.jetty.ee10:jetty-ee10-servlet:12.0.0.beta1") { exclude group: "jakarta.servlet", module: "jakarta.servlet-api" + exclude group: "org.eclipse.jetty", module: "jetty-ee" + exclude group: "org.eclipse.jetty", module: "jetty-security" exclude group: "org.eclipse.jetty", module: "jetty-server" exclude group: "org.eclipse.jetty", module: "jetty-servlet" }