From 60f65eabedb20813fbfae6b60a194f9e6e4257bb Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 23 Dec 2020 19:58:37 -0800 Subject: [PATCH] [ggj][nestedsig] feat: add support for nested method sigs (aip.dev/4232) (#626) * fix: fix dep ordering in Bazel dedupe rules * feat: add a basic trie * fix: add hashCode() and equals() to Field * fix: add equals() and hashCode() for MethodArgument * feat: add a TriFunction * feat: add DFS traversal to Trie * feat: add support for nested method sigs (aip.dev/4232) --- .../api/generator/gapic/composer/BUILD.bazel | 1 + .../composer/ServiceClientClassComposer.java | 68 ++++++++++++++--- .../composer/goldens/IdentityClient.golden | 74 +++++++++++++++++-- .../generator/gapic/testdata/identity.proto | 52 +++++++++++++ 4 files changed, 177 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel index 4555aed048..26874fbd13 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/main/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -23,6 +23,7 @@ java_library( "//src/main/java/com/google/api/generator/gapic/composer/utils", "//src/main/java/com/google/api/generator/gapic/model", "//src/main/java/com/google/api/generator/gapic/utils", + "//src/main/java/com/google/api/generator/util", "@com_google_api_api_common//jar", "@com_google_api_gax_java//gax", "@com_google_api_gax_java//gax-grpc:gax_grpc", diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java index 0fc9325544..3c82ab145d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java @@ -68,6 +68,8 @@ 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.api.generator.util.TriFunction; +import com.google.api.generator.util.Trie; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -84,6 +86,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Collectors; import javax.annotation.Generated; @@ -1336,23 +1339,68 @@ private static ClassDefinition createNestedRpcFixedSizeCollectionClass( static Expr createRequestBuilderExpr( Method method, List signature, TypeStore typeStore) { TypeNode methodInputType = method.inputType(); - MethodInvocationExpr newBuilderExpr = + Expr newBuilderExpr = MethodInvocationExpr.builder() .setMethodName("newBuilder") .setStaticReferenceType(methodInputType) .build(); - for (MethodArgument argument : signature) { - newBuilderExpr = buildNestedSetterInvocationExpr(argument, newBuilderExpr); + // Maintain the args' order of appearance for better determinism. + List rootFields = new ArrayList<>(); + Map> rootFieldToTrie = new HashMap<>(); + for (MethodArgument arg : signature) { + Field rootField = arg.nestedFields().isEmpty() ? arg.field() : arg.nestedFields().get(0); + if (!rootFields.contains(rootField)) { + rootFields.add(rootField); + } + Trie updatedTrie = + rootFieldToTrie.containsKey(rootField) ? rootFieldToTrie.get(rootField) : new Trie(); + List nestedFieldsWithChild = new ArrayList<>(arg.nestedFields()); + nestedFieldsWithChild.add(arg.field()); + updatedTrie.insert(nestedFieldsWithChild); + rootFieldToTrie.put(rootField, updatedTrie); } - MethodInvocationExpr builderExpr = - MethodInvocationExpr.builder() - .setMethodName("build") - .setExprReferenceExpr(newBuilderExpr) - .setReturnType(methodInputType) - .build(); + Function parentPreprocFn = + field -> + (Expr) + MethodInvocationExpr.builder() + .setStaticReferenceType(field.type()) + .setMethodName("newBuilder") + .build(); + TriFunction parentPostprocFn = + (field, baseRefExpr, leafProcessedExpr) -> { + boolean isRootNode = field == null; + return isRootNode + ? leafProcessedExpr + : MethodInvocationExpr.builder() + .setExprReferenceExpr(baseRefExpr) + .setMethodName(String.format("set%s", JavaStyle.toUpperCamelCase(field.name()))) + .setArguments( + MethodInvocationExpr.builder() + .setExprReferenceExpr(leafProcessedExpr) + .setMethodName("build") + .build()) + .build(); + }; + + final Map fieldToMethodArg = + signature.stream().collect(Collectors.toMap(a -> a.field(), a -> a)); + BiFunction leafProcFn = + (field, parentBaseRefExpr) -> + (Expr) buildNestedSetterInvocationExpr(fieldToMethodArg.get(field), parentBaseRefExpr); + + for (Field rootField : rootFields) { + newBuilderExpr = + rootFieldToTrie + .get(rootField) + .dfsTraverseAndReduce(parentPreprocFn, parentPostprocFn, leafProcFn, newBuilderExpr); + } - return builderExpr; + return MethodInvocationExpr.builder() + .setExprReferenceExpr(newBuilderExpr) + .setMethodName("build") + .setReturnType(methodInputType) + .build(); } @VisibleForTesting diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden index 323a5cdbb3..edd19b1601 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/IdentityClient.golden @@ -133,8 +133,7 @@ public class IdentityClient implements BackgroundResource { CreateUserRequest request = CreateUserRequest.newBuilder() .setParent(parent) - .setDisplayName(displayName) - .setEmail(email) + .setUser(User.newBuilder().setDisplayName(displayName).setEmail(email).build()) .build(); return createUser(request); } @@ -161,12 +160,71 @@ public class IdentityClient implements BackgroundResource { CreateUserRequest request = CreateUserRequest.newBuilder() .setParent(parent) - .setDisplayName(displayName) - .setEmail(email) - .setAge(age) - .setNickname(nickname) - .setEnableNotifications(enableNotifications) - .setHeightFeet(heightFeet) + .setUser( + User.newBuilder() + .setDisplayName(displayName) + .setEmail(email) + .setAge(age) + .setNickname(nickname) + .setEnableNotifications(enableNotifications) + .setHeightFeet(heightFeet) + .build()) + .build(); + return createUser(request); + } + + // AUTO-GENERATED DOCUMENTATION AND METHOD. + /** + * @param parent + * @param displayName + * @param email + * @param hobbyName + * @param songName + * @param weeklyFrequency + * @param companyName + * @param title + * @param subject + * @param artistName + * @throws com.google.api.gax.rpc.ApiException if the remote call fails + */ + public final User createUser( + String parent, + String displayName, + String email, + String hobbyName, + String songName, + int weeklyFrequency, + String companyName, + String title, + String subject, + String artistName) { + CreateUserRequest request = + CreateUserRequest.newBuilder() + .setParent(parent) + .setUser( + User.newBuilder() + .setDisplayName(displayName) + .setEmail(email) + .setHobby( + Hobby.newBuilder() + .setHobbyName(hobbyName) + .setWeeklyFrequency(weeklyFrequency) + .setInstructionManual( + Book.newBuilder().setTitle(title).setSubject(subject).build()) + .build()) + .setSong( + Song.newBuilder() + .setSongName(songName) + .setSongArtist( + Artist.newBuilder() + .setRecordingCompany( + RecordingCompany.newBuilder() + .setCompanyName(companyName) + .build()) + .setArtistName(artistName) + .build()) + .build()) + .build()) .build(); return createUser(request); } diff --git a/src/test/java/com/google/api/generator/gapic/testdata/identity.proto b/src/test/java/com/google/api/generator/gapic/testdata/identity.proto index 5ed47e8c59..07d27b2ef3 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/identity.proto +++ b/src/test/java/com/google/api/generator/gapic/testdata/identity.proto @@ -46,6 +46,9 @@ service Identity { "parent,user.display_name,user.email"; option (google.api.method_signature) = "parent,user.display_name,user.email,user.age,user.nickname,user.enable_notifications,user.height_feet"; + // Test nested method arguments and ensure that order doesn't matter. + option (google.api.method_signature) = + "parent,user.display_name,user.email,user.hobby.hobby_name,user.song.song_name,user.hobby.weekly_frequency,user.song.song_artist.recording_company.company_name,user.hobby.instruction_manual.title,user.hobby.instruction_manual.subject,user.song.song_artist.artist_name"; } // Retrieves the User with the given uri. @@ -121,6 +124,55 @@ message User { // (-- aip.dev/not-precedent: The default for the feature is true. // Ordinarily, the default for a `bool` field should be false. --) optional bool enable_notifications = 9; + + // The user's favorite hobby. + optional Hobby hobby = 10; + + // The user's song. + optional Song song = 11; +} + +message Hobby { + // The name of the hobby. + string hobby_name = 1; + + // Weekly frequency this hobby is performed. + int32 weekly_frequency = 2; + + // The instruction manual for the hobby. + Book instruction_manual = 3; +} + +message Book { + // The title of the instruction manual. + string title = 1; + + // The subject of the instruction manual. + string subject = 2; +} + +message Song { + // The name of the song. + string song_name = 1; + + // The song's artist. + Artist song_artist = 2; +} + +message Artist { + // The name of the artist. + string artist_name = 1; + + // The artist's recording company. + RecordingCompany recording_company = 2; +} + +message RecordingCompany { + // The recording company's name + string company_name = 1; + + // The year the company was founded. + int32 founding_year = 2; } // The request message for the google.showcase.v1beta1.Identity\CreateUser