fix: patching keyed store fields (closes #3957)

This commit is contained in:
Greg Johnston
2025-10-13 10:52:45 -04:00
parent c025ae59ac
commit 578b672f14
10 changed files with 207 additions and 23 deletions

View File

@@ -30,6 +30,8 @@ where
defined_at: &'static Location<'static>,
path: Arc<dyn Fn() -> StorePath + Send + Sync>,
get_trigger: Arc<dyn Fn(StorePath) -> StoreFieldTrigger + Send + Sync>,
get_trigger_unkeyed:
Arc<dyn Fn(StorePath) -> StoreFieldTrigger + Send + Sync>,
read: Arc<dyn Fn() -> Option<StoreFieldReader<T>> + Send + Sync>,
pub(crate) write:
Arc<dyn Fn() -> Option<StoreFieldWriter<T>> + Send + Sync>,
@@ -103,6 +105,10 @@ impl<T> StoreField for ArcField<T> {
(self.get_trigger)(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
(self.get_trigger_unkeyed)(path)
}
fn path(&self) -> impl IntoIterator<Item = StorePathSegment> {
(self.path)()
}
@@ -132,6 +138,9 @@ where
defined_at: Location::caller(),
path: Arc::new(move || value.path().into_iter().collect()),
get_trigger: Arc::new(move |path| value.get_trigger(path)),
get_trigger_unkeyed: Arc::new(move |path| {
value.get_trigger_unkeyed(path)
}),
read: Arc::new(move || value.reader().map(StoreFieldReader::new)),
write: Arc::new(move || value.writer().map(StoreFieldWriter::new)),
keys: Arc::new(move || value.keys()),
@@ -158,6 +167,10 @@ where
let value = value.clone();
move |path| value.get_trigger(path)
}),
get_trigger_unkeyed: Arc::new({
let value = value.clone();
move |path| value.get_trigger_unkeyed(path)
}),
read: Arc::new({
let value = value.clone();
move || value.reader().map(StoreFieldReader::new)
@@ -202,6 +215,10 @@ where
let value = value.clone();
move |path| value.get_trigger(path)
}),
get_trigger_unkeyed: Arc::new({
let value = value.clone();
move |path| value.get_trigger_unkeyed(path)
}),
read: Arc::new({
let value = value.clone();
move || value.reader().map(StoreFieldReader::new)
@@ -245,6 +262,10 @@ where
let value = value.clone();
move |path| value.get_trigger(path)
}),
get_trigger_unkeyed: Arc::new({
let value = value.clone();
move |path| value.get_trigger_unkeyed(path)
}),
read: Arc::new({
let value = value.clone();
move || value.reader().map(StoreFieldReader::new)
@@ -289,6 +310,10 @@ where
let value = value.clone();
move |path| value.get_trigger(path)
}),
get_trigger_unkeyed: Arc::new({
let value = value.clone();
move |path| value.get_trigger_unkeyed(path)
}),
read: Arc::new({
let value = value.clone();
move || value.reader().map(StoreFieldReader::new)
@@ -337,6 +362,10 @@ where
let value = value.clone();
move |path| value.get_trigger(path)
}),
get_trigger_unkeyed: Arc::new({
let value = value.clone();
move |path| value.get_trigger_unkeyed(path)
}),
read: Arc::new({
let value = value.clone();
move || value.reader().map(StoreFieldReader::new)
@@ -368,6 +397,7 @@ impl<T> Clone for ArcField<T> {
defined_at: self.defined_at,
path: self.path.clone(),
get_trigger: Arc::clone(&self.get_trigger),
get_trigger_unkeyed: Arc::clone(&self.get_trigger_unkeyed),
read: Arc::clone(&self.read),
write: Arc::clone(&self.write),
keys: Arc::clone(&self.keys),

View File

@@ -68,6 +68,11 @@ where
fn get_trigger(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger_unkeyed(path)
}
fn path(&self) -> impl IntoIterator<Item = StorePathSegment> {
self.inner.path()
}

View File

@@ -59,6 +59,13 @@ where
.unwrap_or_default()
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner
.try_get_value()
.map(|inner| inner.get_trigger_unkeyed(path))
.unwrap_or_default()
}
fn path(&self) -> impl IntoIterator<Item = StorePathSegment> {
self.inner
.try_get_value()

View File

@@ -84,6 +84,10 @@ where
self.inner.get_trigger(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger_unkeyed(path)
}
fn reader(&self) -> Option<Self::Reader> {
let inner = self.inner.reader()?;
let index = self.index;

View File

@@ -110,6 +110,10 @@ where
self.inner.get_trigger(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger_unkeyed(path)
}
fn reader(&self) -> Option<Self::Reader> {
let inner = self.inner.reader()?;
Some(Mapped::new_with_guard(inner, self.read))
@@ -432,7 +436,7 @@ where
let this = keys
.with_field_keys(
inner.clone(),
|keys| keys.get(&self.key),
|keys| (keys.get(&self.key), vec![]),
|| self.inner.latest_keys(),
)
.flatten()
@@ -444,6 +448,10 @@ where
self.inner.get_trigger(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger_unkeyed(path)
}
fn reader(&self) -> Option<Self::Reader> {
let inner = self.inner.reader()?;
@@ -452,7 +460,7 @@ where
let index = keys
.with_field_keys(
inner_path,
|keys| keys.get(&self.key),
|keys| (keys.get(&self.key), vec![]),
|| self.inner.latest_keys(),
)
.flatten()
@@ -476,7 +484,7 @@ where
let index = keys
.with_field_keys(
inner_path.clone(),
|keys| keys.get(&self.key),
|keys| (keys.get(&self.key), vec![]),
|| self.inner.latest_keys(),
)
.flatten()
@@ -624,9 +632,7 @@ where
let latest = self.latest_keys();
keys.with_field_keys(
inner_path,
|keys| {
keys.update(latest);
},
|keys| ((), keys.update(latest)),
|| self.latest_keys(),
);
}

View File

@@ -364,13 +364,18 @@ where
})
}
fn update(&mut self, iter: impl IntoIterator<Item = K>) {
fn update(
&mut self,
iter: impl IntoIterator<Item = K>,
) -> Vec<(usize, StorePathSegment)> {
let new_keys = iter
.into_iter()
.enumerate()
.map(|(idx, key)| (key, idx))
.collect::<FxHashMap<K, usize>>();
let mut index_keys = Vec::with_capacity(new_keys.len());
// remove old keys and recycle the slots
self.keys.retain(|key, old_entry| match new_keys.get(key) {
Some(idx) => {
@@ -385,14 +390,17 @@ where
// add new keys
for (key, idx) in new_keys {
// the suggestion doesn't compile because we need &mut for self.next_key(),
// and we don't want to call that until after the check
#[allow(clippy::map_entry)]
if !self.keys.contains_key(&key) {
let path = self.next_key();
self.keys.insert(key, (path, idx));
match self.keys.get(&key) {
Some((segment, idx)) => index_keys.push((*idx, *segment)),
None => {
let path = self.next_key();
self.keys.insert(key, (path, idx));
index_keys.push((idx, path));
}
}
}
index_keys
}
}
@@ -415,14 +423,20 @@ type HashMap<K, V> = send_wrapper::SendWrapper<
/// A map of the keys for a keyed subfield.
#[derive(Clone)]
pub struct KeyMap(HashMap<StorePath, Box<dyn Any + Send + Sync>>);
pub struct KeyMap(
HashMap<StorePath, Box<dyn Any + Send + Sync>>,
HashMap<(StorePath, usize), StorePathSegment>,
);
impl Default for KeyMap {
fn default() -> Self {
#[cfg(not(target_arch = "wasm32"))]
return Self(Default::default());
return Self(Default::default(), Default::default());
#[cfg(target_arch = "wasm32")]
return Self(send_wrapper::SendWrapper::new(Default::default()));
return Self(
send_wrapper::SendWrapper::new(Default::default()),
send_wrapper::SendWrapper::new(Default::default()),
);
}
}
@@ -430,31 +444,70 @@ impl KeyMap {
fn with_field_keys<K, T>(
&self,
path: StorePath,
fun: impl FnOnce(&mut FieldKeys<K>) -> T,
fun: impl FnOnce(&mut FieldKeys<K>) -> (T, Vec<(usize, StorePathSegment)>),
initialize: impl FnOnce() -> Vec<K>,
) -> Option<T>
where
K: Debug + Hash + PartialEq + Eq + Send + Sync + 'static,
{
let initial_keys = initialize();
#[cfg(not(target_arch = "wasm32"))]
let mut entry = self
.0
.entry(path)
.or_insert_with(|| Box::new(FieldKeys::new(initialize())));
.entry(path.clone())
.or_insert_with(|| Box::new(FieldKeys::new(initial_keys)));
#[cfg(target_arch = "wasm32")]
let entry = if !self.0.borrow().contains_key(&path) {
Some(Box::new(FieldKeys::new(initialize())))
Some(Box::new(FieldKeys::new(initial_keys)))
} else {
None
};
#[cfg(target_arch = "wasm32")]
let mut map = self.0.borrow_mut();
#[cfg(target_arch = "wasm32")]
let entry = map.entry(path).or_insert_with(|| entry.unwrap());
let entry = map.entry(path.clone()).or_insert_with(|| entry.unwrap());
let entry = entry.downcast_mut::<FieldKeys<K>>()?;
Some(fun(entry))
let (result, new_keys) = fun(entry);
if !new_keys.is_empty() {
for (idx, segment) in new_keys {
#[cfg(not(target_arch = "wasm32"))]
self.1.insert((path.clone(), idx), segment);
#[cfg(target_arch = "wasm32")]
self.1.borrow_mut().insert((path.clone(), idx), segment);
}
}
Some(result)
}
fn contains_key(&self, key: &StorePath) -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
self.0.contains_key(key)
}
#[cfg(target_arch = "wasm32")]
{
self.0.borrow_mut().contains_key(key)
}
}
fn get_key_for_index(
&self,
key: &(StorePath, usize),
) -> Option<StorePathSegment> {
#[cfg(not(target_arch = "wasm32"))]
{
self.1.get(key).as_deref().copied()
}
#[cfg(target_arch = "wasm32")]
{
self.1.borrow().get(key).as_deref().copied()
}
}
}

View File

@@ -35,7 +35,7 @@ where
// don't track the writer for the whole store
writer.untrack();
let mut notify = |path: &StorePath| {
self.triggers_for_path(path.to_owned()).notify();
self.triggers_for_path_unkeyed(path.to_owned()).notify();
};
writer.patch_field(new, &path, &mut notify);
}

View File

@@ -11,6 +11,15 @@ impl IntoIterator for StorePath {
}
}
impl<'a> IntoIterator for &'a StorePath {
type Item = &'a StorePathSegment;
type IntoIter = std::slice::Iter<'a, StorePathSegment>;
fn into_iter(self) -> Self::IntoIter {
self.0.iter()
}
}
impl From<Vec<StorePathSegment>> for StorePath {
fn from(value: Vec<StorePathSegment>) -> Self {
Self(value)
@@ -18,6 +27,16 @@ impl From<Vec<StorePathSegment>> for StorePath {
}
impl StorePath {
/// Creates a new path.
pub fn new() -> Self {
Self(Vec::new())
}
/// Creates a new path with storage capacity for `capacity` segments.
pub fn with_capacity(capacity: usize) -> Self {
Self(Vec::with_capacity(capacity))
}
/// Adds a new segment to the path.
pub fn push(&mut self, segment: impl Into<StorePathSegment>) {
self.0.push(segment.into());

View File

@@ -26,6 +26,14 @@ pub trait StoreField: Sized {
#[track_caller]
fn get_trigger(&self, path: StorePath) -> StoreFieldTrigger;
/// Returns the trigger that tracks access and updates for this field.
///
/// This uses *unkeyed* paths: i.e., if any field in the path is keyed, it will
/// try to look up the key for the item at the index given in the path, rather than
/// the keyed item.
#[track_caller]
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger;
/// The path of this field (see [`StorePath`]).
#[track_caller]
fn path(&self) -> impl IntoIterator<Item = StorePathSegment>;
@@ -84,6 +92,26 @@ pub trait StoreField: Sized {
triggers
}
/// Returns triggers for the field at the given path, and all parent fields
fn triggers_for_path_unkeyed(&self, path: StorePath) -> Vec<ArcTrigger> {
// see notes on triggers_for_path() for additional comments on implementation
let trigger = self.get_trigger_unkeyed(path.clone());
let mut full_path = path;
let mut triggers = Vec::with_capacity(full_path.len() + 2);
triggers.push(trigger.this.clone());
triggers.push(trigger.children.clone());
while !full_path.is_empty() {
full_path.pop();
let inner = self.get_trigger_unkeyed(full_path.clone());
triggers.push(inner.children.clone());
}
triggers.reverse();
triggers
}
}
impl<T> StoreField for ArcStore<T>
@@ -101,6 +129,26 @@ where
trigger
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
let orig_path = path.clone();
let mut path = StorePath::with_capacity(orig_path.len());
for segment in &orig_path {
let parent_is_keyed = self.keys.contains_key(&path);
if parent_is_keyed {
let key = self
.keys
.get_key_for_index(&(path.clone(), segment.0))
.expect("could not find key for index");
path.push(key);
} else {
path.push(*segment);
}
}
self.get_trigger(path)
}
#[track_caller]
fn path(&self) -> impl IntoIterator<Item = StorePathSegment> {
iter::empty()
@@ -141,6 +189,14 @@ where
.unwrap_or_default()
}
#[track_caller]
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner
.try_get_value()
.map(|n| n.get_trigger_unkeyed(path))
.unwrap_or_default()
}
#[track_caller]
fn path(&self) -> impl IntoIterator<Item = StorePathSegment> {
self.inner

View File

@@ -88,6 +88,10 @@ where
self.inner.get_trigger(path)
}
fn get_trigger_unkeyed(&self, path: StorePath) -> StoreFieldTrigger {
self.inner.get_trigger_unkeyed(path)
}
fn reader(&self) -> Option<Self::Reader> {
let inner = self.inner.reader()?;
Some(Mapped::new_with_guard(inner, self.read))