feat: add preview MultipartUploadClient#listParts#3359
Closed
ShreyasSinha wants to merge 38 commits intostaged-mpu-merge-trainfrom
Closed
feat: add preview MultipartUploadClient#listParts#3359ShreyasSinha wants to merge 38 commits intostaged-mpu-merge-trainfrom
ShreyasSinha wants to merge 38 commits intostaged-mpu-merge-trainfrom
Conversation
...-storage/src/main/java/com/google/cloud/storage/multipartupload/model/ListPartsResponse.java
Outdated
Show resolved
Hide resolved
Comment on lines
+115
to
+119
| @JsonCreator | ||
| public static StorageClass valueOf(String constant) { | ||
| if (constant == null || constant.isEmpty()) { | ||
| return null; | ||
| } |
Collaborator
There was a problem hiding this comment.
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);
}
Collaborator
Author
There was a problem hiding this comment.
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.
Collaborator
There was a problem hiding this comment.
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
Collaborator
|
I'll spend some time tomorrow figuring out the dependencies failing build. |
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 |
10 tasks
Collaborator
|
Assuming my fixes in #3377 are included in the ultimate merge train, this PR can be considered done. |
BenWhitehead
approved these changes
Oct 31, 2025
Collaborator
There was a problem hiding this comment.
ready for merge train
BenWhitehead
added a commit
that referenced
this pull request
Nov 3, 2025
BEGIN_NESTED_COMMIT BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#createMultipartUpload #3356 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#listParts #3359 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#abortMultipartUpload #3361 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#uploadPart #3375 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#completeMultipartUpload #3372 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadSettings END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE END_COMMIT_OVERRIDE END_NESTED_COMMIT Other changes: 1. chore: refactor retrier creation from HttpStorageOptions to StorageOptions #3350 2. chore: refactorings for CreateMultipartUpload #3364 3. chore: fix xml parsing of StringEnumValue's so that the enum contract is not broken #3377 4. chore: add PredefinedAcl#xmlEntry Co-authored-by: BenWhitehead <BenWhitehead@users.noreply.github.com>
BenWhitehead
added a commit
that referenced
this pull request
Nov 3, 2025
BEGIN_NESTED_COMMIT BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#createMultipartUpload #3356 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#listParts #3359 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#abortMultipartUpload #3361 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#uploadPart #3375 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#completeMultipartUpload #3372 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadSettings END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE END_COMMIT_OVERRIDE END_NESTED_COMMIT Other changes: 1. chore: refactor retrier creation from HttpStorageOptions to StorageOptions #3350 2. chore: refactorings for CreateMultipartUpload #3364 3. chore: fix xml parsing of StringEnumValue's so that the enum contract is not broken #3377 4. chore: add PredefinedAcl#xmlEntry Co-authored-by: BenWhitehead <BenWhitehead@users.noreply.github.com>
BenWhitehead
added a commit
that referenced
this pull request
Nov 3, 2025
BEGIN_NESTED_COMMIT BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#createMultipartUpload #3356 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#listParts #3359 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#abortMultipartUpload #3361 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#uploadPart #3375 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadClient#completeMultipartUpload #3372 END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE feat: add preview MultipartUploadSettings END_COMMIT_OVERRIDE BEGIN_COMMIT_OVERRIDE END_COMMIT_OVERRIDE END_NESTED_COMMIT Other changes: 1. chore: refactor retrier creation from HttpStorageOptions to StorageOptions #3350 2. chore: refactorings for CreateMultipartUpload #3364 3. chore: fix xml parsing of StringEnumValue's so that the enum contract is not broken #3377 4. chore: add PredefinedAcl#xmlEntry Co-authored-by: BenWhitehead <BenWhitehead@users.noreply.github.com>
Collaborator
|
merged to main in #3378 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refer: #3348