Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use stronger write-once semantics for Azure repository #30437

Merged
merged 6 commits into from
Jun 11, 2018
Merged

Use stronger write-once semantics for Azure repository #30437

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
85d5ef7
Use stronger write-once semantics for Azure repository
ywelsch May 7, 2018
158d5f7
rely on StorageErrorCode string instead
ywelsch May 14, 2018
903a07a
extra checks
ywelsch May 14, 2018
18bf1a2
Merge remote-tracking branch 'elastic/master' into remove-blobexists-…
ywelsch May 15, 2018
00b8763
Merge remote-tracking branch 'elastic/master' into remove-blobexists-…
ywelsch Jun 6, 2018
b1ea909
Merge remote-tracking branch 'elastic/master' into remove-blobexists-…
ywelsch Jun 8, 2018
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
4 changes: 1 addition & 3 deletions ...pository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,8 @@ public InputStream readBlob(String blobName) throws IOException {

@Override
public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException {
if (blobExists(blobName)) {
throw new FileAlreadyExistsException("blob [" + blobName + "] already exists, cannot overwrite");
}
logger.trace("writeBlob({}, stream, {})", buildKey(blobName), blobSize);

try {
blobStore.writeBlob(buildKey(blobName), inputStream, blobSize);
} catch (URISyntaxException|StorageException e) {
Expand Down
4 changes: 3 additions & 1 deletion ...s/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.nio.file.FileAlreadyExistsException;
import java.util.Locale;
import java.util.Map;

Expand Down Expand Up @@ -122,7 +123,8 @@ public void moveBlob(String sourceBlob, String targetBlob) throws URISyntaxExcep
this.client.moveBlob(this.clientName, this.locMode, container, sourceBlob, targetBlob);
}

public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws URISyntaxException, StorageException {
public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws URISyntaxException, StorageException,
FileAlreadyExistsException {
this.client.writeBlob(this.clientName, this.locMode, container, blobName, inputStream, blobSize);
}
}
3 changes: 2 additions & 1 deletion ...ository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.nio.file.FileAlreadyExistsException;
import java.util.Map;

/**
Expand Down Expand Up @@ -61,7 +62,7 @@ void moveBlob(String account, LocationMode mode, String container, String source
throws URISyntaxException, StorageException;

void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize) throws
URISyntaxException, StorageException;
URISyntaxException, StorageException, FileAlreadyExistsException;

static InputStream giveSocketPermissionsToStream(InputStream stream) {
return new InputStream() {
Expand Down
14 changes: 12 additions & 2 deletions ...ory-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.repositories.azure;

import com.microsoft.azure.storage.AccessCondition;
import com.microsoft.azure.storage.CloudStorageAccount;
import com.microsoft.azure.storage.LocationMode;
import com.microsoft.azure.storage.OperationContext;
Expand All @@ -43,8 +44,10 @@
import org.elasticsearch.repositories.RepositoryException;

import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.FileAlreadyExistsException;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -312,12 +315,19 @@ public void moveBlob(String account, LocationMode mode, String container, String

@Override
public void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize)
throws URISyntaxException, StorageException {
throws URISyntaxException, StorageException, FileAlreadyExistsException {
logger.trace("writeBlob({}, stream, {})", blobName, blobSize);
CloudBlobClient client = this.getSelectedClient(account, mode);
CloudBlobContainer blobContainer = client.getContainerReference(container);
CloudBlockBlob blob = blobContainer.getBlockBlobReference(blobName);
SocketAccess.doPrivilegedVoidException(() -> blob.upload(inputStream, blobSize, null, null, generateOperationContext(account)));
try {
SocketAccess.doPrivilegedVoidException(() -> blob.upload(inputStream, blobSize, AccessCondition.generateIfNotExistsCondition(),
null, generateOperationContext(account)));
} catch (StorageException se) {
if (se.getHttpStatusCode() == HttpURLConnection.HTTP_CONFLICT) {
throw new FileAlreadyExistsException(blobName, null, se.getMessage());
}
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I saw that after the LGTM: we should rethrow the exception, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right! Good catch.

}
logger.trace("writeBlob({}, stream, {}) - done", blobName, blobSize);
}
}
6 changes: 5 additions & 1 deletion ...ory-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceMock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.io.InputStream;
import java.net.SocketPermission;
import java.net.URISyntaxException;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.NoSuchFileException;
import java.security.AccessController;
import java.util.Locale;
Expand Down Expand Up @@ -120,7 +121,10 @@ public void moveBlob(String account, LocationMode mode, String container, String

@Override
public void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize)
throws URISyntaxException, StorageException {
throws URISyntaxException, StorageException, FileAlreadyExistsException {
if (blobs.containsKey(blobName)) {
throw new FileAlreadyExistsException(blobName);
}
try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
blobs.put(blobName, outputStream);
Streams.copy(inputStream, outputStream);
Expand Down
3 changes: 2 additions & 1 deletion .../framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.NoSuchFileException;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -149,7 +150,7 @@ public void testVerifyOverwriteFails() throws IOException {
final BytesArray bytesArray = new BytesArray(data);
writeBlob(container, blobName, bytesArray);
// should not be able to overwrite existing blob
expectThrows(IOException.class, () -> writeBlob(container, blobName, bytesArray));
expectThrows(FileAlreadyExistsException.class, () -> writeBlob(container, blobName, bytesArray));
container.deleteBlob(blobName);
writeBlob(container, blobName, bytesArray); // after deleting the previous blob, we should be able to write to it again
}
Expand Down