Skip to content

Commit 8fa65fd

Browse files
committed
graph: Exclude VID from change detection in merge_remove_null_fields
Entity::merge_remove_null_fields iterates via the raw Object iterator which, unlike every other Entity API, does not filter the VID field. Since EntityCache::set stamps a fresh VID on every save, the VID always differs from the stored entity's VID, causing merge_remove_null_fields to report a change even when the user-visible data is identical. This produces a spurious Overwrite modification (and therefore a new row in the database) each time an unchanged entity is re-saved. Skip the VID when computing the changed flag while still merging it into the entity so downstream code (e.g. store insert queries) sees the latest value.
1 parent e4dfe52 commit 8fa65fd

File tree

1 file changed

+35
-2
lines changed

1 file changed

+35
-2
lines changed

graph/src/data/store/mod.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,16 +1017,20 @@ impl Entity {
10171017
pub fn merge_remove_null_fields(&mut self, update: Entity) -> Result<bool, InternError> {
10181018
let mut changed = false;
10191019
for (key, value) in update.0.into_iter() {
1020+
// A change in VID, which changes on every save, does not count
1021+
// as a change to the entity's data; this avoids spurious
1022+
// `Overwrite` modifications.
1023+
let is_vid = key.as_str() == VID_FIELD;
10201024
match value {
10211025
Value::Null => {
1022-
if self.0.remove(&key).is_some() {
1026+
if self.0.remove(&key).is_some() && !is_vid {
10231027
changed = true;
10241028
}
10251029
}
10261030
_ => {
10271031
let different = self.0.get(key.as_str()).is_none_or(|old| *old != value);
10281032
self.0.insert(&key, value)?;
1029-
if different {
1033+
if different && !is_vid {
10301034
changed = true;
10311035
}
10321036
}
@@ -1388,3 +1392,32 @@ fn entity_hidden_vid() {
13881392
_ = entity2.set_vid(7i64);
13891393
assert_eq!(entity2.vid(), 7i64);
13901394
}
1395+
1396+
#[test]
1397+
fn merge_remove_null_fields_ignores_vid() {
1398+
use crate::schema::InputSchema;
1399+
let schema = InputSchema::raw("type Thing @entity {id: ID!, name: String!}", "test");
1400+
1401+
let mut current = entity! { schema => id: "1", name: "alice", vid: 3i64 };
1402+
let update = entity! { schema => id: "1", name: "alice", vid: 99i64 };
1403+
1404+
// Merging an identical entity with a different VID must report no change.
1405+
let changed = current.merge_remove_null_fields(update).unwrap();
1406+
assert!(!changed, "VID-only difference must not count as a change");
1407+
// The VID must still be updated to the new value.
1408+
assert_eq!(current.vid(), 99i64);
1409+
}
1410+
1411+
#[test]
1412+
fn merge_remove_null_fields_detects_real_change() {
1413+
use crate::schema::InputSchema;
1414+
let schema = InputSchema::raw("type Thing @entity {id: ID!, name: String!}", "test");
1415+
1416+
let mut current = entity! { schema => id: "1", name: "alice", vid: 3i64 };
1417+
let update = entity! { schema => id: "1", name: "bob", vid: 99i64 };
1418+
1419+
let changed = current.merge_remove_null_fields(update).unwrap();
1420+
assert!(changed, "data change must be detected");
1421+
assert_eq!(current.get("name"), Some(&Value::String("bob".to_string())));
1422+
assert_eq!(current.vid(), 99i64);
1423+
}

0 commit comments

Comments
 (0)