From 1d7dd928da670ba11c644aefac61e2dfc44b6f20 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 5 Dec 2022 16:48:56 +0000 Subject: [PATCH 1/5] feat: spring service settings as its own bean --- .../SpringAutoConfigClassComposer.java | 76 ++++++++++++++----- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java index 3d32292145..e79ff03eed 100644 --- a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java +++ b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java @@ -125,16 +125,15 @@ public GapicClass generate(GapicContext context, Service service) { service, className, credentialsProviderName, dynamicTypes, thisExpr), createTransportChannelProviderBeanMethod( transportChannelProviderName, dynamicTypes), - createClientBeanMethod( + createServiceSettingsBeanMethod( service, - className, credentialsProviderName, transportChannelProviderName, - clientName, dynamicTypes, gapicServiceConfig, thisExpr, hasRestOption), + createClientBeanMethod(dynamicTypes, service), createUserAgentHeaderProviderMethod( serviceNameLowerHyphen, className, dynamicTypes, thisExpr))) .build(); @@ -530,12 +529,10 @@ private static Statement setRetrySettingsForMethod( return ExprStatement.withExpr(clientSettingBuilderChain); } - private static MethodDefinition createClientBeanMethod( + private static MethodDefinition createServiceSettingsBeanMethod( Service service, - String className, String credentialsProviderName, String transportChannelProviderName, - String clientName, Map types, GapicServiceConfig gapicServiceConfig, Expr thisExpr, @@ -849,21 +846,12 @@ private static MethodDefinition createClientBeanMethod( bodyStatements.addAll(retrySettings); // return expressions - MethodInvocationExpr serviceSettingsBuilt = + MethodInvocationExpr returnExpr = MethodInvocationExpr.builder() .setMethodName("build") .setExprReferenceExpr(settingsVarExpr.toBuilder().setIsDecl(false).build()) .setReturnType(types.get("ServiceSettings")) .build(); - MethodInvocationExpr returnExpr = - MethodInvocationExpr.builder() - // read more in client composer: - // src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java#L277-L292 - .setMethodName("create") - .setStaticReferenceType(types.get("ServiceClient")) - .setReturnType(types.get("ServiceClient")) - .setArguments(serviceSettingsBuilt) - .build(); List argumentsVariableExprs = Arrays.asList( credentialsProviderVariableExpr @@ -888,7 +876,23 @@ private static MethodDefinition createClientBeanMethod( .build()); String methodName = - CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Client"; + CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings"; + String conditionalOnMissingBeanNameArgument = methodName; + + // @ConditionalOnMissingBean(name = "[service]Settings") + AnnotationNode conditionalOnMissingBeanAnnotation = + AnnotationNode.builder() + .setType(STATIC_TYPES.get("ConditionalOnMissingBean")) + .addDescription( + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.withVariable( + Variable.builder().setName("name").setType(TypeNode.STRING).build())) + .setValueExpr( + ValueExpr.withValue( + StringObjectValue.withValue(conditionalOnMissingBeanNameArgument))) + .build()) + .build(); return MethodDefinition.builder() .setHeaderCommentStatements( @@ -898,6 +902,43 @@ private static MethodDefinition createClientBeanMethod( transportChannelProviderName)) .setName(methodName) .setScope(ScopeNode.PUBLIC) + .setReturnType(types.get("ServiceSettings")) + .setArguments(argumentsVariableExprs) + .setAnnotations( + Arrays.asList( + AnnotationNode.withType(STATIC_TYPES.get("Bean")), + conditionalOnMissingBeanAnnotation)) + .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) + .setReturnExpr(returnExpr) + .setBody(bodyStatements) + .build(); + } + + private static MethodDefinition createClientBeanMethod( + Map types, Service service) { + VariableExpr clientSettingsVariableExpr = + VariableExpr.withVariable( + Variable.builder() + .setName( + CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings") + .setType(types.get("ServiceSettings")) + .build()); + MethodInvocationExpr returnExpr = + MethodInvocationExpr.builder() + // read more in client composer: + // src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java#L277-L292 + .setMethodName("create") + .setStaticReferenceType(types.get("ServiceClient")) + .setReturnType(types.get("ServiceClient")) + .setArguments(clientSettingsVariableExpr) + .build(); + List argumentsVariableExprs = + Arrays.asList(clientSettingsVariableExpr.toBuilder().setIsDecl(true).build()); + String methodName = + CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Client"; + return MethodDefinition.builder() + .setName(methodName) + .setScope(ScopeNode.PUBLIC) .setReturnType(types.get("ServiceClient")) .setArguments(argumentsVariableExprs) .setAnnotations( @@ -906,7 +947,6 @@ private static MethodDefinition createClientBeanMethod( AnnotationNode.withType(STATIC_TYPES.get("ConditionalOnMissingBean")))) .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) .setReturnExpr(returnExpr) - .setBody(bodyStatements) .build(); } From 7c6d0246e1f561c70782a925dd6ca3f462d82d89 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 5 Dec 2022 16:53:26 +0000 Subject: [PATCH 2/5] chore: update goldens --- .../goldens/EchoSpringAutoConfigurationFull.golden | 12 +++++++++--- .../goldens/EchoSpringAutoConfigurationGrpc.golden | 12 +++++++++--- .../EchoSpringAutoConfigurationGrpcRest.golden | 12 +++++++++--- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden index ff75f66afb..92d5feb240 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden @@ -114,8 +114,8 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean - public EchoClient echoClient( + @ConditionalOnMissingBean(name = "echoSettings") + public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") TransportChannelProvider defaultTransportChannelProvider) @@ -576,7 +576,13 @@ public class EchoSpringAutoConfiguration { clientSettingsBuilder .collideNameSettings() .setRetrySettings(collideNameRetrySettingBuilder.build()); - return EchoClient.create(clientSettingsBuilder.build()); + return clientSettingsBuilder.build(); + } + + @Bean + @ConditionalOnMissingBean + public EchoClient echoClient(EchoSettings echoSettings) throws IOException { + return EchoClient.create(echoSettings); } private HeaderProvider userAgentHeaderProvider() { diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden index 1c34352e5a..a752e857b7 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden @@ -94,8 +94,8 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean - public EchoClient echoClient( + @ConditionalOnMissingBean(name = "echoSettings") + public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") TransportChannelProvider defaultTransportChannelProvider) @@ -556,7 +556,13 @@ public class EchoSpringAutoConfiguration { clientSettingsBuilder .collideNameSettings() .setRetrySettings(collideNameRetrySettingBuilder.build()); - return EchoClient.create(clientSettingsBuilder.build()); + return clientSettingsBuilder.build(); + } + + @Bean + @ConditionalOnMissingBean + public EchoClient echoClient(EchoSettings echoSettings) throws IOException { + return EchoClient.create(echoSettings); } private HeaderProvider userAgentHeaderProvider() { diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden index 00fccb6b39..dac9651db8 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden @@ -95,8 +95,8 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean - public EchoClient echoClient( + @ConditionalOnMissingBean(name = "echoSettings") + public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") TransportChannelProvider defaultTransportChannelProvider) @@ -564,7 +564,13 @@ public class EchoSpringAutoConfiguration { clientSettingsBuilder .collideNameSettings() .setRetrySettings(collideNameRetrySettingBuilder.build()); - return EchoClient.create(clientSettingsBuilder.build()); + return clientSettingsBuilder.build(); + } + + @Bean + @ConditionalOnMissingBean + public EchoClient echoClient(EchoSettings echoSettings) throws IOException { + return EchoClient.create(echoSettings); } private HeaderProvider userAgentHeaderProvider() { From 0a19d81dae9bf11efb6d8291d1b9e945e2f2f29e Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 5 Dec 2022 17:01:06 +0000 Subject: [PATCH 3/5] fix: rename settings creation method --- .../spring/composer/SpringAutoConfigClassComposer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java index e79ff03eed..d51db6a60d 100644 --- a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java +++ b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java @@ -125,7 +125,7 @@ public GapicClass generate(GapicContext context, Service service) { service, className, credentialsProviderName, dynamicTypes, thisExpr), createTransportChannelProviderBeanMethod( transportChannelProviderName, dynamicTypes), - createServiceSettingsBeanMethod( + createSettingsBeanMethod( service, credentialsProviderName, transportChannelProviderName, @@ -529,7 +529,7 @@ private static Statement setRetrySettingsForMethod( return ExprStatement.withExpr(clientSettingBuilderChain); } - private static MethodDefinition createServiceSettingsBeanMethod( + private static MethodDefinition createSettingsBeanMethod( Service service, String credentialsProviderName, String transportChannelProviderName, From e15238b3a142f1ba6071800f8fa71bca091ae280 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Mon, 5 Dec 2022 18:56:48 +0000 Subject: [PATCH 4/5] fix: remove name parameter from conditional bean annotation --- .../SpringAutoConfigClassComposer.java | 18 +----------------- .../EchoSpringAutoConfigurationFull.golden | 2 +- .../EchoSpringAutoConfigurationGrpc.golden | 2 +- .../EchoSpringAutoConfigurationGrpcRest.golden | 2 +- 4 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java index d51db6a60d..841955db29 100644 --- a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java +++ b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java @@ -877,22 +877,6 @@ private static MethodDefinition createSettingsBeanMethod( String methodName = CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings"; - String conditionalOnMissingBeanNameArgument = methodName; - - // @ConditionalOnMissingBean(name = "[service]Settings") - AnnotationNode conditionalOnMissingBeanAnnotation = - AnnotationNode.builder() - .setType(STATIC_TYPES.get("ConditionalOnMissingBean")) - .addDescription( - AssignmentExpr.builder() - .setVariableExpr( - VariableExpr.withVariable( - Variable.builder().setName("name").setType(TypeNode.STRING).build())) - .setValueExpr( - ValueExpr.withValue( - StringObjectValue.withValue(conditionalOnMissingBeanNameArgument))) - .build()) - .build(); return MethodDefinition.builder() .setHeaderCommentStatements( @@ -907,7 +891,7 @@ private static MethodDefinition createSettingsBeanMethod( .setAnnotations( Arrays.asList( AnnotationNode.withType(STATIC_TYPES.get("Bean")), - conditionalOnMissingBeanAnnotation)) + AnnotationNode.withType(STATIC_TYPES.get("ConditionalOnMissingBean")))) .setThrowsExceptions(Arrays.asList(TypeNode.withExceptionClazz(IOException.class))) .setReturnExpr(returnExpr) .setBody(bodyStatements) diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden index 92d5feb240..5f0dc4e559 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationFull.golden @@ -114,7 +114,7 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean(name = "echoSettings") + @ConditionalOnMissingBean public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden index a752e857b7..5b0a54395b 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpc.golden @@ -94,7 +94,7 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean(name = "echoSettings") + @ConditionalOnMissingBean public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") diff --git a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden index dac9651db8..e432d7220e 100644 --- a/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden +++ b/src/test/java/com/google/api/generator/spring/composer/goldens/EchoSpringAutoConfigurationGrpcRest.golden @@ -95,7 +95,7 @@ public class EchoSpringAutoConfiguration { * default retry settings when they are not specified in EchoSpringProperties. */ @Bean - @ConditionalOnMissingBean(name = "echoSettings") + @ConditionalOnMissingBean public EchoSettings echoSettings( @Qualifier("echoCredentials") CredentialsProvider credentialsProvider, @Qualifier("defaultEchoTransportChannelProvider") From 33d3070d535e93421a7b6fae1f5cbbd445cc2dfc Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Sun, 11 Dec 2022 21:55:02 +0000 Subject: [PATCH 5/5] fix: common service settings method name and unnecessary comment --- .../SpringAutoConfigClassComposer.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java index 841955db29..6f89728788 100644 --- a/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java +++ b/src/main/java/com/google/api/generator/spring/composer/SpringAutoConfigClassComposer.java @@ -107,6 +107,8 @@ public GapicClass generate(GapicContext context, Service service) { Expr thisExpr = ValueExpr.withValue(ThisObjectValue.withType(dynamicTypes.get(className))); Transport transport = context.transport(); boolean hasRestOption = transport.equals(Transport.GRPC_REST); + String serviceSettingsMethodName = + CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings"; ClassDefinition classDef = ClassDefinition.builder() @@ -132,8 +134,9 @@ public GapicClass generate(GapicContext context, Service service) { dynamicTypes, gapicServiceConfig, thisExpr, - hasRestOption), - createClientBeanMethod(dynamicTypes, service), + hasRestOption, + serviceSettingsMethodName), + createClientBeanMethod(dynamicTypes, service, serviceSettingsMethodName), createUserAgentHeaderProviderMethod( serviceNameLowerHyphen, className, dynamicTypes, thisExpr))) .build(); @@ -536,7 +539,8 @@ private static MethodDefinition createSettingsBeanMethod( Map types, GapicServiceConfig gapicServiceConfig, Expr thisExpr, - boolean hasRestOption) { + boolean hasRestOption, + String serviceSettingsMethodName) { // argument variables: VariableExpr credentialsProviderVariableExpr = VariableExpr.withVariable( @@ -875,16 +879,13 @@ private static MethodDefinition createSettingsBeanMethod( .build())) .build()); - String methodName = - CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings"; - return MethodDefinition.builder() .setHeaderCommentStatements( SpringAutoconfigCommentComposer.createClientBeanComment( service.name(), Utils.getServicePropertiesClassName(service), transportChannelProviderName)) - .setName(methodName) + .setName(serviceSettingsMethodName) .setScope(ScopeNode.PUBLIC) .setReturnType(types.get("ServiceSettings")) .setArguments(argumentsVariableExprs) @@ -899,18 +900,15 @@ private static MethodDefinition createSettingsBeanMethod( } private static MethodDefinition createClientBeanMethod( - Map types, Service service) { + Map types, Service service, String serviceSettingsMethodName) { VariableExpr clientSettingsVariableExpr = VariableExpr.withVariable( Variable.builder() - .setName( - CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL, service.name()) + "Settings") + .setName(serviceSettingsMethodName) .setType(types.get("ServiceSettings")) .build()); MethodInvocationExpr returnExpr = MethodInvocationExpr.builder() - // read more in client composer: - // src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java#L277-L292 .setMethodName("create") .setStaticReferenceType(types.get("ServiceClient")) .setReturnType(types.get("ServiceClient"))