Skip to content

Commit 40b82fc

Browse files
committed
Release 0.10.2: three entity-manager and schema-mapper fixes
All three are behind non-default features or affect types a default build never reaches, so nothing here changes a default-feature project. Two unsaved entities registered with an entity manager aliased each other. `tide_pk_key` is infallible and has no notion of "unsaved", so a default primary key renders as an ordinary string — "0" for an i64. `register` and `put` filed entities under it, so the second new instance of a model collided with the first and was handed the first one back: a `HasMany` holding two new children silently dropped one and inserted the other twice. `TideEntityManagerMeta` gains `tide_pk_is_new`, defaulting to false so hand-written impls are unaffected, and both call sites return early rather than aliasing. `persist` left an identity-map entry no path could remove. `persisted_key` answered two different questions — whether a row exists in the database, and which key the entry is filed under in the identity map. They diverge for an entity given to `persist` with a client-assigned primary key, which is trackable immediately but not yet inserted. All three removal paths keyed off `persisted_key`, so `detach` silently did nothing and `remove` plus a flush left `find_managed` returning a row that was never written. The map key is now its own field, threaded through flush, detach and checkpoint rollback; the DELETE stays gated on whether a row actually exists. `Text` and `JsonArray` are exported from `tideorm::types` and documented as model field types, and `canonical_schema_type` already normalised both names, but neither had an arm in the `ColumnType` match — so both reached the catch-all and failed to compile. Four regression tests, each checked to fail against the unfixed code. They are deliberately database-free: the entity-manager ones use a disconnected handle, and the removal test drives the entry's own flush rather than `EntityManager::flush`, which opens a transaction. Putting database-requiring tests in the lib suite is what broke CI two releases ago.
1 parent 18bcb53 commit 40b82fc

11 files changed

Lines changed: 269 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [0.10.2] - 2026-08-25
11+
12+
Three defects behind the `entity-manager` feature and the schema type mapper.
13+
Nothing here affects a default-feature build.
14+
15+
### Added
16+
17+
- `TideEntityManagerMeta::tide_pk_is_new`, reporting whether an entity's primary key is still the
18+
type's default. It defaults to `false`, so hand-written implementations are unaffected; the derive
19+
emits it from `ModelMeta::primary_key_is_new`.
20+
21+
### Fixed
22+
23+
- **Two unsaved entities registered with an entity manager aliased each other.** `tide_pk_key` is
24+
infallible and has no notion of "unsaved", so it renders a default primary key as an ordinary
25+
string — `"0"` for an `i64`. `register` and `put` filed entities under it, so the second new
26+
instance of a model collided with the first and was handed the first one back. A `HasMany` holding
27+
two new children silently dropped one and inserted the other twice. Both now return early for an
28+
entity whose primary key is still new: it has no identity to share until the insert assigns one.
29+
- **`persist` left an identity-map entry that nothing could remove.** `persisted_key` answered two
30+
different questions — whether a row exists in the database, and which key the entry is filed under
31+
in the identity map. Those diverge for an entity given to `persist` with a client-assigned primary
32+
key: trackable immediately, but not yet inserted. All three removal paths keyed off
33+
`persisted_key`, so `detach` silently did nothing and `remove` + flush left a `find_managed`
34+
returning a row that was never written. The map key is tracked separately now, while the `DELETE`
35+
stays gated on whether a row actually exists.
36+
- **`Text` and `JsonArray` could not be used as model field types.** Both are exported from
37+
`tideorm::types` and documented for model fields, and `canonical_schema_type` already recognised
38+
the names, but neither had an arm in the `ColumnType` match — so both fell through to the
39+
catch-all and failed to compile. `Vec<serde_json::Value>` is accepted for `JsonArray` too.
40+
1041
## [0.10.1] - 2026-08-25
1142

1243
### Changed

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "tideorm"
3-
version = "0.10.1"
3+
version = "0.10.2"
44
edition = "2024"
55
authors = ["Mohamad Al Zohbie <alzoubi528@gmail.com>"]
66
description = "A developer-friendly ORM for Rust with clean, expressive syntax"
@@ -86,7 +86,7 @@ rust_decimal = { version = "1.41.0", features = ["serde"] }
8686
thiserror = "2.0.18"
8787

8888
# Derive macros (our own crate)
89-
tideorm-macros = { version = "0.10.1", path = "tideorm-macros" }
89+
tideorm-macros = { version = "0.10.2", path = "tideorm-macros" }
9090

9191
# Utils
9292
parking_lot = "0.12.5"

