gaul requested changes on this pull request.
This seems reasonable. CI may fail for unrelated reasons so please run these
locally which I will also do before merging:
```
mvn verify -pl :filesystem
mvn integration-test -pl :filesystem -Plive
```
> @@ -115,6 +116,7 @@
private static final String XATTR_USER_METADATA_PREFIX =
"user.user-metadata.";
private static final byte[] DIRECTORY_MD5 =
Hashing.md5().hashBytes(new byte[0]).asBytes();
+ private static final String MPU_ETAG_FORMAT = "^\"[a-f0-9]{32}-\\d+\"$";
Prefer `format = Pattern.compile()` and `format.matcher().matches`. Also are
^$ necessary since you use `match` instead of `find`?
> @@ -488,23 +498,37 @@ public String putBlob(final String containerName, final
> Blob blob) throws IOExce
String tmpBlobName = blobKey + "-" + UUID.randomUUID();
File tmpFile = getFileForBlobKey(containerName, tmpBlobName);
Path tmpPath = tmpFile.toPath();
- HashingInputStream his = null;
+ boolean isMpu = false;
+ if (blob.getMetadata() != null && blob.getMetadata().getETag() != null)
+ isMpu = blob.getMetadata().getETag().matches(MPU_ETAG_FORMAT);
+ InputStream inputStream = null;
+ byte [] eTag = null;
Remove space before `[]`.
> try {
Files.createParentDirs(tmpFile);
- his = new HashingInputStream(Hashing.md5(), payload.openStream());
- long actualSize = Files.asByteSink(tmpFile).writeFrom(his);
+ if (isMpu) {
+ inputStream = payload.openStream();
+ eTag = blob.getMetadata().getETag().getBytes();
+ }
+ else {
Put `} else {` on a single line.
> }
+ StringBuilder mpuETagBuilder = new StringBuilder("\"");
+ mpuETagBuilder.append(Hashing.md5().hashString(partHashes.toString(),
US_ASCII).toString());
+ mpuETagBuilder.append(String.format("-%d\"", parts.size()));
+ String mpuETag = mpuETagBuilder.toString();
Use the return values from `StringBuilder` and avoid the `String.format` call:
```java
String mpuETag = new StringBuilder("\"")
.append(Hashing.md5().hashString(partHashes.toString(),
US_ASCII).toString())
.append("-")
.append(parts.size())
.toString();
```
Repeated above.
> @@ -373,7 +376,13 @@ public Blob getBlob(final String container, final String
> key) {
if (attributes.contains(XATTR_CONTENT_MD5)) {
ByteBuffer buf =
ByteBuffer.allocate(view.size(XATTR_CONTENT_MD5));
view.read(XATTR_CONTENT_MD5, buf);
- hashCode = HashCode.fromBytes(buf.array());
+ byte [] etagBytes = buf.array();
+ if (etagBytes.length == 16) {
+ hashCode = HashCode.fromBytes(buf.array());
+ eTag = String.format("\"%s\"", hashCode.toString());
`String.format` will actually call `toString` for you, although this may read
better:
```java
eTag = "\"" + hashCode + "\"";
```
> @@ -373,7 +376,13 @@ public Blob getBlob(final String container, final String
> key) {
if (attributes.contains(XATTR_CONTENT_MD5)) {
ByteBuffer buf =
ByteBuffer.allocate(view.size(XATTR_CONTENT_MD5));
view.read(XATTR_CONTENT_MD5, buf);
- hashCode = HashCode.fromBytes(buf.array());
+ byte [] etagBytes = buf.array();
+ if (etagBytes.length == 16) {
+ hashCode = HashCode.fromBytes(buf.array());
+ eTag = String.format("\"%s\"", hashCode.toString());
+ } else {
+ eTag = new String(etagBytes);
Should this have a second `Charset` parameter?
> @@ -878,7 +888,7 @@ public String completeMultipartUpload(MultipartUpload
> mpu, List<MultipartPart> p
setBlobAccess(mpu.containerName(), mpu.blobName(),
mpu.putOptions().getBlobAccess());
- return eTag;
+ return mpuETag;
Does this return an MPU ETag for single-part uploads?
> @@ -373,7 +376,13 @@ public Blob getBlob(final String container, final String
> key) {
if (attributes.contains(XATTR_CONTENT_MD5)) {
ByteBuffer buf =
ByteBuffer.allocate(view.size(XATTR_CONTENT_MD5));
view.read(XATTR_CONTENT_MD5, buf);
- hashCode = HashCode.fromBytes(buf.array());
+ byte [] etagBytes = buf.array();
+ if (etagBytes.length == 16) {
Add comments:
```
// single-part upload
// multi-part upload
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1251#pullrequestreview-167697206
