Skip to content

Commit 7ce4456

Browse files
nitzanjnitzanj
authored
Remove a (not required) check from DefaultRequestProcessor and Improve tests
1 parent bf7d741 commit 7ce4456

File tree

3 files changed

+115
-63
lines changed

3 files changed

+115
-63
lines changed

‎lib/src/androidTest/java/com/cloudinary/android/RequestProcessorTest.java‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.cloudinary.android.payload.FilePayload;
1111
import com.cloudinary.android.payload.LocalUriPayload;
1212
import com.cloudinary.android.payload.ResourcePayload;
13+
import com.cloudinary.strategies.AbstractUploaderStrategy;
1314

1415
import org.awaitility.Awaitility;
1516
import org.awaitility.Duration;
@@ -23,6 +24,7 @@
2324

2425
import static org.junit.Assert.assertEquals;
2526
import static org.junit.Assert.assertNotNull;
27+
import static org.junit.Assert.assertNull;
2628

2729
@RunWith(AndroidJUnit4.class)
2830
public class RequestProcessorTest extends AbstractTest {
@@ -60,11 +62,12 @@ public void testValidUploadWithParams() throws IOException {
6062
options.put("unsigned", true);
6163
options.put("upload_preset", TEST_PRESET);
6264
params.putString("options", UploadRequest.encodeOptions(options));
63-
65+
params.putInt("maxErrorRetries", 0);
6466
DefaultCallbackDispatcher callbackDispatcher = provideCallbackDispatcher();
6567
RequestProcessor processor = provideRequestProcessor(callbackDispatcher);
6668
final StatefulCallback statefulCallback = new StatefulCallback();
6769
callbackDispatcher.registerCallback(statefulCallback);
70+
6871
processor.processRequest(InstrumentationRegistry.getTargetContext(), params);
6972

7073
// wait for result
@@ -182,7 +185,7 @@ public Boolean call() throws Exception {
182185
}
183186

184187
@Test
185-
public void testMaxRetries() throws IOException {
188+
public void testMaxRetries() throws IOException, NoSuchFieldException, IllegalAccessException {
186189
RequestParams params = provideRequestParams();
187190
params.putString("uri", new FilePayload(assetToFile(TEST_IMAGE).getAbsolutePath()).toUri());
188191
HashMap<String, Object> options = new HashMap<>();
@@ -192,12 +195,35 @@ public void testMaxRetries() throws IOException {
192195
options.put("unsigned", true);
193196
options.put("upload_preset", TEST_PRESET);
194197
params.putString("options", UploadRequest.encodeOptions(options));
195-
params.putInt("errorCount", 15);
198+
params.putInt("errorCount", 5);
199+
params.putInt("maxErrorRetries", 6);
196200

197201
DefaultCallbackDispatcher callbackDispatcher = provideCallbackDispatcher();
198202
RequestProcessor processor = provideRequestProcessor(callbackDispatcher);
199203
final StatefulCallback statefulCallback = new StatefulCallback();
200204
callbackDispatcher.registerCallback(statefulCallback);
205+
206+
AbstractUploaderStrategy prev = TestUtils.replaceWithTimeoutStrategy(MediaManager.get().getCloudinary());
207+
208+
// run once, expecting the request to fail but 'reschedule'
209+
processor.processRequest(InstrumentationRegistry.getTargetContext(), params);
210+
211+
// wait for result
212+
Awaitility.await().atMost(Duration.TEN_SECONDS).until(new Callable<Boolean>() {
213+
@Override
214+
public Boolean call() {
215+
return statefulCallback.lastReschedule != null;
216+
}
217+
});
218+
219+
assertNotNull(statefulCallback.lastReschedule);
220+
assertNull(statefulCallback.lastSuccess);
221+
assertEquals(6, params.getInt("errorCount", 0));
222+
223+
StatefulCallback anotherStateful = new StatefulCallback();
224+
callbackDispatcher.registerCallback(anotherStateful);
225+
226+
// run a second time, expecting a failure (too many errors):
201227
processor.processRequest(InstrumentationRegistry.getTargetContext(), params);
202228

203229
// wait for result
@@ -210,6 +236,10 @@ public Boolean call() {
210236

211237
assertNotNull(statefulCallback.lastErrorObject);
212238
assertEquals(ErrorInfo.TOO_MANY_ERRORS, statefulCallback.lastErrorObject.getCode());
239+
240+
241+
// put it back:
242+
TestUtils.replaceStrategyForIntsance(MediaManager.get().getCloudinary(), prev);
213243
}
214244

215245
/**

‎lib/src/androidTest/java/com/cloudinary/android/TestUtils.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package com.cloudinary.android;
2+
3+
import com.cloudinary.Cloudinary;
4+
import com.cloudinary.ProgressCallback;
5+
import com.cloudinary.strategies.AbstractUploaderStrategy;
6+
7+
import java.io.IOException;
8+
import java.lang.reflect.Field;
9+
import java.util.Map;
10+
11+
public class TestUtils {
12+
public static AbstractUploaderStrategy replaceWithTimeoutStrategy(Cloudinary cloudinary) throws NoSuchFieldException, IllegalAccessException, IOException {
13+
return TestUtils.replaceStrategyForIntsance(cloudinary, new AbstractUploaderStrategy() {
14+
15+
@Override
16+
public Map callApi(String action, Map<String, Object> params, Map options, Object file, ProgressCallback progressCallback) throws IOException {
17+
throw new IOException();
18+
}
19+
});
20+
}
21+
22+
public static AbstractUploaderStrategy replaceStrategyForIntsance(Cloudinary cld, AbstractUploaderStrategy replacement) throws NoSuchFieldException, IllegalAccessException {
23+
Field uploaderStrategy = Cloudinary.class.getDeclaredField("uploaderStrategy");
24+
uploaderStrategy.setAccessible(true);
25+
AbstractUploaderStrategy prev = (AbstractUploaderStrategy) uploaderStrategy.get(cld);
26+
uploaderStrategy.set(cld, replacement);
27+
return prev;
28+
}
29+
}

‎lib/src/main/java/com/cloudinary/android/DefaultRequestProcessor.java‎

Lines changed: 53 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -76,79 +76,72 @@ public UploadStatus processRequest(Context context, RequestParams params) {
7676

7777
ErrorInfo error = null;
7878

79-
if (errorCount < maxErrorRetries) {
80-
if (optionsLoadedSuccessfully) {
81-
if (StringUtils.isNotBlank(uri)) {
82-
Payload payload = PayloadFactory.fromUri(uri);
83-
if (payload != null) {
84-
try {
85-
runningJobs.incrementAndGet();
86-
resultData = doProcess(requestId, appContext, options, params, payload);
87-
requestResultStatus = SUCCESS;
88-
} catch (FileNotFoundException e) {
89-
Logger.e(TAG, String.format("FileNotFoundException for request %s.", requestId), e);
90-
requestResultStatus = FAILURE;
91-
error = new ErrorInfo(ErrorInfo.FILE_DOES_NOT_EXIST, e.getMessage());
92-
} catch (LocalUriNotFoundException e) {
93-
Logger.e(TAG, String.format("LocalUriNotFoundException for request %s.", requestId), e);
94-
requestResultStatus = FAILURE;
95-
error = new ErrorInfo(ErrorInfo.URI_DOES_NOT_EXIST, e.getMessage());
96-
} catch (ResourceNotFoundException e) {
97-
Logger.e(TAG, String.format("ResourceNotFoundException for request %s.", requestId), e);
98-
error = new ErrorInfo(ErrorInfo.RESOURCE_DOES_NOT_EXIST, e.getMessage());
99-
requestResultStatus = FAILURE;
100-
} catch (EmptyByteArrayException e) {
101-
Logger.e(TAG, String.format("EmptyByteArrayException for request %s.", requestId), e);
102-
requestResultStatus = FAILURE;
103-
error = new ErrorInfo(ErrorInfo.BYTE_ARRAY_PAYLOAD_EMPTY, e.getMessage());
104-
} catch (InterruptedIOException e) {
105-
Logger.e(TAG, String.format("InterruptedIO exception for request %s.", requestId), e);
106-
error = new ErrorInfo(ErrorInfo.REQUEST_CANCELLED, "Request cancelled.");
107-
requestResultStatus = FAILURE;
108-
} catch (ErrorRetrievingSignatureException e) {
109-
Logger.e(TAG, String.format("Error retrieving signature for request %s.", requestId), e);
110-
requestResultStatus = FAILURE;
111-
error = new ErrorInfo(ErrorInfo.SIGNATURE_FAILURE, e.getMessage());
112-
} catch (IOException e) {
113-
Logger.e(TAG, String.format("IOException for request %s.", requestId), e);
79+
if (optionsLoadedSuccessfully) {
80+
if (StringUtils.isNotBlank(uri)) {
81+
Payload payload = PayloadFactory.fromUri(uri);
82+
if (payload != null) {
83+
try {
84+
runningJobs.incrementAndGet();
85+
resultData = doProcess(requestId, appContext, options, params, payload);
86+
requestResultStatus = SUCCESS;
87+
} catch (FileNotFoundException e) {
88+
Logger.e(TAG, String.format("FileNotFoundException for request %s.", requestId), e);
89+
requestResultStatus = FAILURE;
90+
error = new ErrorInfo(ErrorInfo.FILE_DOES_NOT_EXIST, e.getMessage());
91+
} catch (LocalUriNotFoundException e) {
92+
Logger.e(TAG, String.format("LocalUriNotFoundException for request %s.", requestId), e);
93+
requestResultStatus = FAILURE;
94+
error = new ErrorInfo(ErrorInfo.URI_DOES_NOT_EXIST, e.getMessage());
95+
} catch (ResourceNotFoundException e) {
96+
Logger.e(TAG, String.format("ResourceNotFoundException for request %s.", requestId), e);
97+
error = new ErrorInfo(ErrorInfo.RESOURCE_DOES_NOT_EXIST, e.getMessage());
98+
requestResultStatus = FAILURE;
99+
} catch (EmptyByteArrayException e) {
100+
Logger.e(TAG, String.format("EmptyByteArrayException for request %s.", requestId), e);
101+
requestResultStatus = FAILURE;
102+
error = new ErrorInfo(ErrorInfo.BYTE_ARRAY_PAYLOAD_EMPTY, e.getMessage());
103+
} catch (InterruptedIOException e) {
104+
Logger.e(TAG, String.format("InterruptedIO exception for request %s.", requestId), e);
105+
error = new ErrorInfo(ErrorInfo.REQUEST_CANCELLED, "Request cancelled.");
106+
requestResultStatus = FAILURE;
107+
} catch (ErrorRetrievingSignatureException e) {
108+
Logger.e(TAG, String.format("Error retrieving signature for request %s.", requestId), e);
109+
requestResultStatus = FAILURE;
110+
error = new ErrorInfo(ErrorInfo.SIGNATURE_FAILURE, e.getMessage());
111+
} catch (IOException e) {
112+
Logger.e(TAG, String.format("IOException for request %s.", requestId), e);
114113

115-
if (errorCount >= maxErrorRetries) {
116-
// failure
117-
error = getMaxRetryError(errorCount);
118-
requestResultStatus = FAILURE;
119-
} else {
120-
// one up error count and reschedule
121-
params.putInt(ERROR_COUNT_PARAM, errorCount + 1);
122-
error = new ErrorInfo(ErrorInfo.NETWORK_ERROR, e.getMessage());
123-
requestResultStatus = RESCHEDULE;
124-
}
125-
} catch (Exception e) {
126-
Logger.e(TAG, String.format("Unexpected exception for request %s.", requestId), e);
127-
error = new ErrorInfo(ErrorInfo.UNKNOWN_ERROR, e.getMessage());
114+
if (errorCount >= maxErrorRetries) {
115+
// failure
116+
error = getMaxRetryError(errorCount);
128117
requestResultStatus = FAILURE;
129-
} finally {
130-
runningJobs.decrementAndGet();
118+
} else {
119+
// one up error count and reschedule
120+
params.putInt(ERROR_COUNT_PARAM, errorCount + 1);
121+
error = new ErrorInfo(ErrorInfo.NETWORK_ERROR, e.getMessage());
122+
requestResultStatus = RESCHEDULE;
131123
}
132-
} else {
133-
Logger.d(TAG, String.format("Failing request %s, payload cannot be loaded.", requestId));
134-
error = new ErrorInfo(ErrorInfo.PAYLOAD_LOAD_FAILURE, "Request payload could not be loaded.");
124+
} catch (Exception e) {
125+
Logger.e(TAG, String.format("Unexpected exception for request %s.", requestId), e);
126+
error = new ErrorInfo(ErrorInfo.UNKNOWN_ERROR, e.getMessage());
135127
requestResultStatus = FAILURE;
128+
} finally {
129+
runningJobs.decrementAndGet();
136130
}
137131
} else {
132+
Logger.d(TAG, String.format("Failing request %s, payload cannot be loaded.", requestId));
133+
error = new ErrorInfo(ErrorInfo.PAYLOAD_LOAD_FAILURE, "Request payload could not be loaded.");
138134
requestResultStatus = FAILURE;
139-
error = new ErrorInfo(ErrorInfo.PAYLOAD_EMPTY, "Request payload is empty.");
140-
Logger.d(TAG, String.format("Failing request %s, Uri is empty.", requestId));
141135
}
142136
} else {
143137
requestResultStatus = FAILURE;
144-
error = new ErrorInfo(ErrorInfo.OPTIONS_FAILURE, "Options could not be loaded.");
145-
Logger.d(TAG, String.format("Failing request %s, cannot load options.", requestId));
138+
error = new ErrorInfo(ErrorInfo.PAYLOAD_EMPTY, "Request payload is empty.");
139+
Logger.d(TAG, String.format("Failing request %s, Uri is empty.", requestId));
146140
}
147141
} else {
148-
// safety - this should not really happen - the last error should already trigger
149-
// as failure and should not be rescheduled
150-
error = getMaxRetryError(errorCount);
151142
requestResultStatus = FAILURE;
143+
error = new ErrorInfo(ErrorInfo.OPTIONS_FAILURE, "Options could not be loaded.");
144+
Logger.d(TAG, String.format("Failing request %s, cannot load options.", requestId));
152145
}
153146

154147
if (requestResultStatus.isFinal()) {

0 commit comments

Comments
 (0)