Skip to content

xds: add more route matching types in converted Route data structure - #7031

Merged
voidzcy merged 17 commits into
grpc:masterfrom
voidzcy:impl/support_more_route_matching_types
May 18, 2020
Merged

xds: add more route matching types in converted Route data structure#7031
voidzcy merged 17 commits into
grpc:masterfrom
voidzcy:impl/support_more_route_matching_types

Conversation

@voidzcy

@voidzcy voidzcy commented May 12, 2020

Copy link
Copy Markdown
Contributor
  • All objects of converted types should be valid to gRPC (if the original proto message contains invalid data, the conversion should fail and no object should be instantiated).
  • Use null in converted objects for unset fields in proto messages. This puts a much stronger/clearer signal for proto's oneof type.

Most conversion logic and tests are trivial. But the way we represent data and perform checks matters.

/** See corresponding Envoy proto message {@link io.envoyproxy.envoy.api.v2.route.Route}. */
static final class Route {
private final RouteMatch routeMatch;
@Nullable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Justification: a valid Route should always have a valid matcher and a valid action.

Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java
@voidzcy
voidzcy marked this pull request as ready for review May 14, 2020 23:23
@voidzcy
voidzcy requested review from dapengzhang0 and removed request for dapengzhang0 May 14, 2020 23:23
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/EnvoyProtoData.java Outdated
Comment thread xds/src/main/java/io/grpc/xds/XdsClientImpl.java
Comment thread xds/src/main/java/io/grpc/xds/XdsClientImpl.java
Comment thread xds/src/main/java/io/grpc/xds/XdsClientImpl.java Outdated
@voidzcy

voidzcy commented May 15, 2020

Copy link
Copy Markdown
Contributor Author

I added logic to validate if the regex string is legal when converting the proto. The compiled Pattern will be saved so that consumers do not need to compile again.

&& Objects.equals(pathSafeRegExMatch, that.pathSafeRegExMatch)
&& Objects.equals(
pathSafeRegExMatch == null ? null : pathSafeRegExMatch.pattern(),
that.pathSafeRegExMatch == null ? null : that.pathSafeRegExMatch.pattern())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Equal objects must have equal hashCode.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Good catch. Thanks.

@voidzcy
voidzcy merged commit 02e3c00 into grpc:master May 18, 2020
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
…rpc#7031)

Parse other matcher types (e.g., header matchers, runtime fraction matchers, etc) that xDS Route supports.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants