Bug 1352502 - Part 2. Add API to update the page meta info for Places. r?mak
MozReview-Commit-ID: KZHOCtEA41n
--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -3,43 +3,62 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
/**
* Asynchronous API for managing history.
*
*
- * The API makes use of `PageInfo` and `VisitInfo` objects, defined as follows.
+ * The API makes use of `PageInfo`, `PageMetaInfo`, and `VisitInfo` objects,
+ * defined as follows.
*
* A `PageInfo` object is any object that contains A SUBSET of the
* following properties:
* - guid: (string)
* The globally unique id of the page.
* - url: (URL)
* or (nsIURI)
* or (string)
* The full URI of the page. Note that `PageInfo` values passed as
* argument may hold `nsIURI` or `string` values for property `url`,
* but `PageInfo` objects returned by this module always hold `URL`
* values.
* - title: (string)
* The title associated with the page, if any.
+ * - description: (string)
+ * The description of the page, if any.
+ * - previewImageURL: (URL)
+ * or (nsIURI)
+ * or (string)
+ * The preview image URL of the page, if any.
* - frecency: (number)
* The frecency of the page, if any.
* See https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Places/Frecency_algorithm
* Note that this property may not be used to change the actualy frecency
* score of a page, only to retrieve it. In other words, any `frecency` field
* passed as argument to a function of this API will be ignored.
* - visits: (Array<VisitInfo>)
* All the visits for this page, if any.
*
* See the documentation of individual methods to find out which properties
* are required for `PageInfo` arguments or returned for `PageInfo` results.
*
+ * A `PageMetaInfo` object is any object that contains at least one of the
+ * following properties:
+ *
+ * - description: (string)
+ * The description of the page.
+ * - previewImageURL: (URL)
+ * or (nsIURI)
+ * or (string)
+ *
+ * See the documentation of individual methods to find out which properties are
+ * required for `PageMetaInfo` arguments or returned for `PageMetaInfo` results.
+ *
* A `VisitInfo` object is any object that contains A SUBSET of the following
* properties:
* - date: (Date)
* The time the visit occurred.
* - transition: (number)
* How the user reached the page. See constants `TRANSITIONS.*`
* for the possible transition types.
* - referrer: (URL)
@@ -86,16 +105,20 @@ Cu.importGlobalProperties(["URL"]);
* may emit before we yield.
*/
const NOTIFICATION_CHUNK_SIZE = 300;
const ONRESULT_CHUNK_SIZE = 300;
// This constant determines the maximum number of remove pages before we cycle.
const REMOVE_PAGES_CHUNKLEN = 300;
+// Imposed to limit database size
+const DB_DESCRIPTION_LENGTH_MAX = 1024;
+// TODO: Replace this constant with Database::MaxUrlLength() once it's exposed.
+const DB_PREVIEW_IMAGE_URL_MAX = 2000;
/**
* Sends a bookmarks notification through the given observers.
*
* @param observers
* array of nsINavBookmarkObserver objects.
* @param notification
* the notification name.
@@ -116,16 +139,18 @@ this.History = Object.freeze({
*
* @param guidOrURI: (string) or (URL, nsIURI or href)
* Either the full URI of the page or the GUID of the page.
* @param [optional] options (object)
* An optional object whose properties describe options:
* - `includeVisits` (boolean) set this to true if `visits` in the
* PageInfo needs to contain VisitInfo in a reverse chronological order.
* By default, `visits` is undefined inside the returned `PageInfo`.
+ * - `includePageMeta` (boolean) set this to true to fetch page meta
+ * related fields, i.e. `description` and `preview_image_url`.
*
* @return (Promise)
* A promise resolved once the operation is complete.
* @resolves (PageInfo | null) If the page could be found, the information
* on that page.
* @note the VisitInfo objects returned while fetching visits do not
* contain the property `referrer`.
* TODO: Add `referrer` to VisitInfo. See Bug #1365913.
@@ -144,16 +169,21 @@ this.History = Object.freeze({
throw new TypeError("options should be an object and not null");
}
let hasIncludeVisits = "includeVisits" in options;
if (hasIncludeVisits && typeof options.includeVisits !== "boolean") {
throw new TypeError("includeVisits should be a boolean if exists");
}
+ let hasIncludePageMeta = "includePageMeta" in options;
+ if (hasIncludePageMeta && typeof options.includePageMeta !== "boolean") {
+ throw new TypeError("includePageMeta should be a boolean if exists");
+ }
+
return PlacesUtils.promiseDBConnection()
.then(db => fetch(db, guidOrURI, options));
},
/**
* Adds a number of visits for a single page.
*
* Any change may be observed through nsINavHistoryObserver
@@ -557,16 +587,69 @@ this.History = Object.freeze({
* Throw if an object is not a Date object.
*/
ensureDate(arg) {
if (!arg || typeof arg != "object" || arg.constructor.name != "Date") {
throw new TypeError("Expected a Date, got " + arg);
}
},
+ /**
+ * Set meta information (description and preview image URL) for a single page.
+ * To get page meta information, use `History.fetch`.
+ *
+ * Note that this function will ignore the update if the target page has not
+ * yet been stored in the database. Again, `History.fetch` could be used to
+ * check whether the page and its meta information exist or not. Beware that
+ * fetch&update might fail as they are not executed in a single transaction.
+ *
+ * @param guidOrURI: (URL)
+ * or (nsIURI)
+ * or (string)
+ * Either the full URI of the page or the GUID of the page.
+ * @param pageMetaInfo: (PageMetaInfo)
+ * If a property `description` is provided, the description of the
+ * page is updated. Note that:
+ * 1). An empty string or null `description` will clear the existing
+ * value in the database.
+ * 2). Descriptions longer than DB_DESCRIPTION_LENGTH_MAX will be
+ * truncated.
+ * If a property `previewImageURL` is provided, the preview image
+ * URI of the page is updated. Note that a null `previewImageURL`
+ * will clear the existing value in the database.
+ *
+ * @return (Promise)
+ * A promise resolved once the update is complete.
+ * @rejects (Error)
+ * Rejects if the update was unsuccessful.
+ *
+ * @throws (Error)
+ * If `guidOrURI` is neither as a valid URL nor as a GUID.
+ * @throws (Error)
+ * If `pageMetaInfo` has an unexpected type.
+ * @throws (Error)
+ * If `pageMetaInfo` has neither `description` nor `previewImageURL`.
+ * @throws (Error)
+ * If the length of `pageMetaInfo.previewImageURL` is greater than
+ * DB_PREVIEW_IMAGE_URL_MAX.
+ */
+ setPageMetaInfo(guidOrURI, pageMetaInfo) {
+ guidOrURI = PlacesUtils.normalizeToURLOrGUID(guidOrURI);
+
+ if (typeof pageMetaInfo !== "object" || !pageMetaInfo) {
+ throw new TypeError("pageMetaInfo must be an object");
+ }
+
+ let info = validatePageMetaInfo(pageMetaInfo);
+
+ return PlacesUtils.withConnectionWrapper("History.jsm: setPageMetaInfo",
+ db => setPageMetaInfo(db, guidOrURI, info));
+ },
+
+
/**
* Possible values for the `transition` property of `VisitInfo`
* objects.
*/
TRANSITIONS: {
/**
* The user followed a link and got a new toplevel window.
@@ -616,16 +699,48 @@ this.History = Object.freeze({
/**
* The user reloaded a page.
*/
RELOAD: Ci.nsINavHistoryService.TRANSITION_RELOAD,
},
});
+ /**
+ * Validate an input PageMetaInfo object, returning a valid PageMetaInfo object.
+ *
+ * @param pageMetaInfo: (PageMetaInfo)
+ * @return (PageMetaInfo)
+ */
+function validatePageMetaInfo(pageMetaInfo) {
+ let info = {
+ };
+
+ if (typeof pageMetaInfo.description === "string" || pageMetaInfo.description === null) {
+ info.description = pageMetaInfo.description;
+ } else if (pageMetaInfo.description !== undefined) {
+ throw new TypeError(`description property of pageMetaInfo object: ${pageMetaInfo.description} must be a string if provided`);
+ }
+
+ if (pageMetaInfo.previewImageURL) {
+ info.previewImageURL = PlacesUtils.normalizeToURLOrGUID(pageMetaInfo.previewImageURL);
+ if (info.previewImageURL.href.length > DB_PREVIEW_IMAGE_URL_MAX) {
+ throw new TypeError("previewImageURL property of pageMetaInfo object is too long");
+ }
+ } else if (pageMetaInfo.previewImageURL === null) {
+ info.previewImageURL = null;
+ }
+
+ if (info.description === undefined && info.previewImageURL === undefined) {
+ throw new TypeError("pageMetaInfo object must at least have either a description or a previewImageURL property");
+ }
+ return info;
+}
+
+
/**
* Convert a PageInfo object into the format expected by updatePlaces.
*
* Note: this assumes that the PageInfo object has already been validated
* via PlacesUtils.validatePageInfo.
*
* @param pageInfo: (PageInfo)
* @return (info)
@@ -855,17 +970,23 @@ var fetch = async function(db, guidOrURL
let joinFragment = "";
let visitOrderFragment = ""
if (options.includeVisits) {
visitSelectionFragment = ", v.visit_date, v.visit_type";
joinFragment = "JOIN moz_historyvisits v ON h.id = v.place_id";
visitOrderFragment = "ORDER BY v.visit_date DESC";
}
- let query = `SELECT h.id, guid, url, title, frecency ${visitSelectionFragment}
+ let pageMetaSelectionFragment = "";
+ if (options.includePageMeta) {
+ pageMetaSelectionFragment = ", description, preview_image_url";
+ }
+
+ let query = `SELECT h.id, guid, url, title, frecency
+ ${pageMetaSelectionFragment} ${visitSelectionFragment}
FROM moz_places h ${joinFragment}
${whereClauseFragment}
${visitOrderFragment}`;
let pageInfo = null;
await db.executeCached(
query,
params,
row => {
@@ -873,16 +994,21 @@ var fetch = async function(db, guidOrURL
// This means we're on the first row, so we need to get all the page info.
pageInfo = {
guid: row.getResultByName("guid"),
url: new URL(row.getResultByName("url")),
frecency: row.getResultByName("frecency"),
title: row.getResultByName("title") || ""
};
}
+ if (options.includePageMeta) {
+ pageInfo.description = row.getResultByName("description") || ""
+ let previewImageURL = row.getResultByName("preview_image_url");
+ pageInfo.previewImageURL = previewImageURL ? new URL(previewImageURL) : null;
+ }
if (options.includeVisits) {
// On every row (not just the first), we need to collect visit data.
if (!("visits" in pageInfo)) {
pageInfo.visits = [];
}
let date = PlacesUtils.toDate(row.getResultByName("visit_date"));
let transition = row.getResultByName("visit_type");
@@ -1248,8 +1374,68 @@ var insertMany = async function(db, page
resolve();
} else {
reject({message: "No items were added to history."})
}
}
}, true);
});
};
+
+/**
+ * Convert a PageMetaInfo object into the format expected by setPageMetaInfo.
+ *
+ * Note: this assumes that the PageMetaInfo object has already been validated
+ * via PlacesUtils.validatePageMetaInfo.
+ *
+ * @param pageMetaInfo: (PageMetaInfo)
+ * @return (info)
+ */
+function convertForSetPageMetaInfo(pageMetaInfo) {
+ let info = {
+ };
+
+ if (pageMetaInfo.description) {
+ let description = pageMetaInfo.description;
+ if (description.length <= DB_DESCRIPTION_LENGTH_MAX) {
+ info.description = description;
+ } else {
+ info.description = description.slice(0, DB_DESCRIPTION_LENGTH_MAX);
+ }
+ } else if (pageMetaInfo.description === null ||
+ pageMetaInfo.description === "") {
+ info.description = null;
+ }
+
+ if (pageMetaInfo.previewImageURL) {
+ info.previewImageURL = pageMetaInfo.previewImageURL.href;
+ } else if (pageMetaInfo.previewImageURL === null) {
+ info.previewImageURL = null;
+ }
+
+ return info;
+}
+
+// Inner implementation of History.setPageMetaInfo.
+var setPageMetaInfo = async function(db, guidOrURI, pageMetaInfo) {
+ let updateFragments = [];
+ let info = convertForSetPageMetaInfo(pageMetaInfo);
+ let whereClauseFragment = "";
+
+ if (guidOrURI instanceof URL) {
+ whereClauseFragment = "WHERE url_hash = hash(:url) AND url = :url";
+ info.url = guidOrURI.href;
+ } else {
+ whereClauseFragment = "WHERE guid = :guid";
+ info.guid = guidOrURI;
+ }
+
+ if (info.description || info.description === null) {
+ updateFragments.push("description = :description");
+ }
+ if (info.previewImageURL || info.previewImageURL === null) {
+ updateFragments.push("preview_image_url = :previewImageURL");
+ }
+ let query = `UPDATE moz_places
+ SET ${updateFragments.join(", ")}
+ ${whereClauseFragment}`;
+ await db.execute(query, info);
+}
--- a/toolkit/components/places/tests/history/test_fetch.js
+++ b/toolkit/components/places/tests/history/test_fetch.js
@@ -72,16 +72,46 @@ add_task(async function test_fetch_exist
Assert.lessOrEqual(pageInfo.visits[i + 1].date.getTime(), pageInfo.visits[i].date.getTime());
}
}
Assert.deepEqual(idealPageInfo, pageInfo);
}
}
});
+add_task(async function test_fetch_page_meta_info() {
+ await PlacesTestUtils.clearHistory();
+ await PlacesUtils.bookmarks.eraseEverything();
+
+ let TEST_URI = NetUtil.newURI("http://mozilla.com/test_fetch_page_meta_info");
+ await PlacesTestUtils.addVisits(TEST_URI);
+ Assert.ok(page_in_database(TEST_URI));
+
+ // Test fetching the null values
+ let includePageMeta = true;
+ let pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includePageMeta});
+ Assert.strictEqual(null, pageInfo.previewImageURL, "fetch should return a null previewImageURL");
+ Assert.equal("", pageInfo.description, "fetch should return a empty string description");
+
+ // Now set the pageMetaInfo for this page
+ let description = "Test description";
+ let previewImageURL = "http://mozilla.com/test_preview_image.png";
+ await PlacesUtils.history.setPageMetaInfo(TEST_URI, {description, previewImageURL});
+
+ includePageMeta = true;
+ pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includePageMeta});
+ Assert.equal(previewImageURL, pageInfo.previewImageURL.href, "fetch should return a previewImageURL");
+ Assert.equal(description, pageInfo.description, "fetch should return a description");
+
+ includePageMeta = false;
+ pageInfo = await PlacesUtils.history.fetch(TEST_URI, {includePageMeta});
+ Assert.ok(!("description" in pageInfo), "fetch should not return a description if includePageMeta is false")
+ Assert.ok(!("previewImageURL" in pageInfo), "fetch should not return a previewImageURL if includePageMeta is false")
+});
+
add_task(async function test_fetch_nonexistent() {
await PlacesTestUtils.clearHistory();
await PlacesUtils.bookmarks.eraseEverything();
let uri = NetUtil.newURI("http://doesntexist.in.db");
let pageInfo = await PlacesUtils.history.fetch(uri);
Assert.equal(pageInfo, null);
});
@@ -99,12 +129,16 @@ add_task(async function test_error_cases
() => PlacesUtils.history.fetch("http://valid.uri.com", "not an object"),
/TypeError: options should be/
);
Assert.throws(
() => PlacesUtils.history.fetch("http://valid.uri.com", null),
/TypeError: options should be/
);
Assert.throws(
- () => PlacesUtils.history.fetch("http://valid.uri.come", { includeVisits: "not a boolean"}),
+ () => PlacesUtils.history.fetch("http://valid.uri.come", {includeVisits: "not a boolean"}),
/TypeError: includeVisits should be a/
);
+ Assert.throws(
+ () => PlacesUtils.history.fetch("http://valid.uri.come", {includePageMeta: "not a boolean"}),
+ /TypeError: includePageMeta should be a/
+ );
});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/history/test_setPageMetaInfo.js
@@ -0,0 +1,139 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests for `History.setPageMetaInfo` as implemented in History.jsm
+
+"use strict";
+
+add_task(async function test_error_cases() {
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo({not: "a valid url string"}),
+ /TypeError: Invalid url or guid/,
+ "passing an invalid url should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("not a valid url string"),
+ /TypeError: not a valid url string is/,
+ "passing an invalid url should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", "not an object"),
+ /TypeError: pageMetaInfo must be/,
+ "passing a string as pageMetaInfo should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", null),
+ /TypeError: pageMetaInfo must be/,
+ "passing a null as pageMetaInfo should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", { description: 123 }),
+ /TypeError: description property of/,
+ "passing a non-string description in pageMetaInfo should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", { previewImageURL: "not a valid url string"}),
+ /TypeError: not a valid url string is/,
+ "passing an invlid preview image url in pageMetaInfo should throw a TypeError"
+ );
+ Assert.throws(
+ () => {
+ let imageName = "a-very-long-string".repeat(200);
+ let previewImageURL = `http://valid.uri.com/${imageName}.png`;
+ PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", { previewImageURL });
+ },
+ /TypeError: previewImageURL property of/,
+ "passing an oversized previewImageURL in pageMetaInfo should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.setPageMetaInfo("http://valid.uri.com", {}),
+ /TypeError: pageMetaInfo object must at least/,
+ "passing a pageMetaInfo with neither description nor previewImageURL should throw a TypeError"
+ );
+});
+
+add_task(async function test_description_change_saved() {
+ await PlacesTestUtils.clearHistory();
+
+ let TEST_URL = NetUtil.newURI("http://mozilla.org/test_description_change_saved");
+ await PlacesTestUtils.addVisits(TEST_URL);
+ Assert.ok(page_in_database(TEST_URL));
+
+ let description = "Test description";
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description});
+ let descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ Assert.equal(description, descriptionInDB, "description should be updated via URL as expected");
+
+ description = "";
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description});
+ descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ Assert.strictEqual(null, descriptionInDB, "an empty description should set it to null in the database");
+
+ let guid = await PlacesTestUtils.fieldInDB(TEST_URL, "guid");
+ description = "Test description";
+ await PlacesUtils.history.setPageMetaInfo(guid, {description});
+ descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ Assert.equal(description, descriptionInDB, "description should be updated via GUID as expected");
+
+ description = "Test descipriton".repeat(1000);
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description});
+ descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ Assert.ok(0 < descriptionInDB.length < description.length, "a long description should be truncated");
+
+ description = null;
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description});
+ descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ Assert.strictEqual(description, descriptionInDB, "a null description should set it to null in the database");
+});
+
+add_task(async function test_previewImageURL_change_saved() {
+ await PlacesTestUtils.clearHistory();
+
+ let TEST_URL = NetUtil.newURI("http://mozilla.org/test_previewImageURL_change_saved");
+ let IMAGE_URL = "http://mozilla.org/test_preview_image.png";
+ await PlacesTestUtils.addVisits(TEST_URL);
+ Assert.ok(page_in_database(TEST_URL));
+
+ let previewImageURL = IMAGE_URL;
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {previewImageURL});
+ let previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+ Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via URL as expected");
+
+ previewImageURL = null;
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {previewImageURL});
+ previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+ Assert.strictEqual(null, previewImageURLInDB, "a null previewImageURL should set it to null in the database");
+
+ let guid = await PlacesTestUtils.fieldInDB(TEST_URL, "guid");
+ previewImageURL = IMAGE_URL;
+ await PlacesUtils.history.setPageMetaInfo(guid, {previewImageURL});
+ previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+ Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via GUID as expected");
+});
+
+add_task(async function test_pageMetaInfo_change_saved() {
+ await PlacesTestUtils.clearHistory();
+
+ let TEST_URL = NetUtil.newURI("http://mozilla.org/test_pageMetaInfo_change_saved");
+ await PlacesTestUtils.addVisits(TEST_URL);
+ Assert.ok(page_in_database(TEST_URL));
+
+ let description = "Test description";
+ let previewImageURL = "http://mozilla.org/test_preview_image.png";
+
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description, previewImageURL});
+ let descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ let previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+ Assert.equal(description, descriptionInDB, "description should be updated via URL as expected");
+ Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via URL as expected");
+
+ // Update description should not touch other fields
+ description = null;
+ await PlacesUtils.history.setPageMetaInfo(TEST_URL, {description});
+ descriptionInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "description");
+ previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url");
+ Assert.strictEqual(description, descriptionInDB, "description should be updated via URL as expected");
+ Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should not be updated");
+});
--- a/toolkit/components/places/tests/history/xpcshell.ini
+++ b/toolkit/components/places/tests/history/xpcshell.ini
@@ -6,9 +6,10 @@ head = head_history.js
[test_insert.js]
[test_insertMany.js]
[test_remove.js]
[test_removeMany.js]
[test_removeVisits.js]
[test_removeByFilter.js]
[test_removeVisitsByFilter.js]
[test_sameUri_titleChanged.js]
+[test_setPageMetaInfo.js]
[test_updatePlaces_embed.js]