Skip to content

Commit b8bd9f1

Browse files
AgraVatorAgraVator
committed
Fix: suggested changes
1 parent 4ff8d40 commit b8bd9f1

20 files changed

Lines changed: 172 additions & 130 deletions

‎api/src/main/java/io/grpc/ChannelConfigurator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
* <p>Implementations must be thread-safe as the configure methods may be invoked concurrently
4747
* by multiple internal components.
4848
*
49-
* @since 1.81.0
49+
* @since 1.83.0
5050
*/
5151
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574")
5252
public interface ChannelConfigurator {
@@ -59,5 +59,5 @@ public interface ChannelConfigurator {
5959
*
6060
* @param builder the mutable channel builder for the new child channel
6161
*/
62-
default void configureChannelBuilder(ManagedChannelBuilder<?> builder) {}
62+
void configureChannelBuilder(ManagedChannelBuilder<?> builder);
6363
}

‎api/src/main/java/io/grpc/ForwardingServerBuilder.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,6 @@ public T setBinaryLog(BinaryLog binaryLog) {
192192
return thisT();
193193
}
194194

195-
@Override
196-
public T childChannelConfigurator(ChannelConfigurator channelConfigurator) {
197-
delegate().childChannelConfigurator(channelConfigurator);
198-
return thisT();
199-
}
200-
201195
/**
202196
* Returns the {@link Server} built by the delegate by default. Overriding method can return
203197
* different value.

‎api/src/main/java/io/grpc/ManagedChannelBuilder.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ protected T interceptWithTarget(InterceptorFactory factory) {
168168
throw new UnsupportedOperationException();
169169
}
170170

171+
/** Internal-only. */
172+
@Internal
171173
protected interface InterceptorFactory {
172174
ClientInterceptor newInterceptor(String target);
173175
}
@@ -636,7 +638,8 @@ public T disableServiceConfigLookUp() {
636638
* @return this
637639
* @since 1.64.0
638640
*/
639-
public T addMetricSink(MetricSink metricSink) {
641+
@Internal
642+
protected T addMetricSink(MetricSink metricSink) {
640643
throw new UnsupportedOperationException();
641644
}
642645

@@ -668,7 +671,7 @@ public <X> T setNameResolverArg(NameResolver.Args.Key<X> key, X value) {
668671
*
669672
* @param channelConfigurator the configurator to apply.
670673
* @return this
671-
* @since 1.81.0
674+
* @since 1.83.0
672675
*/
673676
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574")
674677
public T childChannelConfigurator(ChannelConfigurator channelConfigurator) {

‎api/src/main/java/io/grpc/NameResolver.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ public static final class Args {
358358
private final MetricRecorder metricRecorder;
359359
@Nullable private final NameResolverRegistry nameResolverRegistry;
360360
@Nullable private final IdentityHashMap<Key<?>, Object> customArgs;
361-
@Nullable private final ChannelConfigurator channelConfigurator;
361+
private final ChannelConfigurator channelConfigurator;
362362

363363
private Args(Builder builder) {
364364
this.defaultPort = checkNotNull(builder.defaultPort, "defaultPort not set");
@@ -476,9 +476,8 @@ public ChannelLogger getChannelLogger() {
476476
/**
477477
* Returns the configurator for child channels.
478478
*
479-
* @since 1.81.0
479+
* @since 1.83.0
480480
*/
481-
@Nullable
482481
@Internal
483482
public ChannelConfigurator getChildChannelConfigurator() {
484483
return channelConfigurator;
@@ -592,7 +591,7 @@ public static final class Builder {
592591
private MetricRecorder metricRecorder;
593592
private NameResolverRegistry nameResolverRegistry;
594593
private IdentityHashMap<Key<?>, Object> customArgs;
595-
private ChannelConfigurator channelConfigurator = new ChannelConfigurator() {};
594+
private ChannelConfigurator channelConfigurator = builder -> { };
596595

597596
Builder() {
598597
}
@@ -711,10 +710,10 @@ public Builder setNameResolverRegistry(NameResolverRegistry registry) {
711710
/**
712711
* See {@link Args#getChildChannelConfigurator()}. This is an optional field.
713712
*
714-
* @since 1.81.0
713+
* @since 1.83.0
715714
*/
716715
public Builder setChildChannelConfigurator(ChannelConfigurator channelConfigurator) {
717-
this.channelConfigurator = channelConfigurator;
716+
this.channelConfigurator = checkNotNull(channelConfigurator, "channelConfigurator");
718717
return this;
719718
}
720719

‎api/src/main/java/io/grpc/ServerBuilder.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -425,21 +425,7 @@ public T setBinaryLog(BinaryLog binaryLog) {
425425
}
426426

427427

428-
/**
429-
* Sets a configurator that will be applied to all internal child channels created by this server.
430-
*
431-
* <p>This allows injecting configuration (like credentials, interceptors, or flow control)
432-
* into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections
433-
* or OOB load balancing channels.
434-
*
435-
* @param channelConfigurator the configurator to apply.
436-
* @return this
437-
* @since 1.81.0
438-
*/
439-
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574")
440-
public T childChannelConfigurator(ChannelConfigurator channelConfigurator) {
441-
throw new UnsupportedOperationException("Not implemented");
442-
}
428+
443429

444430

445431
/**

‎api/src/test/java/io/grpc/ChannelConfiguratorTest.java‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎api/src/test/java/io/grpc/NameResolverTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void args() {
105105
}
106106

107107
private NameResolver.Args createArgs() {
108-
ChannelConfigurator channelConfigurator = new ChannelConfigurator() {};
108+
ChannelConfigurator channelConfigurator = builder -> { };
109109
return NameResolver.Args.newBuilder()
110110
.setDefaultPort(defaultPort)
111111
.setProxyDetector(proxyDetector)

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public Result selectConfig(PickSubchannelArgs args) {
162162
* <p>This is intended for use by gRPC internal components
163163
* that are responsible for creating auxiliary {@code ManagedChannel} instances.
164164
*/
165-
private ChannelConfigurator channelConfigurator = new ChannelConfigurator() {};
165+
private final ChannelConfigurator channelConfigurator;
166166

167167
private final InternalLogId logId;
168168
private final String target;
@@ -554,9 +554,8 @@ ClientStream newSubstream(
554554
Supplier<Stopwatch> stopwatchSupplier,
555555
List<ClientInterceptor> interceptors,
556556
final TimeProvider timeProvider) {
557-
if (builder.channelConfigurator != null) {
558-
this.channelConfigurator = builder.channelConfigurator;
559-
}
557+
this.channelConfigurator = checkNotNull(builder.channelConfigurator,
558+
"channelConfigurator");
560559
this.target = checkNotNull(builder.target, "target");
561560
this.logId = InternalLogId.allocate("Channel", target);
562561
this.timeProvider = checkNotNull(timeProvider, "timeProvider");
@@ -1501,9 +1500,7 @@ protected ManagedChannelBuilder<?> delegate() {
15011500

15021501
// Note that we follow the global configurator pattern and try to fuse the configurations as
15031502
// soon as the builder gets created
1504-
if (channelConfigurator != null) {
1505-
channelConfigurator.configureChannelBuilder(builder);
1506-
}
1503+
channelConfigurator.configureChannelBuilder(builder);
15071504

15081505
return builder
15091506
// TODO(zdapeng): executors should not outlive the parent channel.

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,7 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
128128

129129
private static final Method GET_CLIENT_INTERCEPTOR_METHOD;
130130

131-
ChannelConfigurator channelConfigurator = new ChannelConfigurator() {};
132131

133-
@Override
134-
public ManagedChannelImplBuilder childChannelConfigurator(
135-
ChannelConfigurator channelConfigurator) {
136-
this.channelConfigurator = checkNotNull(channelConfigurator,
137-
"childChannelConfigurator");
138-
return this;
139-
}
140132

141133
static {
142134
Method getClientInterceptorMethod = null;
@@ -160,6 +152,8 @@ public ManagedChannelImplBuilder childChannelConfigurator(
160152
}
161153

162154

155+
ChannelConfigurator channelConfigurator = builder -> { };
156+
163157
ObjectPool<? extends Executor> executorPool = DEFAULT_EXECUTOR_POOL;
164158

165159
ObjectPool<? extends Executor> offloadExecutorPool = DEFAULT_EXECUTOR_POOL;
@@ -728,6 +722,14 @@ public ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) {
728722
return this;
729723
}
730724

725+
@Override
726+
public ManagedChannelImplBuilder childChannelConfigurator(
727+
ChannelConfigurator channelConfigurator) {
728+
this.channelConfigurator = checkNotNull(channelConfigurator,
729+
"childChannelConfigurator");
730+
return this;
731+
}
732+
731733
@Override
732734
public ManagedChannel build() {
733735
ClientTransportFactory clientTransportFactory =

‎core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,7 @@ public void setNameResolverExtArgs() {
786786

787787
@Test
788788
public void childChannelConfigurator_setsField() {
789-
ChannelConfigurator configurator = new ChannelConfigurator() {};
789+
ChannelConfigurator configurator = builder -> { };
790790
assertSame(builder, builder.childChannelConfigurator(configurator));
791791
assertSame(configurator, builder.channelConfigurator);
792792
}
@@ -811,13 +811,10 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
811811
};
812812

813813
// Define the Configurator
814-
ChannelConfigurator configurator = new ChannelConfigurator() {
815-
@Override
816-
public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
817-
builder.addMetricSink(mockMetricSink);
814+
ChannelConfigurator configurator = builder -> {
815+
InternalManagedChannelBuilder.addMetricSink(builder, mockMetricSink);
818816

819-
InternalManagedChannelBuilder.interceptWithTarget(builder, target -> mockInterceptor);
820-
}
817+
InternalManagedChannelBuilder.interceptWithTarget(builder, target -> mockInterceptor);
821818
};
822819

823820
// Use NameResolver.Factory to capture Args

0 commit comments

Comments
 (0)