Skip to content

fix(Spring CodeGen): Update missing and placeholder annotations #1045

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.

Sign up for GitHub

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

Merged
emmileaf merged 5 commits into from
Oct 4, 2022
Merged

fix(Spring CodeGen): Update missing and placeholder annotations #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62cd638
Update annotations to use classes and parameters instead of strings
emmileaf Sep 26, 2022
3efc657
Add qualifier annotations on variables
emmileaf Sep 28, 2022
93c1765
Reuse types that are already defined
emmileaf Sep 28, 2022
82699cb
Add member variable annotation to credentials property
emmileaf Sep 28, 2022
b7188f2
Merge branch 'autoconfig-gen-draft2' into autoconfig-gen-draft2-emmwang
emmileaf Oct 3, 2022
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
93 changes: 79 additions & 14 deletions src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
import com.google.api.generator.engine.ast.MethodDefinition;
import com.google.api.generator.engine.ast.MethodInvocationExpr;
import com.google.api.generator.engine.ast.NewObjectExpr;
import com.google.api.generator.engine.ast.PrimitiveValue;
import com.google.api.generator.engine.ast.RelationalOperationExpr;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.ast.Statement;
import com.google.api.generator.engine.ast.StringObjectValue;
import com.google.api.generator.engine.ast.ThisObjectValue;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.ValueExpr;
Expand Down Expand Up @@ -401,34 +403,68 @@ private static List<AnnotationNode> createClassAnnotations(
// @ConditionalOnProperty(value = "spring.cloud.gcp.language.enabled", matchIfMissing = true)
// @EnableConfigurationProperties(LanguageProperties.class)

// TODO: AnnotationNode description only accepts String for now. need to extend to params
// and classes.
AssignmentExpr valueStringAssignmentExpr =
AssignmentExpr.builder()
.setVariableExpr(
VariableExpr.withVariable(
Variable.builder().setName("value").setType(TypeNode.STRING).build()))
.setValueExpr(
ValueExpr.withValue(
StringObjectValue.withValue(
Utils.springPropertyPrefix(libName, service.name()) + ".enabled")))
.build();
AssignmentExpr matchIfMissingAssignmentExpr =
AssignmentExpr.builder()
.setVariableExpr(
VariableExpr.withVariable(
Variable.builder().setName("matchIfMissing").setType(TypeNode.BOOLEAN).build()))
.setValueExpr(
ValueExpr.withValue(
PrimitiveValue.builder().setValue("false").setType(TypeNode.BOOLEAN).build()))
.build();
AnnotationNode conditionalOnPropertyNode =
AnnotationNode.builder()
.setType(types.get("ConditionalOnProperty"))
.setDescription(
"value = \""
+ Utils.springPropertyPrefix(libName, service.name())
+ ".enabled\", matchIfMissing = false")
.addDescription(valueStringAssignmentExpr)
.addDescription(matchIfMissingAssignmentExpr)
.build();

AnnotationNode conditionalOnClassNode =
AnnotationNode.builder()
.setType(types.get("ConditionalOnClass"))
.setDescription(
"value = "
+ ClassNames.getServiceClientClassName(service)
+ ".class") // TODO: change after annotation feature merged. need to produce
// XXX.class
VariableExpr.builder()
.setVariable(
Variable.builder().setType(TypeNode.CLASS_OBJECT).setName("class").build())
.setStaticReferenceType(types.get("ServiceClient"))
.build())
.build();

AssignmentExpr proxyBeanMethodsAssignmentExpr =
AssignmentExpr.builder()
.setVariableExpr(
VariableExpr.withVariable(
Variable.builder()
.setName("proxyBeanMethods")
.setType(TypeNode.BOOLEAN)
.build()))
.setValueExpr(
ValueExpr.withValue(
PrimitiveValue.builder().setValue("false").setType(TypeNode.BOOLEAN).build()))
.build();
AnnotationNode configurationNode =
AnnotationNode.builder().setType(types.get("AutoConfiguration")).build();
AnnotationNode enableConfigurationPropertiesNode =
AnnotationNode.builder()
.setType(types.get("EnableConfigurationProperties"))
.setDescription(
types.get(service.name() + "Properties").reference().name()
+ ".Class") // TODO: change to parameters
VariableExpr.builder()
.setVariable(
Variable.builder().setType(TypeNode.CLASS_OBJECT).setName("class").build())
.setStaticReferenceType(types.get(service.name() + "Properties"))
.build())
.build();

