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

fix: Use bindings for resolving multi pattern resources #1818

Merged
merged 24 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0d6bb9f
fix: Use bindings for resolving multi pattern resources
lqiu96 Jun 29, 2023
01e6a18
chore: Fix lint issues
lqiu96 Jun 29, 2023
cbbf112
chore: Add unit tests for the behavior
lqiu96 Jul 5, 2023
265b11c
chore: Add comments for the tests
lqiu96 Jul 6, 2023
8c1b754
chore: Remove unused comment
lqiu96 Jul 6, 2023
23c8e9c
feat: support GDC-H Credentials (#1642)
diegomarquezp Jun 28, 2023
d4b2c54
fix: [gapic-generator-java] handle response and metadata type ambigui…
emmileaf Jul 5, 2023
d768cb4
chore: Bump grpc-java version to 1.55.3 (#1829)
lqiu96 Jul 5, 2023
b1d32b5
chore: Bump gapic-showcase version to 0.28.2 (#1830)
lqiu96 Jul 5, 2023
6d64b8f
build(deps): Bump guava version to 32.1.1-jre (#1832)
lqiu96 Jul 7, 2023
78446a7
chore: Add j2obc-annotations to shared-dependencies (#1834)
lqiu96 Jul 7, 2023
3de86e1
ci: fix showcase-clirr check on release PRs (#1835)
burkedavison Jul 7, 2023
8c6a19b
chore(main): release 2.23.0 (#1806)
release-please[bot] Jul 7, 2023
a6d9ef8
chore(main): release 2.23.1-SNAPSHOT (#1836)
release-please[bot] Jul 7, 2023
1fe186a
chore: Implement gRPC and HttpJson showcase tests for IAM (#1789)
lqiu96 Jul 7, 2023
c9cb1e4
ci: showcase native check (#1833)
burkedavison Jul 10, 2023
e965680
chore: Resolve merge conflicts
lqiu96 Jul 12, 2023
6accafc
Merge branch 'main' into fix-default-resource-name
lqiu96 Jul 12, 2023
9a96130
chore: Use bindings for any matching resource patterns
lqiu96 Jul 13, 2023
cb61c4d
chore: Fix lint issues
lqiu96 Jul 13, 2023
161c70e
chore: Remove unused code
lqiu96 Jul 14, 2023
821b8c9
chore: Fix lint issues
lqiu96 Jul 14, 2023
f77800e
chore: Add test for resourceName matching bindings
lqiu96 Jul 14, 2023
92b51dc
chore: Fix lint issues
lqiu96 Jul 14, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,25 @@ public static Expr createMethodArgValue(
}

if (methodArg.type().equals(methodArg.field().type())) {
return createValue(methodArg.field(), false, resourceNames, messageTypes, valuePatterns);
return createValue(
methodArg.field(), false, resourceNames, messageTypes, valuePatterns, bindings);
}

return createValue(Field.builder().setName(methodArg.name()).setType(methodArg.type()).build());
}

public static Expr createValue(Field field) {
return createValue(
field, false, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap());
field, false, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), null);
}

public static Expr createValue(
Field field,
boolean useExplicitInitTypeInGenerics,
Map<String, ResourceName> resourceNames,
Map<String, Message> messageTypes,
Map<String, String> valuePatterns) {
Map<String, String> valuePatterns,
HttpBindings bindings) {
if (field.isRepeated()) {
ConcreteReference.Builder refBuilder =
ConcreteReference.builder().setClazz(field.isMap() ? HashMap.class : ArrayList.class);
Expand Down Expand Up @@ -161,7 +163,7 @@ public static Expr createValue(
Message nestedMessage = messageTypes.get(field.type().reference().fullName());
if (nestedMessage != null) {
return createSimpleMessageBuilderValue(
nestedMessage, resourceNames, messageTypes, nestedValuePatterns, null);
nestedMessage, resourceNames, messageTypes, nestedValuePatterns, bindings);
}
}

Expand Down Expand Up @@ -274,7 +276,7 @@ static Expr createResourceHelperValue(
if (!resname.isOnlyWildcard()
&& (bindings == null || resname.getMatchingPattern(bindings) != null)) {
return createResourceHelperValue(
resname, false, unexaminedResnames, fieldOrMessageName, null);
resname, false, unexaminedResnames, fieldOrMessageName, bindings);
}
}

Expand Down Expand Up @@ -420,7 +422,8 @@ public static Expr createSimpleMessageBuilderValue(
}

if (defaultExpr == null) {
defaultExpr = createValue(field, true, resourceNames, messageTypes, valuePatterns);
defaultExpr =
createValue(field, true, resourceNames, messageTypes, valuePatterns, bindings);
}
}
builderExpr =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.protoparser.Parser;
import com.google.api.generator.test.utils.LineFormatter;
import com.google.common.collect.ImmutableList;
import com.google.protobuf.ByteString;
import com.google.protobuf.Descriptors.FileDescriptor;
import com.google.protobuf.StructProto;
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.MessagingOuterClass;
import com.google.testgapic.v1beta1.LockerProto;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -174,7 +176,7 @@ public void defaultValue_resourceNameWithOnePattern() {
}

@Test
public void defaultValue_resourceNameWithMultiplePatterns() {
public void defaultValue_resourceNameWithMultiplePatterns_noBindings() {
FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor();
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(lockerServiceFileDescriptor);
Expand All @@ -188,10 +190,45 @@ public void defaultValue_resourceNameWithMultiplePatterns() {
"ignored",
null);
expr.accept(writerVisitor);
/*
There are two patterns:
- pattern: "projects/{project}/folders/{folder}"
- pattern: "folders/{folder}"
It matches the first one given no bindings
*/
assertEquals(
"FolderName.ofProjectFolderName(\"[PROJECT]\", \"[FOLDER]\")", writerVisitor.write());
}

@Test
public void defaultValue_resourceNameWithMultiplePatterns_matchesBindings() {
FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor();
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(lockerServiceFileDescriptor);
ResourceName resourceName =
typeStringsToResourceNames.get("cloudresourcemanager.googleapis.com/Folder");
Expr expr =
DefaultValueComposer.createResourceHelperValue(
resourceName,
false,
typeStringsToResourceNames.values().stream().collect(Collectors.toList()),
"folder",
HttpBindings.builder()
.setHttpVerb(HttpVerb.POST)
.setPattern("/v1/{name=folders/*}")
.setAdditionalPatterns(ImmutableList.of())
.setIsAsteriskBody(true)
.build());
expr.accept(writerVisitor);
/*
There are two patterns:
- pattern: "projects/{project}/folders/{folder}"
- pattern: "folders/{folder}"
It attempts to match the correct HttpBinding
*/
assertEquals("FolderName.ofFolderName(\"[FOLDER]\")", writerVisitor.write());
}

@Test
public void defaultValue_resourceNameWithWildcardPattern() {
FileDescriptor lockerServiceFileDescriptor = LockerProto.getDescriptor();
Expand Down Expand Up @@ -374,6 +411,131 @@ public void defaultValue_resourceNameWithOnlyWildcards_allowAnonResourceNameClas
assertEquals(expected, writerVisitor.write());
}

@Test
public void createSimpleMessageBuilderValue_resourceNameMultiplePatterns_matchesHttpBinding() {
FileDescriptor messagingFileDescriptor = MessagingOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(messagingFileDescriptor);
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(messagingFileDescriptor);
/*
Blurb Resource contains four patterns (in order of):
- pattern: "users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}"
- pattern: "users/{user}/profile/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}"
*/
Message message = messageTypes.get("com.google.showcase.v1beta1.Blurb");

HttpBindings bindings =
HttpBindings.builder()
.setHttpVerb(HttpVerb.PATCH)
.setPattern("/v1beta1/{blurb.name=rooms/*/blurbs/*}")
.setAdditionalPatterns(
Collections.singletonList("/v1beta1/{blurb.name=users/*/profile/blurbs/*}"))
.setIsAsteriskBody(false)
.build();

Expr expr =
DefaultValueComposer.createSimpleMessageBuilderValue(
message, typeStringsToResourceNames, messageTypes, bindings);
expr.accept(writerVisitor);
// Matches with the default pattern in the HttpBindings and uses the variables in the pattern
assertEquals(
"Blurb.newBuilder().setName(BlurbName.ofRoomBlurbName(\"[ROOM]\", \"[BLURB]\").toString()).build()",
writerVisitor.write());
}

@Test
public void
createSimpleMessageBuilderValue_resourceNameMultiplePatterns_matchesAdditionalHttpBinding() {
FileDescriptor messagingFileDescriptor = MessagingOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(messagingFileDescriptor);
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(messagingFileDescriptor);
/*
Blurb Resource contains four patterns (in order of):
- pattern: "users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}"
- pattern: "users/{user}/profile/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}"
*/
Message message = messageTypes.get("com.google.showcase.v1beta1.Blurb");

HttpBindings bindings =
HttpBindings.builder()
.setHttpVerb(HttpVerb.PATCH)
.setPattern("/v1beta1/{blurb.name=invalid/pattern/*}")
.setAdditionalPatterns(
Collections.singletonList("/v1beta1/{blurb.name=users/*/profile/blurbs/*}"))
.setIsAsteriskBody(false)
.build();

Expr expr =
DefaultValueComposer.createSimpleMessageBuilderValue(
message, typeStringsToResourceNames, messageTypes, bindings);
expr.accept(writerVisitor);
// Because the default pattern does not match, it will attempt to match with patterns in the
// additional bindings and use variables for any pattern that matches
assertEquals(
"Blurb.newBuilder().setName(BlurbName.ofUserBlurbName(\"[USER]\", \"[BLURB]\").toString()).build()",
writerVisitor.write());
}

@Test
public void
createSimpleMessageBuilderValue_resourceNameMultiplePatterns_doesNotMatchHttpBinding() {
FileDescriptor messagingFileDescriptor = MessagingOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(messagingFileDescriptor);
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(messagingFileDescriptor);
/*
Blurb Resource contains four patterns (in order of):
- pattern: "users/{user}/profile/blurbs/legacy/{legacy_user}~{blurb}"
- pattern: "users/{user}/profile/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/{blurb}"
- pattern: "rooms/{room}/blurbs/legacy/{legacy_room}.{blurb}"
*/
Message message = messageTypes.get("com.google.showcase.v1beta1.Blurb");

HttpBindings bindings =
HttpBindings.builder()
.setHttpVerb(HttpVerb.PATCH)
.setPattern("/v1beta1/{blurb.name=invalid/pattern/*}")
.setAdditionalPatterns(
Collections.singletonList("/v1beta1/{blurb.name=nothing/matches/*}"))
.setIsAsteriskBody(false)
.build();

Expr expr =
DefaultValueComposer.createSimpleMessageBuilderValue(
message, typeStringsToResourceNames, messageTypes, bindings);
expr.accept(writerVisitor);
// If no pattern matches (default and additional bindings), it will simply pick the first
// resource pattern in the resource definition.
assertEquals(
"Blurb.newBuilder().setName(BlurbName.ofUserLegacyUserBlurbName(\"[USER]\", \"[LEGACY_USER]\", \"[BLURB]\").toString()).build()",
writerVisitor.write());
}

@Test
public void defaultValue_resourceNameMultiplePatterns_noHttpBinding() {
FileDescriptor messagingFileDescriptor = MessagingOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(messagingFileDescriptor);
Map<String, ResourceName> typeStringsToResourceNames =
Parser.parseResourceNames(messagingFileDescriptor);
Message message = messageTypes.get("com.google.showcase.v1beta1.Blurb");

Expr expr =
DefaultValueComposer.createSimpleMessageBuilderValue(
message, typeStringsToResourceNames, messageTypes, null);
expr.accept(writerVisitor);
// If no pattern matches (default and additional bindings), it will simply pick the first
// resource pattern in the resource definition.
assertEquals(
"Blurb.newBuilder().setName(BlurbName.ofUserLegacyUserBlurbName(\"[USER]\", \"[LEGACY_USER]\", \"[BLURB]\").toString()).build()",
writerVisitor.write());
}

@Test
public void createSimpleMessage_basicPrimitivesOnly() {
FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncGetIamPolicy() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
GetIamPolicyRequest request =
GetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setOptions(GetPolicyOptions.newBuilder().build())
.build();
ApiFuture<Policy> future = complianceClient.getIamPolicyCallable().futureCall(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncGetIamPolicy() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
GetIamPolicyRequest request =
GetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setOptions(GetPolicyOptions.newBuilder().build())
.build();
Policy response = complianceClient.getIamPolicy(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncSetIamPolicy() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setPolicy(Policy.newBuilder().build())
.setUpdateMask(FieldMask.newBuilder().build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncSetIamPolicy() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setPolicy(Policy.newBuilder().build())
.setUpdateMask(FieldMask.newBuilder().build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncTestIamPermissions() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
TestIamPermissionsRequest request =
TestIamPermissionsRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.addAllPermissions(new ArrayList<String>())
.build();
ApiFuture<TestIamPermissionsResponse> future =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncTestIamPermissions() throws Exception {
try (ComplianceClient complianceClient = ComplianceClient.create()) {
TestIamPermissionsRequest request =
TestIamPermissionsRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.addAllPermissions(new ArrayList<String>())
.build();
TestIamPermissionsResponse response = complianceClient.testIamPermissions(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncGetIamPolicy() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
GetIamPolicyRequest request =
GetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setOptions(GetPolicyOptions.newBuilder().build())
.build();
ApiFuture<Policy> future = echoClient.getIamPolicyCallable().futureCall(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncGetIamPolicy() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
GetIamPolicyRequest request =
GetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setOptions(GetPolicyOptions.newBuilder().build())
.build();
Policy response = echoClient.getIamPolicy(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncSetIamPolicy() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setPolicy(Policy.newBuilder().build())
.setUpdateMask(FieldMask.newBuilder().build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncSetIamPolicy() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setPolicy(Policy.newBuilder().build())
.setUpdateMask(FieldMask.newBuilder().build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncTestIamPermissions() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
TestIamPermissionsRequest request =
TestIamPermissionsRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.addAllPermissions(new ArrayList<String>())
.build();
ApiFuture<TestIamPermissionsResponse> future =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public static void syncTestIamPermissions() throws Exception {
try (EchoClient echoClient = EchoClient.create()) {
TestIamPermissionsRequest request =
TestIamPermissionsRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.addAllPermissions(new ArrayList<String>())
.build();
TestIamPermissionsResponse response = echoClient.testIamPermissions(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ public static void asyncGetIamPolicy() throws Exception {
try (IdentityClient identityClient = IdentityClient.create()) {
GetIamPolicyRequest request =
GetIamPolicyRequest.newBuilder()
.setResource(
BlurbName.ofUserLegacyUserBlurbName("[USER]", "[LEGACY_USER]", "[BLURB]")
.toString())
.setResource(BlurbName.ofRoomBlurbName("[ROOM]", "[BLURB]").toString())
.setOptions(GetPolicyOptions.newBuilder().build())
.build();
ApiFuture<Policy> future = identityClient.getIamPolicyCallable().futureCall(request);
Expand Down
Loading
Loading