Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ggj][codegen] fix: use both map generics in ServiceClientTest codegen #428

Merged
merged 35 commits into from
Oct 31, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
90c4f38
fix: support non-name fields with res-refs in resname def parsing
miraleung Oct 27, 2020
a74be70
fix: add workaround for missing default_host and oauth_scopes annotation
miraleung Oct 27, 2020
aef9843
Merge branch 'master' of github.com:googleapis/gapic-generator-java i…
miraleung Oct 29, 2020
fdb1ffb
fix: clarify LRO parsing error messages
miraleung Oct 27, 2020
642c606
feat: support deeply-nested types in AST and proto message parsing
miraleung Oct 28, 2020
3e26d87
fix: prevent resname tokens from matching subcomponents
miraleung Oct 28, 2020
837da38
fix: use TypeParser for proto message parsing
miraleung Oct 28, 2020
a5fc332
fix: use generic types in field instantiation in ServiceClientTest
miraleung Oct 28, 2020
b71b786
fix: prevent descension into map types in nested message parsing
miraleung Oct 29, 2020
602e1df
fix: merge master
miraleung Oct 29, 2020
07dddb8
fix: use both map generics in ServiceClientTest codegen
miraleung Oct 29, 2020
1d15125
Merge branch 'master' of github.com:googleapis/gapic-generator-java i…
miraleung Oct 30, 2020
7db263e
fix: add workaround for missing default_host and oauth_scopes annotation
miraleung Oct 27, 2020
abebd63
[ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (…
xiaozhenliu-gg5 Oct 26, 2020
f2a1674
[ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (…
xiaozhenliu-gg5 Oct 28, 2020
8f906fa
[ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (…
xiaozhenliu-gg5 Oct 26, 2020
58271c3
[ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (…
xiaozhenliu-gg5 Oct 28, 2020
9cd2c89
[ggj][infra][3/5]feat: add goldens update bazel rules for Redis API (…
xiaozhenliu-gg5 Oct 26, 2020
bb9e50c
[ggj][infra][4/5]feat: add goldens update bazel rules for Asset API (…
xiaozhenliu-gg5 Oct 28, 2020
9792aab
fix: clarify LRO parsing error messages
miraleung Oct 27, 2020
e337c6a
feat: support deeply-nested types in AST and proto message parsing
miraleung Oct 28, 2020
d53ea8a
fix: prevent resname tokens from matching subcomponents
miraleung Oct 28, 2020
b274fdc
fix: use TypeParser for proto message parsing
miraleung Oct 28, 2020
930edb2
fix: merge master
miraleung Oct 30, 2020
74dbafb
fix: use generic types in field instantiation in ServiceClientTest
miraleung Oct 28, 2020
cb03085
fix: prevent descension into map types in nested message parsing
miraleung Oct 29, 2020
c57b166
fix: merge master
miraleung Oct 29, 2020
ff8cd7e
fix: merge master
miraleung Oct 30, 2020
ba0d2a5
fix: merge master
miraleung Oct 30, 2020
5767af9
fix: merge master
miraleung Oct 30, 2020
5bc44ea
fix: prevent descension into map types in nested message parsing
miraleung Oct 29, 2020
a6b57db
build: add logging, redis gradle assembly rules
miraleung Oct 30, 2020
5308fb6
fix: merge master
miraleung Oct 30, 2020
bcc23c7
fix: merge master
miraleung Oct 30, 2020
70fca6d
Merge branch 'master' into alpha/g9
miraleung Oct 31, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,18 @@ public String pakkage() {
public abstract boolean useFullName();

@Override
public String enclosingClassName() {
public ImmutableList<String> enclosingClassNames() {
if (!hasEnclosingClass()) {
return null;
return ImmutableList.of();
}
return clazz().getEnclosingClass().getSimpleName();
// The innermost type will be the last element in the list.
ImmutableList.Builder<String> listBuilder = new ImmutableList.Builder<>();
Class currentClz = clazz();
while (currentClz.getEnclosingClass() != null) {
listBuilder.add(currentClz.getEnclosingClass().getSimpleName());
currentClz = currentClz.getEnclosingClass();
}
return listBuilder.build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public interface Reference {
boolean useFullName();

@Nullable
String enclosingClassName();
ImmutableList<String> enclosingClassNames();

@Nullable
Reference wildcardUpperBound();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
package com.google.api.generator.engine.ast;

import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nullable;
Expand All @@ -43,7 +43,7 @@ public abstract class VaporReference implements Reference {

@Nullable
@Override
public abstract String enclosingClassName();
public abstract ImmutableList<String> enclosingClassNames();

@Nullable
public abstract Reference supertypeReference();
Expand All @@ -56,9 +56,9 @@ public Reference wildcardUpperBound() {

@Override
public String fullName() {
// TODO(unsupported): Nested classes with depth greater than 1.
if (hasEnclosingClass()) {
return String.format("%s.%s.%s", pakkage(), enclosingClassName(), plainName());
return String.format(
"%s.%s.%s", pakkage(), String.join(DOT, enclosingClassNames()), plainName());
}
return String.format("%s.%s", pakkage(), plainName());
}
Expand All @@ -68,7 +68,7 @@ public String fullName() {

@Override
public boolean hasEnclosingClass() {
return !Strings.isNullOrEmpty(enclosingClassName());
return !enclosingClassNames().isEmpty();
}

@Override
Expand All @@ -86,7 +86,7 @@ public boolean isSupertypeOrEquals(Reference other) {
VaporReference ref = (VaporReference) other;
return pakkage().equals(ref.pakkage())
&& plainName().equals(ref.plainName())
&& Objects.equals(enclosingClassName(), ref.enclosingClassName());
&& Objects.equals(enclosingClassNames(), ref.enclosingClassNames());
}

@Override
Expand All @@ -112,14 +112,14 @@ public boolean equals(Object o) {
return pakkage().equals(ref.pakkage())
&& name().equals(ref.name())
&& generics().equals(ref.generics())
&& Objects.equals(enclosingClassName(), ref.enclosingClassName());
&& Objects.equals(enclosingClassNames(), ref.enclosingClassNames());
}

@Override
public int hashCode() {
int hash = 17 * pakkage().hashCode() + 19 * name().hashCode() + 23 * generics().hashCode();
if (!Strings.isNullOrEmpty(enclosingClassName())) {
hash += 29 * enclosingClassName().hashCode();
if (!enclosingClassNames().isEmpty()) {
hash += 29 * enclosingClassNames().hashCode();
}
return hash;
}
Expand All @@ -133,7 +133,8 @@ public static Builder builder() {
return new AutoValue_VaporReference.Builder()
.setUseFullName(false)
.setGenerics(ImmutableList.of())
.setIsStaticImport(false);
.setIsStaticImport(false)
.setEnclosingClassNames(Collections.emptyList());
}

// Private.
Expand All @@ -153,7 +154,11 @@ public Builder setGenerics(Reference... references) {

public abstract Builder setGenerics(List<Reference> clazzes);

public abstract Builder setEnclosingClassName(String enclosingClassName);
public Builder setEnclosingClassNames(String... enclosingClassNames) {
return setEnclosingClassNames(Arrays.asList(enclosingClassNames));
}

public abstract Builder setEnclosingClassNames(List<String> enclosingClassNames);

public abstract Builder setIsStaticImport(boolean isStaticImport);

Expand All @@ -166,8 +171,7 @@ public Builder setGenerics(Reference... references) {

abstract ImmutableList<Reference> generics();

@Nullable
abstract String enclosingClassName();
abstract ImmutableList<String> enclosingClassNames();

abstract boolean isStaticImport();

Expand All @@ -180,11 +184,11 @@ public VaporReference build() {

setPlainName(name());

setIsStaticImport(enclosingClassName() != null && isStaticImport());
setIsStaticImport(!enclosingClassNames().isEmpty() && isStaticImport());

StringBuilder sb = new StringBuilder();
if (enclosingClassName() != null && !isStaticImport()) {
sb.append(enclosingClassName());
if (!enclosingClassNames().isEmpty() && !isStaticImport()) {
sb.append(String.join(DOT, enclosingClassNames()));
sb.append(DOT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import javax.annotation.Nullable;

public class ImportWriterVisitor implements AstNodeVisitor {
private static final String DOT = ".";
private static final String NEWLINE = "\n";
private static final String PKG_JAVA_LANG = "java.lang";

Expand Down Expand Up @@ -423,7 +424,8 @@ private void references(List<Reference> refs) {

if (ref.isStaticImport()
&& !Strings.isNullOrEmpty(currentClassName)
&& ref.enclosingClassName().equals(currentClassName)) {
&& !ref.enclosingClassNames().isEmpty()
&& ref.enclosingClassNames().contains(currentClassName)) {
continue;
}

Expand All @@ -432,7 +434,8 @@ private void references(List<Reference> refs) {
staticImports.add(ref.fullName());
} else {
if (ref.hasEnclosingClass()) {
imports.add(String.format("%s.%s", ref.pakkage(), ref.enclosingClassName()));
imports.add(
String.format("%s.%s", ref.pakkage(), String.join(DOT, ref.enclosingClassNames())));
} else {
imports.add(ref.fullName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private static MethodDefinition createGetRequestBuilderMethod(
TypeNode builderType =
TypeNode.withReference(
VaporReference.builder()
.setEnclosingClassName(method.inputType().reference().name())
.setEnclosingClassNames(method.inputType().reference().name())
.setName("Builder")
.setPakkage(method.inputType().reference().pakkage())
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,22 @@ static Expr createDefaultValue(
}

static Expr createDefaultValue(Field f) {
return createDefaultValue(f, false);
}

static Expr createDefaultValue(Field f, boolean useExplicitInitTypeInGenerics) {
if (f.isRepeated()) {
TypeNode newType =
TypeNode.withReference(
ConcreteReference.withClazz(f.isMap() ? HashMap.class : ArrayList.class));
ConcreteReference.Builder refBuilder =
ConcreteReference.builder().setClazz(f.isMap() ? HashMap.class : ArrayList.class);
if (useExplicitInitTypeInGenerics) {
if (f.isMap()) {
refBuilder = refBuilder.setGenerics(f.type().reference().generics().subList(0, 2));
} else {
refBuilder = refBuilder.setGenerics(f.type().reference().generics().get(0));
}
}

TypeNode newType = TypeNode.withReference(refBuilder.build());
return NewObjectExpr.builder().setType(newType).setIsGeneric(true).build();
}

Expand Down Expand Up @@ -238,7 +250,7 @@ static Expr createSimpleMessageBuilderExpr(
.setReturnType(TypeNode.STRING)
.build();
} else {
defaultExpr = createDefaultValue(field);
defaultExpr = createDefaultValue(field, true);
}
builderExpr =
MethodInvocationExpr.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service, String
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(String.format("%sClient", service.name()))
.setEnclosingClassNames(String.format("%sClient", service.name()))
.setIsStaticImport(true)
.build()))));
return types;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1575,7 +1575,7 @@ private static Map<String, TypeNode> createDynamicTypes(
VaporReference.builder()
.setName("Builder")
.setPakkage(resourceName.pakkage())
.setEnclosingClassName(thisClassName)
.setEnclosingClassNames(thisClassName)
.setIsStaticImport(true)
.build()));

Expand All @@ -1591,7 +1591,7 @@ private static Map<String, TypeNode> createDynamicTypes(
VaporReference.builder()
.setName(s)
.setPakkage(resourceName.pakkage())
.setEnclosingClassName(thisClassName)
.setEnclosingClassNames(thisClassName)
.setIsStaticImport(true)
.build()))));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@
package com.google.api.generator.gapic.composer;

import com.google.api.pathtemplate.PathTemplate;
import com.google.common.base.Preconditions;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class ResourceNameTokenizer {
private static final String SLASH = "/";
private static final String LEFT_BRACE = "{";
private static final String RIGHT_BRACE = "}";
private static final String SLASH = "/";
private static final String EMPTY = "";

private static final String EQUALS_WILDCARD = "=*";
private static final String EQUALS_PATH_WILDCARD = "=**";

static List<List<String>> parseTokenHierarchy(List<String> patterns) {
List<String> nonSlashSepStrings = Arrays.asList("}_{", "}-{", "}.{", "}~{");
Expand All @@ -36,22 +41,38 @@ static List<List<String>> parseTokenHierarchy(List<String> patterns) {
String[] patternTokens = pattern.split(SLASH);
for (String patternToken : patternTokens) {
if (patternToken.startsWith(LEFT_BRACE) && patternToken.endsWith(RIGHT_BRACE)) {
String processedPatternToken = patternToken;
String processedPatternToken =
// Replacement order matters - ensure the first is not a subcomponent of the second.
patternToken.replace(EQUALS_PATH_WILDCARD, EMPTY).replace(EQUALS_WILDCARD, EMPTY);

// Handle non-slash separators.
if (nonSlashSepStrings.stream().anyMatch(s -> patternToken.contains(s))) {
for (String str : nonSlashSepStrings) {
processedPatternToken = processedPatternToken.replace(str, "_");
}
} else {
final int processedPatternTokenLength = processedPatternToken.length();
// Handles wildcards.
processedPatternToken =
List<String> candidateVars =
vars.stream()
.filter(v -> patternToken.contains(v))
.collect(Collectors.toList())
.get(0);
// Check that the token size is within ~3 of the var, to avoid mismatching on
// variables with same-named subcomponents.
// Otherwise, "customer_client_link" will match with "customer".
.filter(
v ->
patternToken.contains(v)
// Accounting for braces.
&& processedPatternTokenLength - v.length() < 3)
.collect(Collectors.toList());
Preconditions.checkState(
!candidateVars.isEmpty(),
String.format(
"No variable candidates found for token %s in pattern %s",
processedPatternToken, pattern));
processedPatternToken = candidateVars.get(0);
}
hierarchy.add(processedPatternToken.replace("{", "").replace("}", ""));
hierarchy.add(
processedPatternToken.replace(LEFT_BRACE, EMPTY).replace(RIGHT_BRACE, EMPTY));
}
}
tokenHierachies.add(hierarchy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,7 @@ private static Map<String, TypeNode> createVaporTypes(Service service) {
VaporReference.builder()
.setName(
String.format(t, JavaStyle.toUpperCamelCase(method.name())))
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setPakkage(service.pakkage())
.setIsStaticImport(true) // Same class, so they won't be imported.
.build()))));
Expand All @@ -1465,7 +1465,7 @@ private static Map<String, TypeNode> createVaporTypes(Service service) {
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setIsStaticImport(true)
.build()))));
return types;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service) {
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setIsStaticImport(true)
.build()))));
return types;
Expand Down Expand Up @@ -1864,7 +1864,7 @@ private static TypeNode getPagedResponseType(Method method, Service service) {
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setIsStaticImport(true)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ private static MethodDefinition createCreatorMethod(
TypeNode.withReference(
VaporReference.builder()
.setName("Builder")
.setEnclosingClassName(String.format("%sSettings", service.name()))
.setEnclosingClassNames(String.format("%sSettings", service.name()))
.setPakkage(service.pakkage())
.build());
Expr returnMethodExpr =
Expand Down Expand Up @@ -712,7 +712,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service) {
TypeNode.withReference(
VaporReference.builder()
.setName(BUILDER_CLASS_NAME)
.setEnclosingClassName(getThisClassName(service.name()))
.setEnclosingClassNames(getThisClassName(service.name()))
.setPakkage(service.pakkage())
.setIsStaticImport(true)
.build()));
Expand All @@ -729,7 +729,7 @@ private static Map<String, TypeNode> createDynamicTypes(Service service) {
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setIsStaticImport(true)
.build()))));
return types;
Expand Down Expand Up @@ -828,7 +828,7 @@ private static TypeNode getStubSettingsBuilderType(Service service) {
VaporReference.builder()
.setPakkage(service.pakkage())
.setName(BUILDER_CLASS_NAME)
.setEnclosingClassName(getStubSettingsClassName(service.name()))
.setEnclosingClassNames(getStubSettingsClassName(service.name()))
.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ private static Map<String, TypeNode> createTypes(
VaporReference.builder()
.setName(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, m.name()))
.setPakkage(service.pakkage())
.setEnclosingClassName(getClientClassName(service.name()))
.setEnclosingClassNames(getClientClassName(service.name()))
.setIsStaticImport(true)
.build()))));

Expand Down
Loading