From 302f04ecf32ae6c1687cf82b3cca475bea37b272 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 31 Mar 2025 10:11:07 +0000 Subject: [PATCH] Replace Map argument with GroupsMetadata In preparation for HTTP Service registry AOT support. See gh-33992 --- .../AbstractHttpServiceRegistrar.java | 163 +++--------------- .../web/service/registry/GroupsMetadata.java | 160 +++++++++++++++++ .../service/registry/HttpServiceGroup.java | 10 +- .../HttpServiceProxyRegistryFactoryBean.java | 4 +- .../registry/HttpServiceRegistrarTests.java | 21 ++- 5 files changed, 211 insertions(+), 147 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java b/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java index 1bcfca89447..a7ace071771 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/AbstractHttpServiceRegistrar.java @@ -16,13 +16,6 @@ package org.springframework.web.service.registry; -import java.util.Collection; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; - import org.jspecify.annotations.Nullable; import org.springframework.beans.BeansException; @@ -45,7 +38,6 @@ import org.springframework.core.type.MethodMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.filter.AnnotationTypeFilter; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.web.service.annotation.HttpExchange; @@ -91,13 +83,13 @@ public abstract class AbstractHttpServiceRegistrar implements private @Nullable BeanFactory beanFactory; - private final Map groupMap = new LinkedHashMap<>(); + private final GroupsMetadata groupsMetadata = new GroupsMetadata(); private @Nullable ClassPathScanningCandidateComponentProvider scanner; /** - * Set the client type to use when the client type for an HTTP Service group + * Set the client type to use when an HTTP Service group's client type * remains {@link HttpServiceGroup.ClientType#UNSPECIFIED}. *

By default, when this property is not set, then {@code REST_CLIENT} * is used for any HTTP Service group whose client type remains unspecified. @@ -141,7 +133,7 @@ public abstract class AbstractHttpServiceRegistrar implements proxyRegistryBeanDef = new GenericBeanDefinition(); proxyRegistryBeanDef.setBeanClass(HttpServiceProxyRegistryFactoryBean.class); ConstructorArgumentValues args = proxyRegistryBeanDef.getConstructorArgumentValues(); - args.addIndexedArgumentValue(0, new LinkedHashMap()); + args.addIndexedArgumentValue(0, new GroupsMetadata()); beanRegistry.registerBeanDefinition(proxyRegistryBeanName, proxyRegistryBeanDef); } else { @@ -150,9 +142,10 @@ public abstract class AbstractHttpServiceRegistrar implements mergeGroups(proxyRegistryBeanDef); - this.groupMap.forEach((groupName, group) -> group.httpServiceTypeNames().forEach(type -> { + this.groupsMetadata.forEachRegistration(group -> group.httpServiceTypeNames().forEach(type -> { GenericBeanDefinition proxyBeanDef = new GenericBeanDefinition(); proxyBeanDef.setBeanClassName(type); + String groupName = group.name(); String beanName = (groupName + "#" + type); proxyBeanDef.setInstanceSupplier(() -> getProxyInstance(proxyRegistryBeanName, groupName, type)); if (!beanRegistry.containsBeanDefinition(beanName)) { @@ -182,50 +175,23 @@ public abstract class AbstractHttpServiceRegistrar implements return this.scanner; } - @SuppressWarnings("unchecked") private void mergeGroups(GenericBeanDefinition proxyRegistryBeanDef) { ConstructorArgumentValues args = proxyRegistryBeanDef.getConstructorArgumentValues(); - ConstructorArgumentValues.ValueHolder valueHolder = args.getArgumentValue(0, Map.class); - Assert.state(valueHolder != null, "Expected Map constructor argument at index 0"); - Map targetMap = (Map) valueHolder.getValue(); - Assert.state(targetMap != null, "No constructor argument value"); - - this.groupMap.forEach((name, group) -> { - RegisteredGroup previousGroup = targetMap.putIfAbsent(name, group); - if (previousGroup != null) { - if (!compatibleClientTypes(group.clientType(), previousGroup.clientType())) { - throw new IllegalArgumentException("ClientType conflict for group '" + name + "'"); - } - previousGroup.addHttpServiceTypeNames(group.httpServiceTypeNames()); - } - }); - } - - private static boolean compatibleClientTypes( - HttpServiceGroup.ClientType clientTypeA, HttpServiceGroup.ClientType clientTypeB) { - - return (clientTypeA == clientTypeB || - clientTypeA == HttpServiceGroup.ClientType.UNSPECIFIED || - clientTypeB == HttpServiceGroup.ClientType.UNSPECIFIED); + ConstructorArgumentValues.ValueHolder valueHolder = args.getArgumentValue(0, GroupsMetadata.class); + Assert.state(valueHolder != null, "Expected GroupsMetadata constructor argument at index 0"); + GroupsMetadata target = (GroupsMetadata) valueHolder.getValue(); + Assert.state(target != null, "No constructor argument value"); + target.mergeWith(this.groupsMetadata); } - private Object getProxyInstance(String registryBeanName, String groupName, String type) { + private Object getProxyInstance(String registryBeanName, String groupName, String httpServiceType) { Assert.state(this.beanFactory != null, "BeanFactory has not been set"); HttpServiceProxyRegistry registry = this.beanFactory.getBean(registryBeanName, HttpServiceProxyRegistry.class); - Object proxy = registry.getClient(groupName, loadClass(type)); - Assert.notNull(proxy, "No proxy for HTTP Service [" + type + "]"); + Object proxy = registry.getClient(groupName, GroupsMetadata.loadClass(httpServiceType)); + Assert.notNull(proxy, "No proxy for HTTP Service [" + httpServiceType + "]"); return proxy; } - private static Class loadClass(String type) { - try { - return ClassUtils.forName(type, AbstractHttpServiceRegistrar.class.getClassLoader()); - } - catch (ClassNotFoundException ex) { - throw new IllegalStateException("Failed to load '" + type + "'", ex); - } - } - /** * Registry API to allow subclasses to register HTTP Services. @@ -287,32 +253,23 @@ public abstract class AbstractHttpServiceRegistrar implements return new DefaultGroupSpec(name, clientType); } + /** + * Default implementation of {@link GroupSpec}. + */ private class DefaultGroupSpec implements GroupSpec { - private final String groupName; - - private final HttpServiceGroup.ClientType clientType; + private final GroupsMetadata.Registration registration; public DefaultGroupSpec(String groupName, HttpServiceGroup.ClientType clientType) { - this.groupName = groupName; - this.clientType = initClientType(clientType); - } - - private HttpServiceGroup.ClientType initClientType(HttpServiceGroup.ClientType clientType) { - if (clientType != HttpServiceGroup.ClientType.UNSPECIFIED) { - return clientType; - } - else if (defaultClientType != HttpServiceGroup.ClientType.UNSPECIFIED) { - return defaultClientType; - } - else { - return HttpServiceGroup.ClientType.REST_CLIENT; - } + clientType = (clientType != HttpServiceGroup.ClientType.UNSPECIFIED ? clientType : defaultClientType); + this.registration = groupsMetadata.getOrCreateGroup(groupName, clientType); } @Override public GroupSpec register(Class... serviceTypes) { - getOrCreateGroup().addHttpServiceTypes(serviceTypes); + for (Class serviceType : serviceTypes) { + this.registration.httpServiceTypeNames().add(serviceType.getName()); + } return this; } @@ -335,84 +292,10 @@ public abstract class AbstractHttpServiceRegistrar implements private void detect(String packageName) { for (BeanDefinition definition : getScanner().findCandidateComponents(packageName)) { if (definition.getBeanClassName() != null) { - getOrCreateGroup().addHttpServiceTypeName(definition.getBeanClassName()); + this.registration.httpServiceTypeNames().add(definition.getBeanClassName()); } } } - - private RegisteredGroup getOrCreateGroup() { - return groupMap.computeIfAbsent(this.groupName, name -> new RegisteredGroup(name, this.clientType)); - } - } - } - - - /** - * A simple holder of registered HTTP Service type names, deferring the - * loading of classes until {@link #httpServiceTypes()} is called. - */ - private static class RegisteredGroup implements HttpServiceGroup { - - private final String name; - - private final Set httpServiceTypeNames = new LinkedHashSet<>(); - - private final ClientType clientType; - - public RegisteredGroup(String name, ClientType clientType) { - this.name = name; - this.clientType = clientType; - } - - @Override - public String name() { - return this.name; - } - - public Set httpServiceTypeNames() { - return this.httpServiceTypeNames; - } - - @Override - public Set> httpServiceTypes() { - return this.httpServiceTypeNames.stream() - .map(AbstractHttpServiceRegistrar::loadClass) - .collect(Collectors.toSet()); - } - - @Override - public ClientType clientType() { - return this.clientType; - } - - public void addHttpServiceTypes(Class... httpServiceTypes) { - for (Class type : httpServiceTypes) { - this.httpServiceTypeNames.add(type.getName()); - } - } - - public void addHttpServiceTypeNames(Collection httpServiceTypeNames) { - this.httpServiceTypeNames.addAll(httpServiceTypeNames); - } - - public void addHttpServiceTypeName(String httpServiceTypeName) { - this.httpServiceTypeNames.add(httpServiceTypeName); - } - - @Override - public final boolean equals(Object other) { - return (other instanceof RegisteredGroup otherGroup && this.name.equals(otherGroup.name)); - } - - @Override - public int hashCode() { - return this.name.hashCode(); - } - - @Override - public String toString() { - return "RegisteredGroup[name='" + this.name + "', httpServiceTypes=" + - this.httpServiceTypeNames + ", clientType=" + this.clientType + "]"; } } diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java b/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java new file mode 100644 index 00000000000..083095e20ac --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/service/registry/GroupsMetadata.java @@ -0,0 +1,160 @@ +/* + * Copyright 2002-2025 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. + * You may obtain a copy of the License at + * + * https://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. + */ + +package org.springframework.web.service.registry; + +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + +/** + * Container for HTTP Service type registrations, initially storing HTTP Service + * type names as {@link Registration}s, and later exposing access to those + * registrations as {@link HttpServiceGroup}s via {@link #groups()}. + * + * @author Rossen Stoyanchev + * @since 7.0 + */ +final class GroupsMetadata { + + private final Map groupMap = new LinkedHashMap<>(); + + + /** + * Create a registration for the given group name, or return an existing + * registration. If there is an existing registration, merge the client + * types after checking they don't conflict. + */ + public Registration getOrCreateGroup(String groupName, HttpServiceGroup.ClientType clientType) { + return this.groupMap.computeIfAbsent(groupName, name -> new DefaultRegistration(name, clientType)) + .clientType(clientType); + } + + /** + * Merge all registrations from the given {@link GroupsMetadata} into this one. + */ + public void mergeWith(GroupsMetadata other) { + other.forEachRegistration(registration -> + getOrCreateGroup(registration.name(), registration.clientType()) + .httpServiceTypeNames() + .addAll(registration.httpServiceTypeNames())); + } + + public void forEachRegistration(Consumer consumer) { + this.groupMap.values().forEach(consumer); + } + + public Collection groups() { + return this.groupMap.values().stream().map(DefaultRegistration::toHttpServiceGroup).toList(); + } + + public static Class loadClass(String type) { + try { + return ClassUtils.forName(type, GroupsMetadata.class.getClassLoader()); + } + catch (ClassNotFoundException ex) { + throw new IllegalStateException("Failed to load '" + type + "'", ex); + } + } + + + /** + * Registration metadata for an {@link HttpServiceGroup}. + */ + interface Registration { + + String name(); + + HttpServiceGroup.ClientType clientType(); + + Set httpServiceTypeNames(); + } + + + /** + * Default implementation of {@link Registration}. + */ + private static class DefaultRegistration implements Registration { + + private final String name; + + private HttpServiceGroup.ClientType clientType; + + private final Set typeNames = new LinkedHashSet<>(); + + DefaultRegistration(String name, HttpServiceGroup.ClientType clientType) { + this.name = name; + this.clientType = clientType; + } + + @Override + public String name() { + return this.name; + } + + @Override + public HttpServiceGroup.ClientType clientType() { + return this.clientType; + } + + @Override + public Set httpServiceTypeNames() { + return this.typeNames; + } + + /** + * Update the client type if it does not conflict with the existing value. + */ + public DefaultRegistration clientType(HttpServiceGroup.ClientType other) { + if (this.clientType.isUnspecified()) { + this.clientType = other; + } + else { + Assert.isTrue(this.clientType == other || other.isUnspecified(), + "ClientType conflict for HttpServiceGroup '" + this.name + "'"); + } + return this; + } + + /** + * Create the {@link HttpServiceGroup} from the metadata. + */ + public HttpServiceGroup toHttpServiceGroup() { + return new RegisteredGroup(this.name, + (this.clientType.isUnspecified() ? HttpServiceGroup.ClientType.REST_CLIENT : this.clientType), + this.typeNames.stream().map(GroupsMetadata::loadClass).collect(Collectors.toSet())); + } + + @Override + public String toString() { + return "Group '" + this.name + "', ClientType." + this.clientType + ", " + this.typeNames; + } + } + + + private record RegisteredGroup(String name, ClientType clientType, Set> httpServiceTypes) + implements HttpServiceGroup { + + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceGroup.java b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceGroup.java index 75d89015b65..0f8c1453f85 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceGroup.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceGroup.java @@ -73,7 +73,15 @@ public interface HttpServiceGroup { * @see HttpServiceGroups#clientType() * @see AbstractHttpServiceRegistrar#setDefaultClientType */ - UNSPECIFIED + UNSPECIFIED; + + + /** + * Shortcut to check if this is the UNSPECIFIED enum value. + */ + boolean isUnspecified() { + return (this == UNSPECIFIED); + } } } diff --git a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java index 77233855ae9..177ca207f0b 100644 --- a/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java +++ b/spring-web/src/main/java/org/springframework/web/service/registry/HttpServiceProxyRegistryFactoryBean.java @@ -69,8 +69,8 @@ public final class HttpServiceProxyRegistryFactoryBean private @Nullable HttpServiceProxyRegistry proxyRegistry; - HttpServiceProxyRegistryFactoryBean(Map groupMap) { - this.groupSet = groupMap.values().stream() + HttpServiceProxyRegistryFactoryBean(GroupsMetadata groupsMetadata) { + this.groupSet = groupsMetadata.groups().stream() .map(group -> { HttpServiceGroupAdapter adapter = groupAdapters.get(group.clientType()); Assert.state(adapter != null, "No HttpServiceGroupAdapter for type " + group.clientType()); diff --git a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java index 0a09efd78c1..fa6c2bcc4fe 100644 --- a/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/registry/HttpServiceRegistrarTests.java @@ -19,6 +19,8 @@ package org.springframework.web.service.registry; import java.util.Map; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; @@ -98,6 +100,17 @@ public class HttpServiceRegistrarTests { registry -> registry.forGroup(ECHO_GROUP, ClientType.WEB_CLIENT).register(EchoB.class))); } + @Test + void mergeWithClientTypeOverride() { + doRegister( + registry -> registry.forGroup(ECHO_GROUP).register(EchoA.class), + registry -> registry.forGroup(ECHO_GROUP, ClientType.WEB_CLIENT).register(EchoA.class)); + + assertRegistryBeanDef(new TestGroup(ECHO_GROUP, ClientType.WEB_CLIENT, EchoA.class)); + assertProxyBeanDef(ECHO_GROUP, EchoA.class); + assertBeanDefinitionCount(2); + } + @Test void defaultClientType() { doRegister(ClientType.WEB_CLIENT, registry -> registry.forGroup(ECHO_GROUP).register(EchoA.class)); @@ -135,7 +148,6 @@ public class HttpServiceRegistrarTests { } } - @SuppressWarnings("unchecked") private Map groupMap() { BeanDefinition beanDef = this.beanDefRegistry.getBeanDefinition("httpServiceProxyRegistry"); assertThat(beanDef.getBeanClassName()).isEqualTo(HttpServiceProxyRegistryFactoryBean.class.getName()); @@ -144,10 +156,11 @@ public class HttpServiceRegistrarTests { ConstructorArgumentValues.ValueHolder valueHolder = args.getArgumentValue(0, Map.class); assertThat(valueHolder).isNotNull(); - Map groupMap = (Map) valueHolder.getValue(); - assertThat(groupMap).isNotNull(); + GroupsMetadata metadata = (GroupsMetadata) valueHolder.getValue(); + assertThat(metadata).isNotNull(); - return groupMap; + return metadata.groups().stream() + .collect(Collectors.toMap(HttpServiceGroup::name, Function.identity())); } private void assertProxyBeanDef(String group, Class httpServiceType) {