--- a/toolkit/components/places/History.jsm
+++ b/toolkit/components/places/History.jsm
@@ -603,17 +603,17 @@ var History = Object.freeze({
if (!arg || typeof arg != "object" || arg.constructor.name != "Date") {
throw new TypeError("Expected a Date, got " + arg);
}
},
/**
* Update information for a page.
*
- * Currently, it supports updating the description and the preview image URL
+ * Currently, it supports updating the description, preview image URL and annotations
* for a page, any other fields will be ignored.
*
* Note that this function will ignore the update if the target page has not
* yet been stored in the database. `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 pageInfo: (PageInfo)
@@ -629,16 +629,24 @@ var History = Object.freeze({
*
* If a property `previewImageURL` is provided, the preview image
* URL of the page is updated. Note that:
* 1). A null `previewImageURL` will clear the existing value in the
* database.
* 2). It throws if its length is greater than DB_URL_LENGTH_MAX
* defined in PlacesUtils.jsm.
*
+ * If a property `annotations` is provided, the annotations will be
+ * updated. Note that:
+ * 1). It should be a Map containing key/value pairs to be updated.
+ * 2). If the value is falsy, the annotation will be removed.
+ * 3). If the value is non-falsy, the annotation will be added or updated.
+ * For `annotations` the keys must all be strings, the values should be
+ * Boolean, Number or Strings. null and undefined are supported as falsy values.
+ *
* @return (Promise)
* A promise resolved once the update is complete.
* @rejects (Error)
* Rejects if the update was unsuccessful.
*
* @throws (Error)
* If `pageInfo` has an unexpected type.
* @throws (Error)
@@ -647,18 +655,19 @@ var History = Object.freeze({
* If `pageInfo` has neither `description` nor `previewImageURL`.
* @throws (Error)
* If the length of `pageInfo.previewImageURL` is greater than
* DB_URL_LENGTH_MAX defined in PlacesUtils.jsm.
*/
update(pageInfo) {
let info = PlacesUtils.validatePageInfo(pageInfo, false);
- if (info.description === undefined && info.previewImageURL === undefined) {
- throw new TypeError("pageInfo object must at least have either a description or a previewImageURL property");
+ if (info.description === undefined && info.previewImageURL === undefined &&
+ info.annotations === undefined) {
+ throw new TypeError("pageInfo object must at least have either a description, previewImageURL or annotations property.");
}
return PlacesUtils.withConnectionWrapper("History.jsm: update", db => update(db, info));
},
/**
* Possible values for the `transition` property of `VisitInfo`
@@ -1428,36 +1437,99 @@ var insertMany = function(db, pageInfos,
}, true);
});
};
// Inner implementation of History.update.
var update = async function(db, pageInfo) {
let updateFragments = [];
let whereClauseFragment = "";
+ let baseParams = {};
let params = {};
// Prefer GUID over url if it's present
if (typeof pageInfo.guid === "string") {
whereClauseFragment = "guid = :guid";
- params.guid = pageInfo.guid;
+ baseParams.guid = pageInfo.guid;
} else {
whereClauseFragment = "url_hash = hash(:url) AND url = :url";
- params.url = pageInfo.url.href;
+ baseParams.url = pageInfo.url.href;
}
if (pageInfo.description || pageInfo.description === null) {
updateFragments.push("description");
params.description = pageInfo.description;
}
if (pageInfo.previewImageURL || pageInfo.previewImageURL === null) {
updateFragments.push("preview_image_url");
params.preview_image_url = pageInfo.previewImageURL ? pageInfo.previewImageURL.href : null;
}
- // Since this data may be written at every visit and is textual, avoid
- // overwriting the existing record if it didn't change.
- await db.execute(`
- UPDATE moz_places
- SET ${updateFragments.map(v => `${v} = :${v}`).join(", ")}
- WHERE ${whereClauseFragment}
- AND (${updateFragments.map(v => `IFNULL(${v}, "") <> IFNULL(:${v}, "")`).join(" OR ")})
- `, params);
+ if (updateFragments.length > 0) {
+ // Since this data may be written at every visit and is textual, avoid
+ // overwriting the existing record if it didn't change.
+ await db.execute(`
+ UPDATE moz_places
+ SET ${updateFragments.map(v => `${v} = :${v}`).join(", ")}
+ WHERE ${whereClauseFragment}
+ AND (${updateFragments.map(v => `IFNULL(${v}, "") <> IFNULL(:${v}, "")`).join(" OR ")})
+ `, {...baseParams, ...params});
+ }
+
+ if (pageInfo.annotations) {
+ let annosToRemove = [];
+ let annosToUpdate = [];
+
+ for (let anno of pageInfo.annotations) {
+ anno[1] ? annosToUpdate.push(anno[0]) : annosToRemove.push(anno[0]);
+ }
+
+ await db.executeTransaction(async function() {
+ if (annosToUpdate.length) {
+ await db.execute(`
+ INSERT OR IGNORE INTO moz_anno_attributes (name)
+ VALUES ${Array.from(annosToUpdate.keys()).map(k => `(:${k})`).join(", ")}
+ `, Object.assign({}, annosToUpdate));
+
+ for (let anno of annosToUpdate) {
+ let content = pageInfo.annotations.get(anno);
+ // TODO: We only really need to save the type whilst we still support
+ // accessing page annotations via the annotation service.
+ let type = typeof content == "string" ? Ci.nsIAnnotationService.TYPE_STRING :
+ Ci.nsIAnnotationService.TYPE_INT64;
+ let date = PlacesUtils.toPRTime(new Date());
+
+ // This will replace the id every time an annotation is updated. This is
+ // not currently an issue as we're not joining on the id field.
+ await db.execute(`
+ INSERT OR REPLACE INTO moz_annos
+ (place_id, anno_attribute_id, content, flags,
+ expiration, type, dateAdded, lastModified)
+ VALUES ((SELECT id FROM moz_places WHERE ${whereClauseFragment}),
+ (SELECT id FROM moz_anno_attributes WHERE name = :anno_name),
+ :content, 0, :expiration, :type, :date_added,
+ :last_modified)
+ `, {
+ ...baseParams,
+ anno_name: anno,
+ content,
+ expiration: PlacesUtils.annotations.EXPIRE_NEVER,
+ type,
+ // The date fields are unused, so we just set them both to the latest.
+ date_added: date,
+ last_modified: date,
+ });
+ }
+ }
+
+ for (let anno of annosToRemove) {
+ // We don't remove anything from the moz_anno_attributes table. If we
+ // delete the last item of a given name, that item really should go away.
+ // It will be cleaned up by expiration.
+ await db.execute(`
+ DELETE FROM moz_annos
+ WHERE place_id = (SELECT id FROM moz_places WHERE ${whereClauseFragment})
+ AND anno_attribute_id =
+ (SELECT id FROM moz_anno_attributes WHERE name = :anno_name)
+ `, { ...baseParams, anno_name: anno });
+ }
+ });
+ }
};
--- a/toolkit/components/places/tests/history/test_update.js
+++ b/toolkit/components/places/tests/history/test_update.js
@@ -58,17 +58,51 @@ add_task(async function test_error_cases
});
},
/TypeError: previewImageURL property of/,
"passing an oversized previewImageURL in pageInfo should throw a TypeError"
);
Assert.throws(
() => PlacesUtils.history.update({ url: "http://valid.uri.com" }),
/TypeError: pageInfo object must at least/,
- "passing a pageInfo with neither description nor previewImageURL should throw a TypeError"
+ "passing a pageInfo with neither description, previewImageURL, nor annotations should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.update({ url: "http://valid.uri.com", annotations: "asd" }),
+ /TypeError: annotations must be a Map/,
+ "passing a pageInfo with incorrect annotations type should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.update({ url: "http://valid.uri.com", annotations: new Map() }),
+ /TypeError: there must be at least one annotation/,
+ "passing a pageInfo with an empty annotations type should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.update({
+ url: "http://valid.uri.com",
+ annotations: new Map([[1234, "value"]]),
+ }),
+ /TypeError: all annotation keys must be strings/,
+ "passing a pageInfo with an invalid key type should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.update({
+ url: "http://valid.uri.com",
+ annotations: new Map([["test", ["myarray"]]]),
+ }),
+ /TypeError: all annotation values must be Boolean, Numbers or Strings/,
+ "passing a pageInfo with an invalid key type should throw a TypeError"
+ );
+ Assert.throws(
+ () => PlacesUtils.history.update({
+ url: "http://valid.uri.com",
+ annotations: new Map([["test", {anno: "value"}]]),
+ }),
+ /TypeError: all annotation values must be Boolean, Numbers or Strings/,
+ "passing a pageInfo with an invalid key type should throw a TypeError"
);
});
add_task(async function test_description_change_saved() {
await PlacesUtils.history.clear();
let TEST_URL = "http://mozilla.org/test_description_change_saved";
await PlacesTestUtils.addVisits(TEST_URL);
@@ -121,17 +155,17 @@ add_task(async function test_previewImag
let guid = await PlacesTestUtils.fieldInDB(TEST_URL, "guid");
previewImageURL = IMAGE_URL;
await PlacesUtils.history.update({ url: TEST_URL, 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_change_both_saved() {
+add_task(async function test_change_description_and_preview_saved() {
await PlacesUtils.history.clear();
let TEST_URL = "http://mozilla.org/test_change_both_saved";
await PlacesTestUtils.addVisits(TEST_URL);
Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL));
let description = "Test description";
let previewImageURL = "http://mozilla.org/test_preview_image.png";
@@ -145,8 +179,182 @@ add_task(async function test_change_both
// Update description should not touch other fields
description = null;
await PlacesUtils.history.update({ url: 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");
});
+
+async function getAnnotationInfoFromDB(pageUrl, annoName) {
+ let db = await PlacesUtils.promiseDBConnection();
+
+ let rows = await db.execute(`
+ SELECT a.content, a.flags, a.expiration, a.type FROM moz_anno_attributes n
+ JOIN moz_annos a ON n.id = a.anno_attribute_id
+ JOIN moz_places h ON h.id = a.place_id
+ WHERE h.url_hash = hash(:pageUrl) AND h.url = :pageUrl
+ AND n.name = :annoName
+ `, {annoName, pageUrl});
+
+ let result = rows.map(row => {
+ return {
+ content: row.getResultByName("content"),
+ flags: row.getResultByName("flags"),
+ expiration: row.getResultByName("expiration"),
+ type: row.getResultByName("type"),
+ };
+ });
+
+ return result;
+}
+
+add_task(async function test_simple_change_annotations() {
+ await PlacesUtils.history.clear();
+
+ const TEST_URL = "http://mozilla.org/test_change_both_saved";
+ await PlacesTestUtils.addVisits(TEST_URL);
+ Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL),
+ "Should have inserted the page into the database.");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([["test/annotation", "testContent"]]),
+ });
+
+ let pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 1,
+ "Should have one annotation for the page");
+ Assert.equal(pageInfo.annotations.get("test/annotation"), "testContent",
+ "Should have the correct annotation");
+
+ let annotationInfo = await getAnnotationInfoFromDB(TEST_URL, "test/annotation");
+ Assert.deepEqual({
+ content: "testContent",
+ flags: 0,
+ type: Ci.nsIAnnotationService.TYPE_STRING,
+ expiration: Ci.nsIAnnotationService.EXPIRE_NEVER,
+ }, annotationInfo[0], "Should have stored the correct annotation data in the db");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([["test/annotation2", "testAnno"]]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 2,
+ "Should have two annotations for the page");
+ Assert.equal(pageInfo.annotations.get("test/annotation"), "testContent",
+ "Should have the correct value for the first annotation");
+ Assert.equal(pageInfo.annotations.get("test/annotation2"), "testAnno",
+ "Should have the correct value for the second annotation");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([["test/annotation", 1234]]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 2,
+ "Should still have two annotations for the page");
+ Assert.equal(pageInfo.annotations.get("test/annotation"), 1234,
+ "Should have the updated the first annotation value");
+ Assert.equal(pageInfo.annotations.get("test/annotation2"), "testAnno",
+ "Should have kept the value for the second annotation");
+
+ annotationInfo = await getAnnotationInfoFromDB(TEST_URL, "test/annotation");
+ Assert.deepEqual({
+ content: 1234,
+ flags: 0,
+ type: Ci.nsIAnnotationService.TYPE_INT64,
+ expiration: Ci.nsIAnnotationService.EXPIRE_NEVER,
+ }, annotationInfo[0], "Should have updated the annotation data in the db");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([["test/annotation", null]]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 1,
+ "Should have removed only the first annotation");
+ Assert.strictEqual(pageInfo.annotations.get("test/annotation"), undefined,
+ "Should have removed only the first annotation");
+ Assert.equal(pageInfo.annotations.get("test/annotation2"), "testAnno",
+ "Should have kept the value for the second annotation");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([["test/annotation2", null]]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 0,
+ "Should have no annotations left");
+
+ let db = await PlacesUtils.promiseDBConnection();
+ let rows = await db.execute(`
+ SELECT * FROM moz_annos
+ `);
+ Assert.equal(rows.length, 0, "Should be no annotations left in the db");
+});
+
+add_task(async function test_change_multiple_annotations() {
+ await PlacesUtils.history.clear();
+
+ const TEST_URL = "http://mozilla.org/test_change_both_saved";
+ await PlacesTestUtils.addVisits(TEST_URL);
+ Assert.ok(await PlacesTestUtils.isPageInDB(TEST_URL),
+ "Should have inserted the page into the database.");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([
+ ["test/annotation", "testContent"],
+ ["test/annotation2", "testAnno"],
+ ]),
+ });
+
+ let pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 2,
+ "Should have inserted the two annotations for the page.");
+ Assert.equal(pageInfo.annotations.get("test/annotation"), "testContent",
+ "Should have the correct value for the first annotation");
+ Assert.equal(pageInfo.annotations.get("test/annotation2"), "testAnno",
+ "Should have the correct value for the second annotation");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([
+ ["test/annotation", 123456],
+ ["test/annotation2", 135246],
+ ]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 2,
+ "Should have two annotations for the page");
+ Assert.equal(pageInfo.annotations.get("test/annotation"), 123456,
+ "Should have the correct value for the first annotation");
+ Assert.equal(pageInfo.annotations.get("test/annotation2"), 135246,
+ "Should have the correct value for the second annotation");
+
+ await PlacesUtils.history.update({
+ url: TEST_URL,
+ annotations: new Map([
+ ["test/annotation", null],
+ ["test/annotation2", null],
+ ]),
+ });
+
+ pageInfo = await PlacesUtils.history.fetch(TEST_URL, {includeAnnotations: true});
+
+ Assert.equal(pageInfo.annotations.size, 0,
+ "Should have no annotations left");
+});