From 0244418d2d1cd29e30b9beba23d8efcb5746bc9a Mon Sep 17 00:00:00 2001 From: Kun Zhang Date: Wed, 10 Apr 2019 16:28:23 -0700 Subject: [PATCH] core: Move ConfigOrError up level up. (#5578) This class is used in other places than just NameResolver.Helper. It should not be an inner class of Helper. Strictly speaking this is an API-breaking change. However, this is part of the service config error handling API that hasn't been done yet. Nobody has a legitimate reason to use it. --- .../java/io/grpc/LoadBalancerProvider.java | 2 +- core/src/main/java/io/grpc/NameResolver.java | 174 +++++++++--------- .../AutoConfiguredLoadBalancerFactory.java | 2 +- .../io/grpc/internal/DnsNameResolver.java | 2 +- .../io/grpc/internal/ManagedChannelImpl.java | 1 + .../PickFirstLoadBalancerProvider.java | 2 +- .../SecretRoundRobinLoadBalancerProvider.java | 2 +- .../io/grpc/internal/DnsNameResolverTest.java | 2 +- .../grpc/internal/ManagedChannelImplTest.java | 2 +- .../grpclb/GrpclbLoadBalancerProvider.java | 2 +- ...heckingRoundRobinLoadBalancerProvider.java | 2 +- .../java/io/grpc/xds/XdsLoadBalancer.java | 2 +- .../io/grpc/xds/XdsLoadBalancerProvider.java | 2 +- 13 files changed, 98 insertions(+), 99 deletions(-) diff --git a/core/src/main/java/io/grpc/LoadBalancerProvider.java b/core/src/main/java/io/grpc/LoadBalancerProvider.java index b6079d617e..0884aa1e5e 100644 --- a/core/src/main/java/io/grpc/LoadBalancerProvider.java +++ b/core/src/main/java/io/grpc/LoadBalancerProvider.java @@ -17,7 +17,7 @@ package io.grpc; import com.google.common.base.MoreObjects; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import java.util.Map; /** diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java index 343907e4e1..ae8eed04aa 100644 --- a/core/src/main/java/io/grpc/NameResolver.java +++ b/core/src/main/java/io/grpc/NameResolver.java @@ -21,7 +21,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; -import io.grpc.NameResolver.Helper.ConfigOrError; import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -355,93 +354,6 @@ public abstract class NameResolver { public ConfigOrError parseServiceConfig(Map rawServiceConfig) { throw new UnsupportedOperationException("should have been implemented"); } - - /** - * Represents either a successfully parsed service config, containing all necessary parts to be - * later applied by the channel, or a Status containing the error encountered while parsing. - * - * @since 1.20.0 - */ - public static final class ConfigOrError { - - private static final class UnknownConfig { - - UnknownConfig() {} - - @Override - public String toString() { - return "service config is unused"; - } - } - - /** - * A sentinel value indicating that service config is not supported. This can be used to - * indicate that parsing of the service config is neither right nor wrong, but doesn't have - * any meaning. - */ - public static final ConfigOrError UNKNOWN_CONFIG = - ConfigOrError.fromConfig(new UnknownConfig()); - - /** - * Returns a {@link ConfigOrError} for the successfully parsed config. - */ - public static ConfigOrError fromConfig(Object config) { - return new ConfigOrError(config); - } - - /** - * Returns a {@link ConfigOrError} for the failure to parse the config. - * - * @param status a non-OK status - */ - public static ConfigOrError fromError(Status status) { - return new ConfigOrError(status); - } - - private final Status status; - private final Object config; - - private ConfigOrError(Object config) { - this.config = checkNotNull(config, "config"); - this.status = null; - } - - private ConfigOrError(Status status) { - this.config = null; - this.status = checkNotNull(status, "status"); - checkArgument(!status.isOk(), "cannot use OK status: %s", status); - } - - /** - * Returns config if exists, otherwise null. - */ - @Nullable - public Object getConfig() { - return config; - } - - /** - * Returns error status if exists, otherwise null. - */ - @Nullable - public Status getError() { - return status; - } - - @Override - public String toString() { - if (config != null) { - return MoreObjects.toStringHelper(this) - .add("config", config) - .toString(); - } else { - assert status != null; - return MoreObjects.toStringHelper(this) - .add("error", status) - .toString(); - } - } - } } /** @@ -598,4 +510,90 @@ public abstract class NameResolver { } } } + + /** + * Gets the attributes associated with the servers resolved by name resolution. If there are + * no attributes, {@link Attributes#EMPTY} will be returned. + * + * @since 1.21.0 + */ + public static final class ConfigOrError { + private static final class UnknownConfig { + + UnknownConfig() {} + + @Override + public String toString() { + return "service config is unused"; + } + } + + /** + * A sentinel value indicating that service config is not supported. This can be used to + * indicate that parsing of the service config is neither right nor wrong, but doesn't have + * any meaning. + */ + public static final ConfigOrError UNKNOWN_CONFIG = + ConfigOrError.fromConfig(new UnknownConfig()); + + /** + * Returns a {@link ConfigOrError} for the successfully parsed config. + */ + public static ConfigOrError fromConfig(Object config) { + return new ConfigOrError(config); + } + + /** + * Returns a {@link ConfigOrError} for the failure to parse the config. + * + * @param status a non-OK status + */ + public static ConfigOrError fromError(Status status) { + return new ConfigOrError(status); + } + + private final Status status; + private final Object config; + + private ConfigOrError(Object config) { + this.config = checkNotNull(config, "config"); + this.status = null; + } + + private ConfigOrError(Status status) { + this.config = null; + this.status = checkNotNull(status, "status"); + checkArgument(!status.isOk(), "cannot use OK status: %s", status); + } + + /** + * Returns config if exists, otherwise null. + */ + @Nullable + public Object getConfig() { + return config; + } + + /** + * Returns error status if exists, otherwise null. + */ + @Nullable + public Status getError() { + return status; + } + + @Override + public String toString() { + if (config != null) { + return MoreObjects.toStringHelper(this) + .add("config", config) + .toString(); + } else { + assert status != null; + return MoreObjects.toStringHelper(this) + .add("error", status) + .toString(); + } + } + } } diff --git a/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java b/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java index 3c70d9adfa..c0872e823b 100644 --- a/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java +++ b/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java @@ -31,7 +31,7 @@ import io.grpc.LoadBalancer.PickSubchannelArgs; import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerRegistry; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; import io.grpc.internal.ServiceConfigUtil.LbConfig; import java.util.ArrayList; diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index 79d577791e..95c3aefb02 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -28,7 +28,7 @@ import com.google.common.base.VerifyException; import io.grpc.Attributes; import io.grpc.EquivalentAddressGroup; import io.grpc.NameResolver; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.ProxiedSocketAddress; import io.grpc.ProxyDetector; import io.grpc.Status; diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 1ae984476a..a1109f1a63 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -61,6 +61,7 @@ import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.NameResolver; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ResolutionResult; import io.grpc.ProxyDetector; import io.grpc.Status; diff --git a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java index 4a4fd61de9..0b11af94e2 100644 --- a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java +++ b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java @@ -18,7 +18,7 @@ package io.grpc.internal; import io.grpc.LoadBalancer; import io.grpc.LoadBalancerProvider; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import java.util.Map; /** diff --git a/core/src/main/java/io/grpc/util/SecretRoundRobinLoadBalancerProvider.java b/core/src/main/java/io/grpc/util/SecretRoundRobinLoadBalancerProvider.java index 08297347cd..7843832cd5 100644 --- a/core/src/main/java/io/grpc/util/SecretRoundRobinLoadBalancerProvider.java +++ b/core/src/main/java/io/grpc/util/SecretRoundRobinLoadBalancerProvider.java @@ -18,7 +18,7 @@ package io.grpc.util; import io.grpc.LoadBalancer; import io.grpc.LoadBalancerProvider; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import java.util.Map; /** diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 721ed8abc4..a8f8fb8fa5 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -39,7 +39,7 @@ import com.google.common.testing.FakeTicker; import io.grpc.EquivalentAddressGroup; import io.grpc.HttpConnectProxiedSocketAddress; import io.grpc.NameResolver; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ResolutionResult; import io.grpc.ProxyDetector; import io.grpc.Status; diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index a5654a186b..89ce41881c 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -91,7 +91,7 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; import io.grpc.NameResolver; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.NameResolver.ResolutionResult; import io.grpc.ProxiedSocketAddress; import io.grpc.ProxyDetector; diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancerProvider.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancerProvider.java index 61f8dae2b3..c70197ab6d 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancerProvider.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancerProvider.java @@ -20,7 +20,7 @@ import com.google.common.base.Stopwatch; import io.grpc.Internal; import io.grpc.LoadBalancer; import io.grpc.LoadBalancerProvider; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; import io.grpc.grpclb.GrpclbState.Mode; import io.grpc.internal.ExponentialBackoffPolicy; diff --git a/services/src/main/java/io/grpc/services/internal/HealthCheckingRoundRobinLoadBalancerProvider.java b/services/src/main/java/io/grpc/services/internal/HealthCheckingRoundRobinLoadBalancerProvider.java index 5929d81a54..c2c6357b68 100644 --- a/services/src/main/java/io/grpc/services/internal/HealthCheckingRoundRobinLoadBalancerProvider.java +++ b/services/src/main/java/io/grpc/services/internal/HealthCheckingRoundRobinLoadBalancerProvider.java @@ -22,7 +22,7 @@ import io.grpc.Internal; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancerProvider; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.services.HealthCheckingLoadBalancerUtil; import java.util.Map; diff --git a/xds/src/main/java/io/grpc/xds/XdsLoadBalancer.java b/xds/src/main/java/io/grpc/xds/XdsLoadBalancer.java index df9c45d1ad..e98738a598 100644 --- a/xds/src/main/java/io/grpc/xds/XdsLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/XdsLoadBalancer.java @@ -30,7 +30,7 @@ import io.grpc.ConnectivityStateInfo; import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.LoadBalancerRegistry; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; import io.grpc.SynchronizationContext.ScheduledHandle; import io.grpc.internal.ServiceConfigUtil.LbConfig; diff --git a/xds/src/main/java/io/grpc/xds/XdsLoadBalancerProvider.java b/xds/src/main/java/io/grpc/xds/XdsLoadBalancerProvider.java index 73134bc113..8177c3222c 100644 --- a/xds/src/main/java/io/grpc/xds/XdsLoadBalancerProvider.java +++ b/xds/src/main/java/io/grpc/xds/XdsLoadBalancerProvider.java @@ -23,7 +23,7 @@ import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerRegistry; -import io.grpc.NameResolver.Helper.ConfigOrError; +import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; import io.grpc.internal.ServiceConfigUtil; import io.grpc.internal.ServiceConfigUtil.LbConfig;