Skip to content

Commit

Permalink
Add a getBookmarkURLForKeyword API for iOS.
Browse files Browse the repository at this point in the history
Rust Places doesn't support setting keywords for URLs, and it's unclear
if we'll ever port keywords as they exist on Desktop today. However,
we do sync and round-trip keywords set on Desktop, so we can read them
directly from the `moz_bookmarks_synced` table.

Closes #953.
  • Loading branch information
linabutler authored and tinoism committed Jul 25, 2019
1 parent fa3a638 commit faaa8ab
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 0 deletions.
Expand Up @@ -214,6 +214,17 @@ interface ReadableBookmarksConnection : InterruptibleConnection {
*/
fun getBookmarksWithURL(url: String): List<BookmarkItem>

/**
* Returns the URL for the provided search keyword, if one exists.
*
* @param The search keyword.
* @return The bookmarked URL for the keyword, if set.
*
* @throws OperationInterrupted if this database implements [InterruptibleConnection] and
* has its `interrupt()` method called on another thread.
*/
fun getBookmarkUrlForKeyword(keyword: String): String?

/**
* Returns the list of bookmarks that match the provided search string.
*
Expand Down
Expand Up @@ -186,6 +186,12 @@ internal interface LibPlacesFFI : Library {
error: RustError.ByReference
): RustBuffer.ByValue

fun bookmarks_get_url_for_keyword(
handle: PlacesConnectionHandle,
keyword: String,
error: RustError.ByReference
): Pointer?

fun bookmarks_get_tree(
handle: PlacesConnectionHandle,
optRootId: String?,
Expand Down
Expand Up @@ -327,6 +327,18 @@ open class PlacesReaderConnection internal constructor(connHandle: Long) :
}
}

override fun getBookmarkUrlForKeyword(keyword: String): String? {
val urlPtr = rustCall { error ->
LibPlacesFFI.INSTANCE.bookmarks_get_url_for_keyword(this.handle.get(), keyword, error)
}

try {
return urlPtr?.getString(0, "utf-8")
} finally {
urlPtr?.let { LibPlacesFFI.INSTANCE.places_destroy_string(it) }
}
}

override fun searchBookmarks(query: String, limit: Int): List<BookmarkItem> {
val rustBuf = rustCall { err ->
LibPlacesFFI.INSTANCE.bookmarks_search(this.handle.get(), query, limit, err)
Expand Down
13 changes: 13 additions & 0 deletions components/places/ffi/src/lib.rs
Expand Up @@ -568,6 +568,19 @@ pub extern "C" fn bookmarks_get_all_with_url(
})
}

#[no_mangle]
pub unsafe extern "C" fn bookmarks_get_url_for_keyword(
handle: u64,
keyword: FfiStr<'_>,
error: &mut ExternError,
) -> *mut c_char {
log::debug!("bookmarks_get_url_for_keyword");
CONNECTIONS.call_with_result(error, handle, |conn| -> places::Result<_> {
let url = bookmarks::bookmarks_get_url_for_keyword(conn, keyword.as_str())?;
Ok(url.map(url::Url::into_string))
})
}

#[no_mangle]
pub extern "C" fn bookmarks_search(
handle: u64,
Expand Down
31 changes: 31 additions & 0 deletions components/places/ios/Places/Places.swift
Expand Up @@ -382,6 +382,37 @@ public class PlacesReadConnection {
}
}

/**
* Returns the URL for the provided search keyword, if one exists.
*
* - Parameter keyword: The search keyword.
* - Returns: The bookmarked URL for the keyword, if set.
* - Throws:
* - `PlacesError.databaseInterrupted`: If a call is made to `interrupt()` on this
* object from another thread.
* - `PlacesError.connUseAfterAPIClosed`: If the PlacesAPI that returned this connection
* object has been closed. This indicates API
* misuse.
* - `PlacesError.databaseBusy`: If this query times out with a SQLITE_BUSY error.
* - `PlacesError.unexpected`: When an error that has not specifically been exposed
* to Swift is encountered (for example IO errors from
* the database code, etc).
* - `PlacesError.panic`: If the rust code panics while completing this
* operation. (If this occurs, please let us know).
*/
open func getBookmarkURLForKeyword(keyword: String) throws -> String? {
return try queue.sync {
try self.checkApi()
let maybeURL = try PlacesError.tryUnwrap { error in
bookmarks_get_url_for_keyword(self.handle, keyword, error)
}
guard let url = maybeURL else {
return nil
}
return String(freeingPlacesString: url)
}
}

/**
* Returns the list of bookmarks that match the provided search string.
*
Expand Down
4 changes: 4 additions & 0 deletions components/places/ios/Places/RustPlacesAPI.h
Expand Up @@ -157,6 +157,10 @@ PlacesRustBuffer bookmarks_get_all_with_url(PlacesConnectionHandle handle,
char const *_Nonnull url,
PlacesRustError *_Nonnull out_err);

char *_Nullable bookmarks_get_url_for_keyword(PlacesConnectionHandle handle,
char const *_Nonnull keyword,
PlacesRustError *_Nonnull out_err);

PlacesRustBuffer bookmarks_search(PlacesConnectionHandle handle,
char const *_Nonnull query,
int32_t limit,
Expand Down
68 changes: 68 additions & 0 deletions components/places/src/storage/bookmarks.rs
Expand Up @@ -901,6 +901,23 @@ impl<'de> Deserialize<'de> for BookmarkTreeNode {
}
}

/// Get the URL of the bookmark matching a keyword
pub fn bookmarks_get_url_for_keyword(db: &PlacesDb, keyword: &str) -> Result<Option<Url>> {
let bookmark_url = db.try_query_row(
"SELECT url FROM moz_places p
JOIN moz_bookmarks_synced b ON b.placeId = p.id
WHERE b.keyword = :keyword",
&[(":keyword", &keyword)],
|row| row.get::<_, String>("url"),
true,
)?;

match bookmark_url {
Some(b) => Ok(Some(Url::parse(&b)?)),
None => Ok(None),
}
}

#[cfg(test)]
mod test_serialize {
use super::*;
Expand Down Expand Up @@ -1408,6 +1425,57 @@ mod tests {
.position
}

#[test]
fn test_bookmark_url_for_keyword() -> Result<()> {
let conn = new_mem_connection();

let url = Url::parse("http://example.com")
.expect("valid url")
.into_string();

conn.execute_named_cached(
"INSERT INTO moz_places (guid, url, url_hash) VALUES ('fake_guid___', :url, hash(:url))",
&[(":url", &url)],
)
.expect("should work");
let place_id = conn.last_insert_rowid();

// create a bookmark with keyword 'donut' pointing at it.
conn.execute_named_cached(
"INSERT INTO moz_bookmarks_synced
(keyword, placeId, guid)
VALUES
('donut', :place_id, 'fake_guid___')",
&[(":place_id", &place_id)],
)
.expect("should work");

assert_eq!(
bookmarks_get_url_for_keyword(&conn, "donut")?,
Some(Url::parse("http://example.com")?)
);
assert_eq!(bookmarks_get_url_for_keyword(&conn, "juice")?, None);

// now change the keyword to 'ice cream'
conn.execute_named_cached(
"REPLACE INTO moz_bookmarks_synced
(keyword, placeId, guid)
VALUES
('ice cream', :place_id, 'fake_guid___')",
&[(":place_id", &place_id)],
)
.expect("should work");

assert_eq!(
bookmarks_get_url_for_keyword(&conn, "ice cream")?,
Some(Url::parse("http://example.com")?)
);
assert_eq!(bookmarks_get_url_for_keyword(&conn, "donut")?, None);
assert_eq!(bookmarks_get_url_for_keyword(&conn, "ice")?, None);

Ok(())
}

#[test]
fn test_insert() -> Result<()> {
let _ = env_logger::try_init();
Expand Down

0 comments on commit faaa8ab

Please sign in to comment.