Synchronize ab test event reporting to prevent duplicate reports

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

@@ -61,33 +61,30 @@ public final class ABTestEventReportingService {
     }
 
     private void pushAllUnreportedABTestEvents() {
-        if (StatisticsService.dontCollectStatistics())
+        synchronized(ABTestEventReportingService.class) {
-            return;
+            if (StatisticsService.dontCollectStatistics())
-        final TodorooCursor<ABTestEvent> unreported = abTestEventDao.query(Query.select(ABTestEvent.PROPERTIES)
+                return;
-                .where(ABTestEvent.REPORTED.eq(0))
+            final TodorooCursor<ABTestEvent> unreported = abTestEventDao.query(Query.select(ABTestEvent.PROPERTIES)
-                .orderBy(Order.asc(ABTestEvent.TEST_NAME), Order.asc(ABTestEvent.TIME_INTERVAL)));
+                    .where(ABTestEvent.REPORTED.eq(0))
-        if (unreported.getCount() > 0) {
+                    .orderBy(Order.asc(ABTestEvent.TEST_NAME), Order.asc(ABTestEvent.TIME_INTERVAL)));
-            new Thread(new Runnable() {
+            if (unreported.getCount() > 0) {
-                @Override
+                try {
-                public void run() {
+                    JSONArray payload = jsonArrayFromABTestEvents(unreported);
-                    try {
+                    abTestInvoker.post(payload);
-                        JSONArray payload = jsonArrayFromABTestEvents(unreported);
+                    ABTestEvent model = new ABTestEvent();
-                        abTestInvoker.post(payload);
+                    for (unreported.moveToFirst(); !unreported.isAfterLast(); unreported.moveToNext()) {
-                        ABTestEvent model = new ABTestEvent();
+                        model.readFromCursor(unreported);
-                        for (unreported.moveToFirst(); !unreported.isAfterLast(); unreported.moveToNext()) {
+                        model.setValue(ABTestEvent.REPORTED, 1);
-                            model.readFromCursor(unreported);
+                        abTestEventDao.saveExisting(model);
-                            model.setValue(ABTestEvent.REPORTED, 1);
-                            abTestEventDao.saveExisting(model);
-                        }
-                    }  catch (JSONException e) {
-                        handleException(e);
-                    } catch (IOException e) {
-                        handleException(e);
-                    } finally {
-                        unreported.close();
                     }
+                } catch (JSONException e) {
+                    handleException(e);
+                } catch (IOException e) {
+                    handleException(e);
+                } finally {
+                    unreported.close();
                 }
-            }).start();
+            }
         }
     }