Skip to content

Commit acf1bc2

Browse files
committed
Bug 2006505 - allow folders to be searched.
1 parent f46182e commit acf1bc2

File tree

4 files changed

+135
-0
lines changed

4 files changed

+135
-0
lines changed

components/places/android/src/main/java/mozilla/appservices/places/PlacesConnection.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,12 @@ open class PlacesReaderConnection internal constructor(conn: UniffiPlacesConnect
262262
}
263263
}
264264

265+
override fun searchFolders(query: String, limit: Int): List<BookmarkFolder`> {
266+
return readQueryCounters.measure {
267+
this.conn.foldersSearch(query, limit)
268+
}
269+
}
270+
265271
override fun getRecentBookmarks(limit: Int): List<BookmarkItem> {
266272
return readQueryCounters.measure {
267273
this.conn.bookmarksGetRecent(limit)

components/places/src/ffi.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,11 @@ impl PlacesConnection {
535535
})
536536
}
537537

538+
#[handle_error(crate::Error)]
539+
pub fn folders_search(&self, query: String, limit: i32) -> ApiResult<Vec<BookmarkFolder>> {
540+
self.with_conn(|conn| bookmarks::fetch::search_folders(conn, query.as_str(), limit as u32))
541+
}
542+
538543
#[handle_error(crate::Error)]
539544
pub fn bookmarks_get_recent(&self, limit: i32) -> ApiResult<Vec<BookmarkItem>> {
540545
self.with_conn(|conn| {

components/places/src/places.udl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,12 @@ interface PlacesConnection {
190190
[Throws=PlacesApiError]
191191
sequence<BookmarkItem> bookmarks_search(string query, i32 limit);
192192

193+
/// Searches folders for a string in the folder title.
194+
/// Matching folders are returned without any child information - ie, both `child_guids`
195+
/// and `child_nodes` will be null.
196+
[Throws=PlacesApiError]
197+
sequence<BookmarkFolder> folders_search(string query, i32 limit);
198+
193199
// XXX - should return BookmarkData
194200
[Throws=PlacesApiError]
195201
sequence<BookmarkItem> bookmarks_get_recent(i32 limit);

components/places/src/storage/bookmarks/fetch.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,37 @@ pub fn recent_bookmarks(db: &PlacesDb, limit: u32) -> Result<Vec<BookmarkData>>
359359
.collect())
360360
}
361361

362+
fn folder_from_row(row: &Row<'_>) -> Result<Folder> {
363+
Ok(Folder {
364+
guid: row.get("guid")?,
365+
parent_guid: row.get("parentGuid")?,
366+
position: row.get("position")?,
367+
date_added: row.get("dateAdded")?,
368+
last_modified: row.get("lastModified")?,
369+
title: row.get("title")?,
370+
child_guids: None,
371+
child_nodes: None,
372+
})
373+
}
374+
375+
pub fn search_folders(db: &PlacesDb, search: &str, limit: u32) -> Result<Vec<Folder>> {
376+
let scope = db.begin_interrupt_scope()?;
377+
Ok(db
378+
.query_rows_into_cached::<Vec<Folder>, _, _, _, _>(
379+
&SEARCH_FOLDERS_QUERY,
380+
&[
381+
(":search", &search as &dyn rusqlite::ToSql),
382+
(":limit", &limit),
383+
],
384+
|row| -> Result<_> {
385+
scope.err_if_interrupted()?;
386+
folder_from_row(row)
387+
},
388+
)?
389+
.into_iter()
390+
.collect())
391+
}
392+
362393
lazy_static::lazy_static! {
363394
pub static ref SEARCH_QUERY: String = format!(
364395
"SELECT
@@ -413,6 +444,43 @@ lazy_static::lazy_static! {
413444
LIMIT :limit",
414445
bookmark_type = BookmarkType::Bookmark as u8
415446
);
447+
448+
// We use AUTOCOMPLETE_MATCH for folders too - it's a little overkill, but we
449+
// get some of the unicode handling etc for free.
450+
pub static ref SEARCH_FOLDERS_QUERY: String = format!(
451+
"SELECT
452+
b.guid,
453+
p.guid AS parentGuid,
454+
b.position,
455+
b.dateAdded,
456+
b.lastModified,
457+
-- Note we return null for titles with an empty string.
458+
NULLIF(b.title, '') AS title
459+
FROM moz_bookmarks b
460+
JOIN moz_bookmarks p ON p.id = b.parent
461+
WHERE b.type = {folder_type}
462+
AND b.guid NOT IN ('{root_guid}', '{menu_guid}', '{mobile_guid}', '{toolbar_guid}', '{unfiled_guid}')
463+
AND AUTOCOMPLETE_MATCH(
464+
:search, '', IFNULL(b.title, ''),
465+
NULL,
466+
0, -- visit_count
467+
0, -- typed
468+
1, -- bookmarked
469+
NULL, -- open page count
470+
{match_bhvr},
471+
{search_bhvr}
472+
)
473+
LIMIT :limit",
474+
folder_type = BookmarkType::Folder as u8,
475+
match_bhvr = crate::match_impl::MatchBehavior::Anywhere as u32,
476+
search_bhvr = crate::match_impl::SearchBehavior::BOOKMARK.bits(),
477+
root_guid = BookmarkRootGuid::Root.as_str(),
478+
menu_guid = BookmarkRootGuid::Menu.as_str(),
479+
mobile_guid = BookmarkRootGuid::Mobile.as_str(),
480+
toolbar_guid = BookmarkRootGuid::Toolbar.as_str(),
481+
unfiled_guid = BookmarkRootGuid::Unfiled.as_str(),
482+
483+
);
416484
}
417485

418486
#[cfg(test)]
@@ -580,6 +648,56 @@ mod test {
580648
}
581649
Ok(())
582650
}
651+
652+
#[test]
653+
fn test_search_folders() -> Result<()> {
654+
let conns = new_mem_connections();
655+
insert_json_tree(
656+
&conns.write,
657+
json!({
658+
"guid": String::from(BookmarkRootGuid::Unfiled.as_str()),
659+
"children": [
660+
{
661+
"guid": "folder1_____",
662+
"title": "my example folder",
663+
"children": [
664+
{
665+
"guid": "bookmark1___",
666+
"url": "https://www.example1.com/",
667+
"title": "example bookmark",
668+
},
669+
{
670+
"guid": "folder2_____",
671+
"title": "another folder",
672+
"children": []
673+
}
674+
]
675+
}
676+
]
677+
}),
678+
);
679+
let mut folders = search_folders(&conns.read, "ample", 10)?;
680+
folders.sort_by_key(|b| b.guid.as_str().to_string());
681+
assert_eq!(folders.len(), 1);
682+
assert_eq!(folders[0].guid, "folder1_____");
683+
684+
let mut folders = search_folders(&conns.read, "older", 10)?;
685+
folders.sort_by_key(|b| b.guid.as_str().to_string());
686+
assert_eq!(folders.len(), 2);
687+
assert_eq!(folders[0].guid, "folder1_____");
688+
assert_eq!(
689+
folders[0].parent_guid,
690+
Some(BookmarkRootGuid::Unfiled.as_guid())
691+
);
692+
assert_eq!(folders[1].guid, "folder2_____");
693+
assert_eq!(folders[1].parent_guid, Some(SyncGuid::new("folder1_____")));
694+
695+
// check we never get the roots.
696+
assert_eq!(search_folders(&conns.read, "", 10)?.len(), 2);
697+
698+
Ok(())
699+
}
700+
583701
#[test]
584702
fn test_fetch_bookmark() -> Result<()> {
585703
let conns = new_mem_connections();

0 commit comments

Comments
 (0)