Skip to content

Conversation

@ShreyasSinha
Copy link

  1. Adding model classes for List request and response.
  2. Implemented List for Multipart Upload.

Refer: #3348

@product-auto-label product-auto-label bot added size: l Pull request size is large. api: storage Issues related to the googleapis/java-storage API. labels Oct 22, 2025
@ShreyasSinha ShreyasSinha marked this pull request as ready for review October 22, 2025 18:32
@ShreyasSinha ShreyasSinha requested a review from a team as a code owner October 22, 2025 18:32
@ShreyasSinha ShreyasSinha requested a review from a team as a code owner October 22, 2025 20:01
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Oct 26, 2025
Comment on lines +115 to +119
@JsonCreator
public static StorageClass valueOf(String constant) {
if (constant == null || constant.isEmpty()) {
return null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary?

If I remove it and run the following test, the test passes:

  private static final class TestXmlObject2 {
    @JacksonXmlProperty(localName = "storageClass")
    private StorageClass storageClass;
  }
  @Test
  public void testParseStringValueEnum() throws IOException {
    //language=xml
    String xml = "<TestXmlObject2>\n"
        + "  <storageClass>STANDARD</storageClass>"
        + "</TestXmlObject2>";
    InputStream in = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8));
    TestXmlObject2 expected = new TestXmlObject2(StorageClass.STANDARD);
    TestXmlObject2 actual =
        xmlObjectParser.parseAndClose(in, StandardCharsets.UTF_8, TestXmlObject2.class);
    assertThat(actual).isEqualTo(expected);
  }

Copy link
Author

Choose a reason for hiding this comment

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

This is required incase we have a null string. Try to run test https://paste.googleplex.com/5523315682836480 after removing it. You'll get https://paste.googleplex.com/5523315682836480 failure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, but that is a deserialization issue not an enum resolution issue.

Enums can't have null values, and changing this method breaks that contract.
The exception explicitly states this fact:

Caused by: java.lang.IllegalArgumentException: Empty enum constants not allowed.
        at com.google.cloud.StringEnumType.valueOf(StringEnumType.java:66)
        at com.google.cloud.storage.StorageClass.valueOf(StorageClass.java:116)

Here is a PR that makes enum parsing work for xml without modifying any behavior or adding additional annotations: #3377

@BenWhitehead
Copy link
Collaborator

I'll spend some time tomorrow figuring out the dependencies failing build.

@BenWhitehead
Copy link
Collaborator

Applying the following patch to this branch should fix the failing dependency check

Index: google-cloud-storage/pom.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml
--- a/google-cloud-storage/pom.xml	(revision c7549b10d56e40c5576a6a7f471d7cd6f8bba397)
+++ b/google-cloud-storage/pom.xml	(date 1761754153688)
@@ -28,6 +28,14 @@
       <artifactId>jackson-datatype-jsr310</artifactId>
     </dependency>
     <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-databind</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-annotations</artifactId>
+    </dependency>
+    <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
     </dependency>

The fix can be verified locally by running mvn dependency:analyze -DfailOnWarning=true

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Oct 30, 2025
@BenWhitehead BenWhitehead mentioned this pull request Oct 30, 2025
9 tasks
@BenWhitehead
Copy link
Collaborator

Assuming my fixes in #3377 are included in the ultimate merge train, this PR can be considered done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the googleapis/java-storage API. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants