Skip to content

Commit

Permalink
[ggj][codegen] fix: clean up resname codegen, lower_snake varnames (#328
Browse files Browse the repository at this point in the history 
)

* feat: add protobuf comment parser util

* fix: add basic proto build rules

* feat: add header comments to ServiceClient

* fix: build protoc at test time

* fix!: wrap protobuf location and process comments

* feat: add comment parsing to methods and fields

* fix: test

* feat: add protobuf comments to ServiceClient

* fix: solidify codegen method order with TypeNode/MethodArg and Comparable

* fix: clean up tests

* fix: ServiceClient member variables and method calls

* fix: ServiceStubSettings builder type

* fix: ServiceSettings Builder construction

* fix: ServiceStub callable types

* feat: java_gapic_library rule impl

* fix: remove debugging comments

* feat: add gradle assembly Bazel rules

* feat: add java_gapic_test Bazel rule

* fix: use Java packages for resname codegen

* fix: build resnames separately and extract into proto/ dir

* fix: remove debug printf

* feat: add ServiceClient.MethodPagedResponse inner class

* feat: add ServiceClient.MethodPage inner class

* feat: add ServiceClient.MethodFixedSizeCollection innser class

* fix: clean up resname codegen, lower_snake varnames
  • Loading branch information
@miraleung
miraleung authored Sep 19, 2020
1 parent 1a1422e commit b05d1d8
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 40 deletions.
57 changes: 24 additions & 33 deletions src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.api.generator.engine.ast.ForStatement;
import com.google.api.generator.engine.ast.IfStatement;
import com.google.api.generator.engine.ast.JavaDocComment;
import com.google.api.generator.engine.ast.LogicalOperationExpr;
import com.google.api.generator.engine.ast.MethodDefinition;
import com.google.api.generator.engine.ast.MethodInvocationExpr;
import com.google.api.generator.engine.ast.NewObjectExpr;
Expand Down Expand Up @@ -166,11 +167,15 @@ private static Map<String, VariableExpr> createPatternTokenClassMembers(
List<List<String>> tokenHierarchies) {
Set<String> tokenSet = getTokenSet(tokenHierarchies);
return tokenSet.stream()
.map(
t ->
VariableExpr.withVariable(
Variable.builder().setName(t).setType(TypeNode.STRING).build()))
.collect(Collectors.toMap(v -> v.variable().identifier().name(), v -> v));
.collect(
Collectors.toMap(
t -> t,
t ->
VariableExpr.withVariable(
Variable.builder()
.setName(JavaStyle.toLowerCamelCase(t))
.setType(TypeNode.STRING)
.build())));
}

