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

ResourceNameOneofConfig fixes #2704

Merged
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
31 changes: 16 additions & 15 deletions src/main/java/com/google/api/codegen/config/GapicProductConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -764,14 +764,6 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
fullyQualifiedFixedResourceNameConfigsFromProtoFile =
ImmutableMap.copyOf(fullyQualifiedFixedResourcesFromProtoFileCollector);

ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromProtoFile =
createResourceNameOneofConfigsFromProtoFile(
diagCollector,
fullyQualifiedSingleResourceNameConfigsFromProtoFile,
fullyQualifiedFixedResourceNameConfigsFromProtoFile,
resourceSetDefs,
protoParser);

// Populate a SingleResourceNameConfigs map, using just the unqualified names.
LinkedHashMap<String, SingleResourceNameConfig> singleResourceConfigsFromProtoFile =
new LinkedHashMap<>();
Expand Down Expand Up @@ -809,11 +801,26 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
sampleProtoFile);

// Combine the ResourceNameConfigs from the GAPIC and protofile.
Map<String, SingleResourceNameConfig> finalSingleResourceNameConfigs =
ImmutableMap<String, SingleResourceNameConfig> finalSingleResourceNameConfigs =
mergeSingleResourceNameConfigsFromGapicConfigAndProtoFile(
singleResourceNameConfigsFromGapicConfig, singleResourceConfigsFromProtoFile);
validateSingleResourceNameConfigs(finalSingleResourceNameConfigs);

// TODO(andrealin): Remove this once explicit fixed resource names are gone-zos.
ImmutableMap<String, FixedResourceNameConfig> finalFixedResourceNameConfigs =
mergeResourceNameConfigs(
diagCollector,
fixedResourceNameConfigsFromGapicConfig,
fixedResourceConfigsFromProtoFile);

ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromProtoFile =
createResourceNameOneofConfigsFromProtoFile(
diagCollector,
finalSingleResourceNameConfigs,
finalFixedResourceNameConfigs,
resourceSetDefs,
protoParser);

// TODO(andrealin): Remove this once ResourceSets are approved.
ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromGapicConfig =
createResourceNameOneofConfigs(
Expand All @@ -826,12 +833,6 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
return null;
}

// TODO(andrealin): Remove this once explicit fixed resource names are gone-zos.
Map<String, FixedResourceNameConfig> finalFixedResourceNameConfigs =
mergeResourceNameConfigs(
diagCollector,
fixedResourceNameConfigsFromGapicConfig,
fixedResourceConfigsFromProtoFile);
// TODO(andrealin): Remove this once ResourceSets are approved.
Map<String, ResourceNameOneofConfig> finalResourceOneofNameConfigs =
mergeResourceNameConfigs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;

