Skip to content
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public enum Stream {

public abstract boolean isBatching();

public abstract boolean isResumableUpload();

public boolean isPaged() {
return pageSizeFieldName() != null;
}
Expand Down Expand Up @@ -137,7 +139,8 @@ public static Builder builder() {
.setIsInternalApi(false)
.setIsBatching(false)
.setIsDeprecated(false)
.setOperationPollingMethod(false);
.setOperationPollingMethod(false)
.setIsResumableUpload(false);
}

public static Stream toStream(boolean isClientStreaming, boolean isServerStreaming) {
Expand Down Expand Up @@ -177,6 +180,8 @@ public abstract static class Builder {

public abstract Builder setIsBatching(boolean isBatching);

public abstract Builder setIsResumableUpload(boolean isResumableUpload);

public abstract Builder setPageSizeFieldName(String pagedFieldName);

public abstract Builder setIsDeprecated(boolean isDeprecated);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.jspecify.annotations.NullMarked;
Expand Down Expand Up @@ -134,6 +135,9 @@
"google.cloud.bigquery.v2.ModelService.ListModels",
"google.cloud.bigquery.v2.TableService.ListTables");

private static final ImmutableList<Pattern> RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS =
ImmutableList.of();
Comment on lines +138 to +139

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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"));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This omission is intentional - allowlist will be populated when all composers are complete, unit tests use a different mechanism to enable for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@whowes whowes Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Allow other parsers to access this.
protected static final SourceCodeInfoParser SOURCE_CODE_INFO_PARSER = new SourceCodeInfoParser();

Expand Down Expand Up @@ -817,6 +821,31 @@
Optional<com.google.api.Service> serviceYamlProtoOpt,
Set<ResourceName> outputArgResourceNames,
Transport transport) {
return parseMethods(
serviceDescriptor,
protoPackage,
servicePackage,
messageTypes,
resourceNames,
serviceConfigOpt,
serviceYamlProtoOpt,
outputArgResourceNames,
transport,
RESUMABLE_UPLOAD_ALLOWLIST_PATTERNS);
}

@VisibleForTesting
static List<Method> parseMethods(

Check warning on line 838 in sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Method has 10 parameters, which is greater than 7 authorized.

See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_generator&issues=AaCRiFteAssgigl7pfvH&open=AaCRiFteAssgigl7pfvH&pullRequest=14317
ServiceDescriptor serviceDescriptor,
String protoPackage,
String servicePackage,
Map<String, Message> messageTypes,
Map<String, ResourceName> resourceNames,
Optional<GapicServiceConfig> serviceConfigOpt,
Optional<com.google.api.Service> serviceYamlProtoOpt,
Set<ResourceName> outputArgResourceNames,
Transport transport,
List<Pattern> resumableUploadAllowlistPatterns) {
List<Method> methods = new ArrayList<>();

// Parse the serviceYaml for autopopulated methods and fields once and put into a map
Expand Down Expand Up @@ -872,6 +901,9 @@
.getOptions()
.getExtension(ExtendedOperationsProto.operationPollingMethod)
: false;
boolean isResumableUpload =
resumableUploadAllowlistPatterns.stream()
.anyMatch(pattern -> pattern.matcher(protoMethod.getFullName()).matches());
RoutingHeaderRule routingHeaderRule =
RoutingRuleParser.parse(protoMethod, inputMessage, messageTypes);
methods.add(
Expand All @@ -895,6 +927,7 @@
.setAutoPopulatedFields(autoPopulatedFields)
.setRoutingHeaderRule(routingHeaderRule)
.setIsBatching(isBatching)
.setIsResumableUpload(isResumableUpload)
.setPageSizeFieldName(parsePageSizeFieldName(protoMethod, messageTypes, transport))
.setIsDeprecated(isDeprecated)
.setOperationPollingMethod(operationPollingMethod)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest;
import com.google.selective.generate.v1beta1.SelectiveApiGenerationOuterClass;
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.ResumableUpload;
import com.google.showcase.v1beta1.TestingOuterClass;
import com.google.testgapic.v1beta1.LockerProto;
import java.nio.file.Path;
Expand All @@ -66,6 +67,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.junit.Assert;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -163,6 +165,7 @@ void parseMethods_basic() {
assertEquals(echoMethod.name(), "Echo");
assertEquals(echoMethod.stream(), Method.Stream.NONE);
assertEquals(false, echoMethod.hasAutoPopulatedFields());
assertFalse(echoMethod.isResumableUpload());

// Detailed method signature parsing tests are in a separate unit test.
List<List<MethodArgument>> methodSignatures = echoMethod.methodSignatures();
Expand Down Expand Up @@ -203,6 +206,67 @@ void parseMethods_basic() {
assertEquals(false, chatMethod.hasAutoPopulatedFields());
}

@Test
void parseMethods_resumableUpload() {
FileDescriptor resumableUploadFileDescriptor = ResumableUpload.getDescriptor();
ServiceDescriptor resumableUploadService = resumableUploadFileDescriptor.getServices().get(0);
Map<String, Message> messageTypes = Parser.parseMessages(resumableUploadFileDescriptor);
Map<String, ResourceName> resourceNames =
Parser.parseResourceNames(resumableUploadFileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
String protoPackage = resumableUploadFileDescriptor.getPackage();
String servicePackage = TypeParser.getPackage(resumableUploadFileDescriptor);
List<Method> methods =
Parser.parseMethods(
resumableUploadService,
protoPackage,
servicePackage,
messageTypes,
resourceNames,
Optional.empty(),
Optional.empty(),
outputResourceNames,
Transport.GRPC);

assertEquals(1, methods.size());
Method uploadMethod = methods.get(0);
assertEquals("UploadMedia", uploadMethod.name());
assertFalse(uploadMethod.isResumableUpload());
Comment on lines +232 to +234

This comment was marked as outdated.

}

@Test
void parseMethods_resumableUpload_withConfiguredAllowlist() {
FileDescriptor resumableUploadFileDescriptor = ResumableUpload.getDescriptor();
ServiceDescriptor resumableUploadService = resumableUploadFileDescriptor.getServices().get(0);
Map<String, Message> messageTypes = Parser.parseMessages(resumableUploadFileDescriptor);
Map<String, ResourceName> resourceNames =
Parser.parseResourceNames(resumableUploadFileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
String protoPackage = resumableUploadFileDescriptor.getPackage();
String servicePackage = TypeParser.getPackage(resumableUploadFileDescriptor);
List<Pattern> allowlist =
Arrays.asList(
Pattern.compile(
"^google\\.showcase\\.v1beta1\\.ResumableUploadService\\.UploadMedia$"));
List<Method> methods =
Parser.parseMethods(
resumableUploadService,
protoPackage,
servicePackage,
messageTypes,
resourceNames,
Optional.empty(),
Optional.empty(),
outputResourceNames,
Transport.GRPC,
allowlist);

assertEquals(1, methods.size());
Method uploadMethod = methods.get(0);
assertEquals("UploadMedia", uploadMethod.name());
assertTrue(uploadMethod.isResumableUpload());
}

@Test
void parseMethods_basicLro() {
Map<String, Message> messageTypes = Parser.parseMessages(echoFileDescriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.IdentityOuterClass;
import com.google.showcase.v1beta1.MessagingOuterClass;
import com.google.showcase.v1beta1.ResumableUpload;
import com.google.showcase.v1beta1.TestingOuterClass;
import com.google.test.callablenamingtype.CallableNameType;
import com.google.testdata.v1.DeprecatedServiceOuterClass;
Expand Down Expand Up @@ -278,6 +279,46 @@ public GapicContext parseShowcaseTesting() {
.build();
}

public GapicContext parseShowcaseResumableUpload() {
FileDescriptor fileDescriptor = ResumableUpload.getDescriptor();
ServiceDescriptor serviceDescriptor = fileDescriptor.getServices().get(0);
assertEquals("ResumableUploadService", serviceDescriptor.getName());

Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
Set<ResourceName> outputResourceNames = new HashSet<>();
List<Service> services =
Parser.parseService(
fileDescriptor, messageTypes, resourceNames, Optional.empty(), outputResourceNames);

return GapicContext.builder()
.setMessages(messageTypes)
.setResourceNames(resourceNames)
.setServices(adaptShowcaseResumableUploadForTest(services))
.setHelperResourceNames(outputResourceNames)
.setTransport(transport)
.setServiceConfig(GapicServiceConfig.create(Optional.empty()))
.build();
}

// Temporary test scaffolding; removed in PR #14325 once allowlist patterns are activated.
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());
}
Comment on lines +294 to +320

This comment was marked as outdated.


public GapicContext parseExplicitDynamicRoutingHeaderTesting() {
FileDescriptor testingFileDescriptor =
ExplicitDynamicRoutingHeaderTestingOuterClass.getDescriptor();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package google.showcase.v1beta1;

import "google/api/annotations.proto";
import "google/api/client.proto";

option go_package = "github.com/googleapis/gapic-showcase/server/genproto";
option java_package = "com.google.showcase.v1beta1";
option java_multiple_files = true;
option ruby_package = "Google::Showcase::V1beta1";

// A service showcasing universal resumable upload protocol support.
service ResumableUploadService {
option (google.api.default_host) = "localhost:7469";

// A method with media_upload annotation enabled.
rpc UploadMedia(UploadMediaRequest) returns (UploadMediaResponse) {
option (google.api.http) = {
post: "/v1beta1/files:upload"
body: "*"
};
}
}

message UploadMediaRequest {
string name = 1;
}

message UploadMediaResponse {
string name = 1;
int64 size = 2;
}
Loading