Skip to content

On result2 resolution result have addresses or error #11330

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a4e92d6
Introducing NameResolver Listener2::onResult2 that returns Status of …
kannanjgithub Jun 24, 2024
8ac1581
Add comment for the deprecation.
kannanjgithub Jun 24, 2024
82dc9c5
Introducing NameResolver Listener2::onResult2 that returns Status of …
kannanjgithub Jun 24, 2024
a42e66e
Merge remote-tracking branch 'origin/onResult2' into onResult2
kannanjgithub Jun 24, 2024
5e8b3ea
Whitespace fix.
kannanjgithub Jun 24, 2024
7deb491
Combining passing name resolution result or error via ResolutionResult.
kannanjgithub Jun 26, 2024
fb39b62
Combining passing name resolution result or error via ResolutionResult.
kannanjgithub Jun 26, 2024
3a1f3eb
Merge remote-tracking branch 'origin/onResult2_resolutionResultHaveAd…
kannanjgithub Jun 26, 2024
7169585
nit.
kannanjgithub Jun 26, 2024
3aedd65
Test fixes.
kannanjgithub Jun 26, 2024
96a7612
Merge branch 'grpc:master' into onResult2_resolutionResultHaveAddress…
kannanjgithub Jul 20, 2024
8909812
Merge branch 'master' into onResult2_resolutionResultHaveAddressesOrE…
kannanjgithub Jul 26, 2024
96890d2
nit
kannanjgithub Jul 26, 2024
57ee8e2
Fix merge errors.
kannanjgithub Jul 27, 2024
b1e7802
Fix warnings.
kannanjgithub Jul 27, 2024
f2a7b32
Fix style issues.
kannanjgithub Jul 27, 2024
4c0742d
Fix style issues.
kannanjgithub Jul 27, 2024
7f83dee
Fix a failing test, and address review comments for StatusOr.java
kannanjgithub Jul 29, 2024
f725239
Fix style issues.
kannanjgithub Jul 29, 2024
5dc6cd9
Fix tests.
kannanjgithub Jul 29, 2024
3102374
Fix style.
kannanjgithub Jul 29, 2024
d6ee012
Fix tests
kannanjgithub Jul 30, 2024
c44fbc2
Remove the warning as error added in my previous commit since in koko…
kannanjgithub Jul 30, 2024
27af56b
Trigger Build to retry the Android test RetriableStreamTest that fail…
kannanjgithub Jul 30, 2024
d02e4aa
Implement equals and hashCode in StatusOr and other things discussed.
kannanjgithub Aug 12, 2024
49dffd5
Fix warning as error for unchecked cast
kannanjgithub Aug 12, 2024
fea7744
Add suppress unchecked cast
kannanjgithub Aug 12, 2024
ad34729
Fix copyright year.
kannanjgithub Aug 12, 2024
e7f7fda
Revert back of() in favor of fromValue(). Made getValue() throw Illeg…
kannanjgithub Sep 26, 2024
97cb232
Merge remote-tracking branch 'origin/onResult2_resolutionResultHaveAd…
kannanjgithub Sep 26, 2024
e136fe5
Add experimental API label. Allow null value.
kannanjgithub Sep 26, 2024
bcf8578
Merge branch 'grpc:master' into onResult2_resolutionResultHaveAddress…
kannanjgithub Oct 4, 2024
18681d1
Address review comments.
kannanjgithub Oct 4, 2024
b67a02b
Merge remote-tracking branch 'origin/onResult2_resolutionResultHaveAd…
kannanjgithub Oct 4, 2024
70056e4
Fix review comments.
kannanjgithub Oct 4, 2024
7dff20e
Address review comments.
kannanjgithub Oct 4, 2024
c80aafb
Fix code coverage by adding missing test cases.
kannanjgithub Oct 4, 2024
1706016
Fix new review comments.
kannanjgithub Oct 4, 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
71 changes: 49 additions & 22 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.MoreObjects;
import com.google.common.base.MoreObjects.ToStringHelper;
import com.google.common.base.Objects;
import com.google.errorprone.annotations.InlineMe;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -95,7 +95,8 @@ public void onError(Status error) {

@Override
public void onResult(ResolutionResult resolutionResult) {
listener.onAddresses(resolutionResult.getAddresses(), resolutionResult.getAttributes());
listener.onAddresses(resolutionResult.getAddressesOrError().getValue(),
resolutionResult.getAttributes());
}
});
}
Expand Down Expand Up @@ -218,19 +219,21 @@ public abstract static class Listener2 implements Listener {
@Override
@Deprecated
@InlineMe(
replacement = "this.onResult(ResolutionResult.newBuilder().setAddresses(servers)"
+ ".setAttributes(attributes).build())",
imports = "io.grpc.NameResolver.ResolutionResult")
replacement = "this.onResult2(ResolutionResult.newBuilder().setAddressesOrError("
+ "StatusOr.fromValue(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
onResult(
ResolutionResult.newBuilder().setAddresses(servers).setAttributes(attributes).build());
onResult2(
ResolutionResult.newBuilder().setAddressesOrError(
StatusOr.fromValue(servers)).setAttributes(attributes).build());
}

