Skip to content

Commit 94b9ef9

Browse files
onobcdsyer
onobc
authored and
dsyer
committed
Split service discoverer and configurer
This commit simplifies the DefaultGrpcServiceDiscoverer by removing its dependence on the DefaultGrpcServiceConfigurer. Now the expectation is the discoverer returns a spec of the service definition rather than a bound service. The configurer then accepts that spec and binds and configures it. See #208 Signed-off-by: Chris Bono <[email protected]>
1 parent 7dd7cbd commit 94b9ef9

File tree

15 files changed

+183
-140
lines changed

15 files changed

+183
-140
lines changed

‎spring-grpc-core/src/main/java/org/springframework/grpc/server/service/DefaultGrpcServiceConfigurer.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 the original author or authors.
2+
* Copyright 2023-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,8 +31,8 @@
3131
import io.grpc.ServerServiceDefinition;
3232

3333
/**
34-
* Default {@link GrpcServiceConfigurer} that binds and configures services with
35-
* interceptors.
34+
* Default {@link GrpcServiceConfigurer} implementation that binds and configures services
35+
* with interceptors.
3636
*
3737
* @author Chris Bono
3838
*/
@@ -52,8 +52,8 @@ public void afterPropertiesSet() {
5252
}
5353

5454
@Override
55-
public ServerServiceDefinition configure(BindableService bindableService, @Nullable GrpcServiceInfo serviceInfo) {
56-
return bindInterceptors(bindableService, serviceInfo);
55+
public ServerServiceDefinition configure(ServerServiceDefinitionSpec serviceDefinitionSpec) {
56+
return bindInterceptors(serviceDefinitionSpec.service(), serviceDefinitionSpec.serviceInfo());
5757
}
5858

5959
private List<ServerInterceptor> findGlobalInterceptors() {

‎spring-grpc-core/src/main/java/org/springframework/grpc/server/service/DefaultGrpcServiceDiscoverer.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 the original author or authors.
2+
* Copyright 2023-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,32 +24,41 @@
2424

2525
import io.grpc.BindableService;
2626
import io.grpc.ServerServiceDefinition;
27+
import io.grpc.ServiceDescriptor;
2728

2829
/**
29-
* The default {@link GrpcServiceDiscoverer} that finds all {@link BindableService} beans
30-
* and configures and binds them.
30+
* Default {@link GrpcServiceDiscoverer} implementation that finds all
31+
* {@link BindableService} beans in the application context.
3132
*
3233
* @author Chris Bono
3334
*/
3435
public class DefaultGrpcServiceDiscoverer implements GrpcServiceDiscoverer {
3536

36-
private final GrpcServiceConfigurer serviceConfigurer;
37-
3837
private final ApplicationContext applicationContext;
3938

40-
public DefaultGrpcServiceDiscoverer(GrpcServiceConfigurer serviceConfigurer,
41-
ApplicationContext applicationContext) {
42-
this.serviceConfigurer = serviceConfigurer;
39+
public DefaultGrpcServiceDiscoverer(ApplicationContext applicationContext) {
4340
this.applicationContext = applicationContext;
4441
}
4542

4643
@Override
47-
public List<ServerServiceDefinition> findServices() {
44+
public List<ServerServiceDefinitionSpec> findServices() {
4845
return ApplicationContextBeanLookupUtils
4946
.getOrderedBeansWithAnnotation(this.applicationContext, BindableService.class, GrpcService.class)
5047
.entrySet()
5148
.stream()
52-
.map((e) -> this.serviceConfigurer.configure(e.getKey(), this.serviceInfo(e.getValue())))
49+
.map((e) -> new ServerServiceDefinitionSpec(e.getKey(), this.serviceInfo(e.getValue())))
50+
.toList();
51+
}
52+
53+
@Override
54+
public List<String> listServiceNames() {
55+
return ApplicationContextBeanLookupUtils
56+
.getOrderedBeansWithAnnotation(this.applicationContext, BindableService.class, GrpcService.class)
57+
.keySet()
58+
.stream()
59+
.map(BindableService::bindService)
60+
.map(ServerServiceDefinition::getServiceDescriptor)
61+
.map(ServiceDescriptor::getName)
5362
.toList();
5463
}
5564

‎spring-grpc-core/src/main/java/org/springframework/grpc/server/service/GrpcServiceConfigurer.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 the original author or authors.
2+
* Copyright 2023-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,25 +16,25 @@
1616

1717
package org.springframework.grpc.server.service;
1818

19-
import org.springframework.lang.Nullable;
20-
21-
import io.grpc.BindableService;
2219
import io.grpc.ServerServiceDefinition;
2320

2421
/**
25-
* Configures and binds a {@link BindableService gRPC Service}.
22+
* Configures and binds a {@link ServerServiceDefinitionSpec service spec} into a
23+
* {@link ServerServiceDefinition service definition} that can then be added to a gRPC
24+
* server.
2625
*
2726
* @author Chris Bono
2827
*/
2928
@FunctionalInterface
3029
public interface GrpcServiceConfigurer {
3130

3231
/**
33-
* Configure and bind a gRPC service.
34-
* @param bindableService service to bind and configure
35-
* @param serviceInfo optional additional service information
36-
* @return configured service definition
32+
* Configure and bind a gRPC service spec resulting in a service definition that can
33+
* then be added to a gRPC server.
34+
* @param serviceSpec the spec containing the info about the service
35+
* @return bound and configured service definition that is ready to be added to the
36+
* server
3737
*/
38-
ServerServiceDefinition configure(BindableService bindableService, @Nullable GrpcServiceInfo serviceInfo);
38+
ServerServiceDefinition configure(ServerServiceDefinitionSpec serviceSpec);
3939

4040
}

‎spring-grpc-core/src/main/java/org/springframework/grpc/server/service/GrpcServiceDiscoverer.java‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2024 the original author or authors.
2+
* Copyright 2023-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,44 +18,26 @@
1818

1919
import java.util.List;
2020

21-
import io.grpc.ServerServiceDefinition;
22-
2321
/**
24-
* Discovers {@link ServerServiceDefinition gRPC services} to be provided by the server.
22+
* Discovers gRPC services to be provided by the server.
2523
*
2624
* @author Michael ([email protected])
2725
* @author Chris Bono
2826
*/
29-
@FunctionalInterface
3027
public interface GrpcServiceDiscoverer {
3128

3229
/**
33-
* Find gRPC services for the server to provide.
34-
* @return list of services to add to the server - empty when no services available
30+
* Find the specs of the available gRPC services. The spec can then be passed into a
31+
* {@link GrpcServiceConfigurer service configurer} to bind and configure an actual
32+
* service definition.
33+
* @return list of service specs - empty when no services available
3534
*/
36-
List<ServerServiceDefinition> findServices();
35+
List<ServerServiceDefinitionSpec> findServices();
3736

3837
/**
39-
* Find gRPC service names.
38+
* Find the names of the available gRPC services.
4039
* @return list of service names - empty when no services available
4140
*/
42-
default List<String> listServiceNames() {
43-
return findServices().stream()
44-
.map(ServerServiceDefinition::getServiceDescriptor)
45-
.map(descriptor -> descriptor.getName())
46-
.toList();
47-
}
48-
49-
/**
50-
* Find gRPC service.
51-
* @param name the service name
52-
* @return a service - null if no service has this name
53-
*/
54-
default ServerServiceDefinition findService(String name) {
55-
return findServices().stream()
56-
.filter(service -> service.getServiceDescriptor().getName().equals(name))
57-
.findFirst()
58-
.orElse(null);
59-
}
41+
List<String> listServiceNames();
6042

6143
}

‎spring-grpc-core/src/main/java/org/springframework/grpc/server/service/ServerServiceDefinitionSpec.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2024-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.grpc.server.service;
17+
18+
import org.springframework.lang.Nullable;
19+
import org.springframework.util.Assert;
20+
21+
import io.grpc.BindableService;
22+
import io.grpc.ServerServiceDefinition;
23+
24+
/**
25+
* Encapsulates enough information to construct an actual {@link ServerServiceDefinition}.
26+
*
27+
* @param service the bindable service
28+
* @param serviceInfo optional additional information about the service (e.g.
29+
* interceptors)
30+
* @author Chris Bono
31+
*/
32+
public record ServerServiceDefinitionSpec(BindableService service, @Nullable GrpcServiceInfo serviceInfo) {
33+
public ServerServiceDefinitionSpec {
34+
Assert.notNull(service, "service must not be null");
35+
}
36+
37+
}

‎spring-grpc-core/src/test/java/org/springframework/grpc/server/service/DefaultGrpcServiceConfigurerTests.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,13 @@ private void customizeContextAndRunServiceConfigurerWithServiceInfo(
108108
.run((context) -> {
109109
DefaultGrpcServiceConfigurer configurer = context.getBean(DefaultGrpcServiceConfigurer.class);
110110
if (expectedExceptionType != null) {
111-
assertThatThrownBy(() -> configurer.configure(service, serviceInfo))
111+
assertThatThrownBy(
112+
() -> configurer.configure(new ServerServiceDefinitionSpec(service, serviceInfo)))
112113
.isInstanceOf(expectedExceptionType);
113114
serverInterceptorsMocked.verifyNoInteractions();
114115
}
115116
else {
116-
configurer.configure(service, serviceInfo);
117+
configurer.configure(new ServerServiceDefinitionSpec(service, serviceInfo));
117118
serverInterceptorsMocked
118119
.verify(() -> ServerInterceptors.interceptForward(serviceDef, expectedInterceptors));
119120
}

‎spring-grpc-core/src/test/java/org/springframework/grpc/server/service/DefaultGrpcServiceDiscovererTests.java‎

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,10 @@
1717
package org.springframework.grpc.server.service;
1818

1919
import static org.assertj.core.api.Assertions.assertThat;
20-
import static org.springframework.grpc.server.service.DefaultGrpcServiceDiscovererTests.DefaultGrpcServiceDiscovererTestsConfig.SERVICE_A;
21-
import static org.springframework.grpc.server.service.DefaultGrpcServiceDiscovererTests.DefaultGrpcServiceDiscovererTestsConfig.SERVICE_B;
22-
23-
import java.util.LinkedHashMap;
24-
import java.util.Map;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.when;
22+
import static org.springframework.grpc.server.service.DefaultGrpcServiceDiscovererTests.DefaultGrpcServiceDiscovererTestsServiceConfig.SERVICE_A;
23+
import static org.springframework.grpc.server.service.DefaultGrpcServiceDiscovererTests.DefaultGrpcServiceDiscovererTestsServiceConfig.SERVICE_B;
2524

2625
import org.assertj.core.api.InstanceOfAssertFactories;
2726
import org.junit.jupiter.api.Test;
@@ -35,6 +34,7 @@
3534

3635
import io.grpc.BindableService;
3736
import io.grpc.ServerServiceDefinition;
37+
import io.grpc.ServiceDescriptor;
3838

3939
/**
4040
* Tests for {@link DefaultGrpcServiceDiscoverer}.
@@ -43,28 +43,55 @@
4343
*/
4444
class DefaultGrpcServiceDiscovererTests {
4545

46+
@Test
47+
void whenNoServicesRegisteredThenListServiceNamesReturnsEmptyList() {
48+
new ApplicationContextRunner().withUserConfiguration(DefaultGrpcServiceDiscovererTestsBaseConfig.class)
49+
.run((context) -> assertThat(context).getBean(DefaultGrpcServiceDiscoverer.class)
50+
.extracting(DefaultGrpcServiceDiscoverer::listServiceNames, InstanceOfAssertFactories.LIST)
51+
.isEmpty());
52+
}
53+
54+
@Test
55+
void whenServicesRegisteredThenListServiceNamesReturnsNames() {
56+
new ApplicationContextRunner()
57+
.withUserConfiguration(DefaultGrpcServiceDiscovererTestsBaseConfig.class,
58+
DefaultGrpcServiceDiscovererTestsServiceConfig.class)
59+
.run((context) -> assertThat(context).getBean(DefaultGrpcServiceDiscoverer.class)
60+
.extracting(DefaultGrpcServiceDiscoverer::listServiceNames, InstanceOfAssertFactories.LIST)
61+
.containsExactly("serviceB", "serviceA"));
62+
63+
}
64+
4665
@Test
4766
void servicesAreFoundInProperOrderWithExpectedGrpcServiceAnnotations() {
48-
new ApplicationContextRunner().withUserConfiguration(DefaultGrpcServiceDiscovererTestsConfig.class)
49-
.run((context) -> {
50-
assertThat(context).getBean(DefaultGrpcServiceDiscoverer.class)
51-
.extracting(DefaultGrpcServiceDiscoverer::findServices, InstanceOfAssertFactories.LIST)
52-
.containsExactly(DefaultGrpcServiceDiscovererTestsConfig.SERVICE_DEF_B,
53-
DefaultGrpcServiceDiscovererTestsConfig.SERVICE_DEF_A);
54-
TestServiceConfigurer configurer = context.getBean(TestServiceConfigurer.class);
55-
assertThat(configurer.invocations).hasSize(2);
56-
assertThat(configurer.invocations.keySet()).containsExactly(SERVICE_B, SERVICE_A);
57-
assertThat(configurer.invocations).containsEntry(SERVICE_B, null);
58-
assertThat(configurer.invocations).hasEntrySatisfying(SERVICE_A, (serviceInfo) -> {
59-
assertThat(serviceInfo.interceptors()).isEmpty();
60-
assertThat(serviceInfo.interceptorNames()).isEmpty();
61-
assertThat(serviceInfo.blendWithGlobalInterceptors()).isFalse();
62-
});
63-
});
67+
new ApplicationContextRunner()
68+
.withUserConfiguration(DefaultGrpcServiceDiscovererTestsBaseConfig.class,
69+
DefaultGrpcServiceDiscovererTestsServiceConfig.class)
70+
.run((context) -> assertThat(context).getBean(DefaultGrpcServiceDiscoverer.class)
71+
.extracting(DefaultGrpcServiceDiscoverer::findServices,
72+
InstanceOfAssertFactories.list(ServerServiceDefinitionSpec.class))
73+
.satisfies((serviceSpecs) -> {
74+
assertThat(serviceSpecs).hasSize(2);
75+
assertThat(serviceSpecs).element(0).isEqualTo(new ServerServiceDefinitionSpec(SERVICE_B, null));
76+
assertThat(serviceSpecs).element(1).satisfies((spec) -> {
77+
assertThat(spec.service()).isEqualTo(SERVICE_A);
78+
assertThat(spec.serviceInfo()).isNotNull();
79+
});
80+
}));
6481
}
6582

6683
@Configuration(proxyBeanMethods = false)
67-
static class DefaultGrpcServiceDiscovererTestsConfig {
84+
static class DefaultGrpcServiceDiscovererTestsBaseConfig {
85+
86+
@Bean
87+
GrpcServiceDiscoverer grpcServiceDiscoverer(ApplicationContext applicationContext) {
88+
return new DefaultGrpcServiceDiscoverer(applicationContext);
89+
}
90+
91+
}
92+
93+
@Configuration(proxyBeanMethods = false)
94+
static class DefaultGrpcServiceDiscovererTestsServiceConfig {
6895

6996
static BindableService SERVICE_A = Mockito.mock();
7097

@@ -74,44 +101,27 @@ static class DefaultGrpcServiceDiscovererTestsConfig {
74101

75102
static ServerServiceDefinition SERVICE_DEF_B = Mockito.mock();
76103

77-
@Bean
78-
TestServiceConfigurer testServiceConfigurer() {
79-
return new TestServiceConfigurer();
80-
}
81-
82-
@Bean
83-
GrpcServiceDiscoverer grpcServiceDiscoverer(GrpcServiceConfigurer grpcServiceConfigurer,
84-
ApplicationContext applicationContext) {
85-
return new DefaultGrpcServiceDiscoverer(grpcServiceConfigurer, applicationContext);
86-
}
87-
88104
@GrpcService
89105
@Bean
90106
@Order(200)
91107
BindableService serviceA() {
92-
Mockito.when(SERVICE_A.bindService()).thenReturn(SERVICE_DEF_A);
108+
ServiceDescriptor descriptor = mock();
109+
when(descriptor.getName()).thenReturn("serviceA");
110+
when(SERVICE_DEF_A.getServiceDescriptor()).thenReturn(descriptor);
111+
when(SERVICE_A.bindService()).thenReturn(SERVICE_DEF_A);
93112
return SERVICE_A;
94113
}
95114

96115
@Bean
97116
@Order(100)
98117
BindableService serviceB() {
99-
Mockito.when(SERVICE_B.bindService()).thenReturn(SERVICE_DEF_B);
118+
ServiceDescriptor descriptor = mock();
119+
when(descriptor.getName()).thenReturn("serviceB");
120+
when(SERVICE_DEF_B.getServiceDescriptor()).thenReturn(descriptor);
121+
when(SERVICE_B.bindService()).thenReturn(SERVICE_DEF_B);
100122
return SERVICE_B;
101123
}
102124

103125
}
104126

105-
static class TestServiceConfigurer implements GrpcServiceConfigurer {
106-
107-
Map<BindableService, GrpcServiceInfo> invocations = new LinkedHashMap<>();
108-
109-
@Override
110-
public ServerServiceDefinition configure(BindableService bindableService, GrpcServiceInfo serviceInfo) {
111-
invocations.put(bindableService, serviceInfo);
112-
return bindableService.bindService();
113-
}
114-
115-
}
116-
117127
}

0 commit comments

Comments
 (0)