mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-12-03 10:33:33 +00:00
Bug 1243585 - review: Address comments. r=me
MozReview-Commit-ID: FrPLa7U57B7 --HG-- extra : rebase_source : 658c3a739a679a17e47822284bd55dfc40cd9e75
This commit is contained in:
parent
88ce3a891a
commit
8e50768a95
@ -79,8 +79,7 @@ import org.mozilla.gecko.tabs.TabHistoryFragment;
|
||||
import org.mozilla.gecko.tabs.TabHistoryPage;
|
||||
import org.mozilla.gecko.tabs.TabsPanel;
|
||||
import org.mozilla.gecko.telemetry.TelemetryDispatcher;
|
||||
import org.mozilla.gecko.telemetry.TelemetryUploadService;
|
||||
import org.mozilla.gecko.telemetry.pingbuilders.TelemetryCorePingBuilder;
|
||||
import org.mozilla.gecko.telemetry.UploadTelemetryCorePingCallback;
|
||||
import org.mozilla.gecko.toolbar.AutocompleteHandler;
|
||||
import org.mozilla.gecko.toolbar.BrowserToolbar;
|
||||
import org.mozilla.gecko.toolbar.BrowserToolbar.TabEditingState;
|
||||
@ -169,7 +168,6 @@ import java.io.ByteArrayOutputStream;
|
||||
import java.io.File;
|
||||
import java.io.FileNotFoundException;
|
||||
import java.io.IOException;
|
||||
import java.lang.ref.WeakReference;
|
||||
import java.lang.reflect.Method;
|
||||
import java.net.MalformedURLException;
|
||||
import java.net.URL;
|
||||
@ -3868,6 +3866,10 @@ public class BrowserApp extends GeckoApp
|
||||
@Override
|
||||
public int getLayout() { return R.layout.gecko_app; }
|
||||
|
||||
public TelemetryDispatcher getTelemetryDispatcher() {
|
||||
return mTelemetryDispatcher;
|
||||
}
|
||||
|
||||
// For use from tests only.
|
||||
@RobocopTarget
|
||||
public ReadingListHelper getReadingListHelper() {
|
||||
@ -3945,66 +3947,6 @@ public class BrowserApp extends GeckoApp
|
||||
mDynamicToolbar.setTemporarilyVisible(false, VisibilityTransition.IMMEDIATE);
|
||||
}
|
||||
|
||||
private static class UploadTelemetryCorePingCallback implements SearchEngineManager.SearchEngineCallback {
|
||||
private final WeakReference<BrowserApp> activityWeakReference;
|
||||
|
||||
public UploadTelemetryCorePingCallback(final BrowserApp activity) {
|
||||
this.activityWeakReference = new WeakReference<>(activity);
|
||||
}
|
||||
|
||||
// May be called from any thread.
|
||||
@Override
|
||||
public void execute(final org.mozilla.gecko.search.SearchEngine engine) {
|
||||
// Don't waste resources queueing to the background thread if we don't have a reference.
|
||||
if (this.activityWeakReference.get() == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// The containing method can be called from onStart: queue this work so that
|
||||
// the first launch of the activity doesn't trigger profile init too early.
|
||||
//
|
||||
// Additionally, getAndIncrementSequenceNumberSync must be called from a worker thread.
|
||||
ThreadUtils.postToBackgroundThread(new Runnable() {
|
||||
@WorkerThread
|
||||
@Override
|
||||
public void run() {
|
||||
final BrowserApp activity = activityWeakReference.get();
|
||||
if (activity == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
final GeckoProfile profile = activity.getProfile();
|
||||
if (!TelemetryUploadService.isUploadEnabledByProfileConfig(activity, profile)) {
|
||||
Log.d(LOGTAG, "Core ping upload disabled by profile config. Returning.");
|
||||
return;
|
||||
}
|
||||
|
||||
final String clientID;
|
||||
try {
|
||||
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 TelemetryCorePingBuilder pingBuilder = new TelemetryCorePingBuilder(activity)
|
||||
.setClientID(clientID)
|
||||
.setDefaultSearchEngine(TelemetryCorePingBuilder.getEngineIdentifier(engine))
|
||||
.setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile))
|
||||
.setSequenceNumber(TelemetryCorePingBuilder.getAndIncrementSequenceNumberSync(sharedPrefs));
|
||||
final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
|
||||
if (distributionId != null) {
|
||||
pingBuilder.setOptDistributionID(distributionId);
|
||||
}
|
||||
|
||||
activity.mTelemetryDispatcher.queuePingForUpload(activity, pingBuilder);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
public static interface TabStripInterface {
|
||||
public void refresh();
|
||||
void setOnTabChangedListener(OnTabAddedOrRemovedListener listener);
|
||||
|
@ -17,7 +17,6 @@ import org.mozilla.gecko.util.ThreadUtils;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.lang.ref.WeakReference;
|
||||
|
||||
/**
|
||||
* The entry-point for Java-based telemetry. This class handles:
|
||||
@ -68,9 +67,9 @@ public class TelemetryDispatcher {
|
||||
uploadAllPingsImmediatelyScheduler = new TelemetryUploadAllPingsImmediatelyScheduler();
|
||||
}
|
||||
|
||||
private void queuePingForUpload(final Context context, final int uniqueID, final TelemetryPing ping,
|
||||
final TelemetryPingStore store, final TelemetryUploadScheduler scheduler) {
|
||||
final QueuePingRunnable runnable = new QueuePingRunnable(context, uniqueID, ping, store, scheduler);
|
||||
private void queuePingForUpload(final Context context, final TelemetryPing ping, final TelemetryPingStore store,
|
||||
final TelemetryUploadScheduler scheduler) {
|
||||
final QueuePingRunnable runnable = new QueuePingRunnable(context, ping, store, scheduler);
|
||||
ThreadUtils.postToBackgroundThread(runnable); // TODO: Investigate how busy this thread is. See if we want another.
|
||||
}
|
||||
|
||||
@ -79,21 +78,18 @@ public class TelemetryDispatcher {
|
||||
*/
|
||||
public void queuePingForUpload(final Context context, final TelemetryCorePingBuilder pingBuilder) {
|
||||
final TelemetryPing ping = pingBuilder.build();
|
||||
final int id = ping.getPayload().getIntegerSafely(TelemetryCorePingBuilder.SEQ);
|
||||
queuePingForUpload(context, id, ping, coreStore, uploadAllPingsImmediatelyScheduler);
|
||||
queuePingForUpload(context, ping, coreStore, uploadAllPingsImmediatelyScheduler);
|
||||
}
|
||||
|
||||
private static class QueuePingRunnable implements Runnable {
|
||||
private final WeakReference<Context> contextWeakReference;
|
||||
private final int uniqueID;
|
||||
private final Context applicationContext;
|
||||
private final TelemetryPing ping;
|
||||
private final TelemetryPingStore store;
|
||||
private final TelemetryUploadScheduler scheduler;
|
||||
|
||||
public QueuePingRunnable(final Context context, final int uniqueID, final TelemetryPing ping,
|
||||
final TelemetryPingStore store, final TelemetryUploadScheduler scheduler) {
|
||||
this.contextWeakReference = new WeakReference<>(context);
|
||||
this.uniqueID = uniqueID;
|
||||
public QueuePingRunnable(final Context context, final TelemetryPing ping, final TelemetryPingStore store,
|
||||
final TelemetryUploadScheduler scheduler) {
|
||||
this.applicationContext = context.getApplicationContext();
|
||||
this.ping = ping;
|
||||
this.store = store;
|
||||
this.scheduler = scheduler;
|
||||
@ -101,21 +97,16 @@ public class TelemetryDispatcher {
|
||||
|
||||
@Override
|
||||
public void run() {
|
||||
final Context context = contextWeakReference.get();
|
||||
if (context == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// We block while storing the ping so the scheduled upload is guaranteed to have the newly-stored value.
|
||||
try {
|
||||
store.storePing(uniqueID, ping);
|
||||
store.storePing(ping);
|
||||
} catch (final IOException e) {
|
||||
// Don't log exception to avoid leaking profile path.
|
||||
Log.e(LOGTAG, "Unable to write ping to disk. Continuing with upload attempt");
|
||||
}
|
||||
|
||||
if (scheduler.isReadyToUpload(store)) {
|
||||
scheduler.scheduleUpload(context, store);
|
||||
scheduler.scheduleUpload(applicationContext, store);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -10,6 +10,10 @@ 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.
|
||||
*
|
||||
* If you want to create one of these, consider extending
|
||||
* {@link org.mozilla.gecko.telemetry.pingbuilders.TelemetryPingBuilder}
|
||||
* or one of its descendants.
|
||||
@ -17,12 +21,15 @@ import org.mozilla.gecko.sync.ExtendedJSONObject;
|
||||
public class TelemetryPing {
|
||||
private final String urlPath;
|
||||
private final ExtendedJSONObject payload;
|
||||
private final int uniqueID;
|
||||
|
||||
public TelemetryPing(final String urlPath, final ExtendedJSONObject payload) {
|
||||
public TelemetryPing(final String urlPath, final ExtendedJSONObject payload, final int uniqueID) {
|
||||
this.urlPath = urlPath;
|
||||
this.payload = payload;
|
||||
this.uniqueID = uniqueID;
|
||||
}
|
||||
|
||||
public String getURLPath() { return urlPath; }
|
||||
public ExtendedJSONObject getPayload() { return payload; }
|
||||
public int getUniqueID() { return uniqueID; }
|
||||
}
|
||||
|
@ -1,24 +0,0 @@
|
||||
/* 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.sync.ExtendedJSONObject;
|
||||
import org.mozilla.gecko.telemetry.stores.TelemetryPingStore;
|
||||
|
||||
/**
|
||||
* Container for telemetry data and the data necessary to upload it, as
|
||||
* returned by a {@link TelemetryPingStore}.
|
||||
*/
|
||||
public class TelemetryPingFromStore extends TelemetryPing {
|
||||
private final int uniqueID;
|
||||
|
||||
public TelemetryPingFromStore(final String url, final ExtendedJSONObject payload, final int uniqueID) {
|
||||
super(url, payload);
|
||||
this.uniqueID = uniqueID;
|
||||
}
|
||||
|
||||
public int getUniqueID() { return uniqueID; }
|
||||
}
|
||||
|
@ -25,8 +25,8 @@ import org.mozilla.gecko.util.StringUtils;
|
||||
import java.io.IOException;
|
||||
import java.net.URISyntaxException;
|
||||
import java.security.GeneralSecurityException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
/**
|
||||
@ -79,7 +79,7 @@ public class TelemetryUploadService extends IntentService {
|
||||
* @return true if all pings were uploaded successfully, false otherwise.
|
||||
*/
|
||||
private static boolean uploadPendingPingsFromStore(final Context context, final TelemetryPingStore store) {
|
||||
final ArrayList<TelemetryPingFromStore> pingsToUpload = store.getAllPings();
|
||||
final List<TelemetryPing> pingsToUpload = store.getAllPings();
|
||||
if (pingsToUpload.isEmpty()) {
|
||||
return true;
|
||||
}
|
||||
@ -87,16 +87,16 @@ public class TelemetryUploadService extends IntentService {
|
||||
final String serverSchemeHostPort = getServerSchemeHostPort(context);
|
||||
final HashSet<Integer> successfulUploadIDs = new HashSet<>(pingsToUpload.size()); // used for side effects.
|
||||
final PingResultDelegate delegate = new PingResultDelegate(successfulUploadIDs);
|
||||
for (final TelemetryPingFromStore ping : pingsToUpload) {
|
||||
// There are minimal gains in trying to upload if we already failed one attempt.
|
||||
if (delegate.hadConnectionError()) {
|
||||
break;
|
||||
}
|
||||
|
||||
for (final TelemetryPing ping : pingsToUpload) {
|
||||
// TODO: It'd be great to re-use the same HTTP connection for each upload request.
|
||||
delegate.setPingID(ping.getUniqueID());
|
||||
final String url = serverSchemeHostPort + "/" + ping.getURLPath();
|
||||
uploadPayload(url, ping.getPayload(), delegate);
|
||||
|
||||
// There are minimal gains in trying to upload if we already failed one attempt.
|
||||
if (delegate.hadConnectionError()) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
final boolean wereAllUploadsSuccessful = !delegate.hadConnectionError();
|
||||
@ -213,7 +213,7 @@ public class TelemetryUploadService extends IntentService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Logs on success & failure and appends the set ID to the given ArrayList on success.
|
||||
* Logs on success & failure and appends the set ID to the given Set on success.
|
||||
*
|
||||
* Note: you *must* set the ping ID before attempting upload or we'll throw!
|
||||
*
|
||||
|
@ -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 android.content.SharedPreferences;
|
||||
import android.support.annotation.WorkerThread;
|
||||
import android.util.Log;
|
||||
import org.mozilla.gecko.BrowserApp;
|
||||
import org.mozilla.gecko.GeckoProfile;
|
||||
import org.mozilla.gecko.GeckoSharedPrefs;
|
||||
import org.mozilla.gecko.distribution.DistributionStoreCallback;
|
||||
import org.mozilla.gecko.search.SearchEngineManager;
|
||||
import org.mozilla.gecko.telemetry.pingbuilders.TelemetryCorePingBuilder;
|
||||
import org.mozilla.gecko.util.StringUtils;
|
||||
import org.mozilla.gecko.util.ThreadUtils;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.lang.ref.WeakReference;
|
||||
|
||||
/**
|
||||
* A search engine callback that will attempt to upload the core ping.
|
||||
*/
|
||||
public class UploadTelemetryCorePingCallback implements SearchEngineManager.SearchEngineCallback {
|
||||
private static final String LOGTAG = StringUtils.safeSubstring(
|
||||
"Gecko" + UploadTelemetryCorePingCallback.class.getSimpleName(), 0, 23);
|
||||
|
||||
private final WeakReference<BrowserApp> activityWeakReference;
|
||||
|
||||
public UploadTelemetryCorePingCallback(final BrowserApp activity) {
|
||||
this.activityWeakReference = new WeakReference<>(activity);
|
||||
}
|
||||
|
||||
// May be called from any thread.
|
||||
@Override
|
||||
public void execute(final org.mozilla.gecko.search.SearchEngine engine) {
|
||||
// Don't waste resources queueing to the background thread if we don't have a reference.
|
||||
if (this.activityWeakReference.get() == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// The containing method can be called from onStart: queue this work so that
|
||||
// the first launch of the activity doesn't trigger profile init too early.
|
||||
//
|
||||
// Additionally, getAndIncrementSequenceNumber must be called from a worker thread.
|
||||
ThreadUtils.postToBackgroundThread(new Runnable() {
|
||||
@WorkerThread
|
||||
@Override
|
||||
public void run() {
|
||||
final BrowserApp activity = activityWeakReference.get();
|
||||
if (activity == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
final GeckoProfile profile = activity.getProfile();
|
||||
if (!TelemetryUploadService.isUploadEnabledByProfileConfig(activity, profile)) {
|
||||
Log.d(LOGTAG, "Core ping upload disabled by profile config. Returning.");
|
||||
return;
|
||||
}
|
||||
|
||||
final String clientID;
|
||||
try {
|
||||
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)
|
||||
.setClientID(clientID)
|
||||
.setDefaultSearchEngine(TelemetryCorePingBuilder.getEngineIdentifier(engine))
|
||||
.setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile));
|
||||
final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
|
||||
if (distributionId != null) {
|
||||
pingBuilder.setOptDistributionID(distributionId);
|
||||
}
|
||||
|
||||
activity.getTelemetryDispatcher().queuePingForUpload(activity, pingBuilder);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
@ -51,8 +51,9 @@ public class TelemetryCorePingBuilder extends TelemetryPingBuilder {
|
||||
public static final String SEQ = "seq";
|
||||
private static final String VERSION_ATTR = "v";
|
||||
|
||||
public TelemetryCorePingBuilder(final Context context) {
|
||||
super();
|
||||
public TelemetryCorePingBuilder(final Context context, final int sequenceNumber) {
|
||||
super(sequenceNumber);
|
||||
setSequenceNumber(sequenceNumber);
|
||||
initPayloadConstants(context);
|
||||
}
|
||||
|
||||
@ -135,20 +136,22 @@ public class TelemetryCorePingBuilder extends TelemetryPingBuilder {
|
||||
/**
|
||||
* @param seq a positive sequence number.
|
||||
*/
|
||||
public TelemetryCorePingBuilder setSequenceNumber(final int seq) {
|
||||
private void 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 getAndIncrementSequenceNumberSync(final SharedPreferences sharedPrefsForProfile) {
|
||||
public static int getAndIncrementSequenceNumber(final SharedPreferences sharedPrefsForProfile) {
|
||||
final int seq = sharedPrefsForProfile.getInt(TelemetryConstants.PREF_SEQ_COUNT, 1);
|
||||
|
||||
// We store synchronously before constructing to ensure this sequence number will not be re-used.
|
||||
sharedPrefsForProfile.edit().putInt(TelemetryConstants.PREF_SEQ_COUNT, seq + 1).commit();
|
||||
sharedPrefsForProfile.edit().putInt(TelemetryConstants.PREF_SEQ_COUNT, seq + 1).apply();
|
||||
return seq;
|
||||
}
|
||||
|
||||
@ -168,6 +171,7 @@ public class TelemetryCorePingBuilder extends TelemetryPingBuilder {
|
||||
/**
|
||||
* @return the search engine identifier in the format expected by the core ping.
|
||||
*/
|
||||
@Nullable
|
||||
public static String getEngineIdentifier(final SearchEngine searchEngine) {
|
||||
final String identifier = searchEngine.getIdentifier();
|
||||
return TextUtils.isEmpty(identifier) ? null : identifier;
|
||||
|
@ -27,10 +27,12 @@ abstract class TelemetryPingBuilder {
|
||||
|
||||
private final String serverPath;
|
||||
protected final ExtendedJSONObject payload;
|
||||
private final int uniqueID;
|
||||
|
||||
public TelemetryPingBuilder() {
|
||||
public TelemetryPingBuilder(final int uniqueID) {
|
||||
serverPath = getTelemetryServerPath(getDocType());
|
||||
payload = new ExtendedJSONObject();
|
||||
this.uniqueID = uniqueID;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -46,7 +48,7 @@ abstract class TelemetryPingBuilder {
|
||||
|
||||
public TelemetryPing build() {
|
||||
validatePayload();
|
||||
return new TelemetryPing(serverPath, payload);
|
||||
return new TelemetryPing(serverPath, payload, uniqueID);
|
||||
}
|
||||
|
||||
private void validatePayload() {
|
||||
|
@ -23,10 +23,10 @@ public class TelemetryUploadAllPingsImmediatelyScheduler implements TelemetryUpl
|
||||
}
|
||||
|
||||
@Override
|
||||
public void scheduleUpload(final Context context, final TelemetryPingStore store) {
|
||||
public void scheduleUpload(final Context applicationContext, final TelemetryPingStore store) {
|
||||
final Intent i = new Intent(TelemetryUploadService.ACTION_UPLOAD);
|
||||
i.setClass(context, TelemetryUploadService.class);
|
||||
i.setClass(applicationContext, TelemetryUploadService.class);
|
||||
i.putExtra(TelemetryUploadService.EXTRA_STORE, store);
|
||||
context.startService(i);
|
||||
applicationContext.startService(i);
|
||||
}
|
||||
}
|
||||
|
@ -22,5 +22,5 @@ import org.mozilla.gecko.telemetry.stores.TelemetryPingStore;
|
||||
*/
|
||||
public interface TelemetryUploadScheduler {
|
||||
boolean isReadyToUpload(TelemetryPingStore store);
|
||||
void scheduleUpload(Context context, TelemetryPingStore store);
|
||||
void scheduleUpload(Context applicationContext, TelemetryPingStore store);
|
||||
}
|
||||
|
@ -15,8 +15,8 @@ import org.json.JSONObject;
|
||||
import org.mozilla.gecko.sync.ExtendedJSONObject;
|
||||
import org.mozilla.gecko.sync.NonObjectJSONException;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPing;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPingFromStore;
|
||||
import org.mozilla.gecko.util.FileUtils;
|
||||
import org.mozilla.gecko.util.StringUtils;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.FileInputStream;
|
||||
@ -44,7 +44,7 @@ import java.util.regex.Pattern;
|
||||
* in the ping, such as a sequence number.
|
||||
*
|
||||
* Using separate files for this store allows for less restrictive concurrency:
|
||||
* * requires locking: {@link #storePing(long, TelemetryPing)} writes a new file
|
||||
* * requires locking: {@link #storePing(TelemetryPing)} writes a new file
|
||||
* * requires locking: {@link #getAllPings()} reads all files, including those potentially being written,
|
||||
* hence locking
|
||||
* * no locking: {@link #maybePrunePings()} deletes the least recently written pings, none of which should
|
||||
@ -53,7 +53,8 @@ import java.util.regex.Pattern;
|
||||
* currently written
|
||||
*/
|
||||
public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
private static final String LOGTAG = "Gecko" + TelemetryJSONFilePingStore.class.getSimpleName();
|
||||
private static final String LOGTAG = StringUtils.safeSubstring(
|
||||
"Gecko" + TelemetryJSONFilePingStore.class.getSimpleName(), 0, 23);
|
||||
|
||||
@VisibleForTesting static final int MAX_PING_COUNT = 40; // TODO: value.
|
||||
|
||||
@ -88,7 +89,7 @@ public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void storePing(final long uniqueID, final TelemetryPing ping) throws IOException {
|
||||
public void storePing(final TelemetryPing ping) throws IOException {
|
||||
final String output;
|
||||
try {
|
||||
output = new JSONObject()
|
||||
@ -100,7 +101,7 @@ public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
throw new IOException("Unable to create JSON to store to disk");
|
||||
}
|
||||
|
||||
final FileOutputStream outputStream = new FileOutputStream(getPingFile(uniqueID), false);
|
||||
final FileOutputStream outputStream = new FileOutputStream(getPingFile(ping.getUniqueID()), false);
|
||||
blockForLockAndWriteFileAndCloseStream(outputStream, output);
|
||||
}
|
||||
|
||||
@ -140,9 +141,9 @@ public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
}
|
||||
|
||||
@Override
|
||||
public ArrayList<TelemetryPingFromStore> getAllPings() {
|
||||
public ArrayList<TelemetryPing> getAllPings() {
|
||||
final File[] files = storeDir.listFiles(new PingFileFilter());
|
||||
final ArrayList<TelemetryPingFromStore> out = new ArrayList<>(files.length);
|
||||
final ArrayList<TelemetryPing> out = new ArrayList<>(files.length);
|
||||
for (final File file : files) {
|
||||
final FileInputStream inputStream;
|
||||
try {
|
||||
@ -175,7 +176,7 @@ public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
throw new IllegalStateException("These files are already filtered - did not expect to see " +
|
||||
"an invalid ID in these files");
|
||||
}
|
||||
out.add(new TelemetryPingFromStore(url, payload, id));
|
||||
out.add(new TelemetryPing(url, payload, id));
|
||||
} catch (final IOException | JSONException | NonObjectJSONException e) {
|
||||
Log.w(LOGTAG, "Bad json in ping. Ignoring.");
|
||||
continue;
|
||||
@ -233,7 +234,7 @@ public class TelemetryJSONFilePingStore implements TelemetryPingStore {
|
||||
// hit to exception handling so instead we assume the file lock will be closed.
|
||||
return new JSONObject(FileUtils.readStringFromInputStreamAndCloseStream(inputStream, fileSize));
|
||||
} finally {
|
||||
inputStream.close(); // redundant: closed when the steram is closed, but let's be safe.
|
||||
inputStream.close(); // redundant: closed when the stream is closed, but let's be safe.
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -8,32 +8,32 @@ package org.mozilla.gecko.telemetry.stores;
|
||||
|
||||
import android.os.Parcelable;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPing;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPingFromStore;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
/**
|
||||
* Persistent storage for TelemetryPings that are queued for upload.
|
||||
*
|
||||
* An implementation of this class is expected to be thread-safe.
|
||||
* An implementation of this class is expected to be thread-safe. Additionally,
|
||||
* multiple instances can be created and run simultaneously so they must be able
|
||||
* to synchronize state (or be stateless!).
|
||||
*/
|
||||
public interface TelemetryPingStore extends Parcelable {
|
||||
|
||||
/**
|
||||
* @return a list of all the telemetry pings in the store that are ready for upload.
|
||||
*/
|
||||
ArrayList<TelemetryPingFromStore> getAllPings();
|
||||
List<TelemetryPing> getAllPings();
|
||||
|
||||
/**
|
||||
* Save a ping to the store.
|
||||
*
|
||||
* @param uniqueID a unique identifier for the ping - will be returned in {@link #onUploadAttemptComplete(Set)}
|
||||
* @param ping the ping to store
|
||||
* @throws IOException for underlying store access errors
|
||||
*/
|
||||
void storePing(long uniqueID, TelemetryPing ping) throws IOException;
|
||||
void storePing(TelemetryPing ping) throws IOException;
|
||||
|
||||
/**
|
||||
* Removes telemetry pings from the store if there are too many pings or they take up too much space.
|
||||
@ -44,7 +44,7 @@ public interface TelemetryPingStore extends Parcelable {
|
||||
* Removes the successfully uploaded pings from the database and performs another other actions necessary
|
||||
* for when upload is completed.
|
||||
*
|
||||
* @param successfulRemoveIDs unique ids of pings passed to {@link #storePing(long, TelemetryPing)} that were
|
||||
* @param successfulRemoveIDs unique ids of pings passed to {@link #storePing(TelemetryPing)} that were
|
||||
* successfully uploaded
|
||||
*/
|
||||
void onUploadAttemptComplete(Set<Integer> successfulRemoveIDs);
|
||||
|
@ -582,8 +582,8 @@ gbjar.sources += ['java/org/mozilla/gecko/' + x for x in [
|
||||
'telemetry/TelemetryConstants.java',
|
||||
'telemetry/TelemetryDispatcher.java',
|
||||
'telemetry/TelemetryPing.java',
|
||||
'telemetry/TelemetryPingFromStore.java',
|
||||
'telemetry/TelemetryUploadService.java',
|
||||
'telemetry/UploadTelemetryCorePingCallback.java',
|
||||
'TelemetryContract.java',
|
||||
'text/FloatingActionModeCallback.java',
|
||||
'text/FloatingToolbarTextSelection.java',
|
||||
|
@ -40,6 +40,8 @@ public class TestTelemetryPingBuilder {
|
||||
}
|
||||
|
||||
private static class NoMandatoryFieldsBuilder extends TelemetryPingBuilder {
|
||||
public NoMandatoryFieldsBuilder() { super(1); }
|
||||
|
||||
@Override
|
||||
public String getDocType() {
|
||||
return "";
|
||||
@ -59,6 +61,8 @@ public class TestTelemetryPingBuilder {
|
||||
private static class MandatoryFieldsBuilder extends TelemetryPingBuilder {
|
||||
private static final String MANDATORY_FIELD = "mandatory-field";
|
||||
|
||||
public MandatoryFieldsBuilder() { super(0); }
|
||||
|
||||
@Override
|
||||
public String getDocType() {
|
||||
return "";
|
||||
|
@ -15,7 +15,6 @@ import org.junit.runner.RunWith;
|
||||
import org.mozilla.gecko.background.testhelpers.TestRunner;
|
||||
import org.mozilla.gecko.sync.ExtendedJSONObject;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPing;
|
||||
import org.mozilla.gecko.telemetry.TelemetryPingFromStore;
|
||||
import org.mozilla.gecko.util.FileUtils;
|
||||
|
||||
import java.io.File;
|
||||
@ -77,8 +76,8 @@ public class TestTelemetryJSONFilePingStore {
|
||||
assertStoreFileCount(0);
|
||||
|
||||
final int expectedID = 48679;
|
||||
final TelemetryPing expectedPing = new TelemetryPing("a/server/url", generateTelemetryPayload());
|
||||
testStore.storePing(expectedID, expectedPing);
|
||||
final TelemetryPing expectedPing = new TelemetryPing("a/server/url", generateTelemetryPayload(), expectedID);
|
||||
testStore.storePing(expectedPing);
|
||||
|
||||
assertStoreFileCount(1);
|
||||
final String filename = testDir.list()[0];
|
||||
@ -92,7 +91,7 @@ public class TestTelemetryJSONFilePingStore {
|
||||
public void testStorePingMultiplePingsStoreSeparateFiles() throws Exception {
|
||||
assertStoreFileCount(0);
|
||||
for (int i = 1; i < 10; ++i) {
|
||||
testStore.storePing(i, new TelemetryPing("server " + i, generateTelemetryPayload()));
|
||||
testStore.storePing(new TelemetryPing("server " + i, generateTelemetryPayload(), i));
|
||||
assertStoreFileCount(i);
|
||||
}
|
||||
}
|
||||
@ -100,7 +99,7 @@ public class TestTelemetryJSONFilePingStore {
|
||||
@Test
|
||||
public void testStorePingReleasesFileLock() throws Exception {
|
||||
assertStoreFileCount(0);
|
||||
testStore.storePing(0, new TelemetryPing("server", generateTelemetryPayload()));
|
||||
testStore.storePing(new TelemetryPing("server", generateTelemetryPayload(), 0));
|
||||
assertStoreFileCount(1);
|
||||
final File file = new File(testDir, testDir.list()[0]);
|
||||
final FileOutputStream stream = new FileOutputStream(file);
|
||||
@ -116,8 +115,8 @@ public class TestTelemetryJSONFilePingStore {
|
||||
final String urlPrefix = "url";
|
||||
writeTestPingsToStore(3, urlPrefix);
|
||||
|
||||
final ArrayList<TelemetryPingFromStore> pings = testStore.getAllPings();
|
||||
for (final TelemetryPingFromStore ping : pings) {
|
||||
final ArrayList<TelemetryPing> pings = testStore.getAllPings();
|
||||
for (final TelemetryPing ping : pings) {
|
||||
final int id = ping.getUniqueID(); // we use ID as a key for specific pings and check against the url values.
|
||||
assertEquals("Expected url path value received", urlPrefix + id, ping.getURLPath());
|
||||
assertIsGeneratedPayload(ping.getPayload());
|
||||
|
Loading…
Reference in New Issue
Block a user