/**
* Handles updates on resolved addresses and attributes. If
* {@link ResolutionResult#getAddresses()} is empty, {@link #onError(Status)} will be called.
* {@link ResolutionResult#getAddressesOrError()} is empty, {@link #onError(Status)} will be
* called.
*
* @param resolutionResult the resolved server addresses, attributes, and Service Config.
* @since 1.21.0
Expand Down Expand Up @@ -584,17 +587,17 @@ public abstract static class ServiceConfigParser {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770")
public static final class ResolutionResult {
private final List<EquivalentAddressGroup> addresses;
private final StatusOr<List<EquivalentAddressGroup>> addressesOrError;
@ResolutionResultAttr
private final Attributes attributes;
@Nullable
private final ConfigOrError serviceConfig;

ResolutionResult(
List<EquivalentAddressGroup> addresses,
StatusOr<List<EquivalentAddressGroup>> addressesOrError,
@ResolutionResultAttr Attributes attributes,
ConfigOrError serviceConfig) {
this.addresses = Collections.unmodifiableList(new ArrayList<>(addresses));
this.addressesOrError = addressesOrError;
this.attributes = checkNotNull(attributes, "attributes");
this.serviceConfig = serviceConfig;
}
Expand All @@ -615,7 +618,7 @@ public static Builder newBuilder() {
*/
public Builder toBuilder() {
return newBuilder()
.setAddresses(addresses)
.setAddressesOrError(addressesOrError)
.setAttributes(attributes)
.setServiceConfig(serviceConfig);
}
Expand All @@ -624,9 +627,20 @@ public Builder toBuilder() {
* Gets the addresses resolved by name resolution.
*
* @since 1.21.0
* @deprecated Will be superseded by getAddressesOrError
*/
@Deprecated
public List<EquivalentAddressGroup> getAddresses() {
return addresses;
return addressesOrError.getValue();
}

/**
* Gets the addresses resolved by name resolution or the error in doing so.
*
* @since 1.65.0
*/
public StatusOr<List<EquivalentAddressGroup>> getAddressesOrError() {
return addressesOrError;
}

/**
Expand All @@ -652,11 +666,11 @@ public ConfigOrError getServiceConfig() {

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("addresses", addresses)
.add("attributes", attributes)
.add("serviceConfig", serviceConfig)
.toString();
ToStringHelper stringHelper = MoreObjects.toStringHelper(this);
stringHelper.add("addressesOrError", addressesOrError.toString());
stringHelper.add("attributes", attributes);
stringHelper.add("serviceConfigOrError", serviceConfig);
return stringHelper.toString();
}

/**
Expand All @@ -668,7 +682,7 @@ public boolean equals(Object obj) {
return false;
}
ResolutionResult that = (ResolutionResult) obj;
return Objects.equal(this.addresses, that.addresses)
return Objects.equal(this.addressesOrError, that.addressesOrError)
&& Objects.equal(this.attributes, that.attributes)
&& Objects.equal(this.serviceConfig, that.serviceConfig);
}
Expand All @@ -678,7 +692,7 @@ public boolean equals(Object obj) {
*/
@Override
public int hashCode() {
return Objects.hashCode(addresses, attributes, serviceConfig);
return Objects.hashCode(addressesOrError, attributes, serviceConfig);
}

/**
Expand All @@ -688,7 +702,8 @@ public int hashCode() {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770")
public static final class Builder {
private List<EquivalentAddressGroup> addresses = Collections.emptyList();
private StatusOr<List<EquivalentAddressGroup>> addresses =
StatusOr.fromValue(Collections.emptyList());
private Attributes attributes = Attributes.EMPTY;
@Nullable
private ConfigOrError serviceConfig;
Expand All @@ -700,9 +715,21 @@ public static final class Builder {
* Sets the addresses resolved by name resolution. This field is required.
*
* @since 1.21.0
* @deprecated Will be superseded by setAddressesOrError
*/
@Deprecated
public Builder setAddresses(List<EquivalentAddressGroup> addresses) {
this.addresses = addresses;
setAddressesOrError(StatusOr.fromValue(addresses));
return this;
}

/**
* Sets the addresses resolved by name resolution or the error in doing so. This field is
* required.
* @param addresses Resolved addresses or an error in resolving addresses
*/
public Builder setAddressesOrError(StatusOr<List<EquivalentAddressGroup>> addresses) {
this.addresses = checkNotNull(addresses, "StatusOr addresses cannot be null.");
return this;
}

Expand Down
102 changes: 102 additions & 0 deletions api/src/main/java/io/grpc/StatusOr.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright 2024 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.MoreObjects;
import com.google.common.base.MoreObjects.ToStringHelper;
import com.google.common.base.Objects;
import javax.annotation.Nullable;

/** Either a Status or a value. */
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/11563")
public class StatusOr<T> {
private StatusOr(Status status, T value) {
this.status = status;
this.value = value;
}

/** Construct from a value. */
public static <T> StatusOr<T> fromValue(@Nullable T value) {
StatusOr<T> result = new StatusOr<T>(null, value);
return result;
}

/** Construct from a non-Ok status. */
public static <T> StatusOr<T> fromStatus(Status status) {
StatusOr<T> result = new StatusOr<T>(checkNotNull(status, "status"), null);
checkArgument(!status.isOk(), "cannot use OK status: %s", status);
return result;
}

/** Returns whether there is a value. */
public boolean hasValue() {
return status == null;
}

/**
* Returns the value if set or throws exception if there is no value set. This method is meant
* to be called after checking the return value of hasValue() first.
*/
public @Nullable T getValue() {
if (status != null) {
throw new IllegalStateException("No value present.");
}
return value;
}

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

@Override
public boolean equals(Object other) {
if (!(other instanceof StatusOr)) {
return false;

Check warning on line 72 in api/src/main/java/io/grpc/StatusOr.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/StatusOr.java#L72

Added line #L72 was not covered by tests
}
StatusOr<?> otherStatus = (StatusOr<?>) other;
if (hasValue() != otherStatus.hasValue()) {
return false;

Check warning on line 76 in api/src/main/java/io/grpc/StatusOr.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/StatusOr.java#L76

Added line #L76 was not covered by tests
}
if (hasValue()) {
return Objects.equal(value, otherStatus.value);
}
return Objects.equal(status, otherStatus.status);
}

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

@Override
public String toString() {
ToStringHelper stringHelper = MoreObjects.toStringHelper(this);
if (status == null) {
stringHelper.add("value", value);
} else {
stringHelper.add("error", status);
}
return stringHelper.toString();
}

private final Status status;
private final T value;
}
Loading