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

--HG--
extra : rebase_source : 2ed71c7e6ce269cc43878f0e166dd9b149f3ccbc
This commit is contained in:
Edouard Oger 2018-02-20 15:01:24 +08:00
parent 505ab9d57e
commit b8c9f187a4
5 changed files with 23 additions and 40 deletions

View File

@ -16,8 +16,6 @@ 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;
@ -39,27 +37,19 @@ public class TelemetrySyncPingBuilderTest {
@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"));

View File

@ -105,6 +105,8 @@ public class TelemetrySyncPingBundleBuilderTest {
public void testGeneralShape() throws Exception {
builder.setSyncStore(syncPings);
builder.setSyncEventStore(eventPings);
builder.setDeviceID("device-id-1");
builder.setUID("uid-1");
TelemetryOutgoingPing outgoingPing = builder.build();
@ -128,13 +130,16 @@ public class TelemetrySyncPingBundleBuilderTest {
// 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");
@ -151,10 +156,8 @@ public class TelemetrySyncPingBundleBuilderTest {
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);
@ -166,14 +169,12 @@ public class TelemetrySyncPingBundleBuilderTest {
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);
@ -184,14 +185,12 @@ public class TelemetrySyncPingBundleBuilderTest {
.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();

View File

@ -151,14 +151,6 @@ public class TelemetryBackgroundReceiver extends BroadcastReceiver {
telemetryStore = syncTelemetryStore;
TelemetrySyncPingBuilder localPingBuilder = new TelemetrySyncPingBuilder();
if (uid != null) {
localPingBuilder.setUID(uid);
}
if (deviceID != null) {
localPingBuilder.setDeviceID(deviceID);
}
if (devices != null) {
localPingBuilder.setDevices(devices);
}
@ -233,6 +225,8 @@ public class TelemetryBackgroundReceiver extends BroadcastReceiver {
// 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)

View File

@ -89,16 +89,6 @@ public class TelemetrySyncPingBuilder extends TelemetryLocalPingBuilder {
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) {

View File

@ -69,6 +69,16 @@ public class TelemetrySyncPingBundleBuilder extends TelemetryPingBuilder {
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(