Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce NameResolver.Args.Extensions #11669

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
6 changes: 6 additions & 0 deletions api/src/main/java/io/grpc/ForwardingChannelBuilder2.java
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ protected T addMetricSink(MetricSink metricSink) {
return thisT();
}

@Override
public <X> T setNameResolverArg(NameResolver.Args.Key<X> key, X value) {
delegate().setNameResolverArg(key, value);
return thisT();
}

/**
* Returns the {@link ManagedChannel} built by the delegate by default. Overriding method can
* return different value.
Expand Down
17 changes: 17 additions & 0 deletions api/src/main/java/io/grpc/ManagedChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,23 @@
throw new UnsupportedOperationException();
}

/**
* Provides an "extended" argument for the {@link NameResolver}, if applicable, replacing any
* 'value' previously provided for 'key'.
*
* <p>NB: If the selected {@link NameResolver} does not understand 'key', or target URI resolution
* isn't needed at all, your extended argument will be silently ignored.
*
* <p>See {@link NameResolver.Args#getExtension(NameResolver.Args.Key)} for more.
*
* @param key identifies the argument in a type-safe manner
* @param value the argument itself
* @return this
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770")
public <X> T setNameResolverArg(NameResolver.Args.Key<X> key, X value) {
throw new UnsupportedOperationException();

Check warning on line 651 in api/src/main/java/io/grpc/ManagedChannelBuilder.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L651 was not covered by tests
}

/**
* Builds a channel using the given parameters.
Expand Down
217 changes: 195 additions & 22 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@
import java.lang.annotation.RetentionPolicy;
import java.net.URI;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Executor;
import java.util.concurrent.ScheduledExecutorService;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.ThreadSafe;

/**
Expand Down Expand Up @@ -274,7 +277,12 @@
/**
* Information that a {@link Factory} uses to create a {@link NameResolver}.
*
* <p>Note this class doesn't override neither {@code equals()} nor {@code hashCode()}.
* <p>Args applicable to all {@link NameResolver}s are defined here using ordinary setters and
* getters. This container can also hold externally-defined "extended" args that aren't so widely
* useful or that would be inappropriate dependencies for this low level API. See {@link
* Args#getExtension} for more.
*
* <p>Note this class overrides neither {@code equals()} nor {@code hashCode()}.
*
* @since 1.21.0
*/
Expand All @@ -284,28 +292,24 @@
private final ProxyDetector proxyDetector;
private final SynchronizationContext syncContext;
private final ServiceConfigParser serviceConfigParser;
private final Extensions extensions;
@Nullable private final ScheduledExecutorService scheduledExecutorService;
@Nullable private final ChannelLogger channelLogger;
@Nullable private final Executor executor;
@Nullable private final String overrideAuthority;

private Args(
Integer defaultPort,
ProxyDetector proxyDetector,
SynchronizationContext syncContext,
ServiceConfigParser serviceConfigParser,
@Nullable ScheduledExecutorService scheduledExecutorService,
@Nullable ChannelLogger channelLogger,
@Nullable Executor executor,
@Nullable String overrideAuthority) {
this.defaultPort = checkNotNull(defaultPort, "defaultPort not set");
this.proxyDetector = checkNotNull(proxyDetector, "proxyDetector not set");
this.syncContext = checkNotNull(syncContext, "syncContext not set");
this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser not set");
this.scheduledExecutorService = scheduledExecutorService;
this.channelLogger = channelLogger;
this.executor = executor;
this.overrideAuthority = overrideAuthority;
private Args(Builder builder) {
jdcormie marked this conversation as resolved.
Show resolved Hide resolved
this.defaultPort = checkNotNull(builder.defaultPort, "defaultPort not set");
this.proxyDetector = checkNotNull(builder.proxyDetector, "proxyDetector not set");
this.syncContext = checkNotNull(builder.syncContext, "syncContext not set");
this.serviceConfigParser =
checkNotNull(builder.serviceConfigParser, "serviceConfigParser not set");
this.extensions =
builder.extensionsBuilder != null ? builder.extensionsBuilder.build() : Extensions.EMPTY;
this.scheduledExecutorService = builder.scheduledExecutorService;
this.channelLogger = builder.channelLogger;
this.executor = builder.executor;
this.overrideAuthority = builder.overrideAuthority;
}

/**
Expand All @@ -314,6 +318,7 @@
*
* @since 1.21.0
*/
// <p>TODO: Only meaningful for InetSocketAddress producers. Move to {@link Extensions}?
public int getDefaultPort() {
return defaultPort;
}
Expand Down Expand Up @@ -366,6 +371,29 @@
return serviceConfigParser;
}

