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

[ggj][codgen] fix: add HTTP's additional_bindings to GrpcServiceStub call settings #591

Merged
merged 7 commits into from
Dec 7, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.api.generator.gapic.model.Method;
import com.google.api.generator.gapic.model.MethodArgument;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.JavaStyle;
import com.google.common.base.Strings;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -181,7 +182,7 @@ static List<CommentStatement> createRpcMethodHeaderComment(
// TODO(miraleung): Remove the newline replacement when we support CommonMark.
String description =
argument.field().hasDescription() ? argument.field().description() : EMPTY_STRING;
methodJavadocBuilder.addParam(argument.name(), description);
methodJavadocBuilder.addParam(JavaStyle.toLowerCamelCase(argument.name()), description);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@
import com.google.protobuf.Descriptors.MethodDescriptor;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

public class HttpRuleParser {
private static final String ASTERISK = "*";
Expand All @@ -48,11 +50,20 @@ public static Optional<List<String>> parseHttpBindings(
}

// Get pattern.
List<String> bindings = getPatternBindings(httpRule);
if (bindings.isEmpty()) {
Set<String> uniqueBindings = getPatternBindings(httpRule);
if (uniqueBindings.isEmpty()) {
return Optional.empty();
}

if (httpRule.getAdditionalBindingsCount() > 0) {
for (HttpRule additionalRule : httpRule.getAdditionalBindingsList()) {
uniqueBindings.addAll(getPatternBindings(additionalRule));
}
}

List<String> bindings = new ArrayList<>(uniqueBindings);
Collections.sort(bindings);

// Binding validation.
for (String binding : bindings) {
// Handle foo.bar cases by descending into the subfields.
Expand All @@ -77,7 +88,7 @@ public static Optional<List<String>> parseHttpBindings(
return Optional.of(bindings);
}

private static List<String> getPatternBindings(HttpRule httpRule) {
private static Set<String> getPatternBindings(HttpRule httpRule) {
String pattern = null;
// Assign a temp variable to prevent the formatter from removing the import.
PatternCase patternCase = httpRule.getPatternCase();
Expand All @@ -100,12 +111,11 @@ private static List<String> getPatternBindings(HttpRule httpRule) {
case CUSTOM: // Invalid pattern.
// Fall through.
default:
return Collections.emptyList();
return Collections.emptySet();
}

PathTemplate template = PathTemplate.create(pattern);
List<String> bindings = new ArrayList<String>(template.vars());
Collections.sort(bindings);
Set<String> bindings = new HashSet<String>(template.vars());
return bindings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ public class EchoClient implements BackgroundResource {

// AUTO-GENERATED DOCUMENTATION AND METHOD.
/**
* @param end_time
* @param endTime
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final OperationFuture<WaitResponse, WaitMetadata> waitAsync(Timestamp endTime) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,11 @@ public class GrpcTestingStub extends TestingStub {
@Override
public Map<String, String> extract(VerifyTestRequest request) {
ImmutableMap.Builder<String, String> params = ImmutableMap.builder();
params.put("answer", String.valueOf(request.getAnswer()));
params.put("foo", String.valueOf(request.getFoo()));
params.put("name", String.valueOf(request.getName()));
params.put(
"test_to_verify.name", String.valueOf(request.getTestToVerify().getName()));
return params.build();
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public class IdentityClient implements BackgroundResource {
// AUTO-GENERATED DOCUMENTATION AND METHOD.
/**
* @param parent
* @param display_name
* @param displayName
* @param email
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
Expand All @@ -142,12 +142,12 @@ public class IdentityClient implements BackgroundResource {
// AUTO-GENERATED DOCUMENTATION AND METHOD.
/**
* @param parent
* @param display_name
* @param displayName
* @param email
* @param age
* @param nickname
* @param enable_notifications
* @param height_feet
* @param enableNotifications
* @param heightFeet
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final User createUser(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,8 @@ public class TestingClientTest {
.setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString())
.setAnswer(ByteString.EMPTY)
.addAllAnswers(new ArrayList<ByteString>())
.setFoo("foo101574")
.setTestToVerify(com.google.showcase.v1beta1.Test.newBuilder().build())
.build();

VerifyTestResponse actualResponse = client.verifyTest(request);
Expand All @@ -477,6 +479,8 @@ public class TestingClientTest {
Assert.assertEquals(request.getName(), actualRequest.getName());
Assert.assertEquals(request.getAnswer(), actualRequest.getAnswer());
Assert.assertEquals(request.getAnswersList(), actualRequest.getAnswersList());
Assert.assertEquals(request.getFoo(), actualRequest.getFoo());
Assert.assertEquals(request.getTestToVerify(), actualRequest.getTestToVerify());
Assert.assertTrue(
channelProvider.isHeaderSent(
ApiClientHeaderProvider.getDefaultApiClientHeaderKey(),
Expand All @@ -494,6 +498,8 @@ public class TestingClientTest {
.setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString())
.setAnswer(ByteString.EMPTY)
.addAllAnswers(new ArrayList<ByteString>())
.setFoo("foo101574")
.setTestToVerify(com.google.showcase.v1beta1.Test.newBuilder().build())
.build();
client.verifyTest(request);
Assert.fail("No exception raised");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,33 @@ public void parseHttpAnnotation_basic() {
HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
assertFalse(httpBindingsOpt.isPresent());

// VerityTest method.
rpcMethod = testingService.getMethods().get(testingService.getMethods().size() - 1);
inputMessage = messages.get("VerifyTestRequest");
// GetSession method.
rpcMethod = testingService.getMethods().get(1);
inputMessage = messages.get("GetSessionRequest");
httpBindingsOpt = HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
assertTrue(httpBindingsOpt.isPresent());
assertThat(httpBindingsOpt.get()).containsExactly("name");
}

@Test
public void parseHttpAnnotation_multipleBindings() {
FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();
ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0);
assertEquals("Testing", testingService.getName());

Map<String, Message> messages = Parser.parseMessages(testingFileDescriptor);

// VerityTest method.
MethodDescriptor rpcMethod =
testingService.getMethods().get(testingService.getMethods().size() - 1);
Message inputMessage = messages.get("VerifyTestRequest");
Optional<List<String>> httpBindingsOpt =
HttpRuleParser.parseHttpBindings(rpcMethod, inputMessage, messages);
assertTrue(httpBindingsOpt.isPresent());
assertThat(httpBindingsOpt.get())
.containsExactly("answer", "foo", "name", "test_to_verify.name");
}

@Test
public void parseHttpAnnotation_missingFieldFromMessage() {
FileDescriptor testingFileDescriptor = TestingOuterClass.getDescriptor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,19 @@ service Testing {
rpc VerifyTest(VerifyTestRequest) returns (VerifyTestResponse) {
option (google.api.http) = {
post: "/v1beta1/{name=sessions/*/tests/*}:check"
body: "*"
additional_bindings {
post: "/v1beta1/{foo=sessions/*/tests/*}:check"
body: "*"
}
additional_bindings {
post: "/v1beta1/{answer=sessions/*/tests/*}:check"
body: "*"
}
additional_bindings {
post: "/v1beta1/{test_to_verify.name=sessions/*/tests/*}:check"
body: "*"
}
};
}
}
Expand Down Expand Up @@ -392,6 +405,11 @@ message VerifyTestRequest {

// The answers from the test if multiple are to be checked
repeated bytes answers = 3;

// Test fields for HTTP annotations.
string foo = 4;

Test test_to_verify = 5;
}

message VerifyTestResponse {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/goldens/asset/AssetServiceClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ public final UnaryCallable<DeleteFeedRequest, Empty> deleteFeedCallable() {
* "us-west1" region or the "global" location.
* </ul>
*
* @param asset_types Optional. A list of asset types that this request searches for. If empty, it
* @param assetTypes Optional. A list of asset types that this request searches for. If empty, it
* will search all the [searchable asset
* types](https://cloud.google.com/asset-inventory/docs/supported-asset-types#searchable_asset_types).
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
Expand Down
16 changes: 8 additions & 8 deletions test/integration/goldens/library/LibraryServiceClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ public final UnaryCallable<DeleteShelfRequest, Empty> deleteShelfCallable() {
* shelves are the same.
*
* @param name The name of the shelf we're adding books to.
* @param other_shelf_name The name of the shelf we're removing books from and deleting.
* @param otherShelfName The name of the shelf we're removing books from and deleting.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Shelf mergeShelves(ShelfName name, ShelfName otherShelfName) {
Expand All @@ -358,7 +358,7 @@ public final Shelf mergeShelves(ShelfName name, ShelfName otherShelfName) {
* shelves are the same.
*
* @param name The name of the shelf we're adding books to.
* @param other_shelf_name The name of the shelf we're removing books from and deleting.
* @param otherShelfName The name of the shelf we're removing books from and deleting.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Shelf mergeShelves(ShelfName name, String otherShelfName) {
Expand All @@ -380,7 +380,7 @@ public final Shelf mergeShelves(ShelfName name, String otherShelfName) {
* shelves are the same.
*
* @param name The name of the shelf we're adding books to.
* @param other_shelf_name The name of the shelf we're removing books from and deleting.
* @param otherShelfName The name of the shelf we're removing books from and deleting.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Shelf mergeShelves(String name, ShelfName otherShelfName) {
Expand All @@ -402,7 +402,7 @@ public final Shelf mergeShelves(String name, ShelfName otherShelfName) {
* shelves are the same.
*
* @param name The name of the shelf we're adding books to.
* @param other_shelf_name The name of the shelf we're removing books from and deleting.
* @param otherShelfName The name of the shelf we're removing books from and deleting.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Shelf mergeShelves(String name, String otherShelfName) {
Expand Down Expand Up @@ -668,7 +668,7 @@ public final UnaryCallable<UpdateBookRequest, Book> updateBookCallable() {
* the same as the original book.
*
* @param name The name of the book to move.
* @param other_shelf_name The name of the destination shelf.
* @param otherShelfName The name of the destination shelf.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Book moveBook(BookName name, ShelfName otherShelfName) {
Expand All @@ -686,7 +686,7 @@ public final Book moveBook(BookName name, ShelfName otherShelfName) {
* the same as the original book.
*
* @param name The name of the book to move.
* @param other_shelf_name The name of the destination shelf.
* @param otherShelfName The name of the destination shelf.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Book moveBook(BookName name, String otherShelfName) {
Expand All @@ -704,7 +704,7 @@ public final Book moveBook(BookName name, String otherShelfName) {
* the same as the original book.
*
* @param name The name of the book to move.
* @param other_shelf_name The name of the destination shelf.
* @param otherShelfName The name of the destination shelf.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Book moveBook(String name, ShelfName otherShelfName) {
Expand All @@ -722,7 +722,7 @@ public final Book moveBook(String name, ShelfName otherShelfName) {
* the same as the original book.
*
* @param name The name of the book to move.
* @param other_shelf_name The name of the destination shelf.
* @param otherShelfName The name of the destination shelf.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Book moveBook(String name, String otherShelfName) {
Expand Down
Loading