Skip to content
This repository has been archived by the owner on Jan 14, 2018. It is now read-only.

Commit

Permalink
Request now have a retry policy. Issue #99. Tested.
Browse files Browse the repository at this point in the history
  • Loading branch information
stephanenicolas committed May 26, 2013
1 parent 48a4c4e commit 5bfb851
Show file tree
Hide file tree
Showing 9 changed files with 304 additions and 29 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.



Expand Down
2 changes: 1 addition & 1 deletion RoboSpice-samples
Submodule RoboSpice-samples updated from d2cc8d to 766018
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
<properties>
<!-- DEPENDENCIES VERSIONS -->
<android.version>4.1.1.4</android.version>
<android-platform.version>14</android-platform.version>
<android-platform.version>17</android-platform.version>
<android-annotations.version>4.1.1.4</android-annotations.version>
<android-support.version>r7</android-support.version>
<easymock.version>2.5.2</easymock.version>
Expand All @@ -103,7 +103,7 @@
<commons-logging.version>1.1.1</commons-logging.version>

<!-- PLUGINS VERSIONS -->
<android-maven-plugin.version>3.5.1</android-maven-plugin.version>
<android-maven-plugin.version>3.6.0</android-maven-plugin.version>
<site-maven-plugin.version>0.8</site-maven-plugin.version>
<maven-project-info-reports-plugin.version>2.6</maven-project-info-reports-plugin.version>
<maven-surefire-report-plugin.version>2.6</maven-surefire-report-plugin.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String> stubRequest = createFailedRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION);
stubRequest.setRetryPolicy(null);

RequestListenerWithProgressStub<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
Expand Down Expand Up @@ -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<String> stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA);
stubRequest.setRetryPolicy(null);

RequestListenerWithProgressStub<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
Expand All @@ -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<String> stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA);
stubRequest.setRetryPolicy(null);

RequestListenerWithProgressStub<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
Expand Down Expand Up @@ -481,6 +488,7 @@ public void testRemoveAllDataFromCache_for_given_class_and_cachekey() {
public void testAddRequestWhenNetworkIsDown() throws CacheLoadingException, CacheSavingException, InterruptedException {
// given
CachedSpiceRequestStub<String> stubRequest = createSuccessfulRequest(TEST_CLASS, TEST_CACHE_KEY, TEST_DURATION, TEST_RETURNED_DATA);
stubRequest.setRetryPolicy(null);

RequestListenerStub<String> mockRequestListener = new RequestListenerStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
Expand Down Expand Up @@ -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<String> 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<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
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<String> 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<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
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<String> 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<String> mockRequestListener = new RequestListenerWithProgressStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
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<String> 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<String> mockRequestListener = new RequestListenerStub<String>();
Set<RequestListener<?>> requestListenerSet = new HashSet<RequestListener<?>>();
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
// ============================================================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,6 +31,16 @@ public CachedSpiceRequest(final SpiceRequest<RESULT> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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());
Expand All @@ -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;
}

Expand All @@ -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.");
}
Expand All @@ -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()) {
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,11 +35,26 @@ public abstract class SpiceRequest<RESULT> implements Comparable<SpiceRequest<RE
private RequestProgress progress = new RequestProgress(RequestStatus.PENDING);
private RequestCancellationListener requestCancellationListener;

private RetryPolicy retryPolicy = new DefaultRetryPolicy();

public SpiceRequest(final Class<RESULT> 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.
Expand Down
Loading

0 comments on commit 5bfb851

Please sign in to comment.