src/entity_manager/managed.rs

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,17 @@ pub(crate) struct ManagedEntry<T> {
268268
snapshot: RwLock<Option<T>>,
269269
state: RwLock<EntityState>,
270270
persisted_key: RwLock<Option<String>>,
271+
/// The key this entry is filed under in the manager's identity map.
272+
///
273+
/// Deliberately separate from [`Self::persisted_key`], which answers a
274+
/// different question: whether a row for this entity exists in the database.
275+
/// The two coincide for a loaded entity but not for one handed to `persist`
276+
/// with a client-assigned primary key — that is trackable immediately, so it
277+
/// goes into the identity map, while nothing has been inserted yet. Keying
278+
/// removal off `persisted_key` there left an entry no path could clear, so
279+
/// `detach` did not detach and a later `find_managed` returned a row that had
280+
/// never been written.
281+
identity_key: RwLock<Option<String>>,
271282
}
272283

273284
impl<T> ManagedEntry<T> {
@@ -281,10 +292,22 @@ impl<T> ManagedEntry<T> {
281292
current: RwLock::new(entity),
282293
snapshot: RwLock::new(snapshot),
283294
state: RwLock::new(state),
295+
// A caller that already knows the persisted key is filing the entry
296+
// under it; `persist` overrides this for the client-assigned case.
297+
identity_key: RwLock::new(persisted_key.clone()),
284298
persisted_key: RwLock::new(persisted_key),
285299
}
286300
}
287301

302+
/// Record the identity-map key this entry was filed under.
303+
pub(crate) fn set_identity_key(&self, key: Option<String>) {
304+
*self.identity_key.write() = key;
305+
}
306+
307+
pub(crate) fn identity_key(&self) -> Option<String> {
308+
self.identity_key.read().clone()
309+
}
310+
288311
pub(crate) fn state(&self) -> EntityState {
289312
*self.state.read()
290313
}
@@ -324,10 +347,6 @@ impl<T> ManagedEntry<T> {
324347
*self.state.write() = EntityState::Removed;
325348
}
326349

327-
pub(crate) fn persisted_key(&self) -> Option<String> {
328-
self.persisted_key.read().clone()
329-
}
330-
331350
fn mark_detached(&self) {
332351
*self.state.write() = EntityState::Detached;
333352
}
@@ -369,9 +388,12 @@ where
369388
}
370389

371390
fn detach_from_context(&self, entity_manager: &EntityManager) {
372-
if let Some(key) = self.persisted_key.read().as_ref() {
391+
// `identity_key`, not `persisted_key`: an entity given to `persist` with a
392+
// client-assigned primary key is in the map without having been inserted.
393+
if let Some(key) = self.identity_key.read().as_ref() {
373394
entity_manager.remove_managed_entry::<T>(key);
374395
}
396+
*self.identity_key.write() = None;
375397

376398
self.mark_detached();
377399
}
@@ -383,6 +405,7 @@ where
383405
snapshot: self.snapshot.read().clone(),
384406
state: self.state(),
385407
persisted_key: self.persisted_key.read().clone(),
408+
identity_key: self.identity_key.read().clone(),
386409
})
387410
}
388411

@@ -393,8 +416,14 @@ where
393416
match self.state() {
394417
EntityState::Detached => Ok(()),
395418
EntityState::Removed => {
396-
let key = self.persisted_key.read().as_ref().cloned();
397-
if let Some(key) = key {
419+
// Only a row that exists gets a DELETE, so that stays gated on
420+
// `persisted_key`. Evicting from the identity map is a separate
421+
// question and uses `identity_key`, which is also set for an
422+
// entity `persist`ed under a client-assigned key and never
423+
// inserted — without this it survived the remove and a later
424+
// `find_managed` handed back a row that was never written.
425+
let persisted = self.persisted_key.read().as_ref().cloned();
426+
if let Some(key) = persisted {
398427
let entity = self
399428
.snapshot
400429
.read()
@@ -406,11 +435,15 @@ where
406435
)
407436
.await?;
408437
entity_manager.remove_by_entity_manager_key::<T>(&key);
409-
entity_manager.remove_managed_entry::<T>(&key);
438+
}
439+
440+
if let Some(key) = self.identity_key.read().as_ref() {
441+
entity_manager.remove_managed_entry::<T>(key);
410442
}
411443

412444
*self.snapshot.write() = None;
413445
*self.persisted_key.write() = None;
446+
*self.identity_key.write() = None;
414447
self.mark_detached();
415448
Ok(())
416449
}
@@ -422,7 +455,10 @@ where
422455
None => true,
423456
};
424457

