Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
707e1da
Introduce NameResolver.Args extensions.
jdcormie Nov 5, 2024
54e089b
Introduce NameResolver.Args extensions.
jdcormie Nov 5, 2024
f2a7b81
Merge remote-tracking branch 'origin/multi-user-nrp' into multi-user-nrp
jdcormie Nov 5, 2024
ce2930f
Introduce NameResolver.Args extensions.
jdcormie Nov 5, 2024
2f30771
fixes
jdcormie Nov 5, 2024
1a21575
understand
jdcormie Nov 5, 2024
07cb7c2
appropriate
jdcormie Nov 5, 2024
169e993
meaning
jdcormie Nov 5, 2024
a5c714a
javadoc
jdcormie Nov 5, 2024
1874264
polish
jdcormie Nov 6, 2024
2fbfc89
value/key
jdcormie Nov 6, 2024
a4b6f07
rename
jdcormie Nov 6, 2024
7f48a33
Address code review comments.
jdcormie Nov 20, 2024
76d1742
javadoc
jdcormie Nov 20, 2024
a216397
Copy forward AttributesTest.java
jdcormie Nov 20, 2024
29d01c7
Beef up unit tests
jdcormie Nov 20, 2024
9fbbc6e
Merge branch 'master' into multi-user-nrp
jdcormie Nov 26, 2024
1a79734
Merge branch 'grpc:master' into multi-user-nrp
jdcormie Dec 10, 2024
9ff19f7
Rename get/setExtension to be like CallOptions#get/setOption
jdcormie Dec 10, 2024
3124a91
Delete Extensions nested class completely.
jdcormie Dec 17, 2024
7653270
A few more extended -> custom renames
jdcormie Dec 17, 2024
5d4a24d
one more extension rename
jdcormie Dec 17, 2024
38946a4
Disallow null arg keys/values just like CallOptions does.
jdcormie Dec 17, 2024
2575fb1
returns
jdcormie Dec 17, 2024
e7fbb5c
extension
jdcormie Dec 18, 2024
2a97cb7
Merge branch `grpc:master` into multi-user-nrp
jdcormie Dec 18, 2024
0857a68
checkNotNull
jdcormie Dec 20, 2024
079a548
Merge branch 'multi-user-nrp' of https://github.com/jdcormie/grpc-jav…
jdcormie Dec 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 a "custom" 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 custom argument will be silently ignored.
*
* <p>See {@link NameResolver.Args#getArg(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
125 changes: 100 additions & 25 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
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.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 @@ -276,7 +278,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 "custom" args that aren't so widely
* useful or that would be inappropriate dependencies for this low level API. See {@link
* Args#getArg} for more.
*
* <p>Note this class overrides neither {@code equals()} nor {@code hashCode()}.
*
* @since 1.21.0
*/
Expand All @@ -291,26 +298,20 @@
@Nullable private final Executor executor;
@Nullable private final String overrideAuthority;
@Nullable private final MetricRecorder metricRecorder;
@Nullable private final IdentityHashMap<Key<?>, Object> customArgs;

private Args(
Integer defaultPort,
ProxyDetector proxyDetector,
SynchronizationContext syncContext,
ServiceConfigParser serviceConfigParser,
@Nullable ScheduledExecutorService scheduledExecutorService,
@Nullable ChannelLogger channelLogger,
@Nullable Executor executor,
@Nullable String overrideAuthority,
@Nullable MetricRecorder metricRecorder) {
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;
this.metricRecorder = metricRecorder;
private Args(Builder builder) {
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.scheduledExecutorService = builder.scheduledExecutorService;
this.channelLogger = builder.channelLogger;
this.executor = builder.executor;
this.overrideAuthority = builder.overrideAuthority;
this.metricRecorder = builder.metricRecorder;
this.customArgs = cloneCustomArgs(builder.customArgs);
}

/**
Expand All @@ -319,6 +320,7 @@
*
* @since 1.21.0
*/
// <p>TODO: Only meaningful for InetSocketAddress producers. Make this a custom arg?
public int getDefaultPort() {
return defaultPort;
}
Expand Down Expand Up @@ -371,6 +373,30 @@
return serviceConfigParser;
}

/**
* Returns the value of a custom arg named 'key', or {@code null} if it's not set.
*
* <p>While ordinary {@link Args} should be universally useful and meaningful, custom 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.
* Custom args are identified by an instance of {@link Args.Key} which should be a constant
* defined in a java package and class appropriate for 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.
* Custom args, in contrast, can hold arbitrary Java types, making them a useful work around in
* these cases.
*
* <p>Custom args can also be used simply to avoid adding inappropriate deps to the low level
* io.grpc package.
*/
@SuppressWarnings("unchecked") // Cast is safe because all put()s go through the setArg() API.
@Nullable
public <T> T getArg(Key<T> key) {
return customArgs != null ? (T) customArgs.get(key) : null;
}

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

Check warning on line 453 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#L453

Added line #L453 was not covered by tests
.add("scheduledExecutorService", scheduledExecutorService)
.add("channelLogger", channelLogger)
.add("executor", executor)
Expand All @@ -448,6 +475,7 @@
builder.setOffloadExecutor(executor);
builder.setOverrideAuthority(overrideAuthority);
builder.setMetricRecorder(metricRecorder);
builder.customArgs = cloneCustomArgs(customArgs);
return builder;
}

Expand Down Expand Up @@ -475,6 +503,7 @@
private Executor executor;
private String overrideAuthority;
private MetricRecorder metricRecorder;
private IdentityHashMap<Key<?>, Object> customArgs;

Builder() {
}
Expand Down Expand Up @@ -561,6 +590,17 @@
return this;
}