private static List<Statement> createClassStatements(
Expand Down Expand Up @@ -281,19 +286,6 @@ private static List<MethodDefinition> createConstructorMethods(
boolean hasVariants = tokenHierarchies.size() > 1;

List<MethodDefinition> javaMethods = new ArrayList<>();
if (hasVariants) {
MethodDefinition deprecatedCtor =
MethodDefinition.constructorBuilder()
.setScope(ScopeNode.PROTECTED)
.setAnnotations(
Arrays.asList(
AnnotationNode.withType(
TypeNode.withReference(ConcreteReference.withClazz(Deprecated.class)))))
.setReturnType(thisClassType)
.build();
javaMethods.add(deprecatedCtor);
}

for (int i = 0; i < tokenHierarchies.size(); i++) {
List<String> tokens = tokenHierarchies.get(i);
List<Expr> bodyExprs = new ArrayList<>();
Expand Down Expand Up @@ -476,6 +468,7 @@ private static List<MethodDefinition> createOfOrFormatMethodHelper(
MethodInvocationExpr returnExpr =
MethodInvocationExpr.builder().setMethodName(builderMethodName).build();
for (String token : tokens) {
String javaTokenVarName = JavaStyle.toLowerCamelCase(token);
returnExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(returnExpr)
Expand All @@ -484,7 +477,10 @@ private static List<MethodDefinition> createOfOrFormatMethodHelper(
.setArguments(
Arrays.asList(
VariableExpr.withVariable(
Variable.builder().setName(token).setType(TypeNode.STRING).build())))
Variable.builder()
.setName(javaTokenVarName)
.setType(TypeNode.STRING)
.build())))
.build();
}
returnExpr =
Expand Down Expand Up @@ -888,28 +884,23 @@ private static MethodDefinition createIsParseableFromMethod(
VariableExpr formattedStringVarExpr =
VariableExpr.withVariable(
Variable.builder().setName("formattedString").setType(TypeNode.STRING).build());
MethodInvocationExpr returnOrExpr =
Expr returnOrExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(templateFinalVarExprs.get(0))
.setMethodName("matches")
.setArguments(Arrays.asList(formattedStringVarExpr))
.setReturnType(TypeNode.BOOLEAN)
.build();
for (int i = 1; i < templateFinalVarExprs.size(); i++) {
// TODO(miraleung): Use actual or operations here.
returnOrExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(returnOrExpr)
.setMethodName("todoOr")
.setArguments(
Arrays.asList(
MethodInvocationExpr.builder()
.setExprReferenceExpr(templateFinalVarExprs.get(i))
.setMethodName("matches")
.setArguments(Arrays.asList(formattedStringVarExpr))
.build()))
.setReturnType(TypeNode.BOOLEAN)
.build();
LogicalOperationExpr.logicalOrWithExprs(
returnOrExpr,
MethodInvocationExpr.builder()
.setExprReferenceExpr(templateFinalVarExprs.get(i))
.setMethodName("matches")
.setArguments(Arrays.asList(formattedStringVarExpr))
.setReturnType(TypeNode.BOOLEAN)
.build());
}

return MethodDefinition.builder()
Expand Down
83 changes: 76 additions & 7 deletions ...est/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,16 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " \"projects/{project}/chocolate/variants/{variant}/foobars/{foobar}\");\n"
+ " private static final PathTemplate FOOBAR =\n"
+ " PathTemplate.createWithoutUrlEncoding(\"foobars/{foobar}\");\n"
+ " private static final PathTemplate BAR_FOO_FOOBAR =\n"
+ " "
+ " PathTemplate.createWithoutUrlEncoding(\"bar_foos/{bar_foo}/foobars/{foobar}\");\n"
+ " private volatile Map<String, String> fieldValuesMap;\n"
+ " private PathTemplate pathTemplate;\n"
+ " private String fixedValue;\n"
+ " private final String project;\n"
+ " private final String foobar;\n"
+ " private final String variant;\n"
+ "\n"
+ " @Deprecated\n"
+ " protected FoobarName() {}\n"
+ " private final String barFoo;\n"
+ "\n"
+ " private FoobarName(Builder builder) {\n"
+ " project = Preconditions.checkNotNull(builder.getProject());\n"
Expand All @@ -259,6 +260,12 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " pathTemplate = FOOBAR;\n"
+ " }\n"
+ "\n"
+ " private FoobarName(BarFooFoobarBuilder builder) {\n"
+ " barFoo = Preconditions.checkNotNull(builder.getBarFoo());\n"
+ " foobar = Preconditions.checkNotNull(builder.getFoobar());\n"
+ " pathTemplate = BAR_FOO_FOOBAR;\n"
+ " }\n"
+ "\n"
+ " public String getProject() {\n"
+ " return project;\n"
+ " }\n"
Expand All @@ -271,6 +278,10 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " return variant;\n"
+ " }\n"
+ "\n"
+ " public String getBarFoo() {\n"
+ " return barFoo;\n"
+ " }\n"
+ "\n"
+ " public static Builder newBuilder() {\n"
+ " return new Builder();\n"
+ " }\n"
Expand All @@ -293,6 +304,12 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " return new FoobarBuilder();\n"
+ " }\n"
+ "\n"
+ " @BetaApi(\"The per-pattern Builders are not stable yet and may be changed in the"
+ " future.\")\n"
+ " public static BarFooFoobarBuilder newBarFooFoobarBuilder() {\n"
+ " return new BarFooFoobarBuilder();\n"
+ " }\n"
+ "\n"
+ " public Builder toBuilder() {\n"
+ " return new Builder(this);\n"
+ " }\n"
Expand Down Expand Up @@ -324,6 +341,12 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " return newFoobarBuilder().setFoobar(foobar).build();\n"
+ " }\n"
+ "\n"
+ " @BetaApi(\"The static create methods are not stable yet and may be changed in the"
+ " future.\")\n"
+ " public static FoobarName ofBarFooFoobarBuilder(String barFoo, String foobar) {\n"
+ " return newBarFooFoobarBuilder().setBarFoo(barFoo).setFoobar(foobar).build();\n"
+ " }\n"
+ "\n"
+ " public static String format(String project, String foobar) {\n"
+ " return newBuilder().setProject(project).setFoobar(foobar).build().toString();\n"
+ " }\n"
Expand Down Expand Up @@ -352,6 +375,13 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " return newFoobarBuilder().setFoobar(foobar).build().toString();\n"
+ " }\n"
+ "\n"
+ " @BetaApi(\"The static format methods are not stable yet and may be changed in the"
+ " future.\")\n"
+ " public static String formatBarFooFoobarBuilder(String barFoo, String foobar) {\n"
+ " return"
+ " newBarFooFoobarBuilder().setBarFoo(barFoo).setFoobar(foobar).build().toString();\n"
+ " }\n"
+ "\n"
+ " public static FoobarName parse(String formattedString) {\n"
+ " if (formattedString.isEmpty()) {\n"
+ " return null;\n"
Expand All @@ -368,6 +398,10 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " } else if (FOOBAR.matches(formattedString)) {\n"
+ " Map<String, String> matchMap = FOOBAR.match(formattedString);\n"
+ " return ofFoobarBuilder(matchMap.get(\"foobar\"));\n"
+ " } else if (BAR_FOO_FOOBAR.matches(formattedString)) {\n"
+ " Map<String, String> matchMap = BAR_FOO_FOOBAR.match(formattedString);\n"
+ " return ofBarFooFoobarBuilder(matchMap.get(\"bar_foo\"),"
+ " matchMap.get(\"foobar\"));\n"
+ " }\n"
+ " throw new ValidationException(\"FoobarName.parse: formattedString not in valid"
+ " format\");\n"
Expand All @@ -394,10 +428,10 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " }\n"
+ "\n"
+ " public static boolean isParsableFrom(String formattedString) {\n"
+ " return PROJECT_FOOBAR\n"
+ " .matches(formattedString)\n"
+ " .todoOr(PROJECT_VARIANT_FOOBAR.matches(formattedString))\n"
+ " .todoOr(FOOBAR.matches(formattedString));\n"
+ " return PROJECT_FOOBAR.matches(formattedString)\n"
+ " || PROJECT_VARIANT_FOOBAR.matches(formattedString)\n"
+ " || FOOBAR.matches(formattedString)\n"
+ " || BAR_FOO_FOOBAR.matches(formattedString);\n"
+ " }\n"
+ "\n"
+ " @Override\n"
Expand All @@ -416,6 +450,9 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " if (!Objects.isNull(variant)) {\n"
+ " fieldMapBuilder.put(\"variant\", variant);\n"
+ " }\n"
+ " if (!Objects.isNull(barFoo)) {\n"
+ " fieldMapBuilder.put(\"bar_foo\", barFoo);\n"
+ " }\n"
+ " fieldValuesMap = fieldMapBuilder.build();\n"
+ " }\n"
+ " }\n"
Expand Down Expand Up @@ -536,6 +573,38 @@ public void generateResourceNameClass_testingSessionOnePattern() {
+ " return new FoobarName(this);\n"
+ " }\n"
+ " }\n"
+ "\n"
+ " /** Builder for bar_foos/{bar_foo}/foobars/{foobar}. */\n"
+ " @BetaApi(\"The per-pattern Builders are not stable yet and may be changed in the"
+ " future.\")\n"
+ " public static class BarFooFoobarBuilder {\n"
+ " private String barFoo;\n"
+ " private String foobar;\n"
+ "\n"
+ " private BarFooFoobarBuilder() {}\n"
+ "\n"
+ " public String getBarFoo() {\n"
+ " return barFoo;\n"
+ " }\n"
+ "\n"
+ " public String getFoobar() {\n"
+ " return foobar;\n"
+ " }\n"
+ "\n"
+ " public BarFooFoobarBuilder setBarFoo(String barFoo) {\n"
+ " this.barFoo = barFoo;\n"
+ " return this;\n"
+ " }\n"
+ "\n"
+ " public BarFooFoobarBuilder setFoobar(String foobar) {\n"
+ " this.foobar = foobar;\n"
+ " return this;\n"
+ " }\n"
+ "\n"
+ " public FoobarName build() {\n"
+ " return new FoobarName(this);\n"
+ " }\n"
+ " }\n"
+ "}\n";

private static final String EXPECTED_SESSION_NAME_CLASS_STRING =
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/api/generator/gapic/testdata/echo.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ message Foobar {
pattern: "projects/{project}/foobars/{foobar}"
pattern: "projects/{project}/chocolate/variants/{variant}/foobars/{foobar}"
pattern: "foobars/{foobar}"
pattern: "bar_foos/{bar_foo}/foobars/{foobar}"
pattern: "*"
};

Expand Down

0 comments on commit b05d1d8

Please sign in to comment.