From 43b178a03a33101b2d06e9eabc2687aa3c31a7aa Mon Sep 17 00:00:00 2001 From: Vikas Saurabh Date: Tue, 14 Nov 2017 03:56:05 +0530 Subject: [PATCH] JCR-4213: It is useful to return an error code with DataStoreException when a record is not found --- .../apache/jackrabbit/aws/ext/ds/S3Backend.java | 10 ++++- .../jackrabbit/core/data/DBDataStoreTest.java | 14 ++++++ .../jackrabbit/core/data/AbstractDataStore.java | 2 +- .../jackrabbit/core/data/CachingDataStore.java | 2 +- .../jackrabbit/core/data/DataStoreException.java | 52 ++++++++++++++++++++-- .../org/apache/jackrabbit/core/data/FSBackend.java | 2 +- .../jackrabbit/core/data/InMemoryBackend.java | 9 ++-- .../apache/jackrabbit/core/data/TestCaseBase.java | 28 ++++++++++++ .../apache/jackrabbit/vfs/ext/ds/VFSBackend.java | 3 +- 9 files changed, 109 insertions(+), 13 deletions(-) diff --git a/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java b/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java index 01e87ddb4..b08a1c2e9 100644 --- a/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java +++ b/jackrabbit-aws-ext/src/main/java/org/apache/jackrabbit/aws/ext/ds/S3Backend.java @@ -456,8 +456,10 @@ public class S3Backend extends AbstractBackend { LOG.info( "getLastModified:Identifier [{}] not found. Took [{}] ms.", identifier, (System.currentTimeMillis() - start)); + throw new DataStoreException(e, DataStoreException.ErrorCode.NOT_FOUND); + } else { + throw new DataStoreException(e); } - throw new DataStoreException(e); } finally { if (contextClassLoader != null) { Thread.currentThread().setContextClassLoader(contextClassLoader); @@ -480,8 +482,12 @@ public class S3Backend extends AbstractBackend { (System.currentTimeMillis() - start) }); return length; } catch (AmazonServiceException e) { + DataStoreException.ErrorCode errorCode = DataStoreException.ErrorCode.UNKNOWN; + if (e.getStatusCode() == 404 || e.getStatusCode() == 403) { + errorCode = DataStoreException.ErrorCode.NOT_FOUND; + } throw new DataStoreException("Could not length of dataIdentifier " - + identifier, e); + + identifier, e, errorCode); } finally { if (contextClassLoader != null) { Thread.currentThread().setContextClassLoader(contextClassLoader); diff --git a/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/DBDataStoreTest.java b/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/DBDataStoreTest.java index 8152d197a..115edac8b 100644 --- a/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/DBDataStoreTest.java +++ b/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/DBDataStoreTest.java @@ -83,6 +83,20 @@ public class DBDataStoreTest extends JUnitTest { } } + public void testGetDeletedRecord() throws Exception { + DataIdentifier id = + store.addRecord(new ByteArrayInputStream(new byte[2])).getIdentifier(); + store.deleteRecord(id); + try { + store.getRecord(id); + fail("Get record for deleted identifier must fail!"); + } catch (DataStoreException dse) { + if (dse.getErrorCode() != DataStoreException.ErrorCode.NOT_FOUND) { + fail("Get record for deleted identifier must fail with not found!"); + } + } + } + public void testDbInputStreamReset() throws Exception { DataRecord record = store.getRecord(identifier); InputStream in = record.getStream(); diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractDataStore.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractDataStore.java index da28a75a5..1864d63c9 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractDataStore.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/AbstractDataStore.java @@ -56,7 +56,7 @@ public abstract class AbstractDataStore implements DataStore { return record; } else { throw new DataStoreException( - "Record " + identifier + " does not exist"); + "Record " + identifier + " does not exist", DataStoreException.ErrorCode.NOT_FOUND); } } diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/CachingDataStore.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/CachingDataStore.java index eef22103a..7fa248270 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/CachingDataStore.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/CachingDataStore.java @@ -480,7 +480,7 @@ public abstract class CachingDataStore extends AbstractDataStore implements throw new DataStoreException("error in getting record [" + identifier + "]", ioe); } - throw new DataStoreException("Record not found: " + identifier); + throw new DataStoreException("Record not found: " + identifier, DataStoreException.ErrorCode.NOT_FOUND); } /** diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/DataStoreException.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/DataStoreException.java index 4b173ac84..f1f082933 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/DataStoreException.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/DataStoreException.java @@ -22,6 +22,12 @@ import javax.jcr.RepositoryException; * Exception thrown by the Data Store module. */ public class DataStoreException extends RepositoryException { + public enum ErrorCode { + NOT_FOUND, + UNKNOWN + } + + private final ErrorCode errorCode; /** * Constructs a new instance of this class with the specified detail @@ -30,7 +36,20 @@ public class DataStoreException extends RepositoryException { * @param message the detailed message. */ public DataStoreException(String message) { + this(message, ErrorCode.UNKNOWN); + } + + + /** + * Constructs a new instance of this class with the specified detail + * message and specified errorCode. + * + * @param message the detailed message. + * @param errorCode {@link ErrorCode} of exception + */ + public DataStoreException(String message, ErrorCode errorCode) { super(message); + this.errorCode = errorCode; } /** @@ -38,19 +57,46 @@ public class DataStoreException extends RepositoryException { * message and root cause. * * @param message the detailed message. - * @param cause root failure cause + * @param cause root failure errorCode */ public DataStoreException(String message, Throwable cause) { + this(message, cause, ErrorCode.UNKNOWN); + } + + /** + * Constructs a new instance of this class with the specified detail + * message, root cause and specified error errorCode . + * + * @param message the detailed message. + * @param cause root failure errorCode + * @param errorCode {@link ErrorCode} of exception + */ + public DataStoreException(String message, Throwable cause, ErrorCode errorCode) { super(message, cause); + this.errorCode = errorCode; } /** - * Constructs a new instance of this class with the specified root cause. + * Constructs a new instance of this class with the specified root errorCode. * - * @param rootCause root failure cause + * @param rootCause root failure errorCode */ public DataStoreException(Throwable rootCause) { + this(rootCause, ErrorCode.UNKNOWN); + } + + /** + * Constructs a new instance of this class with the specified root errorCode. + * + * @param rootCause root failure errorCode + * @param errorCode {@link ErrorCode} of exception + */ + public DataStoreException(Throwable rootCause, ErrorCode errorCode) { super(rootCause); + this.errorCode = errorCode; } + public ErrorCode getErrorCode() { + return errorCode; + } } diff --git a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java index 1437178d5..5cdba5866 100644 --- a/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java +++ b/jackrabbit-data/src/main/java/org/apache/jackrabbit/core/data/FSBackend.java @@ -127,7 +127,7 @@ public class FSBackend extends AbstractBackend { return file.length(); } throw new DataStoreException("Could not length of dataIdentifier [" - + identifier + "]"); + + identifier + "]", DataStoreException.ErrorCode.NOT_FOUND); } @Override diff --git a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java index 5360a7425..35e3638c2 100644 --- a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java +++ b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/InMemoryBackend.java @@ -126,11 +126,12 @@ public class InMemoryBackend implements Backend { @Override public long getLength(final DataIdentifier identifier) throws DataStoreException { - try { - return data.get(identifier).length; - } catch (Exception e) { - throw new DataStoreException(e); + byte [] d = data.get(identifier); + if (d == null) { + throw new DataStoreException( + "Record " + identifier + " does not exist", DataStoreException.ErrorCode.NOT_FOUND); } + return d.length; } @Override diff --git a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCaseBase.java b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCaseBase.java index dad1e0938..2e0dfd595 100644 --- a/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCaseBase.java +++ b/jackrabbit-data/src/test/java/org/apache/jackrabbit/core/data/TestCaseBase.java @@ -273,6 +273,34 @@ public abstract class TestCaseBase extends TestCase { } + /** + * Testcase to validate that getRecord fills error code for not-found + */ + public void testNotExistingGetRecord() { + try { + DataIdentifier identifier = new DataIdentifier("some-random-identifier"); + + ds = createDataStore(); + try { + ds.getRecord(identifier); + fail("Get record for non existent identifier must fail!"); + } catch (DataStoreException dse) { + if (dse.getErrorCode() != DataStoreException.ErrorCode.NOT_FOUND) { + fail("Get record for non existent identifier must fail with not found!"); + } + } + } catch (Exception e) { + LOG.error("error:", e); + fail(e.getMessage()); + } finally { + try { + ds.close(); + } catch (DataStoreException dse) { + //ignored + } + } + } + protected abstract DataStore createDataStore() throws RepositoryException ; /** diff --git a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java index 3b2a7f92a..3bacd37cd 100644 --- a/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java +++ b/jackrabbit-vfs-ext/src/main/java/org/apache/jackrabbit/vfs/ext/ds/VFSBackend.java @@ -129,7 +129,8 @@ public class VFSBackend extends AbstractBackend { FileObject fileObject = getExistingFileObject(identifier); if (fileObject == null) { - throw new DataStoreException("Could not find file object for: " + identifier); + throw new DataStoreException("Could not find file object for: " + identifier, + DataStoreException.ErrorCode.NOT_FOUND); } try { -- 2.14.1