Author: mattryan
Date: Wed Jul 17 23:57:36 2019
New Revision: 1863238
URL: http://svn.apache.org/viewvc?rev=1863238&view=rev
Log:
OAK-7998: Verify existence of binary before generating direct download URI
This change adds code and tests to ensure that when generating a direct
download URI we make sure the binary actually exists first. If caching
is being used it is possible that a client requests a direct download
URI for a blob that was already added the standard (i.e. non-direct)
way but is still in the cache and hasn't actually been uploaded to
cloud storage yet - and therefore cannot be downloaded by direct
download URI.
This change also adds some documentation changes to reflect this
possible situation.
Modified:
jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
jackrabbit/oak/trunk/oak-doc/src/site/markdown/features/direct-binary-access.md
Modified:
jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java?rev=1863238&r1=1863237&r2=1863238&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
(original)
+++
jackrabbit/oak/trunk/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
Wed Jul 17 23:57:36 2019
@@ -765,7 +765,26 @@ public class AzureBlobStoreBackend exten
URI createHttpDownloadURI(@NotNull DataIdentifier identifier,
@NotNull DataRecordDownloadOptions
downloadOptions) {
URI uri = null;
+
+ // When running unit test from Maven, it doesn't always honor the
@NotNull decorators
+ if (null == identifier) throw new NullPointerException("identifier");
+ if (null == downloadOptions) throw new
NullPointerException("downloadOptions");
+
if (httpDownloadURIExpirySeconds > 0) {
+
+ // Check if this identifier exists. If not, we want to return null
+ // even if the identifier is in the download URI cache.
+ try {
+ if (! exists(identifier)) {
+ LOG.warn("Cannot create download URI for nonexistent blob
{}; returning null", getKeyName(identifier));
+ return null;
+ }
+ }
+ catch (DataStoreException e) {
+ LOG.warn("Cannot create download URI for blob {} (caught
DataStoreException); returning null", getKeyName(identifier), e);
+ return null;
+ }
+
if (null != httpDownloadURICache) {
uri = httpDownloadURICache.getIfPresent(identifier);
}
Modified:
jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java?rev=1863238&r1=1863237&r2=1863238&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java
(original)
+++
jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java
Wed Jul 17 23:57:36 2019
@@ -17,6 +17,10 @@
package org.apache.jackrabbit.oak.blob.cloud.s3;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.Iterables.filter;
+import static java.lang.Thread.currentThread;
+
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
@@ -92,10 +96,6 @@ import org.jetbrains.annotations.NotNull
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.Iterables.filter;
-import static java.lang.Thread.currentThread;
-
/**
* A data store backend that stores data on Amazon S3.
*/
@@ -740,6 +740,21 @@ public class S3Backend extends AbstractS
return null;
}
+ // When running unit test from Maven, it doesn't always honor the
@NotNull decorators
+ if (null == identifier) throw new NullPointerException("identifier");
+ if (null == downloadOptions) throw new
NullPointerException("downloadOptions");
+
+ try {
+ if (! exists(identifier)) {
+ LOG.warn("Cannot create download URI for nonexistent blob {};
returning null", getKeyName(identifier));
+ return null;
+ }
+ }
+ catch (DataStoreException e) {
+ LOG.warn("Cannot create download URI for blob {} (caught
DataStoreException); returning null", getKeyName(identifier), e);
+ return null;
+ }
+
URI uri = null;
// if cache is enabled, check the cache
if (httpDownloadURICache != null) {
Modified:
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java?rev=1863238&r1=1863237&r2=1863238&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
(original)
+++
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/directaccess/AbstractDataRecordAccessProviderTest.java
Wed Jul 17 23:57:36 2019
@@ -81,10 +81,21 @@ public abstract class AbstractDataRecord
// Direct download tests
//
@Test
- public void testGetDownloadURIProvidesValidURI() {
- DataIdentifier id = new DataIdentifier("testIdentifier");
- URI uri = getDataStore().getDownloadURI(id,
DataRecordDownloadOptions.DEFAULT);
- assertNotNull(uri);
+ public void testGetDownloadURIProvidesValidURIIT() throws
DataStoreException {
+ DataRecord record = null;
+ ConfigurableDataRecordAccessProvider dataStore = getDataStore();
+ try {
+ InputStream testStream = randomStream(0, 256);
+ record = doSynchronousAddRecord((DataStore) dataStore, testStream);
+ DataIdentifier id = record.getIdentifier();
+ URI uri = getDataStore().getDownloadURI(id,
DataRecordDownloadOptions.DEFAULT);
+ assertNotNull(uri);
+ }
+ finally {
+ if (null != record) {
+ doDeleteRecord((DataStore) dataStore, record.getIdentifier());
+ }
+ }
}
@Test
@@ -219,6 +230,20 @@ public abstract class AbstractDataRecord
}
}
+ @Test
+ public void testGetDownloadURINonexistentBlobFailsIT() throws
DataStoreException {
+ ConfigurableDataRecordAccessProvider dataStore = getDataStore();
+ InputStream testStream = randomStream(0, 256);
+
+ DataRecord record = doSynchronousAddRecord((DataStore) dataStore,
testStream);
+
+ doDeleteRecord((DataStore) dataStore, record.getIdentifier());
+
+ URI uri = dataStore.getDownloadURI(record.getIdentifier(),
DataRecordDownloadOptions.DEFAULT);
+
+ assertNull(uri);
+ }
+
//
// Direct upload tests
//
Modified:
jackrabbit/oak/trunk/oak-doc/src/site/markdown/features/direct-binary-access.md
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/features/direct-binary-access.md?rev=1863238&r1=1863237&r2=1863238&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-doc/src/site/markdown/features/direct-binary-access.md
(original)
+++
jackrabbit/oak/trunk/oak-doc/src/site/markdown/features/direct-binary-access.md
Wed Jul 17 23:57:36 2019
@@ -106,6 +106,11 @@ if (binary instanceof BinaryDownload) {
Please note that only `Binary` objects returned from `Property.getBinary()`,
`Property.getValue().getBinary()` or `Property.getValues() ... getBinary()`
will support a functional `BinaryDownload`.
+Also note that clients should always check whether the URI returned from the
`getURI()` call is null. A null return value generally indicates that the
feature is not available. But this situation is also possible in two other
cases:
+
+* If the binary is stored in-line in the node store. If the binary is smaller
than the minimum upload size, it will be stored in the node store instead of in
cloud blob storage, and thus a direct download URI cannot be provided.
+* If the data store implementation is using asynchronous uploads and the
binary is still in cache. If a client adds a binary via the repository (i.e.
not using the direct binary upload feature) and then immediately requests a
download URI for it, it is possible that the binary is still in cache and not
yet uploaded to cloud storage, and thus a direct download URI cannot be
provided.
+
### Upload
The direct binary upload process is split into 3 phases:
@@ -178,6 +183,8 @@ public class InitiateUploadServlet exten
}
```
+Clients should always check whether the `BinaryUpload` returned from
`valueFactory.initiateBinaryUpload()` is null, and also should handle the case
where no upload URIs are returned. Either situation indicates that the feature
is not supported.
+
#### 2. Upload
The remote client will upload using the instructions according to the [upload
algorithm described in
BinaryUpload](http://jackrabbit.apache.org/api/trunk/org/apache/jackrabbit/api/binary/BinaryUpload.html).