/**
* Gets the value of an "extension" arg by key, or {@code null} if it's not set.
*
* <p>While ordinary {@link Args} should be universally useful and meaningful, extension
* arguments can apply just to resolvers of a certain URI scheme, just to resolvers producing a
* particular type of {@link java.net.SocketAddress}, or even an individual {@link NameResolver}
* subclass. Extension args are identified by an instance of {@link Args.Key} which should be
* defined in a java package and class appropriate to the argument's scope.
*
* <p>{@link Args} are normally reserved for information in *support* of name resolution, not
* the name to be resolved itself. However, there are rare cases where all or part of the target
* name can't be represented by any standard URI scheme or can't be encoded as a String at all.
* Extension args, in contrast, can be an arbitrary Java type, making them a useful work around
* in these cases.
*
* <p>Extension args can also be used simply to avoid adding inappropriate deps to the low level
* io.grpc package.
*/
@Nullable
public <T> T getExtension(Key<T> key) {
return extensions.get(key);
}

/**
* Returns the {@link ChannelLogger} for the Channel served by this NameResolver.
*
Expand Down Expand Up @@ -411,6 +439,7 @@
.add("proxyDetector", proxyDetector)
.add("syncContext", syncContext)
.add("serviceConfigParser", serviceConfigParser)
.add("extensions", extensions)

Check warning on line 442 in api/src/main/java/io/grpc/NameResolver.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/NameResolver.java#L442

Added line #L442 was not covered by tests
.add("scheduledExecutorService", scheduledExecutorService)
.add("channelLogger", channelLogger)
.add("executor", executor)
Expand All @@ -429,6 +458,7 @@
builder.setProxyDetector(proxyDetector);
builder.setSynchronizationContext(syncContext);
builder.setServiceConfigParser(serviceConfigParser);
builder.extensionsBuilder = extensions.toBuilder();
builder.setScheduledExecutorService(scheduledExecutorService);
builder.setChannelLogger(channelLogger);
builder.setOffloadExecutor(executor);
Expand Down Expand Up @@ -459,6 +489,7 @@
private ChannelLogger channelLogger;
private Executor executor;
private String overrideAuthority;
private Extensions.Builder extensionsBuilder = Extensions.newBuilder();

Builder() {
}
Expand Down Expand Up @@ -545,16 +576,158 @@
return this;
}

/** See {@link Args#getExtension(Key)}. */
public <T> Builder setExtension(Key<T> key, T value) {
extensionsBuilder.set(key, value);
return this;
}

/** Copies each extension argument from 'extensions' into this Builder. */
@Internal
public Builder setAllExtensions(Extensions extensions) {
extensionsBuilder.setAll(extensions);
return this;
}

/**
* Builds an {@link Args}.
*
* @since 1.21.0
*/
public Args build() {
return
new Args(
defaultPort, proxyDetector, syncContext, serviceConfigParser,
scheduledExecutorService, channelLogger, executor, overrideAuthority);
return new Args(this);
}
}

