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

Commit 28e8372

Browse files
andreamlinandreamlin
authored
ResourceNameOneofConfig fixes (#2704)
When creating a ResourceNameOneofConfig from protofile annotations, use the already-created collections of SingleResourceNameConfigs and FixedResourceNameConfigs derived from the protofile annotations; this allows for GAPIC config language-specific overrides for resource names.
1 parent 5be1b6d commit 28e8372

File tree

3 files changed

+76
-62
lines changed

3 files changed

+76
-62
lines changed

‎src/main/java/com/google/api/codegen/config/GapicProductConfig.java

+16-15
Original file line numberDiff line numberDiff line change
@@ -764,14 +764,6 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
764764
fullyQualifiedFixedResourceNameConfigsFromProtoFile =
765765
ImmutableMap.copyOf(fullyQualifiedFixedResourcesFromProtoFileCollector);
766766

767-
ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromProtoFile =
768-
createResourceNameOneofConfigsFromProtoFile(
769-
diagCollector,
770-
fullyQualifiedSingleResourceNameConfigsFromProtoFile,
771-
fullyQualifiedFixedResourceNameConfigsFromProtoFile,
772-
resourceSetDefs,
773-
protoParser);
774-
775767
// Populate a SingleResourceNameConfigs map, using just the unqualified names.
776768
LinkedHashMap<String, SingleResourceNameConfig> singleResourceConfigsFromProtoFile =
777769
new LinkedHashMap<>();
@@ -809,11 +801,26 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
809801
sampleProtoFile);
810802

811803
// Combine the ResourceNameConfigs from the GAPIC and protofile.
812-
Map<String, SingleResourceNameConfig> finalSingleResourceNameConfigs =
804+
ImmutableMap<String, SingleResourceNameConfig> finalSingleResourceNameConfigs =
813805
mergeSingleResourceNameConfigsFromGapicConfigAndProtoFile(
814806
singleResourceNameConfigsFromGapicConfig, singleResourceConfigsFromProtoFile);
815807
validateSingleResourceNameConfigs(finalSingleResourceNameConfigs);
816808

809+
// TODO(andrealin): Remove this once explicit fixed resource names are gone-zos.
810+
ImmutableMap<String, FixedResourceNameConfig> finalFixedResourceNameConfigs =
811+
mergeResourceNameConfigs(
812+
diagCollector,
813+
fixedResourceNameConfigsFromGapicConfig,
814+
fixedResourceConfigsFromProtoFile);
815+
816+
ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromProtoFile =
817+
createResourceNameOneofConfigsFromProtoFile(
818+
diagCollector,
819+
finalSingleResourceNameConfigs,
820+
finalFixedResourceNameConfigs,
821+
resourceSetDefs,
822+
protoParser);
823+
817824
// TODO(andrealin): Remove this once ResourceSets are approved.
818825
ImmutableMap<String, ResourceNameOneofConfig> resourceNameOneofConfigsFromGapicConfig =
819826
createResourceNameOneofConfigs(
@@ -826,12 +833,6 @@ private static ImmutableMap<String, ResourceNameConfig> createResourceNameConfig
826833
return null;
827834
}
828835

