-
Couldn't load subscription status.
- Fork 85
feat: Adding API for ListParts #3359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mpu-feature-2
Are you sure you want to change the base?
Conversation
| public ListPartsResponse listParts(ListPartsRequest request) throws IOException { | ||
|
|
||
| return retrier.run( | ||
| Retrying.alwaysRetry(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be options.getRetryAlgorithmManager().idempotent().
Retrying.alwaysRetry() is for tests.
| } | ||
|
|
||
| /** A builder for {@link ListPartsRequest}. */ | ||
| public static class Builder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public static class Builder { | |
| @BetaApi | |
| public static final class Builder { |
| private String owner; | ||
|
|
||
| @JacksonXmlProperty(localName = "StorageClass") | ||
| private String storageClass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be StorageClass instead of String?
| private boolean isTruncated; | ||
|
|
||
| @JacksonXmlProperty(localName = "Owner") | ||
| private String owner; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be Acl.Entity instead of String?
| .add("storageClass", storageClass) | ||
| .add("parts", parts) | ||
| .toString(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need a builder?
| } | ||
|
|
||
| /** A builder for {@link Part}. */ | ||
| public static final class Builder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public static final class Builder { | |
| @BetaApi | |
| public static final class Builder { |
| public final class Part { | ||
|
|
||
| @JacksonXmlProperty(localName = "PartNumber") | ||
| private int partNumber; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why int here, but Integer in ListPartsResponse? Can we use primitives everywhere?
| private long size; | ||
|
|
||
| @JacksonXmlProperty(localName = "LastModified") | ||
| private String lastModified; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be an OffsetDateTime?
| String xmlResponse = | ||
| "<?xml version='1.0' encoding='UTF-8'?>\n" | ||
| + "<ListPartsResult>\n" | ||
| + " <Bucket>test-bucket</Bucket>\n" | ||
| + " <Key>test-key</Key>\n" | ||
| + " <UploadId>test-upload-id</UploadId>\n" | ||
| + " <PartNumberMarker>0</PartNumberMarker>\n" | ||
| + " <NextPartNumberMarker>1</NextPartNumberMarker>\n" | ||
| + " <MaxParts>1</MaxParts>\n" | ||
| + " <IsTruncated>false</IsTruncated>\n" | ||
| + " <Part>\n" | ||
| + " <PartNumber>1</PartNumber>\n" | ||
| + " <ETag>\"etag\"</ETag>\n" | ||
| + " <Size>123</Size>\n" | ||
| + " <LastModified>2024-05-08T17:50:00.000Z</LastModified>\n" | ||
| + " </Part>\n" | ||
| + "</ListPartsResult>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be replaced once ListPartsResponse has a builder
| @Test | ||
| public void sendListPartsRequest_success() throws Exception { | ||
| HttpRequestHandler handler = | ||
| req -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should perform some request validation to ensure the proper request is being sent to the server. something like only return ok when the bucket, key and uploadId match expected. otherwise 404 or 400 with a message.
Refer: #3348