/**
* Identifies an externally-defined extension argument that can be stored in {@link Args}.
*
* <p>Uses reference equality so keys should be defined as global constants.
*
* @param <T> type of values that can be stored under this key
*/
@Immutable
@SuppressWarnings("UnusedTypeParameter")
public static final class Key<T> {
private final String debugString;

private Key(String debugString) {
this.debugString = debugString;
}

@Override
public String toString() {
return debugString;
}

Check warning on line 622 in api/src/main/java/io/grpc/NameResolver.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/NameResolver.java#L622

Added line #L622 was not covered by tests
/**
* Creates a new instance of {@link Key}.
*
* @param debugString a string used to describe the key, used for debugging.
* @param <T> Key type
* @return a new instance of Key
*/
public static <T> Key<T> create(String debugString) {
return new Key<>(debugString);
}
}

/**
* An immutable type-safe container of externally-defined {@link NameResolver} arguments.
*
* <p>NB: This class overrides neither {@code equals()} nor {@code hashCode()}.
*/
@Immutable
@Internal
public static final class Extensions {
private static final IdentityHashMap<Key<?>, Object> EMPTY_MAP = new IdentityHashMap<>();

/** The canonical empty instance. */
public static final Extensions EMPTY = new Extensions(EMPTY_MAP);

private final IdentityHashMap<Key<?>, Object> data;

private Extensions(IdentityHashMap<Key<?>, Object> data) {
assert data != null;
this.data = data;
}

/** Gets the value for the key, or {@code null} if it's not present. */
@SuppressWarnings("unchecked")
@Nullable
public <T> T get(Key<T> key) {
return (T) data.get(key);
}

Set<Key<?>> keysForTest() {
return Collections.unmodifiableSet(data.keySet());
}

/** Creates a new builder. */
public static Builder newBuilder() {
return new Builder(EMPTY);
}

/**
* Creates a new builder that is pre-populated with the content of this container.
*
* @return a new builder.
*/
public Builder toBuilder() {
return new Builder(this);
}

@Override
public String toString() {
return data.toString();
}

Check warning on line 684 in api/src/main/java/io/grpc/NameResolver.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/NameResolver.java#L684

Added line #L684 was not covered by tests
/** Fluently builds instances of {@link Extensions}. */
public static final class Builder {
private Extensions base;
private IdentityHashMap<Key<?>, Object> newdata;

private Builder(Extensions base) {
assert base != null;
this.base = base;
}

private IdentityHashMap<Key<?>, Object> data(int size) {
if (newdata == null) {
newdata = new IdentityHashMap<>(size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be nicer to copy base at this point:

newdata = new IdentityHashMap<>(base.data);

Yes, you're doing the sizing of the map here, but that size is pretty arbitrary. If we care:

newdata = new IdentityHashMap<>(base.data.size() + 1);
newdata.putAll(base.data);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm seems like that could work but for me it really muddies the meaning/invariant of 'base' and 'newdata'. Anyway, I just copy/pasted all this from Attributes. Seems to me like the idea is to do all the copying in one place, in build(). Does that change anything for you? Should we consider your proposed change for Attributes too?

}
return newdata;
}

/**
* Associates 'value' with 'key', replacing any previously associated value.
*
* @return this
*/
public <T> Builder set(Key<T> key, T value) {
data(1).put(key, value);
return this;
}

/** Copies all entries from 'other' into this Builder. */
public Builder setAll(Extensions other) {
data(other.data.size()).putAll(other.data);
return this;
}

/** Builds the extensions. */
public Extensions build() {
if (newdata != null) {
for (Map.Entry<Key<?>, Object> entry : base.data.entrySet()) {
if (!newdata.containsKey(entry.getKey())) {
newdata.put(entry.getKey(), entry.getValue());
}
}

Check warning on line 725 in api/src/main/java/io/grpc/NameResolver.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/NameResolver.java#L725

Added line #L725 was not covered by tests
base = new Extensions(newdata);
newdata = null;
}
return base;
}
}
}
}
Expand Down
Loading