Bug 1247489 - Replace TelemetryPingGenerator with TelemetryPingBuilder & friends. r=grisha
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 11 Apr 2016 17:45:29 -0700
changeset 331067 59c84a2beabdb13a97f181ba3fcdaf327357d6cb
parent 331066 d0b6d063cf4981b3c893f511e84f6aac3a96b2f9
child 331068 c2ad1f090a2ee81dc50af2f0dd87151923329075
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgrisha
bugs1247489
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1247489 - Replace TelemetryPingGenerator with TelemetryPingBuilder & friends. r=grisha The Builder pattern has the following benefits: * Encapsulate identifying optional arguments * Encapsulate parameter validation * More fluent parameter insertion (e.g. instead as unnamed arguments to a function) * My implementation makes it fairly straight-forward to construct new telemetry pings. MozReview-Commit-ID: EpcW3N57HJj
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingBuilder.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingBuilder.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingGenerator.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryConstants.java
@@ -19,30 +19,9 @@ public class TelemetryConstants {
     public static final String EXTRA_DEFAULT_SEARCH_ENGINE = "defaultSearchEngine";
     public static final String EXTRA_DOC_ID = "docId";
     public static final String EXTRA_PROFILE_NAME = "geckoProfileName";
     public static final String EXTRA_PROFILE_PATH = "geckoProfilePath";
     public static final String EXTRA_SEQ = "seq";
 
     public static final String PREF_SERVER_URL = "telemetry-serverUrl";
     public static final String PREF_SEQ_COUNT = "telemetry-seqCount";
-
-    public static class CorePing {
-        private CorePing() { /* To prevent instantiation */ }
-
-        public static final String NAME = "core";
-        public static final int VERSION_VALUE = 4; // For version history, see toolkit/components/telemetry/docs/core-ping.rst
-        public static final String OS_VALUE = "Android";
-
-        public static final String ARCHITECTURE = "arch";
-        public static final String CLIENT_ID = "clientId";
-        public static final String DEFAULT_SEARCH_ENGINE = "defaultSearch";
-        public static final String DEVICE = "device";
-        public static final String DISTRIBUTION_ID = "distributionId";
-        public static final String EXPERIMENTS = "experiments";
-        public static final String LOCALE = "locale";
-        public static final String OS_ATTR = "os";
-        public static final String OS_VERSION = "osversion";
-        public static final String PROFILE_CREATION_DATE = "profileDate";
-        public static final String SEQ = "seq";
-        public static final String VERSION_ATTR = "v";
-    }
 }
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingBuilder.java
@@ -0,0 +1,138 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.mozilla.gecko.telemetry;
+
+import android.content.Context;
+import android.os.Build;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import org.mozilla.gecko.AppConstants;
+import org.mozilla.gecko.Locales;
+import org.mozilla.gecko.util.Experiments;
+import org.mozilla.gecko.util.StringUtils;
+
+import java.util.Locale;
+
+/**
+ * Builds a {@link TelemetryPing} representing a core ping.
+ *
+ * See https://gecko.readthedocs.org/en/latest/toolkit/components/telemetry/telemetry/core-ping.html
+ * for details on the core ping.
+ */
+class TelemetryCorePingBuilder extends TelemetryPingBuilder {
+
+    private static final String NAME = "core";
+    private static final int VERSION_VALUE = 4; // For version history, see toolkit/components/telemetry/docs/core-ping.rst
+    private static final String OS_VALUE = "Android";
+
+    private static final String ARCHITECTURE = "arch";
+    private static final String CLIENT_ID = "clientId";
+    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";
+    private static final String SEQ = "seq";
+    private static final String VERSION_ATTR = "v";
+
+    public TelemetryCorePingBuilder(final Context context, final String serverURLSchemeHostPort) {
+        super(serverURLSchemeHostPort);
+        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
+        // manufacturer because we're less likely to have manufacturers with similar names than we are for a
+        // manufacturer to have two devices with the similar names (e.g. Galaxy S6 vs. Galaxy Note 6).
+        final String deviceDescriptor =
+                StringUtils.safeSubstring(Build.MANUFACTURER, 0, 12) + '-' + StringUtils.safeSubstring(Build.MODEL, 0, 19);
+
+        payload.put(ARCHITECTURE, AppConstants.ANDROID_CPU_ARCH);
+        payload.put(DEVICE, deviceDescriptor);
+        payload.put(LOCALE, Locales.getLanguageTag(Locale.getDefault()));
+        payload.put(OS_VERSION, Integer.toString(Build.VERSION.SDK_INT)); // A String for cross-platform reasons.
+        payload.putArray(EXPERIMENTS, Experiments.getActiveExperiments(context));
+    }
+
+    @Override
+    String getDocType() {
+        return NAME;
+    }
+
+    @Override
+    String[] getMandatoryFields() {
+        return new String[] {
+                ARCHITECTURE,
+                CLIENT_ID,
+                DEFAULT_SEARCH_ENGINE,
+                DEVICE,
+                LOCALE,
+                OS_ATTR,
+                OS_VERSION,
+                PROFILE_CREATION_DATE,
+                SEQ,
+                VERSION_ATTR,
+        };
+    }
+
+    public TelemetryCorePingBuilder setClientID(@NonNull final String clientID) {
+        if (clientID == null) {
+            throw new IllegalArgumentException("Expected non-null clientID");
+        }
+        payload.put(CLIENT_ID, clientID);
+        return this;
+    }
+
+    /**
+     * @param engine the default search engine identifier, or null if there is an error.
+     */
+    public TelemetryCorePingBuilder setDefaultSearchEngine(@Nullable final String engine) {
+        if (engine != null && engine.isEmpty()) {
+            throw new IllegalArgumentException("Received empty string. Expected identifier or null.");
+        }
+        payload.put(DEFAULT_SEARCH_ENGINE, engine);
+        return this;
+    }
+
+    public TelemetryCorePingBuilder setOptDistributionID(@NonNull final String distributionID) {
+        if (distributionID == null) {
+            throw new IllegalArgumentException("Expected non-null distribution ID");
+        }
+        payload.put(DISTRIBUTION_ID, distributionID);
+        return this;
+    }
+
+    /**
+     * @param date a positive date value, or null if there is an error.
+     */
+    public TelemetryCorePingBuilder setProfileCreationDate(@Nullable final Long date) {
+        if (date != null && date < 0) {
+            throw new IllegalArgumentException("Expect positive date value. Received: " + date);
+        }
+        payload.put(PROFILE_CREATION_DATE, date);
+        return this;
+    }
+
+    // TODO (mcomella): We can potentially build two pings with the same seq no if we leave seq as an argument.
+    /**
+     * @param seq a positive sequence number.
+     */
+    public TelemetryCorePingBuilder setSequenceNumber(final int seq) {
+        if (seq < 0) {
+            throw new IllegalArgumentException("Expected positive sequence number. Recived: " + seq);
+        }
+        payload.put(SEQ, seq);
+        return this;
+    }
+}
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPing.java
@@ -4,16 +4,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.telemetry;
 
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 
 /**
  * Container for telemetry data and the data necessary to upload it.
+ *
+ * If you want to create one of these, consider extending
+ * {@link TelemetryPingBuilder} or one of its descendants.
  */
 public class TelemetryPing {
     private final String url;
     private final ExtendedJSONObject payload;
 
     public TelemetryPing(final String url, final ExtendedJSONObject payload) {
         this.url = url;
         this.payload = payload;
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingBuilder.java
@@ -0,0 +1,88 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.mozilla.gecko.telemetry;
+
+import org.mozilla.gecko.AppConstants;
+import org.mozilla.gecko.sync.ExtendedJSONObject;
+
+import java.util.Set;
+import java.util.UUID;
+
+/**
+ * A generic Builder for {@link TelemetryPing} instances. Each overriding class is
+ * expected to create a specific type of ping (e.g. "core").
+ *
+ * This base class handles the common ping operations under the hood:
+ *   * Validating mandatory fields
+ *   * 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 serverUrl;
+    protected final ExtendedJSONObject payload;
+
+    public TelemetryPingBuilder(final String serverURLSchemeHostPort) {
+        serverUrl = getTelemetryServerURL(getDocType(), serverURLSchemeHostPort);
+        payload = new ExtendedJSONObject();
+    }
+
+    /**
+     * @return the name of the ping (e.g. "core")
+     */
+    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.
+     */
+    abstract String[] getMandatoryFields();
+
+    public TelemetryPing build() {
+        validatePayload();
+        return new TelemetryPing(serverUrl, payload);
+    }
+
+    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);
+            }
+        }
+    }
+
+    /**
+     * Returns a url of the format:
+     *   http://hostname/submit/telemetry/docId/docType/appName/appVersion/appUpdateChannel/appBuildID
+     *
+     * @param serverURLSchemeHostPort The server url with the scheme, host, and port (e.g. "http://mozilla.org:80")
+     * @param docType The name of the ping (e.g. "main")
+     * @return a url at which to POST the telemetry data to
+     */
+    private static String getTelemetryServerURL(final String docType,
+            final String serverURLSchemeHostPort) {
+        final String docId = UUID.randomUUID().toString();
+        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 serverURLSchemeHostPort + '/' +
+                SERVER_INITIAL_PATH + '/' +
+                docId + '/' +
+                docType + '/' +
+                appName + '/' +
+                appVersion + '/' +
+                appUpdateChannel + '/' +
+                appBuildId;
+    }
+}
deleted file mode 100644
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryPingGenerator.java
+++ /dev/null
@@ -1,107 +0,0 @@
-/* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.gecko.telemetry;
-
-import android.content.Context;
-import android.os.Build;
-import android.support.annotation.Nullable;
-import android.text.TextUtils;
-
-import org.mozilla.gecko.AppConstants;
-import org.mozilla.gecko.Locales;
-import org.mozilla.gecko.sync.ExtendedJSONObject;
-import org.mozilla.gecko.telemetry.TelemetryConstants.CorePing;
-import org.mozilla.gecko.util.Experiments;
-import org.mozilla.gecko.util.StringUtils;
-
-import java.io.IOException;
-import java.util.Locale;
-
-/**
- * A class with static methods to generate the various Java-created Telemetry pings to upload to the telemetry server.
- */
-public class TelemetryPingGenerator {
-
-    // In the server url, the initial path directly after the "scheme://host:port/"
-    private static final String SERVER_INITIAL_PATH = "submit/telemetry";
-
-    /**
-     * Returns a url of the format:
-     *   http://hostname/submit/telemetry/docId/docType/appName/appVersion/appUpdateChannel/appBuildID
-     *
-     * @param docId A unique document ID for the ping associated with the upload to this server
-     * @param serverURLSchemeHostPort The server url with the scheme, host, and port (e.g. "http://mozilla.org:80")
-     * @param docType The name of the ping (e.g. "main")
-     * @return a url at which to POST the telemetry data to
-     */
-    private static String getTelemetryServerURL(final String docId, final String serverURLSchemeHostPort,
-            final String docType) {
-        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 serverURLSchemeHostPort + '/' +
-                SERVER_INITIAL_PATH + '/' +
-                docId + '/' +
-                docType + '/' +
-                appName + '/' +
-                appVersion + '/' +
-                appUpdateChannel + '/' +
-                appBuildId;
-    }
-
-    /**
-     * @param docId A unique document ID for the ping associated with the upload to this server
-     * @param clientId The client ID of this profile (from Gecko)
-     * @param serverURLSchemeHostPort The server url with the scheme, host, and port (e.g. "http://mozilla.org:80")
-     * @param profileCreationDateDays The profile creation date in days to the UNIX epoch, NOT MILLIS.
-     * @throws IOException when client ID could not be created
-     */
-    public static TelemetryPing createCorePing(final Context context, final String docId, final String clientId,
-            final String serverURLSchemeHostPort, final int seq, final long profileCreationDateDays,
-            @Nullable final String distributionId, @Nullable final String defaultSearchEngine) {
-        final String serverURL = getTelemetryServerURL(docId, serverURLSchemeHostPort, CorePing.NAME);
-        final ExtendedJSONObject payload =
-                createCorePingPayload(context, clientId, seq, profileCreationDateDays, distributionId, defaultSearchEngine);
-        return new TelemetryPing(serverURL, payload);
-    }
-
-    private static ExtendedJSONObject createCorePingPayload(final Context context, final String clientId,
-            final int seq, final long profileCreationDate, @Nullable final String distributionId,
-            @Nullable final String defaultSearchEngine) {
-        final ExtendedJSONObject ping = new ExtendedJSONObject();
-        ping.put(CorePing.VERSION_ATTR, CorePing.VERSION_VALUE);
-        ping.put(CorePing.OS_ATTR, CorePing.OS_VALUE);
-
-        // We limit the device descriptor to 32 characters because it can get long. We give fewer characters to the
-        // manufacturer because we're less likely to have manufacturers with similar names than we are for a
-        // manufacturer to have two devices with the similar names (e.g. Galaxy S6 vs. Galaxy Note 6).
-        final String deviceDescriptor =
-                StringUtils.safeSubstring(Build.MANUFACTURER, 0, 12) + '-' + StringUtils.safeSubstring(Build.MODEL, 0, 19);
-
-        ping.put(CorePing.ARCHITECTURE, AppConstants.ANDROID_CPU_ARCH);
-        ping.put(CorePing.CLIENT_ID, clientId);
-        ping.put(CorePing.DEFAULT_SEARCH_ENGINE, TextUtils.isEmpty(defaultSearchEngine) ? null : defaultSearchEngine);
-        ping.put(CorePing.DEVICE, deviceDescriptor);
-        ping.put(CorePing.LOCALE, Locales.getLanguageTag(Locale.getDefault()));
-        ping.put(CorePing.OS_VERSION, Integer.toString(Build.VERSION.SDK_INT)); // A String for cross-platform reasons.
-        ping.put(CorePing.SEQ, seq);
-        ping.putArray(CorePing.EXPERIMENTS, Experiments.getActiveExperiments(context));
-
-        // Optional.
-        if (distributionId != null) {
-            ping.put(CorePing.DISTRIBUTION_ID, distributionId);
-        }
-
-        // `null` indicates failure more clearly than < 0.
-        final Long finalProfileCreationDate = (profileCreationDate < 0) ? null : profileCreationDate;
-        ping.put(CorePing.PROFILE_CREATION_DATE, finalProfileCreationDate);
-        return ping;
-    }
-}
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -5,16 +5,17 @@
 package org.mozilla.gecko.telemetry;
 
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.WorkerThread;
+import android.text.TextUtils;
 import android.util.Log;
 import ch.boye.httpclientandroidlib.HttpResponse;
 import ch.boye.httpclientandroidlib.client.ClientProtocolException;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.background.BackgroundService;
 import org.mozilla.gecko.distribution.DistributionStoreCallback;
 import org.mozilla.gecko.preferences.GeckoPreferences;