425-
let previous_key = self.persisted_key.read().as_ref().cloned();
458+
// The map entry may predate the insert: `persist` files a
459+
// client-assigned key immediately. Evict under whatever it was
460+
// actually filed as, not under the persisted key it may not have.
461+
let previous_key = self.identity_key.read().as_ref().cloned();
426462
let saved = if columns_changed {
427463
save_with_entity_manager_impl(&current, entity_manager).await?
428464
} else {
@@ -442,6 +478,7 @@ where
442478

443479
*self.current.write() = saved.clone();
444480
*self.snapshot.write() = Some(saved.clone());
481+
*self.identity_key.write() = next_key.clone();
445482
*self.persisted_key.write() = next_key;
446483
*self.state.write() = EntityState::Managed;
447484
entity_manager.put(saved);
@@ -457,25 +494,29 @@ struct ManagedEntryCheckpoint<T> {
457494
snapshot: Option<T>,
458495
state: EntityState,
459496
persisted_key: Option<String>,
497+
identity_key: Option<String>,
460498
}
461499

462500
impl<T> ManagedCheckpoint for ManagedEntryCheckpoint<T>
463501
where
464502
T: Send + Sync + 'static,
465503
{
466504
fn rollback(self: Box<Self>, entity_manager: &EntityManager) {
467-
if let Some(current_key) = self.entry.persisted_key.read().as_deref() {
505+
// Both evict and re-file go through `identity_key`, which is what the map
506+
// is actually keyed by; `persisted_key` is restored as plain state.
507+
if let Some(current_key) = self.entry.identity_key.read().as_deref() {
468508
entity_manager.remove_managed_entry::<T>(current_key);
469509
}
470510

471-
if let Some(previous_key) = self.persisted_key.as_deref() {
511+
if let Some(previous_key) = self.identity_key.as_deref() {
472512
entity_manager.put_managed_entry::<T>(previous_key, self.entry.clone());
473513
}
474514

475515
*self.entry.current.write() = self.current;
476516
*self.entry.snapshot.write() = self.snapshot;
477517
*self.entry.state.write() = self.state;
478518
*self.entry.persisted_key.write() = self.persisted_key;
519+
*self.entry.identity_key.write() = self.identity_key;
479520
}
480521
}
481522

src/entity_manager/meta.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,22 @@ pub trait TideEntityManagerMeta {
1414

1515
fn tide_pk_key(&self) -> String;
1616

17+
/// Whether this entity's primary key is still the type's default, i.e. no
18+
/// row has been inserted for it yet.
19+
///
20+
/// [`Self::tide_pk_key`] is infallible and has no notion of "unsaved", so it
21+
/// renders a default key as an ordinary string — `"0"` for an `i64`. Every
22+
/// unsaved instance of a model therefore produces the *same* identity key,
23+
/// and filing two of them in the identity map makes the second collide with
24+
/// the first. Callers that key the map must skip an entity for which this
25+
/// returns `true`: it has no identity to share yet.
26+
///
27+
/// Defaults to `false` for hand-written implementations, which preserves the
28+
/// previous behaviour for anyone not deriving `Model`.
29+
fn tide_pk_is_new(&self) -> bool {
30+
false
31+
}
32+
1733
/// Tables a row of this model points at with a foreign key, so they have to
1834
/// hold a row before this one can be inserted.
1935
///

src/entity_manager/mod.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ impl EntityManager {
168168
self.register_managed_entry(entry.clone());
169169
if let Some(key) = key.as_deref() {
170170
self.put_managed_entry::<T>(key, entry.clone());
171+
// Record what the entry was filed under. `persisted_key` stays `None`
172+
// — nothing is inserted yet — so without this the removal paths, which
173+
// all key off the map entry, would have nothing to evict.
174+
entry.set_identity_key(Some(key.to_string()));
171175
}
172176
Managed::from_entry(entry)
173177
}
@@ -222,9 +226,14 @@ impl EntityManager {
222226
where
223227
T: Send + Sync + 'static,
224228
{
225-
if let Some(key) = managed.entry.persisted_key() {
229+
// `identity_key`, not `persisted_key`: an entity `persist`ed with a
230+
// client-assigned primary key is in the identity map before any insert,
231+
// so keying the eviction off the persisted key left it behind and the
232+
// detach silently did nothing.
233+
if let Some(key) = managed.entry.identity_key() {
226234
self.remove_managed_entry::<T>(&key);
227235
}
236+
managed.entry.set_identity_key(None);
228237

229238
managed.entry.mark_detached_public();
230239
self.remove_managed_ops_entry(managed);
@@ -418,6 +427,16 @@ impl EntityManager {
418427
let mut entity = entity;
419428
entity.tide_attach_entity_manager_database(self.database());
420429

430+
// An unsaved entity has no identity to share. `tide_pk_key` renders its
431+
// default primary key as an ordinary string, so every new instance of a
432+
// model keys to the same thing — file two of them and the second gets
433+
// handed the first one back, which silently drops it and inserts the
434+
// first twice. Hand it straight back instead; the flush files it under a
435+
// real key once the insert assigns one.
436+
if entity.tide_pk_is_new() {
437+
return entity;
438+
}
439+
421440
let key = (TypeId::of::<T>(), entity.tide_pk_key());
422441

423442
if let Some(existing) = self.get_by_key::<T>(&key) {

src/entity_manager/state.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ impl EntityManager {
133133
let mut entity = entity;
134134
entity.tide_attach_entity_manager_database(self.database());
135135

136+
// Same collision as `register`: an unsaved entity's default primary key
137+
// is not an identity, and filing several under it makes them alias.
138+
if entity.tide_pk_is_new() {
139+
return;
140+
}
141+
136142
let key = (TypeId::of::<T>(), entity.tide_pk_key());
137143
save::record_identity_map_rollback::<T>(self, &key);
138144
let mut map = self.identity_map.write();

tests/unit/entity_manager_mod_tests.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,3 +656,99 @@ fn transaction_scope_is_restored_when_a_polled_future_panics() {
656656
"a panic must not leave the transaction scope pinned on this worker thread"
657657
);
658658
}
659+
660+
/// Client-assigned primary key, so `persist` can file it in the identity map
661+
/// before anything is inserted — the case where "tracked" and "exists in the
662+
/// database" genuinely differ.
663+
#[tideorm::model(table = "entity_manager_mod_test_widgets")]
664+
struct EntityManagerModTestWidget {
665+
#[tideorm(primary_key)]
666+
id: i64,
667+
name: String,
668+
}
669+
670+
#[tokio::test]
671+
async fn detach_clears_an_entry_persisted_under_a_client_assigned_key() {
672+
let entity_manager = Arc::new(EntityManager::new(Arc::new(Database::disconnected())));
673+
674+
let managed = entity_manager.persist(EntityManagerModTestWidget {
675+
id: 42,
676+
name: "widget".to_string(),
677+
});
678+
679+
assert!(
680+
entity_manager
681+
.get_managed_by_key::<EntityManagerModTestWidget>("42")
682+
.is_some(),
683+
"persist should track a client-assigned primary key immediately"
684+
);
685+
686+
entity_manager.detach(&managed);
687+
688+
// The eviction has to key off the map key, not `persisted_key`: nothing was
689+
// inserted, so `persisted_key` is `None` and keying off it left the entry
690+
// behind, making `detach` a no-op and a later `find_managed` hand back a row
691+
// that never existed.
692+
assert!(
693+
entity_manager
694+
.get_managed_by_key::<EntityManagerModTestWidget>("42")
695+
.is_none(),
696+
"detach must clear the identity-map entry it was filed under"
697+
);
698+
}
699+
700+
#[tokio::test]
701+
async fn removing_a_never_inserted_entity_evicts_it_without_a_delete() -> crate::error::Result<()> {
702+
// The database is disconnected, so this also asserts the flush issues no
703+
// DELETE for a row that was never written — it would fail if it tried.
704+
let entity_manager = Arc::new(EntityManager::new(Arc::new(Database::disconnected())));
705+
706+
let managed = entity_manager.persist(EntityManagerModTestWidget {
707+
id: 77,
708+
name: "widget".to_string(),
709+
});
710+
entity_manager.remove(&managed);
711+
712+
// Drive the entry's own flush rather than `EntityManager::flush`, which opens
713+
// a transaction a disconnected handle cannot. This is the branch under test,
714+
// and reaching it at all proves no DELETE was attempted.
715+
use crate::entity_manager::managed::ManagedOps;
716+
managed.entry.clone().flush(&entity_manager).await?;
717+
718+
assert!(
719+
entity_manager
720+
.get_managed_by_key::<EntityManagerModTestWidget>("77")
721+
.is_none(),
722+
"a removed entity must not stay in the identity map"
723+
);
724+
725+
Ok(())
726+
}
727+
728+
#[tokio::test]
729+
async fn registering_two_unsaved_entities_keeps_them_distinct() {
730+
let entity_manager = Arc::new(EntityManager::new(Arc::new(Database::disconnected())));
731+
732+
// Both have the default primary key, which `tide_pk_key` renders as "0".
733+
// Filing them under it made the second collide with the first and get the
734+
// first handed back — silently dropping one child and inserting the other
735+
// twice.
736+
let first = entity_manager
737+
.register(EntityManagerModTestUser {
738+
id: 0,
739+
name: "first".to_string(),
740+
})
741+
.await;
742+
let second = entity_manager
743+
.register(EntityManagerModTestUser {
744+
id: 0,
745+
name: "second".to_string(),
746+
})
747+
.await;
748+
749+
assert_eq!(first.name, "first");
750+
assert_eq!(
751+
second.name, "second",
752+
"an unsaved entity has no identity to share, so it must not alias another"
753+
);
754+
}

0 commit comments

Comments
 (0)