feat(generator): add model flag and allowlist parser for resumable upload RPCs - #14317
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for identifying resumable upload methods in the GAPIC generator by adding an isResumableUpload property to the Method model and updating the Parser to match RPC names against an allowlist. The review feedback suggests populating the RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS with the showcase service's pattern to make the parser logic functional and testable, which would also allow the removal of a manual workaround in TestProtoLoader and enable a proper assertion in ParserTest.
| private static final ImmutableList<Pattern> RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS = | ||
| ImmutableList.of(); |
There was a problem hiding this comment.
The RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS list is currently empty, which means no RPCs will ever be identified as resumable uploads by the parser. To make this functional and testable, we should add the showcase service's pattern to this list. This also allows us to write a proper assertion in ParserTest and avoid the manual workaround in TestProtoLoader.
| private static final ImmutableList<Pattern> RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS = | |
| ImmutableList.of(); | |
| private static final ImmutableList<Pattern> RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS = | |
| ImmutableList.of( | |
| Pattern.compile("google\\.showcase\\.v1beta1\\.ResumableUploadService\\.UploadMedia")); |
There was a problem hiding this comment.
This omission is intentional - allowlist will be populated when all composers are complete, unit tests use a different mechanism to enable for now.
There was a problem hiding this comment.
Do we have to wait until composers are complete? Is it to prevent accidentally generation? I think we can at least add the showcase methods here.
There was a problem hiding this comment.
My main motivation here is so that we can generate the showcase ResumableUploadService client library cleanly without resumable upload support (#14324) followed immediately by the regeneration with support added via being allowlisted (#14325). IMO the diff on the latter PR gives a clear rollup overview of the resumable upload changes across the client library that's much easier to review than if the whole generation was bundled together, and adds value that we don't get from just the diffs we see in the individual composer PRs.
I originally tried to do the baseline generation earlier followed by allowlisting (before the composer changes) but I couldn't get it to work across intermediate PRs with both verify.sh working AND the generated library compiling. So keeping the allowlist empty until all composers are in place is was what I landed on.
| Method uploadMethod = methods.get(0); | ||
| assertEquals("UploadMedia", uploadMethod.name()); | ||
| assertFalse(uploadMethod.isResumableUpload()); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| return GapicContext.builder() | ||
| .setMessages(messageTypes) | ||
| .setResourceNames(resourceNames) | ||
| .setServices(adaptShowcaseResumableUploadForTest(services)) | ||
| .setHelperResourceNames(outputResourceNames) | ||
| .setTransport(transport) | ||
| .setServiceConfig(GapicServiceConfig.create(Optional.empty())) | ||
| .build(); | ||
| } | ||
|
|
||
| private static List<Service> adaptShowcaseResumableUploadForTest(List<Service> services) { | ||
| return services.stream() | ||
| .map( | ||
| s -> | ||
| s.toBuilder() | ||
| .setMethods( | ||
| s.methods().stream() | ||
| .map( | ||
| m -> | ||
| m.name().equals("UploadMedia") | ||
| ? m.toBuilder().setIsResumableUpload(true).build() | ||
| : m) | ||
| .collect(Collectors.toList())) | ||
| .build()) | ||
| .collect(Collectors.toList()); | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
e7acfcd to
aa32171
Compare
aa32171 to
681c369
Compare
dbc1403 to
c7dc7e4
Compare
c7dc7e4 to
8351695
Compare
8351695 to
9b6b8b4
Compare
7f9c11a to
0447951
Compare
| option (google.api.default_host) = "localhost:7469"; | ||
|
|
||
| // A method with media_upload annotation enabled. | ||
| rpc UploadMedia(UploadMediaRequest) returns (UploadMediaResponse) { |
There was a problem hiding this comment.
I see that there is only one RPC in this service, which is OK as a starting point. In the follow up PRs, we should have a regular RPC in the same service to verify that the regular RPC is not generated in the resumable upload stub and vice versa.
0447951 to
068f71f
Compare
068f71f to
f5dc929
Compare
|
|
🤖 I have created a release *beep* *boop* --- <details><summary>1.92.0</summary> ## [1.92.0](v1.91.0...v1.92.0) (2026-09-23) ### Features * **bigquery-jdbc:** add `EnableTimestampPicos` connection property and its plumbing ([#14284](#14284)) ([b4aa5ac](b4aa5ac)) * **bigquery-jdbc:** implement picosecond temporal math and formatting engine ([#14286](#14286)) ([2a9612a](2a9612a)) * **bigquery-jdbc:** support picosecond in REST JSON path and nested types ([#14334](#14334)) ([15ffe4a](15ffe4a)) * **bigquery-jdbc:** support picosecond in `PreparedStatement` parameters and batching ([#14373](#14373)) ([c1aac66](c1aac66)) * **bigquery-jdbc:** support picosecond timestamp in `ResultSetMetaData` and `DatabaseMetaData` ([#14358](#14358)) ([43acdd3](43acdd3)) * **bigquery-jdbc:** support picosecond timestamps in Arrow Storage Read API and nested types ([#14332](#14332)) ([b5d9aca](b5d9aca)) * **bigquery-jdbc:** support qualified project delimiter in `DefaultDataset` property ([#14240](#14240)) ([6e8d6c8](6e8d6c8)) * **bigquery:** accelerate row-based query() with Arrow wire format ([#14405](#14405)) ([8d12a8f](8d12a8f)) * **bigquery:** add ArrowDeserializer helper utility ([#13943](#13943)) ([d9a298b](d9a298b)) * **bigquery:** add ArrowQueryPageFetcher for Arrow query result pagination ([#14404](#14404)) ([615409f](615409f)) * **bigquery:** add ArrowQueryResult and ArrowQueryResultImpl for Arrow result streaming ([#13944](#13944)) ([a62fdf8](a62fdf8)) * **bigquery:** add Storage Read API slow-path fallback for row-based query() ([#14409](#14409)) ([26e568a](26e568a)) * **bigquery:** add zero-copy queryArrow API for Arrow VectorSchemaRoot streaming ([#14402](#14402)) ([b44ffe8](b44ffe8)) * **bigquery:** make BigQuery AutoCloseable with default no-op close method ([#14434](#14434)) ([00bf3de](00bf3de)) * **firestore:** add support for BSON types ([#13189](#13189)) ([8a123d9](8a123d9)) * **gax:** add ApiCallContext and request-level settings overloads to ResumableUploadCallable ([#14251](#14251)) ([e8cbd42](e8cbd42)) * **gax:** add globalTimeout settings field to ResumableUploadCallSettings ([#14253](#14253)) ([438cda6](438cda6)) * **gax:** add resumable upload error classification and retry algorithm ([#14419](#14419)) ([b70396d](b70396d)) * **gax:** add ResumableUploadCallable creation to Callables and HttpJsonCallableFactory ([#14242](#14242)) ([7de24de](7de24de)) * **gax:** implement baseline Callable and Future for resumable uploads ([#14241](#14241)) ([5a54db9](5a54db9)) * **generator:** add model flag and allowlist parser for resumable upload RPCs ([#14317](#14317)) ([acc1856](acc1856)) * **generator:** emit resumable upload client surface ([#14319](#14319)) ([a9fed00](a9fed00)) * **generator:** emit resumable upload settings and HttpJson upload stub ([#14321](#14321)) ([c122474](c122474)) * **generator:** enable resumable upload generation for showcase ([#14325](#14325)) ([f9ebd79](f9ebd79)) * **generator:** switch resumable upload specialized stubs to package private ([#14471](#14471)) ([0d4e875](0d4e875)) * **generator:** wire transport stub delegation to resumable upload stubs ([#14322](#14322)) ([cc4b980](cc4b980)) * **google/cloud/backupdr/v1beta:** add backupdr ([#14410](#14410)) ([a4a47da](a4a47da)) * **google/cloud/networkservices/v1beta1:** add networkservices ([#14407](#14407)) ([21c4955](21c4955)) * **pubsub:** add publish telemetry headers for publish attempt observability ([#14338](#14338)) ([c167ab8](c167ab8)) * **pubsub:** implement publish hedging to reduce tail latency ([#13735](#13735)) ([b302615](b302615)) * **spanner:** Support dynamic TLS certificate and key rotation for Spanner Omni ([#14456](#14456)) ([ffc745c](ffc745c)) * **storage/control:** add delete folder recursive sample ([#13642](#13642)) ([f4b1b46](f4b1b46)) * **storage/control:** add delete folder recursive sample ([#14397](#14397)) ([2c01d55](2c01d55)) ### Bug Fixes * **auth:** restore transportFactory upon deserialization in InternalAwsSecurityCredentialsSupplier ([#14340](#14340)) ([beea42f](beea42f)) * **bigquery-jdbc:** ensure row ordering in PCNT IT ([#14330](#14330)) ([a16f048](a16f048)) * **bigquery-jdbc:** fix htapi fallback due to permission logic ([#14418](#14418)) ([21e6dc8](21e6dc8)) * **bigquery-jdbc:** fix Timestamp assertions ([#14290](#14290)) ([533ba14](533ba14)) * **bigquery-jdbc:** handle null parameters in Storage Write API bulk inserts ([#14270](#14270)) ([dd2c41a](dd2c41a)), refs [#14066](#14066) * **bigquery-jdbc:** handle SQL NULLs in ResultSet primitive getters ([#14383](#14383)) ([8e464fe](8e464fe)), refs [#14371](#14371) * **bigquery:** default Arrow pagination stream location to US instead of global ([#14458](#14458)) ([2775eb1](2775eb1)) * **bigquery:** preserve page token and paginate correctly in Arrow query when maxResults is set ([#14469](#14469)) ([f5601f4](f5601f4)) * **bigquery:** use first page row count for Arrow query pagination offset ([#14466](#14466)) ([9d10dd0](9d10dd0)) * **bigtable:** don't notify config listeners while holding the manager lock ([#14294](#14294)) ([4426ccd](4426ccd)) * **bigtable:** fall back to classic path when per-RPC CallCredentials are set on session path ([#14477](#14477)) ([57bacb0](57bacb0)) * **bigtable:** fix abnormal session closures and scale-up in session pool ([#14431](#14431)) ([6361ecd](6361ecd)) * **biqguery:** fix undeclared QueryParameter wiring in QueryStatistics ([#14401](#14401)) ([64cf1d3](64cf1d3)) * **bom:** restore google-cloud-spanner-jdbc to libraries-bom ([#14362](#14362)) ([bc7be5e](bc7be5e)), refs [#14347](#14347) * **spanner:** honor maxAttempts and totalTimeout in streaming resume loop ([#14370](#14370)) ([305f47d](305f47d)) * **spanner:** only set snapshot isolation read timestamp for SI or optimistic txns in CloudClientExecutor ([#14346](#14346)) ([54c0d0f](54c0d0f)) * **spanner:** prevent statement cancellation race in AbstractBaseUnitOfWork ([#14283](#14283)) ([d9a8eef](d9a8eef)) * **spanner:** re-enable ITInstanceAdminTest on cloud-devel and cloud-staging ([#14281](#14281)) ([89a8268](89a8268)) ### Performance Improvements * **spanner:** stop re-parsing the request id on every RPC ([#14353](#14353)) ([46108f4](46108f4)) ### Documentation * Add a Http/Json Post-Quantum Cryptography Guide ([#13963](#13963)) ([fcc65b0](fcc65b0)) * **bigquery:** add QueryArrow code sample and document JDK 17+ JVM requirements ([#14437](#14437)) ([bd363f6](bd363f6)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>



Adds the
isResumableUpload()property to the generator'sMethodmodel and wires pattern matching inParser.javaagainst an initially empty allowlist. The showcase test proto is also added with a hermetic test adapter to enable isolated unit testing across subsequent composers.