mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1841866: History engine roundtrips data it doesn't know about r=lina,mak 

Differential Revision: https://phabricator.services.mozilla.com/D183224
This commit is contained in:
Sammy Khamis 2023-08-01 18:45:08 +00:00
Родитель ede511b1ea
Коммит a4efac9ebc
15 изменённых файлов: 575 добавлений и 164 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

71
services/sync/modules/engines/history.sys.mjs
Просмотреть файл

@ -4,6 +4,11 @@
const HISTORY_TTL = 5184000; // 60 days in milliseconds
const THIRTY_DAYS_IN_MS = 2592000000; // 30 days in milliseconds
// Sync may bring new fields from other clients, not yet understood by our engine.
// Unknown fields outside these fields are aggregated into 'unknownFields' and
// safely synced to prevent data loss.
const VALID_HISTORY_FIELDS = ["id", "title", "histUri", "visits"];
const VALID_VISIT_FIELDS = ["date", "type", "transition"];
import { Async } from "resource://services-common/async.sys.mjs";
import { CommonUtils } from "resource://services-common/utils.sys.mjs";
@ -194,6 +199,8 @@ HistoryStore.prototype = {
let failed = [];
let toAdd = [];
let toRemove = [];
let pageGuidsWithUnknownFields = new Map();
let visitTimesWithUnknownFields = new Map();
await Async.yieldingForEach(records, async record => {
if (record.deleted) {
toRemove.push(record);
@ -202,6 +209,31 @@ HistoryStore.prototype = {
let pageInfo = await this._recordToPlaceInfo(record);
if (pageInfo) {
toAdd.push(pageInfo);
// Pull any unknown fields that may have come from other clients
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
VALID_HISTORY_FIELDS
);
if (unknownFields) {
pageGuidsWithUnknownFields.set(pageInfo.guid, { unknownFields });
}
// Visits themselves could also contain unknown fields
for (const visit of pageInfo.visits) {
let unknownVisitFields =
lazy.PlacesSyncUtils.extractUnknownFields(
visit,
VALID_VISIT_FIELDS
);
if (unknownVisitFields) {
// Visits don't have an id at the time of sync so we'll need
// to use the time instead until it's inserted in the DB
visitTimesWithUnknownFields.set(visit.date.getTime(), {
unknownVisitFields,
});
}
}
}
} catch (ex) {
if (Async.isShutdownException(ex)) {
@ -239,10 +271,33 @@ HistoryStore.prototype = {
// as they are likely to be spurious. We do supply an onError handler
// and log the exceptions seen there as they are likely to be
// informative, but we still never abort the sync based on them.
let unknownFieldsToInsert = [];
try {
await lazy.PlacesUtils.history.insertMany(
chunk,
null,
result => {
const placeToUpdate = pageGuidsWithUnknownFields.get(result.guid);
// Extract the placeId from this result so we can add the unknownFields
// to the proper table
if (placeToUpdate) {
unknownFieldsToInsert.push({
placeId: result.placeId,
unknownFields: placeToUpdate.unknownFields,
});
}
// same for visits
result.visits.forEach(visit => {
let visitToUpdate = visitTimesWithUnknownFields.get(
visit.date.getTime()
);
if (visitToUpdate) {
unknownFieldsToInsert.push({
visitId: visit.visitId,
unknownFields: visitToUpdate.unknownVisitFields,
});
}
});
},
failedVisit => {
this._log.info(
"Failed to insert a history record",
@ -256,6 +311,12 @@ HistoryStore.prototype = {
this._log.info("Failed to insert history records", ex);
countTelemetry.addIncomingFailedReason(ex.message);
}
// All the top level places or visits that had unknown fields are sent
// to be added to the appropiate tables
await lazy.PlacesSyncUtils.history.updateUnknownFieldsBatch(
unknownFieldsToInsert
);
}
}
@ -478,6 +539,14 @@ HistoryStore.prototype = {
record.histUri = foo.url;
record.title = foo.title;
record.sortindex = foo.frecency;
// If we had any unknown fields, ensure we put it back on the
// top-level record
if (foo.unknownFields) {
let unknownFields = JSON.parse(foo.unknownFields);
Object.assign(record.cleartext, unknownFields);
}
try {
record.visits = await lazy.PlacesSyncUtils.history.fetchVisitsForURL(
record.histUri

102
services/sync/tests/unit/test_history_engine.js
Просмотреть файл

@ -325,3 +325,105 @@ add_task(async function test_history_visit_dedupe_old() {
await engine.wipeClient();
await engine.finalize();
});
add_task(async function test_history_unknown_fields() {
let engine = new HistoryEngine(Service);
await engine.initialize();
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
engine._tracker.start();
let id = "aaaaaaaaaaaa";
let oneHourMS = 60 * 60 * 1000;
// Insert a visit with a non-round microsecond timestamp (e.g. it's not evenly
// divisible by 1000). This will typically be the case for visits that occur
// during normal navigation.
let time = (Date.now() - oneHourMS) * 1000 + 555;
// We use the low level history api since it lets us provide microseconds
let { count } = await rawAddVisit(
id,
"https://www.example.com",
time,
PlacesUtils.history.TRANSITIONS.TYPED
);
equal(count, 1);
let collection = server.user("foo").collection("history");
// Sync the visit up to the server.
await sync_engine_and_validate_telem(engine, false);
collection.updateRecord(
id,
cleartext => {
equal(cleartext.visits[0].date, time);
// Add unknown fields to an instance of a visit
cleartext.visits.push({
date: (Date.now() - oneHourMS / 2) * 1000,
type: PlacesUtils.history.TRANSITIONS.LINK,
unknownVisitField: "an unknown field could show up in a visit!",
});
cleartext.title = "A page title";
// Add unknown fields to the payload for this URL
cleartext.unknownStrField = "an unknown str field";
cleartext.unknownObjField = { newField: "a field within an object" };
},
new_timestamp() + 10
);
// Force a remote sync.
await engine.setLastSync(new_timestamp() - 30);
await sync_engine_and_validate_telem(engine, false);
// Add a new visit to ensure we're actually putting things back on the server
let newTime = (Date.now() - oneHourMS) * 1000 + 555;
await rawAddVisit(
id,
"https://www.example.com",
newTime,
PlacesUtils.history.TRANSITIONS.LINK
);
// Sync again
await engine.setLastSync(new_timestamp() - 30);
await sync_engine_and_validate_telem(engine, false);
let placeInfo = await PlacesSyncUtils.history.fetchURLInfoForGuid(id);
// Found the place we're looking for
Assert.equal(placeInfo.title, "A page title");
Assert.equal(placeInfo.url, "https://www.example.com/");
// It correctly returns any unknownFields that might've been
// stored in the moz_places_extra table
deepEqual(JSON.parse(placeInfo.unknownFields), {
unknownStrField: "an unknown str field",
unknownObjField: { newField: "a field within an object" },
});
// Getting visits via SyncUtils also will return unknownFields
// via the moz_historyvisits_extra table
let visits = await PlacesSyncUtils.history.fetchVisitsForURL(
"https://www.example.com"
);
equal(visits.length, 3);
// fetchVisitsForURL is a sync method that gets called during upload
// so unknown field should already be at the top-level
deepEqual(
visits[0].unknownVisitField,
"an unknown field could show up in a visit!"
);
// Remote history record should have the fields back at the top level
let remotePlace = collection.payloads().find(rec => rec.id === id);
deepEqual(remotePlace.unknownStrField, "an unknown str field");
deepEqual(remotePlace.unknownObjField, {
newField: "a field within an object",
});
await engine.wipeClient();
await engine.finalize();
});

185
services/sync/tests/unit/test_history_store.js
Просмотреть файл

@ -83,14 +83,10 @@ add_task(async function test_store() {
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
};
let onVisitObserved = promiseOnVisitObserved();
await applyEnsureNoFailures([
{
id: fxguid,
histUri: record.histUri,
title: "Hol Dir Firefox!",
visits: [record.visits[0], secondvisit],
},
]);
let updatedRec = await store.createRecord(fxguid);
updatedRec.cleartext.title = "Hol Dir Firefox!";
updatedRec.cleartext.visits.push(secondvisit);
await applyEnsureNoFailures([updatedRec]);
await onVisitObserved;
let queryres = await PlacesUtils.history.fetch(fxuri.spec, {
includeVisits: true,
@ -114,16 +110,16 @@ add_task(async function test_store_create() {
tbguid = Utils.makeGUID();
tburi = CommonUtils.makeURI("http://getthunderbird.com");
let onVisitObserved = promiseOnVisitObserved();
await applyEnsureNoFailures([
{
id: tbguid,
histUri: tburi.spec,
title: "The bird is the word!",
visits: [
{ date: TIMESTAMP3, type: Ci.nsINavHistoryService.TRANSITION_TYPED },
],
},
]);
let record = await store.createRecord(tbguid);
record.cleartext = {
id: tbguid,
histUri: tburi.spec,
title: "The bird is the word!",
visits: [
{ date: TIMESTAMP3, type: Ci.nsINavHistoryService.TRANSITION_TYPED },
],
};
await applyEnsureNoFailures([record]);
await onVisitObserved;
Assert.ok(await store.itemExists(tbguid));
do_check_attribute_count(await store.getAllIDs(), 1);
@ -146,16 +142,16 @@ add_task(async function test_null_title() {
);
let resguid = Utils.makeGUID();
let resuri = CommonUtils.makeURI("unknown://title");
await applyEnsureNoFailures([
{
id: resguid,
histUri: resuri.spec,
title: null,
visits: [
{ date: TIMESTAMP3, type: Ci.nsINavHistoryService.TRANSITION_TYPED },
],
},
]);
let record = await store.createRecord(resguid);
record.cleartext = {
id: resguid,
histUri: resuri.spec,
title: null,
visits: [
{ date: TIMESTAMP3, type: Ci.nsINavHistoryService.TRANSITION_TYPED },
],
};
await applyEnsureNoFailures([record]);
do_check_attribute_count(await store.getAllIDs(), 1);
let queryres = await PlacesUtils.history.fetch(resuri.spec, {
@ -331,20 +327,20 @@ add_task(async function test_unknowingly_invalid_records() {
_("Make sure that when places rejects this record we record it as failed");
let guid = Utils.makeGUID();
let countTelemetry = new SyncedRecordsTelemetry();
let result = await store.applyIncomingBatch(
[
let invalidRecord = await store.createRecord(guid);
invalidRecord.cleartext = {
id: guid,
histUri: "javascript:''",
title: "javascript:''",
visits: [
{
id: guid,
histUri: "javascript:''",
title: "javascript:''",
visits: [
{
date: TIMESTAMP3,
type: Ci.nsINavHistoryService.TRANSITION_EMBED,
},
],
date: TIMESTAMP3,
type: Ci.nsINavHistoryService.TRANSITION_EMBED,
},
],
};
let result = await store.applyIncomingBatch(
[invalidRecord],
countTelemetry
);
deepEqual(result, [guid]);
@ -357,60 +353,63 @@ add_task(async function test_clamp_visit_dates() {
let futureVisitTime = Date.now() + 5 * 60 * 1000;
let recentVisitTime = Date.now() - 5 * 60 * 1000;
await applyEnsureNoFailures([
{
id: "visitAAAAAAA",
histUri: "http://example.com/a",
title: "A",
visits: [
{
date: "invalidDate",
type: Ci.nsINavHistoryService.TRANSITION_LINK,
},
],
},
{
id: "visitBBBBBBB",
histUri: "http://example.com/b",
title: "B",
visits: [
{
date: 100,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
{
date: 250,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
{
date: recentVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
],
},
{
id: "visitCCCCCCC",
histUri: "http://example.com/c",
title: "D",
visits: [
{
date: futureVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_BOOKMARK,
},
],
},
{
id: "visitDDDDDDD",
histUri: "http://example.com/d",
title: "D",
visits: [
{
date: recentVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_DOWNLOAD,
},
],
},
]);
let recordA = await store.createRecord("visitAAAAAAA");
recordA.cleartext = {
id: "visitAAAAAAA",
histUri: "http://example.com/a",
title: "A",
visits: [
{
date: "invalidDate",
type: Ci.nsINavHistoryService.TRANSITION_LINK,
},
],
};
let recordB = await store.createRecord("visitBBBBBBB");
recordB.cleartext = {
id: "visitBBBBBBB",
histUri: "http://example.com/b",
title: "B",
visits: [
{
date: 100,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
{
date: 250,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
{
date: recentVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_TYPED,
},
],
};
let recordC = await store.createRecord("visitCCCCCCC");
recordC.cleartext = {
id: "visitCCCCCCC",
histUri: "http://example.com/c",
title: "D",
visits: [
{
date: futureVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_BOOKMARK,
},
],
};
let recordD = await store.createRecord("visitDDDDDDD");
recordD.cleartext = {
id: "visitDDDDDDD",
histUri: "http://example.com/d",
title: "D",
visits: [
{
date: recentVisitTime * 1000,
type: Ci.nsINavHistoryService.TRANSITION_DOWNLOAD,
},
],
};
await applyEnsureNoFailures([recordA, recordB, recordC, recordD]);
let visitsForA = await PlacesSyncUtils.history.fetchVisitsForURL(
"http://example.com/a"

32
toolkit/components/places/Database.cpp
Просмотреть файл

@ -1286,6 +1286,13 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) {
// Firefox 115 uses schema version 74
if (currentSchemaVersion < 75) {
rv = MigrateV75Up();
NS_ENSURE_SUCCESS(rv, rv);
}
// Firefox 118 uses schema version 75
// Schema Upgrades must add migration code here.
// >>> IMPORTANT! <<<
// NEVER MIX UP SYNC AND ASYNC EXECUTION IN MIGRATORS, YOU MAY LOCK THE
@ -1303,6 +1310,8 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) {
// moz_places.
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_PLACES);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_PLACES_EXTRA);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_PLACES_URL_HASH);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_PLACES_REVHOST);
@ -1323,6 +1332,8 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) {
// moz_historyvisits.
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_HISTORYVISITS);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_HISTORYVISITS_EXTRA);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_HISTORYVISITS_PLACEDATE);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_HISTORYVISITS_FROMVISIT);
@ -1721,6 +1732,13 @@ nsresult Database::InitTempEntities() {
rv = mMainConn->ExecuteSimpleSQL(CREATE_PLACES_METADATA_AFTERDELETE_TRIGGER);
NS_ENSURE_SUCCESS(rv, rv);
// Create triggers to remove rows with empty json
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_PLACES_EXTRA_AFTERUPDATE_TRIGGER);
NS_ENSURE_SUCCESS(rv, rv);
rv =
mMainConn->ExecuteSimpleSQL(CREATE_MOZ_HISTORYVISITS_AFTERUPDATE_TRIGGER);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
@ -2547,6 +2565,20 @@ nsresult Database::MigrateV74Up() {
return NS_OK;
}
nsresult Database::MigrateV75Up() {
// Add *_extra tables for moz_places and moz_historyvisits
nsCOMPtr<mozIStorageStatement> stmt;
nsresult rv = mMainConn->CreateStatement(
"SELECT sync_json FROM moz_places_extra"_ns, getter_AddRefs(stmt));
if (NS_FAILED(rv)) {
nsresult rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_PLACES_EXTRA);
NS_ENSURE_SUCCESS(rv, rv);
rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_HISTORYVISITS_EXTRA);
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
nsresult Database::RecalculateOriginFrecencyStatsInternal() {
return mMainConn->ExecuteSimpleSQL(nsLiteralCString(
"INSERT OR REPLACE INTO moz_meta(key, value) VALUES "

3
toolkit/components/places/Database.h
Просмотреть файл

@ -18,7 +18,7 @@
// This is the schema version. Update it at any schema change and add a
// corresponding migrateVxx method below.
#define DATABASE_SCHEMA_VERSION 74
#define DATABASE_SCHEMA_VERSION 75
// Fired after Places inited.
#define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
@ -319,6 +319,7 @@ class Database final : public nsIObserver, public nsSupportsWeakReference {
nsresult MigrateV72Up();
nsresult MigrateV73Up();
nsresult MigrateV74Up();
nsresult MigrateV75Up();
void MigrateV52OriginFrecencies();

2
toolkit/components/places/History.sys.mjs
Просмотреть файл

@ -1542,8 +1542,10 @@ function mergeUpdateInfoIntoPageInfo(updateInfo, pageInfo = {}) {
if (!pageInfo.url) {
pageInfo.url = URL.fromURI(updateInfo.uri);
pageInfo.title = updateInfo.title;
pageInfo.placeId = updateInfo.placeId;
pageInfo.visits = updateInfo.visits.map(visit => {
return {
visitId: visit.visitId,
date: lazy.PlacesUtils.toDate(visit.visitDate),
transition: visit.transitionType,
referrer: visit.referrerURI ? URL.fromURI(visit.referrerURI) : null,

116
toolkit/components/places/PlacesSyncUtils.sys.mjs
Просмотреть файл

@ -298,9 +298,11 @@ const HistorySyncUtils = (PlacesSyncUtils.history = Object.freeze({
let db = await lazy.PlacesUtils.promiseDBConnection();
let rows = await db.executeCached(
`
SELECT visit_type type, visit_date date
FROM moz_historyvisits
JOIN moz_places h ON h.id = place_id
SELECT visit_type type, visit_date date,
json_extract(e.sync_json, '$.unknown_sync_fields') as unknownSyncFields
FROM moz_historyvisits v
JOIN moz_places h ON h.id = v.place_id
LEFT OUTER JOIN moz_historyvisits_extra e ON e.visit_id = v.id
WHERE url_hash = hash(:url) AND url = :url
ORDER BY date DESC LIMIT 20`,
{ url: canonicalURL.href }
@ -308,7 +310,20 @@ const HistorySyncUtils = (PlacesSyncUtils.history = Object.freeze({
return rows.map(row => {
let visitDate = row.getResultByName("date");
let visitType = row.getResultByName("type");
return { date: visitDate, type: visitType };
let visit = { date: visitDate, type: visitType };
// We should grab unknown fields to roundtrip them
// back to the server
let unknownFields = row.getResultByName("unknownSyncFields");
if (unknownFields) {
let unknownFieldsObj = JSON.parse(unknownFields);
for (const key in unknownFieldsObj) {
// We have to manually add it to the cleartext since that's
// what gets processed during upload
visit[key] = unknownFieldsObj[key];
}
}
return visit;
});
},
@ -344,19 +359,29 @@ const HistorySyncUtils = (PlacesSyncUtils.history = Object.freeze({
let db = await lazy.PlacesUtils.promiseDBConnection();
let rows = await db.executeCached(
`
SELECT url, IFNULL(title, '') AS title, frecency
FROM moz_places
SELECT url, IFNULL(title, '') AS title, frecency,
json_extract(e.sync_json, '$.unknown_sync_fields') as unknownSyncFields
FROM moz_places h
LEFT OUTER JOIN moz_places_extra e ON e.place_id = h.id
WHERE guid = :guid`,
{ guid }
);
if (rows.length === 0) {
return null;
}
return {
let info = {
url: rows[0].getResultByName("url"),
title: rows[0].getResultByName("title"),
frecency: rows[0].getResultByName("frecency"),
};
let unknownFields = rows[0].getResultByName("unknownSyncFields");
if (unknownFields) {
// This will be unfurled at the caller since the
// cleartext process will drop this
info.unknownFields = unknownFields;
}
return info;
},
/**
@ -399,6 +424,55 @@ const HistorySyncUtils = (PlacesSyncUtils.history = Object.freeze({
);
return rows.map(row => row.getResultByName("url"));
},
/**
* Insert or update the unknownFields that this client doesn't understand (yet)
* but stores & roundtrips them to prevent other clients from losing that data
*
* @param updates array of objects
* an update object needs to have either a:
* placeId: if we're putting unknownFields for a moz_places item
* visitId: if we're putting unknownFields for a moz_historyvisits item
* Note: Supplying none or both will result in that record being ignored
* unknownFields: the stringified json to insert
*/
async updateUnknownFieldsBatch(updates) {
return lazy.PlacesUtils.withConnectionWrapper(
"HistorySyncUtils: updateUnknownFieldsBatch",
async function (db) {
await db.executeTransaction(async () => {
for await (const update of updates) {
// Validate we only have one of these props
if (
(update.placeId && update.visitId) ||
(!update.placeId && !update.visitId)
) {
continue;
}
let tableName = update.placeId
? "moz_places_extra"
: "moz_historyvisits_extra";
let keyName = update.placeId ? "place_id" : "visit_id";
await db.executeCached(
`
INSERT INTO ${tableName} (${keyName}, sync_json)
VALUES (
:keyValue,
json_object('unknown_sync_fields', :unknownFields)
)
ON CONFLICT(${keyName}) DO UPDATE SET
sync_json=json_patch(${tableName}.sync_json, json_object('unknown_sync_fields',:unknownFields))
`,
{
keyValue: update.placeId ?? update.visitId,
unknownFields: update.unknownFields,
}
);
}
});
}
);
},
// End of history freeze
}));
const BookmarkSyncUtils = (PlacesSyncUtils.bookmarks = Object.freeze({
@ -1992,3 +2066,31 @@ async function resetAllSyncStatuses(db, syncStatus) {
// Drop stale tombstones.
await db.execute("DELETE FROM moz_bookmarks_deleted");
}
/**
* Other clients might have new fields we don't quite understand yet,
* so we add it to a "unknownFields" field to roundtrip back to the server
* so other clients don't experience data loss
* @param record: an object, usually from the server, and will iterate through the
* the keys and extract any fields that are unknown to this client
* @param validFields: an array of keys we know are valid and should ignore
* @returns {String} json object containing unknownfields, null if none found
*/
PlacesSyncUtils.extractUnknownFields = (record, validFields) => {
let { unknownFields, hasUnknownFields } = Object.keys(record).reduce(
({ unknownFields, hasUnknownFields }, key) => {
if (validFields.includes(key)) {
return { unknownFields, hasUnknownFields };
}
unknownFields[key] = record[key];
return { unknownFields, hasUnknownFields: true };
},
{ unknownFields: {}, hasUnknownFields: false }
);
if (hasUnknownFields) {
// For simplicity, we store the unknown fields as a string
// since we never operate on it and just need it for roundtripping
return JSON.stringify(unknownFields);
}
return null;
};

104
toolkit/components/places/SyncedBookmarksMirror.sys.mjs
Просмотреть файл

@ -804,14 +804,17 @@ export class SyncedBookmarksMirror {
? Ci.mozISyncedBookmarksMerger.VALIDITY_VALID
: Ci.mozISyncedBookmarksMerger.VALIDITY_REPLACE;
let unknownFields = extractUnknownFields(record.cleartext, [
"bmkUri",
"description",
"keyword",
"tags",
"title",
...COMMON_UNKNOWN_FIELDS,
]);
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
[
"bmkUri",
"description",
"keyword",
"tags",
"title",
...COMMON_UNKNOWN_FIELDS,
]
);
await this.db.executeCached(
`
REPLACE INTO items(guid, parentGuid, serverModified, needsMerge, kind,
@ -930,16 +933,19 @@ export class SyncedBookmarksMirror {
let dateAdded = determineDateAdded(record);
let title = validateTitle(record.title);
let unknownFields = extractUnknownFields(record.cleartext, [
"bmkUri",
"description",
"folderName",
"keyword",
"queryId",
"tags",
"title",
...COMMON_UNKNOWN_FIELDS,
]);
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
[
"bmkUri",
"description",
"folderName",
"keyword",
"queryId",
"tags",
"title",
...COMMON_UNKNOWN_FIELDS,
]
);
await this.db.executeCached(
`
@ -976,12 +982,10 @@ export class SyncedBookmarksMirror {
let serverModified = determineServerModified(record);
let dateAdded = determineDateAdded(record);
let title = validateTitle(record.title);
let unknownFields = extractUnknownFields(record.cleartext, [
"children",
"description",
"title",
...COMMON_UNKNOWN_FIELDS,
]);
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
["children", "description", "title", ...COMMON_UNKNOWN_FIELDS]
);
await this.db.executeCached(
`
REPLACE INTO items(guid, parentGuid, serverModified, needsMerge, kind,
@ -1047,14 +1051,17 @@ export class SyncedBookmarksMirror {
? Ci.mozISyncedBookmarksMerger.VALIDITY_VALID
: Ci.mozISyncedBookmarksMerger.VALIDITY_REPLACE;
let unknownFields = extractUnknownFields(record.cleartext, [
"children",
"description",
"feedUri",
"siteUri",
"title",
...COMMON_UNKNOWN_FIELDS,
]);
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
[
"children",
"description",
"feedUri",
"siteUri",
"title",
...COMMON_UNKNOWN_FIELDS,
]
);
await this.db.executeCached(
`
@ -1085,10 +1092,10 @@ export class SyncedBookmarksMirror {
);
let serverModified = determineServerModified(record);
let dateAdded = determineDateAdded(record);
let unknownFields = extractUnknownFields(record.cleartext, [
"pos",
...COMMON_UNKNOWN_FIELDS,
]);
let unknownFields = lazy.PlacesSyncUtils.extractUnknownFields(
record.cleartext,
["pos", ...COMMON_UNKNOWN_FIELDS]
);
await this.db.executeCached(
`
@ -2601,29 +2608,4 @@ const COMMON_UNKNOWN_FIELDS = [
"type",
];
// Other clients might have new fields we don't quite understand yet,
// so we add it to a "unknownFields" field to roundtrip back to the server
// so other clients don't experience data loss
function extractUnknownFields(record, validFields) {
let { unknownFields, hasUnknownFields } = Object.keys(record).reduce(
({ unknownFields, hasUnknownFields }, key) => {
if (validFields.includes(key)) {
return { unknownFields, hasUnknownFields };
}
unknownFields[key] = record[key];
return { unknownFields, hasUnknownFields: true };
},
{ unknownFields: {}, hasUnknownFields: false }
);
// If we found some unknown fields, we stringify it to be able
// to properly encrypt it for roundtripping since we can't know if
// it contained sensitive fields or not
if (hasUnknownFields) {
// For simplicity, we store the unknown fields as a string
// since we never operate on it and just need it for roundtripping
return JSON.stringify(unknownFields);
}
return null;
}
// In conclusion, this is why bookmark syncing is hard.

22
toolkit/components/places/nsPlacesTables.h
Просмотреть файл

@ -44,6 +44,28 @@
", triggeringPlaceId INTEGER" \
")")
// These two tables were designed to store data with json in mind
// ideally one column per "consumer" (sync, annotations, etc) to keep
// concerns separate. Using an UPSERT is the suggested way to update
// this table vs INSERT OR REPLACE to avoid clearing out any existing properties
// see PlacesSyncUtils.sys.mjs for an example of how sync does this
#define CREATE_MOZ_PLACES_EXTRA \
nsLiteralCString( \
"CREATE TABLE moz_places_extra (" \
" place_id INTEGER PRIMARY KEY NOT NULL" \
", sync_json TEXT" \
", FOREIGN KEY (place_id) REFERENCES moz_places(id) ON DELETE CASCADE " \
")")
#define CREATE_MOZ_HISTORYVISITS_EXTRA \
nsLiteralCString( \
"CREATE TABLE moz_historyvisits_extra (" \
" visit_id INTEGER PRIMARY KEY NOT NULL" \
", sync_json TEXT" \
", FOREIGN KEY (visit_id) REFERENCES moz_historyvisits(id) ON " \
" DELETE CASCADE" \
")")
#define CREATE_MOZ_INPUTHISTORY \
nsLiteralCString( \
"CREATE TABLE moz_inputhistory (" \

21
toolkit/components/places/nsPlacesTriggers.h
Просмотреть файл

@ -420,4 +420,25 @@
"); " \
"END")
// since moz_places_extra is really just storing json, there could be a
// scenario where we have a valid row but empty json -- we should make sure
// we have triggers to remove any such rows
# define CREATE_MOZ_PLACES_EXTRA_AFTERUPDATE_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_places_extra_trigger " \
"AFTER UPDATE ON moz_places_extra FOR EACH ROW " \
"WHEN (NEW.sync_json = '' OR NEW.sync_json = '{}')" \
"BEGIN " \
"DELETE FROM moz_places_extra WHERE place_id = NEW.place_id;" \
"END")
# define CREATE_MOZ_HISTORYVISITS_AFTERUPDATE_TRIGGER \
nsLiteralCString( \
"CREATE TEMP TRIGGER moz_historyvisits_extra_trigger " \
"AFTER UPDATE ON moz_historyvisits_extra FOR EACH ROW " \
"WHEN (NEW.sync_json = '' OR NEW.sync_json = '{}')" \
"BEGIN " \
"DELETE FROM moz_historyvisits_extra WHERE visit_id = NEW.visit_id;" \
"END")
#endif // __nsPlacesTriggers_h__

2
toolkit/components/places/tests/migration/head_migration.js
Просмотреть файл

@ -13,7 +13,7 @@
// Put any other stuff relative to this test folder below.
const CURRENT_SCHEMA_VERSION = 74;
const CURRENT_SCHEMA_VERSION = 75;
const FIRST_UPGRADABLE_SCHEMA_VERSION = 43;
async function assertAnnotationsRemoved(db, expectedAnnos) {

Двоичные данные
toolkit/components/places/tests/migration/places_v75.sqlite Normal file
Просмотреть файл

Двоичный файл не отображается.

22
toolkit/components/places/tests/migration/test_current_from_v74.js Normal file
Просмотреть файл

@ -0,0 +1,22 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
add_task(async function setup() {
await setupPlacesDatabase("places_v74.sqlite");
});
add_task(async function database_is_valid() {
// Accessing the database for the first time triggers migration.
Assert.equal(
PlacesUtils.history.databaseStatus,
PlacesUtils.history.DATABASE_STATUS_UPGRADED
);
const db = await PlacesUtils.promiseDBConnection();
Assert.equal(await db.getSchemaVersion(), CURRENT_SCHEMA_VERSION);
await db.execute("SELECT * FROM moz_places_extra");
await db.execute("SELECT * from moz_historyvisits_extra");
});

2
toolkit/components/places/tests/migration/xpcshell.ini
Просмотреть файл

@ -14,6 +14,7 @@ support-files =
places_v70.sqlite
places_v72.sqlite
places_v74.sqlite
places_v75.sqlite
[test_current_from_downgraded.js]
[test_current_from_outdated.js]
@ -30,3 +31,4 @@ support-files =
[test_current_from_v69.js]
[test_current_from_v70.js]
[test_current_from_v72.js]
[test_current_from_v74.js]

55
toolkit/components/places/tests/sync/test_sync_utils.js
Просмотреть файл

@ -3072,3 +3072,58 @@ add_task(async function test_history_ensureCurrentSyncId() {
await PlacesSyncUtils.history.reset();
});
add_task(async function test_updateUnknownFieldsBatch() {
// We're just validating we have something where placeId = 1, mainly as a sanity
// since moz_places_extra needs a valid foreign key
let placeId = await PlacesTestUtils.getDatabaseValue("moz_places", "id", {
id: 1,
});
// an example of json with multiple fields in it to test updateUnknownFields
// will update ONLY unknown_sync_fields and not override any others
const test_json = JSON.stringify({
unknown_sync_fields: { unknownStrField: "an old str field " },
extra_str_field: "another field within the json",
extra_obj_field: { inner: "hi" },
});
// Manually put the inital json in the DB
await PlacesUtils.withConnectionWrapper(
"test_update_moz_places_extra",
async function (db) {
await db.executeCached(
`
INSERT INTO moz_places_extra(place_id, sync_json)
VALUES(:placeId, :sync_json)`,
{ placeId, sync_json: test_json }
);
}
);
// call updateUnknownFieldsBatch to validate it ONLY updates
// the unknown_sync_fields in the sync_json
let update = {
placeId,
unknownFields: JSON.stringify({ unknownStrField: "a new unknownStrField" }),
};
await PlacesSyncUtils.history.updateUnknownFieldsBatch([update]);
let updated_sync_json = await PlacesTestUtils.getDatabaseValue(
"moz_places_extra",
"sync_json",
{
place_id: placeId,
}
);
let updated_data = JSON.parse(updated_sync_json);
// unknown_sync_fields has been updated
deepEqual(JSON.parse(updated_data.unknown_sync_fields), {
unknownStrField: "a new unknownStrField",
});
// we didn't override any other fields within
deepEqual(updated_data.extra_str_field, "another field within the json");
});