зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1451537 - Simplify tag query rewriting in the bookmarks mirror. r=mak,markh
MozReview-Commit-ID: 920003s7Jvm --HG-- extra : rebase_source : 7f62f4f283e6d1c065651adfebe97038108504ac
This commit is contained in:
Kit Cambridge
Родитель
0a5d99fe57
Коммит
2ea6b739bc
|
|
@ -648,28 +648,43 @@ class SyncedBookmarksMirror {
|
|||
return;
|
||||
}
|
||||
|
||||
// Legacy tag queries may use a `place:` URL with a `folder` param that
|
||||
// points to the tag folder ID. We need to rewrite these queries to
|
||||
// directly reference the tag.
|
||||
let params = new URLSearchParams(url.pathname);
|
||||
let type = +params.get("type");
|
||||
if (type == Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS) {
|
||||
let tagFolderName = validateTag(record.folderName);
|
||||
if (!tagFolderName) {
|
||||
MirrorLog.warn("Ignoring tag query ${guid} with invalid tag name " +
|
||||
"${tagFolderName}", { guid, tagFolderName });
|
||||
ignoreCounts.query.url++;
|
||||
return;
|
||||
}
|
||||
url = new URL(`place:tag=${tagFolderName}`);
|
||||
}
|
||||
|
||||
await this.maybeStoreRemoteURL(url);
|
||||
|
||||
let serverModified = determineServerModified(record);
|
||||
let dateAdded = determineDateAdded(record);
|
||||
let title = validateTitle(record.title);
|
||||
let tagFolderName = validateTag(record.folderName);
|
||||
let description = validateDescription(record.description);
|
||||
let smartBookmarkName = typeof record.queryId == "string" ?
|
||||
record.queryId : null;
|
||||
|
||||
await this.db.executeCached(`
|
||||
REPLACE INTO items(guid, serverModified, needsMerge, kind,
|
||||
dateAdded, title, tagFolderName,
|
||||
urlId, description, smartBookmarkName)
|
||||
dateAdded, title, urlId, description,
|
||||
smartBookmarkName)
|
||||
VALUES(:guid, :serverModified, :needsMerge, :kind,
|
||||
:dateAdded, NULLIF(:title, ""), :tagFolderName,
|
||||
:dateAdded, NULLIF(:title, ""),
|
||||
(SELECT id FROM urls
|
||||
WHERE hash = hash(:url) AND
|
||||
url = :url),
|
||||
:description, :smartBookmarkName)`,
|
||||
{ guid, serverModified, needsMerge,
|
||||
kind: SyncedBookmarksMirror.KIND.QUERY, dateAdded, title, tagFolderName,
|
||||
kind: SyncedBookmarksMirror.KIND.QUERY, dateAdded, title,
|
||||
url: url.href, description, smartBookmarkName });
|
||||
}
|
||||
|
||||
|
|
@ -1213,7 +1228,7 @@ class SyncedBookmarksMirror {
|
|||
|
||||
/**
|
||||
* Builds a temporary table with the merge states of all nodes in the merged
|
||||
* tree, rewrites tag queries, and updates Places to match the merged tree.
|
||||
* tree and updates Places to match the merged tree.
|
||||
*
|
||||
* Conceptually, we examine the merge state of each item, and either keep the
|
||||
* complete local state, take the complete remote state, or apply a new
|
||||
|
|
@ -1247,9 +1262,6 @@ class SyncedBookmarksMirror {
|
|||
mergeStatesParams);
|
||||
}
|
||||
|
||||
MirrorLog.debug("Rewriting tag queries in mirror");
|
||||
await this.rewriteRemoteTagQueries();
|
||||
|
||||
MirrorLog.debug("Inserting new URLs into Places");
|
||||
await this.db.execute(`
|
||||
INSERT OR IGNORE INTO moz_places(url, url_hash, rev_host, hidden,
|
||||
|
|
@ -1350,55 +1362,6 @@ class SyncedBookmarksMirror {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Rewrites tag query URLs in the mirror to point to the tag.
|
||||
*
|
||||
* For example, an incoming tag query of, say, "place:type=7&folder=3" means
|
||||
* it is a query for whatever tag was defined by the local record with id=3
|
||||
* (and the incoming record has the actual tag in the folderName field).
|
||||
*/
|
||||
async rewriteRemoteTagQueries() {
|
||||
let queryRows = await this.db.execute(`
|
||||
SELECT u.id AS urlId, u.url, v.tagFolderName
|
||||
FROM urls u
|
||||
JOIN items v ON v.urlId = u.id
|
||||
JOIN mergeStates r ON r.mergedGuid = v.guid
|
||||
WHERE r.valueState = :valueState AND
|
||||
v.kind = :queryKind AND
|
||||
v.tagFolderName NOT NULL AND
|
||||
CAST(get_query_param(substr(u.url, 7), "type") AS INT) = :tagContentsType
|
||||
`, { valueState: BookmarkMergeState.TYPE.REMOTE,
|
||||
queryKind: SyncedBookmarksMirror.KIND.QUERY,
|
||||
tagContentsType: Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS });
|
||||
|
||||
let urlsParams = [];
|
||||
for (let row of queryRows) {
|
||||
let url = new URL(row.getResultByName("url"));
|
||||
let tagQueryParams = new URLSearchParams(url.pathname);
|
||||
|
||||
// Rewrite the query to directly reference the tag.
|
||||
tagQueryParams.delete("queryType");
|
||||
tagQueryParams.delete("type");
|
||||
tagQueryParams.delete("folder");
|
||||
tagQueryParams.set("tag", row.getResultByName("tagFolderName"));
|
||||
|
||||
let newURLHref = url.protocol + tagQueryParams;
|
||||
urlsParams.push({
|
||||
urlId: row.getResultByName("urlId"),
|
||||
url: newURLHref,
|
||||
});
|
||||
}
|
||||
|
||||
if (urlsParams.length) {
|
||||
await this.db.execute(`
|
||||
UPDATE urls SET
|
||||
url = :url,
|
||||
hash = hash(:url)
|
||||
WHERE id = :urlId`,
|
||||
urlsParams);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Records Places observer notifications for removed, added, moved, and
|
||||
* changed items.
|
||||
|
|
@ -1975,7 +1938,6 @@ async function initializeMirrorDatabase(db) {
|
|||
urlId INTEGER REFERENCES urls(id)
|
||||
ON DELETE SET NULL,
|
||||
keyword TEXT,
|
||||
tagFolderName TEXT,
|
||||
description TEXT,
|
||||
loadInSidebar BOOLEAN,
|
||||
smartBookmarkName TEXT,
|
||||
|
|
@ -2440,8 +2402,8 @@ async function initializeTempMirrorEntities(db) {
|
|||
// keywords, one tag may be associated with many different URLs. Tags are also
|
||||
// different because they're implemented as bookmarks under the hood. Each tag
|
||||
// is stored as a folder under the tags root, and tagged URLs are stored as
|
||||
// untitled bookmarks under these folders. This complexity, along with tag
|
||||
// query rewriting, can be removed once bug 424160 lands.
|
||||
// untitled bookmarks under these folders. This complexity can be removed once
|
||||
// bug 424160 lands.
|
||||
await db.execute(`
|
||||
CREATE TEMP VIEW localTags(tagEntryId, tagEntryGuid, tagFolderId,
|
||||
tagFolderGuid, tagEntryPosition, tagEntryType,
|
||||
|
|
|
|||
|
|
@ -258,64 +258,117 @@ add_task(async function test_livemarks() {
|
|||
});
|
||||
|
||||
add_task(async function test_queries() {
|
||||
try {
|
||||
let buf = await openMirror("queries");
|
||||
let buf = await openMirror("queries");
|
||||
|
||||
info("Set up places");
|
||||
info("Set up places");
|
||||
|
||||
// create a tag and grab the local folder ID.
|
||||
let tag = await PlacesUtils.bookmarks.insert({
|
||||
type: PlacesUtils.bookmarks.TYPE_FOLDER,
|
||||
parentGuid: PlacesUtils.bookmarks.tagsGuid,
|
||||
title: "a-tag",
|
||||
});
|
||||
// create a tag and grab the local folder ID.
|
||||
let tag = await PlacesUtils.bookmarks.insert({
|
||||
type: PlacesUtils.bookmarks.TYPE_FOLDER,
|
||||
parentGuid: PlacesUtils.bookmarks.tagsGuid,
|
||||
title: "a-tag",
|
||||
});
|
||||
|
||||
await PlacesTestUtils.markBookmarksAsSynced();
|
||||
await PlacesTestUtils.markBookmarksAsSynced();
|
||||
|
||||
await PlacesUtils.bookmarks.insertTree({
|
||||
guid: PlacesUtils.bookmarks.menuGuid,
|
||||
children: [
|
||||
{
|
||||
// this entry has a tag= query param for a tag that exists.
|
||||
guid: "queryAAAAAAA",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY query",
|
||||
url: `place:tag=a-tag&&sort=14&maxResults=10`,
|
||||
},
|
||||
{
|
||||
// this entry has a tag= query param for a tag that doesn't exist.
|
||||
guid: "queryBBBBBBB",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY query but invalid folder id",
|
||||
url: `place:tag=b-tag&sort=14&maxResults=10`,
|
||||
},
|
||||
{
|
||||
// this entry has no tag= query param.
|
||||
guid: "queryCCCCCCC",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY without a folder at all",
|
||||
url: "place:sort=14&maxResults=10",
|
||||
},
|
||||
{
|
||||
// this entry has only a tag= query.
|
||||
guid: "queryDDDDDDD",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY without a folder at all",
|
||||
url: "place:tag=a-tag",
|
||||
},
|
||||
],
|
||||
});
|
||||
await PlacesUtils.bookmarks.insertTree({
|
||||
guid: PlacesUtils.bookmarks.menuGuid,
|
||||
children: [
|
||||
{
|
||||
// this entry has a tag= query param for a tag that exists.
|
||||
guid: "queryAAAAAAA",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY query",
|
||||
url: `place:tag=a-tag&&sort=14&maxResults=10`,
|
||||
},
|
||||
{
|
||||
// this entry has a tag= query param for a tag that doesn't exist.
|
||||
guid: "queryBBBBBBB",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY query but invalid folder id",
|
||||
url: `place:tag=b-tag&sort=14&maxResults=10`,
|
||||
},
|
||||
{
|
||||
// this entry has no tag= query param.
|
||||
guid: "queryCCCCCCC",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY without a folder at all",
|
||||
url: "place:sort=14&maxResults=10",
|
||||
},
|
||||
{
|
||||
// this entry has only a tag= query.
|
||||
guid: "queryDDDDDDD",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
title: "TAG_QUERY without a folder at all",
|
||||
url: "place:tag=a-tag",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
info("Create records to upload");
|
||||
let changes = await buf.apply();
|
||||
Assert.strictEqual(changes.queryAAAAAAA.cleartext.folderName, tag.title);
|
||||
Assert.strictEqual(changes.queryBBBBBBB.cleartext.folderName, "b-tag");
|
||||
Assert.strictEqual(changes.queryCCCCCCC.cleartext.folderName, undefined);
|
||||
Assert.strictEqual(changes.queryDDDDDDD.cleartext.folderName, tag.title);
|
||||
} finally {
|
||||
await PlacesUtils.bookmarks.eraseEverything();
|
||||
await PlacesSyncUtils.bookmarks.reset();
|
||||
}
|
||||
info("Make remote changes");
|
||||
await storeRecords(buf, shuffle([{
|
||||
id: "toolbar",
|
||||
type: "folder",
|
||||
children: ["queryEEEEEEE", "queryFFFFFFF", "queryGGGGGGG"],
|
||||
}, {
|
||||
// Legacy tag query.
|
||||
id: "queryEEEEEEE",
|
||||
type: "query",
|
||||
title: "E",
|
||||
bmkUri: "place:type=7&folder=999",
|
||||
folderName: "taggy",
|
||||
}, {
|
||||
// New tag query.
|
||||
id: "queryFFFFFFF",
|
||||
type: "query",
|
||||
title: "F",
|
||||
bmkUri: "place:tag=a-tag",
|
||||
folderName: "a-tag",
|
||||
}, {
|
||||
// Legacy tag query referencing the same tag as the new query.
|
||||
id: "queryGGGGGGG",
|
||||
type: "query",
|
||||
title: "G",
|
||||
bmkUri: "place:type=7&folder=111&something=else",
|
||||
folderName: "a-tag",
|
||||
}]));
|
||||
|
||||
info("Create records to upload");
|
||||
let changes = await buf.apply();
|
||||
Assert.strictEqual(changes.queryAAAAAAA.cleartext.folderName, tag.title);
|
||||
Assert.strictEqual(changes.queryBBBBBBB.cleartext.folderName, "b-tag");
|
||||
Assert.strictEqual(changes.queryCCCCCCC.cleartext.folderName, undefined);
|
||||
Assert.strictEqual(changes.queryDDDDDDD.cleartext.folderName, tag.title);
|
||||
|
||||
await assertLocalTree(PlacesUtils.bookmarks.toolbarGuid, {
|
||||
guid: PlacesUtils.bookmarks.toolbarGuid,
|
||||
type: PlacesUtils.bookmarks.TYPE_FOLDER,
|
||||
index: 1,
|
||||
title: BookmarksToolbarTitle,
|
||||
children: [{
|
||||
guid: "queryEEEEEEE",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
index: 0,
|
||||
title: "E",
|
||||
url: "place:tag=taggy",
|
||||
}, {
|
||||
guid: "queryFFFFFFF",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
index: 1,
|
||||
title: "F",
|
||||
url: "place:tag=a-tag",
|
||||
}, {
|
||||
guid: "queryGGGGGGG",
|
||||
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
index: 2,
|
||||
title: "G",
|
||||
url: "place:tag=a-tag",
|
||||
}],
|
||||
}, "Should rewrite legacy remote tag queries");
|
||||
|
||||
await buf.finalize();
|
||||
await PlacesUtils.bookmarks.eraseEverything();
|
||||
await PlacesSyncUtils.bookmarks.reset();
|
||||
});
|
||||
|
||||
// Bug 632287.
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче