Bug 1247489 - Replace TelemetryPingGenerator with TelemetryPingBuilder & friends. r=grisha draft
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 11 Apr 2016 17:45:29 -0700
changeset 349609 aef43acaac8de473de17a7b17a1f7e4d71c96cf8
parent 349534 5b37f138e5bfdb84fe3460dd115ec225933e0f5f
child 349610 c19d7bdc2241614e0cffe08cefc8ee883901e064
push id15148
push usermichael.l.comella@gmail.com
push dateTue, 12 Apr 2016 01:35:36 +0000
reviewersgrisha
bugs1247489
milestone48.0a1
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,29 +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 = 3; // 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 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,129 @@
+/*
+ * 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 = 3; // 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 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 value.");
+        }
+        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;
+    }
+
+    /**
+     * @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,101 +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 defaultSearchEngine) {
-        final String serverURL = getTelemetryServerURL(docId, serverURLSchemeHostPort, CorePing.NAME);
-        final ExtendedJSONObject payload =
-                createCorePingPayload(context, clientId, seq, profileCreationDateDays, 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 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));
-
-        // `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.preferences.GeckoPreferences;
 import org.mozilla.gecko.sync.net.BaseResource;
@@ -164,33 +165,38 @@ 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 TelemetryPing corePing = TelemetryPingGenerator.createCorePing(this, docId, clientId,
-                serverURLSchemeHostPort, seq, profileCreationDate, defaultSearchEngine);
+        final long profileCreationDate = getProfileCreationDate(profile);
+        final TelemetryPing corePing = new TelemetryCorePingBuilder(this, serverURLSchemeHostPort)
+                .setClientID(clientId)
+                .setDefaultSearchEngine(TextUtils.isEmpty(defaultSearchEngine) ? null : defaultSearchEngine)
+                .setProfileCreationDate(profileCreationDate < 0 ? null : profileCreationDate)
+                .setSequenceNumber(seq)
+                .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
@@ -564,18 +564,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',