Skip to content

Commit

Permalink
Implement equals and hashCode in StatusOr and other things discussed.
Browse files Browse the repository at this point in the history
  • Loading branch information
kannanjgithub committed Aug 12, 2024
1 parent 27af56b commit d02e4aa
Show file tree
Hide file tree
Showing 15 changed files with 72 additions and 49 deletions.
14 changes: 7 additions & 7 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void onError(Status error) {

@Override
public void onResult(ResolutionResult resolutionResult) {
listener.onAddresses(resolutionResult.getAddressesOrError().value(),
listener.onAddresses(resolutionResult.getAddressesOrError().getValue(),
resolutionResult.getAttributes());
}
});
Expand Down Expand Up @@ -219,14 +219,14 @@ public abstract static class Listener2 implements Listener {
@Deprecated
@InlineMe(
replacement = "this.onResult2(ResolutionResult.newBuilder().setAddressesOrError("
+ "StatusOr.fromValue(servers)).setAttributes(attributes).build())",
+ "StatusOr.of(servers)).setAttributes(attributes).build())",
imports = {"io.grpc.NameResolver.ResolutionResult", "io.grpc.StatusOr"})
public final void onAddresses(
List<EquivalentAddressGroup> servers, @ResolutionResultAttr Attributes attributes) {
// TODO(jihuncho) need to promote Listener2 if we want to use ConfigOrError
onResult2(
ResolutionResult.newBuilder().setAddressesOrError(
StatusOr.fromValue(servers)).setAttributes(attributes).build());
StatusOr.of(servers)).setAttributes(attributes).build());
}

/**
Expand Down Expand Up @@ -605,7 +605,7 @@ public static final class ResolutionResult {
List<EquivalentAddressGroup> addresses,
@ResolutionResultAttr Attributes attributes,
ConfigOrError serviceConfig) {
this.addressesOrError = StatusOr.fromValue(addresses);
this.addressesOrError = StatusOr.of(addresses);
this.attributes = checkNotNull(attributes, "attributes");
this.serviceConfig = serviceConfig;
}
Expand Down Expand Up @@ -639,7 +639,7 @@ public Builder toBuilder() {
*/
@Deprecated
public List<EquivalentAddressGroup> getAddresses() {
return addressesOrError.value();
return addressesOrError.getValue();
}

/**
Expand Down Expand Up @@ -711,7 +711,7 @@ public int hashCode() {
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770")
public static final class Builder {
private StatusOr<List<EquivalentAddressGroup>> addresses =
StatusOr.fromValue(Collections.emptyList());
StatusOr.of(Collections.emptyList());
private Attributes attributes = Attributes.EMPTY;
@Nullable
private ConfigOrError serviceConfig;
Expand All @@ -727,7 +727,7 @@ public static final class Builder {
*/
@Deprecated
public Builder setAddresses(List<EquivalentAddressGroup> addresses) {
this.addresses = StatusOr.fromValue(addresses);
this.addresses = StatusOr.of(addresses);
return this;
}

Expand Down
29 changes: 26 additions & 3 deletions api/src/main/java/io/grpc/StatusOr.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.Objects;

