rls: fix RlcProto parsing issues (#6822)

This commit is contained in:
Jihun Cho 2020-03-12 18:46:05 -07:00 committed by GitHub
parent a680c982f5
commit b72477e282
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 20 additions and 8 deletions

View File

@ -17,6 +17,7 @@
package io.grpc.rls.internal; package io.grpc.rls.internal;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Converter; import com.google.common.base.Converter;
import io.grpc.internal.JsonUtil; import io.grpc.internal.JsonUtil;
@ -31,6 +32,7 @@ import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
/** /**
* RlsProtoConverters is a collection of {@link Converter} between RouteLookupService proto / json * RlsProtoConverters is a collection of {@link Converter} between RouteLookupService proto / json
@ -104,14 +106,17 @@ public final class RlsProtoConverters {
.covertAll(JsonUtil.checkObjectList(JsonUtil.getList(json, "grpcKeyBuilders"))); .covertAll(JsonUtil.checkObjectList(JsonUtil.getList(json, "grpcKeyBuilders")));
String lookupService = JsonUtil.getString(json, "lookupService"); String lookupService = JsonUtil.getString(json, "lookupService");
long timeout = long timeout =
TimeUnit.SECONDS.toMillis(JsonUtil.getNumberAsLong(json, "lookupServiceTimeout")); TimeUnit.SECONDS.toMillis(
orDefault(
JsonUtil.getNumberAsLong(json, "lookupServiceTimeout"),
0L));
Long maxAge = Long maxAge =
convertTimeIfNotNull( convertTimeIfNotNull(
TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "maxAge")); TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "maxAge"));
Long staleAge = Long staleAge =
convertTimeIfNotNull( convertTimeIfNotNull(
TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "staleAge")); TimeUnit.SECONDS, TimeUnit.MILLISECONDS, JsonUtil.getNumberAsLong(json, "staleAge"));
long cacheSize = JsonUtil.getNumberAsLong(json, "cacheSizeBytes"); long cacheSize = orDefault(JsonUtil.getNumberAsLong(json, "cacheSizeBytes"), Long.MAX_VALUE);
List<String> validTargets = JsonUtil.checkStringList(JsonUtil.getList(json, "validTargets")); List<String> validTargets = JsonUtil.checkStringList(JsonUtil.getList(json, "validTargets"));
String defaultTarget = JsonUtil.getString(json, "defaultTarget"); String defaultTarget = JsonUtil.getString(json, "defaultTarget");
RequestProcessingStrategy strategy = RequestProcessingStrategy strategy =
@ -129,6 +134,13 @@ public final class RlsProtoConverters {
strategy); strategy);
} }
private static <T> T orDefault(@Nullable T value, T defaultValue) {
if (value == null) {
return checkNotNull(defaultValue, "defaultValue");
}
return value;
}
private static Long convertTimeIfNotNull(TimeUnit from, TimeUnit to, Long value) { private static Long convertTimeIfNotNull(TimeUnit from, TimeUnit to, Long value) {
if (value == null) { if (value == null) {
return null; return null;

View File

@ -238,10 +238,10 @@ public final class RlsProtoData {
this.requestProcessingStrategy = requestProcessingStrategy; this.requestProcessingStrategy = requestProcessingStrategy;
checkNotNull(requestProcessingStrategy, "requestProcessingStrategy"); checkNotNull(requestProcessingStrategy, "requestProcessingStrategy");
checkState( checkState(
(requestProcessingStrategy == RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR !((requestProcessingStrategy == RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR
|| requestProcessingStrategy || requestProcessingStrategy
== RequestProcessingStrategy.ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS) == RequestProcessingStrategy.ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS)
&& !defaultTarget.isEmpty(), && defaultTarget.isEmpty()),
"defaultTarget cannot be empty if strategy is %s", "defaultTarget cannot be empty if strategy is %s",
requestProcessingStrategy); requestProcessingStrategy);
} }
@ -417,10 +417,10 @@ public final class RlsProtoData {
private final boolean optional; private final boolean optional;
NameMatcher(String key, List<String> names, boolean optional) { NameMatcher(String key, List<String> names, @Nullable Boolean optional) {
this.key = checkNotNull(key, "key"); this.key = checkNotNull(key, "key");
this.names = ImmutableList.copyOf(checkNotNull(names, "names")); this.names = ImmutableList.copyOf(checkNotNull(names, "names"));
this.optional = optional; this.optional = optional != null ? optional : true;
} }
/** The name that will be used in the RLS key_map to refer to this value. */ /** The name that will be used in the RLS key_map to refer to this value. */

View File

@ -173,7 +173,7 @@ public class RlsProtoConvertersTest {
+ " \"validTargets\": [\"a valid target\"]," + " \"validTargets\": [\"a valid target\"],"
+ " \"cacheSizeBytes\": 1000,\n" + " \"cacheSizeBytes\": 1000,\n"
+ " \"defaultTarget\": \"us_east_1.cloudbigtable.googleapis.com\",\n" + " \"defaultTarget\": \"us_east_1.cloudbigtable.googleapis.com\",\n"
+ " \"requestProcessingStrategy\": \"ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS\"\n" + " \"requestProcessingStrategy\": \"SYNC_LOOKUP_CLIENT_SEES_ERROR\"\n"
+ "}"; + "}";
RouteLookupConfig expectedConfig = RouteLookupConfig expectedConfig =
@ -200,7 +200,7 @@ public class RlsProtoConvertersTest {
/* cacheSize= */ 1000, /* cacheSize= */ 1000,
/* validTargets= */ ImmutableList.of("a valid target"), /* validTargets= */ ImmutableList.of("a valid target"),
/* defaultTarget= */ "us_east_1.cloudbigtable.googleapis.com", /* defaultTarget= */ "us_east_1.cloudbigtable.googleapis.com",
RequestProcessingStrategy.ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS); RequestProcessingStrategy.SYNC_LOOKUP_CLIENT_SEES_ERROR);
RouteLookupConfigConverter converter = new RouteLookupConfigConverter(); RouteLookupConfigConverter converter = new RouteLookupConfigConverter();
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")