Merge pull request #1289 from lyuyuan92/master

Let fetch_tree also returns position and parent_guid

components/places/src/storage/bookmarks.rs

@@ -1123,8 +1123,12 @@ fn inflate(
 }
 
 /// Fetch the tree starting at the specified folder guid.
-/// Returns a BookmarkTreeNode::Folder(_)
-pub fn fetch_tree(db: &PlacesDb, item_guid: &SyncGuid) -> Result<Option<BookmarkTreeNode>> {
+/// Returns a `BookmarkTreeNode::Folder(_)`, its parent's guid (if any), and
+/// position inside its parent.
+pub fn fetch_tree(
+    db: &PlacesDb,
+    item_guid: &SyncGuid,
+) -> Result<Option<(BookmarkTreeNode, Option<SyncGuid>, u32)>> {
     // XXX - this needs additional work for tags - unlike desktop, there's no
     // "tags" folder, but instead a couple of tables to join on.
     let sql = r#"
@@ -1172,10 +1176,15 @@ pub fn fetch_tree(db: &PlacesDb, item_guid: &SyncGuid) -> Result<Option<Bookmark
     let mut results =
         stmt.query_and_then_named(&[(":item_guid", item_guid)], FetchedTreeRow::from_row)?;
 
+    let parent_guid: Option<SyncGuid>;
+    let position: u32;
+
     // The first row in the result set is always the root of our tree.
     let mut root = match results.next() {
         Some(result) => {
             let row = result?;
+            parent_guid = row.parent_guid.clone();
+            position = row.position;
             FolderNode {
                 guid: Some(row.guid.clone()),
                 date_added: Some(row.date_added),
@@ -1245,7 +1254,7 @@ pub fn fetch_tree(db: &PlacesDb, item_guid: &SyncGuid) -> Result<Option<Bookmark
 
     // Finally, inflate our tree.
     inflate(&mut root, &mut pseudo_tree);
-    Ok(Some(root))
+    Ok(Some((root, parent_guid, position)))
 }
 
 /// A "raw" bookmark - a representation of the row and some summary fields.
@@ -2069,7 +2078,7 @@ mod tests {
         let conn = new_mem_connection();
 
         // Fetch the root
-        let t = fetch_tree(&conn, &BookmarkRootGuid::Root.into())?.unwrap();
+        let (t, _, _) = fetch_tree(&conn, &BookmarkRootGuid::Root.into())?.unwrap();
         let f = match t {
             BookmarkTreeNode::Folder(ref f) => f,
             _ => panic!("tree root must be a folder"),

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

@@ -212,32 +212,19 @@ pub fn update_bookmark_from_message(db: &PlacesDb, msg: ProtoBookmark) -> Result
 /// function called by the FFI when requesting the tree.
 pub fn fetch_public_tree(db: &PlacesDb, item_guid: &SyncGuid) -> Result<Option<PublicNode>> {
     let _tx = db.begin_transaction()?;
-    let tree = if let Some(tree) = fetch_tree(db, item_guid)? {
-        tree
-    } else {
-        return Ok(None);
-    };
+    let (tree, parent_guid, position) =
+        if let Some((tree, parent_guid, position)) = fetch_tree(db, item_guid)? {
+            (tree, parent_guid, position)
+        } else {
+            return Ok(None);
+        };
 
     // `position` and `parent_guid` will be handled for the children of
     // `item_guid` by `PublicNode::from` automatically, however we
     // still need to fill in it's own `parent_guid` and `position`.
     let mut proto = PublicNode::from(tree);
 
     if item_guid != BookmarkRootGuid::Root {
-        let sql = "
-            SELECT
-                p.guid AS parent_guid,
-                b.position AS position
-            FROM moz_bookmarks b
-            LEFT JOIN moz_bookmarks p ON p.id = b.parent
-            WHERE b.guid = :guid
-        ";
-        let (parent_guid, position) = db.query_row_and_then_named(
-            sql,
-            &[(":guid", &item_guid)],
-            |row| -> Result<_> { Ok((row.get::<_, Option<SyncGuid>>(0)?, row.get::<_, u32>(1)?)) },
-            true,
-        )?;
         proto.parent_guid = parent_guid;
         proto.position = position;
     }
@@ -624,6 +611,10 @@ mod test {
         assert_eq!(mobile.parent_guid.unwrap(), BookmarkRootGuid::Root);
         assert_eq!(mobile.position, mobile_pos.unwrap());
 
+        let bm1 = fetch_public_tree(&conns.read, &SyncGuid::from("bookmark1___"))?.unwrap();
+        assert_eq!(bm1.parent_guid.unwrap(), BookmarkRootGuid::Mobile);
+        assert_eq!(bm1.position, 0);
+
         Ok(())
     }
     #[test]

components/places/src/tests.rs

@@ -23,7 +23,7 @@ pub fn insert_json_tree(conn: &PlacesDb, jtree: Value) {
 }
 
 pub fn assert_json_tree(conn: &PlacesDb, folder: &SyncGuid, expected: Value) {
-    let fetched = fetch_tree(conn, folder)
+    let (fetched, _, _) = fetch_tree(conn, folder)
         .expect("error fetching tree")
         .unwrap();
     let deser_tree: BookmarkTreeNode = serde_json::from_value(expected).unwrap();