Skip to content

Commit 6e0287d

Browse files
perf(metadata): paginate get_version_history to bound history reads
get_version_history scanned every recorded version on each call, so reads grew linearly with metadata churn. Add from_version/limit pagination so callers fetch at most O(limit) entries per call, and assert paged reads reconstruct the full history in order. Fixes #1028.
1 parent a9d0c73 commit 6e0287d

1 file changed

Lines changed: 80 additions & 6 deletions

File tree

contracts/metadata/src/lib.rs

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -570,16 +570,35 @@ mod propchain_metadata {
570570
// METADATA VERSIONING & HISTORY
571571
// ====================================================================
572572

573-
/// Gets metadata version history for a property
573+
/// Gets a page of metadata version history for a property.
574+
///
575+
/// Returns at most `limit` entries in ascending version order, starting
576+
/// at `from_version`, so callers can page through the full history with
577+
/// a bounded number of storage reads per call (O(limit) instead of
578+
/// O(total versions)). Pass the last returned entry's `version + 1` as
579+
/// `from_version` to fetch the next page; an empty result means the end
580+
/// of the history has been reached (or the property is unknown).
574581
#[ink(message)]
575-
pub fn get_version_history(&self, property_id: PropertyId) -> Vec<MetadataVersionEntry> {
582+
pub fn get_version_history(
583+
&self,
584+
property_id: PropertyId,
585+
from_version: MetadataVersion,
586+
limit: u32,
587+
) -> Vec<MetadataVersionEntry> {
576588
let metadata = match self.metadata.get(property_id) {
577589
Some(m) => m,
578590
None => return Vec::new(),
579591
};
592+
if limit == 0 || from_version > metadata.version {
593+
return Vec::new();
594+
}
595+
let end = u32::min(
596+
from_version.saturating_add(limit),
597+
metadata.version.saturating_add(1),
598+
);
580599

581-
let mut history = Vec::new();
582-
for v in 1..=metadata.version {
600+
let mut history = Vec::with_capacity((end - from_version) as usize);
601+
for v in from_version..end {
583602
if let Some(entry) = self.version_history.get((property_id, v)) {
584603
history.push(entry);
585604
}
@@ -929,15 +948,70 @@ mod propchain_metadata {
929948
Ok(2)
930949
);
931950

932-
let history = contract.get_version_history(1);
951+
let history = contract.get_version_history(1, 1, 100);
933952
assert_eq!(history.len(), 2);
934953
assert_eq!(history[0].version, 1);
935954
assert_eq!(history[1].version, 2);
936955
assert_eq!(history[1].change_description, "renovation");
937956
assert_eq!(contract.current_version(1), Some(2));
938957

939958
// Unknown properties have no history
940-
assert!(contract.get_version_history(99).is_empty());
959+
assert!(contract.get_version_history(99, 1, 100).is_empty());
960+
}
961+
962+
#[ink::test]
963+
fn test_paginated_version_history_reconstructs_full_history() {
964+
let accounts = test::default_accounts::<DefaultEnvironment>();
965+
let mut contract = setup();
966+
create_sample_metadata(&mut contract, 1);
967+
968+
// Grow the history to 1_000 versions: `version` is monotonic and
969+
// version entries are never removed, so this is the worst case the
970+
// paginated getter has to deal with.
971+
test::set_caller::<DefaultEnvironment>(accounts.bob);
972+
for i in 2..=1_000u32 {
973+
assert_eq!(
974+
contract.update_metadata(
975+
1,
976+
sample_core(),
977+
sample_ipfs(),
978+
[1u8; 32].into(),
979+
format!("update {}", i),
980+
None
981+
),
982+
Ok(i)
983+
);
984+
}
985+
assert_eq!(contract.current_version(1), Some(1_000));
986+
987+
// Page through the entire history in bounded chunks; the pages
988+
// must reconstruct the full history in ascending version order.
989+
let page_size = 100u32;
990+
let mut from_version = 1u32;
991+
let mut collected = Vec::new();
992+
loop {
993+
let page = contract.get_version_history(1, from_version, page_size);
994+
if page.is_empty() {
995+
break;
996+
}
997+
assert!(page.len() as u32 <= page_size);
998+
for (idx, entry) in page.iter().enumerate() {
999+
assert_eq!(
1000+
entry.version,
1001+
from_version + idx as u32,
1002+
"pages must be contiguous and in order"
1003+
);
1004+
}
1005+
collected.extend(page.iter().map(|e| e.version));
1006+
from_version = collected.last().expect("page is not empty") + 1;
1007+
}
1008+
1009+
assert_eq!(collected.len(), 1_000);
1010+
assert_eq!(collected, (1..=1_000).collect::<Vec<u32>>());
1011+
1012+
// A zero limit and an out-of-range start both return empty pages.
1013+
assert!(contract.get_version_history(1, 1, 0).is_empty());
1014+
assert!(contract.get_version_history(1, 1_001, 100).is_empty());
9411015
}
9421016

9431017
#[ink::test]

0 commit comments

Comments
 (0)