From 2cd2cdda1748ea5b5ac22d2217025b2cfc74a9b5 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:02:03 +0200 Subject: [PATCH 01/12] feat: add convenience method --- .../places/src/storage/history_metadata.rs | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index bfba80fc2a..e4062f18f9 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -440,9 +440,8 @@ lazy_static! { "{common_select_sql} WHERE updated_at >= :start ORDER BY updated_at DESC - LIMIT {max_limit}", - common_select_sql = COMMON_METADATA_SELECT, - max_limit = MAX_QUERY_RESULTS + LIMIT :limit", + common_select_sql = COMMON_METADATA_SELECT ); static ref QUERY_SQL: String = format!( "{common_select_sql} @@ -477,11 +476,30 @@ pub fn get_between(db: &PlacesDb, start: i64, end: i64) -> Result Result> { db.query_rows_and_then_cached( GET_SINCE_SQL.as_str(), rusqlite::named_params! { - ":start": start + ":start": start, + ":limit": MAX_QUERY_RESULTS, + }, + HistoryMetadata::from_row, + ) +} + +// Returns the most recent history metadata entries (newest first), +// limited by `limit`. +// +// Internally this uses [`GET_SINCE_SQL`] with `start = i64::MIN` +// to include all entries, ordered by descending `updated_at`. +pub fn get_most_recent(db: &PlacesDb, limit: i32) -> Result> { + db.query_rows_and_then_cached( + GET_SINCE_SQL.as_str(), + rusqlite::named_params! { + ":start": i64::MIN, + ":limit": limit, }, HistoryMetadata::from_row, ) From 24e7b7162c4f1523a843e15653950173d20566d4 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:03:13 +0200 Subject: [PATCH 02/12] feat: add tests for --- .../places/src/storage/history_metadata.rs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index e4062f18f9..d43e406c15 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1315,6 +1315,62 @@ mod tests { assert_eq!(0, get_since(&conn, after_meta2).unwrap().len()); } + #[test] + fn test_get_most_recent_empty() { + let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); + let rows = get_most_recent(&conn, 5).expect("query ok"); + assert!(rows.is_empty()); + } + + fn test_get_most_recent_orders_and_limits() { + let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); + + note_observation!(&conn, + url "https://example.com/1", + view_time Some(10), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/2", + view_time Some(20), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/3", + view_time Some(30), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + // Limiting to 1 should return the most recent entry only. + let result1 = get_most_recent(&conn, 1).expect("query ok"); + assert_eq!(result1.len(), 1); + assert_eq!(result1[0].url, "https://example.com/3"); + + // Limiting to 2 should return the two most recent entries. + let result2 = get_most_recent(&conn, 2).expect("query ok"); + assert_eq!(result2.len(), 2); + assert_eq!(result2[0].url, "https://example.com/3"); + assert_eq!(result2[1].url, "https://example.com/2"); + + // Limiting to 10 should return all three entries, in the correct order. + let result3 = get_most_recent(&conn, 10).expect("query ok"); + assert_eq!(result3.len(), 3); + assert_eq!(result3[0].url, "https://example.com/3"); + assert_eq!(result3[1].url, "https://example.com/2"); + assert_eq!(result3[2].url, "https://example.com/1"); + } + #[test] fn test_get_highlights() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); From db0f26e4f51104b0e811ca3d0d073be1577a29a3 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:03:31 +0200 Subject: [PATCH 03/12] feat: add ffi wrapper method --- components/places/src/ffi.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/places/src/ffi.rs b/components/places/src/ffi.rs index 07755236f3..610bba5c29 100644 --- a/components/places/src/ffi.rs +++ b/components/places/src/ffi.rs @@ -208,6 +208,11 @@ impl PlacesConnection { self.with_conn(|conn| history_metadata::get_since(conn, start.as_millis_i64())) } + #[handle_error(crate::Error)] + pub fn get_most_recent_history_metadata(&self, limit: i32) -> ApiResult> { + self.with_conn(|conn| history_metadata::get_most_recent(conn, limit)) + } + #[handle_error(crate::Error)] pub fn query_history_metadata( &self, From 4eb21edaf0f7fb7b8732b1e229bb7852be29f5a5 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:05:32 +0200 Subject: [PATCH 04/12] fix: missing test macro --- components/places/src/storage/history_metadata.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index d43e406c15..94d309fef7 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1322,6 +1322,7 @@ mod tests { assert!(rows.is_empty()); } + #[test] fn test_get_most_recent_orders_and_limits() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); From 24d1e3f005184ed8c9a8154e7a0490a13115d051 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:20:05 +0200 Subject: [PATCH 05/12] fix: add more tests --- .../places/src/storage/history_metadata.rs | 63 +++++++++++++++---- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index 94d309fef7..33312cc3f1 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1354,22 +1354,61 @@ mod tests { ); // Limiting to 1 should return the most recent entry only. - let result1 = get_most_recent(&conn, 1).expect("query ok"); - assert_eq!(result1.len(), 1); - assert_eq!(result1[0].url, "https://example.com/3"); + let most_recents1 = get_most_recent(&conn, 1).expect("query ok"); + assert_eq!(most_recents1.len(), 1); + assert_eq!(most_recents1[0].url, "https://example.com/3"); // Limiting to 2 should return the two most recent entries. - let result2 = get_most_recent(&conn, 2).expect("query ok"); - assert_eq!(result2.len(), 2); - assert_eq!(result2[0].url, "https://example.com/3"); - assert_eq!(result2[1].url, "https://example.com/2"); + let most_recents2 = get_most_recent(&conn, 2).expect("query ok"); + assert_eq!(most_recents2.len(), 2); + assert_eq!(most_recents2[0].url, "https://example.com/3"); + assert_eq!(most_recents2[1].url, "https://example.com/2"); // Limiting to 10 should return all three entries, in the correct order. - let result3 = get_most_recent(&conn, 10).expect("query ok"); - assert_eq!(result3.len(), 3); - assert_eq!(result3[0].url, "https://example.com/3"); - assert_eq!(result3[1].url, "https://example.com/2"); - assert_eq!(result3[2].url, "https://example.com/1"); + let most_recents3 = get_most_recent(&conn, 10).expect("query ok"); + assert_eq!(most_recents3.len(), 3); + assert_eq!(most_recents3[0].url, "https://example.com/3"); + assert_eq!(most_recents3[1].url, "https://example.com/2"); + assert_eq!(most_recents3[2].url, "https://example.com/1"); + } + + #[test] + fn test_get_most_recent_negative_limit() { + let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); + + note_observation!(&conn, + url "https://example.com/1", + view_time Some(10), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/2", + view_time Some(20), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/3", + view_time Some(30), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + // Limiting to -1 should return all entries properly ordered. + let most_recents = get_most_recent(&conn, -1).expect("query ok"); + assert_eq!(most_recents.len(), 3); + assert_eq!(most_recents[0].url, "https://example.com/3"); + assert_eq!(most_recents[1].url, "https://example.com/2"); + assert_eq!(most_recents[2].url, "https://example.com/1"); } #[test] From 43a5eb3e20981173356666a0c2a86101ac32ac8a Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:25:15 +0200 Subject: [PATCH 06/12] docs: add changelog entry --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac127266c..130728f12a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## ✨ What's New ✨ ### Swift -- Added `@unchecked Sendable` to classes that conform to `FeatureManifestInterface`. ([#6963](https://github.com/mozilla/application-services/pull/6963) +- Added `@unchecked Sendable` to classes that conform to `FeatureManifestInterface`. ([#6963](https://github.com/mozilla/application-services/pull/6963)) ### Ads Client - Added the Ads Client component to the Megazord. @@ -16,6 +16,10 @@ - Downstream client apps can now handle server errors based on both the `status` and `error_code` fields directly, without additional changes to the Rust component - even as server-side error codes evolve. - **Consumers must update their error handling code to match the new `Api { status, code, detail }` shape.** +### Places +- `places::storage::history_metadata::get_most_recent(limit: i32)` was added to get most recent history metadata limited to a number. ([#7002](https://github.com/mozilla/application-services/pull/7002)) + + ## 🦊 What's Changed 🦊 ### Docs From 3ccc89e53661fb48d0ee4565251d736511b8299e Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Tue, 7 Oct 2025 12:29:05 +0200 Subject: [PATCH 07/12] fix: format --- components/places/src/storage/history_metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index 33312cc3f1..79e8445737 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1322,7 +1322,7 @@ mod tests { assert!(rows.is_empty()); } - #[test] + #[test] fn test_get_most_recent_orders_and_limits() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); @@ -1372,7 +1372,7 @@ mod tests { assert_eq!(most_recents3[2].url, "https://example.com/1"); } - #[test] + #[test] fn test_get_most_recent_negative_limit() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); From 11a247a7752aee50d37251ebc8470a289b9b338d Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Wed, 8 Oct 2025 12:23:19 +0200 Subject: [PATCH 08/12] fix: add test with same viewtimes --- .../places/src/storage/history_metadata.rs | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index 79e8445737..5759856550 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1323,7 +1323,57 @@ mod tests { } #[test] - fn test_get_most_recent_orders_and_limits() { + fn test_get_most_recent_orders_and_limits_same_viewtime() { + let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); + + note_observation!(&conn, + url "https://example.com/1", + view_time Some(10), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/2", + view_time Some(10), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + note_observation!(&conn, + url "https://example.com/3", + view_time Some(10), + search_term None, + document_type Some(DocumentType::Regular), + referrer_url None, + title None + ); + + // Limiting to 1 should return the most recent entry only. + let most_recents1 = get_most_recent(&conn, 1).expect("query ok"); + assert_eq!(most_recents1.len(), 1); + assert_eq!(most_recents1[0].url, "https://example.com/3"); + + // Limiting to 2 should return the two most recent entries. + let most_recents2 = get_most_recent(&conn, 2).expect("query ok"); + assert_eq!(most_recents2.len(), 2); + assert_eq!(most_recents2[0].url, "https://example.com/3"); + assert_eq!(most_recents2[1].url, "https://example.com/2"); + + // Limiting to 10 should return all three entries, in the correct order. + let most_recents3 = get_most_recent(&conn, 10).expect("query ok"); + assert_eq!(most_recents3.len(), 3); + assert_eq!(most_recents3[0].url, "https://example.com/3"); + assert_eq!(most_recents3[1].url, "https://example.com/2"); + assert_eq!(most_recents3[2].url, "https://example.com/1"); + } + + #[test] + fn test_get_most_recent_orders_and_limits_incrementing_viewtime() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); note_observation!(&conn, @@ -1387,7 +1437,7 @@ mod tests { note_observation!(&conn, url "https://example.com/2", - view_time Some(20), + view_time Some(10), search_term None, document_type Some(DocumentType::Regular), referrer_url None, @@ -1396,7 +1446,7 @@ mod tests { note_observation!(&conn, url "https://example.com/3", - view_time Some(30), + view_time Some(10), search_term None, document_type Some(DocumentType::Regular), referrer_url None, From d7eaab98d8dbee7ae1b4f28775a41655f1349e06 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Wed, 8 Oct 2025 12:31:14 +0200 Subject: [PATCH 09/12] doc: add comment about removing --- components/places/src/storage/history_metadata.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index 5759856550..f0fc14a212 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -478,6 +478,8 @@ pub fn get_between(db: &PlacesDb, start: i64, end: i64) -> Result Result> { db.query_rows_and_then_cached( GET_SINCE_SQL.as_str(), From 50eee6bb8cadaf85f4fdd44496410adb35c11e6d Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Wed, 8 Oct 2025 12:32:58 +0200 Subject: [PATCH 10/12] fix: add method to udl file --- components/places/src/places.udl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/places/src/places.udl b/components/places/src/places.udl index 447f7984ed..13e190d4e6 100644 --- a/components/places/src/places.udl +++ b/components/places/src/places.udl @@ -59,6 +59,9 @@ interface PlacesConnection { [Throws=PlacesApiError] sequence get_history_metadata_since(PlacesTimestamp since); + [Throws=PlacesApiError] + sequence get_most_recent_history_metadata(i32 limit); + [Throws=PlacesApiError] sequence query_autocomplete(string search, i32 limit); From 77cca7fd5e273ed2c1049ad641107cc40e38da2e Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Wed, 8 Oct 2025 12:44:20 +0200 Subject: [PATCH 11/12] fix: add test with same/diff observations --- .../places/src/storage/history_metadata.rs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index f0fc14a212..2797428964 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -1325,7 +1325,7 @@ mod tests { } #[test] - fn test_get_most_recent_orders_and_limits_same_viewtime() { + fn test_get_most_recent_orders_and_limits_same_observation() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); note_observation!(&conn, @@ -1338,7 +1338,7 @@ mod tests { ); note_observation!(&conn, - url "https://example.com/2", + url "https://example.com/1", view_time Some(10), search_term None, document_type Some(DocumentType::Regular), @@ -1347,7 +1347,7 @@ mod tests { ); note_observation!(&conn, - url "https://example.com/3", + url "https://example.com/1", view_time Some(10), search_term None, document_type Some(DocumentType::Regular), @@ -1358,24 +1358,21 @@ mod tests { // Limiting to 1 should return the most recent entry only. let most_recents1 = get_most_recent(&conn, 1).expect("query ok"); assert_eq!(most_recents1.len(), 1); - assert_eq!(most_recents1[0].url, "https://example.com/3"); + assert_eq!(most_recents1[0].url, "https://example.com/1"); - // Limiting to 2 should return the two most recent entries. - let most_recents2 = get_most_recent(&conn, 2).expect("query ok"); - assert_eq!(most_recents2.len(), 2); - assert_eq!(most_recents2[0].url, "https://example.com/3"); - assert_eq!(most_recents2[1].url, "https://example.com/2"); + // Limiting to 3 should also return one entry, since we only have one unique URL. + let most_recents2 = get_most_recent(&conn, 3).expect("query ok"); + assert_eq!(most_recents2.len(), 1); + assert_eq!(most_recents2[0].url, "https://example.com/1"); - // Limiting to 10 should return all three entries, in the correct order. + // Limiting to 10 should also return one entry, since we only have one unique URL. let most_recents3 = get_most_recent(&conn, 10).expect("query ok"); - assert_eq!(most_recents3.len(), 3); - assert_eq!(most_recents3[0].url, "https://example.com/3"); - assert_eq!(most_recents3[1].url, "https://example.com/2"); - assert_eq!(most_recents3[2].url, "https://example.com/1"); + assert_eq!(most_recents3.len(), 1); + assert_eq!(most_recents3[0].url, "https://example.com/1"); } #[test] - fn test_get_most_recent_orders_and_limits_incrementing_viewtime() { + fn test_get_most_recent_orders_and_limits_different_observations() { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); note_observation!(&conn, From f8acb29202c6e93b7ca5f15f9cb9ac4ec268e228 Mon Sep 17 00:00:00 2001 From: Issam Mani Date: Wed, 8 Oct 2025 15:41:13 +0200 Subject: [PATCH 12/12] test: add clock helper to sleep between queries --- .../places/src/storage/history_metadata.rs | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/components/places/src/storage/history_metadata.rs b/components/places/src/storage/history_metadata.rs index 2797428964..70e4a0981c 100644 --- a/components/places/src/storage/history_metadata.rs +++ b/components/places/src/storage/history_metadata.rs @@ -819,6 +819,14 @@ mod tests { use crate::VisitTransitionSet; use std::{thread, time}; + // NOTE: `updated_at` timestamps have millisecond precision, so multiple observations + // written in the same millisecond can in theory share the same value. To avoid flaky + // ordering in tests ( since we have `ORDER BY updated_at` in a few queries ) + // this helper sleeps briefly to avoid having overlapping timestamps. + fn bump_clock() { + thread::sleep(time::Duration::from_millis(10)); + } + macro_rules! assert_table_size { ($conn:expr, $table:expr, $count:expr) => { assert_eq!( @@ -1254,7 +1262,7 @@ mod tests { assert_eq!(0, get_between(&conn, 0, beginning - 1).unwrap().len()); assert_eq!(1, get_between(&conn, 0, after_meta1).unwrap().len()); - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/video/", @@ -1337,6 +1345,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/1", view_time Some(10), @@ -1346,6 +1356,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/1", view_time Some(10), @@ -1384,6 +1396,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/2", view_time Some(20), @@ -1393,6 +1407,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/3", view_time Some(30), @@ -1434,6 +1450,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/2", view_time Some(10), @@ -1443,6 +1461,8 @@ mod tests { title None ); + bump_clock(); + note_observation!(&conn, url "https://example.com/3", view_time Some(10), @@ -1787,7 +1807,7 @@ mod tests { title None ); - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); // same observation a bit later: note_observation!(&conn, url "http://mozilla.com/2", @@ -1862,7 +1882,7 @@ mod tests { ); let after_meta1 = Timestamp::now().as_millis() as i64; - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/2", @@ -1873,7 +1893,7 @@ mod tests { title None ); - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/3", @@ -1908,7 +1928,7 @@ mod tests { let conn = PlacesDb::open_in_memory(ConnectionType::ReadWrite).expect("memory db"); let beginning = Timestamp::now().as_millis() as i64; - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/1", @@ -1919,7 +1939,7 @@ mod tests { title None ); - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/2", @@ -1931,7 +1951,7 @@ mod tests { ); let after_meta2 = Timestamp::now().as_millis() as i64; - thread::sleep(time::Duration::from_millis(10)); + bump_clock(); note_observation!(&conn, url "http://mozilla.com/3",