/** Either a Status or a value. */
public class StatusOr<T> {
private StatusOr(Status status, T value) {
Expand All @@ -27,7 +29,7 @@ private StatusOr(Status status, T value) {
}

/** Construct from a value. */
public static <T> StatusOr<T> fromValue(T value) {
public static <T> StatusOr<T> of(T value) {
StatusOr<T> result = new StatusOr<T>(null, checkNotNull(value, "value"));
return result;
}
Expand All @@ -45,15 +47,36 @@ public boolean hasValue() {
}

/** Returns the value if set, or null in case of a non-OK status. */
public T value() {
public T getValue() {
return value;
}

/** Returns the status. If there is a value, returns OK. */
public Status status() {
public Status getStatus() {
return value != null ? Status.OK : status;
}

@Override
@SuppressWarnings("ReferenceEquality")
public boolean equals(Object other) {
if (!(other instanceof StatusOr)) {
return false;
}
StatusOr<T> otherStatus = (StatusOr) other;
if (hasValue() != otherStatus.hasValue()) {
return false;
}
if (hasValue()) {
return Objects.equal(value, otherStatus.value);
}
return status == otherStatus.status;
}

@Override
public int hashCode() {
return Objects.hashCode(status, value);
}

private final Status status;
private final T value;
}
4 changes: 2 additions & 2 deletions core/src/main/java/io/grpc/internal/DnsNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ public void run() {
logger.finer("Using proxy address " + proxiedAddr);
}
resolutionResultBuilder.setAddressesOrError(
StatusOr.fromValue(Collections.singletonList(proxiedAddr)));
StatusOr.of(Collections.singletonList(proxiedAddr)));
} else {
result = doResolve(false);
if (result.error != null) {
Expand All @@ -327,7 +327,7 @@ public void run() {
return;
}
if (result.addresses != null) {
resolutionResultBuilder.setAddressesOrError(StatusOr.fromValue(result.addresses));
resolutionResultBuilder.setAddressesOrError(StatusOr.of(result.addresses));
}
if (result.config != null) {
resolutionResultBuilder.setServiceConfig(result.config);
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 @@ -1705,12 +1705,12 @@ public Status onResult2(final ResolutionResult resolutionResult) {

if (lastResolutionState != ResolutionState.SUCCESS) {
channelLogger.log(ChannelLogLevel.INFO, "Address resolved: {0}",
serversOrError.value());
serversOrError.getValue());
lastResolutionState = ResolutionState.SUCCESS;
}
} else if (serversOrError != null && !serversOrError.hasValue()) {
handleErrorInSyncContext(serversOrError.status());
return serversOrError.status();
handleErrorInSyncContext(serversOrError.getStatus());
return serversOrError.getStatus();
}
ConfigOrError configOrError = resolutionResult.getServiceConfig();
InternalConfigSelector resolvedConfigSelector =
Expand Down Expand Up @@ -1815,7 +1815,7 @@ public Status onResult2(final ResolutionResult resolutionResult) {

ResolvedAddresses.Builder resolvedAddresses = ResolvedAddresses.newBuilder();
if (serversOrError != null && serversOrError.hasValue()) {
resolvedAddresses.setAddresses(serversOrError.value());
resolvedAddresses.setAddresses(serversOrError.getValue());
} else {
resolvedAddresses.setAddresses(new ArrayList<>());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ public void start(Listener2 listener) {
listener.onResult2(
ResolutionResult.newBuilder()
.setAddressesOrError(
StatusOr.fromValue(
StatusOr.of(
Collections.singletonList(new EquivalentAddressGroup(address))))
.setAttributes(Attributes.EMPTY)
.build());
Expand Down
22 changes: 11 additions & 11 deletions core/src/test/java/io/grpc/internal/DnsNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ public List<InetAddress> resolveAddress(String host) throws Exception {
ArgumentCaptor<ResolutionResult> ac = ArgumentCaptor.forClass(ResolutionResult.class);
verify(mockListener).onResult2(ac.capture());
verifyNoMoreInteractions(mockListener);
assertThat(ac.getValue().getAddressesOrError().value()).isEmpty();
assertThat(ac.getValue().getAddressesOrError().getValue()).isEmpty();
assertThat(ac.getValue().getServiceConfig()).isNull();
verify(mockResourceResolver, never()).resolveSrv(anyString());

Expand Down Expand Up @@ -602,7 +602,7 @@ public List<InetAddress> resolveAddress(String host) throws Exception {
ArgumentCaptor<ResolutionResult> ac = ArgumentCaptor.forClass(ResolutionResult.class);
verify(mockListener).onResult2(ac.capture());
verifyNoMoreInteractions(mockListener);
assertThat(ac.getValue().getAddressesOrError().value()).isEmpty();
assertThat(ac.getValue().getAddressesOrError().getValue()).isEmpty();
assertThat(ac.getValue().getServiceConfig()).isNull();
verify(mockResourceResolver, never()).resolveSrv(anyString());

Expand Down Expand Up @@ -630,7 +630,7 @@ public void resolve_nullResourceResolver() throws Exception {
ResolutionResult result = resultCaptor.getValue();
InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
verify(mockAddressResolver).resolveAddress(name);
assertThat(result.getServiceConfig()).isNull();
Expand All @@ -655,7 +655,7 @@ public void resolve_nullResourceResolver_addressFailure() throws Exception {
resolver.start(mockListener);
assertEquals(1, fakeExecutor.runDueTasks());
verify(mockListener).onResult2(resultCaptor.capture());
Status errorStatus = resultCaptor.getValue().getAddressesOrError().status();
Status errorStatus = resultCaptor.getValue().getAddressesOrError().getStatus();
assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");

Expand Down Expand Up @@ -703,7 +703,7 @@ public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
ResolutionResult result = resultCaptor.getValue();
InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
assertThat(result.getServiceConfig().getConfig()).isNotNull();
verify(mockAddressResolver).resolveAddress(name);
Expand All @@ -730,7 +730,7 @@ public void resolve_addressFailure_neverLookUpServiceConfig() throws Exception {
resolver.start(mockListener);
assertEquals(1, fakeExecutor.runDueTasks());
verify(mockListener).onResult2(resultCaptor.capture());
Status errorStatus = resultCaptor.getValue().getAddressesOrError().status();
Status errorStatus = resultCaptor.getValue().getAddressesOrError().getStatus();
assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
verify(mockResourceResolver, never()).resolveTxt(anyString());
Expand Down Expand Up @@ -762,7 +762,7 @@ public void resolve_serviceConfigLookupFails_nullServiceConfig() throws Exceptio
ResolutionResult result = resultCaptor.getValue();
InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
verify(mockAddressResolver).resolveAddress(name);
assertThat(result.getServiceConfig()).isNull();
Expand Down Expand Up @@ -794,7 +794,7 @@ public void resolve_serviceConfigMalformed_serviceConfigError() throws Exception
ResolutionResult result = resultCaptor.getValue();
InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
verify(mockAddressResolver).resolveAddress(name);
assertThat(result.getServiceConfig()).isNotNull();
Expand Down Expand Up @@ -859,7 +859,7 @@ public HttpConnectProxiedSocketAddress proxyFor(SocketAddress targetAddress) {
assertEquals(1, fakeExecutor.runDueTasks());

verify(mockListener).onResult2(resultCaptor.capture());
List<EquivalentAddressGroup> result = resultCaptor.getValue().getAddressesOrError().value();
List<EquivalentAddressGroup> result = resultCaptor.getValue().getAddressesOrError().getValue();
assertThat(result).hasSize(1);
EquivalentAddressGroup eag = result.get(0);
assertThat(eag.getAddresses()).hasSize(1);
Expand Down Expand Up @@ -1299,9 +1299,9 @@ private List<InetAddress> createAddressList(int n) throws UnknownHostException {

private static void assertAnswerMatches(
List<InetAddress> addrs, int port, ResolutionResult resolutionResult) {
assertThat(resolutionResult.getAddressesOrError().value()).hasSize(addrs.size());
assertThat(resolutionResult.getAddressesOrError().getValue()).hasSize(addrs.size());
for (int i = 0; i < addrs.size(); i++) {
EquivalentAddressGroup addrGroup = resolutionResult.getAddressesOrError().value().get(i);
EquivalentAddressGroup addrGroup = resolutionResult.getAddressesOrError().getValue().get(i);
InetSocketAddress socketAddr =
(InetSocketAddress) Iterables.getOnlyElement(addrGroup.getAddresses());
assertEquals("Addr " + i, port, socketAddr.getPort());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ private void deliverResolutionResult() {
// the NameResolver.
ResolutionResult resolutionResult =
ResolutionResult.newBuilder()
.setAddressesOrError(StatusOr.fromValue(servers))
.setAddressesOrError(StatusOr.of(servers))
.setAttributes(Attributes.EMPTY)
.build();
nameResolverListenerCaptor.getValue().onResult(resolutionResult);
Expand Down
14 changes: 7 additions & 7 deletions grpclb/src/test/java/io/grpc/grpclb/GrpclbNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public List<SrvRecord> resolveSrv(String host) throws Exception {

verify(mockListener).onResult2(resultCaptor.capture());
ResolutionResult result = resultCaptor.getValue();
assertThat(result.getAddressesOrError().value()).isEmpty();
assertThat(result.getAddressesOrError().getValue()).isEmpty();
assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
assertThat(result.getServiceConfig()).isNull();
}
Expand Down Expand Up @@ -195,7 +195,7 @@ public ConfigOrError answer(InvocationOnMock invocation) {
ResolutionResult result = resultCaptor.getValue();
InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
EquivalentAddressGroup resolvedBalancerAddr =
Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS));
Expand Down Expand Up @@ -226,7 +226,7 @@ public void resolve_nullResourceResolver() throws Exception {
assertThat(fakeClock.runDueTasks()).isEqualTo(1);
verify(mockListener).onResult2(resultCaptor.capture());
ResolutionResult result = resultCaptor.getValue();
assertThat(result.getAddressesOrError().value())
assertThat(result.getAddressesOrError().getValue())
.containsExactly(
new EquivalentAddressGroup(new InetSocketAddress(backendAddr, DEFAULT_PORT)));
assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
Expand All @@ -245,7 +245,7 @@ public void resolve_nullResourceResolver_addressFailure() throws Exception {
resolver.start(mockListener);
assertThat(fakeClock.runDueTasks()).isEqualTo(1);
verify(mockListener).onResult2(resultCaptor.capture());
Status errorStatus = resultCaptor.getValue().getAddressesOrError().status();
Status errorStatus = resultCaptor.getValue().getAddressesOrError().getStatus();
assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
}
Expand Down Expand Up @@ -273,7 +273,7 @@ public void resolve_addressFailure_stillLookUpBalancersAndServiceConfig() throws
assertThat(fakeClock.runDueTasks()).isEqualTo(1);
verify(mockListener).onResult2(resultCaptor.capture());
ResolutionResult result = resultCaptor.getValue();
assertThat(result.getAddressesOrError().value()).isEmpty();
assertThat(result.getAddressesOrError().getValue()).isEmpty();
EquivalentAddressGroup resolvedBalancerAddr =
Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS));
assertThat(resolvedBalancerAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY))
Expand Down Expand Up @@ -310,7 +310,7 @@ public void resolveAll_balancerLookupFails_stillLookUpServiceConfig() throws Exc

InetSocketAddress resolvedBackendAddr =
(InetSocketAddress) Iterables.getOnlyElement(
Iterables.getOnlyElement(result.getAddressesOrError().value()).getAddresses());
Iterables.getOnlyElement(result.getAddressesOrError().getValue()).getAddresses());
assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
assertThat(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS)).isNull();
verify(mockAddressResolver).resolveAddress(hostName);
Expand All @@ -335,7 +335,7 @@ public void resolve_addressAndBalancersLookupFail_neverLookupServiceConfig() thr
resolver.start(mockListener);
assertThat(fakeClock.runDueTasks()).isEqualTo(1);
verify(mockListener).onResult2(resultCaptor.capture());
Status errorStatus = resultCaptor.getValue().getAddressesOrError().status();
Status errorStatus = resultCaptor.getValue().getAddressesOrError().getStatus();
assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
verify(mockAddressResolver).resolveAddress(hostName);
verify(mockResourceResolver, never()).resolveTxt("_grpc_config." + hostName);
Expand Down
2 changes: 1 addition & 1 deletion netty/src/main/java/io/grpc/netty/UdsNameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private void resolve() {
ResolutionResult.Builder resolutionResultBuilder = ResolutionResult.newBuilder();
List<EquivalentAddressGroup> servers = new ArrayList<>(1);
servers.add(new EquivalentAddressGroup(new DomainSocketAddress(authority)));
resolutionResultBuilder.setAddressesOrError(StatusOr.fromValue(servers));
resolutionResultBuilder.setAddressesOrError(StatusOr.of(servers));
syncContext.execute(() ->
listener.onResult2(resolutionResultBuilder.build()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void testUnixRelativePath() {
udsNameResolver.start(mockListener);
verify(mockListener).onResult2(resultCaptor.capture());
NameResolver.ResolutionResult result = resultCaptor.getValue();
List<EquivalentAddressGroup> list = result.getAddressesOrError().value();
List<EquivalentAddressGroup> list = result.getAddressesOrError().getValue();
assertThat(list).isNotNull();
assertThat(list).hasSize(1);
EquivalentAddressGroup eag = list.get(0);
Expand All @@ -99,7 +99,7 @@ public void testUnixAbsolutePath() {
udsNameResolver.start(mockListener);
verify(mockListener).onResult2(resultCaptor.capture());
NameResolver.ResolutionResult result = resultCaptor.getValue();
List<EquivalentAddressGroup> list = result.getAddressesOrError().value();
List<EquivalentAddressGroup> list = result.getAddressesOrError().getValue();
assertThat(list).isNotNull();
assertThat(list).hasSize(1);
EquivalentAddressGroup eag = list.get(0);
Expand All @@ -119,7 +119,7 @@ public void testUnixAbsoluteAlternatePath() {
udsNameResolver.start(mockListener);
verify(mockListener).onResult2(resultCaptor.capture());
NameResolver.ResolutionResult result = resultCaptor.getValue();
List<EquivalentAddressGroup> list = result.getAddressesOrError().value();
List<EquivalentAddressGroup> list = result.getAddressesOrError().getValue();
assertThat(list).isNotNull();
assertThat(list).hasSize(1);
EquivalentAddressGroup eag = list.get(0);
Expand Down
2 changes: 1 addition & 1 deletion netty/src/test/java/io/grpc/netty/UdsNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testValidTargetPath() {
udsNameResolver.start(mockListener);
verify(mockListener).onResult2(resultCaptor.capture());
NameResolver.ResolutionResult result = resultCaptor.getValue();
List<EquivalentAddressGroup> list = result.getAddressesOrError().value();
List<EquivalentAddressGroup> list = result.getAddressesOrError().getValue();
assertThat(list).isNotNull();
assertThat(list).hasSize(1);
EquivalentAddressGroup eag = list.get(0);
Expand Down
Loading

0 comments on commit d02e4aa

Please sign in to comment.