829-
// TODO(andrealin): Remove this once explicit fixed resource names are gone-zos.
830-
Map<String, FixedResourceNameConfig> finalFixedResourceNameConfigs =
831-
mergeResourceNameConfigs(
832-
diagCollector,
833-
fixedResourceNameConfigsFromGapicConfig,
834-
fixedResourceConfigsFromProtoFile);
835836
// TODO(andrealin): Remove this once ResourceSets are approved.
836837
Map<String, ResourceNameOneofConfig> finalResourceOneofNameConfigs =
837838
mergeResourceNameConfigs(

‎src/main/java/com/google/api/codegen/config/ResourceNameOneofConfig.java

+39-26
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.ArrayList;
3131
import java.util.List;
3232
import javax.annotation.Nullable;
33+
import org.apache.commons.lang3.StringUtils;
3334

3435
/** ResourceNameOneofConfig represents the configuration for a oneof set of resource names. */
3536
@AutoValue
@@ -108,13 +109,13 @@ static ResourceNameOneofConfig createResourceNameOneof(
108109
DiagCollector diagCollector,
109110
ResourceSet resourceSet,
110111
String oneOfName,
111-
ImmutableMap<String, SingleResourceNameConfig> fileLevelSingleResourceNameConfigs,
112-
ImmutableMap<String, FixedResourceNameConfig> fileLevelFixedResourceNameConfigs,
112+
ImmutableMap<String, SingleResourceNameConfig> singleResourceNameConfigs,
113+
ImmutableMap<String, FixedResourceNameConfig> fixedResourceNameConfigs,
113114
ProtoParser protoParser,
114115
ProtoFile file) {
115116

116-
if (fileLevelSingleResourceNameConfigs.containsKey(oneOfName)
117-
|| fileLevelFixedResourceNameConfigs.containsKey(oneOfName)) {
117+
if (singleResourceNameConfigs.containsKey(oneOfName)
118+
|| fixedResourceNameConfigs.containsKey(oneOfName)) {
118119
diagCollector.addDiag(
119120
Diag.error(
120121
SimpleLocation.TOPLEVEL,
@@ -129,34 +130,46 @@ static ResourceNameOneofConfig createResourceNameOneof(
129130
List<String> resourceReferences = resourceSet.getResourceReferencesList();
130131

131132
for (Resource resource : resourceDefs) {
132-
SingleResourceNameConfig singleResourceNameConfig =
133-
SingleResourceNameConfig.createSingleResourceName(
134-
resource, resource.getPattern(), file, diagCollector);
135-
configList.add(singleResourceNameConfig);
136-
}
137-
for (String resourceRef : resourceReferences) {
138-
if (Strings.isNullOrEmpty(resourceRef)) {
133+
if (Strings.isNullOrEmpty(resource.getSymbol())) {
139134
diagCollector.addDiag(
140135
Diag.error(
141136
SimpleLocation.TOPLEVEL,
142-
"name is required for Resources in a ResourceSet,"
143-
+ " but ResourceSet %s has at least one Resource without a name.",
144-
oneOfName));
145-
return null;
137+
String.format(
138+
"google.api.Resource with pattern \"%s\" "
139+
+ "and nested inside ResourceSet \"%s\" was not given a symbol",
140+
resource.getPattern(), resourceSet.getSymbol())));
141+
continue;
146142
}
147-
String qualifiedRef = resourceRef;
148-
if (!qualifiedRef.contains(".")) {
149-
qualifiedRef = protoParser.getProtoPackage(file) + "." + resourceRef;
143+
144+
SingleResourceNameConfig singleResourceNameConfig =
145+
singleResourceNameConfigs.get(resource.getSymbol());
146+
if (singleResourceNameConfig != null) {
147+
configList.add(singleResourceNameConfig);
148+
continue;
149+
}
150+
151+
FixedResourceNameConfig fixedResourceNameConfig =
152+
fixedResourceNameConfigs.get(resource.getSymbol());
153+
if (fixedResourceNameConfig != null) {
154+
configList.add(fixedResourceNameConfig);
155+
continue;
150156
}
157+
158+
// This shouldn't happen because the resource config maps were
159+
// previously constructed from Resource objects.
160+
throw new IllegalStateException(
161+
String.format(
162+
"Failed to create a ResourceNameConfig for the protofile-defined Resource %s.",
163+
resource.getSymbol()));
164+
}
165+
for (String resourceRef : resourceReferences) {
166+
String resourceName =
167+
StringUtils.removeStart(resourceRef, protoParser.getProtoPackage(file) + ".");
151168
ResourceNameConfig resourceNameConfig;
152-
if (fileLevelSingleResourceNameConfigs.containsKey(qualifiedRef)) {
153-
resourceNameConfig = fileLevelSingleResourceNameConfigs.get(qualifiedRef);
154-
} else if (fileLevelSingleResourceNameConfigs.containsKey(resourceRef)) {
155-
resourceNameConfig = fileLevelSingleResourceNameConfigs.get(resourceRef);
156-
} else if (fileLevelFixedResourceNameConfigs.containsKey(qualifiedRef)) {
157-
resourceNameConfig = fileLevelFixedResourceNameConfigs.get(qualifiedRef);
158-
} else if (fileLevelFixedResourceNameConfigs.containsKey(resourceRef)) {
159-
resourceNameConfig = fileLevelFixedResourceNameConfigs.get(resourceRef);
169+
if (singleResourceNameConfigs.containsKey(resourceName)) {
170+
resourceNameConfig = singleResourceNameConfigs.get(resourceName);
171+
} else if (fixedResourceNameConfigs.containsKey(resourceName)) {
172+
resourceNameConfig = fixedResourceNameConfigs.get(resourceName);
160173
} else {
161174
diagCollector.addDiag(
162175
Diag.error(

‎src/test/java/com/google/api/codegen/protoannotations/testdata/java_library_no_gapic_config.baseline

+21-21
Original file line numberDiff line numberDiff line change
@@ -1929,7 +1929,7 @@ public class LibraryServiceClient implements BackgroundResource {
19291929
* Sample code:
19301930
* <pre><code>
19311931
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
1932-
* BookOneOfName name = DeletedBookName.of();
1932+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
19331933
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
19341934
* BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name, altBookName);
19351935
* }
@@ -1957,7 +1957,7 @@ public class LibraryServiceClient implements BackgroundResource {
19571957
* Sample code:
19581958
* <pre><code>
19591959
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
1960-
* BookOneOfName name = DeletedBookName.of();
1960+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
19611961
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
19621962
* BookFromAnywhere response = libraryServiceClient.getBookFromAnywhere(name.toString(), altBookName.toString());
19631963
* }
@@ -1985,7 +1985,7 @@ public class LibraryServiceClient implements BackgroundResource {
19851985
* Sample code:
19861986
* <pre><code>
19871987
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
1988-
* BookOneOfName name = DeletedBookName.of();
1988+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
19891989
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
19901990
* GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
19911991
* .setName(name.toString())
@@ -2009,7 +2009,7 @@ public class LibraryServiceClient implements BackgroundResource {
20092009
* Sample code:
20102010
* <pre><code>
20112011
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
2012-
* BookOneOfName name = DeletedBookName.of();
2012+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
20132013
* BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
20142014
* GetBookFromAnywhereRequest request = GetBookFromAnywhereRequest.newBuilder()
20152015
* .setName(name.toString())
@@ -2032,7 +2032,7 @@ public class LibraryServiceClient implements BackgroundResource {
20322032
* Sample code:
20332033
* <pre><code>
20342034
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
2035-
* BookOneOfName name = DeletedBookName.of();
2035+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
20362036
* BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name);
20372037
* }
20382038
* </code></pre>
@@ -2056,7 +2056,7 @@ public class LibraryServiceClient implements BackgroundResource {
20562056
* Sample code:
20572057
* <pre><code>
20582058
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
2059-
* BookOneOfName name = DeletedBookName.of();
2059+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
20602060
* BookFromAnywhere response = libraryServiceClient.getBookFromAbsolutelyAnywhere(name.toString());
20612061
* }
20622062
* </code></pre>
@@ -2080,7 +2080,7 @@ public class LibraryServiceClient implements BackgroundResource {
20802080
* Sample code:
20812081
* <pre><code>
20822082
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
2083-
* BookOneOfName name = DeletedBookName.of();
2083+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
20842084
* GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
20852085
* .setName(name.toString())
20862086
* .build();
@@ -2102,7 +2102,7 @@ public class LibraryServiceClient implements BackgroundResource {
21022102
* Sample code:
21032103
* <pre><code>
21042104
* try (LibraryServiceClient libraryServiceClient = LibraryServiceClient.create()) {
2105-
* BookOneOfName name = DeletedBookName.of();
2105+
* BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
21062106
* GetBookFromAbsolutelyAnywhereRequest request = GetBookFromAbsolutelyAnywhereRequest.newBuilder()
21072107
* .setName(name.toString())
21082108
* .build();
@@ -7677,7 +7677,7 @@ public class LibraryServiceClientTest {
76777677
@Test
76787678
@SuppressWarnings("all")
76797679
public void createBookTest() {
7680-
BookOneOfName name2 = DeletedBookName.of();
7680+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
76817681
String author = "author-1406328437";
76827682
String title = "title110371416";
76837683
boolean read = true;
@@ -7776,7 +7776,7 @@ public class LibraryServiceClientTest {
77767776
@Test
77777777
@SuppressWarnings("all")
77787778
public void getBookTest() {
7779-
BookOneOfName name2 = DeletedBookName.of();
7779+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
77807780
String author = "author-1406328437";
77817781
String title = "title110371416";
77827782
boolean read = true;
@@ -7911,7 +7911,7 @@ public class LibraryServiceClientTest {
79117911
@Test
79127912
@SuppressWarnings("all")
79137913
public void updateBookTest() {
7914-
BookOneOfName name2 = DeletedBookName.of();
7914+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
79157915
String author = "author-1406328437";
79167916
String title = "title110371416";
79177917
boolean read = true;
@@ -7962,7 +7962,7 @@ public class LibraryServiceClientTest {
79627962
@Test
79637963
@SuppressWarnings("all")
79647964
public void updateBookTest2() {
7965-
BookOneOfName name2 = DeletedBookName.of();
7965+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
79667966
String author = "author-1406328437";
79677967
String title = "title110371416";
79687968
boolean read = true;
@@ -8022,7 +8022,7 @@ public class LibraryServiceClientTest {
80228022
@Test
80238023
@SuppressWarnings("all")
80248024
public void moveBookTest() {
8025-
BookOneOfName name2 = DeletedBookName.of();
8025+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
80268026
String author = "author-1406328437";
80278027
String title = "title110371416";
80288028
boolean read = true;
@@ -8254,7 +8254,7 @@ public class LibraryServiceClientTest {
82548254
@Test
82558255
@SuppressWarnings("all")
82568256
public void getBookFromAnywhereTest() {
8257-
BookOneOfName name2 = DeletedBookName.of();
8257+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
82588258
String author = "author-1406328437";
82598259
String title = "title110371416";
82608260
boolean read = true;
@@ -8266,7 +8266,7 @@ public class LibraryServiceClientTest {
82668266
.build();
82678267
mockLibraryService.addResponse(expectedResponse);
82688268

8269-
BookOneOfName name = DeletedBookName.of();
8269+
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
82708270
BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
82718271

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

82948294
try {
8295-
BookOneOfName name = DeletedBookName.of();
8295+
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
82968296
BookName altBookName = BookName.of("[SHELF_ID]", "[BOOK_ID]");
82978297

82988298
client.getBookFromAnywhere(name, altBookName);
@@ -8305,7 +8305,7 @@ public class LibraryServiceClientTest {
83058305
@Test
83068306
@SuppressWarnings("all")
83078307
public void getBookFromAbsolutelyAnywhereTest() {
8308-
BookOneOfName name2 = DeletedBookName.of();
8308+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
83098309
String author = "author-1406328437";
83108310
String title = "title110371416";
83118311
boolean read = true;
@@ -8317,7 +8317,7 @@ public class LibraryServiceClientTest {
83178317
.build();
83188318
mockLibraryService.addResponse(expectedResponse);
83198319

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

83228322
BookFromAnywhere actualResponse =
83238323
client.getBookFromAbsolutelyAnywhere(name);
@@ -8341,7 +8341,7 @@ public class LibraryServiceClientTest {
83418341
mockLibraryService.addException(exception);
83428342

83438343
try {
8344-
BookOneOfName name = DeletedBookName.of();
8344+
BookOneOfName name = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
83458345

83468346
client.getBookFromAbsolutelyAnywhere(name);
83478347
Assert.fail("No exception raised");
@@ -8443,7 +8443,7 @@ public class LibraryServiceClientTest {
84438443
@Test
84448444
@SuppressWarnings("all")
84458445
public void streamBooksTest() throws Exception {
8446-
BookOneOfName name2 = DeletedBookName.of();
8446+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
84478447
String author = "author-1406328437";
84488448
String title = "title110371416";
84498449
boolean read = true;
@@ -8612,7 +8612,7 @@ public class LibraryServiceClientTest {
86128612
@Test
86138613
@SuppressWarnings("all")
86148614
public void getBigBookTest() throws Exception {
8615-
BookOneOfName name2 = DeletedBookName.of();
8615+
BookOneOfName name2 = ArchivedBookName.of("[ARCHIVE_PATH]", "[BOOK_ID]");
86168616
String author = "author-1406328437";
86178617
String title = "title110371416";
86188618
boolean read = true;

0 commit comments

Comments
 (0)