@@ -165,33 +166,43 @@ public class TelemetryUploadService exte
 
         return true;
     }
 
     @WorkerThread
     private void uploadCorePing(@NonNull final String docId, final int seq, @NonNull final String profileName,
                 @NonNull final String profilePath, @Nullable final String defaultSearchEngine) {
         final GeckoProfile profile = GeckoProfile.get(this, profileName, profilePath);
-        final long profileCreationDate = getProfileCreationDate(profile);
         final String clientId;
         try {
             clientId = profile.getClientId();
         } catch (final IOException e) {
             Log.w(LOGTAG, "Unable to get client ID to generate core ping: returning.", e);
             return;
         }
 
         // Each profile can have different telemetry data so we intentionally grab the shared prefs for the profile.
         final SharedPreferences sharedPrefs = GeckoSharedPrefs.forProfileName(this, profileName);
         // TODO (bug 1241685): Sync this preference with the gecko preference.
         final String serverURLSchemeHostPort =
                 sharedPrefs.getString(TelemetryConstants.PREF_SERVER_URL, TelemetryConstants.DEFAULT_SERVER_URL);
+
+        final long profileCreationDate = getProfileCreationDate(profile);
+        final TelemetryCorePingBuilder builder = new TelemetryCorePingBuilder(this, serverURLSchemeHostPort)
+                .setClientID(clientId)
+                .setDefaultSearchEngine(TextUtils.isEmpty(defaultSearchEngine) ? null : defaultSearchEngine)
+                .setProfileCreationDate(profileCreationDate < 0 ? null : profileCreationDate)
+                .setSequenceNumber(seq);
+
         final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
-        final TelemetryPing corePing = TelemetryPingGenerator.createCorePing(this, docId, clientId,
-                serverURLSchemeHostPort, seq, profileCreationDate, distributionId, defaultSearchEngine);
+        if (distributionId != null) {
+            builder.setOptDistributionID(distributionId);
+        }
+
+        final TelemetryPing corePing = builder.build();
         final CorePingResultDelegate resultDelegate = new CorePingResultDelegate();
         uploadPing(corePing, resultDelegate);
     }
 
     private void uploadPing(final TelemetryPing ping, final ResultDelegate delegate) {
         final BaseResource resource;
         try {
             resource = new BaseResource(ping.getURL());
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -568,18 +568,19 @@ gbjar.sources += ['java/org/mozilla/geck
     'tabs/TabsGridLayout.java',
     'tabs/TabsLayoutAdapter.java',
     'tabs/TabsLayoutItemView.java',
     'tabs/TabsListLayout.java',
     'tabs/TabsPanel.java',
     'tabs/TabsPanelThumbnailView.java',
     'Telemetry.java',
     'telemetry/TelemetryConstants.java',
+    'telemetry/TelemetryCorePingBuilder.java',
     'telemetry/TelemetryPing.java',
-    'telemetry/TelemetryPingGenerator.java',
+    'telemetry/TelemetryPingBuilder.java',
     'telemetry/TelemetryUploadService.java',
     'TelemetryContract.java',
     'text/FloatingActionModeCallback.java',
     'text/FloatingToolbarTextSelection.java',
     'text/TextAction.java',
     'text/TextSelection.java',
     'TextSelectionHandle.java',
     'ThumbnailHelper.java',