Skip to content

Commit

Permalink
fix: [gapic-generator-java] align writer behavior for nested types (#…
Browse files Browse the repository at this point in the history
…1709)

* Removes duplicate handling of enclosing classes in JavaWriterVisitor and ImportWriterVisitor - change to only import outermost class, e.g. com.example.Outer), while type references will be written as Outer.Middle.Inner
* Fixes issue in ConcreteReference where multiple levels of enclosed classes are written in reversed order
  • Loading branch information
emmileaf authored Jun 5, 2023
1 parent dea8426 commit a21ffe8
Show file tree
Hide file tree
Showing 7 changed files with 340 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,15 @@ public ImmutableList<String> enclosingClassNames() {
if (!hasEnclosingClass()) {
return ImmutableList.of();
}
// The innermost type will be the last element in the list.
// Builds list in order of inner to outer.
// Return the reversed list, since the outermost type is expected to lie at index 0.
ImmutableList.Builder<String> listBuilder = new ImmutableList.Builder<>();
Class<?> currentClz = clazz();
while (currentClz.getEnclosingClass() != null) {
listBuilder.add(currentClz.getEnclosingClass().getSimpleName());
currentClz = currentClz.getEnclosingClass();
}
return listBuilder.build();
return listBuilder.build().reverse();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,9 +508,11 @@ private void handleReference(Reference reference) {
staticImports.add(reference.fullName());
} else {
if (reference.hasEnclosingClass()) {
// Only import outermost enclosing class, e.g. import com.foo.bar.Outer
addImport(
String.format(
"%s.%s", reference.pakkage(), String.join(DOT, reference.enclosingClassNames())));
"%s.%s",
reference.pakkage(), String.join(DOT, reference.enclosingClassNames().get(0))));
} else {
addImport(reference.fullName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import com.google.api.generator.test.utils.LineFormatter;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.testgapic.v1beta1.Outer;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -428,6 +429,27 @@ public void writeVariableExprImports_annotationsWithDescription() {
writerVisitor.write());
}

@Test
public void writeVaporReferenceImport_outermostForNestedClass() {
VaporReference nestedVaporReference =
VaporReference.builder()
.setName("Inner")
.setEnclosingClassNames(Arrays.asList("Outer", "Middle"))
.setPakkage("com.google.testgapic.v1beta1")
.build();

TypeNode.withReference(nestedVaporReference).accept(writerVisitor);
assertEquals("import com.google.testgapic.v1beta1.Outer;\n\n", writerVisitor.write());
}

@Test
public void writeConcreteReferenceImport_outermostForNestedClass() {
ConcreteReference nestedConcreteReference =
ConcreteReference.withClazz(Outer.Middle.Inner.class);
TypeNode.withReference(nestedConcreteReference).accept(writerVisitor);
assertEquals("import com.google.testgapic.v1beta1.Outer;\n\n", writerVisitor.write());
}

@Test
public void writeAnonymousClassExprImports() {
// [Constructing] Function<List<IOException>, MethodDefinition>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,16 @@
import com.google.api.generator.engine.ast.Variable;
import com.google.api.generator.engine.ast.VariableExpr;
import com.google.api.generator.engine.ast.WhileStatement;
import com.google.api.generator.gapic.composer.grpc.ServiceClientClassComposer;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicContext;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.test.framework.Assert;
import com.google.api.generator.test.protoloader.TestProtoLoader;
import com.google.api.generator.test.utils.LineFormatter;
import com.google.api.generator.test.utils.TestExprBuilder;
import com.google.common.base.Function;
import com.google.testgapic.v1beta1.Outer;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -129,6 +136,26 @@ public void writeReferenceType_basic() {
assertEquals("FooBar", writerVisitor.write());
}

@Test
public void writeVaporReferenceType_nestedClasses() {
VaporReference nestedVaporReference =
VaporReference.builder()
.setName("Inner")
.setEnclosingClassNames(Arrays.asList("Outer", "Middle"))
.setPakkage("com.google.testgapic.v1beta1")
.build();
TypeNode.withReference(nestedVaporReference).accept(writerVisitor);
assertEquals("Outer.Middle.Inner", writerVisitor.write());
}

@Test
public void writeConcreteReferenceType_nestedClasses() {
ConcreteReference nestedConcreteReference =
ConcreteReference.withClazz(Outer.Middle.Inner.class);
TypeNode.withReference(nestedConcreteReference).accept(writerVisitor);
assertEquals("Outer.Middle.Inner", writerVisitor.write());
}

@Test
public void writeReferenceType_useFullName() {
TypeNode.withReference(
Expand Down Expand Up @@ -2796,6 +2823,17 @@ public void writePackageInfoDefinition() {
writerVisitor.write());
}

/** =============================== GOLDEN TESTS =============================== */
@Test
public void writeSGrpcServiceClientWithNestedClassImport() {
GapicContext context = TestProtoLoader.instance().parseNestedMessage();
Service nestedService = context.services().get(0);
GapicClass clazz = ServiceClientClassComposer.instance().generate(context, nestedService);

Assert.assertGoldenClass(
this.getClass(), clazz, "GrpcServiceClientWithNestedClassImport.golden");
}

/** =============================== HELPERS =============================== */
private static AssignmentExpr createAssignmentExpr(
String variableName, String value, TypeNode type) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
package com.google.types.testing;

import com.google.api.gax.core.BackgroundResource;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.testgapic.v1beta1.Outer;
import com.google.types.testing.stub.NestedMessageServiceStub;
import com.google.types.testing.stub.NestedMessageServiceStubSettings;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import javax.annotation.Generated;

// AUTO-GENERATED DOCUMENTATION AND CLASS.
/**
* This class provides the ability to make remote calls to the backing service through method calls
* that map to API methods. Sample code to get started:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
* // It will require modifications to work:
* // - It may require correct/in-range values for request initialization.
* // - It may require specifying regional endpoints when creating the service client as shown in
* // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
* try (NestedMessageServiceClient nestedMessageServiceClient =
* NestedMessageServiceClient.create()) {
* Outer.Middle request = Outer.Middle.newBuilder().setX(120).build();
* Outer.Middle.Inner response = nestedMessageServiceClient.nestedMessageMethod(request);
* }
* }</pre>
*
* <p>Note: close() needs to be called on the NestedMessageServiceClient object to clean up
* resources such as threads. In the example above, try-with-resources is used, which automatically
* calls close().
*
* <p>The surface of this class includes several types of Java methods for each of the API's
* methods:
*
* <ol>
* <li>A "flattened" method. With this type of method, the fields of the request type have been
* converted into function parameters. It may be the case that not all fields are available as
* parameters, and not every API method will have a flattened method entry point.
* <li>A "request object" method. This type of method only takes one parameter, a request object,
* which must be constructed before the call. Not every API method will have a request object
* method.
* <li>A "callable" method. This type of method takes no parameters and returns an immutable API
* callable object, which can be used to initiate calls to the service.
* </ol>
*
* <p>See the individual methods for example code.
*
* <p>Many parameters require resource names to be formatted in a particular way. To assist with
* these names, this class includes a format method for each type of name, and additionally a parse
* method to extract the individual identifiers contained within names that are returned.
*
* <p>This class can be customized by passing in a custom instance of NestedMessageServiceSettings
* to create(). For example:
*
* <p>To customize credentials:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
* // It will require modifications to work:
* // - It may require correct/in-range values for request initialization.
* // - It may require specifying regional endpoints when creating the service client as shown in
* // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
* NestedMessageServiceSettings nestedMessageServiceSettings =
* NestedMessageServiceSettings.newBuilder()
* .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
* .build();
* NestedMessageServiceClient nestedMessageServiceClient =
* NestedMessageServiceClient.create(nestedMessageServiceSettings);
* }</pre>
*
* <p>To customize the endpoint:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
* // It will require modifications to work:
* // - It may require correct/in-range values for request initialization.
* // - It may require specifying regional endpoints when creating the service client as shown in
* // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
* NestedMessageServiceSettings nestedMessageServiceSettings =
* NestedMessageServiceSettings.newBuilder().setEndpoint(myEndpoint).build();
* NestedMessageServiceClient nestedMessageServiceClient =
* NestedMessageServiceClient.create(nestedMessageServiceSettings);
* }</pre>
*
* <p>Please refer to the GitHub repository's samples for more quickstart code snippets.
*/
@Generated("by gapic-generator-java")
public class NestedMessageServiceClient implements BackgroundResource {
private final NestedMessageServiceSettings settings;
private final NestedMessageServiceStub stub;

/** Constructs an instance of NestedMessageServiceClient with default settings. */
public static final NestedMessageServiceClient create() throws IOException {
return create(NestedMessageServiceSettings.newBuilder().build());
}

/**
* Constructs an instance of NestedMessageServiceClient, using the given settings. The channels
* are created based on the settings passed in, or defaults for any settings that are not set.
*/
public static final NestedMessageServiceClient create(NestedMessageServiceSettings settings)
throws IOException {
return new NestedMessageServiceClient(settings);
}

/**
* Constructs an instance of NestedMessageServiceClient, using the given stub for making calls.
* This is for advanced usage - prefer using create(NestedMessageServiceSettings).
*/
public static final NestedMessageServiceClient create(NestedMessageServiceStub stub) {
return new NestedMessageServiceClient(stub);
}

/**
* Constructs an instance of NestedMessageServiceClient, using the given settings. This is
* protected so that it is easy to make a subclass, but otherwise, the static factory methods
* should be preferred.
*/
protected NestedMessageServiceClient(NestedMessageServiceSettings settings) throws IOException {
this.settings = settings;
this.stub = ((NestedMessageServiceStubSettings) settings.getStubSettings()).createStub();
}

protected NestedMessageServiceClient(NestedMessageServiceStub stub) {
this.settings = null;
this.stub = stub;
}

public final NestedMessageServiceSettings getSettings() {
return settings;
}

public NestedMessageServiceStub getStub() {
return stub;
}

// AUTO-GENERATED DOCUMENTATION AND METHOD.
/**
* Sample code:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
* // It will require modifications to work:
* // - It may require correct/in-range values for request initialization.
* // - It may require specifying regional endpoints when creating the service client as shown in
* // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
* try (NestedMessageServiceClient nestedMessageServiceClient =
* NestedMessageServiceClient.create()) {
* Outer.Middle request = Outer.Middle.newBuilder().setX(120).build();
* Outer.Middle.Inner response = nestedMessageServiceClient.nestedMessageMethod(request);
* }
* }</pre>
*
* @param request The request object containing all of the parameters for the API call.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Outer.Middle.Inner nestedMessageMethod(Outer.Middle request) {
return nestedMessageMethodCallable().call(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD.
/**
* Sample code:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
* // It will require modifications to work:
* // - It may require correct/in-range values for request initialization.
* // - It may require specifying regional endpoints when creating the service client as shown in
* // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
* try (NestedMessageServiceClient nestedMessageServiceClient =
* NestedMessageServiceClient.create()) {
* Outer.Middle request = Outer.Middle.newBuilder().setX(120).build();
* ApiFuture<Outer.Middle.Inner> future =
* nestedMessageServiceClient.nestedMessageMethodCallable().futureCall(request);
* // Do something.
* Outer.Middle.Inner response = future.get();
* }
* }</pre>
*/
public final UnaryCallable<Outer.Middle, Outer.Middle.Inner> nestedMessageMethodCallable() {
return stub.nestedMessageMethodCallable();
}

@Override
public final void close() {
stub.close();
}

@Override
public void shutdown() {
stub.shutdown();
}

@Override
public boolean isShutdown() {
return stub.isShutdown();
}

@Override
public boolean isTerminated() {
return stub.isTerminated();
}

@Override
public void shutdownNow() {
stub.shutdownNow();
}

@Override
public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
return stub.awaitTermination(duration, unit);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import com.google.showcase.v1beta1.MessagingOuterClass;
import com.google.showcase.v1beta1.TestingOuterClass;
import com.google.testdata.v1.DeprecatedServiceOuterClass;
import com.google.testgapic.v1beta1.NestedMessageProto;
import com.google.types.testing.TypesTestingProto;
import google.cloud.CommonResources;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -131,6 +133,30 @@ public GapicContext parseBookshopService() {
.build();
}

public GapicContext parseNestedMessage() {
FileDescriptor fileDescriptor = TypesTestingProto.getDescriptor();
ServiceDescriptor serviceDescriptor = fileDescriptor.getServices().get(0);
assertEquals(serviceDescriptor.getName(), "NestedMessageService");
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);

FileDescriptor messageFileDescriptor = NestedMessageProto.getDescriptor();
messageTypes.putAll(Parser.parseMessages(messageFileDescriptor));

Map<String, ResourceName> resourceNames = new HashMap<>();
Set<ResourceName> outputResourceNames = new HashSet<>();
List<Service> services =
Parser.parseService(
fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames);

return GapicContext.builder()
.setMessages(messageTypes)
.setResourceNames(resourceNames)
.setServices(services)
.setHelperResourceNames(outputResourceNames)
.setTransport(transport)
.build();
}

public GapicContext parseShowcaseEcho() {
FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor();
ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0);
Expand Down
Loading

0 comments on commit a21ffe8

Please sign in to comment.