Bug 1363924 p2 - Move deviceID and uid to payload level in sync ping. r?grisha draft
authorEdouard Oger <eoger@fastmail.com>
Tue, 20 Feb 2018 15:01:24 +0800
changeset 759832 5cf1fabfee63c4f47133f1488d59e898de67a030
parent 759831 788fd955addc194e27bcf69ea778ce5076f940ce
child 759833 838fd8e00280902ac5d4391f19b9a7237917c984
child 759865 c02a92c88df105ff8e380a886575c3d59277bd99
push id100479
push userbmo:eoger@fastmail.com
push dateMon, 26 Feb 2018 17:35:30 +0000
reviewersgrisha
bugs1363924
milestone60.0a1
Bug 1363924 p2 - Move deviceID and uid to payload level in sync ping. r?grisha In the next commit, we will send telemetry events in the sync ping. The "event" JSON object doesn't have "uid"/"deviceID" fields (actually, the "sync" objects shouldn't have them either!). Let's do the right thing and send deviceID and UID as part of the top-level "payload" object. MozReview-Commit-ID: 3D3X3PcJAsW
mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilderTest.java
mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilder.java
--- a/mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilderTest.java
+++ b/mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilderTest.java
@@ -11,18 +11,16 @@ import org.json.JSONException;
 import org.json.simple.JSONArray;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.synchronizer.StoreBatchTracker;
 import org.mozilla.gecko.sync.telemetry.TelemetryStageCollector;
-import org.mozilla.gecko.sync.validation.BookmarkValidationResults;
-import org.mozilla.gecko.sync.validation.ValidationResults;
 import org.mozilla.gecko.telemetry.TelemetryLocalPing;
 
 import java.util.ArrayList;
 import java.util.HashMap;
 
 import static org.junit.Assert.*;
 
 @RunWith(TestRunner.class)
