Skip to content

Commit

Permalink
feat: generate resname classes for all refs, message res defs (#595)
Browse files Browse the repository at this point in the history
* fix: fix dep ordering in Bazel dedupe rules

* feat: generate resname classes for all refs, message res defs
  • Loading branch information
miraleung authored Dec 10, 2020
1 parent b59c7e3 commit d8cbb64
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public static List<List<MethodArgument>> parseMethodSignatures(
return signatures;
}

Map<String, ResourceName> patternsToResourceNames = createPatternResourceNameMap(resourceNames);
Map<String, ResourceName> patternsToResourceNames =
ResourceParserHelpers.createPatternResourceNameMap(resourceNames);
Message inputMessage = messageTypes.get(methodInputType.reference().simpleName());

// Example from Expand in echo.proto:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,15 @@ public static GapicContext parse(CodeGeneratorRequest request) {
? ServiceYamlParser.parse(serviceYamlConfigPathOpt.get())
: Optional.empty();

// Collect the resource references seen in messages.
Set<ResourceReference> outputResourceReferencesSeen = new HashSet<>();
// 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
// and is worth the larger reduced maintenance cost.
Map<String, Message> messages = parseMessages(request);
Map<String, Message> messages = parseMessages(request, outputResourceReferencesSeen);
Map<String, ResourceName> resourceNames = parseResourceNames(request);
messages = updateResourceNamesInMessages(messages, resourceNames.values());

Set<ResourceName> outputArgResourceNames = new HashSet<>();
List<Service> services =
parseServices(
Expand All @@ -128,6 +131,25 @@ public static GapicContext parse(CodeGeneratorRequest request) {
outputArgResourceNames,
serviceYamlProtoOpt,
serviceConfigOpt);

Preconditions.checkState(!services.isEmpty(), "No services found to generate");

// Include all resource names present in message types for backwards-compatibility with the
// monolith. In the future, this should be removed on a client library major semver update.
outputArgResourceNames.addAll(
resourceNames.values().stream()
.filter(r -> r.hasParentMessageName())
.collect(Collectors.toSet()));

String servicePackage = services.get(0).pakkage();
Map<String, ResourceName> patternsToResourceNames =
ResourceParserHelpers.createPatternResourceNameMap(resourceNames);
for (ResourceReference resourceReference : outputResourceReferencesSeen) {
outputArgResourceNames.addAll(
ResourceReferenceParser.parseResourceNames(
resourceReference, servicePackage, null, resourceNames, patternsToResourceNames));
}

return GapicContext.builder()
.setServices(services)
.setMessages(messages)
Expand Down Expand Up @@ -278,33 +300,43 @@ public static List<Service> parseService(
.collect(Collectors.toList());
}

public static Map<String, Message> parseMessages(CodeGeneratorRequest request) {
public static Map<String, Message> parseMessages(
CodeGeneratorRequest request, Set<ResourceReference> outputResourceReferencesSeen) {
Map<String, FileDescriptor> fileDescriptors = getFilesToGenerate(request);
Map<String, Message> messages = new HashMap<>();
// Look for message types amongst all the protos, not just the ones to generate. This will
// ensure we track commonly-used protos like Empty.
for (FileDescriptor fileDescriptor : fileDescriptors.values()) {
messages.putAll(parseMessages(fileDescriptor));
messages.putAll(parseMessages(fileDescriptor, outputResourceReferencesSeen));
}

return messages;
}

// TODO(miraleung): Propagate the internal method to all tests, and remove this wrapper.
public static Map<String, Message> parseMessages(FileDescriptor fileDescriptor) {
return parseMessages(fileDescriptor, new HashSet<>());
}

public static Map<String, Message> parseMessages(
FileDescriptor fileDescriptor, Set<ResourceReference> outputResourceReferencesSeen) {
// TODO(miraleung): Preserve nested type and package data in the type key.
Map<String, Message> messages = new HashMap<>();
for (Descriptor messageDescriptor : fileDescriptor.getMessageTypes()) {
messages.putAll(parseMessages(messageDescriptor));
messages.putAll(parseMessages(messageDescriptor, outputResourceReferencesSeen));
}
return messages;
}

private static Map<String, Message> parseMessages(Descriptor messageDescriptor) {
return parseMessages(messageDescriptor, new ArrayList<String>());
private static Map<String, Message> parseMessages(
Descriptor messageDescriptor, Set<ResourceReference> outputResourceReferencesSeen) {
return parseMessages(messageDescriptor, outputResourceReferencesSeen, new ArrayList<String>());
}

private static Map<String, Message> parseMessages(
Descriptor messageDescriptor, List<String> outerNestedTypes) {
Descriptor messageDescriptor,
Set<ResourceReference> outputResourceReferencesSeen,
List<String> outerNestedTypes) {
Map<String, Message> messages = new HashMap<>();
String messageName = messageDescriptor.getName();
if (!messageDescriptor.getNestedTypes().isEmpty()) {
Expand All @@ -313,15 +345,16 @@ private static Map<String, Message> parseMessages(
continue;
}
outerNestedTypes.add(messageName);
messages.putAll(parseMessages(nestedMessage, outerNestedTypes));
messages.putAll(
parseMessages(nestedMessage, outputResourceReferencesSeen, outerNestedTypes));
}
}
messages.put(
messageName,
Message.builder()
.setType(TypeParser.parseType(messageDescriptor))
.setName(messageName)
.setFields(parseFields(messageDescriptor))
.setFields(parseFields(messageDescriptor, outputResourceReferencesSeen))
.setOuterNestedTypes(outerNestedTypes)
.build());
return messages;
Expand Down Expand Up @@ -532,15 +565,21 @@ static String sanitizeDefaultHost(String rawDefaultHost) {
return String.format("%s:%s", rawDefaultHost, DEFAULT_PORT);
}

private static List<Field> parseFields(Descriptor messageDescriptor) {
private static List<Field> parseFields(
Descriptor messageDescriptor, Set<ResourceReference> outputResourceReferencesSeen) {
List<FieldDescriptor> fields = new ArrayList<>(messageDescriptor.getFields());
// Sort by ascending field index order. This is important for paged responses, where the first
// repeated type is taken.
fields.sort((f1, f2) -> f1.getIndex() - f2.getIndex());
return fields.stream().map(f -> parseField(f, messageDescriptor)).collect(Collectors.toList());
return fields.stream()
.map(f -> parseField(f, messageDescriptor, outputResourceReferencesSeen))
.collect(Collectors.toList());
}

private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor messageDescriptor) {
private static Field parseField(
FieldDescriptor fieldDescriptor,
Descriptor messageDescriptor,
Set<ResourceReference> outputResourceReferencesSeen) {
FieldOptions fieldOptions = fieldDescriptor.getOptions();
MessageOptions messageOptions = messageDescriptor.getOptions();
ResourceReference resourceReference = null;
Expand All @@ -560,6 +599,7 @@ private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor mess
isChildType
? ResourceReference.withChildType(childTypeString)
: ResourceReference.withType(typeString);
outputResourceReferencesSeen.add(resourceReference);

} else if (messageOptions.hasExtension(ResourceProto.resource)) {
ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// 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.ResourceName;
import java.util.HashMap;
import java.util.Map;

public class ResourceParserHelpers {
public static Map<String, ResourceName> createPatternResourceNameMap(
Map<String, ResourceName> resourceNames) {
Map<String, ResourceName> patternsToResourceNames = new HashMap<>();
for (ResourceName resourceName : resourceNames.values()) {
for (String pattern : resourceName.patterns()) {
patternsToResourceNames.put(pattern, resourceName);
}
}
return patternsToResourceNames;
}
}
22 changes: 11 additions & 11 deletions test/integration/goldens/logging/ConfigClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ public void getSinkTest() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -724,7 +724,7 @@ public void getSinkTest2() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -770,7 +770,7 @@ public void createSinkTest() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -819,7 +819,7 @@ public void createSinkTest2() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -868,7 +868,7 @@ public void createSinkTest3() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -917,7 +917,7 @@ public void createSinkTest4() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -966,7 +966,7 @@ public void createSinkTest5() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -1015,7 +1015,7 @@ public void updateSinkTest() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -1064,7 +1064,7 @@ public void updateSinkTest2() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -1113,7 +1113,7 @@ public void updateSinkTest3() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down Expand Up @@ -1165,7 +1165,7 @@ public void updateSinkTest4() throws Exception {
LogSink expectedResponse =
LogSink.newBuilder()
.setName(LogSinkName.ofProjectSinkName("[PROJECT]", "[SINK]").toString())
.setDestination("destination-1429847026")
.setDestination(FolderLocationName.of("[FOLDER]", "[LOCATION]").toString())
.setFilter("filter-1274492040")
.setDescription("description-1724546052")
.setDisabled(true)
Expand Down

0 comments on commit d8cbb64

Please sign in to comment.