Skip to content

Commit 45b6833

Browse files
committed
Return effective values from store upserts
Let callers obtain the object produced by merge semantics without a second lookup. Preserve the existing boolean API so unrelated call sites and their allocation behavior remain unchanged. Co-Authored-By: HAL 9000
1 parent dc4fa49 commit 45b6833

1 file changed

Lines changed: 91 additions & 13 deletions

File tree

src/data_store.rs

Lines changed: 91 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,33 +82,43 @@ where
8282
}
8383

8484
pub(crate) async fn insert_or_update(&self, object: SO) -> Result<bool, Error> {
85+
self.insert_or_update_with(object, |updated, _| updated).await
86+
}
87+
88+
/// Inserts `object` or merges it into an existing object, returning whether the store changed
89+
/// and the effective object after the merge.
90+
pub(crate) async fn insert_or_update_and_get(&self, object: SO) -> Result<(bool, SO), Error> {
91+
self.insert_or_update_with(object, |updated, stored_object| {
92+
(updated, stored_object.clone())
93+
})
94+
.await
95+
}
96+
97+
async fn insert_or_update_with<R>(
98+
&self, object: SO, result_fn: impl FnOnce(bool, &SO) -> R,
99+
) -> Result<R, Error> {
85100
let _guard = self.mutation_lock.lock().await;
86101

87102
let id = object.id();
88-
let data_to_persist = {
103+
let updated_object = {
89104
let locked_objects = self.objects.lock().expect("lock");
90105
if let Some(existing_object) = locked_objects.get(&id) {
91106
let mut updated_object = existing_object.clone();
92107
let updated = updated_object.update(object.to_update());
93108
if updated {
94-
Some(updated_object)
109+
updated_object
95110
} else {
96-
None
111+
return Ok(result_fn(false, existing_object));
97112
}
98113
} else {
99-
Some(object)
114+
object
100115
}
101116
};
102117

103-
match data_to_persist {
104-
Some(updated_object) => {
105-
self.persist(&updated_object).await?;
106-
let mut locked_objects = self.objects.lock().expect("lock");
107-
locked_objects.insert(id, updated_object);
108-
Ok(true)
109-
},
110-
None => Ok(false),
111-
}
118+
self.persist(&updated_object).await?;
119+
let mut locked_objects = self.objects.lock().expect("lock");
120+
let stored_object = locked_objects.entry(id).insert_entry(updated_object).into_mut();
121+
Ok(result_fn(true, stored_object))
112122
}
113123

114124
pub(crate) async fn remove(&self, id: &SO::Id) -> Result<(), Error> {
@@ -287,6 +297,52 @@ mod tests {
287297
(2, data, required),
288298
});
289299

300+
struct MergingTestObjectUpdate {
301+
id: TestObjectId,
302+
data: [u8; 3],
303+
}
304+
305+
impl StorableObjectUpdate<MergingTestObject> for MergingTestObjectUpdate {
306+
fn id(&self) -> TestObjectId {
307+
self.id
308+
}
309+
}
310+
311+
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
312+
struct MergingTestObject {
313+
id: TestObjectId,
314+
data: [u8; 3],
315+
preserved_data: [u8; 3],
316+
}
317+
318+
impl StorableObject for MergingTestObject {
319+
type Id = TestObjectId;
320+
type Update = MergingTestObjectUpdate;
321+
322+
fn id(&self) -> Self::Id {
323+
self.id
324+
}
325+
326+
fn update(&mut self, update: Self::Update) -> bool {
327+
if self.data != update.data {
328+
self.data = update.data;
329+
true
330+
} else {
331+
false
332+
}
333+
}
334+
335+
fn to_update(&self) -> Self::Update {
336+
Self::Update { id: self.id, data: self.data }
337+
}
338+
}
339+
340+
impl_writeable_tlv_based!(MergingTestObject, {
341+
(0, id, required),
342+
(2, data, required),
343+
(4, preserved_data, required),
344+
});
345+
290346
struct FailingStore;
291347

292348
impl KVStore for FailingStore {
@@ -403,6 +459,28 @@ mod tests {
403459
assert_eq!(Ok(true), data_store.insert_or_update(new_iou_object).await);
404460
}
405461

462+
#[tokio::test]
463+
async fn insert_or_update_and_get_returns_merged_object() {
464+
let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
465+
let logger = Arc::new(TestLogger::new());
466+
let id = TestObjectId { id: [42u8; 4] };
467+
let existing = MergingTestObject { id, data: [23u8; 3], preserved_data: [24u8; 3] };
468+
let data_store = DataStore::new(
469+
vec![existing],
470+
"datastore_test_primary".to_string(),
471+
"datastore_test_secondary".to_string(),
472+
store,
473+
logger,
474+
);
475+
476+
let supplied = MergingTestObject { id, data: [25u8; 3], preserved_data: [26u8; 3] };
477+
let expected = MergingTestObject { data: supplied.data, ..existing };
478+
assert_eq!(Ok((true, expected)), data_store.insert_or_update_and_get(supplied).await);
479+
480+
let unchanged = MergingTestObject { preserved_data: [27u8; 3], ..expected };
481+
assert_eq!(Ok((false, expected)), data_store.insert_or_update_and_get(unchanged).await);
482+
}
483+
406484
#[tokio::test]
407485
async fn insert_or_update_does_not_mutate_memory_if_persist_fails() {
408486
let existing_id = TestObjectId { id: [42u8; 4] };

0 commit comments

Comments
 (0)