-
Notifications
You must be signed in to change notification settings - Fork 53
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: [gapic-generator-java] align writer behavior for nested types #1709
Changes from all commits
f505036
101162a
94e8fc6
3024342
e4764b2
ca73ada
158e6a9
6ab1628
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be break into two test cases if we want to test both type of references. However, I think we only need to test one of them if the only change is in the writer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the additional change in |
||
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> | ||
|
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); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we achieve the same thing without this change? Seems we can try to get the last element from the list in the writer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes for the import writer, but the
JavaWriterVisitor
also relies on this ordering for the enclosing class writes.This fix to
ConcreteReference
wasn't a part of the original issue investigated, but it looked like an implementation bug that perhaps didn't surface in generated code anywhere yet. I think the intended result of thisenclosingClassNames
call should have outer to inner ordering, given this original comment and upstream usages of this method in the generator.