From 07257501b4d1af5f36707319cbfca41425e723de Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:13:32 -0800 Subject: [PATCH 01/13] fix: swap assertEquals args in JavaWriterVisitorTest to match (expected, actusl) order --- .../engine/writer/JavaWriterVisitorTest.java | 204 +++++++++--------- 1 file changed, 102 insertions(+), 102 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index dac3d20756..98379fac93 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -90,7 +90,7 @@ public void setUp() { public void writeIdentifier() { String idName = "foobar"; IdentifierNode.builder().setName(idName).build().accept(writerVisitor); - assertEquals(writerVisitor.write(), idName); + assertEquals(idName, writerVisitor.write()); } @Test @@ -98,7 +98,7 @@ public void writePrimitiveType() { TypeNode intType = TypeNode.INT; assertThat(intType).isNotNull(); intType.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int"); + assertEquals("int", writerVisitor.write()); } @Test @@ -107,7 +107,7 @@ public void writePrimitiveArrayType() { TypeNode.builder().setTypeKind(TypeNode.TypeKind.BYTE).setIsArray(true).build(); assertThat(byteArrayType).isNotNull(); byteArrayType.accept(writerVisitor); - assertEquals(writerVisitor.write(), "byte[]"); + assertEquals("byte[]", writerVisitor.write()); } @Test @@ -144,14 +144,14 @@ public void writeReferenceType_useFullName() { public void writeAnnotation_simple() { AnnotationNode annotation = AnnotationNode.OVERRIDE; annotation.accept(writerVisitor); - assertEquals(writerVisitor.write(), "@Override\n"); + assertEquals("@Override\n", writerVisitor.write()); } @Test public void writeAnnotation_withDescription() { AnnotationNode annotation = AnnotationNode.withSuppressWarnings("all"); annotation.accept(writerVisitor); - assertEquals(writerVisitor.write(), "@SuppressWarnings(\"all\")\n"); + assertEquals("@SuppressWarnings(\"all\")\n", writerVisitor.write()); } @Test @@ -161,7 +161,7 @@ public void writeNewObjectExpr_basic() { TypeNode type = TypeNode.withReference(ref); NewObjectExpr newObjectExpr = NewObjectExpr.builder().setIsGeneric(true).setType(type).build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "new List<>()"); + assertEquals("new List<>()", writerVisitor.write()); } @Test @@ -182,7 +182,7 @@ public void writeNewObjectExpr_withMethodExprArgs() { .setArguments(Arrays.asList(msgExpr, causeExpr)) .build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "new IOException(message, cause())"); + assertEquals("new IOException(message, cause())", writerVisitor.write()); } @Test @@ -222,7 +222,7 @@ public void writeNewObjectExpr_withGenericsAndArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "new HashMap>(SomeClass.foobar(), num)"); + "new HashMap>(SomeClass.foobar(), num)", writerVisitor.write()); } /** =============================== EXPRESSIONS =============================== */ @@ -231,7 +231,7 @@ public void writeValueExpr() { Value value = PrimitiveValue.builder().setType(TypeNode.INT).setValue("3").build(); ValueExpr valueExpr = ValueExpr.builder().setValue(value).build(); valueExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "3"); + assertEquals("3", writerVisitor.write()); } @Test @@ -240,7 +240,7 @@ public void writeVariableExpr_basic() { VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x"); + assertEquals("x", writerVisitor.write()); } @Test @@ -257,7 +257,7 @@ public void writeVariableExpr_wildcardType() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "List x"); + assertEquals("List x", writerVisitor.write()); } @Test @@ -277,7 +277,7 @@ public void writeVariableExpr_wildcardTypeWithUpperBound() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "List x"); + assertEquals("List x", writerVisitor.write()); } @Test @@ -290,7 +290,7 @@ public void writeVariableExpr_staticReference() { .build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "Integer.MAX_VALUE"); + assertEquals("Integer.MAX_VALUE", writerVisitor.write()); } @Test @@ -305,7 +305,7 @@ public void writeVariableExpr_nonDeclIgnoresModifiers() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x"); + assertEquals("x", writerVisitor.write()); } @Test @@ -314,7 +314,7 @@ public void writeVariableExpr_basicLocalDecl() { VariableExpr expr = VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int x"); + assertEquals("int x", writerVisitor.write()); } @Test @@ -325,7 +325,7 @@ public void writeVariableExpr_localFinalDecl() { VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsDecl(true).build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "final boolean x"); + assertEquals("final boolean x", writerVisitor.write()); } @Test @@ -340,7 +340,7 @@ public void writeVariableExpr_scopedDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private int x"); + assertEquals("private int x", writerVisitor.write()); } @Test @@ -357,7 +357,7 @@ public void writeVariableExpr_scopedStaticFinalDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public static final boolean x"); + assertEquals("public static final boolean x", writerVisitor.write()); } @Test @@ -375,7 +375,7 @@ public void writeVariableExpr_scopedStaticFinalVolatileDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private static final volatile boolean x"); + assertEquals("private static final volatile boolean x", writerVisitor.write()); } @Test @@ -387,7 +387,7 @@ public void writeVariableExpr_basicReference() { variableExpr = VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.length"); + assertEquals("x.length", writerVisitor.write()); } @Test @@ -405,7 +405,7 @@ public void writeVariableExpr_basicReferenceWithModifiersSet() { .setIsStatic(true) .build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.length"); + assertEquals("x.length", writerVisitor.write()); } @Test @@ -424,7 +424,7 @@ public void writeVariableExpr_nestedReference() { variableExpr = VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.someStringField.anotherStringField.lengthField"); + assertEquals("x.someStringField.anotherStringField.lengthField", writerVisitor.write()); } @Test @@ -463,7 +463,7 @@ public void writeBlockCommentStatement_basic() { CommentStatement commentStatement = CommentStatement.withComment(blockComment); String expected = String.format(createLines(3), "/*\n", "* this is a test comment\n", "*/\n"); commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -473,7 +473,7 @@ public void writeLineCommentStatement_basic() { CommentStatement commentStatement = CommentStatement.withComment(lineComment); String expected = "// this is a test comment\n"; commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -535,7 +535,7 @@ public void writeJavaDocCommentStatement_allComponents() { "* @deprecated Use the {@link ArchivedBookName} class instead.\n", "*/\n"); commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -546,7 +546,7 @@ public void writeBlockComment_shortLines() { String.format( createLines(4), "/*\n", "* Apache License\n", "* This is a test file header\n", "*/\n"); blockComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -566,7 +566,7 @@ public void writeBlockComment_newLineInBetween() { "* you may not use this file except in compliance with the License.\n", "*/\n"); blockComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -582,7 +582,7 @@ public void writeLineComment_longLine() { + " for 3 times,\n", "// blah, blah!\n"); lineComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -599,7 +599,7 @@ public void writeLineComment_specialChar() { + "// \\r" + "\"]']\n"; lineComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -639,7 +639,7 @@ public void writeJavaDocComment_specialChar() { "* RPC method comment may include special characters: <>&\"`'{@literal @}.\n", "*/\n"); javaDocComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -663,7 +663,7 @@ public void writeTernaryExpr_basic() { .setElseExpr(elseExpr) .build(); ternaryExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "condition ? 3 : 4"); + assertEquals("condition ? 3 : 4", writerVisitor.write()); } @Test @@ -679,7 +679,7 @@ public void writeAssignmentExpr_basicValue() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int x = 3"); + assertEquals("int x = 3", writerVisitor.write()); } @Test @@ -701,7 +701,7 @@ public void writeAssignmentExpr_varToVar() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private static final int foobar = y"); + assertEquals("private static final int foobar = y", writerVisitor.write()); } @Test @@ -717,7 +717,7 @@ public void writeAssignmentExpr_nullObjectValueReferenceType() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "String x = null"); + assertEquals("String x = null", writerVisitor.write()); } @Test @@ -749,7 +749,7 @@ public void writeMethodInvocationExpr_basic() { MethodInvocationExpr.builder().setMethodName("foobar").build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "foobar()"); + assertEquals("foobar()", writerVisitor.write()); } @Test @@ -768,7 +768,7 @@ public void writeMethodInvocationExpr_staticRef() { .build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "SomeClass.foobar()"); + assertEquals("SomeClass.foobar()", writerVisitor.write()); } @Test @@ -813,9 +813,9 @@ public void writeMethodInvocationExpr_genericWithArgs() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "final String someStr = anArg.," - + " HashMap>>foobar(anArg, anArg, anArg)"); + + " HashMap>>foobar(anArg, anArg, anArg)", + writerVisitor.write()); } @Test @@ -841,7 +841,7 @@ public void writeMethodInvocationExpr_chained() { methodExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "libraryClient.streamBooksCallable().doAnotherThing().call()"); + "libraryClient.streamBooksCallable().doAnotherThing().call()", writerVisitor.write()); } @Test @@ -854,7 +854,7 @@ public void writeCastExpr_basic() { .setExpr(varExpr) .build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((Object) str)"); + assertEquals("((Object) str)", writerVisitor.write()); } @Test @@ -878,7 +878,7 @@ public void writeCastExpr_methodInvocation() { .setExpr(methodExpr) .build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((Object) SomeClass.foobar())"); + assertEquals("((Object) SomeClass.foobar())", writerVisitor.write()); } @Test @@ -892,7 +892,7 @@ public void writeCastExpr_nested() { .build(); castExpr = CastExpr.builder().setType(TypeNode.STRING).setExpr(castExpr).build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((String) ((Object) str))"); + assertEquals("((String) ((Object) str))", writerVisitor.write()); } @Test @@ -914,13 +914,13 @@ public void writeAnonymousClassExpr_basic() { AnonymousClassExpr.builder().setType(type).setMethods(Arrays.asList(method)).build(); anonymousClassExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(4), "new Runnable() {\n", "@Override\n", "public void run() {\n", - "boolean foobar = false;\n}\n\n}")); + "boolean foobar = false;\n}\n\n}"), + writerVisitor.write()); } @Test @@ -966,7 +966,7 @@ public void writeAnonymousClassExpr_withStatementsMethods() { "@Override\n", "public void run() {\n", "int x = 3;\n}\n\n}"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1023,7 +1023,7 @@ public void writeAnonymousClassExpr_generics() { "public MethodDefinition apply(List arg) {\n", "return returnArg;\n", "}\n\n}"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1032,7 +1032,7 @@ public void writeThrowExpr_basic() { TypeNode.withReference(ConcreteReference.withClazz(NullPointerException.class)); ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException()"); + assertEquals("throw new NullPointerException()", writerVisitor.write()); } @Test @@ -1042,7 +1042,7 @@ public void writeThrowExpr_basicWithMessage() { String message = "Some message asdf"; ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException(\"Some message asdf\")"); + assertEquals("throw new NullPointerException(\"Some message asdf\")", writerVisitor.write()); } @Test @@ -1056,7 +1056,7 @@ public void writeThrowExpr_messageExpr() { ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException(foobar())"); + assertEquals("throw new NullPointerException(foobar())", writerVisitor.write()); } @Test @@ -1066,7 +1066,7 @@ public void writeInstanceofExpr() { InstanceofExpr instanceofExpr = InstanceofExpr.builder().setCheckType(TypeNode.STRING).setExpr(variableExpr).build(); instanceofExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x instanceof String"); + assertEquals("x instanceof String", writerVisitor.write()); } @Test @@ -1080,7 +1080,7 @@ public void writeEnumRefExpr_basic() { EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "TypeKind.VOID"); + assertEquals("TypeKind.VOID", writerVisitor.write()); } @Test @@ -1089,7 +1089,7 @@ public void writeEnumRefExpr_nested() { TypeNode.withReference(ConcreteReference.withClazz(TypeNode.TypeKind.class)); EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "TypeNode.TypeKind.VOID"); + assertEquals("TypeNode.TypeKind.VOID", writerVisitor.write()); } @Test @@ -1097,7 +1097,7 @@ public void writeReturnExpr_basic() { ReturnExpr returnExpr = ReturnExpr.withExpr(ValueExpr.withValue(StringObjectValue.withValue("asdf"))); returnExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "return \"asdf\""); + assertEquals("return \"asdf\"", writerVisitor.write()); } /** =============================== STATEMENTS =============================== */ @@ -1118,14 +1118,14 @@ public void writeExprStatement() { ExprStatement exprStatement = ExprStatement.withExpr(methodExpr); exprStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "SomeClass.foobar();\n"); + assertEquals("SomeClass.foobar();\n", writerVisitor.write()); } @Test public void writeBlockStatement_empty() { BlockStatement blockStatement = BlockStatement.builder().build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "{\n}\n"); + assertEquals("{\n}\n", writerVisitor.write()); } @Test @@ -1146,7 +1146,7 @@ public void writeBlockStatement_simple() { BlockStatement.builder().setBody(Arrays.asList(ExprStatement.withExpr(methodExpr))).build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "{\nSomeClass.foobar();\n}\n"); + assertEquals("{\nSomeClass.foobar();\n}\n", writerVisitor.write()); } @Test @@ -1172,7 +1172,7 @@ public void writeBlockStatement_static() { .build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "static {\nSomeClass.foobar();\nSomeClass.foobar();\n}\n"); + assertEquals("static {\nSomeClass.foobar();\nSomeClass.foobar();\n}\n", writerVisitor.write()); } @Test @@ -1187,8 +1187,8 @@ public void writeIfStatement_simple() { ifStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s%s", "if (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s%s", "if (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1207,7 +1207,6 @@ public void writeIfStatement_withElse() { ifStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s" + "%s%s%s%s", "if (condition) {\n", @@ -1216,7 +1215,8 @@ public void writeIfStatement_withElse() { "} else {\n", "int x = 3;\n", "int x = 3;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1258,7 +1258,7 @@ public void writeIfStatement_elseIfs() { "boolean fooBar = true;\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1322,7 +1322,7 @@ public void writeIfStatement_nested() { "boolean fooBar = true;\n", "}\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1337,8 +1337,8 @@ public void writeWhileStatement_simple() { whileStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s%s", "while (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s%s", "while (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1359,10 +1359,10 @@ public void writeForStatement() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "for (String str : getSomeStrings()) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "for (String str : getSomeStrings()) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1382,10 +1382,10 @@ public void writeGeneralForStatement_basicIsDecl() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1405,9 +1405,9 @@ public void writeGeneralForStatement_basicIsNotDecl() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1426,10 +1426,10 @@ public void writeTryCatchStatement_simple() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "try {\n", "int x = 3;\n", "} catch (IllegalArgumentException e) {\n", "}\n")); + "try {\n", "int x = 3;\n", "} catch (IllegalArgumentException e) {\n", "}\n"), + writerVisitor.write()); } @Test @@ -1452,14 +1452,14 @@ public void writeTryCatchStatement_withResources() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s%s", "try (boolean aBool = false) {\n", "int y = 4;\n", "} catch (IllegalArgumentException e) {\n", "int foobar = 123;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1477,7 +1477,7 @@ public void writeTryCatchStatement_sampleCodeNoCatch() { .build(); tryCatch.accept(writerVisitor); - assertEquals(writerVisitor.write(), String.format("%s%s%s", "try {\n", "int x = 3;\n", "}\n")); + assertEquals(String.format("%s%s%s", "try {\n", "int x = 3;\n", "}\n"), writerVisitor.write()); } @Test @@ -1501,14 +1501,14 @@ public void writeTryCatchStatement_sampleCodeWithCatch() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s%s", "try (boolean aBool = false) {\n", "int y = 4;\n", "} catch (IllegalArgumentException e) {\n", "int foobar = 123;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1524,8 +1524,8 @@ public void writeSynchronizedStatement_basicThis() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(3), "synchronized (this) {\n", "doStuff();\n", "}\n")); + String.format(createLines(3), "synchronized (this) {\n", "doStuff();\n", "}\n"), + writerVisitor.write()); } @Test @@ -1543,8 +1543,8 @@ public void writeSynchronizedStatement_basicVariableExpr() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(3), "synchronized (str) {\n", "doStuff();\n", "}\n")); + String.format(createLines(3), "synchronized (str) {\n", "doStuff();\n", "}\n"), + writerVisitor.write()); } @Test @@ -1560,8 +1560,8 @@ public void writeMethodDefinition_basic() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n\n")); + String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n\n"), + writerVisitor.write()); } @Test @@ -1579,7 +1579,7 @@ public void writeMethodDefinition_constructor() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public LibrarySettings() {\n}\n\n"); + assertEquals("public LibrarySettings() {\n}\n\n", writerVisitor.write()); } @Test @@ -1592,7 +1592,7 @@ public void writeMethodDefinition_basicEmptyBody() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public void close() {\n}\n\n"); + assertEquals("public void close() {\n}\n\n", writerVisitor.write()); } @Test @@ -1609,8 +1609,8 @@ public void writeMethodDefinition_basicAbstract() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n\n")); + String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n\n"), + writerVisitor.write()); } @Test @@ -1624,7 +1624,7 @@ public void writeMethodDefinition_basicAbstractEmptyBody() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public abstract void close();\n"); + assertEquals("public abstract void close();\n", writerVisitor.write()); } @Test @@ -1658,13 +1658,13 @@ public void writeMethodDefinition_withArgumentsAndReturnExpr() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", "public int close(int x, int y) {\n", "boolean foobar = false;\n", "return 3;\n", - "}\n\n")); + "}\n\n"), + writerVisitor.write()); } @Test @@ -1742,7 +1742,7 @@ public void writeMethodDefinition_withCommentsAnnotationsAndThrows() { "boolean foobar = false;\n", "return 3;\n", "}\n\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1779,12 +1779,12 @@ public void writeMethodDefinition_templatedReturnTypeAndArguments() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "public Map close(Map x, Map y) {\n", "return foobar();\n", - "}\n\n")); + "}\n\n"), + writerVisitor.write()); } @Test @@ -1801,7 +1801,6 @@ public void writeClassDefinition_basicWithFileHeader() { classDef.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(6), "/*\n", @@ -1809,7 +1808,8 @@ public void writeClassDefinition_basicWithFileHeader() { " */\n\n", "package com.google.example.library.v1.stub;\n", "\n", - "public class LibraryServiceStub {}\n")); + "public class LibraryServiceStub {}\n"), + writerVisitor.write()); } @Test @@ -1833,7 +1833,6 @@ public void writeClassDefinition_withAnnotationsExtendsAndImplements() { classDef.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(5), "package com.google.example.library.v1.stub;\n", @@ -1841,7 +1840,8 @@ public void writeClassDefinition_withAnnotationsExtendsAndImplements() { "@Deprecated\n", "@SuppressWarnings(\"all\")\n", "public final class LibraryServiceStub extends String implements Appendable," - + " Cloneable, Readable {}\n")); + + " Cloneable, Readable {}\n"), + writerVisitor.write()); } @Test @@ -1981,7 +1981,7 @@ public void writeClassDefinition_commentsStatementsAndMethods() { " }\n", " }\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -2032,8 +2032,8 @@ public void writeThisObjectValue_methodReturn() { .build(); methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("public Student apply() {\n" + "return this;\n" + "}\n\n")); + String.format("public Student apply() {\n" + "return this;\n" + "}\n\n"), + writerVisitor.write()); } @Test @@ -2241,7 +2241,7 @@ public void writeAssignmentOperationExpr_xorAssignment() { public void writeEmptyLineStatement() { EmptyLineStatement statement = EmptyLineStatement.create(); statement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "\n"); + assertEquals("\n", writerVisitor.write()); } @Test From 67f3a152087a9d650bb18bf67106025576f46e51 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:15:21 -0800 Subject: [PATCH 02/13] fix: swap assertEquals args in ImportWriterVisitorTest to match (expected, actusl) order --- .../writer/ImportWriterVisitorTest.java | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java index 5463ac76ad..5c5b2d5773 100644 --- a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java @@ -123,7 +123,7 @@ public void writeNewObjectExprImports_basic() { .setType(TypeNode.withReference(ConcreteReference.withClazz(ArrayList.class))) .build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.util.ArrayList;\n\n"); + assertEquals("import java.util.ArrayList;\n\n", writerVisitor.write()); } @Test @@ -141,9 +141,9 @@ public void writeNewObjectExprImports_withArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - createLines(2), "import java.io.File;\n", "import java.io.FileOutputStream;\n\n")); + createLines(2), "import java.io.File;\n", "import java.io.FileOutputStream;\n\n"), + writerVisitor.write()); } @Test @@ -174,8 +174,8 @@ public void writeNewObjectExprImports_genericsAndVariableArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(2), "import java.util.HashMap;\n", "import java.util.List;\n\n")); + String.format(createLines(2), "import java.util.HashMap;\n", "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -203,9 +203,9 @@ public void writeNewObjectExprImports_methodExprArg() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - createLines(2), "import java.io.IOException;\n", "import java.util.HashMap;\n\n")); + createLines(2), "import java.io.IOException;\n", "import java.util.HashMap;\n\n"), + writerVisitor.write()); } @Test @@ -239,12 +239,12 @@ public void writeTernaryExprImports() { .build(); ternaryExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.Expr;\n", "import com.google.api.generator.engine.ast.TypeNode;\n", - "import com.google.common.base.Strings;\n\n")); + "import com.google.common.base.Strings;\n\n"), + writerVisitor.write()); } @Test @@ -273,12 +273,12 @@ public void writeAssignmentExprImports() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AstNode;\n", "import com.google.api.generator.engine.ast.ClassDefinition;\n", - "import com.google.api.some.pakkage.SomeClass;\n\n")); + "import com.google.api.some.pakkage.SomeClass;\n\n"), + writerVisitor.write()); } @Test @@ -328,7 +328,6 @@ public void writeAssignmentExprImports_concreteAndNestedGenerics() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(7), "import static java.util.Map.Entry;\n\n", @@ -337,7 +336,8 @@ public void writeAssignmentExprImports_concreteAndNestedGenerics() { "import com.google.api.generator.engine.ast.ClassDefinition;\n", "import java.util.ArrayList;\n", "import java.util.List;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -370,12 +370,12 @@ public void writeAssignmentExprImports_static() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import static java.util.Map.Entry;\n\n", "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.AstNode;\n\n")); + "import com.google.api.generator.engine.ast.AstNode;\n\n"), + writerVisitor.write()); } @Test @@ -408,12 +408,12 @@ public void writeAssignmentExprImports_notStatic() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", "import com.google.api.generator.engine.ast.AstNode;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -428,11 +428,11 @@ public void writeCastExprImports() { .build(); castExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -471,12 +471,12 @@ public void importFromVaporAndConcreteReferences() { varExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.MethodDefinition;\n", "import java.util.HashMap;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -489,8 +489,8 @@ public void writeVariableExprImports_basic() { VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(1), "import com.google.api.generator.engine.ast.Expr;\n\n")); + String.format(createLines(1), "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -509,11 +509,11 @@ public void writeVariableExprImports_staticReference() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.TypeNode;\n\n")); + "import com.google.api.generator.engine.ast.TypeNode;\n\n"), + writerVisitor.write()); } @Test @@ -531,7 +531,7 @@ public void writeVariableExprImports_wildcardType() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.util.List;\n\n"); + assertEquals("import java.util.List;\n\n", writerVisitor.write()); } @Test @@ -553,11 +553,11 @@ public void writeVariableExprImport_wildcardTypeWithUpperBound() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -578,11 +578,11 @@ public void writeVariableExprImports_reference() { VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -611,12 +611,12 @@ public void writeVariableExprImports_nestedReference() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", "import com.google.api.generator.engine.ast.Expr;\n", - "import com.google.api.generator.engine.ast.VariableExpr;\n\n")); + "import com.google.api.generator.engine.ast.VariableExpr;\n\n"), + writerVisitor.write()); } @Test @@ -685,14 +685,14 @@ public void writeAnonymousClassExprImports() { .build(); anonymousClassExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(5), "import com.google.api.generator.engine.ast.MethodDefinition;\n", "import com.google.common.base.Function;\n", "import java.io.IOException;\n", "import java.util.HashMap;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -703,7 +703,7 @@ public void writeThrowExprImports_basic() { ThrowExpr throwExpr = ThrowExpr.builder().setType(exceptionTypes).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.io.IOException;\n\n"); + assertEquals("import java.io.IOException;\n\n", writerVisitor.write()); } @Test @@ -727,11 +727,11 @@ public void writeThrowExprImports_messageExpr() { throwExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import com.google.api.generator.engine.ast.IfStatement;\n\n")); + "import com.google.api.generator.engine.ast.IfStatement;\n\n"), + writerVisitor.write()); } @Test @@ -747,11 +747,11 @@ public void writeInstanceofExprImports_basic() { InstanceofExpr.builder().setExpr(variableExpr).setCheckType(exprType).build(); expr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -766,8 +766,8 @@ public void writeEnumRefExprImports_basic() { enumRefExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - "import static com.google.api.generator.engine.ast.TypeNode.TypeKind;\n\n"); + "import static com.google.api.generator.engine.ast.TypeNode.TypeKind;\n\n", + writerVisitor.write()); } @Test @@ -776,7 +776,7 @@ public void writeEnumRefExprImports_nested() { TypeNode.withReference(ConcreteReference.withClazz(TypeNode.TypeKind.class)); EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.TypeNode;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.TypeNode;\n\n", writerVisitor.write()); } @Test @@ -788,7 +788,7 @@ public void writeReturnExprImports_basic() { .setReturnType(TypeNode.withReference(ConcreteReference.withClazz(Expr.class))) .build()); returnExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -827,11 +827,11 @@ public void writeMethodDefinitionImports_templatedMixedNamesAndTypes() { .build(); methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -842,7 +842,7 @@ public void writeReferenceConstructorExprImports_basic() { ReferenceConstructorExpr referenceConstructorExpr = ReferenceConstructorExpr.superBuilder().setType(classType).build(); referenceConstructorExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.example.v1.Parent;\n\n"); + assertEquals("import com.google.example.v1.Parent;\n\n", writerVisitor.write()); } @Test @@ -877,7 +877,7 @@ public void writeArithmeticOperationExprImports() { ArithmeticOperationExpr arithmeticOperationExpr = ArithmeticOperationExpr.concatWithExprs(lhsExpr, rhsExpr); arithmeticOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -897,11 +897,11 @@ public void writeSynchronizedStatementImports_basicThis() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import java.util.Arrays;\n\n")); + "import java.util.Arrays;\n\n"), + writerVisitor.write()); } @Test @@ -920,7 +920,7 @@ public void writeSuperObjectValueImports() { .setReturnType(TypeNode.STRING) .build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.example.examples.v1.Student;\n\n"); + assertEquals("import com.google.example.examples.v1.Student;\n\n", writerVisitor.write()); } @Test @@ -945,11 +945,11 @@ public void writeSynchronizedStatementImports_basicVariableExpr() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -962,7 +962,7 @@ public void writeUnaryOperationExprImports_LogicalNot() { .build(); UnaryOperationExpr unaryOperationExpr = UnaryOperationExpr.logicalNotWithExpr(expr); unaryOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -975,7 +975,7 @@ public void writeUnaryOperationExprImports_PostIncrement() { .build(); UnaryOperationExpr unaryOperationExpr = UnaryOperationExpr.postfixIncrementWithExpr(expr); unaryOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -1002,11 +1002,11 @@ public void writeRelationalOperationExprImports() { RelationalOperationExpr.equalToWithExprs(lhsExpr, rhsExpr); relationalOperationExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.SomeClass;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -1024,8 +1024,8 @@ public void writeLogicalOperationExprImports() { LogicalOperationExpr.logicalAndWithExprs(lhsExpr, rhsExpr); logicalOperationExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - "import com.google.api.generator.engine.ast.UnaryOperationExpr;\n\n"); + "import com.google.api.generator.engine.ast.UnaryOperationExpr;\n\n", + writerVisitor.write()); } @Test @@ -1052,7 +1052,7 @@ public void writePackageInfoDefinitionImports() { .build(); packageInfo.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import javax.annotation.Generated;\n\n"); + assertEquals("import javax.annotation.Generated;\n\n", writerVisitor.write()); } /** =============================== HELPERS =============================== */ From 1e1a86b614fe7141cc851b436cc9da688c1d1b45 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:32:45 -0800 Subject: [PATCH 03/13] fix: add node validator to refactor/centralize null element checks --- .../engine/ast/MethodInvocationExpr.java | 19 +++++------- .../generator/engine/ast/NewObjectExpr.java | 7 ++--- .../generator/engine/ast/NodeValidator.java | 29 +++++++++++++++++++ .../engine/ast/ReferenceConstructorExpr.java | 8 ++--- 4 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 src/main/java/com/google/api/generator/engine/ast/NodeValidator.java diff --git a/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java b/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java index 96cc2d738b..c7970e2990 100644 --- a/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java @@ -19,7 +19,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; import javax.annotation.Nullable; @AutoValue @@ -117,17 +116,15 @@ public MethodInvocationExpr build() { || methodInvocationExpr.staticReferenceType() == null, "Only the expression reference or the static reference can be set, not both"); - Preconditions.checkState( - methodInvocationExpr.arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null expression in arguments %s for %s", - methodInvocationExpr.arguments(), methodInvocationExpr.methodIdentifier().name())); + NodeValidator.checkNoNullElements( + methodInvocationExpr.arguments(), + "arguments", + String.format("method invocation of %s", methodInvocationExpr.methodIdentifier().name())); - Preconditions.checkState( - methodInvocationExpr.generics().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null expression in generics %s for %s", - methodInvocationExpr.generics(), methodInvocationExpr.methodIdentifier().name())); + NodeValidator.checkNoNullElements( + methodInvocationExpr.generics(), + "generics", + String.format("method invocation of %s", methodInvocationExpr.methodIdentifier().name())); return methodInvocationExpr; } diff --git a/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java b/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java index 40e5275f92..50fbbed983 100644 --- a/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java @@ -20,7 +20,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; @AutoValue public abstract class NewObjectExpr implements Expr { @@ -70,10 +69,8 @@ public NewObjectExpr build() { Preconditions.checkState( TypeNode.isReferenceType(type()), "New object expression should be reference types."); - Preconditions.checkState( - arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null argument for the \"new\" constructor of %s", type().reference().name())); + NodeValidator.checkNoNullElements( + arguments(), "the \"new\" constructor", type().reference().name()); // Only the case where generics() is empty and isGeneric() is false, we set isGeneric() to // false. Otherwise, isGeneric() should be true. diff --git a/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java b/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java new file mode 100644 index 0000000000..5eba84e936 --- /dev/null +++ b/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java @@ -0,0 +1,29 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.api.generator.engine.ast; + +import com.google.common.base.Preconditions; +import java.util.Collection; +import java.util.Objects; + +public class NodeValidator { + public static void checkNoNullElements( + Collection collection, String fieldTypeName, String nodeContextInfo) { + Preconditions.checkState( + collection.stream().allMatch(e -> !Objects.isNull(e)), + String.format( + "Found null expression in %s %s for %s", fieldTypeName, collection, nodeContextInfo)); + } +} diff --git a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java index 405712c905..25940b0d3d 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java @@ -20,7 +20,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; @AutoValue public abstract class ReferenceConstructorExpr implements Expr { @@ -77,11 +76,8 @@ public ReferenceConstructorExpr build() { referenceConstructorExpr.type().isReferenceType(referenceConstructorExpr.type()), "A this/super constructor must have a reference type."); - Preconditions.checkState( - arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null argumentfor in the \"this/super\" initialization of %s", - type().reference().name())); + NodeValidator.checkNoNullElements( + arguments(), "the \"this\" or \"super\" initialization", type().reference().name()); return referenceConstructorExpr; } From 0a6662cbcfa49224e47b4fa7cbd1e5115db19abc Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 10 Nov 2020 13:28:28 -0800 Subject: [PATCH 04/13] test: update logging integration goldens with gapic.yaml --- test/integration/BUILD.bazel | 1 + .../logging/LoggingServiceV2StubSettings.java | 97 ++++++++++++++++++- 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index c159d127e2..54f2ca4d53 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -111,6 +111,7 @@ java_gapic_assembly_gradle_pkg( java_gapic_library( name = "logging_java_gapic", srcs = ["@com_google_googleapis//google/logging/v2:logging_proto_with_info"], + gapic_yaml = "@com_google_googleapis//google/logging/v2:logging_gapic.yaml", grpc_service_config = "@com_google_googleapis//google/logging/v2:logging_grpc_service_config.json", package = "google.logging.v2", service_yaml = "@com_google_googleapis//google/logging/v2:logging.yaml", diff --git a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java index 7e6456b3d4..622fb27f9e 100644 --- a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java +++ b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java @@ -24,6 +24,11 @@ import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; +import com.google.api.gax.batching.BatchingSettings; +import com.google.api.gax.batching.FlowControlSettings; +import com.google.api.gax.batching.FlowController; +import com.google.api.gax.batching.PartitionKey; +import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -33,6 +38,9 @@ import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; +import com.google.api.gax.rpc.BatchedRequestIssuer; +import com.google.api.gax.rpc.BatchingCallSettings; +import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.PageContext; import com.google.api.gax.rpc.PagedCallSettings; @@ -59,6 +67,7 @@ import com.google.logging.v2.WriteLogEntriesResponse; import com.google.protobuf.Empty; import java.io.IOException; +import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.Generated; @@ -95,7 +104,7 @@ public class LoggingServiceV2StubSettings extends StubSettings deleteLogSettings; - private final UnaryCallSettings + private final BatchingCallSettings writeLogEntriesSettings; private final PagedCallSettings< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -296,13 +305,73 @@ public ApiFuture getFuturePagedResponse( } }; + private static final BatchingDescriptor + WRITE_LOG_ENTRIES_BATCHING_DESC = + new BatchingDescriptor() { + @Override + public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) { + return new PartitionKey( + request.getLogName(), request.getResource(), request.getLabels()); + } + + @Override + public RequestBuilder getRequestBuilder() { + return new RequestBuilder() { + private WriteLogEntriesRequest.Builder builder; + + @Override + public void appendRequest(WriteLogEntriesRequest request) { + if (Objects.isNull(builder)) { + builder = request.toBuilder(); + } else { + builder.addAllEntries(request.getEntriesList()); + } + } + + @Override + public WriteLogEntriesRequest build() { + return builder.build(); + } + }; + } + + @Override + public void splitResponse( + WriteLogEntriesResponse batchResponse, + Collection> batch) { + for (BatchedRequestIssuer responder : batch) { + WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build(); + responder.setResponse(response); + } + } + + @Override + public void splitException( + Throwable throwable, + Collection> batch) { + for (BatchedRequestIssuer responder : batch) { + responder.setException(throwable); + } + } + + @Override + public long countElements(WriteLogEntriesRequest request) { + return request.getEntriesCount(); + } + + @Override + public long countBytes(WriteLogEntriesRequest request) { + return request.getSerializedSize(); + } + }; + /** Returns the object with the settings used for calls to deleteLog. */ public UnaryCallSettings deleteLogSettings() { return deleteLogSettings; } /** Returns the object with the settings used for calls to writeLogEntries. */ - public UnaryCallSettings + public BatchingCallSettings writeLogEntriesSettings() { return writeLogEntriesSettings; } @@ -410,7 +479,7 @@ protected LoggingServiceV2StubSettings(Builder settingsBuilder) throws IOExcepti public static class Builder extends StubSettings.Builder { private final ImmutableList> unaryMethodSettingsBuilders; private final UnaryCallSettings.Builder deleteLogSettings; - private final UnaryCallSettings.Builder + private final BatchingCallSettings.Builder writeLogEntriesSettings; private final PagedCallSettings.Builder< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -466,7 +535,9 @@ protected Builder(ClientContext clientContext) { super(clientContext); deleteLogSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - writeLogEntriesSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); + writeLogEntriesSettings = + BatchingCallSettings.newBuilder(WRITE_LOG_ENTRIES_BATCHING_DESC) + .setBatchingSettings(BatchingSettings.newBuilder().build()); listLogEntriesSettings = PagedCallSettings.newBuilder(LIST_LOG_ENTRIES_PAGE_STR_FACT); listMonitoredResourceDescriptorsSettings = PagedCallSettings.newBuilder(LIST_MONITORED_RESOURCE_DESCRIPTORS_PAGE_STR_FACT); @@ -518,6 +589,22 @@ private static Builder initDefaults(Builder builder) { .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); + builder + .writeLogEntriesSettings() + .setBatchingSettings( + BatchingSettings.newBuilder() + .setElementCountThreshold(1000L) + .setRequestByteThreshold(1048576L) + .setDelayThreshold(Duration.ofMillis(50L)) + .setFlowControlSettings( + FlowControlSettings.newBuilder() + .setMaxOutstandingElementCount(100000L) + .setMaxOutstandingRequestBytes(10485760L) + .setLimitExceededBehavior( + FlowController.LimitExceededBehavior.ThrowException) + .build()) + .build()); + builder .writeLogEntriesSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) @@ -563,7 +650,7 @@ public UnaryCallSettings.Builder deleteLogSettings() { } /** Returns the builder for the settings used for calls to writeLogEntries. */ - public UnaryCallSettings.Builder + public BatchingCallSettings.Builder writeLogEntriesSettings() { return writeLogEntriesSettings; } From e9cdf585c9018ee2170b9bd09d86f0849e05838d Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 10 Nov 2020 13:33:25 -0800 Subject: [PATCH 05/13] feat!: remove gapic.yaml from Bazel and input interface --- DEVELOPMENT.md | 16 ++++++++++------ rules_java_gapic/java_gapic.bzl | 4 ---- run.sh | 8 +------- test/integration/BUILD.bazel | 1 - 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index b6b5fb825f..93cd783e86 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -41,7 +41,6 @@ below are temporary and better ones will be coming. java_gapic_library2( name = "showcase_java_gapic", srcs = [":showcase_proto_with_info"], - # The gapic_yaml file is needed only for APIs that have batching configs. grpc_service_config = "showcase_grpc_service_config.json", package = "google.showcase.v1beta1", test_deps = [ @@ -100,21 +99,26 @@ below are temporary and better ones will be coming. bazel run //src/test/java/com/google/api/generator/engine:JavaCodeGeneratorTest ``` -- Update goldens files based on code generation in unit test, for example `JavaCodeGeneratorTest.java` +- Update goldens files based on code generation in unit test, for example + `JavaCodeGeneratorTest.java` ```sh bazel run //src/test/java/com/google/api/generator/engine:JavaCodeGeneratorTest_update ``` -- Run a single integration test for API like `Redis`, it generates Java source code using -the Java microgenerator and compares them with the goldens files in `test/integration/goldens/redis`. +- Run a single integration test for API like `Redis`, it generates Java source + code using the Java microgenerator and compares them with the goldens files + in `test/integration/goldens/redis`. ```sh bazel test //test/integration:redis ``` -- Update goldens files based on code generation in integration test, for example `Redis`. It generates Java source code using the Java microgenerator and overwrites the goldens files in `test/integration/goldens/redis` based on code generation. +- Update goldens files based on code generation in integration test, for + example `Redis`. It generates Java source code using the Java microgenerator + and overwrites the goldens files in `test/integration/goldens/redis` based + on code generation. ```sh bazel run //test/integration:redis_update - ``` \ No newline at end of file + ``` diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index aaf83c62e6..7e6442b8a4 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -83,7 +83,6 @@ def java_gapic_library( package = None, service_yaml = None, grpc_service_config = None, - gapic_yaml = None, deps = [], test_deps = [], **kwargs): @@ -92,9 +91,6 @@ def java_gapic_library( if grpc_service_config: file_args_dict[grpc_service_config] = "grpc-service-config" - if gapic_yaml: - file_args_dict[gapic_yaml] = "gapic-config" - # Currently a no-op. if service_yaml: file_args_dict[service_yaml] = "gapic-service-config" diff --git a/run.sh b/run.sh index 6adb1f4771..efe66747a0 100755 --- a/run.sh +++ b/run.sh @@ -18,7 +18,6 @@ DEFINE_string --alias=s service_config '' 'Path to the JSON service config' # Optional flags. DEFINE_bool --alias=c use_cached false 'If true, does not rebuild the plugin.' DEFINE_string --alias=o out '/tmp/test' 'Output directory' -DEFINE_string gapic_config '' 'Path to the config ending in gapic.yaml. Optional' gbash::init_google "$@" @@ -78,11 +77,6 @@ if [ -n "$FLAGS_service_config" ] then SERVICE_CONFIG_OPT="grpc-service-config=$FLAGS_service_config" fi -GAPIC_CONFIG_OPT="" -if [ -n "$FLAGS_gapic_config" ] -then - GAPIC_CONFIG_OPT="gapic-config=$FLAGS_gapic_config" -fi # Run protoc. protoc -I="${PROTOC_INCLUDE_DIR}" -I="${FLAGS_googleapis}" -I="${FLAGS_protos}" \ @@ -90,7 +84,7 @@ protoc -I="${PROTOC_INCLUDE_DIR}" -I="${FLAGS_googleapis}" -I="${FLAGS_protos}" --include_source_info \ --plugin=bazel-bin/protoc-gen-java_gapic ${FLAGS_protos}/*.proto \ --java_gapic_out="${FLAGS_out}" \ - --java_gapic_opt="${SERVICE_CONFIG_OPT},${GAPIC_CONFIG_OPT}" \ + --java_gapic_opt="${SERVICE_CONFIG_OPT}" \ --experimental_allow_proto3_optional echo_success "Output files written to ${FLAGS_out}" diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index 54f2ca4d53..c159d127e2 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -111,7 +111,6 @@ java_gapic_assembly_gradle_pkg( java_gapic_library( name = "logging_java_gapic", srcs = ["@com_google_googleapis//google/logging/v2:logging_proto_with_info"], - gapic_yaml = "@com_google_googleapis//google/logging/v2:logging_gapic.yaml", grpc_service_config = "@com_google_googleapis//google/logging/v2:logging_grpc_service_config.json", package = "google.logging.v2", service_yaml = "@com_google_googleapis//google/logging/v2:logging.yaml", From 827049db0d6b8f949d47135cc15f376f75cfd688 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 10 Nov 2020 14:59:35 -0800 Subject: [PATCH 06/13] feat!: remove all per-RPC batching codegen logic --- run.sh | 1 - .../composer/BatchingDescriptorComposer.java | 557 ------------------ .../gapic/composer/RetrySettingsComposer.java | 109 ---- .../ServiceStubSettingsClassComposer.java | 134 +---- .../gapic/model/GapicBatchingSettings.java | 99 ---- .../gapic/model/GapicServiceConfig.java | 35 +- .../BatchingSettingsConfigParser.java | 190 ------ .../generator/gapic/protoparser/Parser.java | 9 +- .../protoparser/PluginArgumentParser.java | 10 - .../protoparser/ServiceConfigParser.java | 7 +- .../api/generator/gapic/composer/BUILD.bazel | 3 - .../BatchingDescriptorComposerTest.java | 193 ------ .../composer/RetrySettingsComposerTest.java | 187 +----- .../ServiceStubSettingsClassComposerTest.java | 28 +- ...DescriptorComposerTestNoSubresponse.golden | 53 -- ...ngDescriptorComposerTestSubresponse.golden | 59 -- .../LoggingServiceV2StubSettings.golden | 97 +-- .../goldens/PublisherStubSettings.golden | 100 +--- .../gapic/model/GapicServiceConfigTest.java | 84 +-- .../generator/gapic/protoparser/BUILD.bazel | 2 - .../BatchingSettingsConfigParserTest.java | 114 ---- .../protoparser/PluginArgumentParserTest.java | 90 +-- .../api/generator/gapic/testdata/BUILD.bazel | 5 - .../gapic/testdata/datastore_gapic.yaml | 38 -- .../gapic/testdata/logging_gapic.yaml | 79 --- .../gapic/testdata/pubsub_gapic.yaml | 296 ---------- .../gapic/testdata/showcase_gapic.yaml | 22 - 27 files changed, 54 insertions(+), 2547 deletions(-) delete mode 100644 src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java delete mode 100644 src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java delete mode 100644 src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java delete mode 100644 src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java delete mode 100644 src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestNoSubresponse.golden delete mode 100644 src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestSubresponse.golden delete mode 100644 src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java delete mode 100644 src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml delete mode 100644 src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml delete mode 100644 src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml delete mode 100644 src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml diff --git a/run.sh b/run.sh index 6adb1f4771..b766ece3c2 100755 --- a/run.sh +++ b/run.sh @@ -18,7 +18,6 @@ DEFINE_string --alias=s service_config '' 'Path to the JSON service config' # Optional flags. DEFINE_bool --alias=c use_cached false 'If true, does not rebuild the plugin.' DEFINE_string --alias=o out '/tmp/test' 'Output directory' -DEFINE_string gapic_config '' 'Path to the config ending in gapic.yaml. Optional' gbash::init_google "$@" diff --git a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java b/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java deleted file mode 100644 index 4202809ce1..0000000000 --- a/src/main/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposer.java +++ /dev/null @@ -1,557 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.api.generator.gapic.composer; - -import com.google.api.gax.batching.PartitionKey; -import com.google.api.gax.batching.RequestBuilder; -import com.google.api.gax.rpc.BatchedRequestIssuer; -import com.google.api.gax.rpc.BatchingDescriptor; -import com.google.api.generator.engine.ast.AnonymousClassExpr; -import com.google.api.generator.engine.ast.AssignmentExpr; -import com.google.api.generator.engine.ast.ConcreteReference; -import com.google.api.generator.engine.ast.Expr; -import com.google.api.generator.engine.ast.ExprStatement; -import com.google.api.generator.engine.ast.ForStatement; -import com.google.api.generator.engine.ast.GeneralForStatement; -import com.google.api.generator.engine.ast.IfStatement; -import com.google.api.generator.engine.ast.MethodDefinition; -import com.google.api.generator.engine.ast.MethodInvocationExpr; -import com.google.api.generator.engine.ast.NewObjectExpr; -import com.google.api.generator.engine.ast.PrimitiveValue; -import com.google.api.generator.engine.ast.Reference; -import com.google.api.generator.engine.ast.ScopeNode; -import com.google.api.generator.engine.ast.Statement; -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.Variable; -import com.google.api.generator.engine.ast.VariableExpr; -import com.google.api.generator.gapic.model.Field; -import com.google.api.generator.gapic.model.GapicBatchingSettings; -import com.google.api.generator.gapic.model.Message; -import com.google.api.generator.gapic.model.Method; -import com.google.api.generator.gapic.utils.JavaStyle; -import com.google.common.base.Preconditions; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; - -public class BatchingDescriptorComposer { - private static final String BATCHING_DESC_PATTERN = "%s_BATCHING_DESC"; - - private static final Reference BATCHING_DESCRIPTOR_REF = - ConcreteReference.withClazz(BatchingDescriptor.class); - private static final Reference REQUEST_BUILDER_REF = - ConcreteReference.withClazz(RequestBuilder.class); - private static final Reference BATCHED_REQUEST_ISSUER_REF = - ConcreteReference.withClazz(BatchedRequestIssuer.class); - - private static final TypeNode PARTITION_KEY_TYPE = toType(PartitionKey.class); - - private static final String ADD_ALL_METHOD_PATTERN = "addAll%s"; - private static final String BATCH_FOO_INDEX_PATTERN = "batch%sIndex"; - private static final String GET_LIST_METHOD_PATTERN = "get%sList"; - private static final String GET_COUNT_METHOD_PATTERN = "get%sCount"; - - public static Expr createBatchingDescriptorFieldDeclExpr( - Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { - List javaMethods = new ArrayList<>(); - javaMethods.add(createGetBatchPartitionKeyMethod(method, batchingSettings, messageTypes)); - javaMethods.add(createGetRequestBuilderMethod(method, batchingSettings)); - javaMethods.add(createSplitResponseMethod(method, batchingSettings, messageTypes)); - javaMethods.add(createSplitExceptionMethod(method)); - javaMethods.add(createCountElementsMethod(method, batchingSettings)); - javaMethods.add(createCountByteSMethod(method)); - - TypeNode batchingDescriptorType = - toType(BATCHING_DESCRIPTOR_REF, method.inputType(), method.outputType()); - AnonymousClassExpr batchingDescriptorClassExpr = - AnonymousClassExpr.builder() - .setType(batchingDescriptorType) - .setMethods(javaMethods) - .build(); - - String varName = - String.format(BATCHING_DESC_PATTERN, JavaStyle.toUpperSnakeCase(method.name())); - return AssignmentExpr.builder() - .setVariableExpr( - VariableExpr.builder() - .setVariable( - Variable.builder().setType(batchingDescriptorType).setName(varName).build()) - .setIsDecl(true) - .setScope(ScopeNode.PRIVATE) - .setIsStatic(true) - .setIsFinal(true) - .build()) - .setValueExpr(batchingDescriptorClassExpr) - .build(); - } - - private static MethodDefinition createGetBatchPartitionKeyMethod( - Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { - String methodInputTypeName = method.inputType().reference().name(); - Message inputMessage = messageTypes.get(methodInputTypeName); - Preconditions.checkNotNull( - inputMessage, - String.format( - "Message %s not found for RPC method %s", methodInputTypeName, method.name())); - - VariableExpr requestVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(method.inputType()).setName("request").build()); - - List partitionKeyArgExprs = new ArrayList<>(); - for (String discriminatorFieldName : batchingSettings.discriminatorFieldNames()) { - Preconditions.checkNotNull( - inputMessage.fieldMap().get(discriminatorFieldName), - String.format( - "Batching discriminator field %s not found in message %s", - discriminatorFieldName, inputMessage.name())); - String getterMethodName = - String.format("get%s", JavaStyle.toUpperCamelCase(discriminatorFieldName)); - partitionKeyArgExprs.add( - MethodInvocationExpr.builder() - .setExprReferenceExpr(requestVarExpr) - .setMethodName(getterMethodName) - .build()); - } - Expr returnExpr = - NewObjectExpr.builder() - .setType(PARTITION_KEY_TYPE) - .setArguments(partitionKeyArgExprs) - .build(); - - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(PARTITION_KEY_TYPE) - .setName("getBatchPartitionKey") - .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) - .setReturnExpr(returnExpr) - .build(); - } - - private static MethodDefinition createGetRequestBuilderMethod( - Method method, GapicBatchingSettings batchingSettings) { - TypeNode builderType = - TypeNode.withReference( - VaporReference.builder() - .setEnclosingClassNames(method.inputType().reference().name()) - .setName("Builder") - .setPakkage(method.inputType().reference().pakkage()) - .build()); - - VariableExpr builderVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(builderType).setName("builder").build()); - VariableExpr requestVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(method.inputType()).setName("request").build()); - - Expr toBuilderExpr = - AssignmentExpr.builder() - .setVariableExpr(builderVarExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(requestVarExpr) - .setMethodName("toBuilder") - .setReturnType(builderType) - .build()) - .build(); - - String upperBatchedFieldName = JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName()); - String getFooListMethodName = String.format(GET_LIST_METHOD_PATTERN, upperBatchedFieldName); - Expr getFooListExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(requestVarExpr) - .setMethodName(getFooListMethodName) - .build(); - - String addAllMethodName = String.format(ADD_ALL_METHOD_PATTERN, upperBatchedFieldName); - Expr addAllExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(builderVarExpr) - .setMethodName(addAllMethodName) - .setArguments(getFooListExpr) - .build(); - - MethodDefinition appendRequestMethod = - MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(TypeNode.VOID) - .setName("appendRequest") - .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) - .setBody( - Arrays.asList( - IfStatement.builder() - .setConditionExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType(toType(Objects.class)) - .setMethodName("isNull") - .setArguments(builderVarExpr) - .setReturnType(TypeNode.BOOLEAN) - .build()) - .setBody(Arrays.asList(ExprStatement.withExpr(toBuilderExpr))) - .setElseBody(Arrays.asList(ExprStatement.withExpr(addAllExpr))) - .build())) - .build(); - - MethodDefinition buildMethod = - MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(method.inputType()) - .setName("build") - .setReturnExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(builderVarExpr) - .setMethodName("build") - .setReturnType(method.inputType()) - .build()) - .build(); - - TypeNode anonClassType = toType(REQUEST_BUILDER_REF, method.inputType()); - AnonymousClassExpr requestBuilderAnonClassExpr = - AnonymousClassExpr.builder() - .setType(anonClassType) - .setStatements( - Arrays.asList( - ExprStatement.withExpr( - builderVarExpr - .toBuilder() - .setIsDecl(true) - .setScope(ScopeNode.PRIVATE) - .build()))) - .setMethods(Arrays.asList(appendRequestMethod, buildMethod)) - .build(); - - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(anonClassType) - .setName("getRequestBuilder") - .setReturnExpr(requestBuilderAnonClassExpr) - .build(); - } - - private static MethodDefinition createSplitResponseMethod( - Method method, GapicBatchingSettings batchingSettings, Map messageTypes) { - VariableExpr batchResponseVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(method.outputType()).setName("batchResponse").build()); - - TypeNode batchedRequestIssuerType = toType(BATCHED_REQUEST_ISSUER_REF, method.outputType()); - TypeNode batchVarType = - TypeNode.withReference( - ConcreteReference.builder() - .setClazz(Collection.class) - .setGenerics( - Arrays.asList( - ConcreteReference.wildcardWithUpperBound( - batchedRequestIssuerType.reference()))) - .build()); - VariableExpr batchVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(batchVarType).setName("batch").build()); - - VariableExpr responderVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(batchedRequestIssuerType).setName("responder").build()); - - String upperCamelBatchedFieldName = - JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName()); - VariableExpr batchMessageIndexVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(TypeNode.INT).setName("batchMessageIndex").build()); - - VariableExpr subresponseElementsVarExpr = null; - boolean hasSubresponseField = batchingSettings.subresponseFieldName() != null; - - List outerForBody = new ArrayList<>(); - if (hasSubresponseField) { - Message outputMessage = messageTypes.get(method.outputType().reference().name()); - Preconditions.checkNotNull( - outputMessage, String.format("Output message not found for RPC %s", method.name())); - - Field subresponseElementField = - outputMessage.fieldMap().get(batchingSettings.subresponseFieldName()); - Preconditions.checkNotNull( - subresponseElementField, - String.format( - "Subresponse field %s not found in message %s", - batchingSettings.subresponseFieldName(), outputMessage.name())); - TypeNode subresponseElementType = subresponseElementField.type(); - subresponseElementsVarExpr = - VariableExpr.withVariable( - Variable.builder() - .setType(subresponseElementType) - .setName("subresponseElements") - .build()); - - VariableExpr subresponseCountVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(TypeNode.LONG).setName("subresponseCount").build()); - - outerForBody.add( - ExprStatement.withExpr( - AssignmentExpr.builder() - .setVariableExpr(subresponseElementsVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - NewObjectExpr.builder() - .setType( - TypeNode.withReference(ConcreteReference.withClazz(ArrayList.class))) - .setIsGeneric(true) - .build()) - .build())); - - String getFooCountMethodName = "getMessageCount"; - outerForBody.add( - ExprStatement.withExpr( - AssignmentExpr.builder() - .setVariableExpr(subresponseCountVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(responderVarExpr) - .setMethodName(getFooCountMethodName) - .setReturnType(subresponseCountVarExpr.type()) - .build()) - .build())); - - List innerSubresponseForExprs = new ArrayList<>(); - String getSubresponseFieldMethodName = - String.format( - "get%s", JavaStyle.toUpperCamelCase(batchingSettings.subresponseFieldName())); - Expr addMethodArgExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchResponseVarExpr) - .setMethodName(getSubresponseFieldMethodName) - .setArguments(UnaryOperationExpr.postfixIncrementWithExpr(batchMessageIndexVarExpr)) - .build(); - innerSubresponseForExprs.add( - MethodInvocationExpr.builder() - .setExprReferenceExpr(subresponseElementsVarExpr) - .setMethodName("add") - .setArguments(addMethodArgExpr) - .build()); - // TODO(miraleung): Increment batchMessageIndexVarExpr. - - VariableExpr forIndexVarExpr = - VariableExpr.builder() - .setIsDecl(true) - .setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build()) - .build(); - ValueExpr initValueExpr = - ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()); - GeneralForStatement innerSubresponseForStatement = - GeneralForStatement.incrementWith( - forIndexVarExpr, - initValueExpr, - subresponseCountVarExpr, - innerSubresponseForExprs.stream() - .map(e -> ExprStatement.withExpr(e)) - .collect(Collectors.toList())); - - outerForBody.add(innerSubresponseForStatement); - } - - TypeNode responseType = method.outputType(); - Expr responseBuilderExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(responseType) - .setMethodName("newBuilder") - .build(); - if (hasSubresponseField) { - Preconditions.checkNotNull( - subresponseElementsVarExpr, - String.format( - "subresponseElements variable should not be null for method %s", method.name())); - - responseBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(responseBuilderExpr) - .setMethodName( - String.format( - "addAll%s", - JavaStyle.toUpperCamelCase(batchingSettings.subresponseFieldName()))) - .setArguments(subresponseElementsVarExpr) - .build(); - } - responseBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(responseBuilderExpr) - .setMethodName("build") - .setReturnType(responseType) - .build(); - - VariableExpr responseVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(responseType).setName("response").build()); - outerForBody.add( - ExprStatement.withExpr( - AssignmentExpr.builder() - .setVariableExpr(responseVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr(responseBuilderExpr) - .build())); - - outerForBody.add( - ExprStatement.withExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(responderVarExpr) - .setMethodName("setResponse") - .setArguments(responseVarExpr) - .build())); - - ForStatement outerForStatement = - ForStatement.builder() - .setLocalVariableExpr(responderVarExpr.toBuilder().setIsDecl(true).build()) - .setCollectionExpr(batchVarExpr) - .setBody(outerForBody) - .build(); - - List bodyStatements = new ArrayList<>(); - if (hasSubresponseField) { - bodyStatements.add( - ExprStatement.withExpr( - AssignmentExpr.builder() - .setVariableExpr(batchMessageIndexVarExpr.toBuilder().setIsDecl(true).build()) - .setValueExpr( - ValueExpr.withValue( - PrimitiveValue.builder().setType(TypeNode.INT).setValue("0").build())) - .build())); - } - bodyStatements.add(outerForStatement); - - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(TypeNode.VOID) - .setName("splitResponse") - .setArguments( - Arrays.asList(batchResponseVarExpr, batchVarExpr).stream() - .map(v -> v.toBuilder().setIsDecl(true).build()) - .collect(Collectors.toList())) - .setBody(bodyStatements) - .build(); - } - - private static MethodDefinition createSplitExceptionMethod(Method method) { - VariableExpr throwableVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(toType(Throwable.class)).setName("throwable").build()); - - TypeNode batchedRequestIssuerType = toType(BATCHED_REQUEST_ISSUER_REF, method.outputType()); - TypeNode batchVarType = - TypeNode.withReference( - ConcreteReference.builder() - .setClazz(Collection.class) - .setGenerics( - Arrays.asList( - ConcreteReference.wildcardWithUpperBound( - batchedRequestIssuerType.reference()))) - .build()); - VariableExpr batchVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(batchVarType).setName("batch").build()); - VariableExpr responderVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(batchedRequestIssuerType).setName("responder").build()); - - ForStatement forStatement = - ForStatement.builder() - .setLocalVariableExpr(responderVarExpr.toBuilder().setIsDecl(true).build()) - .setCollectionExpr(batchVarExpr) - .setBody( - Arrays.asList( - ExprStatement.withExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(responderVarExpr) - .setMethodName("setException") - .setArguments(throwableVarExpr) - .build()))) - .build(); - - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(TypeNode.VOID) - .setName("splitException") - .setArguments( - Arrays.asList(throwableVarExpr, batchVarExpr).stream() - .map(v -> v.toBuilder().setIsDecl(true).build()) - .collect(Collectors.toList())) - .setBody(Arrays.asList(forStatement)) - .build(); - } - - private static MethodDefinition createCountElementsMethod( - Method method, GapicBatchingSettings batchingSettings) { - String getFooCountMethodName = - String.format( - GET_COUNT_METHOD_PATTERN, - JavaStyle.toUpperCamelCase(batchingSettings.batchedFieldName())); - VariableExpr requestVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(method.inputType()).setName("request").build()); - - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(TypeNode.LONG) - .setName("countElements") - .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) - .setReturnExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(requestVarExpr) - .setMethodName(getFooCountMethodName) - .setReturnType(TypeNode.LONG) - .build()) - .build(); - } - - private static MethodDefinition createCountByteSMethod(Method method) { - VariableExpr requestVarExpr = - VariableExpr.withVariable( - Variable.builder().setType(method.inputType()).setName("request").build()); - return MethodDefinition.builder() - .setIsOverride(true) - .setScope(ScopeNode.PUBLIC) - .setReturnType(TypeNode.LONG) - .setName("countBytes") - .setArguments(requestVarExpr.toBuilder().setIsDecl(true).build()) - .setReturnExpr( - MethodInvocationExpr.builder() - .setExprReferenceExpr(requestVarExpr) - .setMethodName("getSerializedSize") - .setReturnType(TypeNode.LONG) - .build()) - .build(); - } - - private static TypeNode toType(Class clazz) { - return TypeNode.withReference(ConcreteReference.withClazz(clazz)); - } - - private static TypeNode toType(Reference reference, TypeNode... types) { - return TypeNode.withReference( - reference.copyAndSetGenerics( - Arrays.asList(types).stream().map(t -> t.reference()).collect(Collectors.toList()))); - } -} diff --git a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java index 10e722eba1..22608b9f0d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/RetrySettingsComposer.java @@ -14,9 +14,6 @@ package com.google.api.generator.gapic.composer; -import com.google.api.gax.batching.BatchingSettings; -import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.batching.FlowController; import com.google.api.gax.grpc.ProtoOperationTransformers; import com.google.api.gax.longrunning.OperationSnapshot; import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; @@ -37,7 +34,6 @@ import com.google.api.generator.engine.ast.ValueExpr; import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicRetrySettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Method; @@ -369,108 +365,6 @@ public static Expr createLroSettingsBuilderExpr( return builderSettingsExpr; } - public static Expr createBatchingBuilderSettingsExpr( - String settingsGetterMethodName, - GapicBatchingSettings batchingSettings, - VariableExpr builderVarExpr) { - - Expr batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("BatchingSettings")) - .setMethodName("newBuilder") - .build(); - - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("setElementCountThreshold") - .setArguments(toValExpr(batchingSettings.elementCountThreshold())) - .build(); - - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("setRequestByteThreshold") - .setArguments(toValExpr(batchingSettings.requestByteThreshold())) - .build(); - - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("setDelayThreshold") - .setArguments( - createDurationOfMillisExpr(toValExpr(batchingSettings.delayThresholdMillis()))) - .build(); - - // FlowControlSettings. - Expr flowControlSettingsExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("FlowControlSettings")) - .setMethodName("newBuilder") - .build(); - if (batchingSettings.flowControlElementLimit() != null) { - flowControlSettingsExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(flowControlSettingsExpr) - .setMethodName("setMaxOutstandingElementCount") - .setArguments(toValExpr(batchingSettings.flowControlElementLimit())) - .build(); - } - if (batchingSettings.flowControlByteLimit() != null) { - flowControlSettingsExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(flowControlSettingsExpr) - .setMethodName("setMaxOutstandingRequestBytes") - .setArguments(toValExpr(batchingSettings.flowControlByteLimit())) - .build(); - } - flowControlSettingsExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(flowControlSettingsExpr) - .setMethodName("setLimitExceededBehavior") - .setArguments( - EnumRefExpr.builder() - .setType(STATIC_TYPES.get("LimitExceededBehavior")) - .setName( - JavaStyle.toUpperCamelCase( - batchingSettings - .flowControlLimitExceededBehavior() - .name() - .toLowerCase())) - .build()) - .build(); - flowControlSettingsExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(flowControlSettingsExpr) - .setMethodName("build") - .build(); - - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("setFlowControlSettings") - .setArguments(flowControlSettingsExpr) - .build(); - - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("build") - .build(); - - // Put everything together. - Expr builderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(builderVarExpr) - .setMethodName(settingsGetterMethodName) - .build(); - return MethodInvocationExpr.builder() - .setExprReferenceExpr(builderExpr) - .setMethodName("setBatchingSettings") - .setArguments(batchingSettingsBuilderExpr) - .build(); - } - private static Expr createRetryCodeDefinitionExpr( String codeName, List retryCodes, VariableExpr definitionsVarExpr) { // Construct something like `definitions.put("code_name", @@ -706,10 +600,7 @@ private static MethodInvocationExpr createDurationOfMillisExpr(ValueExpr valExpr private static Map createStaticTypes() { List concreteClazzes = Arrays.asList( - BatchingSettings.class, org.threeten.bp.Duration.class, - FlowControlSettings.class, - FlowController.LimitExceededBehavior.class, ImmutableMap.class, ImmutableSet.class, Lists.class, 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 ede0311809..c31bec5317 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 @@ -18,11 +18,6 @@ import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; -import com.google.api.gax.batching.BatchingSettings; -import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.batching.FlowController.LimitExceededBehavior; -import com.google.api.gax.batching.PartitionKey; -import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -35,9 +30,6 @@ import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; -import com.google.api.gax.rpc.BatchedRequestIssuer; -import com.google.api.gax.rpc.BatchingCallSettings; -import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.OperationCallSettings; import com.google.api.gax.rpc.PageContext; @@ -82,7 +74,6 @@ import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.gapic.model.Field; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Message; @@ -221,12 +212,8 @@ private static Map createMethodSettingsClassMemberVarExprs Map varExprs = new LinkedHashMap<>(); // Creates class variables Settings, e.g. echoSettings. - // TODO(miraleung): Handle batching here. for (Method method : service.methods()) { - boolean hasBatchingSettings = - !Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method); - TypeNode settingsType = - getCallSettingsType(method, types, hasBatchingSettings, isNestedClass); + TypeNode settingsType = getCallSettingsType(method, types, isNestedClass); String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name())); varExprs.put( varName, @@ -313,20 +300,6 @@ private static List createClassStatements( statements.add(EMPTY_LINE_STATEMENT); } - for (Method method : service.methods()) { - Optional batchingSettingOpt = - Objects.isNull(serviceConfig) - ? Optional.empty() - : serviceConfig.getBatchingSetting(service, method); - if (batchingSettingOpt.isPresent()) { - statements.add( - exprToStatementFn.apply( - BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( - method, batchingSettingOpt.get(), messageTypes))); - } - statements.add(EMPTY_LINE_STATEMENT); - } - return statements; } @@ -1261,22 +1234,6 @@ private static MethodDefinition createNestedClassInitDefaultsMethod( if (streamKind.equals(Method.Stream.CLIENT) || streamKind.equals(Method.Stream.BIDI)) { continue; } - if (!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method)) { - Optional batchingSettingOpt = - serviceConfig.getBatchingSetting(service, method); - Preconditions.checkState( - batchingSettingOpt.isPresent(), - String.format( - "No batching setting found for service %s, method %s", - service.name(), method.name())); - String settingsGetterMethodName = - String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name())); - bodyStatements.add( - ExprStatement.withExpr( - RetrySettingsComposer.createBatchingBuilderSettingsExpr( - settingsGetterMethodName, batchingSettingOpt.get(), builderVarExpr))); - bodyStatements.add(EMPTY_LINE_STATEMENT); - } bodyStatements.add( ExprStatement.withExpr( @@ -1352,8 +1309,6 @@ private static List createNestedClassConstructorMethods( .build()); Reference pagedSettingsBuilderRef = ConcreteReference.withClazz(PagedCallSettings.Builder.class); - Reference batchingSettingsBuilderRef = - ConcreteReference.withClazz(BatchingCallSettings.Builder.class); Reference unaryCallSettingsBuilderRef = ConcreteReference.withClazz(UnaryCallSettings.Builder.class); Function isUnaryCallSettingsBuilderFn = @@ -1363,9 +1318,6 @@ private static List createNestedClassConstructorMethods( .equals(unaryCallSettingsBuilderRef); Function isPagedCallSettingsBuilderFn = t -> t.reference().copyAndSetGenerics(ImmutableList.of()).equals(pagedSettingsBuilderRef); - Function isBatchingCallSettingsBuilderFn = - t -> - t.reference().copyAndSetGenerics(ImmutableList.of()).equals(batchingSettingsBuilderRef); Function builderToCallSettingsFn = t -> TypeNode.withReference( @@ -1396,61 +1348,20 @@ private static List createNestedClassConstructorMethods( String methodName = getMethodNameFromSettingsVarName(e.getKey()); if (!isPagedCallSettingsBuilderFn.apply(varType)) { - if (!isBatchingCallSettingsBuilderFn.apply(varType)) { - boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType); - Expr builderExpr = - AssignmentExpr.builder() - .setVariableExpr(varExpr) - .setValueExpr( - MethodInvocationExpr.builder() - .setStaticReferenceType( - builderToCallSettingsFn.apply(varExpr.type())) - .setMethodName( - isUnaryCallSettings - ? "newUnaryCallSettingsBuilder" - : "newBuilder") - .setReturnType(varExpr.type()) - .build()) - .build(); - return ExprStatement.withExpr(builderExpr); - } - Expr newBatchingSettingsExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("BatchingSettings")) - .setMethodName("newBuilder") - .build(); - newBatchingSettingsExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(newBatchingSettingsExpr) - .setMethodName("build") - .build(); - - String batchingDescVarName = - String.format( - BATCHING_DESC_PATTERN, JavaStyle.toUpperSnakeCase(methodName)); - Expr batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(builderToCallSettingsFn.apply(varType)) - .setMethodName("newBuilder") - .setArguments( - VariableExpr.withVariable( - Variable.builder() - .setType(STATIC_TYPES.get("BatchingDescriptor")) - .setName(batchingDescVarName) - .build())) - .build(); - batchingSettingsBuilderExpr = - MethodInvocationExpr.builder() - .setExprReferenceExpr(batchingSettingsBuilderExpr) - .setMethodName("setBatchingSettings") - .setArguments(newBatchingSettingsExpr) - .setReturnType(varType) - .build(); - + boolean isUnaryCallSettings = isUnaryCallSettingsBuilderFn.apply(varType); Expr builderExpr = AssignmentExpr.builder() .setVariableExpr(varExpr) - .setValueExpr(batchingSettingsBuilderExpr) + .setValueExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType( + builderToCallSettingsFn.apply(varExpr.type())) + .setMethodName( + isUnaryCallSettings + ? "newUnaryCallSettingsBuilder" + : "newBuilder") + .setReturnType(varExpr.type()) + .build()) .build(); return ExprStatement.withExpr(builderExpr); } @@ -1496,8 +1407,7 @@ private static List createNestedClassConstructorMethods( .filter( v -> isUnaryCallSettingsBuilderFn.apply(v.type()) - || isPagedCallSettingsBuilderFn.apply(v.type()) - || isBatchingCallSettingsBuilderFn.apply(v.type())) + || isPagedCallSettingsBuilderFn.apply(v.type())) .collect(Collectors.toList())) .setReturnType(NESTED_UNARY_METHOD_SETTINGS_BUILDERS_VAR_EXPR.type()) .build()) @@ -1779,15 +1689,10 @@ private static Map createStaticTypes() { ApiClientHeaderProvider.class, ApiFunction.class, ApiFuture.class, - BatchedRequestIssuer.class, - BatchingCallSettings.class, - BatchingDescriptor.class, - BatchingSettings.class, BetaApi.class, ClientContext.class, Duration.class, Empty.class, - FlowControlSettings.class, GaxGrpcProperties.class, GaxProperties.class, Generated.class, @@ -1799,7 +1704,6 @@ private static Map createStaticTypes() { ImmutableSet.class, InstantiatingExecutorProvider.class, InstantiatingGrpcChannelProvider.class, - LimitExceededBehavior.class, List.class, Lists.class, MonitoredResourceDescriptor.class, @@ -1811,9 +1715,7 @@ private static Map createStaticTypes() { PagedCallSettings.class, PagedListDescriptor.class, PagedListResponseFactory.class, - PartitionKey.class, ProtoOperationTransformers.class, - RequestBuilder.class, RetrySettings.class, ServerStreamingCallSettings.class, StatusCode.class, @@ -1969,10 +1871,7 @@ private static String getGrpcServiceStubTypeName(String serviceName) { } private static TypeNode getCallSettingsType( - Method method, - Map types, - boolean isBatchingSettings, - final boolean isSettingsBuilder) { + Method method, Map types, final boolean isSettingsBuilder) { Function typeMakerFn = clz -> TypeNode.withReference(ConcreteReference.withClazz(clz)); // Default: No streaming. @@ -1982,11 +1881,6 @@ private static TypeNode getCallSettingsType( isSettingsBuilder ? PagedCallSettings.Builder.class : PagedCallSettings.class) : typeMakerFn.apply( isSettingsBuilder ? UnaryCallSettings.Builder.class : UnaryCallSettings.class); - if (isBatchingSettings) { - callSettingsType = - typeMakerFn.apply( - isSettingsBuilder ? BatchingCallSettings.Builder.class : BatchingCallSettings.class); - } // Streaming takes precendence over paging, as per the monolith's existing behavior. switch (method.stream()) { diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java b/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java deleted file mode 100644 index 2f7d4de00d..0000000000 --- a/src/main/java/com/google/api/generator/gapic/model/GapicBatchingSettings.java +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.api.generator.gapic.model; - -import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; -import java.util.List; -import javax.annotation.Nullable; - -@AutoValue -public abstract class GapicBatchingSettings { - public enum FlowControlLimitExceededBehavior { - THROW_EXCEPTION, - BLOCK, - IGNORE - }; - - // Threshold fields. - public abstract String protoPakkage(); - - public abstract String serviceName(); - - public abstract String methodName(); - - public abstract int elementCountThreshold(); - - public abstract long requestByteThreshold(); - - public abstract long delayThresholdMillis(); - - @Nullable - public abstract Integer flowControlElementLimit(); - - @Nullable - public abstract Integer flowControlByteLimit(); - - public abstract FlowControlLimitExceededBehavior flowControlLimitExceededBehavior(); - - // Batch descriptor fields. - public abstract String batchedFieldName(); - - public abstract ImmutableList discriminatorFieldNames(); - - @Nullable - public abstract String subresponseFieldName(); - - public boolean matches(Service service, Method method) { - return protoPakkage().equals(service.protoPakkage()) - && serviceName().equals(service.name()) - && methodName().equals(method.name()); - } - - public static Builder builder() { - return new AutoValue_GapicBatchingSettings.Builder() - .setFlowControlLimitExceededBehavior(FlowControlLimitExceededBehavior.IGNORE); - } - - @AutoValue.Builder - public abstract static class Builder { - public abstract Builder setProtoPakkage(String protoPakkage); - - public abstract Builder setServiceName(String serviceName); - - public abstract Builder setMethodName(String methodName); - - public abstract Builder setElementCountThreshold(int elementCountThreshold); - - public abstract Builder setRequestByteThreshold(long requestByteThreshold); - - public abstract Builder setDelayThresholdMillis(long delalyThresholdMillis); - - public abstract Builder setFlowControlElementLimit(Integer flowControlElementLimit); - - public abstract Builder setFlowControlByteLimit(Integer flowControlByteLimit); - - public abstract Builder setFlowControlLimitExceededBehavior( - FlowControlLimitExceededBehavior behavior); - - public abstract Builder setBatchedFieldName(String batchedFieldName); - - public abstract Builder setDiscriminatorFieldNames(List discriminatorFieldNames); - - public abstract Builder setSubresponseFieldName(String subresponseFieldName); - - public abstract GapicBatchingSettings build(); - } -} diff --git a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java index d161f44861..73d0b38b4d 100644 --- a/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java +++ b/src/main/java/com/google/api/generator/gapic/model/GapicServiceConfig.java @@ -42,19 +42,14 @@ public class GapicServiceConfig { private final List methodConfigs; private final Map methodConfigTable; - private final Map batchingSettingsTable; private GapicServiceConfig( - List methodConfigs, - Map methodConfigTable, - Map batchingSettingsTable) { + List methodConfigs, Map methodConfigTable) { this.methodConfigs = methodConfigs; this.methodConfigTable = methodConfigTable; - this.batchingSettingsTable = batchingSettingsTable; } - public static GapicServiceConfig create( - ServiceConfig serviceConfig, Optional> batchingSettingsOpt) { + public static GapicServiceConfig create(ServiceConfig serviceConfig) { // Keep this processing logic out of the constructor. Map methodConfigTable = new HashMap<>(); List methodConfigs = serviceConfig.getMethodConfigList(); @@ -65,21 +60,7 @@ public static GapicServiceConfig create( } } - Map batchingSettingsTable = new HashMap<>(); - if (batchingSettingsOpt.isPresent()) { - for (GapicBatchingSettings batchingSetting : batchingSettingsOpt.get()) { - batchingSettingsTable.put( - MethodConfig.Name.newBuilder() - .setService( - String.format( - "%s.%s", batchingSetting.protoPakkage(), batchingSetting.serviceName())) - .setMethod(batchingSetting.methodName()) - .build(), - batchingSetting); - } - } - - return new GapicServiceConfig(methodConfigs, methodConfigTable, batchingSettingsTable); + return new GapicServiceConfig(methodConfigs, methodConfigTable); } public Map getAllGapicRetrySettings(Service service) { @@ -118,16 +99,6 @@ public String getRetryParamsName(Service service, Method method) { return NO_RETRY_PARAMS_NAME; } - public boolean hasBatchingSetting(Service service, Method method) { - return batchingSettingsTable.containsKey(toName(service, method)); - } - - public Optional getBatchingSetting(Service service, Method method) { - return hasBatchingSetting(service, method) - ? Optional.of(batchingSettingsTable.get(toName(service, method))) - : Optional.empty(); - } - private GapicRetrySettings toGapicRetrySettings(Service service, Method method) { GapicRetrySettings.Kind kind = GapicRetrySettings.Kind.FULL; Optional retryPolicyIndexOpt = retryPolicyIndexLookup(service, method); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java deleted file mode 100644 index 79a1c9817d..0000000000 --- a/src/main/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParser.java +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.api.generator.gapic.protoparser; - -import com.google.api.generator.gapic.model.GapicBatchingSettings; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import org.yaml.snakeyaml.Yaml; -import org.yaml.snakeyaml.constructor.SafeConstructor; - -public class BatchingSettingsConfigParser { - private static String LIMIT_EXCEEDED_BEHAVIOR_THROW_EXCEPTION_YAML_VALUE = "THROW_EXCEPTION"; - private static String LIMIT_EXCEEDED_BEHAVIOR_BLOCK_YAML_VALUE = "BLOCK"; - - private static String YAML_KEY_INTERFACES = "interfaces"; - private static String YAML_KEY_NAME = "name"; - private static String YAML_KEY_METHODS = "methods"; - private static String YAML_KEY_BATCHING = "batching"; - private static String YAML_KEY_THRESHOLDS = "thresholds"; - private static String YAML_KEY_DESCRIPTOR = "batch_descriptor"; - - private static String YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD = "element_count_threshold"; - private static String YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS = "delay_threshold_millis"; - private static String YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD = "request_byte_threshold"; - private static String YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT = "flow_control_element_limit"; - private static String YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT = "flow_control_byte_limit"; - private static String YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR = - "flow_control_limit_exceeded_behavior"; - - private static String YAML_KEY_DESCRIPTOR_BATCHED_FIELD = "batched_field"; - private static String YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD = "discriminator_fields"; - private static String YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD = "subresponse_field"; - - public static Optional> parse( - Optional gapicYamlConfigFilePathOpt) { - return gapicYamlConfigFilePathOpt.isPresent() - ? parse(gapicYamlConfigFilePathOpt.get()) - : Optional.empty(); - } - - @VisibleForTesting - static Optional> parse(String gapicYamlConfigFilePath) { - if (Strings.isNullOrEmpty(gapicYamlConfigFilePath) - || !(new File(gapicYamlConfigFilePath)).exists()) { - return Optional.empty(); - } - - String fileContents = null; - - try { - fileContents = new String(Files.readAllBytes(Paths.get(gapicYamlConfigFilePath))); - } catch (IOException e) { - return Optional.empty(); - } - - Yaml yaml = new Yaml(new SafeConstructor()); - Map yamlMap = yaml.load(fileContents); - return parseFromMap(yamlMap); - } - - private static Optional> parseFromMap(Map yamlMap) { - if (!yamlMap.containsKey(YAML_KEY_INTERFACES)) { - return Optional.empty(); - } - - List settings = new ArrayList<>(); - for (Map serviceYamlConfig : - (List>) yamlMap.get(YAML_KEY_INTERFACES)) { - if (!serviceYamlConfig.containsKey(YAML_KEY_METHODS)) { - continue; - } - for (Map methodYamlConfig : - (List>) serviceYamlConfig.get(YAML_KEY_METHODS)) { - if (!methodYamlConfig.containsKey(YAML_KEY_BATCHING)) { - continue; - } - Map batchingOuterYamlConfig = - (Map) methodYamlConfig.get(YAML_KEY_BATCHING); - if (!batchingOuterYamlConfig.containsKey(YAML_KEY_THRESHOLDS)) { - continue; - } - Preconditions.checkState( - batchingOuterYamlConfig.containsKey(YAML_KEY_DESCRIPTOR), - String.format( - "%s key expected but not found for method %s", - YAML_KEY_DESCRIPTOR, (String) methodYamlConfig.get(YAML_KEY_NAME))); - - // Parse the threshold values first. - Map batchingYamlConfig = - (Map) batchingOuterYamlConfig.get(YAML_KEY_THRESHOLDS); - Preconditions.checkState( - batchingYamlConfig.containsKey(YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD) - && batchingYamlConfig.containsKey(YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD) - && batchingYamlConfig.containsKey(YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS), - String.format( - "Batching YAML config is missing one of %s, %s, or %s fields", - YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD, - YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD, - YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS)); - - String interfaceName = (String) serviceYamlConfig.get(YAML_KEY_NAME); - int lastDotIndex = interfaceName.lastIndexOf("."); - String protoPakkage = interfaceName.substring(0, lastDotIndex); - String serviceName = interfaceName.substring(lastDotIndex + 1); - String methodName = (String) methodYamlConfig.get(YAML_KEY_NAME); - GapicBatchingSettings.Builder settingsBuilder = - GapicBatchingSettings.builder() - .setProtoPakkage(protoPakkage) - .setServiceName(serviceName) - .setMethodName(methodName) - .setElementCountThreshold( - (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_ELEMENT_COUNT_THRESHOLD)) - .setRequestByteThreshold( - (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_REQUEST_BYTE_THRESHOLD)) - .setDelayThresholdMillis( - (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_DELAY_THRESHOLD_MILLIS)); - - if (batchingYamlConfig.containsKey(YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT)) { - settingsBuilder.setFlowControlElementLimit( - (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_ELEMENT_LIMIT)); - } - if (batchingYamlConfig.containsKey(YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT)) { - settingsBuilder.setFlowControlByteLimit( - (Integer) batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_BYTE_LIMIT)); - } - if (batchingYamlConfig.containsKey( - YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR)) { - String behaviorYamlValue = - (String) - batchingYamlConfig.get(YAML_KEY_BATCHING_FLOW_CONTROL_LIMIT_EXCEEDED_BEHAVIOR); - GapicBatchingSettings.FlowControlLimitExceededBehavior behaviorSetting = - GapicBatchingSettings.FlowControlLimitExceededBehavior.IGNORE; - // IGNORE or UNSET_BEHAVIOR YAML values map to FlowControlLimitExceededBehavior.IGNOER. - if (behaviorYamlValue.equals(LIMIT_EXCEEDED_BEHAVIOR_THROW_EXCEPTION_YAML_VALUE)) { - behaviorSetting = - GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION; - } else if (behaviorYamlValue.equals(LIMIT_EXCEEDED_BEHAVIOR_BLOCK_YAML_VALUE)) { - behaviorSetting = GapicBatchingSettings.FlowControlLimitExceededBehavior.BLOCK; - } - settingsBuilder.setFlowControlLimitExceededBehavior(behaviorSetting); - } - - // Parse the descriptor values. - Map descriptorYamlConfig = - (Map) batchingOuterYamlConfig.get(YAML_KEY_DESCRIPTOR); - Preconditions.checkState( - descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_BATCHED_FIELD) - && descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD), - String.format( - "Batching descriptor YAML config is missing one of %s or %s fields", - YAML_KEY_DESCRIPTOR_BATCHED_FIELD, YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD)); - - settingsBuilder.setBatchedFieldName( - (String) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_BATCHED_FIELD)); - settingsBuilder.setDiscriminatorFieldNames( - (List) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_DISCRIMINATOR_FIELD)); - - if (descriptorYamlConfig.containsKey(YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD)) { - settingsBuilder.setSubresponseFieldName( - (String) descriptorYamlConfig.get(YAML_KEY_DESCRIPTOR_SUBRESPONSE_FIELD)); - } - - settings.add(settingsBuilder.build()); - } - } - - return settings.isEmpty() ? Optional.empty() : Optional.of(settings); - } -} 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 2b58e07f51..3d41f3d84e 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 @@ -19,7 +19,6 @@ import com.google.api.ResourceProto; import com.google.api.generator.engine.ast.TypeNode; import com.google.api.generator.gapic.model.Field; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicContext; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.LongrunningOperation; @@ -77,15 +76,9 @@ public GapicParserException(String errorMessage) { } public static GapicContext parse(CodeGeneratorRequest request) { - Optional gapicYamlConfigPathOpt = - PluginArgumentParser.parseGapicYamlConfigPath(request); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(gapicYamlConfigPathOpt); - Optional serviceConfigPathOpt = PluginArgumentParser.parseJsonConfigPath(request); String serviceConfigPath = serviceConfigPathOpt.isPresent() ? serviceConfigPathOpt.get() : null; - Optional serviceConfigOpt = - ServiceConfigParser.parse(serviceConfigPath, batchingSettingsOpt); + Optional serviceConfigOpt = ServiceConfigParser.parse(serviceConfigPath); Optional serviceYamlConfigPathOpt = PluginArgumentParser.parseServiceYamlConfigPath(request); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java index 7cf29a1d29..0cd4a17bc8 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/PluginArgumentParser.java @@ -26,7 +26,6 @@ public class PluginArgumentParser { // Synced to rules_java_gapic/java_gapic.bzl. @VisibleForTesting static final String KEY_GRPC_SERVICE_CONFIG = "grpc-service-config"; - @VisibleForTesting static final String KEY_GAPIC_CONFIG = "gapic-config"; @VisibleForTesting static final String KEY_SERVICE_YAML_CONFIG = "gapic-service-config"; private static final String JSON_FILE_ENDING = "grpc_service_config.json"; @@ -37,10 +36,6 @@ static Optional parseJsonConfigPath(CodeGeneratorRequest request) { return parseJsonConfigPath(request.getParameter()); } - static Optional parseGapicYamlConfigPath(CodeGeneratorRequest request) { - return parseGapicYamlConfigPath(request.getParameter()); - } - static Optional parseServiceYamlConfigPath(CodeGeneratorRequest request) { return parseServiceYamlConfigPath(request.getParameter()); } @@ -51,11 +46,6 @@ static Optional parseJsonConfigPath(String pluginProtocArgument) { return parseArgument(pluginProtocArgument, KEY_GRPC_SERVICE_CONFIG, JSON_FILE_ENDING); } - @VisibleForTesting - static Optional parseGapicYamlConfigPath(String pluginProtocArgument) { - return parseArgument(pluginProtocArgument, KEY_GAPIC_CONFIG, GAPIC_YAML_FILE_ENDING); - } - @VisibleForTesting static Optional parseServiceYamlConfigPath(String pluginProtocArgument) { return parseArgument(pluginProtocArgument, KEY_SERVICE_YAML_CONFIG, SERVICE_YAML_FILE_ENDING); diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java index dcab2c4ec3..24bbe84716 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ServiceConfigParser.java @@ -14,7 +14,6 @@ package com.google.api.generator.gapic.protoparser; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; @@ -23,17 +22,15 @@ import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; -import java.util.List; import java.util.Optional; public class ServiceConfigParser { - public static Optional parse( - String serviceConfigFilePath, Optional> batchingSettingsOpt) { + public static Optional parse(String serviceConfigFilePath) { Optional rawConfigOpt = parseFile(serviceConfigFilePath); if (!rawConfigOpt.isPresent()) { return Optional.empty(); } - return Optional.of(GapicServiceConfig.create(rawConfigOpt.get(), batchingSettingsOpt)); + return Optional.of(GapicServiceConfig.create(rawConfigOpt.get())); } @VisibleForTesting diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index 3a689014d3..939cc4819d 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel @@ -4,7 +4,6 @@ load("//:rules_bazel/java/java_diff_test.bzl", "golden_update") package(default_visibility = ["//visibility:public"]) UPDATE_GOLDENS_TESTS = [ - "BatchingDescriptorComposerTest", "ComposerTest", "GrpcServiceCallableFactoryClassComposerTest", "GrpcServiceStubClassComposerTest", @@ -73,7 +72,6 @@ java_proto_library( ], data = [ "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files", - "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", ], test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), @@ -93,7 +91,6 @@ TEST_CLASS_DIR = "com.google.api.generator.gapic.composer." ], data = [ "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files", - "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", ], test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name), diff --git a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java deleted file mode 100644 index e2d366370e..0000000000 --- a/src/test/java/com/google/api/generator/gapic/composer/BatchingDescriptorComposerTest.java +++ /dev/null @@ -1,193 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.api.generator.gapic.composer; - -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertTrue; - -import com.google.api.generator.engine.ast.Expr; -import com.google.api.generator.engine.writer.JavaWriterVisitor; -import com.google.api.generator.gapic.model.GapicBatchingSettings; -import com.google.api.generator.gapic.model.GapicServiceConfig; -import com.google.api.generator.gapic.model.Message; -import com.google.api.generator.gapic.model.Method; -import com.google.api.generator.gapic.model.ResourceName; -import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.protoparser.BatchingSettingsConfigParser; -import com.google.api.generator.gapic.protoparser.Parser; -import com.google.api.generator.gapic.protoparser.ServiceConfigParser; -import com.google.api.generator.test.framework.Assert; -import com.google.api.generator.test.framework.Utils; -import com.google.logging.v2.LogEntryProto; -import com.google.logging.v2.LoggingConfigProto; -import com.google.logging.v2.LoggingMetricsProto; -import com.google.logging.v2.LoggingProto; -import com.google.protobuf.Descriptors.FileDescriptor; -import com.google.protobuf.Descriptors.ServiceDescriptor; -import com.google.pubsub.v1.PubsubProto; -import google.cloud.CommonResources; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import org.junit.Before; -import org.junit.Test; - -public class BatchingDescriptorComposerTest { - private JavaWriterVisitor writerVisitor; - - @Before - public void setUp() { - writerVisitor = new JavaWriterVisitor(); - } - - @Test - public void batchingDescriptor_hasSubresponseField() { - FileDescriptor serviceFileDescriptor = PubsubProto.getDescriptor(); - FileDescriptor commonResourcesFileDescriptor = CommonResources.getDescriptor(); - ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); - assertEquals("Publisher", serviceDescriptor.getName()); - - Map resourceNames = new HashMap<>(); - resourceNames.putAll(Parser.parseResourceNames(serviceFileDescriptor)); - resourceNames.putAll(Parser.parseResourceNames(commonResourcesFileDescriptor)); - - Map messageTypes = Parser.parseMessages(serviceFileDescriptor); - - Set outputResourceNames = new HashSet<>(); - List services = - Parser.parseService( - serviceFileDescriptor, - messageTypes, - resourceNames, - Optional.empty(), - outputResourceNames); - - String filename = "pubsub_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - - String jsonFilename = "pubsub_grpc_service_config.json"; - Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); - assertTrue(configOpt.isPresent()); - GapicServiceConfig config = configOpt.get(); - - Service service = services.get(0); - assertEquals("Publisher", service.name()); - Method method = findMethod(service, "Publish"); - - GapicBatchingSettings batchingSetting = batchingSettingsOpt.get().get(0); - assertEquals("Publish", batchingSetting.methodName()); - Expr batchingDescriptorExpr = - BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( - method, batchingSetting, messageTypes); - - batchingDescriptorExpr.accept(writerVisitor); - Utils.saveCodegenToFile( - this.getClass(), "BatchingDescriptorComposerTestSubresponse.golden", writerVisitor.write()); - Path goldenFilePath = - Paths.get( - ComposerConstants.GOLDENFILES_DIRECTORY, - "BatchingDescriptorComposerTestSubresponse.golden"); - Assert.assertCodeEquals(goldenFilePath, writerVisitor.write()); - } - - @Test - public void batchingDescriptor_noSubresponseField() { - FileDescriptor serviceFileDescriptor = LoggingProto.getDescriptor(); - ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); - assertEquals(serviceDescriptor.getName(), "LoggingServiceV2"); - - List protoFiles = - Arrays.asList( - serviceFileDescriptor, - LogEntryProto.getDescriptor(), - LoggingConfigProto.getDescriptor(), - LoggingMetricsProto.getDescriptor()); - - Map resourceNames = new HashMap<>(); - Map messageTypes = new HashMap<>(); - for (FileDescriptor fileDescriptor : protoFiles) { - resourceNames.putAll(Parser.parseResourceNames(fileDescriptor)); - messageTypes.putAll(Parser.parseMessages(fileDescriptor)); - } - - Set outputResourceNames = new HashSet<>(); - List services = - Parser.parseService( - serviceFileDescriptor, - messageTypes, - resourceNames, - Optional.empty(), - outputResourceNames); - - String filename = "logging_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - - String jsonFilename = "logging_grpc_service_config.json"; - Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); - assertTrue(configOpt.isPresent()); - GapicServiceConfig config = configOpt.get(); - - Service service = services.get(0); - assertEquals("LoggingServiceV2", service.name()); - Method method = findMethod(service, "WriteLogEntries"); - - GapicBatchingSettings batchingSetting = batchingSettingsOpt.get().get(0); - assertEquals("WriteLogEntries", batchingSetting.methodName()); - Expr batchingDescriptorExpr = - BatchingDescriptorComposer.createBatchingDescriptorFieldDeclExpr( - method, batchingSetting, messageTypes); - - batchingDescriptorExpr.accept(writerVisitor); - Utils.saveCodegenToFile( - this.getClass(), - "BatchingDescriptorComposerTestNoSubresponse.golden", - writerVisitor.write()); - Path goldenFilePath = - Paths.get( - ComposerConstants.GOLDENFILES_DIRECTORY, - "BatchingDescriptorComposerTestNoSubresponse.golden"); - Assert.assertCodeEquals(goldenFilePath, writerVisitor.write()); - } - - private static Method findMethod(Service service, String methodName) { - for (Method m : service.methods()) { - if (m.name().equals(methodName)) { - return m; - } - } - return null; - } - - private static String createLines(String... lines) { - // Cast to get rid of warnings. - return String.format(new String(new char[lines.length]).replace("\0", "%s"), (Object[]) lines); - } -} diff --git a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java index a0f74f2052..00edc18160 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/RetrySettingsComposerTest.java @@ -27,30 +27,21 @@ import com.google.api.generator.engine.ast.Variable; import com.google.api.generator.engine.ast.VariableExpr; import com.google.api.generator.engine.writer.JavaWriterVisitor; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.Method; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.protoparser.BatchingSettingsConfigParser; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protoparser.ServiceConfigParser; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.logging.v2.LogEntryProto; -import com.google.logging.v2.LoggingConfigProto; -import com.google.logging.v2.LoggingMetricsProto; -import com.google.logging.v2.LoggingProto; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.ServiceDescriptor; -import com.google.pubsub.v1.PubsubProto; import com.google.showcase.v1beta1.EchoOuterClass; -import google.cloud.CommonResources; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -89,8 +80,7 @@ public void paramDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -127,8 +117,7 @@ public void paramDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -169,8 +158,7 @@ public void codesDefinitionsBlock_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -207,8 +195,7 @@ public void codesDefinitionsBlock_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -248,8 +235,7 @@ public void simpleBuilderExpr_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -331,8 +317,7 @@ public void lroBuilderExpr() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -369,166 +354,6 @@ public void lroBuilderExpr() { assertEquals(expected, writerVisitor.write()); } - @Test - public void batchingSettings_minimalFlowControlSettings() { - FileDescriptor serviceFileDescriptor = PubsubProto.getDescriptor(); - FileDescriptor commonResourcesFileDescriptor = CommonResources.getDescriptor(); - ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); - assertEquals("Publisher", serviceDescriptor.getName()); - - Map resourceNames = new HashMap<>(); - resourceNames.putAll(Parser.parseResourceNames(serviceFileDescriptor)); - resourceNames.putAll(Parser.parseResourceNames(commonResourcesFileDescriptor)); - - Map messageTypes = Parser.parseMessages(serviceFileDescriptor); - - Set outputResourceNames = new HashSet<>(); - List services = - Parser.parseService( - serviceFileDescriptor, - messageTypes, - resourceNames, - Optional.empty(), - outputResourceNames); - - String filename = "pubsub_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - - String jsonFilename = "pubsub_grpc_service_config.json"; - Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); - assertTrue(configOpt.isPresent()); - GapicServiceConfig config = configOpt.get(); - - Service service = services.get(0); - assertEquals("Publisher", service.name()); - - VariableExpr builderVarExpr = createBuilderVarExpr(service); - String methodSettingsName = "publishSettings"; - GapicBatchingSettings batchingSettings = - GapicBatchingSettings.builder() - .setProtoPakkage("com.google.pubsub.v1") - .setServiceName("Publishing") - .setMethodName("Publish") - .setElementCountThreshold(100) - .setRequestByteThreshold(1048576) - .setDelayThresholdMillis(10) - .setBatchedFieldName("messages") - .setDiscriminatorFieldNames(Arrays.asList("topic")) - .setSubresponseFieldName("message_ids") - .build(); - - Expr builderExpr = - RetrySettingsComposer.createBatchingBuilderSettingsExpr( - methodSettingsName, batchingSettings, builderVarExpr); - builderExpr.accept(writerVisitor); - String expected = - createLines( - "builder" - + ".publishSettings()" - + ".setBatchingSettings(" - + "BatchingSettings.newBuilder()" - + ".setElementCountThreshold(100L)" - + ".setRequestByteThreshold(1048576L)" - + ".setDelayThreshold(Duration.ofMillis(10L))" - + ".setFlowControlSettings(" - + "FlowControlSettings.newBuilder()" - + ".setLimitExceededBehavior(FlowController.LimitExceededBehavior.Ignore)" - + ".build())" - + ".build())"); - assertEquals(expected, writerVisitor.write()); - } - - @Test - public void batchingSettings_fullFlowControlSettings() { - FileDescriptor serviceFileDescriptor = LoggingProto.getDescriptor(); - ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); - assertEquals(serviceDescriptor.getName(), "LoggingServiceV2"); - - List protoFiles = - Arrays.asList( - serviceFileDescriptor, - LogEntryProto.getDescriptor(), - LoggingConfigProto.getDescriptor(), - LoggingMetricsProto.getDescriptor()); - - Map resourceNames = new HashMap<>(); - Map messageTypes = new HashMap<>(); - for (FileDescriptor fileDescriptor : protoFiles) { - resourceNames.putAll(Parser.parseResourceNames(fileDescriptor)); - messageTypes.putAll(Parser.parseMessages(fileDescriptor)); - } - - Set outputResourceNames = new HashSet<>(); - List services = - Parser.parseService( - serviceFileDescriptor, - messageTypes, - resourceNames, - Optional.empty(), - outputResourceNames); - - String filename = "logging_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - - String jsonFilename = "logging_grpc_service_config.json"; - Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); - assertTrue(configOpt.isPresent()); - GapicServiceConfig config = configOpt.get(); - - Service service = services.get(0); - assertEquals("LoggingServiceV2", service.name()); - - VariableExpr builderVarExpr = createBuilderVarExpr(service); - String methodSettingsName = "writeLogEntriesSettings"; - GapicBatchingSettings batchingSettings = - GapicBatchingSettings.builder() - .setProtoPakkage("com.google.logging.v2") - .setServiceName("LoggingServiceV2") - .setMethodName("WriteLogEntries") - .setElementCountThreshold(1000) - .setRequestByteThreshold(1048576) - .setDelayThresholdMillis(50) - .setFlowControlElementLimit(100000) - .setFlowControlByteLimit(10485760) - .setFlowControlLimitExceededBehavior( - GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION) - .setBatchedFieldName("entries") - .setDiscriminatorFieldNames(Arrays.asList("log_name", "resource", "labels")) - .build(); - - Expr builderExpr = - RetrySettingsComposer.createBatchingBuilderSettingsExpr( - methodSettingsName, batchingSettings, builderVarExpr); - builderExpr.accept(writerVisitor); - String expected = - createLines( - "builder" - + ".writeLogEntriesSettings()" - + ".setBatchingSettings(" - + "BatchingSettings.newBuilder()" - + ".setElementCountThreshold(1000L)" - + ".setRequestByteThreshold(1048576L)" - + ".setDelayThreshold(Duration.ofMillis(50L))" - + ".setFlowControlSettings(" - + "FlowControlSettings.newBuilder()" - + ".setMaxOutstandingElementCount(100000L)" - + ".setMaxOutstandingRequestBytes(10485760L)" - + ".setLimitExceededBehavior(FlowController.LimitExceededBehavior.ThrowException)" - + ".build())" - + ".build())"); - assertEquals(expected, writerVisitor.write()); - } - private static Method findMethod(Service service, String methodName) { for (Method m : service.methods()) { if (m.name().equals(methodName)) { diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 527e513101..dfc35f44d5 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -18,13 +18,11 @@ import static junit.framework.Assert.assertTrue; import com.google.api.generator.engine.writer.JavaWriterVisitor; -import com.google.api.generator.gapic.model.GapicBatchingSettings; import com.google.api.generator.gapic.model.GapicClass; import com.google.api.generator.gapic.model.GapicServiceConfig; import com.google.api.generator.gapic.model.Message; import com.google.api.generator.gapic.model.ResourceName; import com.google.api.generator.gapic.model.Service; -import com.google.api.generator.gapic.protoparser.BatchingSettingsConfigParser; import com.google.api.generator.gapic.protoparser.Parser; import com.google.api.generator.gapic.protoparser.ServiceConfigParser; import com.google.api.generator.test.framework.Assert; @@ -53,7 +51,7 @@ public class ServiceStubSettingsClassComposerTest { @Test - public void generateServiceStubSettingsClasses_batchingWithEmptyResponses() throws IOException { + public void generateServiceStubSettingsClasses_batchingNotHandled() throws IOException { FileDescriptor serviceFileDescriptor = LoggingProto.getDescriptor(); ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); assertEquals(serviceDescriptor.getName(), "LoggingServiceV2"); @@ -75,16 +73,9 @@ public void generateServiceStubSettingsClasses_batchingWithEmptyResponses() thro List services = parseServices(serviceFileDescriptor, serviceDescriptor, messageTypes, resourceNames); - String filename = "logging_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - String jsonFilename = "logging_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -102,8 +93,7 @@ public void generateServiceStubSettingsClasses_batchingWithEmptyResponses() thro } @Test - public void generateServiceStubSettingsClasses_batchingWithNonemptyResponses() - throws IOException { + public void generateServiceStubSettingsClasses_batchingNotHandledInPubSub() throws IOException { FileDescriptor serviceFileDescriptor = PubsubProto.getDescriptor(); FileDescriptor commonResourcesFileDescriptor = CommonResources.getDescriptor(); ServiceDescriptor serviceDescriptor = serviceFileDescriptor.getServices().get(0); @@ -118,16 +108,9 @@ public void generateServiceStubSettingsClasses_batchingWithNonemptyResponses() List services = parseServices(serviceFileDescriptor, serviceDescriptor, messageTypes, resourceNames); - String filename = "pubsub_gapic.yaml"; - Path path = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, filename); - Optional> batchingSettingsOpt = - BatchingSettingsConfigParser.parse(Optional.of(path.toString())); - assertTrue(batchingSettingsOpt.isPresent()); - String jsonFilename = "pubsub_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); @@ -157,8 +140,7 @@ public void generateServiceStubSettingsClasses_basic() throws IOException { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(ComposerConstants.TESTFILES_DIRECTORY, jsonFilename); - Optional configOpt = - ServiceConfigParser.parse(jsonPath.toString(), Optional.empty()); + Optional configOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(configOpt.isPresent()); GapicServiceConfig config = configOpt.get(); diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestNoSubresponse.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestNoSubresponse.golden deleted file mode 100644 index b9c9eba122..0000000000 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestNoSubresponse.golden +++ /dev/null @@ -1,53 +0,0 @@ -private static final BatchingDescriptor WRITE_LOG_ENTRIES_BATCHING_DESC = new BatchingDescriptor() { -@Override -public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) { -return new PartitionKey(request.getLogName(), request.getResource(), request.getLabels()); -} - -@Override -public RequestBuilder getRequestBuilder() { -return new RequestBuilder() { -private WriteLogEntriesRequest.Builder builder; -@Override -public void appendRequest(WriteLogEntriesRequest request) { -if (Objects.isNull(builder)) { -builder = request.toBuilder(); -} else { -builder.addAllEntries(request.getEntriesList()); -} -} - -@Override -public WriteLogEntriesRequest build() { -return builder.build(); -} - -}; -} - -@Override -public void splitResponse(WriteLogEntriesResponse batchResponse, Collection> batch) { -for (BatchedRequestIssuer responder : batch) { -WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build(); -responder.setResponse(response); -} -} - -@Override -public void splitException(Throwable throwable, Collection> batch) { -for (BatchedRequestIssuer responder : batch) { -responder.setException(throwable); -} -} - -@Override -public long countElements(WriteLogEntriesRequest request) { -return request.getEntriesCount(); -} - -@Override -public long countBytes(WriteLogEntriesRequest request) { -return request.getSerializedSize(); -} - -} \ No newline at end of file diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestSubresponse.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestSubresponse.golden deleted file mode 100644 index a322c69aec..0000000000 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/BatchingDescriptorComposerTestSubresponse.golden +++ /dev/null @@ -1,59 +0,0 @@ -private static final BatchingDescriptor PUBLISH_BATCHING_DESC = new BatchingDescriptor() { -@Override -public PartitionKey getBatchPartitionKey(PublishRequest request) { -return new PartitionKey(request.getTopic()); -} - -@Override -public RequestBuilder getRequestBuilder() { -return new RequestBuilder() { -private PublishRequest.Builder builder; -@Override -public void appendRequest(PublishRequest request) { -if (Objects.isNull(builder)) { -builder = request.toBuilder(); -} else { -builder.addAllMessages(request.getMessagesList()); -} -} - -@Override -public PublishRequest build() { -return builder.build(); -} - -}; -} - -@Override -public void splitResponse(PublishResponse batchResponse, Collection> batch) { -int batchMessageIndex = 0; -for (BatchedRequestIssuer responder : batch) { -List subresponseElements = new ArrayList<>(); -long subresponseCount = responder.getMessageCount(); -for (int i = 0; i < subresponseCount; i++) { -subresponseElements.add(batchResponse.getMessageIds(batchMessageIndex++)); -} -PublishResponse response = PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build(); -responder.setResponse(response); -} -} - -@Override -public void splitException(Throwable throwable, Collection> batch) { -for (BatchedRequestIssuer responder : batch) { -responder.setException(throwable); -} -} - -@Override -public long countElements(PublishRequest request) { -return request.getMessagesCount(); -} - -@Override -public long countBytes(PublishRequest request) { -return request.getSerializedSize(); -} - -} \ No newline at end of file diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden index 29dc2b3ac5..fe7cd63332 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/LoggingServiceV2StubSettings.golden @@ -8,11 +8,6 @@ import com.google.api.MonitoredResourceDescriptor; import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; -import com.google.api.gax.batching.BatchingSettings; -import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.batching.FlowController; -import com.google.api.gax.batching.PartitionKey; -import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -22,9 +17,6 @@ import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; -import com.google.api.gax.rpc.BatchedRequestIssuer; -import com.google.api.gax.rpc.BatchingCallSettings; -import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.PageContext; import com.google.api.gax.rpc.PagedCallSettings; @@ -51,7 +43,6 @@ import com.google.logging.v2.WriteLogEntriesRequest; import com.google.logging.v2.WriteLogEntriesResponse; import com.google.protobuf.Empty; import java.io.IOException; -import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.Generated; @@ -88,7 +79,7 @@ public class LoggingServiceV2StubSettings extends StubSettings deleteLogSettings; - private final BatchingCallSettings + private final UnaryCallSettings writeLogEntriesSettings; private final PagedCallSettings< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -289,73 +280,13 @@ public class LoggingServiceV2StubSettings extends StubSettings - WRITE_LOG_ENTRIES_BATCHING_DESC = - new BatchingDescriptor() { - @Override - public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) { - return new PartitionKey( - request.getLogName(), request.getResource(), request.getLabels()); - } - - @Override - public RequestBuilder getRequestBuilder() { - return new RequestBuilder() { - private WriteLogEntriesRequest.Builder builder; - - @Override - public void appendRequest(WriteLogEntriesRequest request) { - if (Objects.isNull(builder)) { - builder = request.toBuilder(); - } else { - builder.addAllEntries(request.getEntriesList()); - } - } - - @Override - public WriteLogEntriesRequest build() { - return builder.build(); - } - }; - } - - @Override - public void splitResponse( - WriteLogEntriesResponse batchResponse, - Collection> batch) { - for (BatchedRequestIssuer responder : batch) { - WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build(); - responder.setResponse(response); - } - } - - @Override - public void splitException( - Throwable throwable, - Collection> batch) { - for (BatchedRequestIssuer responder : batch) { - responder.setException(throwable); - } - } - - @Override - public long countElements(WriteLogEntriesRequest request) { - return request.getEntriesCount(); - } - - @Override - public long countBytes(WriteLogEntriesRequest request) { - return request.getSerializedSize(); - } - }; - /** Returns the object with the settings used for calls to deleteLog. */ public UnaryCallSettings deleteLogSettings() { return deleteLogSettings; } /** Returns the object with the settings used for calls to writeLogEntries. */ - public BatchingCallSettings + public UnaryCallSettings writeLogEntriesSettings() { return writeLogEntriesSettings; } @@ -463,7 +394,7 @@ public class LoggingServiceV2StubSettings extends StubSettings { private final ImmutableList> unaryMethodSettingsBuilders; private final UnaryCallSettings.Builder deleteLogSettings; - private final BatchingCallSettings.Builder + private final UnaryCallSettings.Builder writeLogEntriesSettings; private final PagedCallSettings.Builder< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -519,9 +450,7 @@ public class LoggingServiceV2StubSettings extends StubSettings + public UnaryCallSettings.Builder writeLogEntriesSettings() { return writeLogEntriesSettings; } diff --git a/src/test/java/com/google/api/generator/gapic/composer/goldens/PublisherStubSettings.golden b/src/test/java/com/google/api/generator/gapic/composer/goldens/PublisherStubSettings.golden index 01a3714a41..fe73523fcd 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/goldens/PublisherStubSettings.golden +++ b/src/test/java/com/google/api/generator/gapic/composer/goldens/PublisherStubSettings.golden @@ -7,11 +7,6 @@ import static com.google.pubsub.v1.PublisherClient.ListTopicsPagedResponse; import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; -import com.google.api.gax.batching.BatchingSettings; -import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.batching.FlowController; -import com.google.api.gax.batching.PartitionKey; -import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -21,9 +16,6 @@ import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; -import com.google.api.gax.rpc.BatchedRequestIssuer; -import com.google.api.gax.rpc.BatchingCallSettings; -import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.PageContext; import com.google.api.gax.rpc.PagedCallSettings; @@ -54,8 +46,6 @@ import com.google.pubsub.v1.PublishResponse; import com.google.pubsub.v1.Topic; import com.google.pubsub.v1.UpdateTopicRequest; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.Generated; @@ -90,7 +80,7 @@ public class PublisherStubSettings extends StubSettings { private final UnaryCallSettings createTopicSettings; private final UnaryCallSettings updateTopicSettings; - private final BatchingCallSettings publishSettings; + private final UnaryCallSettings publishSettings; private final UnaryCallSettings getTopicSettings; private final PagedCallSettings listTopicsSettings; @@ -285,71 +275,6 @@ public class PublisherStubSettings extends StubSettings { } }; - private static final BatchingDescriptor PUBLISH_BATCHING_DESC = - new BatchingDescriptor() { - @Override - public PartitionKey getBatchPartitionKey(PublishRequest request) { - return new PartitionKey(request.getTopic()); - } - - @Override - public RequestBuilder getRequestBuilder() { - return new RequestBuilder() { - private PublishRequest.Builder builder; - - @Override - public void appendRequest(PublishRequest request) { - if (Objects.isNull(builder)) { - builder = request.toBuilder(); - } else { - builder.addAllMessages(request.getMessagesList()); - } - } - - @Override - public PublishRequest build() { - return builder.build(); - } - }; - } - - @Override - public void splitResponse( - PublishResponse batchResponse, - Collection> batch) { - int batchMessageIndex = 0; - for (BatchedRequestIssuer responder : batch) { - List subresponseElements = new ArrayList<>(); - long subresponseCount = responder.getMessageCount(); - for (int i = 0; i < subresponseCount; i++) { - subresponseElements.add(batchResponse.getMessageIds(batchMessageIndex++)); - } - PublishResponse response = - PublishResponse.newBuilder().addAllMessageIds(subresponseElements).build(); - responder.setResponse(response); - } - } - - @Override - public void splitException( - Throwable throwable, - Collection> batch) { - for (BatchedRequestIssuer responder : batch) { - responder.setException(throwable); - } - } - - @Override - public long countElements(PublishRequest request) { - return request.getMessagesCount(); - } - - @Override - public long countBytes(PublishRequest request) { - return request.getSerializedSize(); - } - }; - /** Returns the object with the settings used for calls to createTopic. */ public UnaryCallSettings createTopicSettings() { return createTopicSettings; @@ -361,7 +286,7 @@ public class PublisherStubSettings extends StubSettings { } /** Returns the object with the settings used for calls to publish. */ - public BatchingCallSettings publishSettings() { + public UnaryCallSettings publishSettings() { return publishSettings; } @@ -487,7 +412,7 @@ public class PublisherStubSettings extends StubSettings { private final ImmutableList> unaryMethodSettingsBuilders; private final UnaryCallSettings.Builder createTopicSettings; private final UnaryCallSettings.Builder updateTopicSettings; - private final BatchingCallSettings.Builder publishSettings; + private final UnaryCallSettings.Builder publishSettings; private final UnaryCallSettings.Builder getTopicSettings; private final PagedCallSettings.Builder< ListTopicsRequest, ListTopicsResponse, ListTopicsPagedResponse> @@ -581,9 +506,7 @@ public class PublisherStubSettings extends StubSettings { createTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); updateTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - publishSettings = - BatchingCallSettings.newBuilder(PUBLISH_BATCHING_DESC) - .setBatchingSettings(BatchingSettings.newBuilder().build()); + publishSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); getTopicSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); listTopicsSettings = PagedCallSettings.newBuilder(LIST_TOPICS_PAGE_STR_FACT); listTopicSubscriptionsSettings = @@ -654,19 +577,6 @@ public class PublisherStubSettings extends StubSettings { .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_0_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_0_params")); - builder - .publishSettings() - .setBatchingSettings( - BatchingSettings.newBuilder() - .setElementCountThreshold(100L) - .setRequestByteThreshold(1048576L) - .setDelayThreshold(Duration.ofMillis(10L)) - .setFlowControlSettings( - FlowControlSettings.newBuilder() - .setLimitExceededBehavior(FlowController.LimitExceededBehavior.Ignore) - .build()) - .build()); - builder .publishSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) @@ -732,7 +642,7 @@ public class PublisherStubSettings extends StubSettings { } /** Returns the builder for the settings used for calls to publish. */ - public BatchingCallSettings.Builder publishSettings() { + public UnaryCallSettings.Builder publishSettings() { return publishSettings; } diff --git a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java index 43564350d5..435b4e7402 100644 --- a/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java +++ b/src/test/java/com/google/api/generator/gapic/model/GapicServiceConfigTest.java @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; import com.google.api.generator.gapic.protoparser.Parser; @@ -29,7 +28,6 @@ import io.grpc.serviceconfig.MethodConfig; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -50,9 +48,7 @@ public void serviceConfig_noConfigsFound() { String jsonFilename = "retrying_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional> batchingSettingsOpt = Optional.empty(); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -81,9 +77,7 @@ public void serviceConfig_basic() { String jsonFilename = "showcase_grpc_service_config.json"; Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - Optional> batchingSettingsOpt = Optional.empty(); - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); + Optional serviceConfigOpt = ServiceConfigParser.parse(jsonPath.toString()); assertTrue(serviceConfigOpt.isPresent()); GapicServiceConfig serviceConfig = serviceConfigOpt.get(); @@ -130,80 +124,6 @@ public void serviceConfig_basic() { assertEquals(GapicServiceConfig.EMPTY_RETRY_CODES, retryCodes.get(retryCodeName)); } - @Test - public void serviceConfig_withBatchingSettings() { - FileDescriptor echoFileDescriptor = EchoOuterClass.getDescriptor(); - ServiceDescriptor echoServiceDescriptor = echoFileDescriptor.getServices().get(0); - Service service = parseService(echoFileDescriptor); - - String jsonFilename = "showcase_grpc_service_config.json"; - Path jsonPath = Paths.get(JSON_DIRECTORY, jsonFilename); - - GapicBatchingSettings origBatchingSetting = - GapicBatchingSettings.builder() - .setProtoPakkage("google.showcase.v1beta1") - .setServiceName("Echo") - .setMethodName("Echo") - .setElementCountThreshold(1000) - .setRequestByteThreshold(2000) - .setDelayThresholdMillis(3000) - .setBatchedFieldName("name") - .setDiscriminatorFieldNames(Arrays.asList("severity")) - .build(); - Optional> batchingSettingsOpt = - Optional.of(Arrays.asList(origBatchingSetting)); - - Optional serviceConfigOpt = - ServiceConfigParser.parse(jsonPath.toString(), batchingSettingsOpt); - assertTrue(serviceConfigOpt.isPresent()); - GapicServiceConfig serviceConfig = serviceConfigOpt.get(); - - Map retrySettings = serviceConfig.getAllGapicRetrySettings(service); - assertEquals(2, retrySettings.size()); - Map> retryCodes = serviceConfig.getAllRetryCodes(service); - assertEquals(2, retryCodes.size()); - - // Echo method has an explicitly-defined setting. - Method method = findMethod(service, "Echo"); - assertThat(method).isNotNull(); - - // No change to the retry settings. - String retryParamsName = serviceConfig.getRetryParamsName(service, method); - assertEquals("retry_policy_1_params", retryParamsName); - GapicRetrySettings settings = retrySettings.get(retryParamsName); - assertThat(settings).isNotNull(); - assertEquals(10, settings.timeout().getSeconds()); - assertEquals(GapicRetrySettings.Kind.FULL, settings.kind()); - - // No changge to the retry codes. - String retryCodeName = serviceConfig.getRetryCodeName(service, method); - assertEquals("retry_policy_1_codes", retryCodeName); - List retryCode = retryCodes.get(retryCodeName); - assertThat(retryCode).containsExactly(Code.UNAVAILABLE, Code.UNKNOWN); - - // Check batching settings. - assertTrue(serviceConfig.hasBatchingSetting(service, method)); - Optional batchingSettingOpt = - serviceConfig.getBatchingSetting(service, method); - assertTrue(batchingSettingOpt.isPresent()); - GapicBatchingSettings batchingSetting = batchingSettingOpt.get(); - assertEquals( - origBatchingSetting.elementCountThreshold(), batchingSetting.elementCountThreshold()); - assertEquals( - origBatchingSetting.requestByteThreshold(), batchingSetting.requestByteThreshold()); - assertEquals( - origBatchingSetting.delayThresholdMillis(), batchingSetting.delayThresholdMillis()); - - // Chat method defaults to the service-defined setting. - method = findMethod(service, "Chat"); - assertThat(method).isNotNull(); - retryParamsName = serviceConfig.getRetryParamsName(service, method); - assertEquals("no_retry_0_params", retryParamsName); - retryCodeName = serviceConfig.getRetryCodeName(service, method); - assertEquals("no_retry_0_codes", retryCodeName); - assertFalse(serviceConfig.hasBatchingSetting(service, method)); - } - private static Service parseService(FileDescriptor fileDescriptor) { Map messageTypes = Parser.parseMessages(fileDescriptor); Map resourceNames = Parser.parseResourceNames(fileDescriptor); diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel index 7df82d359f..eefb4b9789 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/protoparser/BUILD.bazel @@ -3,7 +3,6 @@ load("@rules_java//java:defs.bzl", "java_test") package(default_visibility = ["//visibility:public"]) TESTS = [ - "BatchingSettingsConfigParserTest", "HttpRuleParserTest", "MethodSignatureParserTest", "ParserTest", @@ -26,7 +25,6 @@ filegroup( srcs = ["{0}.java".format(test_name)], data = [ "//src/test/java/com/google/api/generator/gapic/testdata:basic_proto_descriptor", - "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files", "//src/test/java/com/google/api/generator/gapic/testdata:service_yaml_files", ], diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java deleted file mode 100644 index 92ba2e5094..0000000000 --- a/src/test/java/com/google/api/generator/gapic/protoparser/BatchingSettingsConfigParserTest.java +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.api.generator.gapic.protoparser; - -import static com.google.common.truth.Truth.assertThat; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertTrue; - -import com.google.api.generator.gapic.model.GapicBatchingSettings; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.List; -import java.util.Optional; -import org.junit.Test; - -public class BatchingSettingsConfigParserTest { - private static final String YAML_DIRECTORY = - "src/test/java/com/google/api/generator/gapic/testdata/"; - - @Test - public void parseGapicSettings_plain() { - String filename = "datastore_gapic.yaml"; - Path path = Paths.get(YAML_DIRECTORY, filename); - Optional> settingsOpt = - BatchingSettingsConfigParser.parse(path.toString()); - assertFalse(settingsOpt.isPresent()); - } - - @Test - public void parseGapicSettings_noMethodSettings() { - String filename = "showcase_gapic.yaml"; - Path path = Paths.get(YAML_DIRECTORY, filename); - Optional> settingsOpt = - BatchingSettingsConfigParser.parse(path.toString()); - assertFalse(settingsOpt.isPresent()); - } - - @Test - public void parseBatchingSettings_logging() { - String filename = "logging_gapic.yaml"; - Path path = Paths.get(YAML_DIRECTORY, filename); - Optional> settingsOpt = - BatchingSettingsConfigParser.parse(path.toString()); - assertTrue(settingsOpt.isPresent()); - - List batchingSettings = settingsOpt.get(); - assertEquals(1, batchingSettings.size()); - GapicBatchingSettings setting = batchingSettings.get(0); - - assertEquals("google.logging.v2", setting.protoPakkage()); - assertEquals("LoggingServiceV2", setting.serviceName()); - assertEquals("WriteLogEntries", setting.methodName()); - - assertEquals(1000, setting.elementCountThreshold()); - assertEquals(1048576, setting.requestByteThreshold()); - assertEquals(50, setting.delayThresholdMillis()); - - assertThat(setting.flowControlElementLimit()).isNotNull(); - assertEquals(100000, (long) setting.flowControlElementLimit()); - assertThat(setting.flowControlByteLimit()).isNotNull(); - assertEquals(10485760, (long) setting.flowControlByteLimit()); - assertEquals( - GapicBatchingSettings.FlowControlLimitExceededBehavior.THROW_EXCEPTION, - setting.flowControlLimitExceededBehavior()); - - assertEquals("entries", setting.batchedFieldName()); - assertThat(setting.discriminatorFieldNames()).containsExactly("log_name", "resource", "labels"); - assertThat(setting.subresponseFieldName()).isNull(); - } - - @Test - public void parseBatchingSettings_pubsub() { - String filename = "pubsub_gapic.yaml"; - Path path = Paths.get(YAML_DIRECTORY, filename); - Optional> settingsOpt = - BatchingSettingsConfigParser.parse(path.toString()); - assertTrue(settingsOpt.isPresent()); - - List batchingSettings = settingsOpt.get(); - assertEquals(1, batchingSettings.size()); - GapicBatchingSettings setting = batchingSettings.get(0); - - assertEquals("google.pubsub.v1", setting.protoPakkage()); - assertEquals("Publisher", setting.serviceName()); - assertEquals("Publish", setting.methodName()); - - assertEquals(100, setting.elementCountThreshold()); - assertEquals(1048576, setting.requestByteThreshold()); - assertEquals(10, setting.delayThresholdMillis()); - - assertThat(setting.flowControlElementLimit()).isNull(); - assertThat(setting.flowControlByteLimit()).isNull(); - assertEquals( - GapicBatchingSettings.FlowControlLimitExceededBehavior.IGNORE, - setting.flowControlLimitExceededBehavior()); - - assertEquals("messages", setting.batchedFieldName()); - assertThat(setting.discriminatorFieldNames()).containsExactly("topic"); - assertEquals("message_ids", setting.subresponseFieldName()); - } -} diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java index c8a84d4f82..b843a734b7 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/PluginArgumentParserTest.java @@ -53,101 +53,43 @@ public void parseJsonPath_similarFileAppearsFirst() { String.join( ",", Arrays.asList( - createGapicConfig(gapicPath), + createServiceConfig(gapicPath), createGrpcServiceConfig("/tmp/something.json"), createGrpcServiceConfig("/tmp/some_grpc_service_configjson"), createGrpcServiceConfig(jsonPath), - createGapicConfig(gapicPath))); + createServiceConfig(gapicPath))); assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get()); } @Test public void parseJsonPath_argumentHasSpaces() { String jsonPath = "/tmp/foo_grpc_service_config.json"; - String gapicPath = "/tmp/something_gapic.yaml"; String rawArgument = String.join( " , ", Arrays.asList( - createGapicConfig(gapicPath), createGrpcServiceConfig("/tmp/something.json"), createGrpcServiceConfig("/tmp/some_grpc_service_configjson"), - createGrpcServiceConfig(jsonPath), - createGapicConfig(gapicPath))); + createGrpcServiceConfig(jsonPath))); assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get()); } @Test public void parseJsonPath_restAreEmpty() { String jsonPath = "/tmp/foobar_grpc_service_config.json"; - String gapicPath = ""; + String emptyPath = ""; String rawArgument = - String.join(",", Arrays.asList(gapicPath, createGrpcServiceConfig(jsonPath), gapicPath)); + String.join(",", Arrays.asList(emptyPath, createGrpcServiceConfig(jsonPath), emptyPath)); assertEquals(jsonPath, PluginArgumentParser.parseJsonConfigPath(rawArgument).get()); } @Test public void parseJsonPath_noneFound() { - String gapicPath = "/tmp/something_gapic.yaml"; - String rawArgument = String.join(",", Arrays.asList(gapicPath)); + String someOtherPath = "/tmp/something_gapic.yaml"; + String rawArgument = String.join(",", Arrays.asList(someOtherPath)); assertFalse(PluginArgumentParser.parseJsonConfigPath(rawArgument).isPresent()); } - @Test - public void parseGapicYamlPath_onlyOnePresent() { - String gapicPath = "/tmp/something_gapic.yaml"; - assertEquals( - gapicPath, - PluginArgumentParser.parseGapicYamlConfigPath(createGapicConfig(gapicPath)).get()); - } - - @Test - public void parseGapicYamlPath_returnsFirstOneFound() { - String gapicPathOne = "/tmp/something_gapic.yaml"; - String gapicPathTwo = "/tmp/other_gapic.yaml"; - assertEquals( - gapicPathOne, - PluginArgumentParser.parseGapicYamlConfigPath( - String.join( - ",", - Arrays.asList( - createGapicConfig(gapicPathOne), createGapicConfig(gapicPathTwo)))) - .get()); - } - - @Test - public void parseGapicYamlPath_similarFileAppearsFirst() { - String jsonPath = "/tmp/foo_grpc_service_config.json"; - String gapicPath = "/tmp/something_gapic.yaml"; - String rawArgument = - String.join( - ",", - Arrays.asList( - createGrpcServiceConfig(jsonPath), - createGapicConfig("/tmp/something.yaml"), - createGapicConfig("/tmp/some_gapicyaml"), - createGapicConfig(gapicPath))); - assertEquals(gapicPath, PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).get()); - } - - @Test - public void parseGapicYamlPath_restAreEmpty() { - String jsonPath = ""; - String gapicPath = "/tmp/something_gapic.yaml"; - String rawArgument = - String.join(",", Arrays.asList(jsonPath, createGapicConfig(gapicPath), jsonPath)); - assertEquals(gapicPath, PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).get()); - } - - @Test - public void parseGapicYamlPath_noneFound() { - String jsonPath = "/tmp/foo_grpc_service_config.json"; - String gapicPath = ""; - String rawArgument = - String.join(",", Arrays.asList(createGrpcServiceConfig(jsonPath), gapicPath)); - assertFalse(PluginArgumentParser.parseGapicYamlConfigPath(rawArgument).isPresent()); - } - @Test public void parseServiceYamlPath_onlyOnePresent() { String servicePath = "/tmp/something.yaml"; @@ -183,19 +125,13 @@ public void parseServiceYamlPath_gapicFilePresent() { // Passed under the right flags. rawArgument = String.join( - ",", Arrays.asList(createGapicConfig(gapicPath), createServiceConfig(servicePath))); + ",", Arrays.asList(createServiceConfig(gapicPath), createServiceConfig(servicePath))); assertEquals(servicePath, PluginArgumentParser.parseServiceYamlConfigPath(rawArgument).get()); // Swapped flags. rawArgument = String.join( - ",", Arrays.asList(createGapicConfig(servicePath), createServiceConfig(gapicPath))); - assertFalse(PluginArgumentParser.parseServiceYamlConfigPath(rawArgument).isPresent()); - - // Both passed under the Gapic yaml flag. - rawArgument = - String.join( - ",", Arrays.asList(createGapicConfig(gapicPath), createGapicConfig(servicePath))); + ",", Arrays.asList(createServiceConfig(gapicPath), createServiceConfig(gapicPath))); assertFalse(PluginArgumentParser.parseServiceYamlConfigPath(rawArgument).isPresent()); } @@ -209,8 +145,8 @@ public void parseServiceYamlPath_similarFileAppearsFirst() { ",", Arrays.asList( createGrpcServiceConfig(jsonPath), - createGapicConfig("/tmp/something.yaml"), - createGapicConfig("/tmp/some_gapicyaml"), + createServiceConfig("/tmp/something.yaml"), + createServiceConfig("/tmp/some_gapicyaml"), createServiceConfig(gapicPath), createServiceConfig(servicePath))); assertEquals(servicePath, PluginArgumentParser.parseServiceYamlConfigPath(rawArgument).get()); @@ -229,10 +165,6 @@ private static String createGrpcServiceConfig(String path) { return String.format("%s=%s", PluginArgumentParser.KEY_GRPC_SERVICE_CONFIG, path); } - private static String createGapicConfig(String path) { - return String.format("%s=%s", PluginArgumentParser.KEY_GAPIC_CONFIG, path); - } - private static String createServiceConfig(String path) { return String.format("%s=%s", PluginArgumentParser.KEY_SERVICE_YAML_CONFIG, path); } diff --git a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel index 4909db60e6..ed949b95fa 100644 --- a/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/testdata/BUILD.bazel @@ -8,11 +8,6 @@ filegroup( srcs = glob(["*.json"]), ) -filegroup( - name = "gapic_config_files", - srcs = glob(["*_gapic.yaml"]), -) - filegroup( name = "service_yaml_files", srcs = ["logging.yaml"], diff --git a/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml deleted file mode 100644 index e9860d5efe..0000000000 --- a/src/test/java/com/google/api/generator/gapic/testdata/datastore_gapic.yaml +++ /dev/null @@ -1,38 +0,0 @@ -type: com.google.api.codegen.ConfigProto -config_schema_version: 2.0.0 -language_settings: - java: - package_name: com.google.cloud.datastore.v1 - python: - package_name: google.cloud.datastore_v1.gapic - go: - package_name: cloud.google.com/go/datastore/apiv1 - csharp: - package_name: Google.Cloud.Datastore.V1 - release_level: GA - ruby: - package_name: Google::Cloud::Datastore::V1 - release_level: GA - php: - package_name: Google\Cloud\Datastore\V1 - nodejs: - package_name: datastore.v1 - domain_layer_location: google-cloud -interfaces: -- name: google.datastore.v1.Datastore - retry_params_def: - - name: default - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 - initial_rpc_timeout_millis: 60000 - rpc_timeout_multiplier: 1 - max_rpc_timeout_millis: 60000 - total_timeout_millis: 600000 - methods: - - name: Lookup - retry_codes_name: idempotent - - name: RunQuery - retry_codes_name: idempotent - - name: ReserveIds - retry_codes_name: idempotent diff --git a/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml deleted file mode 100644 index 75c6deda26..0000000000 --- a/src/test/java/com/google/api/generator/gapic/testdata/logging_gapic.yaml +++ /dev/null @@ -1,79 +0,0 @@ -type: com.google.api.codegen.ConfigProto -config_schema_version: 2.0.0 -language_settings: - java: - package_name: com.google.cloud.logging.v2 - interface_names: - google.logging.v2.ConfigServiceV2: Config - google.logging.v2.LoggingServiceV2: Logging - google.logging.v2.MetricsServiceV2: Metrics - python: - package_name: google.cloud.logging_v2.gapic - go: - package_name: cloud.google.com/go/logging/apiv2 - domain_layer_location: cloud.google.com/go/logging - csharp: - package_name: Google.Cloud.Logging.V2 - release_level: GA - ruby: - package_name: Google::Cloud::Logging::V2 - php: - package_name: Google\Cloud\Logging\V2 - nodejs: - package_name: logging.v2 - domain_layer_location: google-cloud -interfaces: -- name: google.logging.v2.ConfigServiceV2 - retry_codes_def: - - name: idempotent - retry_codes: - - UNAVAILABLE - - DEADLINE_EXCEEDED - - INTERNAL - methods: - - name: DeleteSink - retry_codes_name: idempotent - - name: UpdateSink - retry_codes_name: idempotent - - name: DeleteExclusion - retry_codes_name: idempotent -- name: google.logging.v2.MetricsServiceV2 - retry_codes_def: - - name: idempotent - retry_codes: - - UNAVAILABLE - - DEADLINE_EXCEEDED - - INTERNAL - methods: - - name: UpdateLogMetric - retry_codes_name: idempotent - - name: DeleteLogMetric - retry_codes_name: idempotent -- name: google.logging.v2.LoggingServiceV2 - retry_codes_def: - - name: idempotent - retry_codes: - - UNAVAILABLE - - DEADLINE_EXCEEDED - - INTERNAL - methods: - - name: DeleteLog - retry_codes_name: idempotent - - name: ListLogEntries - retry_codes_name: idempotent - - name: WriteLogEntries - retry_codes_name: idempotent - batching: - thresholds: - element_count_threshold: 1000 - request_byte_threshold: 1048576 - delay_threshold_millis: 50 - flow_control_element_limit: 100000 - flow_control_byte_limit: 10485760 - flow_control_limit_exceeded_behavior: THROW_EXCEPTION - batch_descriptor: - batched_field: entries - discriminator_fields: - - log_name - - resource - - labels diff --git a/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml deleted file mode 100644 index e26d535faf..0000000000 --- a/src/test/java/com/google/api/generator/gapic/testdata/pubsub_gapic.yaml +++ /dev/null @@ -1,296 +0,0 @@ -type: com.google.api.codegen.ConfigProto -config_schema_version: 2.0.0 -language_settings: - java: - package_name: com.google.cloud.pubsub.v1 - interface_names: - google.pubsub.v1.Publisher: TopicAdmin - google.pubsub.v1.Subscriber: SubscriptionAdmin - release_level: GA - python: - package_name: google.cloud.pubsub_v1.gapic - release_level: GA - go: - package_name: cloud.google.com/go/pubsub/apiv1 - domain_layer_location: cloud.google.com/go/pubsub - release_level: GA - csharp: - package_name: Google.Cloud.PubSub.V1 - interface_names: - google.pubsub.v1.Publisher: PublisherServiceApi - google.pubsub.v1.Subscriber: SubscriberServiceApi - release_level: GA - ruby: - package_name: Google::Cloud::PubSub::V1 - release_level: BETA - php: - package_name: Google\Cloud\PubSub\V1 - release_level: GA - nodejs: - package_name: pubsub.v1 - domain_layer_location: google-cloud - release_level: GA -collections: -# Language overrides are for backward-compatibility in Java. -- entity_name: snapshot - language_overrides: - - language: java - entity_name: project_snapshot -# Language overrides are for backward-compatibility in Java. -- entity_name: subscription - language_overrides: - - language: java - entity_name: project_subscription -interfaces: -- name: google.pubsub.v1.Subscriber - # Deprecated collections are for backward-compatibility in Ruby, PHP and Python - deprecated_collections: - - name_pattern: "projects/{project}/topics/{topic}" - entity_name: topic - lang_doc: - java: To retrieve messages from a subscription, see the Subscriber class. - retry_codes_def: - - name: idempotent - retry_codes: - - UNKNOWN - - ABORTED - - UNAVAILABLE - - name: non_idempotent - retry_codes: - - UNAVAILABLE - - name: streaming_pull - retry_codes: - - DEADLINE_EXCEEDED - - RESOURCE_EXHAUSTED - - ABORTED - - INTERNAL - - UNAVAILABLE - retry_params_def: - - name: default - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 # 60 seconds - initial_rpc_timeout_millis: 60000 # 60 seconds - rpc_timeout_multiplier: 1 - max_rpc_timeout_millis: 60000 # 60 seconds - total_timeout_millis: 600000 # 10 minutes - - name: messaging - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 # 60 seconds - initial_rpc_timeout_millis: 25000 # 25 seconds - rpc_timeout_multiplier: 1 - max_rpc_timeout_millis: 25000 # 25 seconds - total_timeout_millis: 600000 # 10 minutes - - name: streaming_messaging - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 # 60 seconds - initial_rpc_timeout_millis: 600000 # 10 minutes - rpc_timeout_multiplier: 1 - max_rpc_timeout_millis: 600000 # 10 minutes - total_timeout_millis: 600000 # 10 minutes - methods: - # TODO: remove the per method retry_codes_name field once - # https://github.com/googleapis/gapic-generator/issues/3137 - # is fixed - - name: CreateSubscription - retry_codes_name: idempotent - - name: GetSubscription - retry_codes_name: idempotent - - name: UpdateSubscription - retry_codes_name: non_idempotent - sample_code_init_fields: - - update_mask.paths[0]="ack_deadline_seconds" - - subscription.ack_deadline_seconds=42 - - name: ListSubscriptions - retry_codes_name: idempotent - - name: DeleteSubscription - retry_codes_name: non_idempotent - - name: GetSnapshot - surface_treatments: - - include_languages: - - go - - java - - csharp - - ruby - - nodejs - - python - - php - visibility: PACKAGE - - name: ModifyAckDeadline - retry_codes_name: non_idempotent - surface_treatments: - - include_languages: - - java - visibility: PACKAGE - - name: Acknowledge - retry_codes_name: non_idempotent - retry_params_name: messaging - surface_treatments: - - include_languages: - - java - visibility: PACKAGE - - name: Pull - retry_codes_name: idempotent - retry_params_name: messaging - surface_treatments: - - include_languages: - - java - visibility: PACKAGE - - name: StreamingPull - retry_codes_name: streaming_pull - retry_params_name: streaming_messaging - timeout_millis: 900000 - surface_treatments: - - include_languages: - - java - visibility: PACKAGE - - name: ModifyPushConfig - retry_codes_name: non_idempotent - - name: ListSnapshots - retry_codes_name: idempotent - - name: CreateSnapshot - retry_codes_name: non_idempotent - - name: UpdateSnapshot - retry_codes_name: non_idempotent - sample_code_init_fields: - - update_mask.paths[0]="expire_time" - - snapshot.expire_time.seconds=123456 - - name: DeleteSnapshot - retry_codes_name: non_idempotent - - name: Seek - retry_codes_name: idempotent - - name: SetIamPolicy - retry_codes_name: non_idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - surface_treatments: - - include_languages: - - go - visibility: DISABLED - - name: GetIamPolicy - retry_codes_name: idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - surface_treatments: - - include_languages: - - go - visibility: DISABLED - - name: TestIamPermissions - retry_codes_name: non_idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - surface_treatments: - - include_languages: - - go - visibility: DISABLED -- name: google.pubsub.v1.Publisher - lang_doc: - java: To publish messages to a topic, see the Publisher class. - smoke_test: - method: ListTopics - init_fields: - - project%project=$PROJECT_ID - retry_codes_def: - - name: idempotent - retry_codes: - - UNKNOWN - - ABORTED - - UNAVAILABLE - - name: non_idempotent - retry_codes: - - UNAVAILABLE - - name: none - retry_codes: [] - - name: publish - retry_codes: - - ABORTED - - CANCELLED - - INTERNAL - - RESOURCE_EXHAUSTED - - UNKNOWN - - UNAVAILABLE - - DEADLINE_EXCEEDED - retry_params_def: - - name: default - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 # 60 seconds - initial_rpc_timeout_millis: 60000 # 60 seconds - rpc_timeout_multiplier: 1 - max_rpc_timeout_millis: 60000 # 60 seconds - total_timeout_millis: 600000 # 10 minutes - - name: messaging - initial_retry_delay_millis: 100 - retry_delay_multiplier: 1.3 - max_retry_delay_millis: 60000 # 60 seconds - initial_rpc_timeout_millis: 5000 # 5 seconds - rpc_timeout_multiplier: 1.3 - max_rpc_timeout_millis: 60000 # 60 seconds - total_timeout_millis: 60000 # 60 seconds - methods: - - name: CreateTopic - retry_codes_name: non_idempotent - - name: UpdateTopic - retry_codes_name: non_idempotent - - name: Publish - retry_codes_name: publish - retry_params_name: messaging - batching: - thresholds: - element_count_threshold: 100 - element_count_limit: 1000 # TO BE REMOVED LATER - request_byte_threshold: 1048576 # 1 MiB - request_byte_limit: 10485760 # TO BE REMOVED LATER - delay_threshold_millis: 10 - batch_descriptor: - batched_field: messages - discriminator_fields: - - topic - subresponse_field: message_ids - sample_code_init_fields: - - messages[0].data - surface_treatments: - - include_languages: - - java - visibility: PACKAGE - - name: GetTopic - retry_codes_name: idempotent - - name: ListTopics - retry_codes_name: idempotent - - name: ListTopicSubscriptions - retry_codes_name: idempotent - - name: ListTopicSnapshots - surface_treatments: - - include_languages: - - go - - java - - csharp - - ruby - - nodejs - - python - - php - visibility: PACKAGE - - name: DeleteTopic - retry_codes_name: non_idempotent - - name: SetIamPolicy - retry_codes_name: non_idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - # TODO: surface_treatments are here only to make bazel presubmit pass - # they can be removed once presubmit starts using gapic-generator-go - surface_treatments: - - include_languages: - - go - visibility: DISABLED - - name: GetIamPolicy - retry_codes_name: idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - surface_treatments: - - include_languages: - - go - visibility: DISABLED - - name: TestIamPermissions - retry_codes_name: non_idempotent - reroute_to_grpc_interface: google.iam.v1.IAMPolicy - surface_treatments: - - include_languages: - - go - visibility: DISABLED diff --git a/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml b/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml deleted file mode 100644 index 8c50128718..0000000000 --- a/src/test/java/com/google/api/generator/gapic/testdata/showcase_gapic.yaml +++ /dev/null @@ -1,22 +0,0 @@ -# FIXME: Address all the FIXMEs in this generated config before using it for -# client generation. Remove this paragraph after you closed all the FIXMEs. The -# retry_codes_name, required_fields, flattening, and timeout properties cannot -# be precisely decided by the tooling and may require some configuration. -type: com.google.api.codegen.ConfigProto -config_schema_version: 2.0.0 -# The settings of generated code in a specific language. -language_settings: - java: - package_name: com.google.showcase.v1beta1 - python: - package_name: google.showcase_v1beta1.gapic - go: - package_name: cloud.google.com/go/showcase/apiv1beta1 - csharp: - package_name: Google.Showcase.V1beta1 - ruby: - package_name: Google::Showcase::V1beta1 - php: - package_name: Google\Showcase\V1beta1 - nodejs: - package_name: showcase.v1beta1 From ea153c5f9dc64b27b7b7da47b4beb05634be6c12 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Tue, 10 Nov 2020 15:31:37 -0800 Subject: [PATCH 07/13] fix: update tests --- .../logging/LoggingServiceV2StubSettings.java | 97 +------------------ 1 file changed, 5 insertions(+), 92 deletions(-) diff --git a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java index 622fb27f9e..7e6456b3d4 100644 --- a/test/integration/goldens/logging/LoggingServiceV2StubSettings.java +++ b/test/integration/goldens/logging/LoggingServiceV2StubSettings.java @@ -24,11 +24,6 @@ import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.BetaApi; -import com.google.api.gax.batching.BatchingSettings; -import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.batching.FlowController; -import com.google.api.gax.batching.PartitionKey; -import com.google.api.gax.batching.RequestBuilder; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.core.GoogleCredentialsProvider; import com.google.api.gax.core.InstantiatingExecutorProvider; @@ -38,9 +33,6 @@ import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiClientHeaderProvider; -import com.google.api.gax.rpc.BatchedRequestIssuer; -import com.google.api.gax.rpc.BatchingCallSettings; -import com.google.api.gax.rpc.BatchingDescriptor; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.PageContext; import com.google.api.gax.rpc.PagedCallSettings; @@ -67,7 +59,6 @@ import com.google.logging.v2.WriteLogEntriesResponse; import com.google.protobuf.Empty; import java.io.IOException; -import java.util.Collection; import java.util.List; import java.util.Objects; import javax.annotation.Generated; @@ -104,7 +95,7 @@ public class LoggingServiceV2StubSettings extends StubSettings deleteLogSettings; - private final BatchingCallSettings + private final UnaryCallSettings writeLogEntriesSettings; private final PagedCallSettings< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -305,73 +296,13 @@ public ApiFuture getFuturePagedResponse( } }; - private static final BatchingDescriptor - WRITE_LOG_ENTRIES_BATCHING_DESC = - new BatchingDescriptor() { - @Override - public PartitionKey getBatchPartitionKey(WriteLogEntriesRequest request) { - return new PartitionKey( - request.getLogName(), request.getResource(), request.getLabels()); - } - - @Override - public RequestBuilder getRequestBuilder() { - return new RequestBuilder() { - private WriteLogEntriesRequest.Builder builder; - - @Override - public void appendRequest(WriteLogEntriesRequest request) { - if (Objects.isNull(builder)) { - builder = request.toBuilder(); - } else { - builder.addAllEntries(request.getEntriesList()); - } - } - - @Override - public WriteLogEntriesRequest build() { - return builder.build(); - } - }; - } - - @Override - public void splitResponse( - WriteLogEntriesResponse batchResponse, - Collection> batch) { - for (BatchedRequestIssuer responder : batch) { - WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build(); - responder.setResponse(response); - } - } - - @Override - public void splitException( - Throwable throwable, - Collection> batch) { - for (BatchedRequestIssuer responder : batch) { - responder.setException(throwable); - } - } - - @Override - public long countElements(WriteLogEntriesRequest request) { - return request.getEntriesCount(); - } - - @Override - public long countBytes(WriteLogEntriesRequest request) { - return request.getSerializedSize(); - } - }; - /** Returns the object with the settings used for calls to deleteLog. */ public UnaryCallSettings deleteLogSettings() { return deleteLogSettings; } /** Returns the object with the settings used for calls to writeLogEntries. */ - public BatchingCallSettings + public UnaryCallSettings writeLogEntriesSettings() { return writeLogEntriesSettings; } @@ -479,7 +410,7 @@ protected LoggingServiceV2StubSettings(Builder settingsBuilder) throws IOExcepti public static class Builder extends StubSettings.Builder { private final ImmutableList> unaryMethodSettingsBuilders; private final UnaryCallSettings.Builder deleteLogSettings; - private final BatchingCallSettings.Builder + private final UnaryCallSettings.Builder writeLogEntriesSettings; private final PagedCallSettings.Builder< ListLogEntriesRequest, ListLogEntriesResponse, ListLogEntriesPagedResponse> @@ -535,9 +466,7 @@ protected Builder(ClientContext clientContext) { super(clientContext); deleteLogSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); - writeLogEntriesSettings = - BatchingCallSettings.newBuilder(WRITE_LOG_ENTRIES_BATCHING_DESC) - .setBatchingSettings(BatchingSettings.newBuilder().build()); + writeLogEntriesSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); listLogEntriesSettings = PagedCallSettings.newBuilder(LIST_LOG_ENTRIES_PAGE_STR_FACT); listMonitoredResourceDescriptorsSettings = PagedCallSettings.newBuilder(LIST_MONITORED_RESOURCE_DESCRIPTORS_PAGE_STR_FACT); @@ -589,22 +518,6 @@ private static Builder initDefaults(Builder builder) { .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) .setRetrySettings(RETRY_PARAM_DEFINITIONS.get("retry_policy_1_params")); - builder - .writeLogEntriesSettings() - .setBatchingSettings( - BatchingSettings.newBuilder() - .setElementCountThreshold(1000L) - .setRequestByteThreshold(1048576L) - .setDelayThreshold(Duration.ofMillis(50L)) - .setFlowControlSettings( - FlowControlSettings.newBuilder() - .setMaxOutstandingElementCount(100000L) - .setMaxOutstandingRequestBytes(10485760L) - .setLimitExceededBehavior( - FlowController.LimitExceededBehavior.ThrowException) - .build()) - .build()); - builder .writeLogEntriesSettings() .setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("retry_policy_1_codes")) @@ -650,7 +563,7 @@ public UnaryCallSettings.Builder deleteLogSettings() { } /** Returns the builder for the settings used for calls to writeLogEntries. */ - public BatchingCallSettings.Builder + public UnaryCallSettings.Builder writeLogEntriesSettings() { return writeLogEntriesSettings; } From 072224629c02d7d64c109560a3300337588474a9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 14:28:26 -0800 Subject: [PATCH 08/13] fix: handle wildcard-only resource_reference --- .../api/generator/gapic/model/ResourceReference.java | 5 +++++ .../gapic/protoparser/ResourceReferenceParser.java | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java b/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java index 90a63199e3..cbd0ff8548 100644 --- a/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java +++ b/src/main/java/com/google/api/generator/gapic/model/ResourceReference.java @@ -14,6 +14,7 @@ package com.google.api.generator.gapic.model; +import com.google.api.generator.gapic.utils.ResourceNameConstants; import com.google.auto.value.AutoValue; @AutoValue @@ -23,6 +24,10 @@ public abstract class ResourceReference { public abstract boolean isChildType(); + public boolean isOnlyWildcard() { + return resourceTypeString().equals(ResourceNameConstants.WILDCARD_PATTERN); + } + public static ResourceReference withType(String resourceTypeString) { return builder().setResourceTypeString(resourceTypeString).setIsChildType(false).build(); } diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java index 7970598d34..7d198fbb99 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/ResourceReferenceParser.java @@ -43,7 +43,14 @@ public static List parseResourceNames( @Nullable String description, Map resourceNames, Map patternsToResourceNames) { - ResourceName resourceName = resourceNames.get(resourceReference.resourceTypeString()); + ResourceName resourceName = null; + if (resourceReference.isOnlyWildcard()) { + resourceName = ResourceName.createWildcard("*", "com.google.api.wildcard.placeholder"); + resourceNames.put(resourceName.resourceTypeString(), resourceName); + } else { + resourceName = resourceNames.get(resourceReference.resourceTypeString()); + } + resourceName = resourceNames.get(resourceReference.resourceTypeString()); Preconditions.checkNotNull( resourceName, String.format( From 8087ace087c5383d32d296e8baaa4d798cd54505 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 17:49:20 -0800 Subject: [PATCH 09/13] fix: make service.yaml optional --- .../com/google/api/generator/gapic/protoparser/Parser.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 3d41f3d84e..a794bf0d49 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 @@ -65,7 +65,6 @@ public class Parser { private static final String COLON = ":"; private static final String DEFAULT_PORT = "443"; private static final String DOT = "."; - // Allow other parsers to access this. protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser(); @@ -83,7 +82,9 @@ public static GapicContext parse(CodeGeneratorRequest request) { Optional serviceYamlConfigPathOpt = PluginArgumentParser.parseServiceYamlConfigPath(request); Optional serviceYamlProtoOpt = - ServiceYamlParser.parse(serviceYamlConfigPathOpt.get()); + serviceYamlConfigPathOpt.isPresent() + ? ServiceYamlParser.parse(serviceYamlConfigPathOpt.get()) + : Optional.empty(); // Keep message and resource name parsing separate for cleaner logic. // While this takes an extra pass through the protobufs, the extra time is relatively trivial From b9c5d42efa1ec7d012ae0aabd7d4658d2fa9e7ce Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 17:51:28 -0800 Subject: [PATCH 10/13] fix: remove iam, longrunning from codegen for all non-iam,longrunning APIs --- .../generator/gapic/protoparser/Parser.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 a794bf0d49..b46c94cc9d 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 @@ -65,6 +65,10 @@ public class Parser { private static final String COLON = ":"; private static final String DEFAULT_PORT = "443"; private static final String DOT = "."; + + private static final Set DUPE_SERVICE_CODEGEN_BLOCKLIST = + new HashSet<>(Arrays.asList("google.longrunning", "google.iam.v1")); + // Allow other parsers to access this. protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser(); @@ -131,6 +135,26 @@ public static List parseServices( outputArgResourceNames)); } + // Prevent codegen for IAM or LRO if there are other services present, since that is an + // indicator that we are not generating a GAPIC client for IAM or LRO. + Set serviceProtoPackages = + services.stream().map(s -> s.protoPakkage()).collect(Collectors.toSet()); + boolean servicesContainBlocklistedApi = false; + for (String blocklistedPackage : DUPE_SERVICE_CODEGEN_BLOCKLIST) { + // It's very unlikely the blocklisted APIs will contain the other, or any other service. + if (serviceProtoPackages.contains(blocklistedPackage) && serviceProtoPackages.size() > 1) { + servicesContainBlocklistedApi = true; + break; + } + } + + if (servicesContainBlocklistedApi) { + services = + services.stream() + .filter(s -> !DUPE_SERVICE_CODEGEN_BLOCKLIST.contains(s.protoPakkage())) + .collect(Collectors.toList()); + } + return services; } From 727c98eec1e156c0702fb43b6d72fee9d8917a1b Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 17:53:22 -0800 Subject: [PATCH 11/13] feat: handle IAM RPCs in pubsub and KMS APIs --- .../GrpcServiceStubClassComposer.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java index b556f03dcb..2fe59e9a7d 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java @@ -63,9 +63,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import java.util.function.Function; @@ -96,6 +98,13 @@ public class GrpcServiceStubClassComposer implements ClassComposer { private static final Map STATIC_TYPES = createStaticTypes(); + // Legacy support for the original reroute_to_grpc_interface option in gapic.yaml. These two APIs + // predate the modern way, which is to add the RPCs directly into the proto. + private static final Set REROUTE_TO_GRPC_INTERFACE_SERVICE_ALLOWLIST = + new HashSet<>(Arrays.asList("google.cloud.kms.v1", "google.pubsub.v1")); + private static final Set REROUTE_TO_GRPC_INTERFACE_IAM_METHOD_ALLOWLIST = + new HashSet<>(Arrays.asList("SetIamPolicy", "GetIamPolicy", "TestIamPermissions")); + private GrpcServiceStubClassComposer() {} public static GrpcServiceStubClassComposer instance() { @@ -218,8 +227,7 @@ private static Statement createMethodDescriptorVariableDecl( .apply("setType", getMethodDescriptorMethodTypeExpr(protoMethod)) .apply(methodDescriptorMaker); - String codeMethodNameArg = - String.format("%s.%s/%s", service.protoPakkage(), service.name(), protoMethod.name()); + String codeMethodNameArg = getProtoRpcFullMethodName(service, protoMethod); methodDescriptorMaker = methodMakerFn .apply( @@ -1106,6 +1114,17 @@ private static EnumRefExpr getMethodDescriptorMethodTypeExpr(Method protoMethod) .build(); } + private static String getProtoRpcFullMethodName(Service protoService, Method protoMethod) { + if (!REROUTE_TO_GRPC_INTERFACE_SERVICE_ALLOWLIST.contains(protoService.protoPakkage()) + || !REROUTE_TO_GRPC_INTERFACE_IAM_METHOD_ALLOWLIST.contains(protoMethod.name())) { + return String.format( + "%s.%s/%s", protoService.protoPakkage(), protoService.name(), protoMethod.name()); + } + // This is meant to be a temporary workaround until the allow-listed services come up with a + // long-term solution. + return String.format("google.iam.v1.IAMPolicy/%s", protoMethod.name()); + } + private static String getThisClassName(String serviceName) { return String.format(CLASS_NAME_PATTERN, serviceName); } From c100c93a876cd413b1e8af5188a1f057a0cc85a3 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 18:02:27 -0800 Subject: [PATCH 12/13] feat: prevent non-allowlisted APIs from supplying service.yaml --- rules_java_gapic/java_gapic.bzl | 14 ++++++++++++-- test/integration/BUILD.bazel | 3 --- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/rules_java_gapic/java_gapic.bzl b/rules_java_gapic/java_gapic.bzl index 7e6442b8a4..26d87cacaf 100644 --- a/rules_java_gapic/java_gapic.bzl +++ b/rules_java_gapic/java_gapic.bzl @@ -14,6 +14,8 @@ load("@com_google_api_codegen//rules_gapic:gapic.bzl", "proto_custom_library", "unzipped_srcjar") +SERVICE_YAML_ALLOWLIST = ["googleads"] + def _java_gapic_postprocess_srcjar_impl(ctx): gapic_srcjar = ctx.file.gapic_srcjar output_srcjar_name = ctx.label.name @@ -91,9 +93,17 @@ def java_gapic_library( if grpc_service_config: file_args_dict[grpc_service_config] = "grpc-service-config" - # Currently a no-op. + # Check the allow-list. if service_yaml: - file_args_dict[service_yaml] = "gapic-service-config" + service_yaml_in_allowlist = False + for keyword in SERVICE_YAML_ALLOWLIST: + if keyword in service_yaml: + service_yaml_in_allowlist = True + break + if service_yaml_in_allowlist: + file_args_dict[service_yaml] = "gapic-service-config" + else: + fail("Service.yaml is no longer supported in the Java microgenerator") srcjar_name = name + "_srcjar" raw_srcjar_name = srcjar_name + "_raw" diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index c159d127e2..8ffe6cd1bf 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -46,7 +46,6 @@ java_gapic_library( srcs = ["@com_google_googleapis//google/cloud/asset/v1:asset_proto_with_info"], grpc_service_config = "@com_google_googleapis//google/cloud/asset/v1:cloudasset_grpc_service_config.json", package = "google.cloud.asset.v1", - service_yaml = "@com_google_googleapis//google/cloud/asset/v1:cloudasset_v1.yaml", test_deps = [ "@com_google_googleapis//google/cloud/asset/v1:asset_java_grpc", "@com_google_googleapis//google/iam/v1:iam_java_grpc", @@ -88,7 +87,6 @@ java_gapic_library( srcs = ["redis_proto_with_info"], grpc_service_config = "@com_google_googleapis//google/cloud/redis/v1:redis_grpc_service_config.json", package = "google.cloud.redis.v1", - service_yaml = "@com_google_googleapis//google/cloud/redis/v1:redis_v1.yaml", test_deps = [ "@com_google_googleapis//google/cloud/redis/v1:redis_java_grpc", ], @@ -113,7 +111,6 @@ java_gapic_library( srcs = ["@com_google_googleapis//google/logging/v2:logging_proto_with_info"], grpc_service_config = "@com_google_googleapis//google/logging/v2:logging_grpc_service_config.json", package = "google.logging.v2", - service_yaml = "@com_google_googleapis//google/logging/v2:logging.yaml", test_deps = [ "@com_google_googleapis//google/logging/v2:logging_java_grpc", ], From f963f7ec2c96532f3045d8c73a1c109b4f993bda Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Wed, 11 Nov 2020 18:17:05 -0800 Subject: [PATCH 13/13] fix: update goldens --- test/integration/goldens/asset/package-info.java | 4 +--- test/integration/goldens/logging/package-info.java | 4 +--- test/integration/goldens/redis/package-info.java | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/test/integration/goldens/asset/package-info.java b/test/integration/goldens/asset/package-info.java index 519d666f43..e57f641808 100644 --- a/test/integration/goldens/asset/package-info.java +++ b/test/integration/goldens/asset/package-info.java @@ -15,9 +15,7 @@ */ /** - * A client to Cloud Asset API - * - *

The interfaces provided are listed below, along with usage samples. + * The interfaces provided are listed below, along with usage samples. * *

======================= AssetServiceClient ======================= * diff --git a/test/integration/goldens/logging/package-info.java b/test/integration/goldens/logging/package-info.java index f42783ea3f..13c3b7b556 100644 --- a/test/integration/goldens/logging/package-info.java +++ b/test/integration/goldens/logging/package-info.java @@ -15,9 +15,7 @@ */ /** - * A client to Cloud Logging API - * - *

The interfaces provided are listed below, along with usage samples. + * The interfaces provided are listed below, along with usage samples. * *

======================= LoggingServiceV2Client ======================= * diff --git a/test/integration/goldens/redis/package-info.java b/test/integration/goldens/redis/package-info.java index 8b83c3b636..3ed9ea15f2 100644 --- a/test/integration/goldens/redis/package-info.java +++ b/test/integration/goldens/redis/package-info.java @@ -15,9 +15,7 @@ */ /** - * A client to Google Cloud Memorystore for Redis API - * - *

The interfaces provided are listed below, along with usage samples. + * The interfaces provided are listed below, along with usage samples. * *

======================= CloudRedisClient ======================= *