From bb8f2f2d59d121864b2ded15dbc74f71e0478bf0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 12:09:40 -0800 Subject: [PATCH 01/12] fix: align resname ctors with superclass variants --- .../ResourceNameHelperClassComposer.java | 32 +++++++++++++++---- .../goldens/BillingAccountLocationName.golden | 8 ++++- .../gapic/composer/goldens/FoobarName.golden | 16 +++++++--- .../gapic/composer/goldens/SessionName.golden | 7 +++- .../gapic/composer/goldens/TestName.golden | 9 +++++- test/integration/goldens/asset/FeedName.java | 14 ++++++-- .../logging/BillingAccountLocationName.java | 8 ++++- .../goldens/logging/BillingAccountName.java | 7 +++- .../goldens/logging/CmekSettingsName.java | 16 +++++++--- .../goldens/logging/FolderLocationName.java | 8 ++++- .../goldens/logging/FolderName.java | 7 +++- .../goldens/logging/LocationName.java | 8 ++++- .../goldens/logging/LogBucketName.java | 18 ++++++++--- .../goldens/logging/LogExclusionName.java | 17 +++++++--- .../goldens/logging/LogMetricName.java | 8 ++++- test/integration/goldens/logging/LogName.java | 17 +++++++--- .../goldens/logging/LogSinkName.java | 17 +++++++--- .../logging/OrganizationLocationName.java | 8 ++++- .../goldens/logging/OrganizationName.java | 7 +++- .../goldens/logging/ProjectName.java | 7 +++- .../goldens/redis/InstanceName.java | 9 +++++- .../goldens/redis/LocationName.java | 8 ++++- 22 files changed, 208 insertions(+), 48 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java index 00c8f03c34..7469af4ec2 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -289,6 +289,29 @@ private static List createConstructorMethods( boolean hasVariants = tokenHierarchies.size() > 1; List javaMethods = new ArrayList<>(); + final ValueExpr nullExpr = ValueExpr.withValue(NullObjectValue.create()); + Function assignTokenToNullExpr = + t -> + AssignmentExpr.builder() + .setVariableExpr(patternTokenVarExprs.get(t)) + .setValueExpr(nullExpr) + .build(); + + // First deprecated constructor. + javaMethods.add( + MethodDefinition.constructorBuilder() + .setAnnotations( + Arrays.asList( + AnnotationNode.withType( + TypeNode.withReference(ConcreteReference.withClazz(Deprecated.class))))) + .setScope(ScopeNode.PROTECTED) + .setReturnType(thisClassType) + .setBody( + getTokenSet(tokenHierarchies).stream() + .map(t -> ExprStatement.withExpr(assignTokenToNullExpr.apply(t))) + .collect(Collectors.toList())) + .build()); + for (int i = 0; i < tokenHierarchies.size(); i++) { List tokens = tokenHierarchies.get(i); List bodyExprs = new ArrayList<>(); @@ -316,16 +339,11 @@ private static List createConstructorMethods( .build()); } // Initialize the rest to null. - ValueExpr nullExpr = ValueExpr.withValue(NullObjectValue.create()); for (String token : getTokenSet(tokenHierarchies)) { if (tokens.contains(token)) { continue; } - bodyExprs.add( - AssignmentExpr.builder() - .setVariableExpr(patternTokenVarExprs.get(token)) - .setValueExpr(nullExpr) - .build()); + bodyExprs.add(assignTokenToNullExpr.apply(token)); } if (hasVariants) { @@ -1389,7 +1407,7 @@ private static ClassDefinition createNestedBuilderClass( TypeNode thisClassType = types.get(className); MethodDefinition ctor = MethodDefinition.constructorBuilder() - .setScope(ScopeNode.PRIVATE) + .setScope(ScopeNode.PROTECTED) .setReturnType(thisClassType) .build(); nestedClassMethods.add(ctor); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/BillingAccountLocationName.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/BillingAccountLocationName.golden index 94fd7c738d..7500be4b9d 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/BillingAccountLocationName.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/BillingAccountLocationName.golden @@ -20,6 +20,12 @@ public class BillingAccountLocationName implements ResourceName { private final String billingAccount; private final String location; + @Deprecated + protected BillingAccountLocationName() { + billingAccount = null; + location = null; + } + private BillingAccountLocationName(Builder builder) { billingAccount = Preconditions.checkNotNull(builder.getBillingAccount()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -141,7 +147,7 @@ public class BillingAccountLocationName implements ResourceName { private String billingAccount; private String location; - private Builder() {} + protected Builder() {} public String getBillingAccount() { return billingAccount; diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/FoobarName.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/FoobarName.golden index 7f693a446a..9823f297c5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/FoobarName.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/FoobarName.golden @@ -32,6 +32,14 @@ public class FoobarName implements ResourceName { private final String variant; private final String barFoo; + @Deprecated + protected FoobarName() { + project = null; + foobar = null; + variant = null; + barFoo = null; + } + private FoobarName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); foobar = Preconditions.checkNotNull(builder.getFoobar()); @@ -285,7 +293,7 @@ public class FoobarName implements ResourceName { private String project; private String foobar; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -325,7 +333,7 @@ public class FoobarName implements ResourceName { private String variant; private String foobar; - private ProjectVariantFoobarBuilder() {} + protected ProjectVariantFoobarBuilder() {} public String getProject() { return project; @@ -364,7 +372,7 @@ public class FoobarName implements ResourceName { public static class FoobarBuilder { private String foobar; - private FoobarBuilder() {} + protected FoobarBuilder() {} public String getFoobar() { return foobar; @@ -386,7 +394,7 @@ public class FoobarName implements ResourceName { private String barFoo; private String foobar; - private BarFooFoobarBuilder() {} + protected BarFooFoobarBuilder() {} public String getBarFoo() { return barFoo; diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/SessionName.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/SessionName.golden index b17c2718c1..e0ef56bae0 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/SessionName.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/SessionName.golden @@ -18,6 +18,11 @@ public class SessionName implements ResourceName { private volatile Map fieldValuesMap; private final String session; + @Deprecated + protected SessionName() { + session = null; + } + private SessionName(Builder builder) { session = Preconditions.checkNotNull(builder.getSession()); } @@ -125,7 +130,7 @@ public class SessionName implements ResourceName { public static class Builder { private String session; - private Builder() {} + protected Builder() {} public String getSession() { return session; diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestName.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestName.golden index f4a72ec1ad..af723a66c3 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestName.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestName.golden @@ -20,6 +20,13 @@ public class TestName implements ResourceName { private final String shardId; private final String testId; + @Deprecated + protected TestName() { + session = null; + shardId = null; + testId = null; + } + private TestName(Builder builder) { session = Preconditions.checkNotNull(builder.getSession()); shardId = Preconditions.checkNotNull(builder.getShardId()); @@ -157,7 +164,7 @@ public class TestName implements ResourceName { private String shardId; private String testId; - private Builder() {} + protected Builder() {} public String getSession() { return session; diff --git a/test/integration/goldens/asset/FeedName.java b/test/integration/goldens/asset/FeedName.java index f05af8f2e6..3d76da97b4 100644 --- a/test/integration/goldens/asset/FeedName.java +++ b/test/integration/goldens/asset/FeedName.java @@ -45,6 +45,14 @@ public class FeedName implements ResourceName { private final String folder; private final String organization; + @Deprecated + protected FeedName() { + project = null; + feed = null; + folder = null; + organization = null; + } + private FeedName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); feed = Preconditions.checkNotNull(builder.getFeed()); @@ -263,7 +271,7 @@ public static class Builder { private String project; private String feed; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -302,7 +310,7 @@ public static class FolderFeedBuilder { private String folder; private String feed; - private FolderFeedBuilder() {} + protected FolderFeedBuilder() {} public String getFolder() { return folder; @@ -333,7 +341,7 @@ public static class OrganizationFeedBuilder { private String organization; private String feed; - private OrganizationFeedBuilder() {} + protected OrganizationFeedBuilder() {} public String getOrganization() { return organization; diff --git a/test/integration/goldens/logging/BillingAccountLocationName.java b/test/integration/goldens/logging/BillingAccountLocationName.java index ead5f6cdec..deaf554a9f 100644 --- a/test/integration/goldens/logging/BillingAccountLocationName.java +++ b/test/integration/goldens/logging/BillingAccountLocationName.java @@ -36,6 +36,12 @@ public class BillingAccountLocationName implements ResourceName { private final String billingAccount; private final String location; + @Deprecated + protected BillingAccountLocationName() { + billingAccount = null; + location = null; + } + private BillingAccountLocationName(Builder builder) { billingAccount = Preconditions.checkNotNull(builder.getBillingAccount()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -157,7 +163,7 @@ public static class Builder { private String billingAccount; private String location; - private Builder() {} + protected Builder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/BillingAccountName.java b/test/integration/goldens/logging/BillingAccountName.java index aac4c70071..d9667ac9c6 100644 --- a/test/integration/goldens/logging/BillingAccountName.java +++ b/test/integration/goldens/logging/BillingAccountName.java @@ -34,6 +34,11 @@ public class BillingAccountName implements ResourceName { private volatile Map fieldValuesMap; private final String billingAccount; + @Deprecated + protected BillingAccountName() { + billingAccount = null; + } + private BillingAccountName(Builder builder) { billingAccount = Preconditions.checkNotNull(builder.getBillingAccount()); } @@ -141,7 +146,7 @@ public int hashCode() { public static class Builder { private String billingAccount; - private Builder() {} + protected Builder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/CmekSettingsName.java b/test/integration/goldens/logging/CmekSettingsName.java index e90b6428a1..a3125e93e9 100644 --- a/test/integration/goldens/logging/CmekSettingsName.java +++ b/test/integration/goldens/logging/CmekSettingsName.java @@ -47,6 +47,14 @@ public class CmekSettingsName implements ResourceName { private final String folder; private final String billingAccount; + @Deprecated + protected CmekSettingsName() { + project = null; + organization = null; + folder = null; + billingAccount = null; + } + private CmekSettingsName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); organization = null; @@ -287,7 +295,7 @@ public int hashCode() { public static class Builder { private String project; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -315,7 +323,7 @@ public CmekSettingsName build() { public static class OrganizationBuilder { private String organization; - private OrganizationBuilder() {} + protected OrganizationBuilder() {} public String getOrganization() { return organization; @@ -336,7 +344,7 @@ public CmekSettingsName build() { public static class FolderBuilder { private String folder; - private FolderBuilder() {} + protected FolderBuilder() {} public String getFolder() { return folder; @@ -357,7 +365,7 @@ public CmekSettingsName build() { public static class BillingAccountBuilder { private String billingAccount; - private BillingAccountBuilder() {} + protected BillingAccountBuilder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/FolderLocationName.java b/test/integration/goldens/logging/FolderLocationName.java index 24d56ee303..afde415244 100644 --- a/test/integration/goldens/logging/FolderLocationName.java +++ b/test/integration/goldens/logging/FolderLocationName.java @@ -35,6 +35,12 @@ public class FolderLocationName implements ResourceName { private final String folder; private final String location; + @Deprecated + protected FolderLocationName() { + folder = null; + location = null; + } + private FolderLocationName(Builder builder) { folder = Preconditions.checkNotNull(builder.getFolder()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -154,7 +160,7 @@ public static class Builder { private String folder; private String location; - private Builder() {} + protected Builder() {} public String getFolder() { return folder; diff --git a/test/integration/goldens/logging/FolderName.java b/test/integration/goldens/logging/FolderName.java index 8f0c022439..20940167a0 100644 --- a/test/integration/goldens/logging/FolderName.java +++ b/test/integration/goldens/logging/FolderName.java @@ -34,6 +34,11 @@ public class FolderName implements ResourceName { private volatile Map fieldValuesMap; private final String folder; + @Deprecated + protected FolderName() { + folder = null; + } + private FolderName(Builder builder) { folder = Preconditions.checkNotNull(builder.getFolder()); } @@ -141,7 +146,7 @@ public int hashCode() { public static class Builder { private String folder; - private Builder() {} + protected Builder() {} public String getFolder() { return folder; diff --git a/test/integration/goldens/logging/LocationName.java b/test/integration/goldens/logging/LocationName.java index 9bc33a8940..ab2a8319ea 100644 --- a/test/integration/goldens/logging/LocationName.java +++ b/test/integration/goldens/logging/LocationName.java @@ -35,6 +35,12 @@ public class LocationName implements ResourceName { private final String project; private final String location; + @Deprecated + protected LocationName() { + project = null; + location = null; + } + private LocationName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -154,7 +160,7 @@ public static class Builder { private String project; private String location; - private Builder() {} + protected Builder() {} public String getProject() { return project; diff --git a/test/integration/goldens/logging/LogBucketName.java b/test/integration/goldens/logging/LogBucketName.java index cb94fab601..32d0b934c4 100644 --- a/test/integration/goldens/logging/LogBucketName.java +++ b/test/integration/goldens/logging/LogBucketName.java @@ -53,6 +53,16 @@ public class LogBucketName implements ResourceName { private final String folder; private final String billingAccount; + @Deprecated + protected LogBucketName() { + project = null; + location = null; + bucket = null; + organization = null; + folder = null; + billingAccount = null; + } + private LogBucketName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -372,7 +382,7 @@ public static class Builder { private String location; private String bucket; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -422,7 +432,7 @@ public static class OrganizationLocationBucketBuilder { private String location; private String bucket; - private OrganizationLocationBucketBuilder() {} + protected OrganizationLocationBucketBuilder() {} public String getOrganization() { return organization; @@ -463,7 +473,7 @@ public static class FolderLocationBucketBuilder { private String location; private String bucket; - private FolderLocationBucketBuilder() {} + protected FolderLocationBucketBuilder() {} public String getFolder() { return folder; @@ -504,7 +514,7 @@ public static class BillingAccountLocationBucketBuilder { private String location; private String bucket; - private BillingAccountLocationBucketBuilder() {} + protected BillingAccountLocationBucketBuilder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/LogExclusionName.java b/test/integration/goldens/logging/LogExclusionName.java index 8c0bc46049..037561fda3 100644 --- a/test/integration/goldens/logging/LogExclusionName.java +++ b/test/integration/goldens/logging/LogExclusionName.java @@ -49,6 +49,15 @@ public class LogExclusionName implements ResourceName { private final String folder; private final String billingAccount; + @Deprecated + protected LogExclusionName() { + project = null; + exclusion = null; + organization = null; + folder = null; + billingAccount = null; + } + private LogExclusionName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); exclusion = Preconditions.checkNotNull(builder.getExclusion()); @@ -321,7 +330,7 @@ public static class Builder { private String project; private String exclusion; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -360,7 +369,7 @@ public static class OrganizationExclusionBuilder { private String organization; private String exclusion; - private OrganizationExclusionBuilder() {} + protected OrganizationExclusionBuilder() {} public String getOrganization() { return organization; @@ -391,7 +400,7 @@ public static class FolderExclusionBuilder { private String folder; private String exclusion; - private FolderExclusionBuilder() {} + protected FolderExclusionBuilder() {} public String getFolder() { return folder; @@ -422,7 +431,7 @@ public static class BillingAccountExclusionBuilder { private String billingAccount; private String exclusion; - private BillingAccountExclusionBuilder() {} + protected BillingAccountExclusionBuilder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/LogMetricName.java b/test/integration/goldens/logging/LogMetricName.java index 1d7150abad..2b50476244 100644 --- a/test/integration/goldens/logging/LogMetricName.java +++ b/test/integration/goldens/logging/LogMetricName.java @@ -35,6 +35,12 @@ public class LogMetricName implements ResourceName { private final String project; private final String metric; + @Deprecated + protected LogMetricName() { + project = null; + metric = null; + } + private LogMetricName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); metric = Preconditions.checkNotNull(builder.getMetric()); @@ -153,7 +159,7 @@ public static class Builder { private String project; private String metric; - private Builder() {} + protected Builder() {} public String getProject() { return project; diff --git a/test/integration/goldens/logging/LogName.java b/test/integration/goldens/logging/LogName.java index b4f4cea34e..7a1e540e8f 100644 --- a/test/integration/goldens/logging/LogName.java +++ b/test/integration/goldens/logging/LogName.java @@ -48,6 +48,15 @@ public class LogName implements ResourceName { private final String folder; private final String billingAccount; + @Deprecated + protected LogName() { + project = null; + log = null; + organization = null; + folder = null; + billingAccount = null; + } + private LogName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); log = Preconditions.checkNotNull(builder.getLog()); @@ -307,7 +316,7 @@ public static class Builder { private String project; private String log; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -346,7 +355,7 @@ public static class OrganizationLogBuilder { private String organization; private String log; - private OrganizationLogBuilder() {} + protected OrganizationLogBuilder() {} public String getOrganization() { return organization; @@ -377,7 +386,7 @@ public static class FolderLogBuilder { private String folder; private String log; - private FolderLogBuilder() {} + protected FolderLogBuilder() {} public String getFolder() { return folder; @@ -408,7 +417,7 @@ public static class BillingAccountLogBuilder { private String billingAccount; private String log; - private BillingAccountLogBuilder() {} + protected BillingAccountLogBuilder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/LogSinkName.java b/test/integration/goldens/logging/LogSinkName.java index b2211390dc..c078226190 100644 --- a/test/integration/goldens/logging/LogSinkName.java +++ b/test/integration/goldens/logging/LogSinkName.java @@ -48,6 +48,15 @@ public class LogSinkName implements ResourceName { private final String folder; private final String billingAccount; + @Deprecated + protected LogSinkName() { + project = null; + sink = null; + organization = null; + folder = null; + billingAccount = null; + } + private LogSinkName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); sink = Preconditions.checkNotNull(builder.getSink()); @@ -311,7 +320,7 @@ public static class Builder { private String project; private String sink; - private Builder() {} + protected Builder() {} public String getProject() { return project; @@ -350,7 +359,7 @@ public static class OrganizationSinkBuilder { private String organization; private String sink; - private OrganizationSinkBuilder() {} + protected OrganizationSinkBuilder() {} public String getOrganization() { return organization; @@ -381,7 +390,7 @@ public static class FolderSinkBuilder { private String folder; private String sink; - private FolderSinkBuilder() {} + protected FolderSinkBuilder() {} public String getFolder() { return folder; @@ -412,7 +421,7 @@ public static class BillingAccountSinkBuilder { private String billingAccount; private String sink; - private BillingAccountSinkBuilder() {} + protected BillingAccountSinkBuilder() {} public String getBillingAccount() { return billingAccount; diff --git a/test/integration/goldens/logging/OrganizationLocationName.java b/test/integration/goldens/logging/OrganizationLocationName.java index 2f9cde61d8..f01aea1f66 100644 --- a/test/integration/goldens/logging/OrganizationLocationName.java +++ b/test/integration/goldens/logging/OrganizationLocationName.java @@ -35,6 +35,12 @@ public class OrganizationLocationName implements ResourceName { private final String organization; private final String location; + @Deprecated + protected OrganizationLocationName() { + organization = null; + location = null; + } + private OrganizationLocationName(Builder builder) { organization = Preconditions.checkNotNull(builder.getOrganization()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -154,7 +160,7 @@ public static class Builder { private String organization; private String location; - private Builder() {} + protected Builder() {} public String getOrganization() { return organization; diff --git a/test/integration/goldens/logging/OrganizationName.java b/test/integration/goldens/logging/OrganizationName.java index a8860f3e9c..fdc5f8eaa3 100644 --- a/test/integration/goldens/logging/OrganizationName.java +++ b/test/integration/goldens/logging/OrganizationName.java @@ -34,6 +34,11 @@ public class OrganizationName implements ResourceName { private volatile Map fieldValuesMap; private final String organization; + @Deprecated + protected OrganizationName() { + organization = null; + } + private OrganizationName(Builder builder) { organization = Preconditions.checkNotNull(builder.getOrganization()); } @@ -141,7 +146,7 @@ public int hashCode() { public static class Builder { private String organization; - private Builder() {} + protected Builder() {} public String getOrganization() { return organization; diff --git a/test/integration/goldens/logging/ProjectName.java b/test/integration/goldens/logging/ProjectName.java index 2003259874..58101d3f7a 100644 --- a/test/integration/goldens/logging/ProjectName.java +++ b/test/integration/goldens/logging/ProjectName.java @@ -34,6 +34,11 @@ public class ProjectName implements ResourceName { private volatile Map fieldValuesMap; private final String project; + @Deprecated + protected ProjectName() { + project = null; + } + private ProjectName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); } @@ -141,7 +146,7 @@ public int hashCode() { public static class Builder { private String project; - private Builder() {} + protected Builder() {} public String getProject() { return project; diff --git a/test/integration/goldens/redis/InstanceName.java b/test/integration/goldens/redis/InstanceName.java index ac6e5b1ce5..56f2d728b1 100644 --- a/test/integration/goldens/redis/InstanceName.java +++ b/test/integration/goldens/redis/InstanceName.java @@ -37,6 +37,13 @@ public class InstanceName implements ResourceName { private final String location; private final String instance; + @Deprecated + protected InstanceName() { + project = null; + location = null; + instance = null; + } + private InstanceName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -174,7 +181,7 @@ public static class Builder { private String location; private String instance; - private Builder() {} + protected Builder() {} public String getProject() { return project; diff --git a/test/integration/goldens/redis/LocationName.java b/test/integration/goldens/redis/LocationName.java index 89cad0d585..4f2c953ed7 100644 --- a/test/integration/goldens/redis/LocationName.java +++ b/test/integration/goldens/redis/LocationName.java @@ -35,6 +35,12 @@ public class LocationName implements ResourceName { private final String project; private final String location; + @Deprecated + protected LocationName() { + project = null; + location = null; + } + private LocationName(Builder builder) { project = Preconditions.checkNotNull(builder.getProject()); location = Preconditions.checkNotNull(builder.getLocation()); @@ -154,7 +160,7 @@ public static class Builder { private String project; private String location; - private Builder() {} + protected Builder() {} public String getProject() { return project; From 7336db7fa27488e24642506705aa9c4d3dc510a6 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 17:06:47 -0800 Subject: [PATCH 02/12] fix: avoid type collisions - refactor {Concrete,Vapor}Ref into AstNodes --- .../generator/engine/ast/AstNodeVisitor.java | 4 + .../engine/ast/ConcreteReference.java | 9 ++ .../api/generator/engine/ast/Reference.java | 4 +- .../generator/engine/ast/VaporReference.java | 5 + .../engine/writer/ImportWriterVisitor.java | 101 ++++++++++++------ .../engine/writer/JavaWriterVisitor.java | 80 ++++++++++++-- .../composer/goldens/TestingClientTest.golden | 18 ++-- 7 files changed, 170 insertions(+), 51 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java index 14f75eb452..b13213a311 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java +++ b/src/main/java/com/google/api/generator/engine/ast/AstNodeVisitor.java @@ -24,6 +24,10 @@ public interface AstNodeVisitor { public void visit(AnnotationNode annotation); + public void visit(ConcreteReference reference); + + public void visit(VaporReference reference); + /** =============================== EXPRESSIONS =============================== */ public void visit(ValueExpr valueExpr); diff --git a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java index 64f34d1927..4cf5728648 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java @@ -37,6 +37,11 @@ public abstract class ConcreteReference implements Reference { // Private. abstract Class clazz(); + @Override + public void accept(AstNodeVisitor visitor) { + visitor.visit(this); + } + @Nullable @Override public abstract Reference wildcardUpperBound(); @@ -188,6 +193,10 @@ public Reference copyAndSetGenerics(List generics) { return toBuilder().setGenerics(generics).build(); } + public String simpleName() { + return clazz().getSimpleName(); + } + public static ConcreteReference withClazz(Class clazz) { return builder().setClazz(clazz).build(); } diff --git a/src/main/java/com/google/api/generator/engine/ast/Reference.java b/src/main/java/com/google/api/generator/engine/ast/Reference.java index 7031b5cbe6..a0b9325f5d 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Reference.java +++ b/src/main/java/com/google/api/generator/engine/ast/Reference.java @@ -18,7 +18,9 @@ import java.util.List; import javax.annotation.Nullable; -public interface Reference { +public interface Reference extends AstNode { + void accept(AstNodeVisitor visitor); + ImmutableList generics(); String name(); diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index 0461fabab4..a522582734 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -29,6 +29,11 @@ public abstract class VaporReference implements Reference { private static final String RIGHT_ANGLE = ">"; private static final String COMMA = ", "; + @Override + public void accept(AstNodeVisitor visitor) { + visitor.visit(this); + } + @Override public abstract ImmutableList generics(); diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index 557c43c447..a6a5b1bc8f 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -26,6 +26,7 @@ import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; @@ -55,6 +56,7 @@ import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.engine.ast.UnaryOperationExpr; import com.google.api.generator.engine.ast.ValueExpr; +import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.engine.ast.WhileStatement; import com.google.common.base.Preconditions; @@ -65,6 +67,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -76,6 +79,9 @@ public class ImportWriterVisitor implements AstNodeVisitor { private final Set staticImports = new TreeSet<>(); private final Set imports = new TreeSet<>(); + // Cache the list of short names, since it will be used relatively often. + private final Set importShortNames = new TreeSet<>(); + private String currentPackage; @Nullable private String currentClassName; @@ -109,6 +115,24 @@ public String write() { return sb.toString(); } + public boolean collidesWithImport(String pakkage, String shortName) { + // This is a sufficiently-good heuristic since it's unlikely that the AST structure has changed + // if the size is the same. + if (importShortNames.size() != imports.size()) { + importShortNames.clear(); + importShortNames.addAll( + imports.stream() + .map(s -> s.substring(s.lastIndexOf(DOT) + 1)) + .collect(Collectors.toSet())); + } + return importShortNames.contains(shortName) + && imports.stream() + .filter(s -> s.equals(String.format("%s.%s", pakkage, shortName))) + .findFirst() + .orElse("") + .isEmpty(); + } + @Override public void visit(IdentifierNode identifier) { // Nothing to do. @@ -127,6 +151,16 @@ public void visit(TypeNode type) { references(refs); } + @Override + public void visit(ConcreteReference reference) { + handleReference(reference); + } + + @Override + public void visit(VaporReference reference) { + handleReference(reference); + } + @Override public void visit(ScopeNode scope) { // Nothing to do. @@ -410,44 +444,49 @@ private void variableExpressions(List expressions) { } } - private void references(List refs) { - for (Reference ref : refs) { - // Don't need to import this. - if (ref.useFullName()) { - continue; - } - if (!ref.isStaticImport() - && (ref.isFromPackage(PKG_JAVA_LANG) || ref.isFromPackage(currentPackage))) { - continue; - } + private void handleReference(Reference reference) { + // Don't need to import this. + if (reference.useFullName()) { + return; + } + if (!reference.isStaticImport() + && (reference.isFromPackage(PKG_JAVA_LANG) || reference.isFromPackage(currentPackage))) { + return; + } - if (ref.isWildcard()) { - if (ref.wildcardUpperBound() != null) { - references(Arrays.asList(ref.wildcardUpperBound())); - } - continue; + if (reference.isWildcard()) { + if (reference.wildcardUpperBound() != null) { + references(Arrays.asList(reference.wildcardUpperBound())); } + return; + } - if (ref.isStaticImport() - && !Strings.isNullOrEmpty(currentClassName) - && !ref.enclosingClassNames().isEmpty() - && ref.enclosingClassNames().contains(currentClassName)) { - continue; - } + if (reference.isStaticImport() + && !Strings.isNullOrEmpty(currentClassName) + && !reference.enclosingClassNames().isEmpty() + && reference.enclosingClassNames().contains(currentClassName)) { + return; + } - if (ref.isStaticImport()) { - // This is a static import. - staticImports.add(ref.fullName()); + if (reference.isStaticImport()) { + // This is a static import. + staticImports.add(reference.fullName()); + } else { + if (reference.hasEnclosingClass()) { + imports.add( + String.format( + "%s.%s", reference.pakkage(), String.join(DOT, reference.enclosingClassNames()))); } else { - if (ref.hasEnclosingClass()) { - imports.add( - String.format("%s.%s", ref.pakkage(), String.join(DOT, ref.enclosingClassNames()))); - } else { - imports.add(ref.fullName()); - } + imports.add(reference.fullName()); } + } - references(ref.generics()); + references(reference.generics()); + } + + private void references(List refs) { + for (Reference ref : refs) { + ref.accept(this); } } diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 77801d69cd..f2984f36ad 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -26,6 +26,7 @@ import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; import com.google.api.generator.engine.ast.CommentStatement; +import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EmptyLineStatement; import com.google.api.generator.engine.ast.EnumRefExpr; import com.google.api.generator.engine.ast.Expr; @@ -43,6 +44,7 @@ import com.google.api.generator.engine.ast.NewObjectExpr; import com.google.api.generator.engine.ast.OperatorKind; import com.google.api.generator.engine.ast.PackageInfoDefinition; +import com.google.api.generator.engine.ast.Reference; import com.google.api.generator.engine.ast.ReferenceConstructorExpr; import com.google.api.generator.engine.ast.RelationalOperationExpr; import com.google.api.generator.engine.ast.ReturnExpr; @@ -56,6 +58,7 @@ import com.google.api.generator.engine.ast.TypeNode.TypeKind; import com.google.api.generator.engine.ast.UnaryOperationExpr; import com.google.api.generator.engine.ast.ValueExpr; +import com.google.api.generator.engine.ast.VaporReference; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.engine.ast.WhileStatement; @@ -145,23 +148,15 @@ public void visit(IdentifierNode identifier) { @Override public void visit(TypeNode type) { TypeKind typeKind = type.typeKind(); - StringBuilder generatedCodeBuilder = new StringBuilder(); if (type.isPrimitiveType()) { - generatedCodeBuilder.append(typeKind.toString().toLowerCase()); + buffer.append(typeKind.toString().toLowerCase()); } else { - if (type.reference().useFullName()) { - generatedCodeBuilder.append(type.reference().pakkage()); - generatedCodeBuilder.append(DOT); - } - // A null pointer exception will be thrown if reference is null, which is WAI. - generatedCodeBuilder.append(type.reference().name()); + type.reference().accept(this); } if (type.isArray()) { - generatedCodeBuilder.append("[]"); + buffer.append("[]"); } - - buffer.append(generatedCodeBuilder.toString()); } @Override @@ -181,6 +176,68 @@ public void visit(AnnotationNode annotation) { newline(); } + @Override + public void visit(ConcreteReference reference) { + if (reference.isWildcard()) { + buffer.append(QUESTION_MARK); + if (reference.wildcardUpperBound() != null) { + // Handle the upper bound. + buffer.append(SPACE); + buffer.append(EXTENDS); + buffer.append(SPACE); + reference.wildcardUpperBound().accept(this); + } + return; + } + String pakkage = reference.pakkage(); + String shortName = reference.name(); + if (reference.useFullName() || importWriterVisitor.collidesWithImport(pakkage, shortName)) { + buffer.append(pakkage); + buffer.append(DOT); + } + + if (reference.hasEnclosingClass() && !reference.isStaticImport()) { + buffer.append(String.join(DOT, reference.enclosingClassNames())); + buffer.append(DOT); + } + + buffer.append(reference.simpleName()); + + if (!reference.generics().isEmpty()) { + buffer.append(LEFT_ANGLE); + for (int i = 0; i < reference.generics().size(); i++) { + Reference r = reference.generics().get(i); + r.accept(this); + if (i < reference.generics().size() - 1) { + buffer.append(COMMA); + buffer.append(SPACE); + } + } + buffer.append(RIGHT_ANGLE); + } + } + + @Override + public void visit(VaporReference reference) { + // This implementation should be more extensive, but there are no existing use cases that + // exercise the edge cases. + // TODO(miraleung): Give this behavioral parity with ConcreteReference. + String pakkage = reference.pakkage(); + String shortName = reference.name(); + + if (reference.useFullName() || importWriterVisitor.collidesWithImport(pakkage, shortName)) { + buffer.append(pakkage); + buffer.append(DOT); + if (reference.hasEnclosingClass()) { + buffer.append(String.join(DOT, reference.enclosingClassNames())); + buffer.append(DOT); + } + } + + // A null pointer exception will be thrown if reference is null, which is WAI. + buffer.append(shortName); + } + /** =============================== EXPRESSIONS =============================== */ @Override public void visit(ValueExpr valueExpr) { @@ -798,6 +855,7 @@ public void visit(ClassDefinition classDefinition) { newline(); } + // This must go first, so that we can check for type collisions. classDefinition.accept(importWriterVisitor); if (!classDefinition.isNested()) { buffer.append(importWriterVisitor.write()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden index f1bdaeeff7..cae284d0c9 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden @@ -281,8 +281,8 @@ public class TestingClientTest { @Test public void getTestTest() throws Exception { - Test expectedResponse = - Test.newBuilder() + com.google.showcase.v1beta1.Test expectedResponse = + com.google.showcase.v1beta1.Test.newBuilder() .setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString()) .setDescription("description-1724546052") .build(); @@ -290,7 +290,7 @@ public class TestingClientTest { TestName name = TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]"); - Test actualResponse = client.getTest(name); + com.google.showcase.v1beta1.Test actualResponse = client.getTest(name); Assert.assertEquals(expectedResponse, actualResponse); List actualRequests = mockTesting.getRequests(); @@ -320,8 +320,8 @@ public class TestingClientTest { @Test public void getTestTest2() throws Exception { - Test expectedResponse = - Test.newBuilder() + com.google.showcase.v1beta1.Test expectedResponse = + com.google.showcase.v1beta1.Test.newBuilder() .setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString()) .setDescription("description-1724546052") .build(); @@ -329,7 +329,7 @@ public class TestingClientTest { String name = "name3373707"; - Test actualResponse = client.getTest(name); + com.google.showcase.v1beta1.Test actualResponse = client.getTest(name); Assert.assertEquals(expectedResponse, actualResponse); List actualRequests = mockTesting.getRequests(); @@ -359,7 +359,8 @@ public class TestingClientTest { @Test public void listTestsTest() throws Exception { - Test responsesElement = Test.newBuilder().build(); + com.google.showcase.v1beta1.Test responsesElement = + com.google.showcase.v1beta1.Test.newBuilder().build(); ListTestsResponse expectedResponse = ListTestsResponse.newBuilder() .setNextPageToken("") @@ -376,7 +377,8 @@ public class TestingClientTest { ListTestsPagedResponse pagedListResponse = client.listTests(request); - List resources = Lists.newArrayList(pagedListResponse.iterateAll()); + List resources = + Lists.newArrayList(pagedListResponse.iterateAll()); Assert.assertEquals(1, resources.size()); Assert.assertEquals(expectedResponse.getTestsList().get(0), resources.get(0)); From 2f7c8571cac663c92bb0b7800d5b264e54dda3e0 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 17:46:13 -0800 Subject: [PATCH 03/12] fix: better message null checks in Parser --- .../google/api/generator/gapic/protoparser/Parser.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index eea185b777..861504b14d 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -344,9 +344,11 @@ static List parseMethods( } } + Message inputMessage = messageTypes.get(inputType.reference().name()); + Preconditions.checkNotNull( + inputMessage, String.format("No message found for %s", inputType.reference().name())); Optional> httpBindingsOpt = - HttpRuleParser.parseHttpBindings( - protoMethod, messageTypes.get(inputType.reference().name()), messageTypes); + HttpRuleParser.parseHttpBindings(protoMethod, inputMessage, messageTypes); List httpBindings = httpBindingsOpt.isPresent() ? httpBindingsOpt.get() : Collections.emptyList(); @@ -370,8 +372,7 @@ static List parseMethods( .setIsPaged(parseIsPaged(protoMethod, messageTypes)) .build()); - // Any input type that has a resource reference will need a resource name helper class. - Message inputMessage = messageTypes.get(inputType.reference().name()); + // Any input type that has a resource reference will need a resource name helper calss. for (Field field : inputMessage.fields()) { if (field.hasResourceReference()) { String resourceTypeString = field.resourceReference().resourceTypeString(); From 88145424225e387b2d3c141adfdf5712104a80f6 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 19:33:32 -0800 Subject: [PATCH 04/12] fix: map proto.Empty RPC types to void in ServiceClient --- .../composer/ServiceClientClassComposer.java | 65 ++++++++++++------- .../ServiceClientTestClassComposer.java | 23 +++++-- .../composer/goldens/IdentityClient.golden | 12 ++-- .../composer/goldens/LoggingClientTest.golden | 6 +- .../goldens/SubscriberClientTest.golden | 30 +++------ .../composer/goldens/TestingClientTest.golden | 6 +- .../goldens/asset/AssetServiceClient.java | 12 ++-- .../goldens/asset/AssetServiceClientTest.java | 6 +- .../logging/ConfigServiceV2Client.java | 24 +++---- .../logging/ConfigServiceV2ClientTest.java | 12 ++-- .../logging/LoggingServiceV2Client.java | 12 ++-- .../logging/LoggingServiceV2ClientTest.java | 6 +- .../logging/MetricsServiceV2Client.java | 12 ++-- .../logging/MetricsServiceV2ClientTest.java | 6 +- .../goldens/redis/CloudRedisClientTest.java | 6 +- 15 files changed, 122 insertions(+), 116 deletions(-) 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 edc3b6a265..b15b2de2a2 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 @@ -539,28 +539,35 @@ private static List createMethodVariants( .setVariableExpr(requestVarExpr) .setValueExpr(requestBuilderExpr) .build(); - List statements = Arrays.asList(ExprStatement.withExpr(requestAssignmentExpr)); - // Return expression. - MethodInvocationExpr returnExpr = + List statements = new ArrayList<>(); + statements.add(ExprStatement.withExpr(requestAssignmentExpr)); + + MethodInvocationExpr rpcInvocationExpr = MethodInvocationExpr.builder() .setMethodName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) .setArguments(Arrays.asList(requestVarExpr.toBuilder().setIsDecl(false).build())) .setReturnType(methodOutputType) .build(); - javaMethods.add( + MethodDefinition.Builder methodVariantBuilder = MethodDefinition.builder() .setHeaderCommentStatements( ServiceClientCommentComposer.createRpcMethodHeaderComment(method, signature)) .setScope(ScopeNode.PUBLIC) .setIsFinal(true) - .setReturnType(methodOutputType) .setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) - .setArguments(arguments) - .setBody(statements) - .setReturnExpr(returnExpr) - .build()); + .setArguments(arguments); + + if (isProtoEmptyType(methodOutputType)) { + statements.add(ExprStatement.withExpr(rpcInvocationExpr)); + methodVariantBuilder = methodVariantBuilder.setReturnType(TypeNode.VOID); + } else { + methodVariantBuilder = + methodVariantBuilder.setReturnType(methodOutputType).setReturnExpr(rpcInvocationExpr); + } + methodVariantBuilder = methodVariantBuilder.setBody(statements); + javaMethods.add(methodVariantBuilder.build()); } return javaMethods; @@ -600,25 +607,34 @@ private static MethodDefinition createMethodDefaultMethod( callableMethodName = String.format(OPERATION_CALLABLE_NAME_PATTERN, methodName); } - MethodInvocationExpr methodReturnExpr = + MethodInvocationExpr callableMethodExpr = MethodInvocationExpr.builder().setMethodName(callableMethodName).build(); - methodReturnExpr = + callableMethodExpr = MethodInvocationExpr.builder() .setMethodName(method.hasLro() ? "futureCall" : "call") .setArguments(Arrays.asList(requestArgVarExpr.toBuilder().setIsDecl(false).build())) - .setExprReferenceExpr(methodReturnExpr) + .setExprReferenceExpr(callableMethodExpr) .setReturnType(methodOutputType) .build(); - return MethodDefinition.builder() - .setHeaderCommentStatements( - ServiceClientCommentComposer.createRpcMethodHeaderComment(method)) - .setScope(ScopeNode.PUBLIC) - .setIsFinal(true) - .setReturnType(methodOutputType) - .setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) - .setArguments(Arrays.asList(requestArgVarExpr)) - .setReturnExpr(methodReturnExpr) - .build(); + MethodDefinition.Builder methodBuilder = + MethodDefinition.builder() + .setHeaderCommentStatements( + ServiceClientCommentComposer.createRpcMethodHeaderComment(method)) + .setScope(ScopeNode.PUBLIC) + .setIsFinal(true) + .setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName)) + .setArguments(Arrays.asList(requestArgVarExpr)); + + if (isProtoEmptyType(methodOutputType)) { + methodBuilder = + methodBuilder + .setBody(Arrays.asList(ExprStatement.withExpr(callableMethodExpr))) + .setReturnType(TypeNode.VOID); + } else { + methodBuilder = + methodBuilder.setReturnExpr(callableMethodExpr).setReturnType(methodOutputType); + } + return methodBuilder.build(); } private static MethodDefinition createLroCallableMethod( @@ -1537,4 +1553,9 @@ private static String getCallableName(CallableMethodKind kind, String rawMethodN } return String.format(CALLABLE_NAME_PATTERN, rawMethodName); } + + private static boolean isProtoEmptyType(TypeNode type) { + return type.reference().pakkage().equals("com.google.protobuf") + && type.reference().name().equals("Empty"); + } } diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index 9e9119032c..82ba668318 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -650,11 +650,17 @@ private static MethodDefinition createRpcTestMethod( .setReturnType(rpcJavaMethodInvocationExpr.type()) .build(); } - methodExprs.add( - AssignmentExpr.builder() - .setVariableExpr(actualResponseVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr(rpcJavaMethodInvocationExpr) - .build()); + + boolean returnsVoid = isProtoEmptyType(methodOutputType); + if (returnsVoid) { + methodExprs.add(rpcJavaMethodInvocationExpr); + } else { + methodExprs.add( + AssignmentExpr.builder() + .setVariableExpr(actualResponseVarExpr.toBuilder().setIsDecl(true).build()) + .setValueExpr(rpcJavaMethodInvocationExpr) + .build()); + } if (method.isPaged()) { // Assign the resources variable. @@ -747,7 +753,7 @@ private static MethodDefinition createRpcTestMethod( .setMethodName("assertEquals") .setArguments(expectedPagedResponseExpr, actualPagedResponseExpr) .build()); - } else { + } else if (!returnsVoid) { methodExprs.add( MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("Assert")) @@ -1898,4 +1904,9 @@ private static String getServiceSettingsClassName(String serviceName) { private static String getMockServiceVarName(String serviceName) { return String.format(MOCK_SERVICE_VAR_NAME_PATTERN, serviceName); } + + private static boolean isProtoEmptyType(TypeNode type) { + return type.reference().pakkage().equals("com.google.protobuf") + && type.reference().name().equals("Empty"); + } } 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 2ac3ee2add..9a09f41143 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 @@ -257,12 +257,12 @@ public class IdentityClient implements BackgroundResource { * @param name * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteUser(UserName name) { + public final void deleteUser(UserName name) { DeleteUserRequest request = DeleteUserRequest.newBuilder() .setName(Objects.isNull(name) ? null : name.toString()) .build(); - return deleteUser(request); + deleteUser(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -272,9 +272,9 @@ public class IdentityClient implements BackgroundResource { * @param name * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteUser(String name) { + public final void deleteUser(String name) { DeleteUserRequest request = DeleteUserRequest.newBuilder().setName(name).build(); - return deleteUser(request); + deleteUser(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -284,8 +284,8 @@ public class IdentityClient implements BackgroundResource { * @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 Empty deleteUser(DeleteUserRequest request) { - return deleteUserCallable().call(request); + public final void deleteUser(DeleteUserRequest request) { + deleteUserCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingClientTest.golden index 044c5691fe..0329f4f7ef 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingClientTest.golden @@ -81,8 +81,7 @@ public class LoggingServiceV2ClientTest { LogName logName = LogName.ofProjectLogName("[PROJECT]", "[LOG]"); - Empty actualResponse = client.deleteLog(logName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLog(logName); List actualRequests = mockLoggingServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -116,8 +115,7 @@ public class LoggingServiceV2ClientTest { String logName = "log_name2013526694"; - Empty actualResponse = client.deleteLog(logName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLog(logName); List actualRequests = mockLoggingServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden index 0e03fd0e11..721a37df25 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/SubscriberClientTest.golden @@ -557,8 +557,7 @@ public class SubscriberClientTest { SubscriptionName subscription = SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]"); - Empty actualResponse = client.deleteSubscription(subscription); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSubscription(subscription); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -592,8 +591,7 @@ public class SubscriberClientTest { String subscription = "subscription341203229"; - Empty actualResponse = client.deleteSubscription(subscription); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSubscription(subscription); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -629,8 +627,7 @@ public class SubscriberClientTest { List ackIds = new ArrayList<>(); int ackDeadlineSeconds = 2135351438; - Empty actualResponse = client.modifyAckDeadline(subscription, ackIds, ackDeadlineSeconds); - Assert.assertEquals(expectedResponse, actualResponse); + client.modifyAckDeadline(subscription, ackIds, ackDeadlineSeconds); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -670,8 +667,7 @@ public class SubscriberClientTest { List ackIds = new ArrayList<>(); int ackDeadlineSeconds = 2135351438; - Empty actualResponse = client.modifyAckDeadline(subscription, ackIds, ackDeadlineSeconds); - Assert.assertEquals(expectedResponse, actualResponse); + client.modifyAckDeadline(subscription, ackIds, ackDeadlineSeconds); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -710,8 +706,7 @@ public class SubscriberClientTest { SubscriptionName subscription = SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]"); List ackIds = new ArrayList<>(); - Empty actualResponse = client.acknowledge(subscription, ackIds); - Assert.assertEquals(expectedResponse, actualResponse); + client.acknowledge(subscription, ackIds); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -748,8 +743,7 @@ public class SubscriberClientTest { String subscription = "subscription341203229"; List ackIds = new ArrayList<>(); - Empty actualResponse = client.acknowledge(subscription, ackIds); - Assert.assertEquals(expectedResponse, actualResponse); + client.acknowledge(subscription, ackIds); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -939,8 +933,7 @@ public class SubscriberClientTest { SubscriptionName subscription = SubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]"); PushConfig pushConfig = PushConfig.newBuilder().build(); - Empty actualResponse = client.modifyPushConfig(subscription, pushConfig); - Assert.assertEquals(expectedResponse, actualResponse); + client.modifyPushConfig(subscription, pushConfig); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -977,8 +970,7 @@ public class SubscriberClientTest { String subscription = "subscription341203229"; PushConfig pushConfig = PushConfig.newBuilder().build(); - Empty actualResponse = client.modifyPushConfig(subscription, pushConfig); - Assert.assertEquals(expectedResponse, actualResponse); + client.modifyPushConfig(subscription, pushConfig); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -1397,8 +1389,7 @@ public class SubscriberClientTest { SnapshotName snapshot = SnapshotName.of("[PROJECT]", "[SNAPSHOT]"); - Empty actualResponse = client.deleteSnapshot(snapshot); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSnapshot(snapshot); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -1432,8 +1423,7 @@ public class SubscriberClientTest { String snapshot = "snapshot284874180"; - Empty actualResponse = client.deleteSnapshot(snapshot); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSnapshot(snapshot); List actualRequests = mockSubscriber.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden index cae284d0c9..441300e8a4 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/TestingClientTest.golden @@ -212,8 +212,7 @@ public class TestingClientTest { DeleteSessionRequest request = DeleteSessionRequest.newBuilder().setName(SessionName.of("[SESSION]").toString()).build(); - Empty actualResponse = client.deleteSession(request); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSession(request); List actualRequests = mockTesting.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -425,8 +424,7 @@ public class TestingClientTest { .setName(TestName.of("[SESSION]", "[SHARD_ID]", "[TEST_ID]").toString()) .build(); - Empty actualResponse = client.deleteTest(request); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteTest(request); List actualRequests = mockTesting.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/test/integration/goldens/asset/AssetServiceClient.java b/test/integration/goldens/asset/AssetServiceClient.java index b3d174be9f..7cf1944064 100644 --- a/test/integration/goldens/asset/AssetServiceClient.java +++ b/test/integration/goldens/asset/AssetServiceClient.java @@ -429,12 +429,12 @@ public final UnaryCallable updateFeedCallable() { * organizations/organization_number/feeds/feed_id * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteFeed(FeedName name) { + public final void deleteFeed(FeedName name) { DeleteFeedRequest request = DeleteFeedRequest.newBuilder() .setName(Objects.isNull(name) ? null : name.toString()) .build(); - return deleteFeed(request); + deleteFeed(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -448,9 +448,9 @@ public final Empty deleteFeed(FeedName name) { * organizations/organization_number/feeds/feed_id * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteFeed(String name) { + public final void deleteFeed(String name) { DeleteFeedRequest request = DeleteFeedRequest.newBuilder().setName(name).build(); - return deleteFeed(request); + deleteFeed(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -462,8 +462,8 @@ public final Empty deleteFeed(String name) { * @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 Empty deleteFeed(DeleteFeedRequest request) { - return deleteFeedCallable().call(request); + public final void deleteFeed(DeleteFeedRequest request) { + deleteFeedCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. diff --git a/test/integration/goldens/asset/AssetServiceClientTest.java b/test/integration/goldens/asset/AssetServiceClientTest.java index dfc1344534..f12688c9d7 100644 --- a/test/integration/goldens/asset/AssetServiceClientTest.java +++ b/test/integration/goldens/asset/AssetServiceClientTest.java @@ -414,8 +414,7 @@ public void deleteFeedTest() throws Exception { FeedName name = FeedName.ofProjectFeedName("[PROJECT]", "[FEED]"); - Empty actualResponse = client.deleteFeed(name); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteFeed(name); List actualRequests = mockAssetService.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -449,8 +448,7 @@ public void deleteFeedTest2() throws Exception { String name = "name3373707"; - Empty actualResponse = client.deleteFeed(name); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteFeed(name); List actualRequests = mockAssetService.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/test/integration/goldens/logging/ConfigServiceV2Client.java b/test/integration/goldens/logging/ConfigServiceV2Client.java index 575980738f..97616e6197 100644 --- a/test/integration/goldens/logging/ConfigServiceV2Client.java +++ b/test/integration/goldens/logging/ConfigServiceV2Client.java @@ -884,12 +884,12 @@ public final UnaryCallable updateSinkCallable() { *

Example: `"projects/my-project-id/sinks/my-sink-id"`. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteSink(LogSinkName sinkName) { + public final void deleteSink(LogSinkName sinkName) { DeleteSinkRequest request = DeleteSinkRequest.newBuilder() .setSinkName(Objects.isNull(sinkName) ? null : sinkName.toString()) .build(); - return deleteSink(request); + deleteSink(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -908,9 +908,9 @@ public final Empty deleteSink(LogSinkName sinkName) { *

Example: `"projects/my-project-id/sinks/my-sink-id"`. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteSink(String sinkName) { + public final void deleteSink(String sinkName) { DeleteSinkRequest request = DeleteSinkRequest.newBuilder().setSinkName(sinkName).build(); - return deleteSink(request); + deleteSink(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -923,8 +923,8 @@ public final Empty deleteSink(String sinkName) { * @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 Empty deleteSink(DeleteSinkRequest request) { - return deleteSinkCallable().call(request); + public final void deleteSink(DeleteSinkRequest request) { + deleteSinkCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -1374,12 +1374,12 @@ public final UnaryCallable updateExclusion *

Example: `"projects/my-project-id/exclusions/my-exclusion-id"`. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteExclusion(LogExclusionName name) { + public final void deleteExclusion(LogExclusionName name) { DeleteExclusionRequest request = DeleteExclusionRequest.newBuilder() .setName(Objects.isNull(name) ? null : name.toString()) .build(); - return deleteExclusion(request); + deleteExclusion(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -1396,9 +1396,9 @@ public final Empty deleteExclusion(LogExclusionName name) { *

Example: `"projects/my-project-id/exclusions/my-exclusion-id"`. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteExclusion(String name) { + public final void deleteExclusion(String name) { DeleteExclusionRequest request = DeleteExclusionRequest.newBuilder().setName(name).build(); - return deleteExclusion(request); + deleteExclusion(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -1410,8 +1410,8 @@ public final Empty deleteExclusion(String name) { * @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 Empty deleteExclusion(DeleteExclusionRequest request) { - return deleteExclusionCallable().call(request); + public final void deleteExclusion(DeleteExclusionRequest request) { + deleteExclusionCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. diff --git a/test/integration/goldens/logging/ConfigServiceV2ClientTest.java b/test/integration/goldens/logging/ConfigServiceV2ClientTest.java index 90207f524d..b129c373ac 100644 --- a/test/integration/goldens/logging/ConfigServiceV2ClientTest.java +++ b/test/integration/goldens/logging/ConfigServiceV2ClientTest.java @@ -1185,8 +1185,7 @@ public void deleteSinkTest() throws Exception { LogSinkName sinkName = LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]"); - Empty actualResponse = client.deleteSink(sinkName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSink(sinkName); List actualRequests = mockConfigServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -1220,8 +1219,7 @@ public void deleteSinkTest2() throws Exception { String sinkName = "sink_name-1391757129"; - Empty actualResponse = client.deleteSink(sinkName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteSink(sinkName); List actualRequests = mockConfigServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -1889,8 +1887,7 @@ public void deleteExclusionTest() throws Exception { LogExclusionName name = LogExclusionName.ofProjectExclusionName("[PROJECT]", "[EXCLUSION]"); - Empty actualResponse = client.deleteExclusion(name); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteExclusion(name); List actualRequests = mockConfigServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -1924,8 +1921,7 @@ public void deleteExclusionTest2() throws Exception { String name = "name3373707"; - Empty actualResponse = client.deleteExclusion(name); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteExclusion(name); List actualRequests = mockConfigServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/test/integration/goldens/logging/LoggingServiceV2Client.java b/test/integration/goldens/logging/LoggingServiceV2Client.java index d50f10ebce..2bdffceb8d 100644 --- a/test/integration/goldens/logging/LoggingServiceV2Client.java +++ b/test/integration/goldens/logging/LoggingServiceV2Client.java @@ -163,12 +163,12 @@ public LoggingServiceV2Stub getStub() { * information about log names, see [LogEntry][google.logging.v2.LogEntry]. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteLog(LogName logName) { + public final void deleteLog(LogName logName) { DeleteLogRequest request = DeleteLogRequest.newBuilder() .setLogName(Objects.isNull(logName) ? null : logName.toString()) .build(); - return deleteLog(request); + deleteLog(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -187,9 +187,9 @@ public final Empty deleteLog(LogName logName) { * information about log names, see [LogEntry][google.logging.v2.LogEntry]. * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteLog(String logName) { + public final void deleteLog(String logName) { DeleteLogRequest request = DeleteLogRequest.newBuilder().setLogName(logName).build(); - return deleteLog(request); + deleteLog(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -203,8 +203,8 @@ public final Empty deleteLog(String logName) { * @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 Empty deleteLog(DeleteLogRequest request) { - return deleteLogCallable().call(request); + public final void deleteLog(DeleteLogRequest request) { + deleteLogCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. diff --git a/test/integration/goldens/logging/LoggingServiceV2ClientTest.java b/test/integration/goldens/logging/LoggingServiceV2ClientTest.java index a3a8022c52..07dca33347 100644 --- a/test/integration/goldens/logging/LoggingServiceV2ClientTest.java +++ b/test/integration/goldens/logging/LoggingServiceV2ClientTest.java @@ -93,8 +93,7 @@ public void deleteLogTest() throws Exception { LogName logName = LogName.ofProjectLogName("[PROJECT]", "[LOG]"); - Empty actualResponse = client.deleteLog(logName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLog(logName); List actualRequests = mockLoggingServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -128,8 +127,7 @@ public void deleteLogTest2() throws Exception { String logName = "log_name2013526694"; - Empty actualResponse = client.deleteLog(logName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLog(logName); List actualRequests = mockLoggingServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/test/integration/goldens/logging/MetricsServiceV2Client.java b/test/integration/goldens/logging/MetricsServiceV2Client.java index 09761c6979..dc3334a607 100644 --- a/test/integration/goldens/logging/MetricsServiceV2Client.java +++ b/test/integration/goldens/logging/MetricsServiceV2Client.java @@ -409,12 +409,12 @@ public final UnaryCallable updateLogMetricCal *

"projects/[PROJECT_ID]/metrics/[METRIC_ID]" * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteLogMetric(LogMetricName metricName) { + public final void deleteLogMetric(LogMetricName metricName) { DeleteLogMetricRequest request = DeleteLogMetricRequest.newBuilder() .setMetricName(Objects.isNull(metricName) ? null : metricName.toString()) .build(); - return deleteLogMetric(request); + deleteLogMetric(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -427,10 +427,10 @@ public final Empty deleteLogMetric(LogMetricName metricName) { *

"projects/[PROJECT_ID]/metrics/[METRIC_ID]" * @throws com.google.api.gax.rpc.ApiException if the remote call fails */ - public final Empty deleteLogMetric(String metricName) { + public final void deleteLogMetric(String metricName) { DeleteLogMetricRequest request = DeleteLogMetricRequest.newBuilder().setMetricName(metricName).build(); - return deleteLogMetric(request); + deleteLogMetric(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. @@ -442,8 +442,8 @@ public final Empty deleteLogMetric(String metricName) { * @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 Empty deleteLogMetric(DeleteLogMetricRequest request) { - return deleteLogMetricCallable().call(request); + public final void deleteLogMetric(DeleteLogMetricRequest request) { + deleteLogMetricCallable().call(request); } // AUTO-GENERATED DOCUMENTATION AND METHOD. diff --git a/test/integration/goldens/logging/MetricsServiceV2ClientTest.java b/test/integration/goldens/logging/MetricsServiceV2ClientTest.java index e4715490da..2d54ee79c9 100644 --- a/test/integration/goldens/logging/MetricsServiceV2ClientTest.java +++ b/test/integration/goldens/logging/MetricsServiceV2ClientTest.java @@ -459,8 +459,7 @@ public void deleteLogMetricTest() throws Exception { LogMetricName metricName = LogMetricName.of("[PROJECT]", "[METRIC]"); - Empty actualResponse = client.deleteLogMetric(metricName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLogMetric(metricName); List actualRequests = mockMetricsServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -494,8 +493,7 @@ public void deleteLogMetricTest2() throws Exception { String metricName = "metric_name-1737602118"; - Empty actualResponse = client.deleteLogMetric(metricName); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteLogMetric(metricName); List actualRequests = mockMetricsServiceV2.getRequests(); Assert.assertEquals(1, actualRequests.size()); diff --git a/test/integration/goldens/redis/CloudRedisClientTest.java b/test/integration/goldens/redis/CloudRedisClientTest.java index 277155a0c6..47640c187d 100644 --- a/test/integration/goldens/redis/CloudRedisClientTest.java +++ b/test/integration/goldens/redis/CloudRedisClientTest.java @@ -879,8 +879,7 @@ public void deleteInstanceTest() throws Exception { InstanceName name = InstanceName.of("[PROJECT]", "[LOCATION]", "[INSTANCE]"); - Empty actualResponse = client.deleteInstanceAsync(name).get(); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteInstanceAsync(name).get(); List actualRequests = mockCloudRedis.getRequests(); Assert.assertEquals(1, actualRequests.size()); @@ -922,8 +921,7 @@ public void deleteInstanceTest2() throws Exception { String name = "name3373707"; - Empty actualResponse = client.deleteInstanceAsync(name).get(); - Assert.assertEquals(expectedResponse, actualResponse); + client.deleteInstanceAsync(name).get(); List actualRequests = mockCloudRedis.getRequests(); Assert.assertEquals(1, actualRequests.size()); From ca8398f96ca2062649c2a5198102768c279cca8a Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 21:58:34 -0800 Subject: [PATCH 05/12] fix: improve import(shortName) state-keeping in ImportWriterVisitor --- .../engine/writer/ImportWriterVisitor.java | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java index a6a5b1bc8f..589a7a527d 100644 --- a/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/ImportWriterVisitor.java @@ -88,6 +88,7 @@ public class ImportWriterVisitor implements AstNodeVisitor { public void clear() { staticImports.clear(); imports.clear(); + importShortNames.clear(); } public void initialize(@Nonnull String currentPackage) { @@ -119,11 +120,7 @@ public boolean collidesWithImport(String pakkage, String shortName) { // This is a sufficiently-good heuristic since it's unlikely that the AST structure has changed // if the size is the same. if (importShortNames.size() != imports.size()) { - importShortNames.clear(); - importShortNames.addAll( - imports.stream() - .map(s -> s.substring(s.lastIndexOf(DOT) + 1)) - .collect(Collectors.toSet())); + updateShortNames(); } return importShortNames.contains(shortName) && imports.stream() @@ -426,6 +423,21 @@ public void visit(PackageInfoDefinition packageInfoDefinition) { } /** =============================== PRIVATE HELPERS =============================== */ + private void addImport(String packageToImport) { + String shortName = packageToImport.substring(packageToImport.lastIndexOf(DOT) + 1); + if (importShortNames.contains(shortName)) { + return; + } + importShortNames.add(shortName); + imports.add(packageToImport); + } + + private void updateShortNames() { + importShortNames.clear(); + importShortNames.addAll( + imports.stream().map(s -> s.substring(s.lastIndexOf(DOT) + 1)).collect(Collectors.toSet())); + } + private void annotations(List annotations) { for (AnnotationNode annotation : annotations) { annotation.accept(this); @@ -470,14 +482,16 @@ private void handleReference(Reference reference) { if (reference.isStaticImport()) { // This is a static import. + // TODO(miraleung): This should have a variant of addImports as well. Handle static import + // collisions. staticImports.add(reference.fullName()); } else { if (reference.hasEnclosingClass()) { - imports.add( + addImport( String.format( "%s.%s", reference.pakkage(), String.join(DOT, reference.enclosingClassNames()))); } else { - imports.add(reference.fullName()); + addImport(reference.fullName()); } } From b48f4946d689ea3dd48ad61b256672a6939bf3ad Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 22:00:19 -0800 Subject: [PATCH 06/12] fix: fix stub pkg for all ServiceSettingsStub usages --- .../generator/gapic/composer/ServiceSettingsClassComposer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java index 3cf805932f..11a314e0db 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceSettingsClassComposer.java @@ -836,7 +836,7 @@ private static String getStubSettingsClassName(String serviceName) { private static TypeNode getStubSettingsBuilderType(Service service) { return TypeNode.withReference( VaporReference.builder() - .setPakkage(service.pakkage()) + .setPakkage(String.format("%s.stub", service.pakkage())) .setName(BUILDER_CLASS_NAME) .setEnclosingClassNames(getStubSettingsClassName(service.name())) .build()); From 93ed4bd62513c8351fd6cc5de1ecea08bb150b97 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 23:10:13 -0800 Subject: [PATCH 07/12] fix: handle older protos with javaMultipleFiles=false --- .../engine/ast/ConcreteReference.java | 9 +++--- .../api/generator/engine/ast/Reference.java | 3 ++ .../generator/engine/ast/VaporReference.java | 15 +++++----- .../composer/ServiceClientClassComposer.java | 2 +- .../ServiceClientTestClassComposer.java | 22 +++++++++------ .../ServiceStubSettingsClassComposer.java | 4 +-- .../protoparser/MethodSignatureParser.java | 3 +- .../generator/gapic/protoparser/Parser.java | 6 ++-- .../gapic/protoparser/TypeParser.java | 28 +++++++++++++++---- 9 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java index 4cf5728648..144c4beca1 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java @@ -85,6 +85,11 @@ public String name() { return sb.toString(); } + @Override + public String simpleName() { + return clazz().getSimpleName(); + } + @Override public String pakkage() { return clazz().getPackage().getName(); @@ -193,10 +198,6 @@ public Reference copyAndSetGenerics(List generics) { return toBuilder().setGenerics(generics).build(); } - public String simpleName() { - return clazz().getSimpleName(); - } - public static ConcreteReference withClazz(Class clazz) { return builder().setClazz(clazz).build(); } diff --git a/src/main/java/com/google/api/generator/engine/ast/Reference.java b/src/main/java/com/google/api/generator/engine/ast/Reference.java index a0b9325f5d..e735f5be1c 100644 --- a/src/main/java/com/google/api/generator/engine/ast/Reference.java +++ b/src/main/java/com/google/api/generator/engine/ast/Reference.java @@ -25,6 +25,9 @@ public interface Reference extends AstNode { String name(); + // Just the class name, no generics. + String simpleName(); + String fullName(); String pakkage(); diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index a522582734..8c0cc858fa 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -40,6 +40,9 @@ public void accept(AstNodeVisitor visitor) { @Override public abstract String name(); + @Override + public abstract String simpleName(); + @Override public abstract String pakkage(); @@ -63,9 +66,9 @@ public Reference wildcardUpperBound() { public String fullName() { if (hasEnclosingClass()) { return String.format( - "%s.%s.%s", pakkage(), String.join(DOT, enclosingClassNames()), plainName()); + "%s.%s.%s", pakkage(), String.join(DOT, enclosingClassNames()), simpleName()); } - return String.format("%s.%s", pakkage(), plainName()); + return String.format("%s.%s", pakkage(), simpleName()); } @Override @@ -90,7 +93,7 @@ public boolean isSupertypeOrEquals(Reference other) { VaporReference ref = (VaporReference) other; return pakkage().equals(ref.pakkage()) - && plainName().equals(ref.plainName()) + && simpleName().equals(ref.simpleName()) && Objects.equals(enclosingClassNames(), ref.enclosingClassNames()); } @@ -105,8 +108,6 @@ public boolean isWildcard() { return false; } - abstract String plainName(); - @Override public boolean equals(Object o) { if (!(o instanceof VaporReference)) { @@ -170,7 +171,7 @@ public Builder setEnclosingClassNames(String... enclosingClassNames) { public abstract Builder setSupertypeReference(Reference supertypeReference); // Private. - abstract Builder setPlainName(String plainName); + abstract Builder setSimpleName(String simpleName); abstract String name(); @@ -191,7 +192,7 @@ public VaporReference build() { IdentifierNode.builder().setName(name()).build(); // No exception thrown, so we can proceed. - setPlainName(name()); + setSimpleName(name()); setIsStaticImport(!enclosingClassNames().isEmpty() && isStaticImport()); 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 b15b2de2a2..5376eb91d0 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 @@ -838,7 +838,7 @@ private static List createNestedPagingClasses( continue; } // Find the repeated field. - Message methodOutputMessage = messageTypes.get(method.outputType().reference().name()); + Message methodOutputMessage = messageTypes.get(method.outputType().reference().simpleName()); Field repeatedPagedResultsField = methodOutputMessage.findAndUnwrapFirstRepeatedField(); Preconditions.checkNotNull( repeatedPagedResultsField, diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index 82ba668318..7d652941bd 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -486,7 +486,7 @@ private static MethodDefinition createRpcTestMethod( TypeNode repeatedResponseType = null; VariableExpr responsesElementVarExpr = null; if (method.isPaged()) { - Message methodOutputMessage = messageTypes.get(method.outputType().reference().name()); + Message methodOutputMessage = messageTypes.get(method.outputType().reference().simpleName()); Field repeatedPagedResultsField = methodOutputMessage.findAndUnwrapFirstRepeatedField(); Preconditions.checkNotNull( repeatedPagedResultsField, @@ -517,7 +517,7 @@ private static MethodDefinition createRpcTestMethod( Variable.builder().setType(methodOutputType).setName("expectedResponse").build()); Expr expectedResponseValExpr = null; if (method.isPaged()) { - Message methodOutputMessage = messageTypes.get(method.outputType().reference().name()); + Message methodOutputMessage = messageTypes.get(method.outputType().reference().simpleName()); Field firstRepeatedField = methodOutputMessage.findAndUnwrapFirstRepeatedField(); Preconditions.checkNotNull( firstRepeatedField, @@ -532,7 +532,9 @@ private static MethodDefinition createRpcTestMethod( if (messageTypes.containsKey(methodOutputType.reference().name())) { expectedResponseValExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( - messageTypes.get(methodOutputType.reference().name()), resourceNames, messageTypes); + messageTypes.get(methodOutputType.reference().simpleName()), + resourceNames, + messageTypes); } else { // Wrap this in a field so we don't have to split the helper into lots of different methods, // or duplicate it for VariableExpr. @@ -596,7 +598,7 @@ private static MethodDefinition createRpcTestMethod( VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); argExprs.add(requestVarExpr); - requestMessage = messageTypes.get(method.inputType().reference().name()); + requestMessage = messageTypes.get(method.inputType().reference().simpleName()); Preconditions.checkNotNull(requestMessage); Expr valExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( @@ -717,7 +719,7 @@ private static MethodDefinition createRpcTestMethod( .build()); // Assert the responses are equivalent. - Message methodOutputMessage = messageTypes.get(method.outputType().reference().name()); + Message methodOutputMessage = messageTypes.get(method.outputType().reference().simpleName()); Field repeatedPagedResultsField = methodOutputMessage.findAndUnwrapFirstRepeatedField(); Preconditions.checkNotNull( repeatedPagedResultsField, @@ -947,7 +949,9 @@ private static MethodDefinition createStreamingRpcTestMethod( if (messageTypes.containsKey(methodOutputType.reference().name())) { expectedResponseValExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( - messageTypes.get(methodOutputType.reference().name()), resourceNames, messageTypes); + messageTypes.get(methodOutputType.reference().simpleName()), + resourceNames, + messageTypes); } else { // Wrap this in a field so we don't have to split the helper into lots of different methods, // or duplicate it for VariableExpr. @@ -998,7 +1002,7 @@ private static MethodDefinition createStreamingRpcTestMethod( VariableExpr requestVarExpr = VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); - Message requestMessage = messageTypes.get(method.inputType().reference().name()); + Message requestMessage = messageTypes.get(method.inputType().reference().simpleName()); Preconditions.checkNotNull(requestMessage); Expr valExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( @@ -1265,7 +1269,7 @@ private static List createStreamingRpcExceptionTestStatements( VariableExpr requestVarExpr = VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); - Message requestMessage = messageTypes.get(method.inputType().reference().name()); + Message requestMessage = messageTypes.get(method.inputType().reference().simpleName()); Preconditions.checkNotNull(requestMessage); Expr valExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( @@ -1447,7 +1451,7 @@ private static List createRpcExceptionTestStatements( VariableExpr.withVariable( Variable.builder().setType(method.inputType()).setName("request").build()); argVarExprs.add(varExpr); - Message requestMessage = messageTypes.get(method.inputType().reference().name()); + Message requestMessage = messageTypes.get(method.inputType().reference().simpleName()); Preconditions.checkNotNull(requestMessage); Expr valExpr = DefaultValueComposer.createSimpleMessageBuilderExpr( diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index 9fe65b37b6..a693aec56d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -327,10 +327,10 @@ private static List createPagingStaticAssignExprs( // Find the repeated type. String pagedResponseMessageKey = - JavaStyle.toUpperCamelCase(method.outputType().reference().name()); + JavaStyle.toUpperCamelCase(method.outputType().reference().simpleName()); if (method.hasLro()) { pagedResponseMessageKey = - JavaStyle.toUpperCamelCase(method.lro().responseType().reference().name()); + JavaStyle.toUpperCamelCase(method.lro().responseType().reference().simpleName()); } Message pagedResponseMessage = messageTypes.get(pagedResponseMessageKey); Preconditions.checkNotNull( diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java index 707ff91e71..ebf877d133 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/MethodSignatureParser.java @@ -54,8 +54,7 @@ public static List> parseMethodSignatures( } Map patternsToResourceNames = createPatternResourceNameMap(resourceNames); - String methodInputTypeName = methodInputType.reference().name(); - Message inputMessage = messageTypes.get(methodInputTypeName); + Message inputMessage = messageTypes.get(methodInputType.reference().simpleName()); // Example from Expand in echo.proto: // stringSigs: ["content,error", "content,error,info"]. diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 861504b14d..be08a41762 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -255,7 +255,6 @@ private static Map parseMessages( messages.putAll(parseMessages(nestedMessage, outerNestedTypes)); } } - String pakkage = TypeParser.getPackage(messageDescriptor.getFile()); messages.put( messageName, Message.builder() @@ -344,9 +343,10 @@ static List parseMethods( } } - Message inputMessage = messageTypes.get(inputType.reference().name()); + Message inputMessage = messageTypes.get(inputType.reference().simpleName()); Preconditions.checkNotNull( - inputMessage, String.format("No message found for %s", inputType.reference().name())); + inputMessage, + String.format("No message found for %s", inputType.reference().simpleName())); Optional> httpBindingsOpt = HttpRuleParser.parseHttpBindings(protoMethod, inputMessage, messageTypes); List httpBindings = diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java index 8291fd28ab..e700f6f26a 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/TypeParser.java @@ -23,6 +23,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; +import com.google.protobuf.DescriptorProtos.FileOptions; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.EnumDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; @@ -35,6 +36,8 @@ import javax.annotation.Nonnull; public class TypeParser { + private static final String DOT = "."; + private static Reference REFERENCE_BYTE_STRING = ConcreteReference.withClazz(ByteString.class); private static TypeNode TYPE_NODE_BYTE_STRING = TypeNode.withReference(REFERENCE_BYTE_STRING); @@ -115,7 +118,16 @@ static Reference parseFieldReference(FieldDescriptor field) { @VisibleForTesting static Reference parseMessageReference(@Nonnull Descriptor messageDescriptor) { List outerNestedTypeNames = new ArrayList<>(); + FileOptions fileOptions = messageDescriptor.getFile().getOptions(); + String javaOuterClassname = + fileOptions.hasJavaOuterClassname() ? fileOptions.getJavaOuterClassname() : null; + boolean hasJavaOuterClass = + !Strings.isNullOrEmpty(javaOuterClassname) && !fileOptions.getJavaMultipleFiles(); + if (hasJavaOuterClass) { + outerNestedTypeNames.add(javaOuterClassname); + } Descriptor containingType = messageDescriptor.getContainingType(); + // Handles nesting. while (containingType != null) { // Outermost type in the nested type hierarchy lies at index 0. @@ -131,15 +143,21 @@ static Reference parseMessageReference(@Nonnull Descriptor messageDescriptor) { .setEnclosingClassNames(outerNestedTypeNames) .build(); String protoPackage = messageDescriptor.getFile().getPackage(); + String messageFullName = messageDescriptor.getFullName(); + if (hasJavaOuterClass) { + messageFullName = + String.format( + "%s.%s.%s", + messageFullName.substring(0, messageFullName.lastIndexOf(DOT)), + javaOuterClassname, + messageFullName.substring(messageFullName.lastIndexOf(DOT) + 1)); + } Preconditions.checkState( - messageReference - .fullName() - .replace(pakkage, protoPackage) - .equals(messageDescriptor.getFullName()), + messageReference.fullName().replace(pakkage, protoPackage).equals(messageFullName), String.format( "Parsed message name %s does not match actual name %s", messageReference.fullName().replace(pakkage, ""), - messageDescriptor.getFullName().replace(protoPackage, ""))); + messageFullName.replace(protoPackage, ""))); return messageReference; } From bb7d59f6e1740f7fc1327af462cb5f01956cb9fc Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 23:27:22 -0800 Subject: [PATCH 08/12] fix: gate isPaged on all fields for monolith back-compat --- .../google/api/generator/gapic/protoparser/Parser.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index be08a41762..cacf0336f2 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -430,10 +430,14 @@ static LongrunningOperation parseLro( static boolean parseIsPaged( MethodDescriptor methodDescriptor, Map messageTypes) { Message inputMessage = messageTypes.get(methodDescriptor.getInputType().getName()); - Message outputMessage = messageTypes.get(methodDescriptor.getInputType().getName()); + Message outputMessage = messageTypes.get(methodDescriptor.getOutputType().getName()); + + // This should technically handle the absence of either of these fields (aip.dev/158), but we + // gate on their collective presence to ensure the generated surface is backawrds-compatible + // with monolith-gnerated libraries. return inputMessage.fieldMap().containsKey("page_size") - || inputMessage.fieldMap().containsKey("page_token") - || outputMessage.fieldMap().containsKey("next_page_token"); + && inputMessage.fieldMap().containsKey("page_token") + && outputMessage.fieldMap().containsKey("next_page_token"); } @VisibleForTesting From 15c781c895332c53cf135b4bd7801ff1155b79a2 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 19 Nov 2020 23:48:30 -0800 Subject: [PATCH 09/12] fix: fail early if bazel lacks a grpc_service_config.json file --- rules_java_gapic/java_gapic.bzl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index 26d87cacaf..79811e4d21 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -92,6 +92,8 @@ def java_gapic_library( if grpc_service_config: file_args_dict[grpc_service_config] = "grpc-service-config" + else: + fail("Missing a gRPC service config file") # Check the allow-list. if service_yaml: From 2dcee4edd8dd5dadbf41ec7bffbf86bc4508cf29 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 16:12:40 -0800 Subject: [PATCH 10/12] fix: add Bazel handling for lack of resname helper classes --- rules_java_gapic/java_gapic.bzl | 21 ++++++++++++++++++++- rules_java_gapic/java_gapic_pkg.bzl | 4 ++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index 79811e4d21..eb4dd02815 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -34,13 +34,32 @@ def _java_gapic_postprocess_srcjar_impl(ctx): # This may fail if there are spaces and/or too many files (exceed max length of command length). {formatter} --replace $(find {output_dir_path} -type f -printf "%p ") WORKING_DIR=`pwd` + + # Main source files. cd {output_dir_path}/src/main/java zip -r $WORKING_DIR/{output_srcjar_name}.srcjar ./ + + # Resource name source files. + PROTO_DIR=$WORKING_DIR/{output_dir_path}/proto/src/main/java + PROTO_SRCJAR=$WORKING_DIR/{output_srcjar_name}-resource-name.srcjar + if [ -d $PROTO_DIR ] + then + # Some APIs don't have resource name helpers, like BigQuery v2. + # Create an empty file so we can finish building. Gating the resource name rule definition + # on file existences go against Bazel's design patterns, so we'll simply delete all empty + # files during the final packaging process (see java_gapic_pkg.bzl) + mkdir -p $PROTO_DIR + touch $PROTO_DIR/PlaceholderFile.java + fi cd $WORKING_DIR/{output_dir_path}/proto/src/main/java - zip -r $WORKING_DIR/{output_srcjar_name}-resource-name.srcjar ./ + zip -r $PROTO_SRCJAR ./ + + # Test source files. cd $WORKING_DIR/{output_dir_path}/src/test/java zip -r $WORKING_DIR/{output_srcjar_name}-tests.srcjar ./ + cd $WORKING_DIR + mv {output_srcjar_name}.srcjar {output_main} mv {output_srcjar_name}-resource-name.srcjar {output_resource_name} mv {output_srcjar_name}-tests.srcjar {output_test} diff --git a/rules_java_gapic/java_gapic_pkg.bzl b/rules_java_gapic/java_gapic_pkg.bzl index ad9cb74bcc..f06c515f72 100644 --- a/rules_java_gapic/java_gapic_pkg.bzl +++ b/rules_java_gapic/java_gapic_pkg.bzl @@ -252,6 +252,10 @@ def _java_gapic_srcs_pkg_impl(ctx): mkdir -p {package_dir_path}/src/main/java unzip -q -o $src -d {package_dir_path}/src/main/java rm -r -f {package_dir_path}/src/main/java/META-INF + + # Remove empty files. If there are no resource names, one such file might have + # been created. See java_gapic.bzl. + rm $(find {package_dir_path}/src/main/java -size 0) done for proto_src in {proto_srcs}; do mkdir -p {package_dir_path}/src/main/proto From 3c732cfc36e36661e592229deff308d1618e03ea Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 16:20:18 -0800 Subject: [PATCH 11/12] fix: reverse conditional --- rules_java_gapic/java_gapic.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index eb4dd02815..2bd5ea9c1a 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -42,7 +42,7 @@ def _java_gapic_postprocess_srcjar_impl(ctx): # Resource name source files. PROTO_DIR=$WORKING_DIR/{output_dir_path}/proto/src/main/java PROTO_SRCJAR=$WORKING_DIR/{output_srcjar_name}-resource-name.srcjar - if [ -d $PROTO_DIR ] + if [ ! -d $PROTO_DIR ] then # Some APIs don't have resource name helpers, like BigQuery v2. # Create an empty file so we can finish building. Gating the resource name rule definition From c60b0386527e4ad686b2336e760a74cecdf307a3 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 20 Nov 2020 17:55:21 -0800 Subject: [PATCH 12/12] fix: handle floating-point types for test codegen assertEquals --- .../composer/ServiceClientTestClassComposer.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java index 7d652941bd..b2e53ce06e 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceClientTestClassComposer.java @@ -851,11 +851,23 @@ private static MethodDefinition createRpcTestMethod( .build(); Expr expectedFieldExpr = checkExprFn.apply(requestVarExpr); Expr actualFieldExpr = checkExprFn.apply(actualRequestVarExpr); + List assertEqualsArguments = new ArrayList<>(); + assertEqualsArguments.add(expectedFieldExpr); + assertEqualsArguments.add(actualFieldExpr); + if (TypeNode.isFloatingPointType(field.type())) { + boolean isFloat = field.type().equals(TypeNode.FLOAT); + assertEqualsArguments.add( + ValueExpr.withValue( + PrimitiveValue.builder() + .setType(isFloat ? TypeNode.FLOAT : TypeNode.DOUBLE) + .setValue(String.format("0.0001%s", isFloat ? "f" : "")) + .build())); + } methodExprs.add( MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("Assert")) .setMethodName("assertEquals") - .setArguments(expectedFieldExpr, actualFieldExpr) + .setArguments(assertEqualsArguments) .build()); } } else {