Bug 1270213 - Update TelemetryPing & builders to use docID. r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 05 May 2016 16:32:40 -0700
changeset 364563 51271da24968ce2e6e7653c430dcaa5373b4f4a9
parent 364562 e9a8a8a71338d4326a819117cf463f1dcb56bdde
child 364564 7585177671f75e6cd1a362e9ecaf4570c427dc4d
push id17492
push usermichael.l.comella@gmail.com
push dateFri, 06 May 2016 21:43:40 +0000
reviewerssebastian
bugs1270213
milestone49.0a1
Bug 1270213 - Update TelemetryPing & builders to use docID. r=sebastian Future patches will change the remaining code - This is not yet expected to compile. MozReview-Commit-ID: FpqfThcV7dj
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
mobile/android/base/java/org/mozilla/gecko/telemetry/UploadTelemetryCorePingCallback.java
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryPingBuilder.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TestTelemetryPingBuilder.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
@@ -5,31 +5,30 @@
 
 package org.mozilla.gecko.telemetry;
 
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 
 /**
  * Container for telemetry data and the data necessary to upload it.
  *
- * The unique ID is used by a Store to manipulate its internal pings. Some ping
- * payloads already contain a unique ID (e.g. sequence number in core ping) and
- * this field can mirror that value.
+ * The doc ID is used by a Store to manipulate its internal pings and should
+ * be the same value found in the urlPath.
  *
  * If you want to create one of these, consider extending
  * {@link org.mozilla.gecko.telemetry.pingbuilders.TelemetryPingBuilder}
  * or one of its descendants.
  */
 public class TelemetryPing {
     private final String urlPath;
     private final ExtendedJSONObject payload;
-    private final int uniqueID;
+    private final String docID;
 
-    public TelemetryPing(final String urlPath, final ExtendedJSONObject payload, final int uniqueID) {
+    public TelemetryPing(final String urlPath, final ExtendedJSONObject payload, final String docID) {
         this.urlPath = urlPath;
         this.payload = payload;
-        this.uniqueID = uniqueID;
+        this.docID = docID;
     }
 
     public String getURLPath() { return urlPath; }
     public ExtendedJSONObject getPayload() { return payload; }
-    public int getUniqueID() { return uniqueID; }
+    public String getDocID() { return docID; }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/UploadTelemetryCorePingCallback.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/UploadTelemetryCorePingCallback.java
@@ -66,21 +66,21 @@ public class UploadTelemetryCorePingCall
                     clientID = profile.getClientId();
                 } catch (final IOException e) {
                     Log.w(LOGTAG, "Unable to get client ID to generate core ping: " + e);
                     return;
                 }
 
                 // Each profile can have different telemetry data so we intentionally grab the shared prefs for the profile.
                 final SharedPreferences sharedPrefs = GeckoSharedPrefs.forProfileName(activity, profile.getName());
-                final int sequenceNumber = TelemetryCorePingBuilder.getAndIncrementSequenceNumber(sharedPrefs);
-                final TelemetryCorePingBuilder pingBuilder = new TelemetryCorePingBuilder(activity, sequenceNumber)
+                final TelemetryCorePingBuilder pingBuilder = new TelemetryCorePingBuilder(activity)
                         .setClientID(clientID)
                         .setDefaultSearchEngine(TelemetryCorePingBuilder.getEngineIdentifier(engine))
-                        .setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile));
+                        .setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile))
+                        .setSequenceNumber(TelemetryCorePingBuilder.getAndIncrementSequenceNumber(sharedPrefs));
                 final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
                 if (distributionId != null) {
                     pingBuilder.setOptDistributionID(distributionId);
                 }
 
                 activity.getTelemetryDispatcher().queuePingForUpload(activity, pingBuilder);
             }
         });
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryCorePingBuilder.java
@@ -43,22 +43,21 @@ public class TelemetryCorePingBuilder ex
     private static final String DEFAULT_SEARCH_ENGINE = "defaultSearch";
     private static final String DEVICE = "device";
     private static final String DISTRIBUTION_ID = "distributionId";
     private static final String EXPERIMENTS = "experiments";
     private static final String LOCALE = "locale";
     private static final String OS_ATTR = "os";
     private static final String OS_VERSION = "osversion";
     private static final String PROFILE_CREATION_DATE = "profileDate";
-    public static final String SEQ = "seq";
+    private static final String SEQ = "seq";
     private static final String VERSION_ATTR = "v";
 
-    public TelemetryCorePingBuilder(final Context context, final int sequenceNumber) {
-        super(sequenceNumber);
-        setSequenceNumber(sequenceNumber);
+    public TelemetryCorePingBuilder(final Context context) {
+        super();
         initPayloadConstants(context);
     }
 
     private void initPayloadConstants(final Context context) {
         payload.put(VERSION_ATTR, VERSION_VALUE);
         payload.put(OS_ATTR, OS_VALUE);
 
         // We limit the device descriptor to 32 characters because it can get long. We give fewer characters to the
@@ -131,21 +130,22 @@ public class TelemetryCorePingBuilder ex
         }
         payload.put(PROFILE_CREATION_DATE, date);
         return this;
     }
 
     /**
      * @param seq a positive sequence number.
      */
