diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fe7b92d0..30553031a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,12 @@ Version 1.4.3 (Planned) https://github.com/jakenjarvis/Android-OrmLiteContentProvider ** Features : +* Upgrade to android maven plugin 3.6.0, adt 22, and SDK 17. * Request priority management is now built into RoboSpice. Requests with higher priority will get executed first. This is only taken into account for requests that exceed the number of threads of RoboSpice Service, otherwise this is not used. https://github.com/octo-online/robospice/issues/99 Thanks to Volley for inspiring us on this feature. Richard Hyndman and Nick Butcher suggested us to add this a while ago.. Thx Google ! :) - +* Retry Policy is now built into RoboSpice. Request default to a non null retry policy that can be customized. diff --git a/RoboSpice-samples b/RoboSpice-samples index d2cc8d8b5..766018f7c 160000 --- a/RoboSpice-samples +++ b/RoboSpice-samples @@ -1 +1 @@ -Subproject commit d2cc8d8b59fc3aea41c5e713256f5b52ba2f6097 +Subproject commit 766018f7c21732c5d3b37700a52bedb9282b316a diff --git a/pom.xml b/pom.xml index 55a7f45cc..f2472e034 100644 --- a/pom.xml +++ b/pom.xml @@ -94,7 +94,7 @@ 4.1.1.4 - 14 + 17 4.1.1.4 r7 2.5.2 @@ -103,7 +103,7 @@ 1.1.1 - 3.5.1 + 3.6.0 0.8 2.6 2.6 diff --git a/robospice-core-parent/robospice-core-test/src/main/java/com/octo/android/robospice/request/RequestProcessorTest.java b/robospice-core-parent/robospice-core-test/src/main/java/com/octo/android/robospice/request/RequestProcessorTest.java index 7fbf4b44a..07fd60b67 100644 --- a/robospice-core-parent/robospice-core-test/src/main/java/com/octo/android/robospice/request/RequestProcessorTest.java +++ b/robospice-core-parent/robospice-core-test/src/main/java/com/octo/android/robospice/request/RequestProcessorTest.java @@ -20,6 +20,7 @@ import com.octo.android.robospice.priority.PausableThreadPoolExecutor; import com.octo.android.robospice.priority.PriorityThreadPoolExecutor; import com.octo.android.robospice.request.listener.RequestListener; +import com.octo.android.robospice.retry.DefaultRetryPolicy; import com.octo.android.robospice.stub.CachedSpiceRequestStub; import com.octo.android.robospice.stub.RequestListenerStub; import com.octo.android.robospice.stub.RequestListenerWithProgressStub; @@ -38,6 +39,9 @@ public class RequestProcessorTest extends InstrumentationTestCase { private static final String TEST_RETURNED_DATA2 = "toto"; private static final long REQUEST_COMPLETION_TIME_OUT = 2000; private static final long WAIT_BEFORE_REQUEST_EXECUTION = 200; + private static final float TEST_RETRY_BACKOFF_MULTIPLIER = 1.0f; + private static final long TEST_DELAY_BEFORE_RETRY = WAIT_BEFORE_REQUEST_EXECUTION; + private static final int TEST_RETRY_COUNT = 3; private ICacheManager mockCacheManager; private RequestProcessor requestProcessorUnderTest; @@ -138,6 +142,7 @@ public void testAddRequest_when_nothing_is_found_in_cache_and_request_succeeds() public void testAddRequest_when_nothing_is_found_in_cache_and_request_fails() throws CacheLoadingException, CacheSavingException, InterruptedException { // given CachedSpiceRequestStub stubRequest = createFailedRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION); + stubRequest.setRetryPolicy(null); RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); Set> requestListenerSet = new HashSet>(); @@ -278,6 +283,7 @@ public void testAddRequest_fail_on_error_true_when_nothing_is_found_in_cache() t public void testAddRequest_when_fail_on_error_true_loading_from_cache_throws_exception() throws CacheLoadingException, CacheSavingException, InterruptedException { // given CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + stubRequest.setRetryPolicy(null); RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); Set> requestListenerSet = new HashSet>(); @@ -302,6 +308,7 @@ public void testAddRequest_when_fail_on_error_true_loading_from_cache_throws_exc public void testAddRequest_when_fail_on_error_true_saving_to_cache_throws_exception() throws CacheLoadingException, CacheSavingException, InterruptedException { // given CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + stubRequest.setRetryPolicy(null); RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); Set> requestListenerSet = new HashSet>(); @@ -481,6 +488,7 @@ public void testRemoveAllDataFromCache_for_given_class_and_cachekey() { public void testAddRequestWhenNetworkIsDown() throws CacheLoadingException, CacheSavingException, InterruptedException { // given CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + stubRequest.setRetryPolicy(null); RequestListenerStub mockRequestListener = new RequestListenerStub(); Set> requestListenerSet = new HashSet>(); @@ -639,6 +647,131 @@ public void testRequestPriority_should_execute_lazyly_low_priority_requests() th assertEquals(TEST_RETURNED_DATA2, mockRequestListener.getResultHistory().get(2 * lowRequestCount)); } + // ============================================================================================ + // TESTING RETRY POLICY + // ============================================================================================ + + public void testAddRequest_when_nothing_is_found_in_cache_and_request_has_retry_policy() throws CacheLoadingException, CacheSavingException, InterruptedException { + // given + CachedSpiceRequestStub stubRequest = createFailedRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION); + DefaultRetryPolicy retryPolicy = new DefaultRetryPolicy(TEST_RETRY_COUNT, TEST_DELAY_BEFORE_RETRY, TEST_RETRY_BACKOFF_MULTIPLIER); + stubRequest.setRetryPolicy(retryPolicy); + + RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); + Set> requestListenerSet = new HashSet>(); + requestListenerSet.add(mockRequestListener); + + EasyMock.expect(mockCacheManager.loadDataFromCache(EasyMock.eq(TEST_CLASS), EasyMock.eq(TEST_CACHE_KEY), EasyMock.eq(TEST_DURATION))).andReturn(null); + EasyMock.expectLastCall().anyTimes(); + EasyMock.replay(mockCacheManager); + + // when + requestProcessorUnderTest.addRequest(stubRequest, requestListenerSet); + + mockRequestListener.await(RequestProcessorTest.REQUEST_COMPLETION_TIME_OUT); + + // then + assertNotNull(stubRequest.getRetryPolicy()); + assertEquals(0, stubRequest.getRetryPolicy().getRetryCount()); + EasyMock.verify(mockCacheManager); + assertTrue(stubRequest.isLoadDataFromNetworkCalled()); + assertNotNull(mockRequestListener.isSuccessful()); + assertFalse(mockRequestListener.isSuccessful()); + assertTrue(mockRequestListener.isExecutedInUIThread()); + assertTrue(mockRequestListener.isComplete()); + } + + public void testAddRequest_when_fail_on_error_true_and_request_has_retry_policy_loading_from_cache_throws_exception() throws CacheLoadingException, CacheSavingException, InterruptedException { + // given + CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + DefaultRetryPolicy retryPolicy = new DefaultRetryPolicy(TEST_RETRY_COUNT, TEST_DELAY_BEFORE_RETRY, TEST_RETRY_BACKOFF_MULTIPLIER); + stubRequest.setRetryPolicy(retryPolicy); + + RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); + Set> requestListenerSet = new HashSet>(); + requestListenerSet.add(mockRequestListener); + + EasyMock.expect(mockCacheManager.loadDataFromCache(EasyMock.eq(TEST_CLASS), EasyMock.eq(TEST_CACHE_KEY), EasyMock.eq(TEST_DURATION))).andThrow(new CacheLoadingException("")); + EasyMock.expectLastCall().anyTimes(); + EasyMock.replay(mockCacheManager); + + // when + requestProcessorUnderTest.setFailOnCacheError(true); + requestProcessorUnderTest.addRequest(stubRequest, requestListenerSet); + + mockRequestListener.await(REQUEST_COMPLETION_TIME_OUT); + + // then + assertNotNull(stubRequest.getRetryPolicy()); + assertEquals(0, stubRequest.getRetryPolicy().getRetryCount()); + EasyMock.verify(mockCacheManager); + assertFalse(stubRequest.isLoadDataFromNetworkCalled()); + assertFalse(mockRequestListener.isSuccessful()); + assertTrue(mockRequestListener.isComplete()); + } + + public void testAddRequest_when_fail_on_error_and_request_has_retry_polic_true_saving_to_cache_throws_exception() throws CacheLoadingException, CacheSavingException, InterruptedException { + // given + CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + DefaultRetryPolicy retryPolicy = new DefaultRetryPolicy(TEST_RETRY_COUNT, TEST_DELAY_BEFORE_RETRY, TEST_RETRY_BACKOFF_MULTIPLIER); + stubRequest.setRetryPolicy(retryPolicy); + + RequestListenerWithProgressStub mockRequestListener = new RequestListenerWithProgressStub(); + Set> requestListenerSet = new HashSet>(); + requestListenerSet.add(mockRequestListener); + + EasyMock.expect(mockCacheManager.loadDataFromCache(EasyMock.eq(TEST_CLASS), EasyMock.eq(TEST_CACHE_KEY), EasyMock.eq(TEST_DURATION))).andReturn(null); + EasyMock.expectLastCall().anyTimes(); + EasyMock.expect(mockCacheManager.saveDataToCacheAndReturnData(EasyMock.eq(TEST_RETURNED_DATA), EasyMock.eq(TEST_CACHE_KEY))).andThrow(new CacheSavingException("")); + EasyMock.expectLastCall().anyTimes(); + EasyMock.replay(mockCacheManager); + + // when + requestProcessorUnderTest.setFailOnCacheError(true); + requestProcessorUnderTest.addRequest(stubRequest, requestListenerSet); + + mockRequestListener.await(REQUEST_COMPLETION_TIME_OUT); + + // then + assertNotNull(stubRequest.getRetryPolicy()); + assertEquals(0, stubRequest.getRetryPolicy().getRetryCount()); + EasyMock.verify(mockCacheManager); + assertTrue(stubRequest.isLoadDataFromNetworkCalled()); + assertTrue(mockRequestListener.isExecutedInUIThread()); + assertFalse(mockRequestListener.isSuccessful()); + assertTrue(mockRequestListener.isComplete()); + } + + public void testAddRequestWhenNetworkIsDown_and_request_has_retry_policy() throws CacheLoadingException, CacheSavingException, InterruptedException { + // given + CachedSpiceRequestStub stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA); + DefaultRetryPolicy retryPolicy = new DefaultRetryPolicy(TEST_RETRY_COUNT, TEST_DELAY_BEFORE_RETRY, TEST_RETRY_BACKOFF_MULTIPLIER); + stubRequest.setRetryPolicy(retryPolicy); + + RequestListenerStub mockRequestListener = new RequestListenerStub(); + Set> requestListenerSet = new HashSet>(); + requestListenerSet.add(mockRequestListener); + + EasyMock.expect(mockCacheManager.loadDataFromCache(EasyMock.eq(TEST_CLASS), EasyMock.eq(TEST_CACHE_KEY), EasyMock.eq(TEST_DURATION))).andReturn(null); + EasyMock.expectLastCall().anyTimes(); + EasyMock.replay(mockCacheManager); + + // when + requestProcessorUnderTest.setFailOnCacheError(true); + networkStateChecker.setNetworkAvailable(false); + requestProcessorUnderTest.addRequest(stubRequest, requestListenerSet); + + mockRequestListener.await(REQUEST_COMPLETION_TIME_OUT); + + // then + assertNotNull(stubRequest.getRetryPolicy()); + assertEquals(0, stubRequest.getRetryPolicy().getRetryCount()); + EasyMock.verify(mockCacheManager); + assertFalse(stubRequest.isLoadDataFromNetworkCalled()); + assertTrue(mockRequestListener.isExecutedInUIThread()); + assertFalse(mockRequestListener.isSuccessful()); + } + // ============================================================================================ // PRIVATE METHODS // ============================================================================================ diff --git a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/CachedSpiceRequest.java b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/CachedSpiceRequest.java index 9f516f149..57825f382 100644 --- a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/CachedSpiceRequest.java +++ b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/CachedSpiceRequest.java @@ -6,6 +6,7 @@ import com.octo.android.robospice.request.listener.RequestProgress; import com.octo.android.robospice.request.listener.RequestProgressListener; import com.octo.android.robospice.request.listener.RequestStatus; +import com.octo.android.robospice.retry.RetryPolicy; /** * Decorates {@link SpiceRequest} and provides additional information used by @@ -30,6 +31,16 @@ public CachedSpiceRequest(final SpiceRequest spiceRequest, final Object this.spiceRequest = spiceRequest; } + @Override + public RetryPolicy getRetryPolicy() { + return spiceRequest.getRetryPolicy(); + } + + @Override + public void setRetryPolicy(RetryPolicy retryPolicy) { + spiceRequest.setRetryPolicy(retryPolicy); + } + @Override public RESULT loadDataFromNetwork() throws Exception { return spiceRequest.loadDataFromNetwork(); diff --git a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/RequestProcessor.java b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/RequestProcessor.java index 7b203b519..f666b09ed 100644 --- a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/RequestProcessor.java +++ b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/RequestProcessor.java @@ -159,26 +159,7 @@ public void onRequestCancelled() { notifyListenersOfRequestCancellation(request, listRequestListener); return; } else { - - Future future; - future = executorService.submit(new PriorityRunnable() { - - @Override - public void run() { - try { - processRequest(request); - } catch (final Throwable t) { - Ln.d(t, "An unexpected error occured when processsing request %s", request.toString()); - } - } - - @Override - public int getPriority() { - return request.getPriority(); - } - - }); - request.setFuture(future); + planRequestExecution(request); } } @@ -229,7 +210,7 @@ public void onRequestProgressUpdate(final RequestProgress progress) { } catch (final CacheLoadingException e) { Ln.d(e, "Cache file could not be read."); if (failOnCacheError) { - notifyListenersOfRequestFailure(request, e); + handleRetry(request, e); return; } cacheManager.removeDataFromCache(request.getResultType(), request.getRequestCacheKey()); @@ -241,7 +222,7 @@ public void onRequestProgressUpdate(final RequestProgress progress) { Ln.d("Cache content not available or expired or disabled"); if (!isNetworkAvailable(applicationContext) && !request.isOffline()) { Ln.e("Network is down."); - notifyListenersOfRequestFailure(request, new NoNetworkException()); + handleRetry(request, new NoNetworkException()); return; } @@ -257,8 +238,7 @@ public void onRequestProgressUpdate(final RequestProgress progress) { } catch (final Exception e) { if (!request.isCancelled()) { Ln.e(e, "An exception occured during request network execution :" + e.getMessage()); - notifyListenersOfRequestFailure(request, new NetworkException( - "Exception occured during invocation of web service.", e)); + handleRetry(request, new NetworkException("Exception occured during invocation of web service.", e)); } else { Ln.e("An exception occured during request network execution but request was cancelled, so listeners are not called."); } @@ -283,7 +263,7 @@ public void onRequestProgressUpdate(final RequestProgress progress) { } catch (final CacheSavingException e) { Ln.d("An exception occured during service execution :" + e.getMessage(), e); if (failOnCacheError) { - notifyListenersOfRequestFailure(request, e); + handleRetry(request, e); return; } else { if (request.isCancelled()) { @@ -308,6 +288,46 @@ public void onRequestProgressUpdate(final RequestProgress progress) { } } + private void planRequestExecution(final CachedSpiceRequest request) { + Future future = executorService.submit(new PriorityRunnable() { + @Override + public void run() { + try { + processRequest(request); + } catch (final Throwable t) { + Ln.d(t, "An unexpected error occured when processsing request %s", request.toString()); + } + } + + @Override + public int getPriority() { + return request.getPriority(); + } + }); + request.setFuture(future); + } + + private void handleRetry(final CachedSpiceRequest request, final SpiceException e) { + if (request.getRetryPolicy() != null) { + request.getRetryPolicy().retry(e); + if (request.getRetryPolicy().getRetryCount() > 0) { + new Thread(new Runnable() { + @Override + public void run() { + try { + Thread.sleep(request.getRetryPolicy().getDelayBeforeRetry()); + planRequestExecution(request); + } catch (InterruptedException e) { + Ln.e(e, "Retry attempt failed for request " + request); + } + } + }).start(); + return; + } + } + notifyListenersOfRequestFailure(request, e); + } + private void post(final Runnable r, final Object token) { handlerResponse.postAtTime(r, token, SystemClock.uptimeMillis()); } diff --git a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/SpiceRequest.java b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/SpiceRequest.java index 18b4ac07d..a4afa353e 100644 --- a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/SpiceRequest.java +++ b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/request/SpiceRequest.java @@ -9,6 +9,8 @@ import com.octo.android.robospice.request.listener.RequestProgress; import com.octo.android.robospice.request.listener.RequestProgressListener; import com.octo.android.robospice.request.listener.RequestStatus; +import com.octo.android.robospice.retry.DefaultRetryPolicy; +import com.octo.android.robospice.retry.RetryPolicy; /** * Base class for writing requests in RoboSpice. Simply override @@ -33,11 +35,26 @@ public abstract class SpiceRequest implements Comparable clazz) { checkInnerClassDeclarationToPreventMemoryLeak(); this.resultType = clazz; } + public RetryPolicy getRetryPolicy() { + return retryPolicy; + } + + /** + * Set a {@link RetryPolicy} that will be responsible to coordinate retry + * attempts by the RequestProcessor. Can be null (no retry). + * @param retryPolicy + */ + public void setRetryPolicy(RetryPolicy retryPolicy) { + this.retryPolicy = retryPolicy; + } + /** * Sets the priority of the request. Use priority constants or a positive * integer. Will have no effect on a request after it starts being executed. diff --git a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/DefaultRetryPolicy.java b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/DefaultRetryPolicy.java new file mode 100644 index 000000000..7b26c0ad4 --- /dev/null +++ b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/DefaultRetryPolicy.java @@ -0,0 +1,71 @@ +package com.octo.android.robospice.retry; + +import com.octo.android.robospice.persistence.exception.SpiceException; + +/** + * Default {@link RetryPolicy} implementation. Proposes an exponential back off + * algorithm. When {@link #getRetryCount()} returns 0, the request is not + * retried anymore and will fail. Between each retry attempt, the request + * processor will sleel for {@link #getDelayBeforeRetry()} milliseconds. + * @author SNI + */ +public class DefaultRetryPolicy implements RetryPolicy { + + /** The default number of retry attempts. */ + public static final int DEFAULT_RETRY_COUNT = 3; + + /** The default delay before retry a request (in ms). */ + public static final long DEFAULT_DELAY_BEFORE_RETRY = 2500; + + /** The default backoff multiplier. */ + public static final float DEFAULT_BACKOFF_MULT = 1f; + + /** The number of retry attempts. */ + private int retryCount = DEFAULT_RETRY_COUNT; + + /** + * The delay to wait before next retry attempt. Will be multiplied by + * {@link #backOffMultiplier} between every retry attempt. + */ + private long delayBeforeRetry = DEFAULT_DELAY_BEFORE_RETRY; + + /** + * The backoff multiplier. Will be multiplied by {@link #delayBeforeRetry} + * between every retry attempt. + */ + private float backOffMultiplier = DEFAULT_BACKOFF_MULT; + + // ---------------------------------- + // CONSTRUCTORS + // ---------------------------------- + public DefaultRetryPolicy(int retryCount, long delayBeforeRetry, float backOffMultiplier) { + this.retryCount = retryCount; + this.delayBeforeRetry = delayBeforeRetry; + this.backOffMultiplier = backOffMultiplier; + } + + public DefaultRetryPolicy() { + this(DEFAULT_RETRY_COUNT, DEFAULT_DELAY_BEFORE_RETRY, DEFAULT_BACKOFF_MULT); + } + + // ---------------------------------- + // PUBLIC API + // ---------------------------------- + + @Override + public int getRetryCount() { + return retryCount; + } + + @Override + public void retry(SpiceException e) { + retryCount--; + delayBeforeRetry = (long) (delayBeforeRetry * backOffMultiplier); + } + + @Override + public long getDelayBeforeRetry() { + return 0; + } + +} diff --git a/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/RetryPolicy.java b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/RetryPolicy.java new file mode 100644 index 000000000..4dc74e7e5 --- /dev/null +++ b/robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/retry/RetryPolicy.java @@ -0,0 +1,22 @@ +package com.octo.android.robospice.retry; + +import com.octo.android.robospice.persistence.exception.SpiceException; + +/** + * Defines the behavior of a retry policy. + * @author SNI + */ +public interface RetryPolicy { + + /** + * @return the remaining number of retry attempts. When this method returns + * 0, request is not retried anymore. + */ + int getRetryCount(); + + /** @return the delay to sleep between each retry attempt (in ms). */ + void retry(SpiceException e); + + /** @return the delay to sleep between each retry attempt (in ms). */ + long getDelayBeforeRetry(); +}