Skip to content

Commit

Permalink
Add a Places API to recursively count the number of descendants in a …
Browse files Browse the repository at this point in the history
…set of folders
  • Loading branch information
mhammond committed Oct 5, 2023
1 parent b7cb31d commit 38c6ba2
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,19 @@ interface ReadableBookmarksConnection : InterruptibleConnection {
* has its `interrupt()` method called on another thread.
*/
fun getRecentBookmarks(limit: Int): List<BookmarkItem>

/**
* Counts the number of bookmark items in the bookmark trees under the specified GUIDs.
* @param guids The guids of folders to query.
* @return Count of all bookmark items (ie, not folders or separators) in all specified folders recursively.
* Empty folders, non-existing GUIDs and non-existing items will return zero.
* The result is implementation dependant if the trees overlap.
*
* @throws OperationInterrupted if this database implements [InterruptibleConnection] and
* has its `interrupt()` method called on another thread.
*/
fun countBookmarksInTrees(guids: List<Guid>): UInt
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ open class PlacesReaderConnection internal constructor(conn: UniffiPlacesConnect
}
}

override fun countBookmarksInTrees(guids: List<Guid>): UInt {
return readQueryCounters.measure {
this.conn.bookmarksCountBookmarksInTrees(guids)
}
}

private val readQueryCounters: PlacesManagerCounterMetrics by lazy {
PlacesManagerCounterMetrics(
PlacesManagerMetrics.readQueryCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,40 @@ class PlacesConnectionTest {
assertEquals(folder.f.parentGuid, BookmarkRoot.Unfiled.id)
}

@Test
fun testCountBookmarks() {
assertEquals(db.countBookmarksInTrees(listOf(BookmarkRoot.Unfiled.id)), 0U)

db.createBookmarkItem(
parentGUID = BookmarkRoot.Unfiled.id,
url = "https://www.example.com/",
title = "example",
)
assertEquals(db.countBookmarksInTrees(listOf(BookmarkRoot.Unfiled.id)), 1U)

val folderGUID = db.createFolder(
parentGUID = BookmarkRoot.Unfiled.id,
title = "example folder",
)
// Folders don't get counted.
assertEquals(db.countBookmarksInTrees(listOf(BookmarkRoot.Unfiled.id)), 1U)

// new item in the child folder does.
db.createBookmarkItem(
parentGUID = folderGUID,
url = "https://www.example.com/",
title = "example",
)
assertEquals(db.countBookmarksInTrees(listOf(BookmarkRoot.Unfiled.id)), 2U)

// A separator is not counted.
db.createSeparator(
parentGUID = BookmarkRoot.Unfiled.id,
position = 0u,
)
assertEquals(db.countBookmarksInTrees(listOf(BookmarkRoot.Unfiled.id)), 2U)
}

@Test
fun testHistoryMetricsGathering() {
assertNull(PlacesManagerMetrics.writeQueryCount.testGetValue())
Expand Down
5 changes: 5 additions & 0 deletions components/places/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,11 @@ impl PlacesConnection {
self.with_conn(|conn| bookmarks::update_bookmark_from_info(conn, item))
}

#[handle_error(crate::Error)]
pub fn bookmarks_count_bookmarks_in_trees(&self, guids: &[Guid]) -> ApiResult<u32> {
self.with_conn(|conn| bookmarks::count_bookmarks_in_trees(conn, guids))
}

#[handle_error(crate::Error)]
pub fn places_history_import_from_ios(
&self,
Expand Down
7 changes: 7 additions & 0 deletions components/places/src/places.udl
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,13 @@ interface PlacesConnection {
[Throws=PlacesApiError]
Guid bookmarks_insert(InsertableBookmarkItem bookmark);

// Counts the number of bookmarks in the bookmark tree under the specified GUID. Does not count
// the passed item, so an empty folder will return zero, as will a non-existing GUID or the
// guid of a non-folder item.
// Counts only bookmark items - ie, sub-folders and separators are not counted.
[Throws=PlacesApiError]
u32 bookmarks_count_bookmarks_in_trees([ByRef] sequence<Guid> folder_guids);

[Throws=PlacesApiError]
HistoryMigrationResult places_history_import_from_ios(string db_path, i64 last_sync_timestamp);
};
Expand Down
127 changes: 127 additions & 0 deletions components/places/src/storage/bookmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,40 @@ pub fn bookmarks_get_url_for_keyword(db: &PlacesDb, keyword: &str) -> Result<Opt
}
}

fn repeat_vars(count: usize) -> String {
assert_ne!(count, 0);
let mut s = "?,".repeat(count);
// Remove trailing comma
s.pop();
s
}

// Counts the number of bookmark items in the bookmark trees under the specified GUIDs.
// Does not count folder items, separators. A set of empty folders will return zero, as will
// a set of non-existing GUIDs or guids of a non-folder item.
// The result is implementation dependant if the trees overlap.
pub fn count_bookmarks_in_trees(db: &PlacesDb, item_guids: &[SyncGuid]) -> Result<u32> {
if item_guids.is_empty() {
return Ok(0);
}
let vars = repeat_vars(item_guids.len());
let sql = format!(
r#"
WITH RECURSIVE bookmark_tree(id, parent, type)
AS (
SELECT id, parent, type FROM moz_bookmarks
WHERE parent IN (SELECT id from moz_bookmarks WHERE guid IN ({vars}))
UNION ALL
SELECT bm.id, bm.parent, bm.type FROM moz_bookmarks bm, bookmark_tree bt WHERE bm.parent = bt.id
)
SELECT COUNT(*) from bookmark_tree WHERE type = {};
"#,
BookmarkType::Bookmark as u8
);
let params = rusqlite::params_from_iter(item_guids);
Ok(db.query_row_and_then(&sql, params, |row| row.get(0))?)
}

/// Erases all bookmarks and resets all Sync metadata.
pub fn delete_everything(db: &PlacesDb) -> Result<()> {
let tx = db.begin_transaction()?;
Expand Down Expand Up @@ -1919,4 +1953,97 @@ mod tests {

Ok(())
}

#[test]
fn test_count_tree() -> Result<()> {
let conn = new_mem_connection();
let unfiled = BookmarkRootGuid::Unfiled.as_guid();

insert_json_tree(
&conn,
json!({
"guid": &unfiled,
"children": [
{
"guid": "folder1_____",
"title": "A folder",
"children": [
{
"guid": "bookmark1___",
"title": "bookmark in A folder",
"url": "https://www.example2.com/"
},
{
"guid": "separator1__",
"type": BookmarkType::Separator,
},
{
"guid": "bookmark2___",
"title": "next bookmark in A folder",
"url": "https://www.example3.com/"
},
]
},
{
"guid": "folder2_____",
"title": "folder 2",
},
{
"guid": "folder3_____",
"title": "Another folder",
"children": [
{
"guid": "bookmark3___",
"title": "bookmark in folder 3",
"url": "https://www.example2.com/"
},
{
"guid": "separator2__",
"type": BookmarkType::Separator,
},
{
"guid": "bookmark4___",
"title": "next bookmark in folder 3",
"url": "https://www.example3.com/"
},
]
},
]
}),
);
assert_eq!(count_bookmarks_in_trees(&conn, &[])?, 0);
// A folder with sub-folders
assert_eq!(count_bookmarks_in_trees(&conn, &[unfiled])?, 4);
// A folder with items but no folders.
assert_eq!(
count_bookmarks_in_trees(&conn, &[SyncGuid::from("folder1_____")])?,
2
);
// Asking for a bookmark (ie, not a folder) or an invalid guid gives zero.
assert_eq!(
count_bookmarks_in_trees(&conn, &[SyncGuid::from("bookmark1___")])?,
0
);
assert_eq!(
count_bookmarks_in_trees(&conn, &[SyncGuid::from("no_such_guid")])?,
0
);
// empty folder also zero.
assert_eq!(
count_bookmarks_in_trees(&conn, &[SyncGuid::from("folder2_____")])?,
0
);
// multiple folders
assert_eq!(
count_bookmarks_in_trees(
&conn,
&[
SyncGuid::from("folder1_____"),
SyncGuid::from("folder3_____")
]
)?,
4
);
Ok(())
}
}

0 comments on commit 38c6ba2

Please sign in to comment.