Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove unused session history upload code #507

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import com.mparticle.MParticle
import com.mparticle.MParticleOptions
import com.mparticle.networking.Matcher
import com.mparticle.testutils.BaseCleanStartedEachTest
import org.json.JSONObject
import org.junit.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotEquals
Expand All @@ -19,16 +18,6 @@ class BatchSessionInfoTest : BaseCleanStartedEachTest() {
return builder.logLevel(MParticle.LogLevel.INFO)
}

override fun beforeSetup() {
// the condition described in the test only happened when `sessionHistory` is false,
// so set config to return `sessionHistory` == false
mServer.setupConfigResponse(
JSONObject()
.put(ConfigManager.KEY_INCLUDE_SESSION_HISTORY, false)
.toString()
)
}

/**
* This test is in response to a bug where, when many messages (> 1 batch worth)
* are uploaded with for a Session other than the current Session, batches after the first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ public class ConfigManager {
public static final String VALUE_APP_DEFINED = "appdefined";
public static final String VALUE_CUE_CATCH = "forcecatch";
public static final String PREFERENCES_FILE = "mp_preferences";
public static final String KEY_INCLUDE_SESSION_HISTORY = "inhd";
private static final String KEY_DEVICE_PERFORMANCE_METRICS_DISABLED = "dpmd";
public static final String WORKSPACE_TOKEN = "wst";
static final String ALIAS_MAX_WINDOW = "alias_max_window";
Expand Down Expand Up @@ -98,7 +97,6 @@ public class ConfigManager {
private long mInfluenceOpenTimeout = 3600 * 1000;
private JSONArray mTriggerMessageMatches, mTriggerMessageHashes = null;
private ExceptionHandler mExHandler;
private boolean mIncludeSessionHistory = false;
private JSONObject mCurrentCookies;
private String mDataplanId;
private Integer mDataplanVersion;
Expand Down Expand Up @@ -453,7 +451,6 @@ private synchronized void updateCoreConfig(JSONObject responseJSON, boolean newC
mInfluenceOpenTimeout = 30 * 60 * 1000;
}

mIncludeSessionHistory = responseJSON.optBoolean(KEY_INCLUDE_SESSION_HISTORY, true);
if (responseJSON.has(KEY_DEVICE_PERFORMANCE_METRICS_DISABLED)) {
MessageManager.devicePerformanceMetricsDisabled = responseJSON.optBoolean(KEY_DEVICE_PERFORMANCE_METRICS_DISABLED, false);
}
Expand Down Expand Up @@ -504,10 +501,6 @@ public String getActiveModuleIds() {
}
}

public boolean getIncludeSessionHistory() {
return mIncludeSessionHistory;
}

/**
* When the Config manager starts up, we don't want to enable everything immediately to save on app-load time.
* This method will be called from a background thread after startup is already complete.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ public interface MessageKey {
String ERROR_SESSION_COUNT = "sn";
// uploading
String MESSAGES = "msgs";
String HISTORY = "sh";
String REPORTING = "fsr";
String URL = "u";
String METHOD = "m";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,6 @@ private void logUpload(String message) {
for (int i = 0; i < messages.length(); i++) {
Logger.verbose("Message type: " + ((JSONObject) messages.get(i)).getString(Constants.MessageKey.TYPE));
}
} else if (messageJson.has(Constants.MessageKey.HISTORY)) {
JSONArray messages = messageJson.getJSONArray(Constants.MessageKey.HISTORY);
Logger.verbose("Uploading session history batch...");
for (int i = 0; i < messages.length(); i++) {
Logger.verbose("Message type: " + ((JSONObject) messages.get(i)).getString(Constants.MessageKey.TYPE) + " SID: " + ((JSONObject) messages.get(i)).optString(Constants.MessageKey.SESSION_ID));
}
}
} catch (JSONException jse) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,6 @@ public void addDataplanContext(String dataplanId, Integer dataplanVersion) throw
}
}

public void addSessionHistoryMessage(JSONObject message) {
try {
if (!has(Constants.MessageKey.HISTORY)) {
put(Constants.MessageKey.HISTORY, new JSONArray());
}
getJSONArray(Constants.MessageKey.HISTORY).put(message);
} catch (JSONException ignored) {
}
}

public void addMessage(JSONObject message) {
try {
if (!has(Constants.MessageKey.MESSAGES)) {
Expand Down Expand Up @@ -160,14 +150,6 @@ public JSONObject getDeviceInfo() {
}
}

public JSONArray getSessionHistoryMessages() {
try {
return getJSONArray(Constants.MessageKey.HISTORY);
} catch (JSONException e) {
return null;
}
}

public JSONArray getMessages() {
try {
return getJSONArray(Constants.MessageKey.MESSAGES);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@ public class UploadHandler extends BaseHandler {
* Message used to trigger the primary upload logic - will upload all non-history batches that are ready to go.
*/
public static final int UPLOAD_MESSAGES = 1;
/**
* Message that triggers much of the same logic as above, but is specifically for session-history. Typically, the SDK will upload all messages
* in a given batch that are ready for upload. But, some service-providers such as Flurry need to be sent all of the session information at once
* With *history*, the SDK will package all of the messages that occur in a particular session.
*/
public static final int UPLOAD_HISTORY = 3;
/**
* Trigger a configuration update out of band from a typical upload.
*/
Expand Down Expand Up @@ -137,25 +131,15 @@ public void handleMessageImpl(Message msg) {
if (isNetworkConnected) {
if (uploadInterval > 0 || msg.arg1 == 1) {
while (mParticleDBManager.hasMessagesForUpload()) {
prepareMessageUploads(false);
}
boolean needsHistory = upload(false);
if (needsHistory) {
this.sendEmpty(UPLOAD_HISTORY);
prepareMessageUploads();
}
upload();
}
}
if (mAppStateManager.getSession().isActive() && uploadInterval > 0 && msg.arg1 == 0) {
this.sendEmptyDelayed(UPLOAD_MESSAGES, uploadInterval);
}
break;
case UPLOAD_HISTORY:
removeMessage(UPLOAD_HISTORY);
prepareMessageUploads(true);
if (isNetworkConnected) {
upload(true);
}
break;
}
} catch (MParticleApiClientImpl.MPConfigException e) {
Logger.error("Bad API request - is the correct API key and secret configured?");
Expand All @@ -175,7 +159,7 @@ public void handleMessageImpl(Message msg) {
* - persist all of the resulting upload batch objects
* - mark the messages as having been uploaded.
*/
protected void prepareMessageUploads(boolean history) throws Exception {
protected void prepareMessageUploads() throws Exception {
String currentSessionId = mAppStateManager.getSession().mSessionID;
long remainingHeap = MPUtility.getRemainingHeapInBytes();
if (remainingHeap < Constants.LIMIT_MAX_UPLOAD_SIZE) {
Expand All @@ -186,18 +170,9 @@ protected void prepareMessageUploads(boolean history) throws Exception {
if (instance == null) {
return;
}
final boolean sessionHistoryEnabled = instance.Internal().getConfigManager().getIncludeSessionHistory();
try {
mParticleDBManager.cleanupMessages();
if (history && !sessionHistoryEnabled) {
mParticleDBManager.deleteMessagesAndSessions(currentSessionId);
return;
}
if (history) {
mParticleDBManager.createSessionHistoryUploadMessage(mConfigManager, mMessageManager.getDeviceAttributes(), currentSessionId);
} else {
mParticleDBManager.createMessagesForUploadMessage(mConfigManager, mMessageManager.getDeviceAttributes(), currentSessionId, sessionHistoryEnabled);
}
mParticleDBManager.createMessagesForUploadMessage(mConfigManager, mMessageManager.getDeviceAttributes(), currentSessionId);
} catch (Exception e) {
Logger.verbose("Error preparing batch upload in mParticle DB: " + e.getMessage());
}
Expand All @@ -208,34 +183,20 @@ protected void prepareMessageUploads(boolean history) throws Exception {
/**
* This method is responsible for looking for batches that are ready to be uploaded, and uploading them.
*/
protected boolean upload(boolean history) {
protected void upload() {
mParticleDBManager.cleanupUploadMessages();
boolean processingSessionEnd = false;
try {
List<MParticleDBManager.ReadyUpload> readyUploads = mParticleDBManager.getReadyUploads();
if (readyUploads.size() > 0) {
mApiClient.fetchConfig();
}
final boolean includeSessionHistory = mConfigManager.getIncludeSessionHistory();
for (MParticleDBManager.ReadyUpload readyUpload : readyUploads) {
//This case actually shouldn't be needed anymore except for upgrade scenarios.
//As of version 4.9.0, upload batches for session history shouldn't even be created.
if (history && !includeSessionHistory) {
mParticleDBManager.deleteUpload(readyUpload.getId());
String message = readyUpload.getMessage();
InternalListenerManager.getListener().onCompositeObjects(readyUpload, message);
if (readyUpload.isAliasRequest()) {
uploadAliasRequest(readyUpload.getId(), message);
} else {
if (!history) {
// If message is the MessageType.SESSION_END, then remember so the session history can be triggered.
if (!processingSessionEnd && readyUpload.getMessage().contains(containsClause)) {
processingSessionEnd = true;
}
}
String message = readyUpload.getMessage();
InternalListenerManager.getListener().onCompositeObjects(readyUpload, message);
if (readyUpload.isAliasRequest()) {
uploadAliasRequest(readyUpload.getId(), message);
} else {
uploadMessage(readyUpload.getId(), message);
}
uploadMessage(readyUpload.getId(), message);
}
}
} catch (MParticleApiClientImpl.MPThrottleException e) {
Expand All @@ -246,7 +207,6 @@ protected boolean upload(boolean history) {
} catch (Exception e) {
Logger.error(e, "Error processing batch uploads in mParticle DB.");
}
return processingSessionEnd;
}

void uploadMessage(int id, String message) throws IOException, MParticleApiClientImpl.MPThrottleException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,35 +136,12 @@ public interface MessageListener {
* Prepare Messages for Upload.
*/

public void createSessionHistoryUploadMessage(ConfigManager configManager, DeviceAttributes deviceAttributes, String currentSessionId) throws JSONException {
MPDatabase db = getDatabase();
db.beginTransaction();
try {
List<MessageService.ReadyMessage> readyMessages = MessageService.getSessionHistory(db, currentSessionId);
if (readyMessages.size() <= 0) {
db.setTransactionSuccessful();
return;
}

HashMap<BatchId, MessageBatch> uploadMessagesByBatchId = getUploadMessageByBatchIdMap(readyMessages, db, configManager, true);

List<JSONObject> deviceInfos = SessionService.processSessions(db, uploadMessagesByBatchId);
for (JSONObject deviceInfo : deviceInfos) {
deviceAttributes.updateDeviceInfo(mContext, deviceInfo);
}
createUploads(uploadMessagesByBatchId, db, deviceAttributes, configManager, currentSessionId, true);
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
}

public boolean hasMessagesForUpload() {
MPDatabase db = getDatabase();
return MessageService.hasMessagesForUpload(db);
}

public void createMessagesForUploadMessage(ConfigManager configManager, DeviceAttributes deviceAttributes, String currentSessionId, boolean sessionHistoryEnabled) throws JSONException {
public void createMessagesForUploadMessage(ConfigManager configManager, DeviceAttributes deviceAttributes, String currentSessionId) throws JSONException {
MPDatabase db = getDatabase();
db.beginTransaction();
try {
Expand All @@ -173,7 +150,7 @@ public void createMessagesForUploadMessage(ConfigManager configManager, DeviceAt
db.setTransactionSuccessful();
return;
}
HashMap<BatchId, MessageBatch> uploadMessagesByBatchId = getUploadMessageByBatchIdMap(readyMessages, db, configManager, false, sessionHistoryEnabled);
HashMap<BatchId, MessageBatch> uploadMessagesByBatchId = getUploadMessageByBatchIdMap(readyMessages, db, configManager, false);

List<ReportingService.ReportingMessage> reportingMessages = ReportingService.getReportingMessagesForUpload(db);
for (ReportingService.ReportingMessage reportingMessage : reportingMessages) {
Expand Down Expand Up @@ -209,7 +186,7 @@ public void createMessagesForUploadMessage(ConfigManager configManager, DeviceAt
for (JSONObject deviceInfo : deviceInfos) {
deviceAttributes.updateDeviceInfo(mContext, deviceInfo);
}
createUploads(uploadMessagesByBatchId, db, deviceAttributes, configManager, currentSessionId, false, sessionHistoryEnabled);
createUploads(uploadMessagesByBatchId, db, deviceAttributes, configManager, currentSessionId);
db.setTransactionSuccessful();
} finally {
db.endTransaction();
Expand All @@ -228,11 +205,11 @@ public void deleteMessagesAndSessions(String currentSessionId) {
}
}

private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<MessageService.ReadyMessage> readyMessages, MPDatabase db, ConfigManager configManager, boolean isHistory) throws JSONException {
return getUploadMessageByBatchIdMap(readyMessages, db, configManager, isHistory, false);
private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<MessageService.ReadyMessage> readyMessages, MPDatabase db, ConfigManager configManager) throws JSONException {
return getUploadMessageByBatchIdMap(readyMessages, db, configManager, false);
}

private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<MessageService.ReadyMessage> readyMessages, MPDatabase db, ConfigManager configManager, boolean isHistory, boolean markAsUpload) throws JSONException {
private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<MessageService.ReadyMessage> readyMessages, MPDatabase db, ConfigManager configManager, boolean markAsUpload) throws JSONException {
HashMap<BatchId, MessageBatch> uploadMessagesByBatchId = new HashMap<BatchId, MessageBatch>();
int highestUploadedMessageId = -1;
for (MessageService.ReadyMessage readyMessage : readyMessages) {
Expand All @@ -247,11 +224,7 @@ private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<Message
if (messageLength + uploadMessage.getMessageLengthBytes() > Constants.LIMIT_MAX_UPLOAD_SIZE) {
break;
}
if (isHistory) {
uploadMessage.addSessionHistoryMessage(msgObject);
} else {
uploadMessage.addMessage(msgObject);
}
uploadMessage.addMessage(msgObject);
InternalListenerManager.getListener().onCompositeObjects(readyMessage, uploadMessage);
uploadMessage.incrementMessageLengthBytes(messageLength);
highestUploadedMessageId = readyMessage.getMessageId();
Expand All @@ -266,11 +239,7 @@ private HashMap<BatchId, MessageBatch> getUploadMessageByBatchIdMap(List<Message
return uploadMessagesByBatchId;
}

private void createUploads(Map<BatchId, MessageBatch> uploadMessagesByBatchId, MPDatabase db, DeviceAttributes deviceAttributes, ConfigManager configManager, String currentSessionId, boolean historyMessages) {
createUploads(uploadMessagesByBatchId, db, deviceAttributes, configManager, currentSessionId, historyMessages, false);
}

private void createUploads(Map<BatchId, MessageBatch> uploadMessagesByBatchId, MPDatabase db, DeviceAttributes deviceAttributes, ConfigManager configManager, String currentSessionId, boolean historyMessages, boolean sessionHistoryEnabled) {
private void createUploads(Map<BatchId, MessageBatch> uploadMessagesByBatchId, MPDatabase db, DeviceAttributes deviceAttributes, ConfigManager configManager, String currentSessionId) {
for (Map.Entry<BatchId, MessageBatch> messageBatchEntry : uploadMessagesByBatchId.entrySet()) {
BatchId batchId = messageBatchEntry.getKey();
MessageBatch uploadMessage = messageBatchEntry.getValue();
Expand All @@ -284,12 +253,7 @@ private void createUploads(Map<BatchId, MessageBatch> uploadMessagesByBatchId, M
if (uploadMessage.getDeviceInfo() == null || sessionId.equals(currentSessionId)) {
uploadMessage.setDeviceInfo(deviceAttributes.getDeviceInfo(mContext));
}
JSONArray messages;
if (historyMessages) {
messages = uploadMessage.getSessionHistoryMessages();
} else {
messages = uploadMessage.getMessages();
}
JSONArray messages = uploadMessage.getMessages();
JSONArray identities = findIdentityState(configManager, messages, batchId.getMpid());
uploadMessage.setIdentities(identities);
JSONObject userAttributes = findUserAttributeState(messages, batchId.getMpid());
Expand All @@ -311,12 +275,7 @@ private void createUploads(Map<BatchId, MessageBatch> uploadMessagesByBatchId, M
}

UploadService.insertUpload(db, batch, configManager.getApiKey());
//if this was to process session history, or
//if we're never going to process history AND
//this batch contains a previous session, then delete the session.
if (!historyMessages && !sessionHistoryEnabled) {
cleanSessions(currentSessionId);
einsteinx2 marked this conversation as resolved.
Show resolved Hide resolved
}
cleanSessions(currentSessionId);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,28 +459,6 @@ class ConfigManagerTest {
Assert.assertNull(manager.integrationAttributes)
}

@Test
@Throws(Exception::class)
fun testDefaultIncludeSessionHistory() {
Assert.assertTrue(manager.includeSessionHistory)
}

@Test
@Throws(Exception::class)
fun testIncludeSessionHistoryUpdateFromServer() {
Assert.assertTrue(manager.includeSessionHistory)
val config = JSONObject()
config.put("inhd", false)
manager.updateConfig(config)
Assert.assertFalse(manager.includeSessionHistory)
config.put("inhd", true)
manager.updateConfig(config)
Assert.assertTrue(manager.includeSessionHistory)
config.put("inhd", "false")
manager.updateConfig(config)
Assert.assertFalse(manager.includeSessionHistory)
}

@Test
@Throws(Exception::class)
fun testSaveUserIdentityJson() {
Expand Down
Loading
Loading