Skip to content

Commit 7449b74

Browse files
MathieuDutSikMathieuDutSik
authored
Remove load_entry_or_insert from CollectionView. (#4836)
## Motivation The function `load_entry_or_insert` loads an entry or inserts it and then returns a reference that cannot be used. The returned reference is not mutable, but the insertion feature forces the function to take a `&mut self`. So, we get the worst of both worlds: * We need a mutable input. * The returned object is not mutable. Also, if it has been created, then it is a default, which is not very interesting. ## Proposal The following steps are being made: * Remove altogether the `load_entry_or_insert` function. * Move the `do_load_entry_mut` code into `load_entry_mut`. * The `load_entry_or_insert` was used only for test purposes. The code is replaced by `load_entry_mut`. ## Test Plan The CI. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links None.
1 parent 65ac14a commit 7449b74

File tree

2 files changed

+51
-131
lines changed

2 files changed

+51
-131
lines changed

‎linera-views/src/views/collection_view.rs‎

Lines changed: 49 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -232,29 +232,44 @@ impl<W: View> ByteCollectionView<W::Context, W> {
232232
/// # })
233233
/// ```
234234
pub async fn load_entry_mut(&mut self, short_key: &[u8]) -> Result<&mut W, ViewError> {
235-
self.do_load_entry_mut(short_key).await
236-
}
237-
238-
/// Loads a subview for the data at the given index in the collection. If an entry
239-
/// is absent then a default entry is added to the collection. The resulting view
240-
/// is read-only.
241-
/// ```rust
242-
/// # tokio_test::block_on(async {
243-
/// # use linera_views::context::MemoryContext;
244-
/// # use linera_views::collection_view::ByteCollectionView;
245-
/// # use linera_views::register_view::RegisterView;
246-
/// # use linera_views::views::View;
247-
/// # let context = MemoryContext::new_for_testing(());
248-
/// let mut view: ByteCollectionView<_, RegisterView<_, String>> =
249-
/// ByteCollectionView::load(context).await.unwrap();
250-
/// view.load_entry_mut(&[0, 1]).await.unwrap();
251-
/// let subview = view.load_entry_or_insert(&[0, 1]).await.unwrap();
252-
/// let value = subview.get();
253-
/// assert_eq!(*value, String::default());
254-
/// # })
255-
/// ```
256-
pub async fn load_entry_or_insert(&mut self, short_key: &[u8]) -> Result<&W, ViewError> {
257-
Ok(self.do_load_entry_mut(short_key).await?)
235+
match self.updates.get_mut().entry(short_key.to_vec()) {
236+
btree_map::Entry::Occupied(entry) => {
237+
let entry = entry.into_mut();
238+
match entry {
239+
Update::Set(view) => Ok(view),
240+
Update::Removed => {
241+
let key = self
242+
.context
243+
.base_key()
244+
.base_tag_index(KeyTag::Subview as u8, short_key);
245+
let context = self.context.clone_with_base_key(key);
246+
// Obtain a view and set its pending state to the default (e.g. empty) state
247+
let view = W::new(context)?;
248+
*entry = Update::Set(view);
249+
let Update::Set(view) = entry else {
250+
unreachable!();
251+
};
252+
Ok(view)
253+
}
254+
}
255+
}
256+
btree_map::Entry::Vacant(entry) => {
257+
let key = self
258+
.context
259+
.base_key()
260+
.base_tag_index(KeyTag::Subview as u8, short_key);
261+
let context = self.context.clone_with_base_key(key);
262+
let view = if self.delete_storage_first {
263+
W::new(context)?
264+
} else {
265+
W::load(context).await?
266+
};
267+
let Update::Set(view) = entry.insert(Update::Set(view)) else {
268+
unreachable!();
269+
};
270+
Ok(view)
271+
}
272+
}
258273
}
259274

260275
/// Loads a subview for the data at the given index in the collection. If an entry
@@ -270,7 +285,7 @@ impl<W: View> ByteCollectionView<W::Context, W> {
270285
/// let mut view: ByteCollectionView<_, RegisterView<_, String>> =
271286
/// ByteCollectionView::load(context).await.unwrap();
272287
/// {
273-
/// let _subview = view.load_entry_or_insert(&[0, 1]).await.unwrap();
288+
/// let _subview = view.load_entry_mut(&[0, 1]).await.unwrap();
274289
/// }
275290
/// {
276291
/// let subview = view.try_load_entry(&[0, 1]).await.unwrap().unwrap();
@@ -330,7 +345,7 @@ impl<W: View> ByteCollectionView<W::Context, W> {
330345
/// let mut view: ByteCollectionView<_, RegisterView<_, String>> =
331346
/// ByteCollectionView::load(context).await.unwrap();
332347
/// {
333-
/// let _subview = view.load_entry_or_insert(&[0, 1]).await.unwrap();
348+
/// let _subview = view.load_entry_mut(&[0, 1]).await.unwrap();
334349
/// }
335350
/// let short_keys = vec![vec![0, 1], vec![2, 3]];
336351
/// let subviews = view.try_load_entries(short_keys).await.unwrap();
@@ -454,7 +469,7 @@ impl<W: View> ByteCollectionView<W::Context, W> {
454469
/// let mut view: ByteCollectionView<_, RegisterView<_, String>> =
455470
/// ByteCollectionView::load(context).await.unwrap();
456471
/// {
457-
/// let _subview = view.load_entry_or_insert(&[0, 1]).await.unwrap();
472+
/// let _subview = view.load_entry_mut(&[0, 1]).await.unwrap();
458473
/// }
459474
/// let subviews = view.try_load_all_entries().await.unwrap();
460475
/// assert_eq!(subviews.len(), 1);
@@ -621,47 +636,6 @@ impl<W: View> ByteCollectionView<W::Context, W> {
621636
pub fn extra(&self) -> &<W::Context as Context>::Extra {
622637
self.context.extra()
623638
}
624-
625-
async fn do_load_entry_mut(&mut self, short_key: &[u8]) -> Result<&mut W, ViewError> {
626-
match self.updates.get_mut().entry(short_key.to_vec()) {
627-
btree_map::Entry::Occupied(entry) => {
628-
let entry = entry.into_mut();
629-
match entry {
630-
Update::Set(view) => Ok(view),
631-
Update::Removed => {
632-
let key = self
633-
.context
634-
.base_key()
635-
.base_tag_index(KeyTag::Subview as u8, short_key);
636-
let context = self.context.clone_with_base_key(key);
637-
// Obtain a view and set its pending state to the default (e.g. empty) state
638-
let view = W::new(context)?;
639-
*entry = Update::Set(view);
640-
let Update::Set(view) = entry else {
641-
unreachable!();
642-
};
643-
Ok(view)
644-
}
645-
}
646-
}
647-
btree_map::Entry::Vacant(entry) => {
648-
let key = self
649-
.context
650-
.base_key()
651-
.base_tag_index(KeyTag::Subview as u8, short_key);
652-
let context = self.context.clone_with_base_key(key);
653-
let view = if self.delete_storage_first {
654-
W::new(context)?
655-
} else {
656-
W::load(context).await?
657-
};
658-
let Update::Set(view) = entry.insert(Update::Set(view)) else {
659-
unreachable!();
660-
};
661-
Ok(view)
662-
}
663-
}
664-
}
665639
}
666640

667641
impl<W: View> ByteCollectionView<W::Context, W> {
@@ -975,33 +949,6 @@ impl<I: Serialize, W: View> CollectionView<W::Context, I, W> {
975949
self.collection.load_entry_mut(&short_key).await
976950
}
977951

978-
/// Loads a subview for the data at the given index in the collection. If an entry
979-
/// is absent then a default entry is added to the collection. The resulting view
980-
/// is read-only.
981-
/// ```rust
982-
/// # tokio_test::block_on(async {
983-
/// # use linera_views::context::MemoryContext;
984-
/// # use linera_views::collection_view::CollectionView;
985-
/// # use linera_views::register_view::RegisterView;
986-
/// # use linera_views::views::View;
987-
/// # let context = MemoryContext::new_for_testing(());
988-
/// let mut view: CollectionView<_, u64, RegisterView<_, String>> =
989-
/// CollectionView::load(context).await.unwrap();
990-
/// view.load_entry_mut(&23).await.unwrap();
991-
/// let subview = view.load_entry_or_insert(&23).await.unwrap();
992-
/// let value = subview.get();
993-
/// assert_eq!(*value, String::default());
994-
/// # })
995-
/// ```
996-
pub async fn load_entry_or_insert<Q>(&mut self, index: &Q) -> Result<&W, ViewError>
997-
where
998-
I: Borrow<Q>,
999-
Q: Serialize + ?Sized,
1000-
{
1001-
let short_key = BaseKey::derive_short_key(index)?;
1002-
self.collection.load_entry_or_insert(&short_key).await
1003-
}
1004-
1005952
/// Loads a subview for the data at the given index in the collection. If an entry
1006953
/// is absent then `None` is returned. The resulting view cannot be modified.
1007954
/// May fail if one subview is already being visited.
@@ -1015,7 +962,7 @@ impl<I: Serialize, W: View> CollectionView<W::Context, I, W> {
1015962
/// let mut view: CollectionView<_, u64, RegisterView<_, String>> =
1016963
/// CollectionView::load(context).await.unwrap();
1017964
/// {
1018-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
965+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
1019966
/// }
1020967
/// {
1021968
/// let subview = view.try_load_entry(&23).await.unwrap().unwrap();
@@ -1049,7 +996,7 @@ impl<I: Serialize, W: View> CollectionView<W::Context, I, W> {
1049996
/// let mut view: CollectionView<_, u64, RegisterView<_, String>> =
1050997
/// CollectionView::load(context).await.unwrap();
1051998
/// {
1052-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
999+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
10531000
/// }
10541001
/// let indices = vec![23, 24];
10551002
/// let subviews = view.try_load_entries(&indices).await.unwrap();
@@ -1084,7 +1031,7 @@ impl<I: Serialize, W: View> CollectionView<W::Context, I, W> {
10841031
/// let mut view: CollectionView<_, u64, RegisterView<_, String>> =
10851032
/// CollectionView::load(context).await.unwrap();
10861033
/// {
1087-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1034+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
10881035
/// }
10891036
/// let indices = [23, 24];
10901037
/// let subviews = view.try_load_entries_pairs(indices).await.unwrap();
@@ -1116,7 +1063,7 @@ impl<I: Serialize, W: View> CollectionView<W::Context, I, W> {
11161063
/// let mut view: CollectionView<_, u64, RegisterView<_, String>> =
11171064
/// CollectionView::load(context).await.unwrap();
11181065
/// {
1119-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1066+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
11201067
/// }
11211068
/// let subviews = view.try_load_all_entries().await.unwrap();
11221069
/// assert_eq!(subviews.len(), 1);
@@ -1420,33 +1367,6 @@ impl<I: CustomSerialize, W: View> CustomCollectionView<W::Context, I, W> {
14201367
self.collection.load_entry_mut(&short_key).await
14211368
}
14221369

1423-
/// Loads a subview for the data at the given index in the collection. If an entry
1424-
/// is absent then a default entry is added to the collection. The resulting view
1425-
/// is read-only.
1426-
/// ```rust
1427-
/// # tokio_test::block_on(async {
1428-
/// # use linera_views::context::MemoryContext;
1429-
/// # use linera_views::collection_view::CustomCollectionView;
1430-
/// # use linera_views::register_view::RegisterView;
1431-
/// # use linera_views::views::View;
1432-
/// # let context = MemoryContext::new_for_testing(());
1433-
/// let mut view: CustomCollectionView<_, u128, RegisterView<_, String>> =
1434-
/// CustomCollectionView::load(context).await.unwrap();
1435-
/// view.load_entry_mut(&23).await.unwrap();
1436-
/// let subview = view.load_entry_or_insert(&23).await.unwrap();
1437-
/// let value = subview.get();
1438-
/// assert_eq!(*value, String::default());
1439-
/// # })
1440-
/// ```
1441-
pub async fn load_entry_or_insert<Q>(&mut self, index: &Q) -> Result<&W, ViewError>
1442-
where
1443-
I: Borrow<Q>,
1444-
Q: CustomSerialize,
1445-
{
1446-
let short_key = index.to_custom_bytes()?;
1447-
self.collection.load_entry_or_insert(&short_key).await
1448-
}
1449-
14501370
/// Loads a subview for the data at the given index in the collection. If an entry
14511371
/// is absent then `None` is returned. The resulting view cannot be modified.
14521372
/// May fail if one subview is already being visited.
@@ -1460,7 +1380,7 @@ impl<I: CustomSerialize, W: View> CustomCollectionView<W::Context, I, W> {
14601380
/// let mut view: CustomCollectionView<_, u128, RegisterView<_, String>> =
14611381
/// CustomCollectionView::load(context).await.unwrap();
14621382
/// {
1463-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1383+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
14641384
/// }
14651385
/// {
14661386
/// let subview = view.try_load_entry(&23).await.unwrap().unwrap();
@@ -1494,7 +1414,7 @@ impl<I: CustomSerialize, W: View> CustomCollectionView<W::Context, I, W> {
14941414
/// let mut view: CustomCollectionView<_, u128, RegisterView<_, String>> =
14951415
/// CustomCollectionView::load(context).await.unwrap();
14961416
/// {
1497-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1417+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
14981418
/// }
14991419
/// let subviews = view.try_load_entries(&[23, 42]).await.unwrap();
15001420
/// let value0 = subviews[0].as_ref().unwrap().get();
@@ -1528,7 +1448,7 @@ impl<I: CustomSerialize, W: View> CustomCollectionView<W::Context, I, W> {
15281448
/// let mut view: CustomCollectionView<_, u128, RegisterView<_, String>> =
15291449
/// CustomCollectionView::load(context).await.unwrap();
15301450
/// {
1531-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1451+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
15321452
/// }
15331453
/// let indices = [23, 42];
15341454
/// let subviews = view.try_load_entries_pairs(indices).await.unwrap();
@@ -1560,7 +1480,7 @@ impl<I: CustomSerialize, W: View> CustomCollectionView<W::Context, I, W> {
15601480
/// let mut view: CustomCollectionView<_, u128, RegisterView<_, String>> =
15611481
/// CustomCollectionView::load(context).await.unwrap();
15621482
/// {
1563-
/// let _subview = view.load_entry_or_insert(&23).await.unwrap();
1483+
/// let _subview = view.load_entry_mut(&23).await.unwrap();
15641484
/// }
15651485
/// let subviews = view.try_load_all_entries().await.unwrap();
15661486
/// assert_eq!(subviews.len(), 1);

‎linera-views/tests/views_tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ where
426426
assert!(!view.set.contains(&42).await?);
427427
}
428428
if config.with_collection {
429-
let subview = view.collection.load_entry_or_insert("hola").await?;
429+
let subview = view.collection.load_entry_mut("hola").await?;
430430
assert_eq!(subview.read(0..10).await?, Vec::<u32>::new());
431431
let subview = view.collection2.load_entry_mut("ciao").await?;
432432
let subsubview = subview.load_entry_mut("!").await?;
@@ -597,7 +597,7 @@ where
597597
{
598598
let mut view = store.load(1).await?;
599599
if config.with_collection {
600-
let subview = view.collection.load_entry_or_insert("hola").await?;
600+
let subview = view.collection.load_entry_mut("hola").await?;
601601
assert_eq!(subview.read(0..10).await?, Vec::<u32>::new());
602602
}
603603
if config.with_queue {

0 commit comments

Comments
 (0)