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

Check null analytics events #5626

Merged
merged 7 commits into from
Apr 24, 2017
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
@@ -0,0 +1,7 @@
package org.wordpress.android.analytics;

public class AnalyticsException extends Exception {
public AnalyticsException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ public void track(AnalyticsTracker.Stat stat) {

@Override
public void track(AnalyticsTracker.Stat stat, Map<String, ?> properties) {
if (!isValidEvent(stat)) {
return;
}

AnalyticsTrackerMixpanelInstructionsForStat instructions = instructionsForStat(stat);

if (instructions == null) {
Expand Down Expand Up @@ -242,8 +246,10 @@ public void clearAllData() {
}
}

private AnalyticsTrackerMixpanelInstructionsForStat instructionsForStat(
AnalyticsTracker.Stat stat) {
private AnalyticsTrackerMixpanelInstructionsForStat instructionsForStat(AnalyticsTracker.Stat stat) {
if (!isValidEvent(stat)) {
return null;
}
AnalyticsTrackerMixpanelInstructionsForStat instructions;
switch (stat) {
case APPLICATION_OPENED:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> properties) {
return;
}

if (!isValidEvent(stat)) {
return;
}

String eventName = getEventNameForStat(stat);
if (eventName == null) {
AppLog.w(AppLog.T.STATS, "There is NO match for the event " + stat.name() + "stat");
Expand Down Expand Up @@ -156,27 +160,43 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> properties) {
userType = TracksClient.NosaraUserType.ANON;
}

// It seems that we're tracking some events with user = null. Make sure we're catching the error here.
if (user == null) {
try {
throw new AnalyticsException("Trying to track analytics with an null user!");
// TODO add CrashlyticsUtils.logException or track this error in Nosara by using a special test user.
} catch (AnalyticsException e) {
AppLog.e(AppLog.T.STATS, e);
}
return;
}

// create the merged JSON Object of properties
// Properties defined by the user have precedence over the default ones pre-defined at "event level"
final JSONObject propertiesToJSON;
JSONObject propertiesToJSON = null;
if (properties != null && properties.size() > 0) {
propertiesToJSON = new JSONObject(properties);
for (String key : predefinedEventProperties.keySet()) {
try {
if (propertiesToJSON.has(key)) {
AppLog.w(AppLog.T.STATS, "The user has defined a property named: '" + key + "' that will override" +
"the same property pre-defined at event level. This may generate unexpected behavior!!");
AppLog.w(AppLog.T.STATS, "User value: " + propertiesToJSON.get(key).toString() + " - pre-defined value: " +
predefinedEventProperties.get(key).toString());
} else {
propertiesToJSON.put(key, predefinedEventProperties.get(key));
try {
propertiesToJSON = new JSONObject(properties);
for (String key : predefinedEventProperties.keySet()) {
try {
if (propertiesToJSON.has(key)) {
AppLog.w(AppLog.T.STATS, "The user has defined a property named: '" + key + "' that will override" +
"the same property pre-defined at event level. This may generate unexpected behavior!!");
AppLog.w(AppLog.T.STATS, "User value: " + propertiesToJSON.get(key).toString() + " - pre-defined value: " +
predefinedEventProperties.get(key).toString());
} else {
propertiesToJSON.put(key, predefinedEventProperties.get(key));
}
} catch (JSONException e) {
AppLog.e(AppLog.T.STATS, "Error while merging user-defined properties with pre-defined properties", e);
}
} catch (JSONException e) {
AppLog.e(AppLog.T.STATS, "Error while merging user-defined properties with pre-defined properties", e);
}
} catch (NullPointerException e) {
AppLog.e(AppLog.T.STATS, "A property passed to the event " + eventName + " has null key!", e);
}
} else{
}

if (propertiesToJSON == null) {
propertiesToJSON = new JSONObject(predefinedEventProperties);
}

Expand All @@ -187,8 +207,6 @@ public void track(AnalyticsTracker.Stat stat, Map<String, ?> properties) {
}
}



@Override
public void endSession() {
this.flush();
Expand Down Expand Up @@ -251,6 +269,10 @@ public void registerPushNotificationToken(String regId) {
}

public static String getEventNameForStat(AnalyticsTracker.Stat stat) {
if (!isValidEvent(stat)) {
return null;
}

switch (stat) {
case APPLICATION_OPENED:
return "application_opened";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,16 @@ String getWordPressComUserName() {
void setWordPressComUserName(String userName) {
mWpcomUserName = userName;
}

public static boolean isValidEvent(Stat stat) {
if (stat == null) {
try {
throw new AnalyticsException("Trying to track a null stat event!");
} catch (AnalyticsException e) {
AppLog.e(AppLog.T.STATS, e);
}
return false;
}
return true;
}
}