From c772eb0f4e6cf34697821137359c6960d8210fd8 Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Mon, 21 Mar 2022 14:29:02 -0700 Subject: [PATCH] rls: fix wrong grpcKeybuilder field name (#8999) The `grpcKeybuilders` [field](https://github.com/grpc/grpc-proto/blob/9fb243ce29b2e2576d3ef4fa2bd00ecba36aee31/grpc/lookup/v1/rls_config.proto#L176) should not be `grpcKeyBuilders` in json format. The mistake in Java was introduced since the [beginning](https://github.com/grpc/grpc-java/commit/0fd4975d4c519056ec027dadc04872365369866e#diff-585b634c79155b4ac9417f7805e1b9d5f6d5c11a940c88e27fdf53c209e619cfR104). --- .../java/io/grpc/rls/RlsProtoConverters.java | 10 ++++---- .../main/java/io/grpc/rls/RlsProtoData.java | 4 ++-- .../java/io/grpc/rls/RlsRequestFactory.java | 2 +- .../io/grpc/rls/CachingRlsLbClientTest.java | 2 +- .../java/io/grpc/rls/RlsLoadBalancerTest.java | 2 +- .../io/grpc/rls/RlsProtoConvertersTest.java | 24 +++++++++---------- .../io/grpc/rls/RlsRequestFactoryTest.java | 2 +- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java index 14e3e50bd4..d3b4496cf7 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoConverters.java @@ -109,12 +109,12 @@ final class RlsProtoConverters { @Override protected RouteLookupConfig doForward(Map json) { - ImmutableList grpcKeyBuilders = + ImmutableList grpcKeybuilders = GrpcKeyBuilderConverter.covertAll( - checkNotNull(JsonUtil.getListOfObjects(json, "grpcKeyBuilders"), "grpcKeyBuilders")); - checkArgument(!grpcKeyBuilders.isEmpty(), "must have at least one GrpcKeyBuilder"); + checkNotNull(JsonUtil.getListOfObjects(json, "grpcKeybuilders"), "grpcKeybuilders")); + checkArgument(!grpcKeybuilders.isEmpty(), "must have at least one GrpcKeyBuilder"); Set names = new HashSet<>(); - for (GrpcKeyBuilder keyBuilder : grpcKeyBuilders) { + for (GrpcKeyBuilder keyBuilder : grpcKeybuilders) { for (Name name : keyBuilder.names()) { checkArgument(names.add(name), "duplicate names in grpc_keybuilders: " + name); } @@ -147,7 +147,7 @@ final class RlsProtoConverters { cacheSize = Math.min(cacheSize, MAX_CACHE_SIZE); String defaultTarget = Strings.emptyToNull(JsonUtil.getString(json, "defaultTarget")); return RouteLookupConfig.builder() - .grpcKeyBuilders(grpcKeyBuilders) + .grpcKeybuilders(grpcKeybuilders) .lookupService(lookupService) .lookupServiceTimeoutInNanos(timeout) .maxAgeInNanos(maxAge) diff --git a/rls/src/main/java/io/grpc/rls/RlsProtoData.java b/rls/src/main/java/io/grpc/rls/RlsProtoData.java index 929b780075..49f32c6b6e 100644 --- a/rls/src/main/java/io/grpc/rls/RlsProtoData.java +++ b/rls/src/main/java/io/grpc/rls/RlsProtoData.java @@ -75,7 +75,7 @@ final class RlsProtoData { * keyed by name. If no GrpcKeyBuilder matches, an empty key_map will be sent to the lookup * service; it should likely reply with a global default route and raise an alert. */ - abstract ImmutableList grpcKeyBuilders(); + abstract ImmutableList grpcKeybuilders(); /** * Returns the name of the lookup service as a gRPC URI. Typically, this will be a subdomain of @@ -119,7 +119,7 @@ final class RlsProtoData { @AutoValue.Builder abstract static class Builder { - abstract Builder grpcKeyBuilders(ImmutableList grpcKeyBuilders); + abstract Builder grpcKeybuilders(ImmutableList grpcKeybuilders); abstract Builder lookupService(String lookupService); diff --git a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java index 4a32bece2b..2b314585d7 100644 --- a/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java +++ b/rls/src/main/java/io/grpc/rls/RlsRequestFactory.java @@ -50,7 +50,7 @@ final class RlsRequestFactory { private static Map createKeyBuilderTable( RouteLookupConfig config) { Map table = new HashMap<>(); - for (GrpcKeyBuilder grpcKeyBuilder : config.grpcKeyBuilders()) { + for (GrpcKeyBuilder grpcKeyBuilder : config.grpcKeybuilders()) { for (Name name : grpcKeyBuilder.names()) { boolean hasMethod = name.method() == null || name.method().isEmpty(); String method = hasMethod ? "*" : name.method(); diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 588bddcaa6..15c58fa120 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -428,7 +428,7 @@ public class CachingRlsLbClientTest { private static RouteLookupConfig getRouteLookupConfig() { return RouteLookupConfig.builder() - .grpcKeyBuilders(ImmutableList.of( + .grpcKeybuilders(ImmutableList.of( GrpcKeyBuilder.create( ImmutableList.of(Name.create("service1", "create")), ImmutableList.of( diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java index 5dfdf948d4..9470301715 100644 --- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java @@ -392,7 +392,7 @@ public class RlsLoadBalancerTest { private String getRlsConfigJsonStr() { return "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" diff --git a/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java b/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java index 215f8f2ac0..98b7101fd5 100644 --- a/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsProtoConvertersTest.java @@ -101,7 +101,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -177,7 +177,7 @@ public class RlsProtoConvertersTest { RouteLookupConfig expectedConfig = RouteLookupConfig.builder() - .grpcKeyBuilders(ImmutableList.of( + .grpcKeybuilders(ImmutableList.of( GrpcKeyBuilder.create( ImmutableList.of(Name.create("service1", "create")), ImmutableList.of( @@ -216,7 +216,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_emptyKeyBuilders() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [],\n" + + " \"grpcKeybuilders\": [],\n" + " \"lookupService\": \"service1\",\n" + " \"lookupServiceTimeout\": \"2s\",\n" + " \"maxAge\": \"300s\",\n" @@ -240,7 +240,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_namesNotUnique() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -329,7 +329,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_defaultValues() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -345,7 +345,7 @@ public class RlsProtoConvertersTest { RouteLookupConfig expectedConfig = RouteLookupConfig.builder() - .grpcKeyBuilders(ImmutableList.of( + .grpcKeybuilders(ImmutableList.of( GrpcKeyBuilder.create( ImmutableList.of(Name.create("service1", null)), ImmutableList.of(), @@ -369,7 +369,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_staleAgeCappedByMaxAge() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -402,7 +402,7 @@ public class RlsProtoConvertersTest { RouteLookupConfig expectedConfig = RouteLookupConfig.builder() - .grpcKeyBuilders(ImmutableList.of( + .grpcKeybuilders(ImmutableList.of( GrpcKeyBuilder.create( ImmutableList.of(Name.create("service1", "create")), ImmutableList.of( @@ -428,7 +428,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_staleAgeGivenWithoutMaxAge() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -472,7 +472,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_keyBuilderWithoutName() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"headers\": [\n" + " {\n" @@ -510,7 +510,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_nameWithoutService() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" @@ -553,7 +553,7 @@ public class RlsProtoConvertersTest { @Test public void convert_jsonRlsConfig_keysNotUnique() throws IOException { String jsonStr = "{\n" - + " \"grpcKeyBuilders\": [\n" + + " \"grpcKeybuilders\": [\n" + " {\n" + " \"names\": [\n" + " {\n" diff --git a/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java b/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java index 82e8416563..6ee2c01af8 100644 --- a/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsRequestFactoryTest.java @@ -37,7 +37,7 @@ public class RlsRequestFactoryTest { private static final RouteLookupConfig RLS_CONFIG = RouteLookupConfig.builder() - .grpcKeyBuilders(ImmutableList.of( + .grpcKeybuilders(ImmutableList.of( GrpcKeyBuilder.create( ImmutableList.of(Name.create("com.google.service1", "Create")), ImmutableList.of(