Skip to content

Commit b72477e

Browse files
authored
rls: fix RlcProto parsing issues (#6822)
1 parent a680c98 commit b72477e

File tree

3 files changed

+20
-8
lines changed

3 files changed

+20
-8
lines changed

rls/src/main/java/io/grpc/rls/internal/RlsProtoConverters.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.rls.internal;
1818

1919
import static com.google.common.base.Preconditions.checkArgument;
20+
import static com.google.common.base.Preconditions.checkNotNull;
2021

2122
import com.google.common.base.Converter;
2223
import io.grpc.internal.JsonUtil;
@@ -31,6 +32,7 @@
3132
import java.util.List;
3233
import java.util.Map;
3334
import java.util.concurrent.TimeUnit;
35+
import javax.annotation.Nullable;
3436

3537
/**
3638
* RlsProtoConverters is a collection of {@link Converter} between RouteLookupService proto / json
@@ -104,14 +106,17 @@ protected RouteLookupConfig doForward(Map<String, ?> json) {
104106
.covertAll(JsonUtil.checkObjectList(JsonUtil.getList(json, "grpcKeyBuilders")));
105107
String lookupService = JsonUtil.getString(json, "lookupService");
106108
long timeout =
107-
TimeUnit.SECONDS.toMillis(JsonUtil.getNumberAsLong(json, "lookupServiceTimeout"));
109+
TimeUnit.SECONDS.toMillis(
110+
orDefault(
111+
JsonUtil.getNumberAsLong(json, "lookupServiceTimeout"),
112+
0L));
108113
Long maxAge =
109114
convertTimeIfNotNull(
110115
TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "maxAge"));
111116
Long staleAge =
112117
convertTimeIfNotNull(
113118
TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "staleAge"));
114-
long cacheSize = JsonUtil.getNumberAsLong(json, "cacheSizeBytes");
119+
long cacheSize = orDefault(JsonUtil.getNumberAsLong(json, "cacheSizeBytes"), Long.MAX_VALUE);
115120
List<String> validTargets = JsonUtil.checkStringList(JsonUtil.getList(json, "validTargets"));
116121
String defaultTarget = JsonUtil.getString(json, "defaultTarget");
117122
RequestProcessingStrategy strategy =
@@ -129,6 +134,13 @@ protected RouteLookupConfig doForward(Map<String, ?> json) {
129134
strategy);
130135
}
131136

137+
private static <T> T orDefault(@Nullable T value, T defaultValue) {
138+
if (value == null) {
139+
return checkNotNull(defaultValue, "defaultValue");
140+
}
141+
return value;
142+
}
143+
132144
private static Long convertTimeIfNotNull(TimeUnit from, TimeUnit to, Long value) {
133145
if (value == null) {
134146
return null;

rls/src/main/java/io/grpc/rls/internal/RlsProtoData.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,10 @@ public RouteLookupConfig(
238238
this.requestProcessingStrategy = requestProcessingStrategy;
239239
checkNotNull(requestProcessingStrategy, "requestProcessingStrategy");
240240
checkState(
241-
(requestProcessingStrategy == RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR
241+
!((requestProcessingStrategy == RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR
242242
|| requestProcessingStrategy
243243
== RequestProcessingStrategy.ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS)
244-
&& !defaultTarget.isEmpty(),
244+
&& defaultTarget.isEmpty()),
245245
"defaultTarget cannot be empty if strategy is %s",
246246
requestProcessingStrategy);
247247
}
@@ -417,10 +417,10 @@ static final class NameMatcher {
417417

418418
private final boolean optional;
419419

420-
NameMatcher(String key, List<String> names, boolean optional) {
420+
NameMatcher(String key, List<String> names, @Nullable Boolean optional) {
421421
this.key = checkNotNull(key, "key");
422422
this.names = ImmutableList.copyOf(checkNotNull(names, "names"));
423-
this.optional = optional;
423+
this.optional = optional != null ? optional : true;
424424
}
425425

426426
/** The name that will be used in the RLS key_map to refer to this value. */

rls/src/test/java/io/grpc/rls/internal/RlsProtoConvertersTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void convert_jsonRlsConfig() throws IOException {
173173
+ " \"validTargets\": [\"a valid target\"],"
174174
+ " \"cacheSizeBytes\": 1000,\n"
175175
+ " \"defaultTarget\": \"us_east_1.cloudbigtable.googleapis.com\",\n"
176-
+ " \"requestProcessingStrategy\": \"ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS\"\n"
176+
+ " \"requestProcessingStrategy\": \"SYNC_LOOKUP_CLIENT_SEES_ERROR\"\n"
177177
+ "}";
178178

179179
RouteLookupConfig expectedConfig =
@@ -200,7 +200,7 @@ public void convert_jsonRlsConfig() throws IOException {
200200
/* cacheSize= */ 1000,
201201
/* validTargets= */ ImmutableList.of("a valid target"),
202202
/* defaultTarget= */ "us_east_1.cloudbigtable.googleapis.com",
203-
RequestProcessingStrategy.ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS);
203+
RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR);
204204

205205
RouteLookupConfigConverter converter = new RouteLookupConfigConverter();
206206
@SuppressWarnings("unchecked")

0 commit comments

Comments
 (0)