Skip to content
This repository was archived by the owner on Jun 28, 2022. It is now read-only.

Resource names across different protofiles #2711

Merged
merged 6 commits into from
Apr 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -30,6 +30,7 @@ public abstract class FixedResourceNameConfig implements ResourceNameConfig {

public abstract String getFixedValue();

@Nullable
@Override
public abstract ProtoFile getAssignedProtoFile();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -164,19 +165,32 @@ static GapicInterfaceConfig createInterfaceConfig(
}

ImmutableList.Builder<SingleResourceNameConfig> resourcesBuilder = ImmutableList.builder();
for (CollectionConfigProto collectionConfigProto : interfaceConfigProto.getCollectionsList()) {
String entityName = collectionConfigProto.getEntityName();
ResourceNameConfig resourceName = resourceNameConfigs.get(entityName);
if (!(resourceName instanceof SingleResourceNameConfig)) {
diagCollector.addDiag(
Diag.error(
SimpleLocation.TOPLEVEL,
"Inconsistent configuration - single resource name %s specified for interface, "
+ " but was not found in GapicProductConfig configuration.",
entityName));
return null;
if (protoParser.isProtoAnnotationsEnabled()) {
resourceNameConfigs
.values()
.stream()
.filter(
r ->
r.getResourceNameType() == ResourceNameType.SINGLE
&& Objects.equals(r.getAssignedProtoFile(), apiInterface.getFile()))
.map(r -> (SingleResourceNameConfig) r)
.forEach(resourcesBuilder::add);
} else {
for (CollectionConfigProto collectionConfigProto :
interfaceConfigProto.getCollectionsList()) {
String entityName = collectionConfigProto.getEntityName();
ResourceNameConfig resourceName = resourceNameConfigs.get(entityName);
if (!(resourceName instanceof SingleResourceNameConfig)) {
diagCollector.addDiag(
Diag.error(
SimpleLocation.TOPLEVEL,
"Inconsistent configuration - single resource name %s specified for interface, "
+ " but was not found in GapicProductConfig configuration.",
entityName));
return null;
}
resourcesBuilder.add((SingleResourceNameConfig) resourceName);
}
resourcesBuilder.add((SingleResourceNameConfig) resourceName);
}
ImmutableList<SingleResourceNameConfig> singleResourceNames = resourcesBuilder.build();

Expand Down
128 changes: 59 additions & 69 deletions src/main/java/com/google/api/codegen/config/GapicProductConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.api.codegen.CollectionConfigProto;
import com.google.api.codegen.CollectionOneofProto;
import com.google.api.codegen.ConfigProto;
import com.google.api.codegen.FixedResourceNameValueProto;
import com.google.api.codegen.InterfaceConfigProto;
import com.google.api.codegen.LanguageSettingsProto;
import com.google.api.codegen.MethodConfigProto;
Expand Down Expand Up @@ -48,7 +47,6 @@
import com.google.common.collect.Iterables;
import com.google.protobuf.Api;
import com.google.protobuf.DescriptorProtos;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
Expand Down Expand Up @@ -240,6 +238,7 @@ public static GapicProductConfig create(
if (protoParser.isProtoAnnotationsEnabled()) {
resourceNameConfigs =
createResourceNameConfigsWithProtoFileAndGapicConfig(
model,
diagCollector,
configProto,
packageProtoFile,
Expand All @@ -250,7 +249,7 @@ public static GapicProductConfig create(
} else {
resourceNameConfigs =
createResourceNameConfigsForGapicConfigOnly(
diagCollector, configProto, packageProtoFile, language);
model, diagCollector, configProto, packageProtoFile, language);
}

if (resourceNameConfigs == null) {
Expand Down Expand Up @@ -714,7 +713,8 @@ private static ImmutableMap<String, InterfaceConfig> createDiscoGapicInterfaceCo

private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfigs(
DiagCollector diagCollector, ConfigProto configProto, TargetLanguage language) {
return createResourceNameConfigsForGapicConfigOnly(diagCollector, configProto, null, language);
return createResourceNameConfigsForGapicConfigOnly(
null, diagCollector, configProto, null, language);
}

/**
Expand All @@ -725,6 +725,7 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
@Nullable
static ImmutableMap<String, ResourceNameConfig>
createResourceNameConfigsWithProtoFileAndGapicConfig(
Model model,
DiagCollector diagCollector,
ConfigProto configProto,
@Nullable ProtoFile sampleProtoFile,
Expand Down Expand Up @@ -787,22 +788,14 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
fixedResourceConfigsFromProtoFile.put(fullName.substring(periodIndex + 1), config);
}

List<CollectionConfigProto> allCollectionConfigProtos =
new ArrayList<>(configProto.getCollectionsList());
configProto
.getInterfacesList()
.stream()
.forEach(i -> allCollectionConfigProtos.addAll(i.getCollectionsList()));
Map<CollectionConfigProto, Interface> allCollectionConfigProtos =
getAllCollectionConfigProtos(model, configProto);

ImmutableMap<String, SingleResourceNameConfig> singleResourceNameConfigsFromGapicConfig =
createSingleResourceNamesFromGapicConfigOnly(
diagCollector, allCollectionConfigProtos, sampleProtoFile, language);
ImmutableMap<String, FixedResourceNameConfig> fixedResourceNameConfigsFromGapicConfig =
createFixedResourceNamesFromGapicConfigOnly(
diagCollector,
allCollectionConfigProtos,
configProto.getFixedResourceNameValuesList(),
sampleProtoFile);
createFixedResourceNamesFromGapicConfigOnly(diagCollector, allCollectionConfigProtos);

// Combine the ResourceNameConfigs from the GAPIC and protofile.
ImmutableMap<String, SingleResourceNameConfig> finalSingleResourceNameConfigs =
Expand Down Expand Up @@ -859,27 +852,20 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
@Nullable
private static ImmutableMap<String, ResourceNameConfig>
createResourceNameConfigsForGapicConfigOnly(
Model model,
DiagCollector diagCollector,
ConfigProto configProto,
@Nullable ProtoFile file,
@Nullable ProtoFile sampleProtoFile,
TargetLanguage language) {

List<CollectionConfigProto> allCollectionConfigProtos =
new ArrayList<>(configProto.getCollectionsList());
configProto
.getInterfacesList()
.stream()
.forEach(i -> allCollectionConfigProtos.addAll(i.getCollectionsList()));
Map<CollectionConfigProto, Interface> allCollectionConfigProtos =
getAllCollectionConfigProtos(model, configProto);

ImmutableMap<String, SingleResourceNameConfig> singleResourceNameConfigsFromGapicConfig =
createSingleResourceNamesFromGapicConfigOnly(
diagCollector, allCollectionConfigProtos, file, language);
diagCollector, allCollectionConfigProtos, sampleProtoFile, language);
ImmutableMap<String, FixedResourceNameConfig> fixedResourceNameConfigsFromGapicConfig =
createFixedResourceNamesFromGapicConfigOnly(
diagCollector,
allCollectionConfigProtos,
configProto.getFixedResourceNameValuesList(),
file);
createFixedResourceNamesFromGapicConfigOnly(diagCollector, allCollectionConfigProtos);

validateSingleResourceNameConfigs(singleResourceNameConfigsFromGapicConfig);

Expand All @@ -889,7 +875,7 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
configProto.getCollectionOneofsList(),
singleResourceNameConfigsFromGapicConfig,
fixedResourceNameConfigsFromGapicConfig,
file);
sampleProtoFile);
if (diagCollector.getErrorCount() > 0) {
return null;
}
Expand All @@ -905,43 +891,47 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
private static ImmutableMap<String, SingleResourceNameConfig>
createSingleResourceNamesFromGapicConfigOnly(
DiagCollector diagCollector,
List<CollectionConfigProto> allCollectionConfigs,
ProtoFile protoFile,
Map<CollectionConfigProto, Interface> allCollectionConfigs,
ProtoFile sampleProtoFile,
TargetLanguage language) {
Map<String, SingleResourceNameConfig> singleResourceNameConfigMap = new LinkedHashMap<>();
for (CollectionConfigProto c : allCollectionConfigs) {
if (Strings.isNullOrEmpty(c.getNamePattern())
|| !FixedResourceNameConfig.isFixedResourceNameConfig(c.getNamePattern())) {
for (Map.Entry<CollectionConfigProto, Interface> entry : allCollectionConfigs.entrySet()) {
CollectionConfigProto collectionConfigProto = entry.getKey();
Interface anInterface = entry.getValue();
ProtoFile protoFile = anInterface == null ? sampleProtoFile : anInterface.getFile();
if (Strings.isNullOrEmpty(collectionConfigProto.getNamePattern())
|| !FixedResourceNameConfig.isFixedResourceNameConfig(
collectionConfigProto.getNamePattern())) {
createSingleResourceNameConfigFromGapicConfig(
diagCollector, c, singleResourceNameConfigMap, protoFile, language);
diagCollector, collectionConfigProto, singleResourceNameConfigMap, protoFile, language);
}
}
return ImmutableMap.copyOf(singleResourceNameConfigMap);
}

private static ImmutableMap<String, FixedResourceNameConfig>
createFixedResourceNamesFromGapicConfigOnly(
DiagCollector diagCollector,
List<CollectionConfigProto> allCollectionConfigs,
List<FixedResourceNameValueProto> fixedResourceNameValueProtos,
ProtoFile protoFile) {
DiagCollector diagCollector, Map<CollectionConfigProto, Interface> allCollectionConfigs) {
LinkedHashMap<String, FixedResourceNameConfig> fixedResourceNameConfigMap =
new LinkedHashMap<>();
for (CollectionConfigProto c : allCollectionConfigs) {
if (!Strings.isNullOrEmpty(c.getNamePattern())
&& FixedResourceNameConfig.isFixedResourceNameConfig(c.getNamePattern())) {
for (Map.Entry<CollectionConfigProto, Interface> entry : allCollectionConfigs.entrySet()) {
CollectionConfigProto collectionConfigProto = entry.getKey();
Interface interface_ = entry.getValue();
ProtoFile protoFile = interface_ == null ? null : interface_.getFile();
if (!Strings.isNullOrEmpty(collectionConfigProto.getNamePattern())
&& FixedResourceNameConfig.isFixedResourceNameConfig(
collectionConfigProto.getNamePattern())) {
FixedResourceNameConfig fixedResourceNameConfig =
FixedResourceNameConfig.createFixedResourceNameConfig(
diagCollector, c.getEntityName(), c.getNamePattern(), protoFile);
diagCollector,
collectionConfigProto.getEntityName(),
collectionConfigProto.getNamePattern(),
protoFile);
insertFixedResourceNameConfig(
diagCollector, fixedResourceNameConfig, "", fixedResourceNameConfigMap);
}
}

// TODO(andrealin): Remove this once all fixed resource names are removed.
fixedResourceNameConfigMap.putAll(
createFixedResourceNameConfigs(diagCollector, fixedResourceNameValueProtos, protoFile));

return ImmutableMap.copyOf(fixedResourceNameConfigMap);
}

Expand Down Expand Up @@ -1130,28 +1120,6 @@ private static void insertFixedResourceNameConfig(
}
}

// TODO(andrealin): Remove this once existing fixed resource names are removed.
private static ImmutableMap<String, FixedResourceNameConfig> createFixedResourceNameConfigs(
DiagCollector diagCollector,
Iterable<FixedResourceNameValueProto> fixedConfigProtos,
@Nullable ProtoFile file) {
ImmutableMap.Builder<String, FixedResourceNameConfig> fixedConfigBuilder =
ImmutableMap.builder();
for (FixedResourceNameValueProto fixedConfigProto : fixedConfigProtos) {
FixedResourceNameConfig fixedConfig =
FixedResourceNameConfig.createFixedResourceNameConfig(
diagCollector,
fixedConfigProto.getEntityName(),
fixedConfigProto.getFixedValue(),
file);
if (fixedConfig == null) {
continue;
}
fixedConfigBuilder.put(fixedConfig.getEntityId(), fixedConfig);
}
return fixedConfigBuilder.build();
}

private static ImmutableMap<String, ResourceNameOneofConfig> createResourceNameOneofConfigs(
DiagCollector diagCollector,
Iterable<CollectionOneofProto> oneofConfigProtos,
Expand Down Expand Up @@ -1260,4 +1228,26 @@ public List<LongRunningConfig> getAllLongRunningConfigs() {
.filter(Objects::nonNull)
.collect(Collectors.toList());
}

private static Map<CollectionConfigProto, Interface> getAllCollectionConfigProtos(
@Nullable Model model, ConfigProto configProto) {
Map<CollectionConfigProto, Interface> allCollectionConfigProtos =
configProto
.getCollectionsList()
.stream()
.collect(HashMap::new, (map, config) -> map.put(config, null), HashMap::putAll);
configProto
.getInterfacesList()
.forEach(
i ->
i.getCollectionsList()
.forEach(
c ->
allCollectionConfigProtos.put(
c,
model == null
? null
: model.getSymbolTable().lookupInterface(i.getName()))));
return allCollectionConfigProtos;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public static SingleResourceNameConfig createSingleResourceName(
* returned, and diagnostics are reported to the diag collector.
*/
static SingleResourceNameConfig createSingleResourceName(
Resource resource, String pathTemplate, ProtoFile file, DiagCollector diagCollector) {
Resource resource,
String pathTemplate,
@Nullable ProtoFile file,
DiagCollector diagCollector) {
PathTemplate nameTemplate;
try {
nameTemplate = PathTemplate.create(pathTemplate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.api.tools.framework.model.Diag.Kind;
import com.google.api.tools.framework.model.DiagCollector;
import com.google.api.tools.framework.model.Field;
import com.google.api.tools.framework.model.Interface;
import com.google.api.tools.framework.model.MessageType;
import com.google.api.tools.framework.model.Method;
import com.google.api.tools.framework.model.ProtoFile;
Expand Down Expand Up @@ -70,6 +71,7 @@ public class ResourceNameMessageConfigsTest {
private static final MessageType bookMessage = Mockito.mock(MessageType.class);
private static final ProtoFile protoFile = Mockito.mock(ProtoFile.class);
private static final ImmutableList<ProtoFile> sourceProtoFiles = ImmutableList.of(protoFile);
private static final Interface anInterface = Mockito.mock(Interface.class);
private static final Method insertBook = Mockito.mock(Method.class);

private static final String DEFAULT_PACKAGE = "library";
Expand Down Expand Up @@ -172,6 +174,8 @@ public static void startUp() {

Mockito.doReturn(bookMessage).when(insertBook).getInputMessage();
Mockito.doReturn(protoFile).when(bookMessage).getParent();
Mockito.doReturn(ImmutableList.of(anInterface)).when(protoFile).getInterfaces();
Mockito.doReturn("library.LibraryService").when(anInterface).getFullName();
// Mockito.doReturn("Book").when(protoParser).getResourceReference(bookName);
}

Expand Down Expand Up @@ -271,6 +275,7 @@ public void testCreateResourceNameConfigs() {

Map<String, ResourceNameConfig> resourceNameConfigs =
GapicProductConfig.createResourceNameConfigsWithProtoFileAndGapicConfig(
null,
diagCollector,
configProto,
protoFile,
Expand Down Expand Up @@ -364,6 +369,7 @@ public void testCreateFlattenings() {
protoParser);
ImmutableMap<String, ResourceNameConfig> resourceNameConfigs =
GapicProductConfig.createResourceNameConfigsWithProtoFileAndGapicConfig(
null,
diagCollector,
extraConfigProto,
protoFile,
Expand Down
Loading