Skip to content

Commit 5ff6470

Browse files
milos1290e7mac
andauthored
LP-9360 Refactoring callbacks (#383)
* Refactoring callback manager and requests * Removing unused code * Removing unused methods * Fixing failing tests * Fixing docs * Fixing tests * Fix failing test * Fixing wrong null check Co-authored-by: Mayank Sanganeria <[email protected]>
1 parent f7a1a38 commit 5ff6470

File tree

11 files changed

+228
-414
lines changed

11 files changed

+228
-414
lines changed

AndroidSDKCore/src/main/java/com/leanplum/Leanplum.java

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import com.leanplum.internal.LeanplumMessageMatchFilter;
4747
import com.leanplum.internal.LeanplumUIEditorWrapper;
4848
import com.leanplum.internal.Log;
49+
import com.leanplum.internal.Operation;
50+
import com.leanplum.internal.OperationQueue;
4951
import com.leanplum.internal.OsHandler;
5052
import com.leanplum.internal.Registration;
5153
import com.leanplum.internal.RequestOld;
@@ -699,10 +701,10 @@ private static void startHelper(
699701

700702
// Issue start API call.
701703
final RequestOld request = RequestOld.post(Constants.Methods.START, params);
702-
request.onApiResponse(new RequestOld.ApiResponseCallback() {
704+
request.onResponse(new RequestOld.ResponseCallback() {
703705
@Override
704-
public void response(List<Map<String, Object>> requests, JSONObject response, int countOfEvents) {
705-
Leanplum.handleApiResponse(response, requests, request, countOfEvents);
706+
public void response(JSONObject response) {
707+
handleStartResponse(response);
706708
}
707709
});
708710

@@ -715,56 +717,6 @@ public void response(List<Map<String, Object>> requests, JSONObject response, in
715717
LeanplumInternal.triggerStartIssued();
716718
}
717719

718-
private static void handleApiResponse(JSONObject response, List<Map<String, Object>> requests,
719-
final RequestOld request, int countOfUnsentRequests) {
720-
JSONObject lastStartResponse = null;
721-
722-
// Find and handle the last start response.
723-
try {
724-
// Checks if START event inside the current batch. If database index of START event bigger
725-
// then a number of count of events that we got from the database - decrease START event
726-
// database index.
727-
if (request.getDataBaseIndex() >= countOfUnsentRequests) {
728-
request.setDataBaseIndex(request.getDataBaseIndex() - countOfUnsentRequests);
729-
return;
730-
}
731-
lastStartResponse = parseLastStartResponse(response, requests);
732-
} catch (Throwable t) {
733-
Util.handleException(t);
734-
}
735-
736-
if (lastStartResponse != null) {
737-
if (!LeanplumInternal.hasStarted()) {
738-
// Set start response to null.
739-
request.onApiResponse(null);
740-
}
741-
}
742-
Leanplum.handleStartResponse(lastStartResponse);
743-
}
744-
745-
@VisibleForTesting
746-
public static JSONObject parseLastStartResponse(JSONObject response, List<Map<String, Object>> requests) {
747-
final int responseCount = RequestOld.numResponses(response);
748-
for (int i = requests.size() - 1; i >= 0; i--) {
749-
Map<String, Object> currentRequest = requests.get(i);
750-
if (Constants.Methods.START.equals(currentRequest.get(Constants.Params.ACTION))) {
751-
if (currentRequest.containsKey(RequestOld.REQUEST_ID_KEY)) {
752-
for (int j = RequestOld.numResponses(response) - 1; j >= 0; j--) {
753-
JSONObject currentResponse = RequestOld.getResponseAt(response, j);
754-
if (currentRequest.get(RequestOld.REQUEST_ID_KEY)
755-
.equals(currentResponse.optString(RequestOld.REQUEST_ID_KEY))) {
756-
return currentResponse;
757-
}
758-
}
759-
}
760-
if (i < responseCount) {
761-
return RequestOld.getResponseAt(response, i);
762-
}
763-
}
764-
}
765-
return null;
766-
}
767-
768720
private static void handleStartResponse(final JSONObject response) {
769721
Util.executeAsyncTask(false, new AsyncTask<Void, Void, Void>() {
770722
@Override

AndroidSDKCore/src/main/java/com/leanplum/internal/Constants.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ public static class Methods {
122122
}
123123

124124
public static class Params {
125+
public static final String RESPONSE = "response";
126+
125127
public static final String ACTION = "action";
126128
public static final String ACTION_DEFINITIONS = "actionDefinitions";
127129
public static final String APP_ID = "appId";
@@ -172,6 +174,7 @@ public static class Params {
172174
public static final String VALUE = "value";
173175
public static final String VARS = "vars";
174176
public static final String VERSION_NAME = "versionName";
177+
public static final String REQUEST_ID = "reqId";
175178
}
176179

177180
public static class Keys {

AndroidSDKCore/src/main/java/com/leanplum/internal/LeanplumEventCallbackManager.java

Lines changed: 89 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,24 @@
2020
*/
2121
package com.leanplum.internal;
2222

23-
import android.os.AsyncTask;
24-
import androidx.annotation.NonNull;
25-
2623
import com.leanplum.Leanplum;
2724

2825
import org.json.JSONObject;
2926

30-
import java.util.HashMap;
31-
import java.util.Iterator;
27+
import java.util.ArrayList;
3228
import java.util.Map;
29+
import java.util.concurrent.ConcurrentHashMap;
30+
31+
import androidx.annotation.NonNull;
3332

3433
/**
3534
* LeanplumEventCallbackManager class to handle event callbacks.
3635
*
3736
* @author Anna Orlova
3837
*/
3938
class LeanplumEventCallbackManager {
40-
// Event callbacks map.
41-
private final Map<RequestOld, LeanplumEventCallbacks> eventCallbacks = new HashMap<>();
39+
40+
private final ConcurrentHashMap<String, LeanplumEventCallbacks> callbacks = new ConcurrentHashMap<>();
4241

4342
/**
4443
* Add callbacks to the event callbacks Map.
@@ -48,7 +47,7 @@ class LeanplumEventCallbackManager {
4847
* @param errorCallback Error callback.
4948
*/
5049
void addCallbacks(RequestOld request, RequestOld.ResponseCallback responseCallback,
51-
RequestOld.ErrorCallback errorCallback) {
50+
RequestOld.ErrorCallback errorCallback) {
5251
if (request == null) {
5352
return;
5453
}
@@ -57,90 +56,112 @@ void addCallbacks(RequestOld request, RequestOld.ResponseCallback responseCallba
5756
return;
5857
}
5958

60-
eventCallbacks.put(request, new LeanplumEventCallbacks(responseCallback, errorCallback));
59+
callbacks.put(request.requestId(), new LeanplumEventCallbacks(responseCallback, errorCallback));
6160
Leanplum.countAggregator().incrementCount("add_event_callback_at");
6261
}
6362

6463
/**
65-
* Invoke potential error callbacks for all events with database index less than a count of events
66-
* that we got from database.
64+
* Invoke all callbacks for response body. Callbacks will be executed based on success flag for
65+
* each request
6766
*
68-
* @param error Exception.
69-
* @param countOfEvents Count of events that we got from database.
67+
* @param body response body
7068
*/
71-
void invokeAllCallbacksWithError(@NonNull final Exception error, int countOfEvents) {
72-
if (eventCallbacks.size() == 0) {
69+
void invokeCallbacks(JSONObject body) {
70+
if (body == null) {
7371
return;
7472
}
7573

76-
Iterator<Map.Entry<RequestOld, LeanplumEventCallbacks>> iterator =
77-
eventCallbacks.entrySet().iterator();
78-
// Loop over all callbacks.
79-
for (; iterator.hasNext(); ) {
80-
final Map.Entry<RequestOld, LeanplumEventCallbacks> entry = iterator.next();
81-
if (entry.getKey() == null) {
82-
continue;
83-
}
84-
if (entry.getKey().getDataBaseIndex() >= countOfEvents) {
85-
entry.getKey().setDataBaseIndex(entry.getKey().getDataBaseIndex() - countOfEvents);
86-
} else {
87-
if (entry.getValue() != null && entry.getValue().errorCallback != null) {
88-
// Start callback asynchronously, to avoid creation of new RequestOld object from the same
89-
// thread.
90-
Util.executeAsyncTask(false, new AsyncTask<Void, Void, Void>() {
91-
@Override
92-
protected Void doInBackground(Void... params) {
93-
entry.getValue().errorCallback.error(error);
94-
return null;
95-
}
96-
});
74+
if (callbacks.size() == 0) {
75+
return;
76+
}
77+
78+
ArrayList<String> keys = new ArrayList<>();
79+
80+
for (Map.Entry<String, LeanplumEventCallbacks> pair : callbacks.entrySet()) {
81+
final String reqId = pair.getKey();
82+
final LeanplumEventCallbacks callbacks = pair.getValue();
83+
84+
if (reqId != null && callbacks != null) {
85+
// get the response for specified reqId
86+
final JSONObject response = RequestOld.getResponseForId(body, reqId);
87+
if (response != null) {
88+
boolean isSuccess = RequestOld.isResponseSuccess(response);
89+
90+
// if response for event is successful, execute success callback
91+
if (isSuccess) {
92+
93+
OperationQueue.sharedInstance().addParallelOperation(new Runnable() {
94+
@Override
95+
public void run() {
96+
if (callbacks.responseCallback != null) {
97+
callbacks.responseCallback.response(response);
98+
}
99+
}
100+
});
101+
} else {
102+
// otherwise find the error message and execute error callback
103+
final String responseError = RequestOld.getResponseError(response);
104+
final String msg = RequestOld.getReadableErrorMessage(responseError);
105+
106+
OperationQueue.sharedInstance().addParallelOperation(new Runnable() {
107+
@Override
108+
public void run() {
109+
if (callbacks.errorCallback != null) {
110+
callbacks.errorCallback.error(new Exception(msg));
111+
}
112+
}
113+
});
114+
}
115+
116+
// add key for removal
117+
keys.add(reqId);
97118
}
98-
iterator.remove();
99119
}
100120
}
101-
Leanplum.countAggregator().incrementCount("invoke_error_callbacks_on_responses");
121+
122+
// remove all keys for which callbacks were executed
123+
for (String key : keys) {
124+
callbacks.remove(key);
125+
}
102126
}
103127

104128
/**
105-
* Invoke potential response callbacks for all events with database index less than a count of
106-
* events that we got from database.
129+
* Invoke potential error callbacks for all events which have added callbacks.
107130
*
108-
* @param responseBody JSONObject withs server response.
109-
* @param countOfEvents Count of events that we got from database.
131+
* @param error Exception.
110132
*/
111-
void invokeAllCallbacksForResponse(@NonNull final JSONObject responseBody, int countOfEvents) {
112-
if (eventCallbacks.size() == 0) {
133+
void invokeAllCallbacksWithError(@NonNull final Exception error) {
134+
if (callbacks.size() == 0) {
113135
return;
114136
}
115137

116-
Iterator<Map.Entry<RequestOld, LeanplumEventCallbacks>> iterator =
117-
eventCallbacks.entrySet().iterator();
118-
// Loop over all callbacks.
119-
for (; iterator.hasNext(); ) {
120-
final Map.Entry<RequestOld, LeanplumEventCallbacks> entry = iterator.next();
121-
if (entry.getKey() == null) {
122-
continue;
123-
}
138+
ArrayList<String> keys = new ArrayList<>();
124139

125-
if (entry.getKey().getDataBaseIndex() >= countOfEvents) {
126-
entry.getKey().setDataBaseIndex(entry.getKey().getDataBaseIndex() - countOfEvents);
127-
} else {
128-
if (entry.getValue() != null && entry.getValue().responseCallback != null) {
129-
// Start callback asynchronously, to avoid creation of new RequestOld object from the same
130-
// thread.
131-
Util.executeAsyncTask(false, new AsyncTask<Void, Void, Void>() {
132-
@Override
133-
protected Void doInBackground(Void... params) {
134-
entry.getValue().responseCallback.response(RequestOld.getResponseAt(responseBody,
135-
(int) entry.getKey().getDataBaseIndex()));
136-
return null;
140+
for (Map.Entry<String, LeanplumEventCallbacks> pair : callbacks.entrySet()) {
141+
String reqId = pair.getKey();
142+
final LeanplumEventCallbacks callbacks = pair.getValue();
143+
if (callbacks != null) {
144+
// executed all error callbacks in parallel
145+
OperationQueue.sharedInstance().addParallelOperation(new Runnable() {
146+
@Override
147+
public void run() {
148+
if (callbacks.errorCallback != null) {
149+
callbacks.errorCallback.error(error);
137150
}
138-
});
139-
}
140-
iterator.remove();
151+
}
152+
});
153+
154+
// add key for removal
155+
keys.add(reqId);
141156
}
142157
}
143-
Leanplum.countAggregator().incrementCount("invoke_success_callbacks_on_responses");
158+
159+
// remove all keys for which callbacks were executed
160+
for (String key : keys) {
161+
callbacks.remove(key);
162+
}
163+
164+
Leanplum.countAggregator().incrementCount("invoke_error_callbacks_on_responses");
144165
}
145166

146167
private static class LeanplumEventCallbacks {

0 commit comments

Comments
 (0)