Optimizations with reporting using arrays instead of separate requests, bugfixes

astrid/src/com/todoroo/astrid/data/ABTestEvent.java

@@ -89,7 +89,7 @@ public class ABTestEvent extends AbstractModel {
             TABLE, "reported"); // 0 if not yet reported, 1 if reported successfully
 
     /** List of all properties for this model */
-    public static final Property<?>[] PROPERTIES = generateProperties(Task.class);
+    public static final Property<?>[] PROPERTIES = generateProperties(ABTestEvent.class);
 
 
 

astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java

@@ -65,9 +65,9 @@ public class AstridDependencyInjector extends AbstractDependencyInjector {
 
         // com.todoroo.astrid.dao
         injectables.put("database", Database.class);
-        injectables.put("taskDao", new TaskDao());
+        injectables.put("taskDao", TaskDao.class);
-        injectables.put("metadataDao", new MetadataDao());
+        injectables.put("metadataDao", MetadataDao.class);
-        injectables.put("tagDataDao", new TagDataDao());
+        injectables.put("tagDataDao", TagDataDao.class);
         injectables.put("storeObjectDao", StoreObjectDao.class);
         injectables.put("updateDao", UpdateDao.class);
         injectables.put("userDao", UserDao.class);
@@ -103,7 +103,7 @@ public class AstridDependencyInjector extends AbstractDependencyInjector {
         // AB testing
         injectables.put("abChooser", ABChooser.class);
         injectables.put("abTests", new ABTests());
-        injectables.put("abTestEventDao", new ABTestEventDao());
+        injectables.put("abTestEventDao", ABTestEventDao.class);
         injectables.put("abTestEventReportingService", ABTestEventReportingService.class);
         injectables.put("abTestInvoker", ABTestInvoker.class);
         injectables.put("featureFlipper", FeatureFlipper.class);

astrid/src/com/todoroo/astrid/service/StartupService.java

@@ -184,7 +184,7 @@ public class StartupService {
         // perform startup activities in a background thread
         final int finalLatestVersion = latestSetVersion;
 
-        initializeABTesting(latestSetVersion == 0);
+        abChooser.makeChoicesForAllTests(latestSetVersion == 0, taskService.getUserActivationStatus());
 
         new Thread(new Runnable() {
             public void run() {
@@ -220,6 +220,7 @@ public class StartupService {
                 featureFlipper.updateFeatures();
 
                 abTestEventDao.createRelativeDateEvents();
+                abTestEventReportingService.pushAllUnreportedABTestEvents();
             }
         }).start();
 
@@ -232,12 +233,6 @@ public class StartupService {
         hasStartedUp = true;
     }
 
-    private void initializeABTesting(boolean newUser) {
-        abTestEventReportingService.initialize();
-        abTestEventReportingService.pushAllUnreportedABTestEvents();
-        abChooser.makeChoicesForAllTests(newUser, taskService.getUserActivationStatus());
-    }
-
     /**
      * @param context
      * @param e error that was raised

astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java

@@ -8,10 +8,10 @@ import org.json.JSONObject;
 
 import android.util.Log;
 
-import com.todoroo.andlib.data.DatabaseDao.ModelUpdateListener;
 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;
@@ -36,38 +36,11 @@ public final class ABTestEventReportingService {
         DependencyInjectionService.getInstance().inject(this);
     }
 
-    public void initialize() {
-        abTestEventDao.addListener(new ModelUpdateListener<ABTestEvent>() {
-            @Override
-            public void onModelUpdated(ABTestEvent model) {
-                if (model.getValue(ABTestEvent.REPORTED) == 1)
-                    return;
-
-                pushABTestEvent(model);
-            }
-        });
-    }
-
-    public void pushABTestEvent(final ABTestEvent model) {
-        new Thread(new Runnable() {
-            public void run() {
-                try {
-                    JSONArray payload = new JSONArray().put(jsonFromABTestEvent(model));
-                    abTestInvoker.post(payload);
-                    model.setValue(ABTestEvent.REPORTED, 1);
-                    abTestEventDao.saveExisting(model);
-                } catch (JSONException e) {
-                    handleException(e);
-                } catch (IOException e) {
-                    handleException(e);
-                }
-            };
-         }).start();
-    }
-
     public void pushAllUnreportedABTestEvents() {
         final TodorooCursor<ABTestEvent> unreported = abTestEventDao.query(Query.select(ABTestEvent.PROPERTIES)
-                .where(ABTestEvent.REPORTED.eq(0)));
+                .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
@@ -102,7 +75,7 @@ public final class ABTestEventReportingService {
         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, model.getValue(ABTestEvent.TIME_INTERVAL));
+        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);
@@ -112,10 +85,28 @@ public final class ABTestEventReportingService {
 
     private static JSONArray jsonArrayFromABTestEvents(TodorooCursor<ABTestEvent> events) throws JSONException {
         JSONArray result = new JSONArray();
+
+        String lastTestKey = "";
+        JSONObject testAcc = null;
         for (events.moveToFirst(); !events.isAfterLast(); events.moveToNext()) {
             ABTestEvent model = new ABTestEvent(events);
-            result.put(jsonFromABTestEvent(model));
+
+            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;
     }