google.api.resource patterns don't support '~' character
aohren opened this issue · 6 comments
Specifying a compound resource pattern like the following (that includes non-'/' separators between variables) throws an error.
option (google.api.resource) = {
type: "googleads.googleapis.com/AdGroupAd"
pattern: "customers/{customer}/adGroupAds/{ad_group_id}~{ad_id}"
};
gapic-generator fails with the following exception:
java.lang.IllegalArgumentException: Name: identifiers are not all either lower-underscore or lower-camel: ["ad_group_id}~{ad_id", ]
at com.google.api.codegen.util.Name.anyLower(Name.java:67)
at com.google.api.codegen.transformer.InitCodeTransformer.getFormatFunctionArgs(InitCodeTransformer.java:867)
at com.google.api.codegen.transformer.InitCodeTransformer.createResourceNameInitValueView(InitCodeTransformer.java:857)
at com.google.api.codegen.transformer.InitCodeTransformer.createInitValueView(InitCodeTransformer.java:800)
at com.google.api.codegen.transformer.InitCodeTransformer.setInitValueAndComments(InitCodeTransformer.java:672)
at com.google.api.codegen.transformer.InitCodeTransformer.generateSimpleInitCodeLine(InitCodeTransformer.java:526)
at com.google.api.codegen.transformer.InitCodeTransformer.generateSurfaceInitCodeLine(InitCodeTransformer.java:491)
at com.google.api.codegen.transformer.InitCodeTransformer.getFieldSettings(InitCodeTransformer.java:918)
at com.google.api.codegen.transformer.InitCodeTransformer.buildInitCodeView(InitCodeTransformer.java:419)
at com.google.api.codegen.transformer.InitCodeTransformer.buildInitCodeViewFlattened(InitCodeTransformer.java:284)
at com.google.api.codegen.transformer.InitCodeTransformer.generateInitCode(InitCodeTransformer.java:133)
at com.google.api.codegen.transformer.SampleTransformer.generateSample(SampleTransformer.java:312)
at com.google.api.codegen.transformer.SampleTransformer.generateSamples(SampleTransformer.java:222)
at com.google.api.codegen.transformer.SampleTransformer.generateSamples(SampleTransformer.java:169)
at com.google.api.codegen.transformer.SampleTransformer.generateSamples(SampleTransformer.java:135)
at com.google.api.codegen.transformer.StaticLangApiMethodTransformer.setFlattenedMethodFields(StaticLangApiMethodTransformer.java:812)
at com.google.api.codegen.transformer.StaticLangApiMethodTransformer.generateFlattenedMethod(StaticLangApiMethodTransformer.java:356)
at com.google.api.codegen.transformer.StaticLangApiMethodTransformer.generateFlattenedMethod(StaticLangApiMethodTransformer.java:337)
at com.google.api.codegen.transformer.java.JavaApiMethodTransformer.generateUnaryMethods(JavaApiMethodTransformer.java:123)
at com.google.api.codegen.transformer.java.JavaApiMethodTransformer.generateApiMethods(JavaApiMethodTransformer.java:106)
at com.google.api.codegen.transformer.java.JavaApiMethodTransformer.lambda$generateApiMethods$0(JavaApiMethodTransformer.java:61)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1654)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
at com.google.api.codegen.transformer.java.JavaApiMethodTransformer.generateApiMethods(JavaApiMethodTransformer.java:63)
at com.google.api.codegen.transformer.java.JavaSurfaceTransformer.generateApiClass(JavaSurfaceTransformer.java:189)
at com.google.api.codegen.transformer.java.JavaSurfaceTransformer.generateApiFile(JavaSurfaceTransformer.java:168)
at com.google.api.codegen.transformer.java.JavaSurfaceTransformer.transform(JavaSurfaceTransformer.java:125)
at com.google.api.codegen.transformer.java.JavaGapicSurfaceTransformer.transform(JavaGapicSurfaceTransformer.java:74)
at com.google.api.codegen.transformer.java.JavaGapicSurfaceTransformer.transform(JavaGapicSurfaceTransformer.java:36)
at com.google.api.codegen.gapic.GapicGenerator.generate(GapicGenerator.java:64)
at com.google.api.codegen.gapic.GapicGeneratorApp.process(GapicGeneratorApp.java:246)
at com.google.api.tools.framework.tools.GenericToolDriverBase.run(GenericToolDriverBase.java:90)
at com.google.api.tools.framework.tools.ToolDriverBase.run(ToolDriverBase.java:73)
at com.google.api.codegen.GeneratorMain.gapicGeneratorMain(GeneratorMain.java:338)
at com.google.api.codegen.GeneratorMain.main(GeneratorMain.java:184)
This is preventing generation of useful resource name parsing code. For example in Java, we're trying to generate *Name classes that use the above pattern and have builders like:
public static AdGroupAdName of(String customer, String adGroupId, String adId) {
return newBuilder()
.setCustomer(customer)
.setAdGroupId(adGroupId)
.setAdId(adId)
.build();
}
Instead, we're having to workaround this by specifying pattern as "customers/{customer}/adGroupAds/{ad_group_ad}". This masks the fact that {ad_group_ad} is actually of the form {ad_group_id}~{ad_id}. And consequently, the generated helper code is confusing to users:
public static AdGroupAdName of(String customer, String adGroupAd) {
return newBuilder()
.setCustomer(customer)
.setAdGroupAd(adGroupAd)
.build();
}
The above code has no way of conveying to users that adGroupAd is a composite of ~-separated IDs.
AIP-4231 indicates that "Patterns are usually slash-separated, but this is not guaranteed. Client libraries should use string interpolation to piece together the resource name." Thus, I'd expect a valid URL character like ~ to be supported.
+1 to this, it causes issues with all resource names in Google Ads API.
@chrisdunelm , assigning this to you since you have started looking into it.
This is fixed.
We just discovered an issue with this that causes the GAPIC generated Name classes to not work properly. Reported separately in api-commons-java as https://github.com/googleapis/api-common-java/issues/154.
But want to flag here as well, since it ultimately means this isn't working properly yet.
Oops, my mistake - I thought this was in the C# repo!
Re-opening now.
This has been fixed, closing.