diff --git a/api/src/com/todoroo/andlib/utility/AndroidUtilities.java b/api/src/com/todoroo/andlib/utility/AndroidUtilities.java index 7a35e8315..d939484d8 100644 --- a/api/src/com/todoroo/andlib/utility/AndroidUtilities.java +++ b/api/src/com/todoroo/andlib/utility/AndroidUtilities.java @@ -14,9 +14,11 @@ import java.lang.reflect.Method; import java.math.BigInteger; import java.net.URL; import java.net.URLConnection; +import java.net.URLEncoder; import java.security.MessageDigest; import java.util.Arrays; import java.util.Comparator; +import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -200,6 +202,19 @@ public class AndroidUtilities { return -1; } + /** + * Return index of value in integer array + * @param array array to search + * @param value value to look for + * @return + */ + public static int indexOf(int[] array, int value) { + for (int i = 0; i < array.length; i++) + if (array[i] == value) + return i; + return -1; + } + /** * Serializes a content value into a string */ diff --git a/astrid/build.xml b/astrid/build.xml index 47da849f1..6364021fd 100644 --- a/astrid/build.xml +++ b/astrid/build.xml @@ -137,6 +137,13 @@ + + + diff --git a/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java b/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java index e80382ccd..7b7c4be03 100644 --- a/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java +++ b/astrid/src/com/todoroo/astrid/activity/TaskListActivity.java @@ -26,10 +26,11 @@ import android.widget.PopupWindow.OnDismissListener; import android.widget.TextView; import com.timsu.astrid.R; +import com.todoroo.andlib.service.Autowired; +import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.sql.Functions; import com.todoroo.andlib.sql.QueryTemplate; import com.todoroo.andlib.utility.AndroidUtilities; -import com.todoroo.andlib.utility.DateUtilities; import com.todoroo.andlib.utility.DialogUtilities; import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.TagSettingsActivity; @@ -46,13 +47,13 @@ import com.todoroo.astrid.data.Task; import com.todoroo.astrid.service.StatisticsConstants; import com.todoroo.astrid.service.StatisticsService; import com.todoroo.astrid.service.ThemeService; +import com.todoroo.astrid.service.abtesting.ABTestEventReportingService; import com.todoroo.astrid.tags.TagService; import com.todoroo.astrid.ui.DateChangedAlerts; import com.todoroo.astrid.ui.FragmentPopover; import com.todoroo.astrid.ui.MainMenuPopover; import com.todoroo.astrid.ui.MainMenuPopover.MainMenuListener; import com.todoroo.astrid.ui.TaskListFragmentPager; -import com.todoroo.astrid.utility.AstridPreferences; import com.todoroo.astrid.utility.Constants; import com.todoroo.astrid.utility.Flags; @@ -63,6 +64,8 @@ public class TaskListActivity extends AstridActivity implements MainMenuListener /** token for indicating source of TLA launch */ public static final String TOKEN_SOURCE = "source"; //$NON-NLS-1$ + @Autowired private ABTestEventReportingService abTestEventReportingService; + private View listsNav; private ImageView listsNavDisclosure; private TextView lists; @@ -138,6 +141,7 @@ public class TaskListActivity extends AstridActivity implements MainMenuListener protected void onCreate(Bundle savedInstanceState) { ThemeService.applyTheme(this); super.onCreate(savedInstanceState); + DependencyInjectionService.getInstance().inject(this); if (AndroidUtilities.isTabletSized(this)) { setContentView(R.layout.task_list_wrapper_activity_3pane); @@ -201,36 +205,15 @@ public class TaskListActivity extends AstridActivity implements MainMenuListener trackActivitySource(); } - trackUserRetention(); + // Have to call this here because sometimes StartupService + // isn't called (i.e. if the app was silently alive in the background) + abTestEventReportingService.trackUserRetention(); } private boolean swipeIsEnabled() { return fragmentLayout == LAYOUT_SINGLE && swipeEnabled; } - private void trackUserRetention() { - long firstLaunchTime = Preferences.getLong(AstridPreferences.P_FIRST_LAUNCH, 0); - long now = DateUtilities.now(); - long timeSinceFirst = now - firstLaunchTime; - - if (timeSinceFirst > DateUtilities.ONE_DAY * 3 && !Preferences.getBoolean(StatisticsConstants.APP_OPEN_THREE_DAYS, false)) { - StatisticsService.reportEvent(StatisticsConstants.APP_OPEN_THREE_DAYS); - Preferences.setBoolean(StatisticsConstants.APP_OPEN_THREE_DAYS, true); - } - if (timeSinceFirst > DateUtilities.ONE_WEEK && !Preferences.getBoolean(StatisticsConstants.APP_OPEN_ONE_WEEK, false)) { - StatisticsService.reportEvent(StatisticsConstants.APP_OPEN_ONE_WEEK); - Preferences.setBoolean(StatisticsConstants.APP_OPEN_ONE_WEEK, true); - } - if (timeSinceFirst > 2 * DateUtilities.ONE_WEEK && !Preferences.getBoolean(StatisticsConstants.APP_OPEN_TWO_WEEKS, false)) { - StatisticsService.reportEvent(StatisticsConstants.APP_OPEN_TWO_WEEKS); - Preferences.setBoolean(StatisticsConstants.APP_OPEN_TWO_WEEKS, true); - } - if (timeSinceFirst > 3 * DateUtilities.ONE_WEEK && !Preferences.getBoolean(StatisticsConstants.APP_OPEN_THREE_WEEKS, false)) { - StatisticsService.reportEvent(StatisticsConstants.APP_OPEN_THREE_WEEKS); - Preferences.setBoolean(StatisticsConstants.APP_OPEN_THREE_WEEKS, true); - } - } - @Override public TaskListFragment getTaskListFragment() { if (swipeIsEnabled()) { diff --git a/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java b/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java new file mode 100644 index 000000000..c070f9b41 --- /dev/null +++ b/astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java @@ -0,0 +1,118 @@ +package com.todoroo.astrid.dao; + +import com.todoroo.andlib.data.DatabaseDao; +import com.todoroo.andlib.data.TodorooCursor; +import com.todoroo.andlib.service.Autowired; +import com.todoroo.andlib.service.DependencyInjectionService; +import com.todoroo.andlib.sql.Order; +import com.todoroo.andlib.sql.Query; +import com.todoroo.andlib.utility.AndroidUtilities; +import com.todoroo.andlib.utility.DateUtilities; +import com.todoroo.astrid.data.ABTestEvent; + +public class ABTestEventDao extends DatabaseDao { + + @Autowired + private Database database; + + public ABTestEventDao() { + super(ABTestEvent.class); + DependencyInjectionService.getInstance().inject(this); + setDatabase(database); + } + + /** + * Creates the baseline +0 days event (i.e. the first time the user + * launches the app containing this test) + * @param testName - the name of the test + * @param testVariant - which option was chosen for this user + * @param newUser - are they a new user? + * @param activeUser - are they an activated user? + */ + public void createInitialTestEvent(String testName, String testVariant, + boolean newUser, boolean activeUser) { + ABTestEvent event = new ABTestEvent(); + event.setValue(ABTestEvent.TEST_NAME, testName); + event.setValue(ABTestEvent.TEST_VARIANT, testVariant); + event.setValue(ABTestEvent.NEW_USER, newUser ? 1 : 0); + event.setValue(ABTestEvent.ACTIVATED_USER, activeUser ? 1 : 0); + event.setValue(ABTestEvent.TIME_INTERVAL, 0); + event.setValue(ABTestEvent.DATE_RECORDED, DateUtilities.now()); + + createNew(event); + } + + /** + * Only public for unit testing--don't use unless you really mean it! + * + * Creates data points for the specified test name, creating one data point + * for each time interval that hasn't yet been recorded up to the specified one + * @param testName + * @param timeInterval + */ + public void createTestEventWithTimeInterval(String testName, int timeInterval) { + TodorooCursor existing = query(Query.select(ABTestEvent.PROPERTIES) + .where(ABTestEvent.TEST_NAME.eq(testName)).orderBy(Order.asc(ABTestEvent.TIME_INTERVAL))); + + try { + if (existing.getCount() == 0) + return; + + existing.moveToLast(); + ABTestEvent item = new ABTestEvent(existing); + int lastRecordedTimeIntervalIndex = AndroidUtilities.indexOf( + ABTestEvent.TIME_INTERVALS, item.getValue(ABTestEvent.TIME_INTERVAL)); + + int currentTimeIntervalIndex = AndroidUtilities.indexOf( + ABTestEvent.TIME_INTERVALS, timeInterval); + + if (lastRecordedTimeIntervalIndex < 0 || currentTimeIntervalIndex < 0) + return; + + long now = DateUtilities.now(); + for (int i = lastRecordedTimeIntervalIndex + 1; i <= currentTimeIntervalIndex; i++) { + item.clearValue(ABTestEvent.ID); + item.setValue(ABTestEvent.REPORTED, 0); + item.setValue(ABTestEvent.TIME_INTERVAL, ABTestEvent.TIME_INTERVALS[i]); + item.setValue(ABTestEvent.DATE_RECORDED, now); + createNew(item); + } + } finally { + existing.close(); + } + return; + } + + /** + * For each baseline data point that exists in the database, check the current + * time against the time that baseline was recorded and report the appropriate + * +n days events. Called on startup. + */ + public void createRelativeDateEvents() { + TodorooCursor allEvents = query(Query.select(ABTestEvent.TEST_NAME, ABTestEvent.DATE_RECORDED) + .where(ABTestEvent.TIME_INTERVAL.eq(0))); + + try { + long now = DateUtilities.now(); + ABTestEvent event = new ABTestEvent(); + for (allEvents.moveToFirst(); !allEvents.isAfterLast(); allEvents.moveToNext()) { + event.readFromCursor(allEvents); + long baseTime = event.getValue(ABTestEvent.DATE_RECORDED); + long timeSinceBase = now - baseTime; + + String testName = event.getValue(ABTestEvent.TEST_NAME); + int timeInterval = -1; + long days = timeSinceBase / DateUtilities.ONE_DAY; + for(int i = 0; i < ABTestEvent.TIME_INTERVALS.length; i++) + if(days >= ABTestEvent.TIME_INTERVALS[i]) + timeInterval = ABTestEvent.TIME_INTERVALS[i]; + + if (timeInterval > 0) + createTestEventWithTimeInterval(testName, timeInterval); + } + } finally { + allEvents.close(); + } + } + +} diff --git a/astrid/src/com/todoroo/astrid/dao/Database.java b/astrid/src/com/todoroo/astrid/dao/Database.java index 71915080f..f8d4e2836 100644 --- a/astrid/src/com/todoroo/astrid/dao/Database.java +++ b/astrid/src/com/todoroo/astrid/dao/Database.java @@ -13,6 +13,7 @@ import com.todoroo.andlib.data.AbstractModel; import com.todoroo.andlib.data.Property; import com.todoroo.andlib.data.Table; import com.todoroo.andlib.service.ContextManager; +import com.todoroo.astrid.data.ABTestEvent; import com.todoroo.astrid.data.Metadata; import com.todoroo.astrid.data.StoreObject; import com.todoroo.astrid.data.TagData; @@ -38,7 +39,7 @@ public class Database extends AbstractDatabase { * Database version number. This variable must be updated when database * tables are updated, as it determines whether a database needs updating. */ - public static final int VERSION = 23; + public static final int VERSION = 24; /** * Database name (must be unique) @@ -55,7 +56,8 @@ public class Database extends AbstractDatabase { StoreObject.TABLE, TagData.TABLE, Update.TABLE, - User.TABLE + User.TABLE, + ABTestEvent.TABLE, }; // --- listeners @@ -310,6 +312,12 @@ public class Database extends AbstractDatabase { Log.e("astrid", "db-upgrade-" + oldVersion + "-" + newVersion, e); } + case 23: try { + database.execSQL(createTableSql(visitor, ABTestEvent.TABLE.name, ABTestEvent.PROPERTIES)); + } catch (SQLiteException e) { + Log.e("astrid", "db-upgrade-" + oldVersion + "-" + newVersion, e); + } + return true; } diff --git a/astrid/src/com/todoroo/astrid/data/ABTestEvent.java b/astrid/src/com/todoroo/astrid/data/ABTestEvent.java new file mode 100644 index 000000000..145fcc511 --- /dev/null +++ b/astrid/src/com/todoroo/astrid/data/ABTestEvent.java @@ -0,0 +1,131 @@ +package com.todoroo.astrid.data; + +import android.content.ContentValues; +import android.net.Uri; + +import com.todoroo.andlib.data.AbstractModel; +import com.todoroo.andlib.data.Property; +import com.todoroo.andlib.data.Property.IntegerProperty; +import com.todoroo.andlib.data.Property.LongProperty; +import com.todoroo.andlib.data.Property.StringProperty; +import com.todoroo.andlib.data.Table; +import com.todoroo.andlib.data.TodorooCursor; +import com.todoroo.astrid.api.AstridApiConstants; + +/** + * Model for data points used for tracking user retention in AB tests + * @author Sam + * + */ +@SuppressWarnings("nls") +public class ABTestEvent extends AbstractModel { + + public static final int[] TIME_INTERVALS = { 0, 3, 7, 14, 21 }; + + // --- table and uri + + /** table for this model */ + public static final Table TABLE = new Table("abtestevent", ABTestEvent.class); + + /** content uri for this model */ + public static final Uri CONTENT_URI = Uri.parse("content://" + AstridApiConstants.PACKAGE + "/" + + TABLE.name); + + + // --- properties + + /** ID */ + public static final LongProperty ID = new LongProperty( + TABLE, ID_PROPERTY_NAME); + + /** Name of the test -- one of the constant test keys defined in ABOptions */ + public static final StringProperty TEST_NAME = new StringProperty( + TABLE, "testName"); + + /** + * Which variant (option) was chosen for this user -- + * one of the constants in the corresponding descriptions array in ABOptions + */ + public static final StringProperty TEST_VARIANT = new StringProperty( + TABLE, "testVariant"); + + /** + * Indicates if the user should be considered a new user for the purposes + * of this test. + * Should be 0 if no, 1 if yes + */ + public static final IntegerProperty NEW_USER = new IntegerProperty( + TABLE, "newUser"); // 0 if no, 1 if yes + + /** + * Indicates if the user was "activated" at the time of recording this data + * point. + * Should be 0 if no, 1 if yes + * Activated: 3 tasks created, one completed + */ + public static final IntegerProperty ACTIVATED_USER = new IntegerProperty( + TABLE, "activatedUser"); + + /** + * Which time interval event this data point corresponds to. + * Should be one of the time interval constants defined int the + * above array. + */ + public static final IntegerProperty TIME_INTERVAL = new IntegerProperty( + TABLE, "timeInterval"); // one of the constants defined above + + /** The actual date on which this data point was recorded. */ + public static final LongProperty DATE_RECORDED = new LongProperty( + TABLE, "dateRecorded"); + + /** Whether or not this data point has been reported to the server */ + public static final IntegerProperty REPORTED = new IntegerProperty( + TABLE, "reported"); // 0 if not yet reported, 1 if reported successfully + + /** List of all properties for this model */ + public static final Property[] PROPERTIES = generateProperties(ABTestEvent.class); + + + + private static final ContentValues defaultValues = new ContentValues(); + + static { + // initialize with default values + defaultValues.put(REPORTED.name, 0); + } + + @Override + public ContentValues getDefaultValues() { + return defaultValues; + } + + // --- data access boilerplate + + public ABTestEvent() { + super(); + } + + public ABTestEvent(TodorooCursor cursor) { + this(); + readPropertiesFromCursor(cursor); + } + + public void readFromCursor(TodorooCursor cursor) { + super.readPropertiesFromCursor(cursor); + } + + @Override + public long getId() { + return getIdHelper(ID); + } + + // --- parcelable helpers + + public static final Creator CREATOR = new ModelCreator(ABTestEvent.class); + + @Override + protected Creator getCreator() { + return CREATOR; + } + +} diff --git a/astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java b/astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java index 403823427..919abb57b 100644 --- a/astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java +++ b/astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java @@ -12,6 +12,7 @@ import com.todoroo.astrid.actfm.sync.ActFmDataService; import com.todoroo.astrid.actfm.sync.ActFmInvoker; import com.todoroo.astrid.actfm.sync.ActFmPreferenceService; import com.todoroo.astrid.actfm.sync.ActFmSyncService; +import com.todoroo.astrid.dao.ABTestEventDao; import com.todoroo.astrid.dao.Database; import com.todoroo.astrid.dao.MetadataDao; import com.todoroo.astrid.dao.StoreObjectDao; @@ -25,7 +26,9 @@ import com.todoroo.astrid.gtasks.GtasksPreferenceService; import com.todoroo.astrid.gtasks.GtasksTaskListUpdater; import com.todoroo.astrid.gtasks.sync.GtasksSyncService; import com.todoroo.astrid.service.abtesting.ABChooser; -import com.todoroo.astrid.service.abtesting.ABOptions; +import com.todoroo.astrid.service.abtesting.ABTestEventReportingService; +import com.todoroo.astrid.service.abtesting.ABTestInvoker; +import com.todoroo.astrid.service.abtesting.ABTests; import com.todoroo.astrid.service.abtesting.FeatureFlipper; import com.todoroo.astrid.tags.TagService; import com.todoroo.astrid.utility.Constants; @@ -99,7 +102,10 @@ public class AstridDependencyInjector extends AbstractDependencyInjector { // AB testing injectables.put("abChooser", ABChooser.class); - injectables.put("abOptions", new ABOptions()); + injectables.put("abTests", new ABTests()); + injectables.put("abTestEventDao", ABTestEventDao.class); + injectables.put("abTestEventReportingService", ABTestEventReportingService.class); + injectables.put("abTestInvoker", ABTestInvoker.class); injectables.put("featureFlipper", FeatureFlipper.class); // com.todoroo.astrid.tags diff --git a/astrid/src/com/todoroo/astrid/service/StartupService.java b/astrid/src/com/todoroo/astrid/service/StartupService.java index 78c8ada1a..77c1eae61 100644 --- a/astrid/src/com/todoroo/astrid/service/StartupService.java +++ b/astrid/src/com/todoroo/astrid/service/StartupService.java @@ -42,7 +42,6 @@ import com.todoroo.astrid.opencrx.OpencrxCoreUtils; import com.todoroo.astrid.producteev.ProducteevUtilities; import com.todoroo.astrid.reminders.ReminderStartupReceiver; import com.todoroo.astrid.service.abtesting.ABChooser; -import com.todoroo.astrid.service.abtesting.ABOptions; import com.todoroo.astrid.service.abtesting.FeatureFlipper; import com.todoroo.astrid.utility.AstridPreferences; import com.todoroo.astrid.utility.Constants; @@ -176,8 +175,12 @@ public class StartupService { upgradeService.performSecondaryUpgrade(context); - // perform startup activities in a background thread final int finalLatestVersion = latestSetVersion; + + // For any uninitialized ab test, make sure an option is chosen + abChooser.makeChoicesForAllTests(latestSetVersion == 0, taskService.getUserActivationStatus()); + + // perform startup activities in a background thread new Thread(new Runnable() { public void run() { // start widget updating alarm @@ -213,8 +216,6 @@ public class StartupService { } }).start(); - abChooser.getChoiceForOption(ABOptions.AB_OPTION_SWIPE_ENABLED_KEY); - abChooser.getChoiceForOption(ABOptions.AB_OPTION_CONTACTS_PICKER_ENABLED); AstridPreferences.setPreferenceDefaults(); // check for task killers diff --git a/astrid/src/com/todoroo/astrid/service/StatisticsConstants.java b/astrid/src/com/todoroo/astrid/service/StatisticsConstants.java index e442fcc98..e8422c244 100644 --- a/astrid/src/com/todoroo/astrid/service/StatisticsConstants.java +++ b/astrid/src/com/todoroo/astrid/service/StatisticsConstants.java @@ -70,11 +70,4 @@ public class StatisticsConstants { public static final String TASK_ONE_WEEK = "task-created-one-week"; public static final String TASK_TWO_WEEKS = "task-created-two-weeks"; public static final String TASK_THREE_WEEKS = "task-created-three-weeks"; - - public static final String APP_OPEN_THREE_DAYS = "app-open-three-days"; - public static final String APP_OPEN_ONE_WEEK = "app-open-one-week"; - public static final String APP_OPEN_TWO_WEEKS = "app-open-two-weeks"; - public static final String APP_OPEN_THREE_WEEKS = "app-open-three-weeks"; - - } diff --git a/astrid/src/com/todoroo/astrid/service/StatisticsService.java b/astrid/src/com/todoroo/astrid/service/StatisticsService.java index d5db178dd..de519fea6 100644 --- a/astrid/src/com/todoroo/astrid/service/StatisticsService.java +++ b/astrid/src/com/todoroo/astrid/service/StatisticsService.java @@ -11,21 +11,12 @@ import android.content.Context; import com.localytics.android.LocalyticsSession; import com.timsu.astrid.R; -import com.todoroo.andlib.service.Autowired; -import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.utility.Preferences; -import com.todoroo.astrid.service.abtesting.ABOptions; import com.todoroo.astrid.utility.Constants; public class StatisticsService { private static LocalyticsSession localyticsSession; - private static class StatisticsDependencies { - @Autowired ABOptions abOptions; - public StatisticsDependencies() { - DependencyInjectionService.getInstance().inject(this); - } - } /** * Indicate session started @@ -93,15 +84,12 @@ public class StatisticsService { return; if(localyticsSession != null) { - String[] abAttributes = new StatisticsDependencies().abOptions.getLocalyticsAttributeArrayForEvent(event); - if(attributes.length > 0 || abAttributes.length > 0) { + if(attributes.length > 0) { HashMap attrMap = new HashMap(); for(int i = 1; i < attributes.length; i += 2) { if(attributes[i] != null) attrMap.put(attributes[i-1], attributes[i]); } - for (int i = 1; i < abAttributes.length; i += 2) - attrMap.put(abAttributes[i-1], abAttributes[i]); localyticsSession.tagEvent(event, attrMap); } else localyticsSession.tagEvent(event); diff --git a/astrid/src/com/todoroo/astrid/service/TaskService.java b/astrid/src/com/todoroo/astrid/service/TaskService.java index e44a72e37..1e74be88f 100644 --- a/astrid/src/com/todoroo/astrid/service/TaskService.java +++ b/astrid/src/com/todoroo/astrid/service/TaskService.java @@ -17,6 +17,7 @@ import com.todoroo.andlib.sql.Functions; import com.todoroo.andlib.sql.Query; import com.todoroo.andlib.utility.AndroidUtilities; import com.todoroo.andlib.utility.DateUtilities; +import com.todoroo.andlib.utility.Preferences; import com.todoroo.astrid.actfm.sync.ActFmPreferenceService; import com.todoroo.astrid.api.Filter; import com.todoroo.astrid.api.PermaSql; @@ -54,6 +55,10 @@ public class TaskService { public static final String TRANS_EDIT_SAVE = "task-edit-save"; //$NON-NLS-1$ public static final String TRANS_REPEAT_COMPLETE = "repeat-complete"; //$NON-NLS-1$ + + private static final int TOTAL_TASKS_FOR_ACTIVATION = 3; + private static final int COMPLETED_TASKS_FOR_ACTIVATION = 1; + private static final String PREF_USER_ACTVATED = "user-activated"; //$NON-NLS-1$ @Autowired private TaskDao taskDao; @@ -267,6 +272,30 @@ public class TaskService { return taskDao.query(Query.select(properties).withQueryTemplate(sql)); } + public boolean getUserActivationStatus() { + if (Preferences.getBoolean(PREF_USER_ACTVATED, false)) + return true; + + TodorooCursor all = query(Query.select(Task.ID)); + try { + if (all.getCount() < TOTAL_TASKS_FOR_ACTIVATION) + return false; + + TodorooCursor completed = query(Query.select(Task.ID).where(TaskCriteria.completed())); + try { + if (completed.getCount() < COMPLETED_TASKS_FOR_ACTIVATION) + return false; + } finally { + completed.close(); + } + } finally { + all.close(); + } + + Preferences.setBoolean(PREF_USER_ACTVATED, true); + return true; + } + /** * @param query * @return how many tasks are matched by this query diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java index 9cdf6edd0..00ec3350a 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABChooser.java @@ -1,11 +1,12 @@ package com.todoroo.astrid.service.abtesting; import java.util.Random; +import java.util.Set; import com.todoroo.andlib.service.Autowired; import com.todoroo.andlib.service.DependencyInjectionService; import com.todoroo.andlib.utility.Preferences; -import com.todoroo.astrid.service.StatisticsService; +import com.todoroo.astrid.dao.ABTestEventDao; /** * Helper class to facilitate A/B testing by randomly choosing an option @@ -18,7 +19,10 @@ public class ABChooser { public static final int NO_OPTION = -1; @Autowired - private ABOptions abOptions; + private ABTests abTests; + + @Autowired + private ABTestEventDao abTestEventDao; private final Random random; @@ -28,48 +32,57 @@ public class ABChooser { } /** - * Retrieves the choice for the specified feature if already made, - * or chooses one randomly from the distribution of feature probabilities - * if not. - * - * The statistics session needs to be open here in order to collect statistics + * Iterates through the list of all available tests and makes sure that a choice + * is made for each of them + */ + public void makeChoicesForAllTests(boolean newUser, boolean activatedUser) { + Set tests = abTests.getAllTestKeys(); + for (String test : tests) { + makeChoiceForTest(test, newUser, activatedUser); + } + } + + /** + * If a choice/variant has not yet been selected for the specified test, + * make one and create the initial +0 analytics data point * - * @param optionKey - the preference key string of the option (defined in ABOptions) - * @return + * @param testKey - the preference key string of the option (defined in ABTests) */ - public int getChoiceForOption(String optionKey) { - int pref = readChoiceForOption(optionKey); - if (pref > NO_OPTION) return pref; + private void makeChoiceForTest(String testKey, boolean newUser, boolean activatedUser) { + int pref = readChoiceForTest(testKey); + if (pref > NO_OPTION) return; int chosen = NO_OPTION; - if (abOptions.isValidKey(optionKey)) { - int[] optionProbs = abOptions.getProbsForKey(optionKey); + if (abTests.isValidTestKey(testKey)) { + int[] optionProbs = abTests.getProbsForTestKey(testKey, newUser); + String[] optionDescriptions = abTests.getDescriptionsForTestKey(testKey); chosen = chooseOption(optionProbs); - setChoiceForOption(optionKey, chosen); + setChoiceForTest(testKey, chosen); - StatisticsService.reportEvent(abOptions.getDescriptionForOption(optionKey, chosen)); // Session should be open + String desc = optionDescriptions[chosen]; + abTestEventDao.createInitialTestEvent(testKey, desc, newUser, activatedUser); } - return chosen; + return; } /** * Returns the chosen option if set or NO_OPTION if unset - * @param optionKey + * @param testKey * @return */ - public static int readChoiceForOption(String optionKey) { - return Preferences.getInt(optionKey, NO_OPTION); + public static int readChoiceForTest(String testKey) { + return Preferences.getInt(testKey, NO_OPTION); } /** * Changes the choice of an A/B feature in the preferences. Useful for * the feature flipper (can manually override previous choice) - * @param optionKey + * @param testKey * @param choiceIndex */ - public void setChoiceForOption(String optionKey, int choiceIndex) { - if (abOptions.isValidKey(optionKey)) - Preferences.setInt(optionKey, choiceIndex); + public void setChoiceForTest(String testKey, int choiceIndex) { + if (abTests.isValidTestKey(testKey)) + Preferences.setInt(testKey, choiceIndex); } /* diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABOptions.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABOptions.java deleted file mode 100644 index 622e6dff9..000000000 --- a/astrid/src/com/todoroo/astrid/service/abtesting/ABOptions.java +++ /dev/null @@ -1,194 +0,0 @@ -package com.todoroo.astrid.service.abtesting; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; - -import com.todoroo.astrid.service.StatisticsConstants; - -/** - * Helper class to define options with their probabilities and descriptions - * @author Sam Bosley - * - */ -public class ABOptions { - - public ABOptions() { - bundles = new HashMap(); - events = new HashMap>(); - initialize(); - } - - /** - * Gets the integer array of weighted probabilities for an option key - * @param key - * @return - */ - public synchronized int[] getProbsForKey(String key) { - if (bundles.containsKey(key)) { - ABOptionBundle bundle = bundles.get(key); - return bundle.weightedProbs; - } else { - return null; - } - } - - /** - * Updates the weighted probability array for a given key. Returns true - * on success, false if they key doesn't exist or if the array is the wrong - * length. - * @param key - * @param newProbs - * @return - */ - public synchronized boolean setProbsForKey(String key, int[] newProbs) { - if (bundles.containsKey(newProbs)) { - ABOptionBundle bundle = bundles.get(key); - if (bundle.descriptions == null || newProbs.length == bundle.descriptions.length) { - bundle.weightedProbs = newProbs; - return true; - } - } - return false; - } - - /** - * Gets the string array of option descriptions for an option key - * @param key - * @return - */ - public String[] getDescriptionsForKey(String key) { - if (bundles.containsKey(key)) { - ABOptionBundle bundle = bundles.get(key); - return bundle.descriptions; - } else { - return null; - } - } - - /** - * Returns the description for a particular choice of the given option - * @param key - * @param optionIndex - * @return - */ - public String getDescriptionForOption(String key, int optionIndex) { - if (bundles.containsKey(key)) { - ABOptionBundle bundle = bundles.get(key); - if (bundle.descriptions != null && optionIndex < bundle.descriptions.length) { - return bundle.descriptions[optionIndex]; - } - } - return null; - } - - /** - * Maps keys (i.e. preference key identifiers) to feature weights and descriptions - */ - private final HashMap bundles; - private final HashMap> events; // maps events to lists of interested keys - - private static class ABOptionBundle { - public int[] weightedProbs; - public String[] descriptions; - - public ABOptionBundle(int[] weightedProbs, String[] descriptions) { - this.weightedProbs = weightedProbs; - this.descriptions = descriptions; - } - } - - public boolean isValidKey(String key) { - return bundles.containsKey(key); - } - - /** - * Gets a localytics attribute array for the specified event. - * @param event - * @return - */ - public String[] getLocalyticsAttributeArrayForEvent(String event) { - ArrayList attributes = new ArrayList(); - List interestedKeys = events.get(event); - if (interestedKeys != null) - for (String key : interestedKeys) { - // Get choice if exists and add to array - if (isValidKey(key)) { - ABOptionBundle bundle = bundles.get(key); - int choice = ABChooser.readChoiceForOption(key); - if (choice != ABChooser.NO_OPTION && - bundle.descriptions != null && choice < bundle.descriptions.length) { - attributes.add(key); - attributes.add(getDescriptionForOption(key, choice)); - } - } - } - return attributes.toArray(new String[attributes.size()]); - } - - - /** - * A/B testing options are defined below according to the following spec: - * - * @param optionKey = "" - * --This key is used to identify the option in the application and in the preferences - * - * @param probs = { int, int, ... } - * --The different choices in an option correspond to an index in the probability array. - * Probabilities are expressed as integers to easily define relative weights. For example, - * the array { 1, 2 } would mean option 0 would happen one time for every two occurrences of option 1 - * - * (optional) - * @param descriptions = { "...", "...", ... } - * --A string description of each option. Useful for tagging events. The index of - * each description should correspond to the events location in the probability array - * (i.e. the arrays should be the same length if this one exists) - * - * (optional) - * @param relevantEvents = { "...", "...", ... } - * --An arbitrary length list of relevant localytics events. When events are - * tagged from StatisticsService, they will be appended with attributes - * that have that event in this array - */ - public void addOption(String optionKey, int[] probs, String[] descriptions, String[] relevantEvents) { - ABOptionBundle bundle = new ABOptionBundle(probs, descriptions); - bundles.put(optionKey, bundle); - - if (relevantEvents != null) { - for (String event : relevantEvents) { - List interestedKeys = events.get(event); - if (interestedKeys == null) { - interestedKeys = new ArrayList(); - events.put(event, interestedKeys); - } - interestedKeys.add(optionKey); - } - } - } - - private void initialize() { // Set up - //Calls to addOption go here - addOption(AB_OPTION_SWIPE_ENABLED_KEY, AB_OPTION_SWIPE_ENABLED_PROBS, AB_OPTION_SWIPE_ENABLED_DESC, AB_OPTION_SWIPE_ENABLED_EVENTS); - addOption(AB_OPTION_CONTACTS_PICKER_ENABLED, AB_OPTION_CONTACTS_ENABLED_PROBS, AB_OPTION_CONTACTS_ENABLED_DESC, AB_OPTION_CONTACTS_ENABLED_EVENTS); - } - - public static final String AB_OPTION_SWIPE_ENABLED_KEY = "swipeEnabled"; //$NON-NLS-1$ - private static final int[] AB_OPTION_SWIPE_ENABLED_PROBS = { 1, 1 }; - private static final String[] AB_OPTION_SWIPE_ENABLED_DESC = { "swipe-lists-disabled", "swipe-lists-enabled" }; //$NON-NLS-1$//$NON-NLS-2$ - private static final String[] AB_OPTION_SWIPE_ENABLED_EVENTS = { StatisticsConstants.APP_OPEN_THREE_DAYS, - StatisticsConstants.APP_OPEN_ONE_WEEK, - StatisticsConstants.APP_OPEN_TWO_WEEKS, - StatisticsConstants.APP_OPEN_THREE_WEEKS }; - - public static final String AB_OPTION_CONTACTS_PICKER_ENABLED = "contactsEnabled"; //$NON-NLS-1$ - private static final int[] AB_OPTION_CONTACTS_ENABLED_PROBS = { 1, 1 }; - private static final String[] AB_OPTION_CONTACTS_ENABLED_DESC = { "contacts-disabled", "contacts-enabled" }; //$NON-NLS-1$//$NON-NLS-2$ - private static final String[] AB_OPTION_CONTACTS_ENABLED_EVENTS = { StatisticsConstants.APP_OPEN_THREE_DAYS, - StatisticsConstants.APP_OPEN_ONE_WEEK, - StatisticsConstants.APP_OPEN_TWO_WEEKS, - StatisticsConstants.APP_OPEN_THREE_WEEKS, - StatisticsConstants.TASK_ASSIGNED_EMAIL, - StatisticsConstants.TASK_ASSIGNED_PICKER }; - - -} diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java new file mode 100644 index 000000000..bf4b91508 --- /dev/null +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java @@ -0,0 +1,140 @@ +package com.todoroo.astrid.service.abtesting; + +import java.io.IOException; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + +import android.util.Log; + +import com.todoroo.andlib.data.TodorooCursor; +import com.todoroo.andlib.service.Autowired; +import com.todoroo.andlib.service.DependencyInjectionService; +import com.todoroo.andlib.sql.Order; +import com.todoroo.andlib.sql.Query; +import com.todoroo.astrid.dao.ABTestEventDao; +import com.todoroo.astrid.data.ABTestEvent; +import com.todoroo.astrid.service.StatisticsService; + +/** + * Service to manage the reporting of launch events for AB testing. + * On startup, queries the ABTestEvent database for unreported data + * points, merges them into the expected JSONArray format, and + * pushes them to the server. + * @author Sam + * + */ +@SuppressWarnings("nls") +public final class ABTestEventReportingService { + + private static final String KEY_TEST = "test"; + private static final String KEY_VARIANT = "variant"; + private static final String KEY_NEW_USER = "new"; + private static final String KEY_ACTIVE_USER = "activated"; + private static final String KEY_DAYS = "days"; + private static final String KEY_DATE = "date"; + + @Autowired + private ABTestEventDao abTestEventDao; + + @Autowired + private ABTestInvoker abTestInvoker; + + public ABTestEventReportingService() { + DependencyInjectionService.getInstance().inject(this); + } + + /** + * Called on startup from TaskListActivity. Creates any +n days + * launch events that need to be recorded, and pushes all unreported + * data to the server. + */ + public void trackUserRetention() { + new Thread(new Runnable() { + @Override + public void run() { + abTestEventDao.createRelativeDateEvents(); + pushAllUnreportedABTestEvents(); + } + }).start(); + } + + private void pushAllUnreportedABTestEvents() { + if (StatisticsService.dontCollectStatistics()) + return; + final TodorooCursor unreported = abTestEventDao.query(Query.select(ABTestEvent.PROPERTIES) + .where(ABTestEvent.REPORTED.eq(0)) + .orderBy(Order.asc(ABTestEvent.TEST_NAME)) + .orderBy(Order.asc(ABTestEvent.TIME_INTERVAL))); + if (unreported.getCount() > 0) { + new Thread(new Runnable() { + @Override + public void run() { + try { + JSONArray payload = jsonArrayFromABTestEvents(unreported); + abTestInvoker.post(payload); + ABTestEvent model = new ABTestEvent(); + for (unreported.moveToFirst(); !unreported.isAfterLast(); unreported.moveToNext()) { + model.readFromCursor(unreported); + model.setValue(ABTestEvent.REPORTED, 1); + abTestEventDao.saveExisting(model); + } + } catch (JSONException e) { + handleException(e); + } catch (IOException e) { + handleException(e); + } finally { + unreported.close(); + } + } + }).start(); + } + } + + private void handleException(Exception e) { + Log.e("analytics", "analytics-error", e); + } + + private static JSONObject jsonFromABTestEvent(ABTestEvent model) throws JSONException { + JSONObject payload = new JSONObject(); + payload.put(KEY_TEST, model.getValue(ABTestEvent.TEST_NAME)); + payload.put(KEY_VARIANT, model.getValue(ABTestEvent.TEST_VARIANT)); + payload.put(KEY_NEW_USER, model.getValue(ABTestEvent.NEW_USER) > 0 ? true : false); + payload.put(KEY_ACTIVE_USER, model.getValue(ABTestEvent.ACTIVATED_USER) > 0 ? true : false); + payload.put(KEY_DAYS, new JSONArray().put(model.getValue(ABTestEvent.TIME_INTERVAL))); + + long date = model.getValue(ABTestEvent.DATE_RECORDED) / 1000L; + payload.put(KEY_DATE, date); + + return payload; + } + + private static JSONArray jsonArrayFromABTestEvents(TodorooCursor events) throws JSONException { + JSONArray result = new JSONArray(); + + String lastTestKey = ""; + JSONObject testAcc = null; + ABTestEvent model = new ABTestEvent(); + for (events.moveToFirst(); !events.isAfterLast(); events.moveToNext()) { + model.readFromCursor(events); + if (!model.getValue(ABTestEvent.TEST_NAME).equals(lastTestKey)) { + if (testAcc != null) + result.put(testAcc); + testAcc = jsonFromABTestEvent(model); + lastTestKey = model.getValue(ABTestEvent.TEST_NAME); + } else { + int interval = model.getValue(ABTestEvent.TIME_INTERVAL); + if (testAcc != null) { // this should never happen, just stopping the compiler from complaining + JSONArray daysArray = testAcc.getJSONArray(KEY_DAYS); + daysArray.put(interval); + } + } + + } + if (testAcc != null) + result.put(testAcc); + return result; + } + +} diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java new file mode 100644 index 000000000..2334dd5a6 --- /dev/null +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java @@ -0,0 +1,97 @@ +package com.todoroo.astrid.service.abtesting; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.ArrayList; +import java.util.List; + +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.http.HttpEntity; +import org.apache.http.NameValuePair; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.protocol.HTTP; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; + +import com.todoroo.andlib.service.Autowired; +import com.todoroo.andlib.service.DependencyInjectionService; +import com.todoroo.andlib.service.RestClient; + +/** + * Invoker for communicating with the Astrid Analytics server + * @author Sam + * + */ +@SuppressWarnings("nls") +public class ABTestInvoker { + + /** NOTE: these values are development values and will not work on production */ + private static final String URL = "http://analytics.astrid.com/api/1/retention"; + private static final String API_KEY = "ryyubd"; + private static final String API_SECRET = "q9ef3i"; + + @Autowired private RestClient restClient; + + public ABTestInvoker() { + DependencyInjectionService.getInstance().inject(this); + } + + /** + * Posts the payload to the analytics server + * @param payload - JSONArray of data points. Created by the + * helper method in ABTestReportingService + * @return + * @throws IOException + */ + public JSONObject post(JSONArray payload) throws IOException { + try { + HttpEntity postData = createPostData(payload); + String response = restClient.post(URL, postData); + JSONObject object = new JSONObject(response); + if (object.getString("status").equals("error")) { + throw new IOException("Error reporting ABTestEvent: " + + object.optString("message")); + } + return object; + } catch (JSONException e) { + throw new IOException(e.getMessage()); + } + + } + + /** + * Converts the JSONArray payload into an HTTPEntity suitable for + * POSTing. + * @param payload + * @return + */ + private HttpEntity createPostData(JSONArray payload) throws IOException { + List params = new ArrayList(); + params.add(new BasicNameValuePair("apikey", API_KEY)); + params.add(new BasicNameValuePair("payload", payload.toString())); + + StringBuilder sigBuilder = new StringBuilder(); + for(NameValuePair entry : params) { + if(entry.getValue() == null) + continue; + + String key = entry.getName(); + String value = entry.getValue(); + + sigBuilder.append(key).append(value); + } + + sigBuilder.append(API_SECRET); + String signature = DigestUtils.md5Hex(sigBuilder.toString()); + params.add(new BasicNameValuePair("sig", signature)); + + try { + return new UrlEncodedFormEntity(params, HTTP.UTF_8); + } catch (UnsupportedEncodingException e) { + throw new IOException("Unsupported URL encoding"); + } + } + +} diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java b/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java new file mode 100644 index 000000000..66b797819 --- /dev/null +++ b/astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java @@ -0,0 +1,128 @@ +package com.todoroo.astrid.service.abtesting; + +import java.util.HashMap; +import java.util.Set; + +/** + * Helper class to define options with their probabilities and descriptions + * @author Sam Bosley + * + */ +public class ABTests { + + public ABTests() { + bundles = new HashMap(); + initialize(); + } + + /** + * Gets the integer array of weighted probabilities for an option key + * @param key + * @return + */ + public synchronized int[] getProbsForTestKey(String key, boolean newUser) { + if (bundles.containsKey(key)) { + ABTestBundle bundle = bundles.get(key); + if (newUser) + return bundle.newUserProbs; + else + return bundle.existingUserProbs; + } else { + return null; + } + } + + /** + * Gets the string array of option descriptions for an option key + * @param key + * @return + */ + public String[] getDescriptionsForTestKey(String key) { + if (bundles.containsKey(key)) { + ABTestBundle bundle = bundles.get(key); + return bundle.descriptions; + } else { + return null; + } + } + + /** + * Returns the description for a particular choice of the given option + * @param testKey + * @param optionIndex + * @return + */ + public String getDescriptionForTestOption(String testKey, int optionIndex) { + if (bundles.containsKey(testKey)) { + ABTestBundle bundle = bundles.get(testKey); + if (bundle.descriptions != null && optionIndex < bundle.descriptions.length) { + return bundle.descriptions[optionIndex]; + } + } + return null; + } + + public Set getAllTestKeys() { + return bundles.keySet(); + } + + /** + * Maps keys (i.e. preference key identifiers) to feature weights and descriptions + */ + private final HashMap bundles; + + private static class ABTestBundle { + protected final int[] newUserProbs; + protected final int[] existingUserProbs; + protected final String[] descriptions; + + protected ABTestBundle(int[] newUserProbs, int[] existingUserProbs, String[] descriptions) { + this.newUserProbs = newUserProbs; + this.existingUserProbs = existingUserProbs; + this.descriptions = descriptions; + } + } + + public boolean isValidTestKey(String key) { + return bundles.containsKey(key); + } + + /** + * A/B testing options are defined below according to the following spec: + * + * @param testKey = "" + * --This key is used to identify the option in the application and in the preferences + * + * @param newUserProbs = { int, int, ... } + * @param existingUserProbs = { int, int, ... } + * --The different choices in an option correspond to an index in the probability array. + * Probabilities are expressed as integers to easily define relative weights. For example, + * the array { 1, 2 } would mean option 0 would happen one time for every two occurrences of option 1 + * + * The first array is used for new users and the second is used for existing/upgrading users, + * allowing us to specify different distributions for each group. + * + * (optional) + * @param descriptions = { "...", "...", ... } + * --A string description of each option. Useful for tagging events. The index of + * each description should correspond to the events location in the probability array + * (i.e. the arrays should be the same length if this one exists) + * + */ + public void addTest(String testKey, int[] newUserProbs, int[] existingUserProbs, String[] descriptions) { + ABTestBundle bundle = new ABTestBundle(newUserProbs, existingUserProbs, descriptions); + bundles.put(testKey, bundle); + } + + private void initialize() { // Set up + //Calls to addTest go here + addTest(AB_TEST_SWIPE_ENABLED_KEY, new int[] { 1, 1 }, + new int[] { 1, 1 }, new String[] { "swipe-lists-disabled", "swipe-lists-enabled" }); //$NON-NLS-1$//$NON-NLS-2$ + addTest(AB_TEST_CONTACTS_PICKER_ENABLED, new int[] { 1, 1 }, + new int[] { 1, 1 }, new String[] { "contacts-disabled", "contacts-enabled" }); //$NON-NLS-1$//$NON-NLS-2$ + } + + public static final String AB_TEST_SWIPE_ENABLED_KEY = "swipeEnabled"; //$NON-NLS-1$ + public static final String AB_TEST_CONTACTS_PICKER_ENABLED = "contactsEnabled"; //$NON-NLS-1$ + +} diff --git a/astrid/src/com/todoroo/astrid/service/abtesting/FeatureFlipper.java b/astrid/src/com/todoroo/astrid/service/abtesting/FeatureFlipper.java index d95457d78..2d985e2e6 100644 --- a/astrid/src/com/todoroo/astrid/service/abtesting/FeatureFlipper.java +++ b/astrid/src/com/todoroo/astrid/service/abtesting/FeatureFlipper.java @@ -29,7 +29,7 @@ public class FeatureFlipper { @Autowired private RestClient restClient; @Autowired private ABChooser abChooser; - @Autowired private ABOptions abOptions; + @Autowired private ABTests abTests; public FeatureFlipper() { DependencyInjectionService.getInstance().inject(this); @@ -52,16 +52,7 @@ public class FeatureFlipper { if (settings.has(KEY_SET_OPTION)) { int option = settings.getInt(KEY_SET_OPTION); - abChooser.setChoiceForOption(key, option); - } else if (settings.has(KEY_PROBABILITIES)) { - JSONArray newProbabilities = settings.getJSONArray(KEY_PROBABILITIES); - int[] probs = new int[newProbabilities.length()]; - - for (int j = 0; j < newProbabilities.length(); j++) { - probs[j] = newProbabilities.getInt(j); - } - - abOptions.setProbsForKey(key, probs); + abChooser.setChoiceForTest(key, option); } } catch (Exception e) { e.printStackTrace(); diff --git a/astrid/src/com/todoroo/astrid/utility/AstridPreferences.java b/astrid/src/com/todoroo/astrid/utility/AstridPreferences.java index 9592ce3f8..a647ddd73 100644 --- a/astrid/src/com/todoroo/astrid/utility/AstridPreferences.java +++ b/astrid/src/com/todoroo/astrid/utility/AstridPreferences.java @@ -12,7 +12,7 @@ import com.todoroo.astrid.api.AstridApiConstants; import com.todoroo.astrid.data.Task; import com.todoroo.astrid.service.ThemeService; import com.todoroo.astrid.service.abtesting.ABChooser; -import com.todoroo.astrid.service.abtesting.ABOptions; +import com.todoroo.astrid.service.abtesting.ABTests; public class AstridPreferences { @@ -49,10 +49,10 @@ public class AstridPreferences { Preferences.setIfUnset(prefs, editor, r, R.string.p_fontSize, 18); Preferences.setIfUnset(prefs, editor, r, R.string.p_showNotes, false); - boolean swipeEnabled = (ABChooser.readChoiceForOption(ABOptions.AB_OPTION_SWIPE_ENABLED_KEY) == 1); + boolean swipeEnabled = (ABChooser.readChoiceForTest(ABTests.AB_TEST_SWIPE_ENABLED_KEY) == 1); Preferences.setIfUnset(prefs, editor, r, R.string.p_swipe_lists_performance_key, swipeEnabled ? 3 : 0); - boolean contactsPickerEnabled = (ABChooser.readChoiceForOption(ABOptions.AB_OPTION_CONTACTS_PICKER_ENABLED) == 1); + boolean contactsPickerEnabled = (ABChooser.readChoiceForTest(ABTests.AB_TEST_CONTACTS_PICKER_ENABLED) == 1); Preferences.setIfUnset(prefs, editor, r, R.string.p_use_contact_picker, contactsPickerEnabled); if ("white-blue".equals(Preferences.getStringValue(R.string.p_theme))) { //$NON-NLS-1$ migrate from when white-blue wasn't the default diff --git a/tests/src/com/todoroo/astrid/service/ABTestingServiceTest.java b/tests/src/com/todoroo/astrid/service/ABTestingServiceTest.java new file mode 100644 index 000000000..c1802b08f --- /dev/null +++ b/tests/src/com/todoroo/astrid/service/ABTestingServiceTest.java @@ -0,0 +1,95 @@ +package com.todoroo.astrid.service; + +import com.todoroo.andlib.data.TodorooCursor; +import com.todoroo.andlib.service.Autowired; +import com.todoroo.andlib.sql.Order; +import com.todoroo.andlib.sql.Query; +import com.todoroo.andlib.utility.AndroidUtilities; +import com.todoroo.andlib.utility.Preferences; +import com.todoroo.astrid.dao.ABTestEventDao; +import com.todoroo.astrid.data.ABTestEvent; +import com.todoroo.astrid.service.abtesting.ABChooser; +import com.todoroo.astrid.service.abtesting.ABTests; +import com.todoroo.astrid.test.DatabaseTestCase; + +public class ABTestingServiceTest extends DatabaseTestCase { + + @Autowired ABTestEventDao abTestEventDao; + @Autowired ABChooser abChooser; + @Autowired ABTests abTests; + + public void testReportInitialEventNewUser() { + testInitialEvents(true, false); + } + + public void testReportInitialEventExistingUser() { + testInitialEvents(false, true); + } + + private void testInitialEvents(boolean newUser, boolean activatedUser) { + testInterval(newUser, activatedUser, 0); + } + + public void testIntervalEventWithShortIntervalNewUser() { + testIntervalEventWithShortInterval(true, false); + } + + public void testIntervalEventWithShortIntervalExistingUser() { + testIntervalEventWithShortInterval(false, true); + } + + public void testIntervalEventWithLongIntervalNewUser() { + testIntervalEventWithLongInterval(true, false); + } + + public void testIntervalEventWithLongIntervalExistingUser() { + testIntervalEventWithLongInterval(false, true); + } + + private void testIntervalEventWithShortInterval(boolean newUser, boolean activatedUser) { + testInterval(newUser, activatedUser, 3); + } + + private void testIntervalEventWithLongInterval(boolean newUser, boolean activatedUser) { + testInterval(newUser, activatedUser, 14); + } + + private void testInterval(boolean newUser, boolean activatedUser, int testInterval) { + abChooser.makeChoicesForAllTests(newUser, activatedUser); + abTestEventDao.createTestEventWithTimeInterval(TEST_NAME, testInterval); + + TodorooCursor events = abTestEventDao.query( + Query.select(ABTestEvent.PROPERTIES) + .where(ABTestEvent.TEST_NAME.eq(TEST_NAME)) + .orderBy(Order.asc(ABTestEvent.TIME_INTERVAL))); + try { + int maxIntervalIndex = AndroidUtilities.indexOf(ABTestEvent.TIME_INTERVALS, testInterval); + assertEquals(maxIntervalIndex + 1, events.getCount()); + + for (int i = 0; i < events.getCount(); i++) { + events.moveToNext(); + ABTestEvent event = new ABTestEvent(events); + assertExpectedValues(event, newUser, activatedUser, ABTestEvent.TIME_INTERVALS[i]); + } + } finally { + events.close(); + } + } + + private void assertExpectedValues(ABTestEvent event, boolean newUser, boolean activatedUser, int timeInterval) { + assertEquals(TEST_NAME, event.getValue(ABTestEvent.TEST_NAME)); + assertEquals(newUser ? 1 : 0, event.getValue(ABTestEvent.NEW_USER).intValue()); + assertEquals(activatedUser ? 1 : 0, event.getValue(ABTestEvent.ACTIVATED_USER).intValue()); + assertEquals(timeInterval, event.getValue(ABTestEvent.TIME_INTERVAL).intValue()); + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + abTests.addTest(TEST_NAME, new int[] { 9, 1 } , new int[] { 1, 9 }, TEST_OPTIONS); + Preferences.clear(TEST_NAME); + } + + private static final String TEST_NAME = "test_experiment"; + private static final String[] TEST_OPTIONS = new String[] { "opt-1", "opt-2" }; +}