chore: update googleapis commitish to 694f87c#13886
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Java Ad Manager client library by adding new service clients (CreativeSetServiceClient and SlateServiceClient), introducing several batch methods across existing clients (such as ContentBundleServiceClient, OrderServiceClient, and TargetingPresetServiceClient), and updating API documentation. The review feedback highlights a potential NullPointerException risk in the auto-generated clients where @nullable parameters are passed directly to Protobuf setters, suggesting an update to the code generator to handle null values safely.
| public final CreativeSet getCreativeSet(@Nullable CreativeSetName name) { | ||
| GetCreativeSetRequest request = | ||
| GetCreativeSetRequest.newBuilder().setName(name == null ? null : name.toString()).build(); | ||
| return getCreativeSet(request); |
There was a problem hiding this comment.
The parameter name is annotated with @Nullable, indicating that null is a valid argument. However, if name is null, the expression name == null ? null : name.toString() evaluates to null, which is then passed directly to GetCreativeSetRequest.Builder.setName(null). In Protobuf Java, passing null to a string field setter throws a NullPointerException.
Since this file is auto-generated, please avoid editing it manually. Instead, the generator (gapic-generator-java) should be updated to handle nullable resource name parameters safely (for example, by only calling the setter if the value is non-null, or by using an empty string if appropriate).
References
- When annotating a method with @nullable, verify if any callers pass the returned value directly to APIs that do not accept nulls. If null checks are missing, they should be added or tracked for follow-up work.
| public final BatchActivateContentBundlesResponse batchActivateContentBundles( | ||
| @Nullable NetworkName parent, List<String> names) { | ||
| BatchActivateContentBundlesRequest request = | ||
| BatchActivateContentBundlesRequest.newBuilder() | ||
| .setParent(parent == null ? null : parent.toString()) | ||
| .addAllNames(names) | ||
| .build(); | ||
| return batchActivateContentBundles(request); | ||
| } |
There was a problem hiding this comment.
The parameter parent is annotated with @Nullable, indicating that null is a valid argument. However, if parent is null, the expression parent == null ? null : parent.toString() evaluates to null, which is then passed directly to BatchActivateContentBundlesRequest.Builder.setParent(null). In Protobuf Java, passing null to a string field setter throws a NullPointerException.
Since this file is auto-generated, please avoid editing it manually. Instead, the generator (gapic-generator-java) should be updated to handle nullable resource name parameters safely (for example, by only calling the setter if the value is non-null, or by using an empty string if appropriate).
References
- When annotating a method with @nullable, verify if any callers pass the returned value directly to APIs that do not accept nulls. If null checks are missing, they should be added or tracked for follow-up work.
Updated googleapis commitish in librarian.yaml to googleapis/googleapis@694f87c
💡 Note: If this PR is still open when the daily update workflow runs next, it will be closed and replaced with a new PR containing the latest updates.