return Arrays.asList(
AnnotationNode.builder()
.setType(STATIC_TYPES.get("Generated"))
Expand Down Expand Up @@ -893,14 +929,35 @@ private static MethodDefinition createClientBeanMethod(
String methodName =
CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Client";

String transportChannelProviderName = "default" + service.name() + "TransportChannelProvider";
String credentialsProviderName = "googleCredentials";

return MethodDefinition.builder()
.setName(methodName)
.setScope(ScopeNode.PUBLIC)
.setReturnType(types.get("ServiceClient"))
.setArguments(
Arrays.asList(
credentialsProviderVariableExpr.toBuilder().setIsDecl(true).build(),
transportChannelProviderVariableExpr.toBuilder().setIsDecl(true).build()))
credentialsProviderVariableExpr
.toBuilder()
.setIsDecl(true)
.setAnnotations(
Arrays.asList(
AnnotationNode.builder()
.setType(types.get("Qualifier"))
.setDescription(credentialsProviderName)
.build()))
.build(),
transportChannelProviderVariableExpr
.toBuilder()
.setIsDecl(true)
.setAnnotations(
Arrays.asList(
AnnotationNode.builder()
.setType(types.get("Qualifier"))
.setDescription(transportChannelProviderName)
.build()))
.build()))
.setAnnotations(
Arrays.asList(
AnnotationNode.withType(types.get("Bean")),
Expand Down Expand Up @@ -1049,6 +1106,13 @@ private static Map<String, TypeNode> createDynamicTypes(Service service, String
.setPakkage("org.springframework.boot.autoconfigure.condition")
.build());

TypeNode qualifier =
TypeNode.withReference(
VaporReference.builder()
.setName("Qualifier")
.setPakkage("org.springframework.beans.factory.annotation")
.build());

typeMap.put("CredentialsProvider", credentialsProvider);
typeMap.put(service.name() + "Properties", clientProperties);
typeMap.put(service.name() + "AutoConfig", clientAutoconfig);
Expand All @@ -1064,6 +1128,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service, String
typeMap.put("ConditionalOnMissingBean", conditionalOnMissingBean);
typeMap.put("ConditionalOnProperty", conditionalOnProperty);
typeMap.put("ConditionalOnClass", conditionalOnClass);
typeMap.put("Qualifier", qualifier);

return typeMap;
}
Expand Down
27 changes: 20 additions & 7 deletions src/main/java/com/google/api/generator/spring/composer/SpringPropertiesClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,17 @@ public GapicClass generate(GapicContext context, Service service) {
}

private static ExprStatement createMemberVarStatement(
String varName, TypeNode varType, boolean isFinal, Expr defaultVal) {
String varName,
TypeNode varType,
boolean isFinal,
Expr defaultVal,
List<AnnotationNode> annotationNodes) {
Variable memberVar = Variable.builder().setName(varName).setType(varType).build();
VariableExpr memberVarExpr =
VariableExpr.builder()
.setVariable(memberVar)
.setScope(ScopeNode.PRIVATE)
.setAnnotations(annotationNodes == null ? Collections.emptyList() : annotationNodes)
.setIsDecl(true)
.setIsFinal(isFinal)
.build();
Expand Down Expand Up @@ -133,22 +138,30 @@ private static List<Statement> createMemberVariables(
.map(x -> ValueExpr.withValue(StringObjectValue.withValue(x)))
.collect(Collectors.toList()))
.build();
// TODO: credentials field needs annotation.
// Note that the annotations are set on the VariableExpr rather than the ExprStatement.
// The single annotation works fine here,
// but multiple annotations would be written to the same line
List<AnnotationNode> credentialsAnnotations =
Arrays.asList(AnnotationNode.withType(types.get("NestedConfigurationProperty")));
ExprStatement credentialsStatement =
createMemberVarStatement(
"credentials", types.get("Credentials"), true, defaultCredentialScopes);
"credentials",
types.get("Credentials"),
true,
defaultCredentialScopes,
credentialsAnnotations);

// private String quotaProjectId;
ExprStatement quotaProjectIdVarStatement =
createMemberVarStatement("quotaProjectId", TypeNode.STRING, false, null);
createMemberVarStatement("quotaProjectId", TypeNode.STRING, false, null, null);

// private Integer executorThreadCount;
ExprStatement executorThreadCountVarStatement =
createMemberVarStatement("executorThreadCount", TypeNode.INT_OBJECT, false, null);
createMemberVarStatement("executorThreadCount", TypeNode.INT_OBJECT, false, null, null);

// private boolean useRest = false;
ExprStatement useRestVarStatement =
createMemberVarStatement("useRest", TypeNode.BOOLEAN, false, null);
createMemberVarStatement("useRest", TypeNode.BOOLEAN, false, null, null);

//
// private static final ImmutableMap<String, RetrySettings> RETRY_PARAM_DEFINITIONS;
Expand All @@ -170,7 +183,7 @@ private static List<Statement> createMemberVariables(
}
String propertyName = Joiner.on("").join(methodAndPropertyName);
ExprStatement retrySettingsStatement =
createMemberVarStatement(propertyName, propertyType, false, null);
createMemberVarStatement(propertyName, propertyType, false, null, null);
getterAndSetter.add(retrySettingsStatement);
return getterAndSetter;
},
Expand Down
14 changes: 9 additions & 5 deletions src/test/java/com/google/api/generator/spring/SpringAutoConfigClassComposerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,11 @@ public void generatePropertiesTest() {
+ "\n"
+ "@Generated(\"by gapic-generator-java\")\n"
+ "@AutoConfiguration\n"
+ "@ConditionalOnClass(\"value = EchoClient.class\")\n"
+ "@ConditionalOnClass(EchoClient.class)\n"
+ "@ConditionalOnProperty(\n"
+ " \"value = \\\"spring.cloud.gcp.autoconfig.showcase.echo.enabled\\\", matchIfMissing = false\")\n"
+ "@EnableConfigurationProperties(\"EchoSpringProperties.Class\")\n"
+ " value = \"spring.cloud.gcp.autoconfig.showcase.echo.enabled\",\n"
+ " matchIfMissing = false)\n"
+ "@EnableConfigurationProperties(EchoSpringProperties.class)\n"
+ "public class EchoSpringAutoConfiguration {\n"
+ " private final EchoSpringProperties clientProperties;\n"
+ " private static final ImmutableMap<String, RetrySettings> RETRY_PARAM_DEFINITIONS;\n"
Expand Down Expand Up @@ -189,8 +190,9 @@ public void generatePropertiesTest() {
+ " @Bean\n"
+ " @ConditionalOnMissingBean\n"
+ " public EchoClient echoClient(\n"
+ " CredentialsProvider credentialsProvider,\n"
+ " TransportChannelProvider defaultTransportChannelProvider)\n"
+ " @Qualifier(\"googleCredentials\") CredentialsProvider credentialsProvider,\n"
Copy link
Contributor

@zhumin8 zhumin8 Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally missed this: We want to add this @Qualifier so that when multiple services are present, there won't be conflict in which CredentialsProvider bean it takes. So need to rename googleCredentials() to [serviceName]Credentials() and match the name here. (same as defaultEchoTransportChannelProvider())

Copy link
Contributor Author

@emmileaf emmileaf Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this - will address in a separate PR!

+ " @Qualifier(\"defaultEchoTransportChannelProvider\")\n"
+ " TransportChannelProvider defaultTransportChannelProvider)\n"
+ " throws IOException {\n"
+ " EchoSettings.Builder clientSettingsBuilder =\n"
+ " EchoSettings.newBuilder()\n"
Expand Down Expand Up @@ -303,8 +305,10 @@ public void generatePropertiesTest() {
+ "\n"
+ "@ConfigurationProperties(\"spring.cloud.gcp.autoconfig.showcase.echo\")\n"
+ "public class EchoSpringProperties implements CredentialsSupplier {\n"
+ " @NestedConfigurationProperty\n"
+ " private final Credentials credentials =\n"
+ " new Credentials(\"https://www.googleapis.com/auth/cloud-platform\");\n"
+ "\n"
+ " private String quotaProjectId;\n"
+ " private Integer executorThreadCount;\n"
+ " private boolean useRest;\n"
Expand Down