Skip to content

Commit 7384ea9

Browse files
ejona86ejona86
committed
core: Simplify ServerImpl constructor; testable Builder
1 parent 361f038 commit 7384ea9

3 files changed

Lines changed: 110 additions & 110 deletions

File tree

‎core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java‎

Lines changed: 47 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package io.grpc.internal;
1818

19-
import static com.google.common.base.MoreObjects.firstNonNull;
2019
import static com.google.common.base.Preconditions.checkNotNull;
2120

2221
import com.google.common.annotations.VisibleForTesting;
@@ -52,7 +51,9 @@
5251
public abstract class AbstractServerImplBuilder<T extends AbstractServerImplBuilder<T>>
5352
extends ServerBuilder<T> {
5453

55-
private static final HandlerRegistry EMPTY_FALLBACK_REGISTRY = new HandlerRegistry() {
54+
private static final ObjectPool<? extends Executor> DEFAULT_EXECUTOR_POOL =
55+
SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
56+
private static final HandlerRegistry DEFAULT_FALLBACK_REGISTRY = new HandlerRegistry() {
5657
@Override
5758
public List<ServerServiceDefinition> getServices() {
5859
return Collections.emptyList();
@@ -64,32 +65,32 @@ public List<ServerServiceDefinition> getServices() {
6465
return null;
6566
}
6667
};
68+
private static final DecompressorRegistry DEFAULT_DECOMPRESSOR_REGISTRY =
69+
DecompressorRegistry.getDefaultInstance();
70+
private static final CompressorRegistry DEFAULT_COMPRESSOR_REGISTRY =
71+
CompressorRegistry.getDefaultInstance();
6772

68-
private final InternalHandlerRegistry.Builder registryBuilder =
73+
final InternalHandlerRegistry.Builder registryBuilder =
6974
new InternalHandlerRegistry.Builder();
7075

71-
private final List<ServerTransportFilter> transportFilters =
76+
final List<ServerTransportFilter> transportFilters =
7277
new ArrayList<ServerTransportFilter>();
7378

74-
private final List<ServerInterceptor> interceptors = new ArrayList<ServerInterceptor>();
79+
final List<ServerInterceptor> interceptors = new ArrayList<ServerInterceptor>();
7580

7681
private final List<InternalNotifyOnServerBuild> notifyOnBuildList =
7782
new ArrayList<InternalNotifyOnServerBuild>();
7883

7984
private final List<ServerStreamTracer.Factory> streamTracerFactories =
8085
new ArrayList<ServerStreamTracer.Factory>();
8186

82-
@Nullable
83-
private HandlerRegistry fallbackRegistry;
87+
HandlerRegistry fallbackRegistry = DEFAULT_FALLBACK_REGISTRY;
8488

85-
@Nullable
86-
private Executor executor;
89+
ObjectPool<? extends Executor> executorPool = DEFAULT_EXECUTOR_POOL;
8790

88-
@Nullable
89-
private DecompressorRegistry decompressorRegistry;
91+
DecompressorRegistry decompressorRegistry = DEFAULT_DECOMPRESSOR_REGISTRY;
9092

91-
@Nullable
92-
private CompressorRegistry compressorRegistry;
93+
CompressorRegistry compressorRegistry = DEFAULT_COMPRESSOR_REGISTRY;
9394

9495
@Nullable
9596
private StatsContextFactory statsFactory;
@@ -101,7 +102,11 @@ public final T directExecutor() {
101102

102103
@Override
103104
public final T executor(@Nullable Executor executor) {
104-
this.executor = executor;
105+
if (executor != null) {
106+
this.executorPool = new FixedObjectPool<Executor>(executor);
107+
} else {
108+
this.executorPool = DEFAULT_EXECUTOR_POOL;
109+
}
105110
return thisT();
106111
}
107112

@@ -139,19 +144,31 @@ public final T addStreamTracerFactory(ServerStreamTracer.Factory factory) {
139144

140145
@Override
141146
public final T fallbackHandlerRegistry(HandlerRegistry registry) {
142-
this.fallbackRegistry = registry;
147+
if (registry != null) {
148+
this.fallbackRegistry = registry;
149+
} else {
150+
this.fallbackRegistry = DEFAULT_FALLBACK_REGISTRY;
151+
}
143152
return thisT();
144153
}
145154

146155
@Override
147156
public final T decompressorRegistry(DecompressorRegistry registry) {
148-
decompressorRegistry = registry;
157+
if (registry != null) {
158+
decompressorRegistry = registry;
159+
} else {
160+
decompressorRegistry = DEFAULT_DECOMPRESSOR_REGISTRY;
161+
}
149162
return thisT();
150163
}
151164

152165
@Override
153166
public final T compressorRegistry(CompressorRegistry registry) {
154-
compressorRegistry = registry;
167+
if (registry != null) {
168+
compressorRegistry = registry;
169+
} else {
170+
compressorRegistry = DEFAULT_COMPRESSOR_REGISTRY;
171+
}
155172
return thisT();
156173
}
157174

@@ -166,6 +183,18 @@ protected T statsContextFactory(StatsContextFactory statsFactory) {
166183

167184
@Override
168185
public Server build() {
186+
ServerImpl server = new ServerImpl(
187+
this,
188+
SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE),
189+
buildTransportServer(Collections.unmodifiableList(getTracerFactories())),
190+
Context.ROOT);
191+
for (InternalNotifyOnServerBuild notifyTarget : notifyOnBuildList) {
192+
notifyTarget.notifyOnBuild(server);
193+
}
194+
return server;
195+
}
196+
197+
private List<ServerStreamTracer.Factory> getTracerFactories() {
169198
ArrayList<ServerStreamTracer.Factory> tracerFactories =
170199
new ArrayList<ServerStreamTracer.Factory>();
171200
StatsContextFactory statsFactory =
@@ -180,37 +209,7 @@ public Server build() {
180209
new CensusTracingModule(Tracing.getTracer(), Tracing.getBinaryPropagationHandler());
181210
tracerFactories.add(censusTracing.getServerTracerFactory());
182211
tracerFactories.addAll(streamTracerFactories);
183-
184-
io.grpc.internal.InternalServer transportServer =
185-
buildTransportServer(Collections.unmodifiableList(tracerFactories));
186-
ServerImpl server = new ServerImpl(getExecutorPool(),
187-
SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE), registryBuilder.build(),
188-
firstNonNull(fallbackRegistry, EMPTY_FALLBACK_REGISTRY), transportServer,
189-
Context.ROOT, firstNonNull(decompressorRegistry, DecompressorRegistry.getDefaultInstance()),
190-
firstNonNull(compressorRegistry, CompressorRegistry.getDefaultInstance()),
191-
transportFilters, interceptors);
192-
for (InternalNotifyOnServerBuild notifyTarget : notifyOnBuildList) {
193-
notifyTarget.notifyOnBuild(server);
194-
}
195-
return server;
196-
}
197-
198-
private ObjectPool<? extends Executor> getExecutorPool() {
199-
final Executor savedExecutor = executor;
200-
if (savedExecutor == null) {
201-
return SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
202-
}
203-
return new ObjectPool<Executor>() {
204-
@Override
205-
public Executor getObject() {
206-
return savedExecutor;
207-
}
208-
209-
@Override
210-
public Executor returnObject(Object object) {
211-
return null;
212-
}
213-
};
212+
return tracerFactories;
214213
}
215214

216215
/**

‎core/src/main/java/io/grpc/internal/ServerImpl.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -110,25 +110,26 @@ public final class ServerImpl extends io.grpc.Server implements WithLogId {
110110
* @param fallbackRegistry the secondary method registry, used only if the primary registry
111111
* doesn't have the method
112112
*/
113-
ServerImpl(ObjectPool<? extends Executor> executorPool,
113+
ServerImpl(
114+
AbstractServerImplBuilder<?> builder,
114115
ObjectPool<ScheduledExecutorService> timeoutServicePool,
115-
InternalHandlerRegistry registry, HandlerRegistry fallbackRegistry,
116-
InternalServer transportServer, Context rootContext,
117-
DecompressorRegistry decompressorRegistry, CompressorRegistry compressorRegistry,
118-
List<ServerTransportFilter> transportFilters, List<ServerInterceptor> interceptors) {
119-
this.executorPool = Preconditions.checkNotNull(executorPool, "executorPool");
116+
InternalServer transportServer,
117+
Context rootContext) {
118+
this.executorPool = Preconditions.checkNotNull(builder.executorPool, "executorPool");
120119
this.timeoutServicePool = Preconditions.checkNotNull(timeoutServicePool, "timeoutServicePool");
121-
this.registry = Preconditions.checkNotNull(registry, "registry");
122-
this.fallbackRegistry = Preconditions.checkNotNull(fallbackRegistry, "fallbackRegistry");
120+
this.registry = Preconditions.checkNotNull(builder.registryBuilder.build(), "registryBuilder");
121+
this.fallbackRegistry =
122+
Preconditions.checkNotNull(builder.fallbackRegistry, "fallbackRegistry");
123123
this.transportServer = Preconditions.checkNotNull(transportServer, "transportServer");
124124
// Fork from the passed in context so that it does not propagate cancellation, it only
125125
// inherits values.
126126
this.rootContext = Preconditions.checkNotNull(rootContext, "rootContext").fork();
127-
this.decompressorRegistry = decompressorRegistry;
128-
this.compressorRegistry = compressorRegistry;
127+
this.decompressorRegistry = builder.decompressorRegistry;
128+
this.compressorRegistry = builder.compressorRegistry;
129129
this.transportFilters = Collections.unmodifiableList(
130-
new ArrayList<ServerTransportFilter>(transportFilters));
131-
this.interceptors = interceptors.toArray(new ServerInterceptor[interceptors.size()]);
130+
new ArrayList<ServerTransportFilter>(builder.transportFilters));
131+
this.interceptors =
132+
builder.interceptors.toArray(new ServerInterceptor[builder.interceptors.size()]);
132133
}
133134

134135
/**

0 commit comments

Comments
 (0)