/** ResourceNameOneofConfig represents the configuration for a oneof set of resource names. */
@AutoValue
Expand Down Expand Up @@ -108,13 +109,13 @@ static ResourceNameOneofConfig createResourceNameOneof(
DiagCollector diagCollector,
ResourceSet resourceSet,
String oneOfName,
ImmutableMap<String, SingleResourceNameConfig> fileLevelSingleResourceNameConfigs,
ImmutableMap<String, FixedResourceNameConfig> fileLevelFixedResourceNameConfigs,
ImmutableMap<String, SingleResourceNameConfig> singleResourceNameConfigs,
ImmutableMap<String, FixedResourceNameConfig> fixedResourceNameConfigs,
ProtoParser protoParser,
ProtoFile file) {

if (fileLevelSingleResourceNameConfigs.containsKey(oneOfName)
|| fileLevelFixedResourceNameConfigs.containsKey(oneOfName)) {
if (singleResourceNameConfigs.containsKey(oneOfName)
|| fixedResourceNameConfigs.containsKey(oneOfName)) {
diagCollector.addDiag(
Diag.error(
SimpleLocation.TOPLEVEL,
Expand All @@ -129,34 +130,46 @@ static ResourceNameOneofConfig createResourceNameOneof(
List<String> resourceReferences = resourceSet.getResourceReferencesList();

for (Resource resource : resourceDefs) {
SingleResourceNameConfig singleResourceNameConfig =
SingleResourceNameConfig.createSingleResourceName(
resource, resource.getPattern(), file, diagCollector);
configList.add(singleResourceNameConfig);
}
for (String resourceRef : resourceReferences) {
if (Strings.isNullOrEmpty(resourceRef)) {
if (Strings.isNullOrEmpty(resource.getSymbol())) {
diagCollector.addDiag(
Diag.error(
SimpleLocation.TOPLEVEL,
"name is required for Resources in a ResourceSet,"
+ " but ResourceSet %s has at least one Resource without a name.",
oneOfName));
return null;
String.format(
"google.api.Resource with pattern \"%s\" "
+ "and nested inside ResourceSet \"%s\" was not given a symbol",
resource.getPattern(), resourceSet.getSymbol())));
continue;
}
String qualifiedRef = resourceRef;
if (!qualifiedRef.contains(".")) {
qualifiedRef = protoParser.getProtoPackage(file) + "." + resourceRef;

SingleResourceNameConfig singleResourceNameConfig =
singleResourceNameConfigs.get(resource.getSymbol());
if (singleResourceNameConfig != null) {
configList.add(singleResourceNameConfig);
continue;
}

FixedResourceNameConfig fixedResourceNameConfig =
fixedResourceNameConfigs.get(resource.getSymbol());
if (fixedResourceNameConfig != null) {
configList.add(fixedResourceNameConfig);
continue;
}

// This shouldn't happen because the resource config maps were
// previously constructed from Resource objects.
throw new IllegalStateException(
String.format(
"Failed to create a ResourceNameConfig for the protofile-defined Resource %s.",
resource.getSymbol()));
}
for (String resourceRef : resourceReferences) {
String resourceName =
StringUtils.removeStart(resourceRef, protoParser.getProtoPackage(file) + ".");
ResourceNameConfig resourceNameConfig;
if (fileLevelSingleResourceNameConfigs.containsKey(qualifiedRef)) {
resourceNameConfig = fileLevelSingleResourceNameConfigs.get(qualifiedRef);
} else if (fileLevelSingleResourceNameConfigs.containsKey(resourceRef)) {
resourceNameConfig = fileLevelSingleResourceNameConfigs.get(resourceRef);
} else if (fileLevelFixedResourceNameConfigs.containsKey(qualifiedRef)) {
resourceNameConfig = fileLevelFixedResourceNameConfigs.get(qualifiedRef);
} else if (fileLevelFixedResourceNameConfigs.containsKey(resourceRef)) {
resourceNameConfig = fileLevelFixedResourceNameConfigs.get(resourceRef);
if (singleResourceNameConfigs.containsKey(resourceName)) {
resourceNameConfig = singleResourceNameConfigs.get(resourceName);
} else if (fixedResourceNameConfigs.containsKey(resourceName)) {
resourceNameConfig = fixedResourceNameConfigs.get(resourceName);
} else {
diagCollector.addDiag(
Diag.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1929,7 +1929,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
* BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name, altBookName);
* }
Expand Down Expand Up @@ -1957,7 +1957,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
* BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name.toString(), altBookName.toString());
* }
Expand Down Expand Up @@ -1985,7 +1985,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
* GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
* .setName(name.toString())
Expand All @@ -2009,7 +2009,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
* GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
* .setName(name.toString())
Expand All @@ -2032,7 +2032,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name);
* }
* </code></pre>
Expand All @@ -2056,7 +2056,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name.toString());
* }
* </code></pre>
Expand All @@ -2080,7 +2080,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
* .setName(name.toString())
* .build();
Expand All @@ -2102,7 +2102,7 @@ public class LibraryServiceClient implements BackgroundResource {
* Sample code:
* <pre><code>
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
* BookOneOfName name = DeletedBookName.of();
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
* GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
* .setName(name.toString())
* .build();
Expand Down Expand Up @@ -7677,7 +7677,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void createBookTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -7776,7 +7776,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void getBookTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -7911,7 +7911,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void updateBookTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -7962,7 +7962,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void updateBookTest2() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -8022,7 +8022,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void moveBookTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -8254,7 +8254,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void getBookFromAnywhereTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand All @@ -8266,7 +8266,7 @@ public class LibraryServiceClientTest {
.build();
mockLibraryService.addResponse(expectedResponse);

BookOneOfName name = DeletedBookName.of();
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");

BookFromAnywhere actualResponse =
Expand All @@ -8292,7 +8292,7 @@ public class LibraryServiceClientTest {
mockLibraryService.addException(exception);

try {
BookOneOfName name = DeletedBookName.of();
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");

client.getBookFromAnywhere(name, altBookName);
Expand All @@ -8305,7 +8305,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void getBookFromAbsolutelyAnywhereTest() {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand All @@ -8317,7 +8317,7 @@ public class LibraryServiceClientTest {
.build();
mockLibraryService.addResponse(expectedResponse);

BookOneOfName name = DeletedBookName.of();
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");

BookFromAnywhere actualResponse =
client.getBookFromAbsolutelyAnywhere(name);
Expand All @@ -8341,7 +8341,7 @@ public class LibraryServiceClientTest {
mockLibraryService.addException(exception);

try {
BookOneOfName name = DeletedBookName.of();
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");

client.getBookFromAbsolutelyAnywhere(name);
Assert.fail("No exception raised");
Expand Down Expand Up @@ -8443,7 +8443,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void streamBooksTest() throws Exception {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down Expand Up @@ -8612,7 +8612,7 @@ public class LibraryServiceClientTest {
@Test
@SuppressWarnings("all")
public void getBigBookTest() throws Exception {
BookOneOfName name2 = DeletedBookName.of();
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
String author = "author-1406328437";
String title = "title110371416";
boolean read = true;
Expand Down