@@ -34,37 +32,29 @@ public class TelemetrySyncPingBuilderTes
     public void setUp() throws Exception {
         pingStore = new TelemetrySyncPingBundleBuilderTest.MockTelemetryPingStore();
         builder = new TelemetrySyncPingBuilder();
     }
 
     @Test
     public void testGeneralShape() throws Exception {
         TelemetryLocalPing localPing = builder
-                .setDeviceID("device-id")
-                .setUID("uid")
                 .setTook(123L)
                 .setRestarted(false)
                 .build();
         ExtendedJSONObject payload = localPing.getPayload();
-        assertEquals("uid", payload.getString("uid"));
         assertEquals(Long.valueOf(123L), payload.getLong("took"));
-        assertEquals("device-id", payload.getString("deviceID"));
         assertFalse(payload.containsKey("restarted"));
 
         localPing = builder
-                .setDeviceID("device-id")
-                .setUID("uid")
                 .setTook(123L)
                 .setRestarted(true)
                 .build();
         payload = localPing.getPayload();
-        assertEquals("uid", payload.getString("uid"));
         assertEquals(Long.valueOf(123L), payload.getLong("took"));
-        assertEquals("device-id", payload.getString("deviceID"));
         assertTrue(payload.getLong("when") != null);
         assertEquals(true, payload.getBoolean("restarted"));
 
     }
 
     @Test
     public void testStage() throws Exception {
         HashMap<String, TelemetryStageCollector> stages = new HashMap<>();
--- a/mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java
+++ b/mobile/android/app/src/test/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilderTest.java
@@ -100,16 +100,18 @@ public class TelemetrySyncPingBundleBuil
         syncPings = new MockTelemetryPingStore();
         eventPings = new MockTelemetryPingStore();
     }
 
     @Test
     public void testGeneralShape() throws Exception {
         builder.setSyncStore(syncPings);
         builder.setSyncEventStore(eventPings);
+        builder.setDeviceID("device-id-1");
+        builder.setUID("uid-1");
 
         TelemetryOutgoingPing outgoingPing = builder.build();
 
         assertTrue(outgoingPing.getPayload().containsKey("application"));
         assertTrue(outgoingPing.getPayload().containsKey("payload"));
         assertTrue(outgoingPing.getPayload().containsKey("id"));
         assertEquals("sync", outgoingPing.getPayload().getString("type"));
         assertEquals(Integer.valueOf(4), outgoingPing.getPayload().getIntegerSafely("version"));
@@ -123,80 +125,77 @@ public class TelemetrySyncPingBundleBuil
         assertTrue(application.containsKey("version"));
         assertTrue(application.containsKey("name"));
         assertTrue(application.containsKey("channel"));
         assertTrue(application.containsKey("buildId"));
         assertTrue(application.containsKey("xpcomAbi"));
 
 
         // Test general shape of payload. Expecting {"syncs":[],"why":"schedule", "version": 1,
-        // "os": {"name": "Android", "version": "<version>", "locale": "<locale>"}}.
+        // "os": {"name": "Android", "version": "<version>", "locale": "<locale>"},
+        // "deviceID": <Hashed Device ID>, "uid": <Hashed UID>}.
         // NB that even though we set an empty sync event store, it's not in the json string.
         // That's because sync events are not yet instrumented.
         ExtendedJSONObject payload = outgoingPing.getPayload().getObject("payload");
-        assertEquals(4, payload.keySet().size());
+        assertEquals(6, payload.keySet().size());
         assertEquals("schedule", payload.getString("why"));
         assertEquals(Integer.valueOf(1), payload.getIntegerSafely("version"));
+        assertEquals(payload.getString("uid"), "uid-1");
+        assertEquals(payload.getString("deviceID"), "device-id-1");
         assertEquals(0, payload.getArray("syncs").size());
         // Test os key.
         ExtendedJSONObject os = payload.getObject("os");
         assertEquals(3, os.keySet().size());
         assertEquals("Android", os.getString("name"));
         // Going to be different depending on the test environment.
         // Test for presence and type to void random failures.
         assertTrue(os.getIntegerSafely("version") != null);
         // Likely "en-US" in tests, but let's test for presence and type to avoid random failures.
         assertTrue(os.getString("locale") != null);
     }
 
     @Test
     public void testBundlingOfMultiplePings() throws Exception {
         // Try just one ping first.
         syncPings.storePing(new TelemetrySyncPingBuilder()
-                .setDeviceID("test-device-id")
                 .setRestarted(true)
                 .setTook(123L)
-                .setUID("test-uid")
                 .build()
         );
         builder.setSyncStore(syncPings);
 
         TelemetryOutgoingPing outgoingPing = builder.build();
 
         // Ensure we have that one ping.
         ExtendedJSONObject payload = outgoingPing.getPayload().getObject("payload");
         assertEquals("schedule", payload.getString("why"));
         JSONArray syncs = payload.getArray("syncs");
         assertEquals(1, syncs.size());
-        assertSync((ExtendedJSONObject) syncs.get(0), "test-uid", 123L, "test-device-id", true);
+        assertSync((ExtendedJSONObject) syncs.get(0), 123L, true);
 
         // Add another ping.
         syncPings.storePing(new TelemetrySyncPingBuilder()
-                .setDeviceID("test-device-id")
                 .setRestarted(false)
                 .setTook(321L)
-                .setUID("test-uid")
                 .build()
         );
         builder.setSyncStore(syncPings);
 
         // We should have two pings now.
         outgoingPing = builder.build();
         syncs = outgoingPing.getPayload()
                 .getObject("payload")
                 .getArray("syncs");
         assertEquals(2, syncs.size());
-        assertSync((ExtendedJSONObject) syncs.get(0), "test-uid", 123L, "test-device-id", true);
-        assertSync((ExtendedJSONObject) syncs.get(1), "test-uid", 321L, "test-device-id", false);
+        assertSync((ExtendedJSONObject) syncs.get(0), 123L, true);
+        assertSync((ExtendedJSONObject) syncs.get(1), 321L, false);
     }
 