/** See {@link Args#getArg(Key)}. */
public <T> Builder setArg(Key<T> key, T value) {
checkNotNull(key, "key");
checkNotNull(value, "value");
if (customArgs == null) {
customArgs = new IdentityHashMap<>();
}
customArgs.put(key, value);
return this;
}

/**
* See {@link Args#getMetricRecorder()}. This is an optional field.
*/
Expand All @@ -575,11 +615,40 @@
* @since 1.21.0
*/
public Args build() {
return
new Args(
defaultPort, proxyDetector, syncContext, serviceConfigParser,
scheduledExecutorService, channelLogger, executor, overrideAuthority,
metricRecorder);
return new Args(this);
}
}

/**
* Identifies an externally-defined custom 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 640 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#L640

Added line #L640 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);
}
}
}
Expand Down Expand Up @@ -877,4 +946,10 @@
}
}
}

@Nullable
private static IdentityHashMap<Args.Key<?>, Object> cloneCustomArgs(
@Nullable IdentityHashMap<Args.Key<?>, Object> customArgs) {
return customArgs != null ? new IdentityHashMap<>(customArgs) : null;
}
}
16 changes: 13 additions & 3 deletions api/src/test/java/io/grpc/NameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ public class NameResolverTest {
private static final List<EquivalentAddressGroup> ADDRESSES =
Collections.singletonList(
new EquivalentAddressGroup(new FakeSocketAddress("fake-address-1"), Attributes.EMPTY));
private static final Attributes.Key<String> YOLO_KEY = Attributes.Key.create("yolo");
private static Attributes ATTRIBUTES = Attributes.newBuilder()
.set(YOLO_KEY, "To be, or not to be?").build();
private static final Attributes.Key<String> YOLO_ATTR_KEY = Attributes.Key.create("yolo");
private static Attributes ATTRIBUTES =
Attributes.newBuilder().set(YOLO_ATTR_KEY, "To be, or not to be?").build();
private static final NameResolver.Args.Key<Integer> FOO_ARG_KEY =
NameResolver.Args.Key.create("foo");
private static final NameResolver.Args.Key<Integer> BAR_ARG_KEY =
NameResolver.Args.Key.create("bar");
private static ConfigOrError CONFIG = ConfigOrError.fromConfig("foo");

@Rule
Expand All @@ -65,6 +69,7 @@ public class NameResolverTest {
private final Executor executor = Executors.newSingleThreadExecutor();
private final String overrideAuthority = "grpc.io";
private final MetricRecorder metricRecorder = new MetricRecorder() {};
private final int customArgValue = 42;
@Mock NameResolver.Listener mockListener;

@Test
Expand All @@ -79,6 +84,8 @@ public void args() {
assertThat(args.getOffloadExecutor()).isSameInstanceAs(executor);
assertThat(args.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
assertThat(args.getArg(FOO_ARG_KEY)).isEqualTo(customArgValue);
assertThat(args.getArg(BAR_ARG_KEY)).isNull();

NameResolver.Args args2 = args.toBuilder().build();
assertThat(args2.getDefaultPort()).isEqualTo(defaultPort);
Expand All @@ -90,6 +97,8 @@ public void args() {
assertThat(args2.getOffloadExecutor()).isSameInstanceAs(executor);
assertThat(args2.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
assertThat(args.getArg(FOO_ARG_KEY)).isEqualTo(customArgValue);
assertThat(args.getArg(BAR_ARG_KEY)).isNull();

assertThat(args2).isNotSameInstanceAs(args);
assertThat(args2).isNotEqualTo(args);
Expand All @@ -106,6 +115,7 @@ private NameResolver.Args createArgs() {
.setOffloadExecutor(executor)
.setOverrideAuthority(overrideAuthority)
.setMetricRecorder(metricRecorder)
.setArg(FOO_ARG_KEY, customArgValue)
.build();
}

Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,7 @@ ClientStream newSubstream(
this.authorityOverride = builder.authorityOverride;
this.metricRecorder = new MetricRecorderImpl(builder.metricSinks,
MetricInstrumentRegistry.getDefaultRegistry());
this.nameResolverArgs =
NameResolver.Args.newBuilder()
NameResolver.Args.Builder nameResolverArgsBuilder = NameResolver.Args.newBuilder()
.setDefaultPort(builder.getDefaultPort())
.setProxyDetector(proxyDetector)
.setSynchronizationContext(syncContext)
Expand All @@ -601,8 +600,9 @@ ClientStream newSubstream(
.setChannelLogger(channelLogger)
.setOffloadExecutor(this.offloadExecutorHolder)
.setOverrideAuthority(this.authorityOverride)
.setMetricRecorder(this.metricRecorder)
.build();
.setMetricRecorder(this.metricRecorder);
builder.copyAllNameResolverCustomArgsTo(nameResolverArgsBuilder);
this.nameResolverArgs = nameResolverArgsBuilder.build();
this.nameResolver = getNameResolver(
targetUri, authorityOverride, nameResolverProvider, nameResolverArgs);
this.balancerRpcExecutorPool = checkNotNull(balancerRpcExecutorPool, "balancerRpcExecutorPool");
Expand Down
21 changes: 21 additions & 0 deletions core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -159,6 +160,8 @@ public static ManagedChannelBuilder<?> forTarget(String target) {
final ChannelCredentials channelCredentials;
@Nullable
final CallCredentials callCredentials;
@Nullable
IdentityHashMap<NameResolver.Args.Key<?>, Object> nameResolverCustomArgs;

@Nullable
private final SocketAddress directServerAddress;
Expand Down Expand Up @@ -613,6 +616,24 @@ private static List<?> checkListEntryTypes(List<?> list) {
return Collections.unmodifiableList(parsedList);
}

@Override
public <X> ManagedChannelImplBuilder setNameResolverArg(NameResolver.Args.Key<X> key, X value) {
if (nameResolverCustomArgs == null) {
nameResolverCustomArgs = new IdentityHashMap<>();
}
nameResolverCustomArgs.put(key, value);
return this;
}

@SuppressWarnings("unchecked") // This cast is safe because of setNameResolverArg()'s signature.
void copyAllNameResolverCustomArgsTo(NameResolver.Args.Builder dest) {
if (nameResolverCustomArgs != null) {
for (Map.Entry<NameResolver.Args.Key<?>, Object> entry : nameResolverCustomArgs.entrySet()) {
dest.setArg((NameResolver.Args.Key<Object>) entry.getKey(), entry.getValue());
}
}
}

@Override
public ManagedChannelImplBuilder disableServiceConfigLookUp() {
this.lookUpServiceConfig = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,16 @@ public void disableNameResolverServiceConfig() {
assertThat(builder.lookUpServiceConfig).isFalse();
}

@Test
public void setNameResolverExtArgs() {
assertThat(builder.nameResolverCustomArgs)
.isNull();

NameResolver.Args.Key<Integer> testKey = NameResolver.Args.Key.create("test-key");
builder.setNameResolverArg(testKey, 42);
assertThat(builder.nameResolverCustomArgs.get(testKey)).isEqualTo(42);
}

@Test
public void metricSinks() {
MetricSink mocksink = mock(MetricSink.class);
Expand Down
Loading
Loading