corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1828925 - Update recalc_alt_frecency when we update recalc_frecency. r=daisuke

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D177485
This commit is contained in:
Marco Bonardo
2023-05-10 12:35:04 +00:00
parent d29d929ab9
commit 2eb2826ca2
6 changed files with 142 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
toolkit/components/places/PlacesDBUtils.sys.mjs
View File

@@ -347,7 +347,8 @@ export var PlacesDBUtils = {
url = OLD.url;
/* Recalculate frecency for the destination. */
UPDATE moz_places SET recalc_frecency = 1 WHERE id = OLD.id;
UPDATE moz_places SET recalc_frecency = 1, recalc_alt_frecency = 1
WHERE id = OLD.id;
END`,
},
{

4
toolkit/components/places/bookmark_sync/src/store.rs
View File

@@ -929,7 +929,7 @@ fn apply_remote_items(db: &Conn, driver: &Driver, controller: &AbortController)
controller.err_if_aborted()?;
db.exec(
"UPDATE moz_places SET
recalc_frecency = 1
recalc_frecency = 1, recalc_alt_frecency = 1
WHERE frecency <> 0 AND (
id IN (
SELECT oldPlaceId FROM itemsToApply
@@ -997,7 +997,7 @@ fn remove_local_items(
debug!(driver, "Recalculating frecencies for removed bookmark URLs");
let mut frecency_statement = db.prepare(format!(
"UPDATE moz_places SET
recalc_frecency = 1
recalc_frecency = 1, recalc_alt_frecency = 1
WHERE id IN (SELECT b.fk FROM moz_bookmarks b
WHERE b.guid IN ({})) AND
frecency <> 0",

28
toolkit/components/places/nsPlacesTriggers.h
View File

@@ -34,6 +34,7 @@
"UPDATE moz_places SET " \
"visit_count = visit_count + " VISIT_COUNT_INC("NEW.visit_type") ", " \
"recalc_frecency = (frecency <> 0), " \
"recalc_alt_frecency = (frecency <> 0), " \
"last_visit_date = MAX(IFNULL(last_visit_date, 0), NEW.visit_date) " \
"WHERE id = NEW.place_id;" \
"END")
@@ -47,6 +48,7 @@
"UPDATE moz_places SET " \
"visit_count = visit_count - " VISIT_COUNT_INC("OLD.visit_type") ", " \
"recalc_frecency = (frecency <> 0), " \
"recalc_alt_frecency = (frecency <> 0), " \
"last_visit_date = (SELECT visit_date FROM moz_historyvisits " \
"WHERE place_id = OLD.place_id " \
"ORDER BY visit_date DESC LIMIT 1) " \
@@ -284,8 +286,9 @@
"AFTER DELETE ON moz_bookmarks FOR EACH ROW " \
"BEGIN " \
"UPDATE moz_places " \
"SET foreign_count = foreign_count - 1, " \
" recalc_frecency = NOT " IS_PLACE_QUERY \
"SET foreign_count = foreign_count - 1 " \
", recalc_frecency = NOT " IS_PLACE_QUERY \
", recalc_alt_frecency = NOT " IS_PLACE_QUERY \
"WHERE id = OLD.fk;" \
"END")
@@ -310,10 +313,10 @@
"SET frecency = 1 WHERE frecency = -1 AND NOT " IS_PLACE_QUERY \
";" \
"UPDATE moz_places " \
"SET foreign_count = foreign_count + 1, " \
" hidden = " IS_PLACE_QUERY \
"," \
" recalc_frecency = NOT " IS_PLACE_QUERY \
"SET foreign_count = foreign_count + 1 " \
", hidden = " IS_PLACE_QUERY \
", recalc_frecency = NOT " IS_PLACE_QUERY \
", recalc_alt_frecency = NOT " IS_PLACE_QUERY \
"WHERE id = NEW.fk;" \
"END")
@@ -325,14 +328,15 @@
"SELECT note_sync_change() " \
"WHERE NEW.syncChangeCounter <> OLD.syncChangeCounter; " \
"UPDATE moz_places " \
"SET foreign_count = foreign_count + 1, " \
" hidden = " IS_PLACE_QUERY \
"," \
" recalc_frecency = NOT " IS_PLACE_QUERY \
"SET foreign_count = foreign_count + 1 " \
", hidden = " IS_PLACE_QUERY \
", recalc_frecency = NOT " IS_PLACE_QUERY \
", recalc_alt_frecency = NOT " IS_PLACE_QUERY \
"WHERE OLD.fk <> NEW.fk AND id = NEW.fk;" \
"UPDATE moz_places " \
"SET foreign_count = foreign_count - 1, " \
" recalc_frecency = NOT " IS_PLACE_QUERY \
"SET foreign_count = foreign_count - 1 " \
", recalc_frecency = NOT " IS_PLACE_QUERY \
", recalc_alt_frecency = NOT " IS_PLACE_QUERY \
"WHERE OLD.fk <> NEW.fk AND id = OLD.fk;" \
"END")

63
toolkit/components/places/tests/PlacesTestUtils.sys.mjs
View File

@@ -555,7 +555,48 @@ export var PlacesTestUtils = Object.freeze({
* @throws If more than one result is found for the given conditions.
*/
async getDatabaseValue(table, field, conditions) {
let whereClause = [];
let { fragment: where, params } = this._buildWhereClause(table, conditions);
let query = `SELECT ${field} FROM ${table} ${where}`;
let conn = await lazy.PlacesUtils.promiseDBConnection();
let rows = await conn.executeCached(query, params);
if (rows.length > 1) {
throw new Error(
"getDatabaseValue doesn't support returning multiple results"
);
}
return rows[0]?.getResultByName(field);
},
/**
* Updates a specified field in a database table, based on the given
* conditions.
* @param {string} table - The name of the database table to add to.
* @param {string} field - The name of the field to update the value for.
* @param {string} value - The value to set.
* @param {Object} conditions - An object containing the conditions to filter
* the query results. The keys represent the names of the columns to filter
* by, and the values represent the filter values.
* @return {Promise} A Promise that resolves to the number of affected rows.
* @throws If no rows were affected.
*/
async updateDatabaseValue(table, field, value, conditions) {
let { fragment: where, params } = this._buildWhereClause(table, conditions);
let query = `UPDATE ${table} SET ${field} = :val ${where} RETURNING rowid`;
params.val = value;
return lazy.PlacesUtils.withConnectionWrapper(
"setDatabaseValue",
async conn => {
let rows = await conn.executeCached(query, params);
if (!rows.length) {
throw new Error("setDatabaseValue didn't update any value");
}
return rows.length;
}
);
},
_buildWhereClause(table, conditions) {
let fragments = [];
let params = {};
for (let [column, value] of Object.entries(conditions)) {
if (column == "url") {
@@ -566,23 +607,15 @@ export var PlacesTestUtils = Object.freeze({
}
}
if (column == "url" && table == "moz_places") {
whereClause.push("url_hash = hash(:url) AND url = :url");
fragments.push("url_hash = hash(:url) AND url = :url");
} else {
whereClause.push(`${column} = :${column}`);
fragments.push(`${column} = :${column}`);
}
params[column] = value;
}
let whereString = whereClause.length
? `WHERE ${whereClause.join(" AND ")}`
: "";
let query = `SELECT ${field} FROM ${table} ${whereString}`;
let conn = await lazy.PlacesUtils.promiseDBConnection();
let rows = await conn.executeCached(query, params);
if (rows.length > 1) {
throw new Error(
"getDatabaseValue doesn't support returning multiple results"
);
}
return rows[0]?.getResultByName(field);
return {
fragment: fragments.length ? `WHERE ${fragments.join(" AND ")}` : "",
params,
};
},
});

73
toolkit/components/places/tests/unit/test_frecency_pages_recalc_alt.js Normal file
View File

@@ -0,0 +1,73 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
// Tests that recalc_alt_frecency in the moz_places table is updated correctly.
add_task(async function test() {
info("test recalc_alt_frecency is set to 1 when a visit is added");
const now = new Date();
const URL = "https://mozilla.org/test/";
let getValue = url =>
PlacesTestUtils.getDatabaseValue("moz_places", "recalc_alt_frecency", {
url,
});
let setValue = (url, val) =>
PlacesTestUtils.updateDatabaseValue(
"moz_places",
"recalc_alt_frecency",
val,
{ url }
);
await PlacesTestUtils.addVisits([
{
url: URL,
visitDate: now,
},
{
url: URL,
visitDate: new Date(new Date().setDate(now.getDate() - 30)),
},
]);
Assert.equal(await getValue(URL), 1);
await setValue(URL, 0);
info("Remove just one visit (otherwise the page would be orphaned).");
await PlacesUtils.history.removeVisitsByFilter({
beginDate: new Date(now.valueOf() - 10000),
endDate: new Date(now.valueOf() + 10000),
});
Assert.equal(await getValue(URL), 1);
await setValue(URL, 0);
info("Add a bookmark to the page");
let bm = await PlacesUtils.bookmarks.insert({
url: URL,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
});
Assert.equal(await getValue(URL), 1);
await setValue(URL, 0);
info("Clear history");
await PlacesUtils.history.clear();
Assert.equal(await getValue(URL), 1);
await setValue(URL, 0);
// Add back a visit so the page is not an orphan once we remove the bookmark.
await PlacesTestUtils.addVisits(URL);
Assert.equal(await getValue(URL), 1);
await setValue(URL, 0);
info("change the bookmark URL");
const URL2 = "https://editedbookmark.org/";
bm.url = URL2;
await PlacesUtils.bookmarks.update(bm);
Assert.equal(await getValue(URL), 1);
Assert.equal(await getValue(URL2), 1);
await setValue(URL, 0);
await setValue(URL2, 0);
info("Remove the bookmark from the page");
await PlacesUtils.bookmarks.remove(bm);
Assert.equal(await getValue(URL2), 1);
await setValue(URL2, 0);
});

1
toolkit/components/places/tests/unit/xpcshell.ini
View File

@@ -59,6 +59,7 @@ skip-if = os == "linux" # Bug 821781
[test_frecency_decay.js]
[test_frecency_origins_alternative.js]
[test_frecency_origins_recalc.js]
[test_frecency_pages_recalc_alt.js]
[test_frecency_recalc_triggers.js]
[test_frecency_recalculator.js]
[test_frecency_unvisited_bookmark.js]