-    private void assertSync(ExtendedJSONObject sync, String uid, long took, String deviceID, boolean restarted) throws JSONException {
-        assertEquals(uid, sync.getString("uid"));
+    private void assertSync(ExtendedJSONObject sync, long took, boolean restarted) throws JSONException {
         assertEquals(Long.valueOf(took), sync.getLong("took"));
-        assertEquals(deviceID, sync.getString("deviceID"));
 
         // Test that 'when' timestamp looks generally sane.
         final long now = System.currentTimeMillis();
         final long yearAgo = now - 1000L * 60 * 60 * 24 * 365;
         assertTrue(sync.getLong("when") > yearAgo);
         assertTrue(sync.getLong("when") <= now);
 
         if (restarted) {
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
@@ -146,24 +146,16 @@ public class TelemetryBackgroundReceiver
                         final Serializable error = telemetryBundle.getSerializable(TelemetryContract.KEY_ERROR);
                         final Serializable stages = telemetryBundle.getSerializable(TelemetryContract.KEY_STAGES);
                         final long took = telemetryBundle.getLong(TelemetryContract.KEY_TOOK);
                         final boolean didRestart = telemetryBundle.getBoolean(TelemetryContract.KEY_RESTARTED);
 
                         telemetryStore = syncTelemetryStore;
                         TelemetrySyncPingBuilder localPingBuilder = new TelemetrySyncPingBuilder();
 
-                        if (uid != null) {
-                            localPingBuilder.setUID(uid);
-                        }
-
-                        if (deviceID != null) {
-                            localPingBuilder.setDeviceID(deviceID);
-                        }
-
                         if (devices != null) {
                             localPingBuilder.setDevices(devices);
                         }
 
                         if (stages != null) {
                             localPingBuilder.setStages(stages);
                         }
 
@@ -228,16 +220,18 @@ public class TelemetryBackgroundReceiver
                     // we're processing the current telemetry.
                     // Chances of that happening are very small due to how often background telemetry
                     // is actually emitted: very infrequently on a timescale of disk access.
                     final Set<String> localSyncTelemetryToRemove = syncTelemetryStore.getStoredIDs();
                     final Set<String> localSyncEventTelemetryToRemove = syncEventTelemetryStore.getStoredIDs();
 
                     // Bundle up all that we have in our telemetry stores.
                     final TelemetryOutgoingPing syncPing = new TelemetrySyncPingBundleBuilder()
+                            .setUID(uid)
+                            .setDeviceID(deviceID)
                             .setSyncStore(syncTelemetryStore)
                             .setSyncEventStore(syncEventTelemetryStore)
                             .setReason(reasonToUpload)
                             .build();
 
                     // Persist a Sync Ping Bundle.
                     boolean bundledSyncPingPersisted = true;
                     try {
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
@@ -84,26 +84,16 @@ public class TelemetrySyncPingBuilder ex
             }
 
             addUnchecked(engines, stageJSON);
         }
         payload.put("engines", engines);
         return this;
     }
 
-    public TelemetrySyncPingBuilder setUID(@NonNull String uid) {
-        payload.put("uid", uid);
-        return this;
-    }
-
-    public TelemetrySyncPingBuilder setDeviceID(@NonNull String deviceID) {
-        payload.put("deviceID", deviceID);
-        return this;
-    }
-
     @Nullable
     private static JSONArray buildOutgoing(List<StoreBatchTracker.Batch> batches) {
         if (batches == null || batches.size() == 0) {
             return null;
         }
         JSONArray arr = new JSONArray();
         for (int i = 0; i < batches.size(); ++i) {
             StoreBatchTracker.Batch batch = batches.get(i);
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBundleBuilder.java
@@ -64,16 +64,26 @@ public class TelemetrySyncPingBundleBuil
         return new String[0];
     }
 
     public TelemetrySyncPingBundleBuilder setReason(@NonNull String reason) {
         pingData.put("why", reason);
         return this;
     }
 
+    public TelemetrySyncPingBundleBuilder setUID(@NonNull String uid) {
+        pingData.put("uid", uid);
+        return this;
+    }
+
+    public TelemetrySyncPingBundleBuilder setDeviceID(@NonNull String deviceID) {
+        pingData.put("deviceID", deviceID);
+        return this;
+    }
+
     @Override
     public TelemetryOutgoingPing build() {
         final DateFormat pingCreationDateFormat = new SimpleDateFormat(
                 "yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.US);
         pingCreationDateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
 
         payload.put("type", PING_TYPE);
         payload.put("version", PING_BUNDLE_VERSION);