Skip to content

Commit 439218a

Browse files
committed
core: Validate policy name in defaultLoadBalancingPolicy() immediately
1 parent 111f9a2 commit 439218a

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,11 @@ public T useTransportSecurity() {
274274
* <p>Policy implementations are looked up in the
275275
* {@link LoadBalancerRegistry#getDefaultRegistry default LoadBalancerRegistry}.
276276
*
277+
* <p>The provided policy name is validated against the
278+
* {@link LoadBalancerRegistry#getDefaultRegistry default LoadBalancerRegistry} immediately.
279+
* If no provider is found for the given policy name, an {@link IllegalArgumentException}
280+
* is thrown.
281+
*
277282
* <p>This method is implemented by all stock channel builders that are shipped with gRPC, but may
278283
* not be implemented by custom channel builders, in which case this method will throw.
279284
*

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import io.grpc.InternalChannelz;
4040
import io.grpc.InternalConfiguratorRegistry;
4141
import io.grpc.InternalFeatureFlags;
42+
import io.grpc.LoadBalancerProvider;
43+
import io.grpc.LoadBalancerRegistry;
4244
import io.grpc.ManagedChannel;
4345
import io.grpc.ManagedChannelBuilder;
4446
import io.grpc.MethodDescriptor;
@@ -447,7 +449,9 @@ public ManagedChannelImplBuilder defaultLoadBalancingPolicy(String policy) {
447449
"directServerAddress is set (%s), which forbids the use of load-balancing policy",
448450
directServerAddress);
449451
Preconditions.checkArgument(policy != null, "policy cannot be null");
450-
this.defaultLbPolicy = policy;
452+
LoadBalancerProvider provider = LoadBalancerRegistry.getDefaultRegistry().getProvider(policy);
453+
Preconditions.checkArgument(provider != null, "No provider available for the '%s' load balancing policy.", policy);
454+
this.defaultLbPolicy = provider.getPolicyName();
451455
return this;
452456
}
453457

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
import io.grpc.InternalConfiguratorRegistry;
4444
import io.grpc.InternalFeatureFlags;
4545
import io.grpc.InternalManagedChannelBuilder.InternalInterceptorFactory;
46+
import io.grpc.LoadBalancer;
47+
import io.grpc.LoadBalancerProvider;
48+
import io.grpc.LoadBalancerRegistry;
4649
import io.grpc.ManagedChannel;
4750
import io.grpc.ManagedChannelBuilder;
4851
import io.grpc.MethodDescriptor;
@@ -67,6 +70,7 @@
6770
import java.util.concurrent.Executor;
6871
import java.util.concurrent.TimeUnit;
6972
import java.util.regex.Pattern;
73+
import org.junit.After;
7074
import org.junit.Before;
7175
import org.junit.Rule;
7276
import org.junit.Test;
@@ -85,6 +89,7 @@ public class ManagedChannelImplBuilderTest {
8589
private static final String DUMMY_TARGET = "fake-target";
8690
private static final String DUMMY_AUTHORITY_VALID = "valid:1234";
8791
private static final String DUMMY_AUTHORITY_INVALID = "[ : : 1]";
92+
private static final String FAKE_POLICY_NAME = "magic_balancer";
8893
private static final ClientInterceptor DUMMY_USER_INTERCEPTOR =
8994
new ClientInterceptor() {
9095
@Override
@@ -101,6 +106,29 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
101106
return next.newCall(method, callOptions);
102107
}
103108
};
109+
110+
private final LoadBalancerProvider fakeProvider =
111+
new LoadBalancerProvider() {
112+
@Override
113+
public LoadBalancer newLoadBalancer(LoadBalancer.Helper helper) {
114+
return mock(LoadBalancer.class);
115+
}
116+
117+
@Override
118+
public boolean isAvailable() {
119+
return true;
120+
}
121+
122+
@Override
123+
public int getPriority() {
124+
return 5;
125+
}
126+
127+
@Override
128+
public String getPolicyName() {
129+
return FAKE_POLICY_NAME;
130+
}
131+
};
104132

105133
@Parameters(name = "enableRfc3986UrisParam={0}")
106134
public static Iterable<Object[]> data() {
@@ -141,6 +169,13 @@ public void setUp() throws Exception {
141169
DUMMY_TARGET,
142170
new UnsupportedClientTransportFactoryBuilder(),
143171
new FixedPortProvider(DUMMY_PORT));
172+
173+
LoadBalancerRegistry.getDefaultRegistry().register(fakeProvider);
174+
}
175+
176+
@After
177+
public void tearDown() {
178+
LoadBalancerRegistry.getDefaultRegistry().deregister(fakeProvider);
144179
}
145180

146181
/** Ensure getDefaultPort() returns default port when no custom implementation provided. */
@@ -249,6 +284,14 @@ public void nameResolverFactory_notAllowedWithDirectAddress() {
249284
directAddressBuilder.nameResolverFactory(mock(NameResolver.Factory.class));
250285
}
251286

287+
@Test
288+
public void defaultLoadBalancingPolicy_unregisteredPolicy() {
289+
assertThrows(
290+
IllegalArgumentException.class,
291+
() -> builder.defaultLoadBalancingPolicy("unregistered_balancer")
292+
);
293+
}
294+
252295
@Test
253296
public void defaultLoadBalancingPolicy_default() {
254297
assertEquals("pick_first", builder.defaultLbPolicy);

0 commit comments

Comments
 (0)