-    private void setSequenceNumber(final int seq) {
+    public TelemetryCorePingBuilder setSequenceNumber(final int seq) {
         if (seq < 0) {
             throw new IllegalArgumentException("Expected positive sequence number. Recived: " + seq);
         }
         payload.put(SEQ, seq);
+        return this;
     }
 
     /**
      * Gets the sequence number from shared preferences and increments it in the prefs. This method
      * is not thread safe.
      */
     @WorkerThread // synchronous shared prefs write.
     public static int getAndIncrementSequenceNumber(final SharedPreferences sharedPrefsForProfile) {
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryPingBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetryPingBuilder.java
@@ -22,38 +22,38 @@ import java.util.UUID;
  *   * Forming the server url
  */
 abstract class TelemetryPingBuilder {
     // In the server url, the initial path directly after the "scheme://host:port/"
     private static final String SERVER_INITIAL_PATH = "submit/telemetry";
 
     private final String serverPath;
     protected final ExtendedJSONObject payload;
-    private final int uniqueID;
+    private final String docID;
 
-    public TelemetryPingBuilder(final int uniqueID) {
-        serverPath = getTelemetryServerPath(getDocType());
+    public TelemetryPingBuilder() {
+        docID = UUID.randomUUID().toString();
+        serverPath = getTelemetryServerPath(getDocType(), docID);
         payload = new ExtendedJSONObject();
-        this.uniqueID = uniqueID;
     }
 
     /**
      * @return the name of the ping (e.g. "core")
      */
     public abstract String getDocType();
 
     /**
      * @return the fields that are mandatory for the resultant ping to be uploaded to
      *         the server. These will be validated before the ping is built.
      */
     public abstract String[] getMandatoryFields();
 
     public TelemetryPing build() {
         validatePayload();
-        return new TelemetryPing(serverPath, payload, uniqueID);
+        return new TelemetryPing(serverPath, payload, docID);
     }
 
     private void validatePayload() {
         final Set<String> keySet = payload.keySet();
         for (final String mandatoryField : getMandatoryFields()) {
             if (!keySet.contains(mandatoryField)) {
                 throw new IllegalArgumentException("Builder does not contain mandatory field: " +
                         mandatoryField);
@@ -63,26 +63,25 @@ abstract class TelemetryPingBuilder {
 
     /**
      * Returns a url of the format:
      *   http://hostname/submit/telemetry/docId/docType/appName/appVersion/appUpdateChannel/appBuildID
      *
      * @param docType The name of the ping (e.g. "main")
      * @return a url at which to POST the telemetry data to
      */
-    private static String getTelemetryServerPath(final String docType) {
-        final String docId = UUID.randomUUID().toString();
+    private static String getTelemetryServerPath(final String docType, final String docID) {
         final String appName = AppConstants.MOZ_APP_BASENAME;
         final String appVersion = AppConstants.MOZ_APP_VERSION;
         final String appUpdateChannel = AppConstants.MOZ_UPDATE_CHANNEL;
         final String appBuildId = AppConstants.MOZ_APP_BUILDID;
 
         // The compiler will optimize a single String concatenation into a StringBuilder statement.
         // If you change this `return`, be sure to keep it as a single statement to keep it optimized!
         return SERVER_INITIAL_PATH + '/' +
-                docId + '/' +
+                docID + '/' +
                 docType + '/' +
                 appName + '/' +
                 appVersion + '/' +
                 appUpdateChannel + '/' +
                 appBuildId;
     }
 }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TestTelemetryPingBuilder.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/telemetry/pingbuilders/TestTelemetryPingBuilder.java
@@ -35,18 +35,16 @@ public class TestTelemetryPingBuilder {
     public void testMandatoryFieldsIncluded() {
         final MandatoryFieldsBuilder builder = new MandatoryFieldsBuilder();
         builder.setNonMandatoryField()
                 .setMandatoryField();
         assertNotNull("Builder does not throw and returns non-null value", builder.build());
     }
 
     private static class NoMandatoryFieldsBuilder extends TelemetryPingBuilder {
-        public NoMandatoryFieldsBuilder() { super(1); }
-
         @Override
         public String getDocType() {
             return "";
         }
 
         @Override
         public String[] getMandatoryFields() {
             return new String[0];
@@ -56,18 +54,16 @@ public class TestTelemetryPingBuilder {
             payload.put("non-mandatory", true);
             return this;
         }
     }
 
     private static class MandatoryFieldsBuilder extends TelemetryPingBuilder {
         private static final String MANDATORY_FIELD = "mandatory-field";
 
-        public MandatoryFieldsBuilder() { super(0); }
-
         @Override
         public String getDocType() {
             return "";
         }
 
         @Override
         public String[] getMandatoryFields() {
             return new String[] {