diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 297beddb1b..76db47f045 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -229,6 +229,7 @@ jobs: actix_example, axum_example, basic, + basic_typed_pk, graphql_example, jsonrpsee_example, loco_example, diff --git a/.gitignore b/.gitignore index 7ec785cd4a..a093728278 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,8 @@ Cargo.lock .idea/* */.idea/* .env.local -.DS_Store \ No newline at end of file +.DS_Store +# Auto-generated trybuild stderr captures; we only assert must-fail, +# stderr content is regenerated each run via TRYBUILD=overwrite. +tests/value_type_pk_compile_fail/*.stderr +sea-orm-sync/tests/value_type_pk_compile_fail/*.stderr diff --git a/CLAUDE.md b/CLAUDE.md index 75d0848c6f..b593da2a12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -150,6 +150,40 @@ db.get_schema_registry("my_crate::*") .await?; ``` +## Type-safe primary keys + +`find_by_id` / `filter_by_id` / `delete_by_id` accept any `T: FindByIdArg`, which is implemented blanket for `T: Into<::ValueType>`. So `&str → String` and `u8 → i32` style conversions still flow through. The type safety comes from the PK type itself: a newtype like `UserId` has no `From`, so `find_by_id(1)` against a `UserId` PK fails to compile. To get that compile-time protection, wrap each entity's PK in a per-entity newtype with `DeriveValueType` (or use a `sea_orm::Id` alias): + +```rust +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] +pub struct UserId(pub i32); + +#[sea_orm::model] +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "user")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, +} +``` + +**Do not** add `impl From for UserId` (or any `From`). That re-opens the door to `find_by_id(1)` and defeats the safety contract. Construct ids explicitly with the tuple form: `UserId(1)`. + +For foreign keys, spell the column type with the parent's newtype too: + +```rust +// post.rs +pub struct Model { + #[sea_orm(primary_key)] + pub id: PostId, + pub user_id: super::user::UserId, + // ... +} +``` + +The trybuild harness at `tests/value_type_pk_compile_fail/` pins this contract: any regression that re-allows `find_by_id(raw_int)` or cross-PK confusion will start compiling and fail the test. + ## Anti-Patterns -- DO NOT DO THESE ### 1. Do not specify `column_type` on custom wrapper types @@ -209,7 +243,7 @@ Expr::col((self.entity_name(), *self)).like(s) ### 5. Do not manually impl traits that `DeriveValueType` now generates -In 2.0, `DeriveValueType` auto-generates `NotU8`, `IntoActiveValue`, and `TryFromU64`. Remove manual implementations to avoid conflicts. +In 2.0, `DeriveValueType` auto-generates `NotU8`, `IntoActiveValue`, `TryFromU64`, and the primary-key auto-increment hint (`DelegatesPkAutoIncrementHint` for struct wrappers, `PkAutoIncrementHint` for string wrappers). Remove manual implementations to avoid conflicts. Hand-writing the auto-increment hint collides directly for string wrappers and via the blanket bridge for struct wrappers. ### 6. PostgreSQL: `serial` is no longer the default diff --git a/Cargo.toml b/Cargo.toml index 31c30b0570..dd38b1b7a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,6 +104,7 @@ smol-potat = { version = "1.1" } time = { version = "0.3.36", features = ["macros"] } tokio = { version = "1.6", features = ["full"] } tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } +trybuild = { version = "1" } uuid = { version = "1", features = ["v4"] } [features] diff --git a/examples/basic_typed_pk/Cargo.toml b/examples/basic_typed_pk/Cargo.toml new file mode 100644 index 0000000000..2a512c60ff --- /dev/null +++ b/examples/basic_typed_pk/Cargo.toml @@ -0,0 +1,22 @@ +[workspace] +# A separate workspace + +[package] +edition = "2024" +name = "sea-orm-example-basic-typed-pk" +publish = false +rust-version = "1.85.0" +version = "0.1.0" + +[dependencies] +tokio = { version = "1", features = ["full"] } + +[dependencies.sea-orm] +features = [ + "sqlx-sqlite", + "runtime-tokio", + "debug-print", +] +path = "../../" + +[patch.crates-io] diff --git a/examples/basic_typed_pk/Readme.md b/examples/basic_typed_pk/Readme.md new file mode 100644 index 0000000000..2efb4b5b83 --- /dev/null +++ b/examples/basic_typed_pk/Readme.md @@ -0,0 +1,60 @@ +# SeaORM typed-PK example + +A minimal task tracker that exercises the per-entity +`sea_orm::Id` primary key wrappers produced by +`sea-orm-cli generate entity --with-pk-newtypes`. + +Five tables, each chosen to cover one PK-newtype pattern not already +demonstrated by another table: + +- **Cross-entity ID safety.** `user`, `project`, `task` each get a + distinct alias (`UserPk`, `ProjectPk`, `TaskPk`) so passing the + wrong id to the wrong API is a compile error. +- **Composite primary key with typed components.** `project_member` + is keyed by `(ProjectPk, UserPk)`, the alias types carry into + `find_by_id`, `delete_by_id`, and the tuple destructuring. +- **Self-reference.** `task.parent_task_id: Option` resolves + to the local alias, not `super::task::TaskPk`. +- **Multi-FK to the same parent in non-PK position.** `task` has + both `assignee_id` and `reviewer_id` referencing `user.id`. Both + share the parent's `UserPk` type, codegen does **not** role-wrap + non-PK FK columns (role wrappers are PK-only by design). +- **Role wrappers on a junction.** `task_dependency` has two PK + columns, both FK-referencing `task.id`. Codegen emits + `TaskDependencyPkBlockerTaskId` and `TaskDependencyPkBlockedTaskId` + so swapping blocker/blocked at an insert site fails to compile. + +## Running + +```sh +cargo run +``` + +The example uses in-memory SQLite, creates the schema from the +generated entities via `Schema::create_table_from_entity`, and walks +through realistic operations (`reassign_task`, `create_subtask`, +`add_blocker`, `add_project_member`, `tasks_assigned_to`) defined in +`src/operations.rs`. Each operation takes typed PKs in its signature. + +## Regenerating the entities + +Everything under `src/entity/` is produced from `tasks.sql` by +`sea-orm-cli`. To regenerate (e.g. after editing the schema): + +```sh +sqlite3 /tmp/typed_pk_tasks.db < examples/basic_typed_pk/tasks.sql +cargo run --manifest-path sea-orm-cli/Cargo.toml --bin sea-orm-cli -- \ + generate entity \ + --database-url sqlite:///tmp/typed_pk_tasks.db \ + --with-pk-newtypes \ + --output-dir examples/basic_typed_pk/src/entity +``` + +Running the CLI via `cargo run --manifest-path sea-orm-cli/...` uses +the branch's source rather than whatever `sea-orm-cli` is on `$PATH`, +so the generated output reflects the version of codegen this example +ships with. + +The committed entity files are a snapshot, like `examples/basic`, +they're regenerated by hand when the schema changes rather than at +build time. diff --git a/examples/basic_typed_pk/src/entity/mod.rs b/examples/basic_typed_pk/src/entity/mod.rs new file mode 100644 index 0000000000..2a8d548fb7 --- /dev/null +++ b/examples/basic_typed_pk/src/entity/mod.rs @@ -0,0 +1,9 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +pub mod prelude; + +pub mod project; +pub mod project_member; +pub mod task; +pub mod task_dependency; +pub mod user; diff --git a/examples/basic_typed_pk/src/entity/prelude.rs b/examples/basic_typed_pk/src/entity/prelude.rs new file mode 100644 index 0000000000..be7d938fc9 --- /dev/null +++ b/examples/basic_typed_pk/src/entity/prelude.rs @@ -0,0 +1,7 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +pub use super::project::Entity as Project; +pub use super::project_member::Entity as ProjectMember; +pub use super::task::Entity as Task; +pub use super::task_dependency::Entity as TaskDependency; +pub use super::user::Entity as User; diff --git a/examples/basic_typed_pk/src/entity/project.rs b/examples/basic_typed_pk/src/entity/project.rs new file mode 100644 index 0000000000..0e774a1ea4 --- /dev/null +++ b/examples/basic_typed_pk/src/entity/project.rs @@ -0,0 +1,44 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +use sea_orm::entity::prelude::*; + +pub type ProjectPk = sea_orm::Id; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "project")] +pub struct Model { + #[sea_orm(primary_key, auto_increment)] + pub id: ProjectPk, + pub name: String, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm(has_many = "super::project_member::Entity")] + ProjectMember, + #[sea_orm(has_many = "super::task::Entity")] + Task, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::ProjectMember.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Task.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + super::project_member::Relation::User.def() + } + fn via() -> Option { + Some(super::project_member::Relation::Project.def().rev()) + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/examples/basic_typed_pk/src/entity/project_member.rs b/examples/basic_typed_pk/src/entity/project_member.rs new file mode 100644 index 0000000000..90719648a8 --- /dev/null +++ b/examples/basic_typed_pk/src/entity/project_member.rs @@ -0,0 +1,47 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "project_member")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub project_id: super::project::ProjectPk, + #[sea_orm(primary_key, auto_increment = false)] + pub user_id: super::user::UserPk, + pub role: String, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::project::Entity", + from = "Column::ProjectId", + to = "super::project::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + Project, + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::UserId", + to = "super::user::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + User, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Project.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::User.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/examples/basic_typed_pk/src/entity/task.rs b/examples/basic_typed_pk/src/entity/task.rs new file mode 100644 index 0000000000..da9add7558 --- /dev/null +++ b/examples/basic_typed_pk/src/entity/task.rs @@ -0,0 +1,61 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +use sea_orm::entity::prelude::*; + +pub type TaskPk = sea_orm::Id; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "task")] +pub struct Model { + #[sea_orm(primary_key, auto_increment)] + pub id: TaskPk, + pub project_id: super::project::ProjectPk, + pub assignee_id: super::user::UserPk, + pub reviewer_id: Option, + pub parent_task_id: Option, + pub title: String, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::project::Entity", + from = "Column::ProjectId", + to = "super::project::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + Project, + #[sea_orm( + belongs_to = "Entity", + from = "Column::ParentTaskId", + to = "Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + SelfRef, + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::ReviewerId", + to = "super::user::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + User2, + #[sea_orm( + belongs_to = "super::user::Entity", + from = "Column::AssigneeId", + to = "super::user::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + User1, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::Project.def() + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/examples/basic_typed_pk/src/entity/task_dependency.rs b/examples/basic_typed_pk/src/entity/task_dependency.rs new file mode 100644 index 0000000000..f42ca776ef --- /dev/null +++ b/examples/basic_typed_pk/src/entity/task_dependency.rs @@ -0,0 +1,42 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +use sea_orm::entity::prelude::*; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] +#[sea_orm(try_from_u64)] +pub struct TaskDependencyPkBlockerTaskId(pub super::task::TaskPk); + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] +#[sea_orm(try_from_u64)] +pub struct TaskDependencyPkBlockedTaskId(pub super::task::TaskPk); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "task_dependency")] +pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub blocker_task_id: TaskDependencyPkBlockerTaskId, + #[sea_orm(primary_key, auto_increment = false)] + pub blocked_task_id: TaskDependencyPkBlockedTaskId, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm( + belongs_to = "super::task::Entity", + from = "Column::BlockedTaskId", + to = "super::task::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + Task2, + #[sea_orm( + belongs_to = "super::task::Entity", + from = "Column::BlockerTaskId", + to = "super::task::Column::Id", + on_update = "NoAction", + on_delete = "NoAction" + )] + Task1, +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/examples/basic_typed_pk/src/entity/user.rs b/examples/basic_typed_pk/src/entity/user.rs new file mode 100644 index 0000000000..9981d60c5a --- /dev/null +++ b/examples/basic_typed_pk/src/entity/user.rs @@ -0,0 +1,38 @@ +//! `SeaORM` Entity, @generated by sea-orm-codegen 2.0 + +use sea_orm::entity::prelude::*; + +pub type UserPk = sea_orm::Id; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] +#[sea_orm(table_name = "user")] +pub struct Model { + #[sea_orm(primary_key, auto_increment)] + pub id: UserPk, + pub name: String, + #[sea_orm(unique)] + pub email: String, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation { + #[sea_orm(has_many = "super::project_member::Entity")] + ProjectMember, +} + +impl Related for Entity { + fn to() -> RelationDef { + Relation::ProjectMember.def() + } +} + +impl Related for Entity { + fn to() -> RelationDef { + super::project_member::Relation::Project.def() + } + fn via() -> Option { + Some(super::project_member::Relation::User.def().rev()) + } +} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/examples/basic_typed_pk/src/main.rs b/examples/basic_typed_pk/src/main.rs new file mode 100644 index 0000000000..cdadedb4ec --- /dev/null +++ b/examples/basic_typed_pk/src/main.rs @@ -0,0 +1,140 @@ +mod entity; +mod operations; + +use entity::{project, project_member, task, user}; +use sea_orm::{ + ActiveModelTrait, ActiveValue::*, ConnectOptions, ConnectionTrait, Database, DbBackend, DbErr, + EntityTrait, Schema, Statement, +}; + +#[tokio::main] +async fn main() -> Result<(), DbErr> { + let db = Database::connect(ConnectOptions::new("sqlite::memory:")).await?; + create_schema(&db).await?; + + // Create two users. `.insert(...)` returns the persisted Model with + // a typed `UserPk` already populated, no raw `i64` ever appears. + let alice = user::ActiveModel { + name: Set("Alice".to_string()), + email: Set("alice@example.com".to_string()), + ..Default::default() + } + .insert(&db) + .await?; + let bob = user::ActiveModel { + name: Set("Bob".to_string()), + email: Set("bob@example.com".to_string()), + ..Default::default() + } + .insert(&db) + .await?; + + // Project + membership. `add_project_member` takes + // `(ProjectId, UserId, String)`, swapping the two id args would be + // a compile error. + let audit = project::ActiveModel { + name: Set("ATO compliance audit".to_string()), + ..Default::default() + } + .insert(&db) + .await?; + operations::add_project_member(&db, audit.id, alice.id, "Admin".to_string()).await?; + operations::add_project_member(&db, audit.id, bob.id, "Engineer".to_string()).await?; + + // Composite-PK lookup inline: `find_by_id` takes a tuple of typed + // components. Reversing them is a compile error. + let alice_membership = project_member::Entity::find_by_id((audit.id, alice.id)) + .one(&db) + .await? + .expect("alice should be a member"); + println!("alice's membership row: {alice_membership:?}"); + + // Tasks. Each parameter to the typed insert is a distinct PK type; + // a mixup would be rejected at compile time. + let draft_policy = task::ActiveModel { + project_id: Set(audit.id), + assignee_id: Set(bob.id), + reviewer_id: Set(Some(alice.id)), + parent_task_id: Set(None), + title: Set("Draft policy".to_string()), + ..Default::default() + } + .insert(&db) + .await?; + println!("draft policy: {draft_policy:?}"); + + // Subtask via self-ref. `parent_task_id: Set(Some(parent))` carries + // a typed `TaskPk`, there's no way to accidentally pass a UserId. + let outline = + operations::create_subtask(&db, audit.id, draft_policy.id, bob.id, "Outline".to_string()) + .await?; + println!("outline (subtask of draft policy): {outline:?}"); + + let internal_review = task::ActiveModel { + project_id: Set(audit.id), + assignee_id: Set(alice.id), + reviewer_id: Set(None), + parent_task_id: Set(None), + title: Set("Internal review".to_string()), + ..Default::default() + } + .insert(&db) + .await?; + + // Role-wrapped junction. `add_blocker` funnels its two typed args + // through distinct `TaskDependencyPk*` wrappers, so swapping the + // blocker/blocked roles is a compile error at the insert site. + operations::add_blocker(&db, draft_policy.id, internal_review.id).await?; + println!("blocker recorded: \"draft policy\" blocks \"internal review\""); + + // Reassign outline from bob to alice. Typed PK threaded through UPDATE. + let outline = operations::reassign_task(&db, outline.id, alice.id).await?; + println!("outline reassigned to alice: {outline:?}"); + + // Typed PK in `.filter()` position, a different code path than + // `find_by_id`. + let bobs_tasks = operations::tasks_assigned_to(&db, bob.id).await?; + println!("tasks assigned to bob ({} total):", bobs_tasks.len()); + for t in &bobs_tasks { + println!(" {t:?}"); + } + + // Composite-PK delete inline. Reversing the tuple components is a + // compile error because `ProjectPk` and `UserPk` are distinct types. + let removed = project_member::Entity::delete_by_id((audit.id, bob.id)) + .exec(&db) + .await?; + println!( + "bob removed from project: {} row(s) affected", + removed.rows_affected + ); + + Ok(()) +} + +/// Create the schema by running `CREATE TABLE` statements derived from +/// the entity definitions. For a real app you'd run migrations; for an +/// in-memory example this is the lightest path. +async fn create_schema(db: &impl ConnectionTrait) -> Result<(), DbErr> { + let backend = db.get_database_backend(); + let schema = Schema::new(backend); + + // Order matters: parents before children so FKs resolve. + for stmt in [ + schema.create_table_from_entity(user::Entity), + schema.create_table_from_entity(project::Entity), + schema.create_table_from_entity(project_member::Entity), + schema.create_table_from_entity(task::Entity), + schema.create_table_from_entity(entity::task_dependency::Entity), + ] { + db.execute(&stmt).await?; + } + // Enable FK enforcement on SQLite so the example actually exercises + // the constraints. + db.execute_raw(Statement::from_string( + DbBackend::Sqlite, + "PRAGMA foreign_keys = ON".to_string(), + )) + .await?; + Ok(()) +} diff --git a/examples/basic_typed_pk/src/operations.rs b/examples/basic_typed_pk/src/operations.rs new file mode 100644 index 0000000000..fb8b2f9be5 --- /dev/null +++ b/examples/basic_typed_pk/src/operations.rs @@ -0,0 +1,92 @@ +//! Typed-PK domain code over the generated task-tracker entities. +//! +//! Every function signature carries typed IDs, so the compiler catches +//! mixups at every call site. The trybuild fixtures under +//! `tests/value_type_pk_compile_fail/` pin the rejection contract; +//! this module is the positive side. +//! +//! Each function covers a distinct typed-PK call shape: +//! +//! - `reassign_task`, typed PK threaded through an `UPDATE` and +//! cross-entity argument typing (`TaskPk` and `UserPk`). +//! - `create_subtask`, self-ref `parent_task_id: Option` +//! plus four typed PKs in a row. +//! - `add_blocker`, role-wrapped junction insert (the only place +//! the `TaskDependencyPk*` wrappers are user-visible). +//! - `add_project_member`, composite-PK insert with typed components. +//! - `tasks_assigned_to`, typed PK as a value passed into +//! `Column::AssigneeId.eq(...)` for a filter. + +use crate::entity::{project, project_member, task, task_dependency, user}; +use sea_orm::{ActiveValue::*, DbErr, entity::*, query::*}; + +pub async fn reassign_task( + db: &C, + task_id: task::TaskPk, + new_assignee: user::UserPk, +) -> Result { + let existing = task::Entity::find_by_id(task_id) + .one(db) + .await? + .ok_or_else(|| DbErr::RecordNotFound(format!("task {task_id:?}")))?; + let mut active: task::ActiveModel = existing.into(); + active.assignee_id = Set(new_assignee); + active.update(db).await +} + +pub async fn create_subtask( + db: &C, + project_id: project::ProjectPk, + parent: task::TaskPk, + assignee: user::UserPk, + title: String, +) -> Result { + task::ActiveModel { + project_id: Set(project_id), + assignee_id: Set(assignee), + reviewer_id: Set(None), + parent_task_id: Set(Some(parent)), + title: Set(title), + ..Default::default() + } + .insert(db) + .await +} + +pub async fn add_blocker( + db: &C, + blocker: task::TaskPk, + blocked: task::TaskPk, +) -> Result { + task_dependency::ActiveModel { + blocker_task_id: Set(task_dependency::TaskDependencyPkBlockerTaskId(blocker)), + blocked_task_id: Set(task_dependency::TaskDependencyPkBlockedTaskId(blocked)), + } + .insert(db) + .await +} + +pub async fn add_project_member( + db: &C, + project_id: project::ProjectPk, + user_id: user::UserPk, + role: String, +) -> Result { + project_member::ActiveModel { + project_id: Set(project_id), + user_id: Set(user_id), + role: Set(role), + } + .insert(db) + .await +} + +pub async fn tasks_assigned_to( + db: &C, + user_id: user::UserPk, +) -> Result, DbErr> { + task::Entity::find() + .filter(task::Column::AssigneeId.eq(user_id)) + .all(db) + .await +} diff --git a/examples/basic_typed_pk/tasks.sql b/examples/basic_typed_pk/tasks.sql new file mode 100644 index 0000000000..74ebfc2fb9 --- /dev/null +++ b/examples/basic_typed_pk/tasks.sql @@ -0,0 +1,74 @@ +-- Schema for the typed-PK task tracker example. +-- +-- Five tables, each chosen to cover one PK-newtype pattern not +-- already demonstrated by another table: +-- +-- user -- a distinct PK type (`UserId`) so cross-entity +-- confusion is rejectable at the type level. +-- project -- a third distinct PK type (`ProjectId`), +-- parent of task and project_member. +-- project_member -- composite PK (ProjectId, UserId) with typed +-- components. +-- task -- FK to project; two non-PK FKs to user +-- (assignee + reviewer) that share the parent +-- `UserId` type (codegen does NOT role-wrap +-- non-PK FK columns, by design); self-ref +-- `parent_task_id` for subtasks. +-- task_dependency -- junction with two PK columns both FK to +-- task.id. Codegen emits per-column role +-- wrappers (`TaskDependencyPk*`) so swapping +-- blocker/blocked at the call site fails to +-- compile. +-- +-- The entity files under src/entity/ are generated from this schema; see +-- Readme.md ("Regenerating the entities") for the sea-orm-cli command. + +CREATE TABLE user ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name VARCHAR(255) NOT NULL, + email VARCHAR(255) NOT NULL UNIQUE +); + +CREATE TABLE project ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name VARCHAR(255) NOT NULL +); + +-- Composite PK whose components are both typed FKs into other tables. +CREATE TABLE project_member ( + project_id INTEGER NOT NULL, + user_id INTEGER NOT NULL, + role VARCHAR(64) NOT NULL, + PRIMARY KEY (project_id, user_id), + FOREIGN KEY (project_id) REFERENCES project (id), + FOREIGN KEY (user_id) REFERENCES user (id) +); + +-- Self-ref via parent_task_id; two non-PK FKs to user (assignee + +-- reviewer). Both user FKs share `UserId`, they are NOT role-wrapped +-- (role wrappers are PK-only by codegen design). +CREATE TABLE task ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + project_id INTEGER NOT NULL, + assignee_id INTEGER NOT NULL, + reviewer_id INTEGER, + parent_task_id INTEGER, + title VARCHAR(255) NOT NULL, + FOREIGN KEY (project_id) REFERENCES project (id), + FOREIGN KEY (assignee_id) REFERENCES user (id), + FOREIGN KEY (reviewer_id) REFERENCES user (id), + FOREIGN KEY (parent_task_id) REFERENCES task (id) +); + +-- Junction with two PK columns both FK-referencing task.id. This is +-- the canonical role-wrapper case: codegen emits per-column wrapper +-- structs (`TaskDependencyPkBlockerTaskId`, +-- `TaskDependencyPkBlockedTaskId`) so positional swaps fail to +-- compile. +CREATE TABLE task_dependency ( + blocker_task_id INTEGER NOT NULL, + blocked_task_id INTEGER NOT NULL, + PRIMARY KEY (blocker_task_id, blocked_task_id), + FOREIGN KEY (blocker_task_id) REFERENCES task (id), + FOREIGN KEY (blocked_task_id) REFERENCES task (id) +); diff --git a/sea-orm-cli/src/cli.rs b/sea-orm-cli/src/cli.rs index 810a81ba66..c7fc22b35a 100644 --- a/sea-orm-cli/src/cli.rs +++ b/sea-orm-cli/src/cli.rs @@ -391,6 +391,13 @@ pub enum GenerateSubcommands { help = "Also generate a Mermaid ER diagram as `entities.mermaid` in the output directory" )] er_diagram: bool, + + #[arg( + long, + default_value = "false", + help = "Wrap each table's primary key in a per-entity newtype (e.g. `CakeId(i32)`) and propagate it to foreign-key columns." + )] + with_pk_newtypes: bool, }, } diff --git a/sea-orm-cli/src/commands/generate.rs b/sea-orm-cli/src/commands/generate.rs index 7e9c5916e2..da94f1960c 100644 --- a/sea-orm-cli/src/commands/generate.rs +++ b/sea-orm-cli/src/commands/generate.rs @@ -45,6 +45,7 @@ pub async fn run_generate_command( preserve_user_modifications, banner_version, er_diagram, + with_pk_newtypes, } => { if verbose { let _ = tracing_subscriber::fmt() @@ -250,6 +251,11 @@ pub async fn run_generate_command( seaography, impl_active_model_behavior, banner_version.into(), + if with_pk_newtypes { + sea_orm_codegen::PkNewtypeFormat::Inline + } else { + sea_orm_codegen::PkNewtypeFormat::None + }, ); let entity_writer = EntityTransformer::transform(table_stmts)?; diff --git a/sea-orm-codegen/src/entity/base_entity.rs b/sea-orm-codegen/src/entity/base_entity.rs index 865130dc68..c0e027e646 100644 --- a/sea-orm-codegen/src/entity/base_entity.rs +++ b/sea-orm-codegen/src/entity/base_entity.rs @@ -313,6 +313,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -321,6 +322,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ diff --git a/sea-orm-codegen/src/entity/column.rs b/sea-orm-codegen/src/entity/column.rs index 25f650fee7..48759507cd 100644 --- a/sea-orm-codegen/src/entity/column.rs +++ b/sea-orm-codegen/src/entity/column.rs @@ -3,7 +3,21 @@ use heck::{ToSnakeCase, ToUpperCamelCase}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; use sea_query::{ColumnDef, ColumnType, StringLen}; +use std::collections::HashMap; use std::fmt::Write as FmtWrite; +use std::rc::Rc; + +/// Maps `(table_name, column_name)` to the generated PK newtype identifier +/// (e.g. `("cake", "id") -> Ident("CakePk")`). Built once per +/// `write_entities` call when `--with-pk-newtypes` is enabled. +pub type PkNewtypeIndex = HashMap<(String, String), Ident>; + +/// A single FK back-reference: this column points at `table.column`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ColumnRef { + pub table: String, + pub column: String, +} #[derive(Debug, Clone)] pub struct Column { @@ -13,12 +27,39 @@ pub struct Column { pub(crate) not_null: bool, pub(crate) unique: bool, pub(crate) unique_key: Option, + /// FK back-references `(parent_table, parent_column)` pairs for every + /// `BelongsTo` relation that includes this column. Empty for non-FK + /// columns. Populated by `EntityTransformer` from the schema's foreign + /// key constraints. + /// + /// Note: Multiple entries are legal (a column can be + /// constrained against multiple parents); under + /// `--with-pk-newtypes`, codegen opts out of newtyping for such + /// columns and emits the raw scalar instead. See + /// `Column::get_rs_type`. + pub(crate) refs: Vec, } -#[derive(Debug, Default, Copy, Clone)] +#[derive(Debug, Default, Clone)] pub struct ColumnOption { pub(crate) date_time_crate: DateTimeCrate, pub(crate) big_integer_type: BigIntegerType, + pub(crate) pk_newtype: Option, +} + +/// Per-entity resolution context for `--with-pk-newtypes` codegen. +#[derive(Debug, Clone)] +pub struct PkNewtypeContext { + /// Name of the table currently being emitted. Used by + /// `get_rs_type` to look up "is this column a PK in its own table?" + /// against `pk_aliases`. + pub current_table: String, + /// Maps `(table_name, column_name)` to the generated PK newtype identifier + /// (e.g. `("cake", "id") -> Ident("CakePk")`). + pub pk_aliases: Rc, + /// `(own_table, fk_column) -> RoleWrapperIdent` for self-ref junction + /// tables where multiple columns FK-reference the same parent. + pub role_wrappers: Rc, } impl Column { @@ -98,10 +139,86 @@ impl Column { _ => unimplemented!(), } } - let ident: TokenStream = write_rs_type(&self.col_type, opt).parse().unwrap(); + + // PK-newtype mode: resolve the column's emitted Rust type in a fixed + // precedence order. Each step is checked against the column currently + // being emitted (`(current_table, self.name)`): + // + // 1. Role wrapper. The column is one of several in this table that FK + // to the same parent table + column; emit the local per-column + // wrapper struct. Example, on a `user_follower` junction with both + // `user_id` and `follower_id` -> `user.id`: + // + // pub user_id: UserFollowerPkUserId, + // pub follower_id: UserFollowerPkFollowerId, + // + // 2. FK to parent's PK alias. The column is a foreign key with + // exactly one back-reference recorded in `self.refs`; emit the + // parent's alias (`super::ref::Alias` cross-module, bare `Alias` + // if self-referencing). Example: + // + // pub post_id: super::post::PostPk, + // + // Multi-parent FK columns (`self.refs.len() > 1`) deliberately + // fall through to step 4: no single typed alias can faithfully + // represent a column whose value is an id of either parent. + // + // 3. Own-PK alias. The column is this entity's primary key (or part + // of a composite PK); emit the local alias. Example: + // + // pub id: CakePk, + // + // 4. Raw scalar. Fall through to the inferred Rust scalar (`i32`, + // `String`, ...) as if `--with-pk-newtypes` weren't on. + let inner: TokenStream = if let Some(ctx) = opt.pk_newtype.as_ref() { + let current_table = ctx.current_table.as_str(); + let key = (current_table.to_owned(), self.name.clone()); + + // Step 1: role wrapper + if let Some(ident) = ctx.role_wrappers.get(&key) { + quote! { #ident } + } + // Step 2: FK to parent's alias (single ref only, multi-parent + // FKs fall through to step 4). + else if self.refs.len() == 1 { + let first_ref = &self.refs[0]; + if let Some(ident) = ctx + .pk_aliases + .get(&(first_ref.table.clone(), first_ref.column.clone())) + { + if first_ref.table == current_table { + // Self-referencing FK, emit local name (no super::). + quote! { #ident } + } else { + let module = format_ident!( + "{}", + escape_rust_keyword(first_ref.table.to_snake_case()) + ); + quote! { super::#module::#ident } + } + } else { + write_rs_type(&self.col_type, opt).parse().unwrap() + } + } + // Step 3: own-PK alias (only when the column has no FK back-refs; + // multi-FK columns skip both 2 and 3 and land on the raw scalar). + else if self.refs.is_empty() + && let Some(ident) = ctx.pk_aliases.get(&key) + { + quote! { #ident } + } + // Step 4: raw scalar. Catches both "regular non-PK non-FK column" + // and "multi-parent FK that we deliberately don't newtype." + else { + write_rs_type(&self.col_type, opt).parse().unwrap() + } + } else { + write_rs_type(&self.col_type, opt).parse().unwrap() + }; + match self.not_null { - true => quote! { #ident }, - false => quote! { Option<#ident> }, + true => quote! { #inner }, + false => quote! { Option<#inner> }, } } @@ -304,6 +421,7 @@ impl From<&ColumnDef> for Column { not_null, unique, unique_key: None, + refs: Vec::new(), } } } @@ -319,6 +437,7 @@ mod tests { ColumnOption { date_time_crate: DateTimeCrate::Chrono, big_integer_type: Default::default(), + pk_newtype: None, } } @@ -326,6 +445,7 @@ mod tests { ColumnOption { date_time_crate: DateTimeCrate::Time, big_integer_type: Default::default(), + pk_newtype: None, } } @@ -339,6 +459,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), } }; } diff --git a/sea-orm-codegen/src/entity/transformer.rs b/sea-orm-codegen/src/entity/transformer.rs index fcb782bdaf..2dbbe13b59 100644 --- a/sea-orm-codegen/src/entity/transformer.rs +++ b/sea-orm-codegen/src/entity/transformer.rs @@ -124,6 +124,23 @@ impl EntityTransformer { .collect::>() }), ); + // Populate per-column FK back-references from the relations. + // FK columns will carry `refs: Vec` of + // `(parent_table, parent_column)` so downstream codegen can + // resolve the referenced PK type. + for rel in relations.iter() { + if !matches!(rel.rel_type, RelationType::BelongsTo) { + continue; + } + for (fk_col, parent_col) in rel.columns.iter().zip(rel.ref_columns.iter()) { + if let Some(col) = columns.iter_mut().find(|c| &c.name == fk_col) { + col.refs.push(crate::ColumnRef { + table: rel.ref_table.clone(), + column: parent_col.clone(), + }); + } + } + } let entity = Entity { table_name: table_name.clone(), columns, @@ -414,6 +431,147 @@ mod tests { Ok(()) } + #[test] + fn fk_columns_get_ref_table_and_ref_column_populated() -> Result<(), Box> { + // parent has a unary PK; child has a single FK to parent.id + let parent_stmt = Table::create() + .table("parent") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .to_owned(); + + let child_stmt = Table::create() + .table("child") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new("parent_id").integer().not_null()) + .foreign_key( + ForeignKey::create() + .name("fk-child-parent_id") + .from("child", "parent_id") + .to("parent", "id"), + ) + .to_owned(); + + let entities: HashMap<_, _> = EntityTransformer::transform(vec![parent_stmt, child_stmt])? + .entities + .into_iter() + .map(|entity| (entity.table_name.clone(), entity)) + .collect(); + + let child = entities.get("child").expect("missing entity `child`"); + let parent_id_col = child + .columns + .iter() + .find(|c| c.name == "parent_id") + .expect("missing parent_id column"); + assert_eq!(parent_id_col.refs.len(), 1); + assert_eq!(parent_id_col.refs[0].table, "parent"); + assert_eq!(parent_id_col.refs[0].column, "id"); + + // The PK column itself is not an FK, refs must stay empty. + let id_col = child + .columns + .iter() + .find(|c| c.name == "id") + .expect("missing id column"); + assert!(id_col.refs.is_empty()); + + Ok(()) + } + + #[test] + fn multi_fk_same_column_records_both_backrefs() -> Result<(), Box> { + // The same SQL column (`child.user_id`) is constrained against two + // different parents. `Column::refs` is a `Vec`, so both + // back-references survive the transform. + // + // Downstream behavior under `--with-pk-newtypes`: the writer + // deliberately does NOT pick one parent's alias. Instead it + // emits the raw scalar (`pub user_id: i32`) and the user keeps + // the column unwrapped. See + // `pk_newtypes_multi_parent_fk_falls_back_to_scalar` in + // `writer.rs` for the writer-level assertion. + let users = Table::create() + .table("users") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .to_owned(); + let legacy_users = Table::create() + .table("legacy_users") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .to_owned(); + let child = Table::create() + .table("child") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new("user_id").integer().not_null()) + .foreign_key( + ForeignKey::create() + .name("fk-child-user") + .from("child", "user_id") + .to("users", "id"), + ) + .foreign_key( + ForeignKey::create() + .name("fk-child-legacy_user") + .from("child", "user_id") + .to("legacy_users", "id"), + ) + .to_owned(); + + let entities: HashMap<_, _> = + EntityTransformer::transform(vec![users, legacy_users, child])? + .entities + .into_iter() + .map(|entity| (entity.table_name.clone(), entity)) + .collect(); + + let child = entities.get("child").expect("missing entity `child`"); + let user_id_col = child + .columns + .iter() + .find(|c| c.name == "user_id") + .expect("missing user_id column"); + + assert_eq!( + user_id_col.refs.len(), + 2, + "expected both FKs recorded, got refs = {:?}", + user_id_col.refs + ); + assert!(user_id_col.refs.iter().any(|r| r.table == "users")); + assert!(user_id_col.refs.iter().any(|r| r.table == "legacy_users")); + + Ok(()) + } + #[test] fn filter_relations_to_missing_entities() -> Result<(), Box> { let parent_stmt = || { diff --git a/sea-orm-codegen/src/entity/writer.rs b/sea-orm-codegen/src/entity/writer.rs index abcf4bf8e3..b2dbc5ff27 100644 --- a/sea-orm-codegen/src/entity/writer.rs +++ b/sea-orm-codegen/src/entity/writer.rs @@ -1,8 +1,10 @@ -use crate::{ActiveEnum, ColumnOption, Entity, util::escape_rust_keyword}; +use crate::{ActiveEnum, ColumnOption, Entity, PkNewtypeIndex, util::escape_rust_keyword}; use heck::ToUpperCamelCase; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use std::{collections::BTreeMap, str::FromStr}; +use std::collections::{BTreeMap, HashMap, HashSet}; +use std::rc::Rc; +use std::str::FromStr; use syn::{punctuated::Punctuated, token::Comma}; use tracing::info; @@ -76,6 +78,21 @@ pub enum BannerVersion { Patch, } +/// Controls whether codegen wraps each entity's primary key in a per-table +/// `sea_orm::Id` alias (e.g. `pub type CakePk = ...;`) and +/// propagates that alias to foreign-key column types. +#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)] +pub enum PkNewtypeFormat { + /// Default, emit raw scalar types for primary keys. + #[default] + None, + /// Emit `pub type Pk = sea_orm::Id>;` above the + /// `pub struct Model` declaration, and resolve foreign keys to the + /// parent's alias (`super::ref_table::RefTablePk`). Self-ref junction + /// PK columns get per-column role-wrapper structs instead. + Inline, +} + #[derive(Debug)] pub struct EntityWriterContext { pub(crate) entity_format: EntityFormat, @@ -96,6 +113,7 @@ pub struct EntityWriterContext { pub(crate) seaography: bool, pub(crate) impl_active_model_behavior: bool, pub(crate) banner_version: BannerVersion, + pub(crate) pk_newtype_format: PkNewtypeFormat, } impl WithSerde { @@ -127,6 +145,21 @@ impl WithSerde { } } +/// Strip a single outer `Option<...>` wrapper from a Rust type token stream. +/// Used to render the inner type for PK newtype declarations: a PK column +/// is always NOT NULL but `Column::get_rs_type` defensively wraps optional +/// types in `Option<_>`, which we don't want inside the wrapper struct. +fn strip_optional(ts: TokenStream) -> TokenStream { + let s = ts.to_string(); + let trimmed = s.split_whitespace().collect::(); + if let Some(rest) = trimmed.strip_prefix("Option<") { + if let Some(inner) = rest.strip_suffix('>') { + return inner.parse().unwrap_or(ts); + } + } + ts +} + /// Converts *_extra_derives argument to token stream pub(crate) fn bonus_derive(extra_derives: I) -> TokenStream where @@ -234,6 +267,7 @@ impl EntityWriterContext { seaography: bool, impl_active_model_behavior: bool, banner_version: BannerVersion, + pk_newtype_format: PkNewtypeFormat, ) -> Self { Self { entity_format, @@ -254,13 +288,28 @@ impl EntityWriterContext { seaography, impl_active_model_behavior, banner_version, + pk_newtype_format, } } - fn column_option(&self) -> ColumnOption { + fn column_option( + &self, + entity: &Entity, + pk_aliases: Option<&Rc>, + role_wrappers: Option<&Rc>, + ) -> ColumnOption { + let pk_newtype = match (pk_aliases, role_wrappers) { + (Some(aliases), Some(roles)) => Some(crate::entity::column::PkNewtypeContext { + current_table: entity.table_name.clone(), + pk_aliases: Rc::clone(aliases), + role_wrappers: Rc::clone(roles), + }), + _ => None, + }; ColumnOption { date_time_crate: self.date_time_crate, big_integer_type: self.big_integer_type, + pk_newtype, } } } @@ -297,14 +346,30 @@ impl EntityWriter { } pub fn write_entities(&self, context: &EntityWriterContext) -> Vec { + let pk_newtype_index = if context.pk_newtype_format == PkNewtypeFormat::Inline { + Some(Rc::new(Self::build_pk_newtype_index(&self.entities))) + } else { + None + }; + let role_wrapper_index = if context.pk_newtype_format == PkNewtypeFormat::Inline { + Some(Rc::new(Self::build_role_wrapper_index(&self.entities))) + } else { + None + }; + self.entities .iter() .map(|entity| { let entity_file = format!("{}.rs", entity.get_table_name_snake_case()); + let column_option = context.column_option( + entity, + pk_newtype_index.as_ref(), + role_wrapper_index.as_ref(), + ); let column_info = entity .columns .iter() - .map(|column| column.get_info(&context.column_option())) + .map(|column| column.get_info(&column_option)) .collect::>(); // Serde must be enabled to use this let serde_skip_deserializing_primary_key = context @@ -323,11 +388,12 @@ impl EntityWriter { let mut lines = Vec::new(); Self::write_doc_comment(&mut lines, context.banner_version); + let pk_newtype_decls = Self::gen_pk_newtype_decls(entity, &column_option); let code_blocks = if context.entity_format == EntityFormat::Frontend { Self::gen_frontend_code_blocks( entity, &context.with_serde, - &context.column_option(), + &column_option, &context.schema_name, serde_skip_deserializing_primary_key, serde_skip_hidden_column, @@ -341,7 +407,7 @@ impl EntityWriter { Self::gen_expanded_code_blocks( entity, &context.with_serde, - &context.column_option(), + &column_option, &context.schema_name, serde_skip_deserializing_primary_key, serde_skip_hidden_column, @@ -355,7 +421,7 @@ impl EntityWriter { Self::gen_dense_code_blocks( entity, &context.with_serde, - &context.column_option(), + &column_option, &context.schema_name, serde_skip_deserializing_primary_key, serde_skip_hidden_column, @@ -369,7 +435,7 @@ impl EntityWriter { Self::gen_compact_code_blocks( entity, &context.with_serde, - &context.column_option(), + &column_option, &context.schema_name, serde_skip_deserializing_primary_key, serde_skip_hidden_column, @@ -380,6 +446,37 @@ impl EntityWriter { context.impl_active_model_behavior, ) }; + // The PK newtype aliases and role wrappers need to live + // *after* the `use sea_orm::entity::prelude::*;` import + // (so `sea_orm::Id` resolves) and *before* the model + // declaration (so the model field type can name them). + // + // Without splicing, a file would emit: + // + // pub struct Model { + // pub id: TaskPk, // unresolved + // ... + // } + // use sea_orm::entity::prelude::*; + // + // With splicing, the file reads: + // + // use sea_orm::entity::prelude::*; + // pub type TaskPk = sea_orm::Id; + // pub struct Model { + // pub id: TaskPk, // resolves + // ... + // } + let code_blocks: Vec<_> = if pk_newtype_decls.is_empty() { + code_blocks + } else if let Some((imports, rest)) = code_blocks.split_first() { + std::iter::once(imports.clone()) + .chain(pk_newtype_decls) + .chain(rest.iter().cloned()) + .collect() + } else { + pk_newtype_decls + }; Self::write(&mut lines, code_blocks); OutputFile { name: entity_file, @@ -389,6 +486,168 @@ impl EntityWriter { .collect() } + /// Build the `(table, column) -> AliasIdent` lookup of PK newtype + /// aliases. Naming: unary PK is `Pk`; composite PK + /// columns are `` with a one-step + /// collapse if the combined name would end in `IdId` + /// (e.g. table `cake_id` column `id` -> `CakeId`, not `CakeIdId`). + fn build_pk_newtype_index(entities: &[Entity]) -> PkNewtypeIndex { + let mut index = PkNewtypeIndex::new(); + for entity in entities { + let table_camel = entity.table_name.to_upper_camel_case(); + for pk in entity.primary_keys.iter() { + let ident_str = if entity.primary_keys.len() == 1 { + format!("{table_camel}Pk") + } else { + let column_camel = pk.name.to_upper_camel_case(); + let combined = format!("{table_camel}{column_camel}"); + if combined.ends_with("IdId") { + format!("{}Id", &combined[..combined.len() - 4]) + } else { + combined + } + }; + let ident = format_ident!("{}", ident_str); + index.insert((entity.table_name.clone(), pk.name.clone()), ident); + } + } + index + } + + /// Build the `(own_table, fk_column) -> RoleWrapperIdent` lookup + /// for junction tables whose PK has more than one column FK- + /// referencing the same parent. Wrappers are named + /// `Pk`. PK columns only; non-PK role + /// disambiguation is out of scope. + fn build_role_wrapper_index(entities: &[Entity]) -> PkNewtypeIndex { + let mut index = PkNewtypeIndex::new(); + for entity in entities { + // Count how many of this entity's columns FK-reference each parent + // table. Junction PK columns each carry exactly one back-ref (one + // `ColumnRef` per `BelongsTo` column, see `EntityTransformer`), so + // `.first()` is the sole ref here. This index feeds `get_rs_type` + // step 1, which runs before the single-ref/empty-ref checks, so a + // role wrapper must only ever be emitted for a single-parent column. + let mut ref_counts: HashMap = HashMap::new(); + for col in entity.columns.iter() { + if let Some(first_ref) = col.refs.first() { + *ref_counts.entry(first_ref.table.clone()).or_insert(0) += 1; + } + } + // For PK columns whose parent is referenced by more than one + // column of this entity, emit a role wrapper. + let pk_names: HashSet<&String> = + entity.primary_keys.iter().map(|pk| &pk.name).collect(); + for col in entity.columns.iter() { + if !pk_names.contains(&col.name) { + continue; + } + let Some(first_ref) = col.refs.first() else { + continue; + }; + if ref_counts.get(&first_ref.table).copied().unwrap_or(0) <= 1 { + continue; + } + let table_camel = entity.table_name.to_upper_camel_case(); + let col_camel = col.name.to_upper_camel_case(); + let ident_str = format!("{table_camel}Pk{col_camel}"); + let ident = format_ident!("{}", ident_str); + index.insert((entity.table_name.clone(), col.name.clone()), ident); + } + } + index + } + + /// Emit type aliases and role-wrapper structs for an entity's PK + /// columns. See `Column::get_rs_type` for the resolution rule that + /// downstream call sites use to interpret these declarations. + fn gen_pk_newtype_decls(entity: &Entity, opt: &ColumnOption) -> Vec { + use crate::entity::column::PkNewtypeContext; + let Some(ctx) = opt.pk_newtype.as_ref() else { + return Vec::new(); + }; + let mut decls = Vec::new(); + // Render inner types without the newtype context so we get raw scalars + // (or, for role wrappers, the parent's alias resolved via FK ref). + let raw_opt = ColumnOption { + date_time_crate: opt.date_time_crate, + big_integer_type: opt.big_integer_type, + pk_newtype: None, + }; + // For role wrappers we want the parent's alias path, so we synthesize + // a context with the same `pk_aliases` but an empty `role_wrappers` + // map and a `current_table` that intentionally won't match anything + // in `pk_aliases`, that way `get_rs_type` falls through to the FK + // resolution branch (step 2) and emits `super::ref::ParentAlias` + // rather than the local own-PK alias (step 3). + // + // This is a load-bearing dependency on `Column::get_rs_type`'s branch + // ORDER (column.rs steps 1-3): the empty `role_wrappers` map skips + // step 1, and the non-matching `current_table` skips step 3, leaving + // step 2. Reordering those branches would silently change role-wrapper + // inner types with no compile-time signal. + let parent_resolve_opt = ColumnOption { + date_time_crate: opt.date_time_crate, + big_integer_type: opt.big_integer_type, + pk_newtype: Some(PkNewtypeContext { + current_table: String::from("__synthetic_no_match__"), + pk_aliases: Rc::clone(&ctx.pk_aliases), + role_wrappers: Rc::new(PkNewtypeIndex::default()), + }), + }; + for pk in entity.primary_keys.iter() { + let Some(column) = entity.columns.iter().find(|c| c.name == pk.name) else { + continue; + }; + + // Role wrapper? Emit standalone struct around the parent's alias. + // + // The `try_from_u64` attribute forces `DeriveValueType` to emit + // a `TryFromU64` impl by delegation: the parent's alias is + // `Id`, which impls `TryFromU64` whenever its inner `T` + // does, and every PK inner type codegen supports (integers, + // `String`, `Uuid`) qualifies, so this is always satisfiable. + // Without the attribute the macro's textual allowlist + // (i8…u64/String/Uuid) wouldn't fire, and tuple-PK arity + // (composite junctions) would lose its `TryFromU64` bound. + if let Some(ident) = ctx + .role_wrappers + .get(&(entity.table_name.clone(), pk.name.clone())) + { + // Parent alias type, e.g. `super::user::UserId`. + let parent_ty = column.get_rs_type(&parent_resolve_opt); + let parent_ty = strip_optional(parent_ty); + decls.push(quote! { + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + #[sea_orm(try_from_u64)] + pub struct #ident(pub #parent_ty); + }); + continue; + } + + // FK PK without role-wrapper status? No local emission, the + // column resolves to the parent's alias via Column::get_rs_type. + if !column.refs.is_empty() { + continue; + } + + // Own-PK alias. Render the inner type as a raw scalar so the + // alias is `pub type CakePk = sea_orm::Id;`. + let Some(ident) = ctx + .pk_aliases + .get(&(entity.table_name.clone(), pk.name.clone())) + else { + continue; + }; + let inner_rs = column.get_rs_type(&raw_opt); + let inner_rs = strip_optional(inner_rs); + decls.push(quote! { + pub type #ident = sea_orm::Id; + }); + } + decls + } + pub fn write_index_file( &self, lib: bool, @@ -871,8 +1130,9 @@ impl EntityWriter { #[cfg(test)] mod tests { use crate::{ - Column, ColumnOption, ConjunctRelation, Entity, EntityWriter, PrimaryKey, Relation, - RelationType, WithSerde, + BannerVersion, BigIntegerType, Column, ColumnOption, ConjunctRelation, DateTimeCrate, + Entity, EntityFormat, EntityWriter, EntityWriterContext, PkNewtypeFormat, PrimaryKey, + Relation, RelationType, WithPrelude, WithSerde, entity::writer::{bonus_attributes, bonus_derive}, }; use pretty_assertions::assert_eq; @@ -900,6 +1160,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -908,6 +1169,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -939,6 +1201,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "filling_id".to_owned(), @@ -947,6 +1210,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ @@ -993,6 +1257,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "filling_id".to_owned(), @@ -1001,6 +1266,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "price".to_owned(), @@ -1009,6 +1275,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1042,6 +1309,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -1050,6 +1318,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -1071,6 +1340,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -1079,6 +1349,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "cake_id".to_owned(), @@ -1087,6 +1358,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ @@ -1128,6 +1400,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "_name_".to_owned(), @@ -1136,6 +1409,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "fruitId".to_owned(), @@ -1144,6 +1418,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1172,6 +1447,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "testing".to_owned(), @@ -1180,6 +1456,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "rust".to_owned(), @@ -1188,6 +1465,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "keywords".to_owned(), @@ -1196,6 +1474,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "type".to_owned(), @@ -1204,6 +1483,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "typeof".to_owned(), @@ -1212,6 +1492,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "crate".to_owned(), @@ -1220,6 +1501,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "self".to_owned(), @@ -1228,6 +1510,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "self_id1".to_owned(), @@ -1236,6 +1519,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "self_id2".to_owned(), @@ -1244,6 +1528,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "fruit_id1".to_owned(), @@ -1252,6 +1537,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "fruit_id2".to_owned(), @@ -1260,6 +1546,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "cake_id".to_owned(), @@ -1268,6 +1555,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ @@ -1342,6 +1630,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -1350,6 +1639,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "price".to_owned(), @@ -1358,6 +1648,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1389,6 +1680,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -1397,6 +1689,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "price".to_owned(), @@ -1405,6 +1698,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1436,6 +1730,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "integers".to_owned(), @@ -1444,6 +1739,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "integers_opt".to_owned(), @@ -1452,6 +1748,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -1470,6 +1767,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "floats".to_owned(), @@ -1478,6 +1776,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "doubles".to_owned(), @@ -1486,6 +1785,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -1504,6 +1804,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "id2".to_owned(), @@ -1512,6 +1813,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1545,6 +1847,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "parent_id1".to_owned(), @@ -1553,6 +1856,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "parent_id2".to_owned(), @@ -1561,6 +1865,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -1589,6 +1894,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "b".to_owned(), @@ -1597,6 +1903,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "c".to_owned(), @@ -1605,6 +1912,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "d".to_owned(), @@ -1613,6 +1921,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "e".to_owned(), @@ -1621,6 +1930,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "f".to_owned(), @@ -1629,6 +1939,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "g".to_owned(), @@ -1637,6 +1948,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "h".to_owned(), @@ -1645,6 +1957,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "i".to_owned(), @@ -1653,6 +1966,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "j".to_owned(), @@ -1661,6 +1975,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "k".to_owned(), @@ -1671,6 +1986,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -2248,6 +2564,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -2256,6 +2573,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "base_id".to_owned(), @@ -2264,6 +2582,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ @@ -2951,6 +3270,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "payload".to_owned(), @@ -2959,6 +3279,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "payload_binary".to_owned(), @@ -2967,6 +3288,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -3048,6 +3370,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "first_tea".to_owned(), @@ -3062,6 +3385,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "second_tea".to_owned(), @@ -3076,6 +3400,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -3094,6 +3419,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "first_tea".to_owned(), @@ -3108,6 +3434,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "second_tea".to_owned(), @@ -3122,6 +3449,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "size".to_owned(), @@ -3137,6 +3465,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -3217,4 +3546,206 @@ mod tests { Ok(()) } + + #[test] + fn pk_newtypes_emit_wrappers_and_thread_through_fks() { + use crate::EntityTransformer; + use sea_query::{ColumnDef, ForeignKey, Table}; + + let parent = Table::create() + .table("cake") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new("name").string().not_null()) + .to_owned(); + let child = Table::create() + .table("fruit") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new("cake_id").integer()) + .foreign_key( + ForeignKey::create() + .name("fk-fruit-cake") + .from("fruit", "cake_id") + .to("cake", "id"), + ) + .to_owned(); + + let writer = EntityTransformer::transform(vec![parent, child]).unwrap(); + let context = EntityWriterContext::new( + EntityFormat::Compact, + WithPrelude::None, + WithSerde::None, + false, + DateTimeCrate::Chrono, + BigIntegerType::I64, + None, + false, + false, + false, + vec![], + vec![], + vec![], + vec![], + vec![], + false, + true, + BannerVersion::Off, + PkNewtypeFormat::Inline, + ); + let output = writer.generate(&context); + + let cake = output + .files + .iter() + .find(|f| f.name == "cake.rs") + .expect("missing cake.rs"); + let fruit = output + .files + .iter() + .find(|f| f.name == "fruit.rs") + .expect("missing fruit.rs"); + + // The codegen output is rendered from a TokenStream so spacing is + // not stable across versions, so we normalize to whitespace-free strings + // and check that the expected sequences appear. + let cake_norm: String = cake.content.split_whitespace().collect(); + let fruit_norm: String = fruit.content.split_whitespace().collect(); + + // Parent emits a type alias and the PK field is typed. + assert!( + cake_norm.contains("pubtypeCakePk=sea_orm::Id"), + "cake.rs should declare `pub type CakePk = sea_orm::Id;` (got:\n{})", + cake.content + ); + assert!( + cake_norm.contains("pubid:CakePk"), + "cake.rs should type `id` as `CakePk` (got:\n{})", + cake.content + ); + + // Child emits its own alias plus references the parent's via `super::cake::CakePk`. + assert!( + fruit_norm.contains("pubtypeFruitPk=sea_orm::Id"), + "fruit.rs should declare `pub type FruitPk = sea_orm::Id;` (got:\n{})", + fruit.content + ); + assert!( + fruit_norm.contains("super::cake::CakePk"), + "fruit.rs should reference parent PK as `super::cake::CakePk` (got:\n{})", + fruit.content + ); + } + + /// When a single column is FK-constrained against more than one + /// parent, codegen deliberately opts out of newtyping for that + /// column and emits the raw scalar instead. The companion + /// transformer test `multi_fk_same_column_records_both_backrefs` + /// pins the multiple back-refs being recorded; this test pins the + /// downstream writer choice. See `Column::get_rs_type` for the + /// rationale (no single typed alias can faithfully represent a + /// column that may hold either parent's id). + #[test] + fn pk_newtypes_multi_parent_fk_falls_back_to_scalar() { + use crate::EntityTransformer; + use sea_query::{ColumnDef, ForeignKey, Table}; + + let users = Table::create() + .table("users") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .to_owned(); + let legacy_users = Table::create() + .table("legacy_users") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .to_owned(); + let child = Table::create() + .table("child") + .col( + ColumnDef::new("id") + .integer() + .not_null() + .auto_increment() + .primary_key(), + ) + .col(ColumnDef::new("user_id").integer().not_null()) + .foreign_key( + ForeignKey::create() + .name("fk-child-user") + .from("child", "user_id") + .to("users", "id"), + ) + .foreign_key( + ForeignKey::create() + .name("fk-child-legacy_user") + .from("child", "user_id") + .to("legacy_users", "id"), + ) + .to_owned(); + + let writer = EntityTransformer::transform(vec![users, legacy_users, child]).unwrap(); + let context = EntityWriterContext::new( + EntityFormat::Compact, + WithPrelude::None, + WithSerde::None, + false, + DateTimeCrate::Chrono, + BigIntegerType::I64, + None, + false, + false, + false, + vec![], + vec![], + vec![], + vec![], + vec![], + false, + true, + BannerVersion::Off, + PkNewtypeFormat::Inline, + ); + let output = writer.generate(&context); + let child = output + .files + .iter() + .find(|f| f.name == "child.rs") + .expect("missing child.rs"); + let child_norm: String = child.content.split_whitespace().collect(); + + // Multi-parent FK -> raw scalar at the field-type level. + // (The Relation enum still references both parents, that's + // independent of the column type emission.) + assert!( + child_norm.contains("pubuser_id:i32"), + "child.rs should type multi-parent FK `user_id` as raw `i32` (got:\n{})", + child.content + ); + assert!( + !child_norm.contains("pubuser_id:super::"), + "child.rs should NOT type `user_id` as a parent alias (got:\n{})", + child.content + ); + } } diff --git a/sea-orm-codegen/src/entity/writer/compact.rs b/sea-orm-codegen/src/entity/writer/compact.rs index 6f9e0c423c..ab66926d2c 100644 --- a/sea-orm-codegen/src/entity/writer/compact.rs +++ b/sea-orm-codegen/src/entity/writer/compact.rs @@ -76,6 +76,12 @@ impl EntityWriter { attrs.push(quote! { primary_key }); if !col.auto_increment { attrs.push(quote! { auto_increment = false }); + } else if column_option.pk_newtype.is_some() { + // Emit `auto_increment` explicitly so the regenerated + // entity matches the source schema regardless of how + // `PkAutoIncrementHint` resolves for the wrapper type + // chosen by downstream code. + attrs.push(quote! { auto_increment }); } } if let Some(ts) = col.get_col_type_attrs() { diff --git a/sea-orm-codegen/src/entity/writer/dense.rs b/sea-orm-codegen/src/entity/writer/dense.rs index 4338071417..e5606ce512 100644 --- a/sea-orm-codegen/src/entity/writer/dense.rs +++ b/sea-orm-codegen/src/entity/writer/dense.rs @@ -72,6 +72,12 @@ impl EntityWriter { attrs.push(quote! { primary_key }); if !col.auto_increment { attrs.push(quote! { auto_increment = false }); + } else if column_option.pk_newtype.is_some() { + // Emit `auto_increment` explicitly so the regenerated + // entity matches the source schema regardless of how + // `PkAutoIncrementHint` resolves for the wrapper type + // chosen by downstream code. + attrs.push(quote! { auto_increment }); } } if let Some(ts) = col.get_col_type_attrs() { diff --git a/sea-orm-codegen/src/entity/writer/mermaid.rs b/sea-orm-codegen/src/entity/writer/mermaid.rs index 5793ed02db..754d4f6fb1 100644 --- a/sea-orm-codegen/src/entity/writer/mermaid.rs +++ b/sea-orm-codegen/src/entity/writer/mermaid.rs @@ -168,6 +168,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -176,6 +177,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "email".to_owned(), @@ -184,6 +186,7 @@ mod tests { not_null: true, unique: true, unique_key: None, + refs: Vec::new(), }, Column { name: "parent_id".to_owned(), @@ -192,6 +195,7 @@ mod tests { not_null: false, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ @@ -233,6 +237,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "title".to_owned(), @@ -241,6 +246,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "user_id".to_owned(), @@ -249,6 +255,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![Relation { @@ -280,6 +287,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "name".to_owned(), @@ -288,6 +296,7 @@ mod tests { not_null: true, unique: true, unique_key: None, + refs: Vec::new(), }, ], relations: vec![], @@ -309,6 +318,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, Column { name: "tag_id".to_owned(), @@ -317,6 +327,7 @@ mod tests { not_null: true, unique: false, unique_key: None, + refs: Vec::new(), }, ], relations: vec![ diff --git a/sea-orm-macros/src/derives/entity_model.rs b/sea-orm-macros/src/derives/entity_model.rs index a26fea20fd..0542956fc1 100644 --- a/sea-orm-macros/src/derives/entity_model.rs +++ b/sea-orm-macros/src/derives/entity_model.rs @@ -12,8 +12,6 @@ use syn::{ token::Comma, }; -const NOT_AUTO_INCRE_TYPE_SUFFIX: [&str; 2] = ["String", "Uuid"]; - #[allow(dead_code)] fn convert_case(s: &str, case_style: CaseStyle) -> String { match case_style { @@ -244,14 +242,26 @@ pub fn expand_derive_entity_model( ); } } else if meta.path.is_ident("auto_increment") { - let lit = meta.value()?.parse()?; - if let Lit::Bool(litbool) = lit { - is_auto_increment = litbool.value(); - auto_increment = Some(litbool.value()); + // Accept two attribute forms: + // #[sea_orm(primary_key, auto_increment = false)] + // #[sea_orm(primary_key, auto_increment)] + // The second is shorthand for `auto_increment = true`; + // the `if let Lit::Bool ... else` branches below handle + // the explicit `= ` case, and the outer `else` + // handles the bare form. + if meta.input.peek(syn::Token![=]) { + let lit = meta.value()?.parse()?; + if let Lit::Bool(litbool) = lit { + is_auto_increment = litbool.value(); + auto_increment = Some(litbool.value()); + } else { + return Err(meta.error(format!( + "Invalid auto_increment = {lit:?}" + ))); + } } else { - return Err( - meta.error(format!("Invalid auto_increment = {lit:?}")) - ); + is_auto_increment = true; + auto_increment = Some(true); } } else if meta.path.is_ident("comment") { comment = Some(meta.value()?.parse::()?); @@ -463,15 +473,6 @@ pub fn expand_derive_entity_model( }; let field_span = field.span(); - if is_primary_key && auto_increment.is_none() { - for suffix in NOT_AUTO_INCRE_TYPE_SUFFIX { - if field_type.ends_with(suffix) { - auto_increment = Some(false); - break; - } - } - } - let sea_query_col_type = super::value_type_match::column_type_expr(sql_type, field_type, field_span); @@ -535,9 +536,20 @@ pub fn expand_derive_entity_model( } let primary_key = { - let auto_increment = match auto_increment { - Some(value) => value && primary_keys.len() == 1, - None => primary_keys.len() == 1, + // `PrimaryKeyTrait::auto_increment()` body picks one of three sources: + // 1. Composite PKs always emit `false`. + // 2. An explicit `#[sea_orm(auto_increment = ...)]` on the column emits + // the literal bool. + // 3. Otherwise emit `::IS_AUTO`, so + // `DeriveValueType` wrappers and `Id` aliases resolve through + // their inner type at trait-resolution time. + let auto_increment_body = if primary_keys.len() != 1 { + quote! { false } + } else if let Some(value) = auto_increment { + quote! { #value } + } else { + let first = primary_key_types.first(); + quote! { <#first as sea_orm::PkAutoIncrementHint>::IS_AUTO } }; let primary_key_types = if primary_key_types.len() == 1 { let first = primary_key_types.first(); @@ -557,7 +569,7 @@ pub fn expand_derive_entity_model( type ValueType = #primary_key_types; fn auto_increment() -> bool { - #auto_increment + #auto_increment_body } } } diff --git a/sea-orm-macros/src/derives/value_type.rs b/sea-orm-macros/src/derives/value_type.rs index f140eaeeae..56194d2bef 100644 --- a/sea-orm-macros/src/derives/value_type.rs +++ b/sea-orm-macros/src/derives/value_type.rs @@ -169,16 +169,15 @@ impl DeriveValueTypeStruct { let array_type = &self.array_type; let try_from_u64_impl = if self.can_try_from_u64 { + // Delegate to the inner type's `TryFromU64` impl so this works + // for any wrapped type that itself impls `TryFromU64`, including + // `Uuid` (returns `Err(ConvertFromU64)`) and `String` (returns + // the digit string), not just integer primitives. quote!( #[automatically_derived] impl sea_orm::TryFromU64 for #name { - fn try_from_u64(n: u64) -> Result { - use std::convert::TryInto; - Ok(Self(n.try_into().map_err(|e| sea_orm::DbErr::TryIntoErr { - from: stringify!(u64), - into: stringify!(#name), - source: std::sync::Arc::new(e), - })?)) + fn try_from_u64(n: u64) -> std::result::Result { + <#field_type as sea_orm::TryFromU64>::try_from_u64(n).map(#name) } } ) @@ -244,6 +243,11 @@ impl DeriveValueTypeStruct { } } + #[automatically_derived] + impl sea_orm::DelegatesPkAutoIncrementHint for #name { + type Inner = #field_type; + } + #try_from_u64_impl #impl_not_u8 @@ -342,6 +346,15 @@ impl DeriveValueTypeString { } } + // String-backed wrappers are always non-auto, so impl the hint + // directly. The struct path instead emits + // `DelegatesPkAutoIncrementHint` and resolves through the inner + // type; both spellings yield `false` for a `String` inner. + #[automatically_derived] + impl sea_orm::PkAutoIncrementHint for #name { + const IS_AUTO: bool = false; + } + #impl_not_u8 ) } diff --git a/sea-orm-macros/src/derives/value_type_match.rs b/sea-orm-macros/src/derives/value_type_match.rs index cfba21d052..c7133e8962 100644 --- a/sea-orm-macros/src/derives/value_type_match.rs +++ b/sea-orm-macros/src/derives/value_type_match.rs @@ -145,9 +145,22 @@ pub fn array_type_expr( } pub fn can_try_from_u64(field_type: &str) -> bool { + // Types in this list impl `TryFromU64` directly (defined in + // sea-orm itself). A `DeriveValueType` newtype wrapping one of + // them can therefore be used as a primary key, the generated + // `TryFromU64` impl delegates to the inner type. matches!( field_type, - "i8" | "i16" | "i32" | "i64" | "u8" | "u16" | "u32" | "u64" + "i8" | "i16" + | "i32" + | "i64" + | "u8" + | "u16" + | "u32" + | "u64" + | "String" + | "Uuid" + | "uuid::Uuid" ) } diff --git a/sea-orm-macros/tests/derive_value_type_no_spurious_tryfromu64.rs b/sea-orm-macros/tests/derive_value_type_no_spurious_tryfromu64.rs new file mode 100644 index 0000000000..b2339b2094 --- /dev/null +++ b/sea-orm-macros/tests/derive_value_type_no_spurious_tryfromu64.rs @@ -0,0 +1,56 @@ +//! `DeriveValueType` over an inner type that does NOT impl `TryFromU64` +//! must still compile: the macro must not emit a spurious `TryFromU64` +//! delegation. `MyCustomInner` deliberately omits `TryFromU64`, so if +//! `DeriveValueType` tried to delegate to it this file would fail to build. + +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct MyCustomInner(pub String); + +impl From for sea_orm::Value { + fn from(v: MyCustomInner) -> Self { + sea_orm::Value::String(Some(v.0)) + } +} + +impl sea_orm::TryGetable for MyCustomInner { + fn try_get_by( + res: &sea_orm::QueryResult, + idx: I, + ) -> Result { + String::try_get_by(res, idx).map(MyCustomInner) + } +} + +impl sea_orm::sea_query::ValueType for MyCustomInner { + fn try_from(v: sea_orm::Value) -> Result { + ::try_from(v).map(MyCustomInner) + } + fn type_name() -> String { + "MyCustomInner".to_owned() + } + fn array_type() -> sea_orm::sea_query::ArrayType { + sea_orm::sea_query::ArrayType::String + } + fn column_type() -> sea_orm::sea_query::ColumnType { + sea_orm::sea_query::ColumnType::Text + } +} + +impl sea_orm::sea_query::Nullable for MyCustomInner { + fn null() -> sea_orm::Value { + sea_orm::Value::String(None) + } +} + +// Deliberately NO `impl TryFromU64 for MyCustomInner`. If the +// `DeriveValueType` macro tries to delegate to it, this file won't compile. +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct Wrap(pub MyCustomInner); + +#[test] +fn wrap_over_non_tryfromu64_inner_compiles() { + // The fact that this file compiles at all is the assertion. + let _ = Wrap(MyCustomInner("hi".into())); +} diff --git a/sea-orm-sync/Cargo.toml b/sea-orm-sync/Cargo.toml index fc40e706b2..93692907e6 100644 --- a/sea-orm-sync/Cargo.toml +++ b/sea-orm-sync/Cargo.toml @@ -93,6 +93,7 @@ sea-orm-sync = { path = ".", features = [ ] } time = { version = "0.3.36", features = ["macros"] } tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } +trybuild = { version = "1" } uuid = { version = "1", features = ["v4"] } [features] diff --git a/sea-orm-sync/src/entity/auto_increment_hint.rs b/sea-orm-sync/src/entity/auto_increment_hint.rs new file mode 100644 index 0000000000..cd6b62a324 --- /dev/null +++ b/sea-orm-sync/src/entity/auto_increment_hint.rs @@ -0,0 +1,129 @@ +//! Per-type hint for whether a primary key column defaults to +//! `AUTO_INCREMENT` when the entity declaration doesn't say explicitly. +//! +//! `DeriveEntityModel` consults this trait at trait-resolution time. +//! Integer primitives resolve to `true`; `String`, `Vec` and `Uuid` +//! to `false`. Wrapper types and `Id` aliases resolve through their +//! inner type because each emits a delegating impl of +//! [`DelegatesPkAutoIncrementHint`] (e.g. `Id` -> `true`). +//! +//! For custom PK types outside these categories, either impl this trait +//! on the type or specify `auto_increment` explicitly on the column. + +use crate::EntityTrait; + +/// Default `auto_increment` value used by the entity macro when a +/// primary key column does not specify `#[sea_orm(auto_increment = ...)]` +/// explicitly. +#[diagnostic::on_unimplemented( + message = "`{Self}` cannot be used as a primary key without an explicit `auto_increment` setting", + note = "the entity macro looks up the default via `sea_orm::PkAutoIncrementHint`", + note = "either impl `sea_orm::PkAutoIncrementHint` for `{Self}`, or specify \ + `#[sea_orm(primary_key, auto_increment = false)]` (or `= true`) on the column" +)] +pub trait PkAutoIncrementHint { + /// Whether columns of this type default to `AUTO_INCREMENT` when used + /// as a primary key. + const IS_AUTO: bool; +} + +macro_rules! impl_auto_true { + ($($t:ty),* $(,)?) => { + $( + impl PkAutoIncrementHint for $t { + const IS_AUTO: bool = true; + } + )* + }; +} + +macro_rules! impl_auto_false { + ($($t:ty),* $(,)?) => { + $( + impl PkAutoIncrementHint for $t { + const IS_AUTO: bool = false; + } + )* + }; +} + +impl_auto_true!(i8, i16, i32, i64, u8, u16, u32, u64, isize, usize); +impl_auto_false!(String, Vec); + +#[cfg(feature = "with-uuid")] +impl PkAutoIncrementHint for uuid::Uuid { + const IS_AUTO: bool = false; +} + +#[cfg(feature = "with-uuid")] +mod uuid_fmt_impls { + use super::PkAutoIncrementHint; + use uuid::fmt; + + impl PkAutoIncrementHint for fmt::Braced { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Hyphenated { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Simple { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Urn { + const IS_AUTO: bool = false; + } +} + +/// Internal helper trait: marks a wrapper as delegating its +/// `PkAutoIncrementHint` resolution to an inner type. +/// +/// `DeriveValueType` emits an impl of this for every wrapper it generates. +/// The blanket `PkAutoIncrementHint` impl below bridges from it, deferring +/// the inner-type bound to trait-resolution time rather than a concrete +/// `where` clause that would force a compile error on every wrapper whose +/// inner isn't a PK hint, even when the wrapper is never used as a PK. +pub trait DelegatesPkAutoIncrementHint { + /// The inner type whose `PkAutoIncrementHint` impl is delegated to. + type Inner: ?Sized; +} + +impl PkAutoIncrementHint for T +where + T: DelegatesPkAutoIncrementHint, + T::Inner: PkAutoIncrementHint, +{ + const IS_AUTO: bool = ::IS_AUTO; +} + +/// `Id` delegates to its inner `T`. +impl DelegatesPkAutoIncrementHint for crate::Id +where + E: EntityTrait, +{ + type Inner = T; +} + +#[cfg(test)] +mod tests { + use super::PkAutoIncrementHint; + + #[test] + fn integer_primitives_default_true() { + assert!(::IS_AUTO); + assert!(::IS_AUTO); + assert!(::IS_AUTO); + assert!(::IS_AUTO); + } + + #[test] + fn string_and_bytes_default_false() { + assert!(!::IS_AUTO); + assert!(! as PkAutoIncrementHint>::IS_AUTO); + } + + #[cfg(feature = "with-uuid")] + #[test] + fn uuid_defaults_false() { + assert!(!::IS_AUTO); + } +} diff --git a/sea-orm-sync/src/entity/base_entity.rs b/sea-orm-sync/src/entity/base_entity.rs index 2e73761bdb..7b0ae4f10c 100644 --- a/sea-orm-sync/src/entity/base_entity.rs +++ b/sea-orm-sync/src/entity/base_entity.rs @@ -1,5 +1,5 @@ use crate::{ - ActiveModelBehavior, ActiveModelTrait, ColumnTrait, Delete, DeleteMany, DeleteOne, + ActiveModelBehavior, ActiveModelTrait, ColumnTrait, Delete, DeleteMany, DeleteOne, FindByIdArg, FromQueryResult, Identity, Insert, InsertMany, ModelTrait, PrimaryKeyArity, PrimaryKeyToColumn, PrimaryKeyTrait, QueryFilter, Related, RelationBuilder, RelationTrait, RelationType, Select, Update, UpdateMany, UpdateOne, ValidatedDeleteOne, @@ -201,7 +201,39 @@ pub trait EntityTrait: EntityName { Select::::new().join_join(JoinType::InnerJoin, R::to(), R::via()) } - /// Find a model by primary key + /// Find a model by primary key. + /// + /// `values` must satisfy [`FindByIdArg`], which is implemented + /// blanket for any `T: Into<::ValueType>`. + /// In practice that means: + /// + /// - For an entity with a raw scalar PK (`pub id: i32`), pass any + /// `T: Into`, including `i32`, `u8`, `&i32`, etc. + /// - For an entity with a typed PK newtype (`pub id: UserId` where + /// `UserId` is a `DeriveValueType` wrapper or `sea_orm::Id` + /// alias), pass the newtype itself: `find_by_id(UserId::new(7))`. + /// Raw scalars are rejected, there is no `From for UserId` + /// to satisfy the `Into` bound. + /// - For composite PKs, pass a tuple of the component types + /// (e.g. `(i32, String)` or `(UserId, RoleId)`). + /// + /// # Type-safe PKs + /// + /// Wrap each entity's primary key in a per-entity newtype to get + /// compile-time protection against passing the wrong id type: + /// + /// ```ignore + /// use sea_orm::entity::prelude::*; + /// + /// #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + /// pub struct UserId(pub i32); + /// + /// // Now `user::Entity::find_by_id` accepts only `UserId`, not raw `i32` + /// // and not any other entity's id type. + /// ``` + /// + /// Do **not** add `impl From for UserId`, that re-opens the door to + /// `find_by_id(1)` and defeats the safety contract. /// /// # Example /// @@ -288,11 +320,11 @@ pub trait EntityTrait: EntityName { /// ``` fn find_by_id(values: T) -> Select where - T: Into<::ValueType>, + T: FindByIdArg, { let mut select = Self::find(); let mut keys = Self::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); select = select.filter(col.eq(v)); @@ -1019,11 +1051,11 @@ pub trait EntityTrait: EntityName { /// ``` fn delete_by_id(values: T) -> ValidatedDeleteOne where - T: Into<::ValueType>, + T: FindByIdArg, { let mut am = Self::ActiveModel::default(); let mut keys = Self::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); am.set(col, v); @@ -1138,7 +1170,7 @@ mod tests { fn delete_by_id(value: T) where - T: Into<<::PrimaryKey as PrimaryKeyTrait>::ValueType>, + T: crate::FindByIdArg, { assert_eq!( hello::Entity::delete_by_id(value) diff --git a/sea-orm-sync/src/entity/compound.rs b/sea-orm-sync/src/entity/compound.rs index 2df4a05e75..b0c3b03068 100644 --- a/sea-orm-sync/src/entity/compound.rs +++ b/sea-orm-sync/src/entity/compound.rs @@ -1,8 +1,8 @@ #![allow(missing_docs)] -use super::{ColumnTrait, EntityTrait, PrimaryKeyToColumn, PrimaryKeyTrait}; +use super::{ColumnTrait, EntityTrait, PrimaryKeyToColumn}; use crate::{ - ConnectionTrait, DbErr, IntoSimpleExpr, ItemsAndPagesNumber, Iterable, ModelTrait, QueryFilter, - QueryOrder, + ConnectionTrait, DbErr, FindByIdArg, IntoSimpleExpr, ItemsAndPagesNumber, Iterable, ModelTrait, + QueryFilter, QueryOrder, }; use sea_query::{IntoValueTuple, Order, TableRef}; use std::marker::PhantomData; @@ -20,10 +20,10 @@ pub trait EntityLoaderTrait: QueryFilter + QueryOrder + Clone { /// Find a model by primary key fn filter_by_id(mut self, values: T) -> Self where - T: Into<::ValueType>, + T: FindByIdArg, { let mut keys = E::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); self.filter_mut(col.eq(v)); diff --git a/sea-orm-sync/src/entity/id.rs b/sea-orm-sync/src/entity/id.rs new file mode 100644 index 0000000000..b924f2aa30 --- /dev/null +++ b/sea-orm-sync/src/entity/id.rs @@ -0,0 +1,296 @@ +//! Phantom-typed primary-key handle. +//! +//! `Id` wraps a primary-key value of underlying type `T` and tags it +//! with the entity `E` at the type level. Two `Id` types with different +//! entity tags are never inter-convertible, so the compiler rejects +//! cross-entity ID confusion at use sites, e.g. passing a +//! `Id` to `user::Entity::find_by_id` is a type error. +//! +//! `Id` is reachable as `sea_orm::Id` and is intentionally absent from the +//! entity prelude: hand-written aliases spell it `sea_orm::Id`. +//! +//! ## Type parameters +//! +//! `T` is always the raw scalar, `Id::value: T`. Keeping the scalar +//! as an explicit type parameter (rather than projecting it through an +//! associated type on `E`) keeps `PrimaryKey::ValueType = Id` from +//! becoming infinitely recursive (`Id::Inner = Id::Inner = …`), +//! since the alias spells `T` directly: +//! `pub type CakePk = sea_orm::Id;`. +//! +//! ## Usage +//! +//! ```ignore +//! use sea_orm::entity::prelude::*; +//! +//! // Codegen emits this as a one-line alias per entity: +//! pub type CakePk = sea_orm::Id; +//! +//! // The model field uses the alias: +//! pub struct Model { +//! pub id: CakePk, +//! pub name: String, +//! } +//! +//! // Construction is explicit, `Id::new` (no `From` blanket): +//! let id = CakePk::new(7); +//! +//! // Queries use the typed handle: +//! let cake = cake::Entity::find_by_id(id).one(db)?; +//! ``` +//! +//! ## Safety contract +//! +//! `Id` deliberately does NOT impl `From` for any specific scalar. +//! The only construction path is [`Id::new`]. This is what makes +//! `user::Entity::find_by_id(7_i32)` fail to compile when the entity's PK +//! is `Id`: there's no `i32: Into>` +//! impl. Do not add such a `From` impl; it re-opens the cross-entity hole +//! this type exists to close. + +use crate::{ + ColIdx, DbErr, EntityTrait, PrimaryKeyTrait, QueryResult, TryFromU64, TryGetError, TryGetable, +}; +use sea_query::{ArrayType, ColumnType, Nullable, Value, ValueType, ValueTypeErr}; +use std::fmt; +use std::hash::{Hash, Hasher}; +use std::marker::PhantomData; + +/// Phantom-typed wrapper around a primary-key value. +/// +/// `E` is the entity this id belongs to (a marker, never stored at +/// runtime). `T` is the raw stored value: +/// - For unary PKs, the scalar type (`i32`, `Uuid`, `String`, …). +/// - For composite PKs, a tuple of the typed components +/// (e.g. `(super::cake::CakePk, super::filling::FillingPk)`). +/// +/// See the [module-level docs](self) for usage and the safety contract. +#[repr(transparent)] +pub struct Id { + /// The raw stored value. Public for ergonomic read/unwrap; the + /// no-`From` contract blocks implicit call-site conversion, not field + /// access (the entity tag lives in `_marker`, not here, so reading or + /// mutating `value` cannot turn an `Id` into an `Id`). + pub value: T, + // `PhantomData E>` makes `E` invariant (E appears in both + // parameter and return position), so the compiler never widens an + // `Id` to an `Id`. Function-pointer types are always Send + + // Sync, so this marker preserves those auto-traits on `Id`. + _marker: PhantomData E>, +} + +impl Id { + /// Wrap a raw value as a typed entity ID. This is the only construction + /// path, there is no `From` blanket impl, which is what gives + /// `Id` its type-safety contract. + pub const fn new(value: T) -> Self { + Self { + value, + _marker: PhantomData, + } + } + + /// Unwrap to the raw stored value, consuming the wrapper. + pub fn into_inner(self) -> T { + self.value + } +} + +// Manual impls so the trait bounds land on `T` (the stored value) rather +// than `E` (a phantom). + +impl Clone for Id { + fn clone(&self) -> Self { + Self::new(self.value.clone()) + } +} + +impl Copy for Id {} + +impl fmt::Debug for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Include the entity tag so `Id(7)` and + // `Id(7)` don't look identical in logs, that + // defeats the entire reason this wrapper exists. + // + // Every entity struct is named `Entity` by convention, so the + // disambiguating part is the module that contains it. We render + // `::`, the last two `::`-segments + // of `std::any::type_name::()`. Full paths are too verbose + // for log lines; the trailing two segments preserve the + // disambiguation while staying readable. + let full = std::any::type_name::(); + let mut tail = full.rsplitn(3, "::"); + let last = tail.next().unwrap_or(full); + let prev = tail.next(); + let label = match prev { + Some(p) => format!("{p}::{last}"), + None => last.to_owned(), + }; + write!(f, "Id<{label}>({:?})", self.value) + } +} + +impl PartialEq for Id { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +impl Eq for Id {} + +impl Hash for Id { + fn hash(&self, state: &mut H) { + self.value.hash(state); + } +} + +impl fmt::Display for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.value.fmt(f) + } +} + +// === PrimaryKeyTrait::ValueType bounds ====================================== +// +// All five trait bounds below delegate to `T`. `Into` (unary `T`) +// bridges to sea-query's blanket `From for ValueTuple`, auto-deriving +// `Id: IntoValueTuple`. Composite PKs never use `Id`: each +// component is a unary `Id` and the tuple is just +// `(CakePk, FillingPk)`. + +impl From> for Value +where + T: Into, +{ + fn from(id: Id) -> Self { + id.value.into() + } +} + +// `FromValueTuple` comes from sea-query's blanket impl once `Into` +// (above) and `ValueType` (below) are present; for composite `T` neither +// fires, which is intentional. + +// `TryGetable` (single-column read) also auto-derives `TryGetableMany` for +// `Id` and, via the per-arity macro, for tuples of `Id` (what +// composite PKs need). `Id` does not impl `TryGetable` for tuple `T`: +// composite PKs use tuples of unary `Id`, not `Id`. +impl TryGetable for Id { + fn try_get_by(res: &QueryResult, idx: I) -> Result { + T::try_get_by(res, idx).map(Id::new) + } +} + +impl TryFromU64 for Id { + fn try_from_u64(n: u64) -> Result { + T::try_from_u64(n).map(Id::new) + } +} + +// `PrimaryKeyArity` is auto-derived via the blanket +// `impl PrimaryKeyArity for V { const ARITY = 1 }`. + +// `sea_query::ValueType` lets `DeriveEntityModel` call +// `::column_type()` for the SQL column type. Only when +// `T: ValueType` (a single scalar); composite PKs ask each column instead. +impl ValueType for Id { + fn try_from(v: Value) -> Result { + T::try_from(v).map(Id::new) + } + + fn type_name() -> String { + T::type_name() + } + + fn array_type() -> ArrayType { + T::array_type() + } + + fn column_type() -> ColumnType { + T::column_type() + } +} + +// `Nullable` so the macro can wrap the column in `Option>` for +// nullable FK columns. +impl Nullable for Id { + fn null() -> Value { + T::null() + } +} + +// === FindByIdArg ============================================================ +// +// `find_by_id` / `filter_by_id` accept anything convertible to the entity's +// primary-key value type. We could bound that directly with `Into`, but doing +// so makes the compiler's "this argument is wrong" diagnostic +// incomprehensible: it reads something like +// `the trait bound `Id: From>` +// is not satisfied`, +// burying the two entity types inside generic args of `Into`. +// +// `FindByIdArg` is a thin sea-orm-owned wrapper around that same `Into` +// bound. It exists solely so we can attach `#[diagnostic::on_unimplemented]` +// to it, diagnostics can't be attached to `Into` (a std trait). The blanket +// impl forwards through `Into`, so every existing call site still works +// without change. When the bound *fails*, the user sees a message that names +// the entity and the argument type directly. +// +// MSRV is 1.85; `#[diagnostic::on_unimplemented]` is stable since 1.78. + +/// Helper bound used by `find_by_id` / `filter_by_id`. +/// +/// Implemented for every `T` that converts into `E`'s primary-key value type +/// via `Into`. This trait exists to provide a better compiler error than the +/// raw `Into` bound when the argument doesn't match, see the module docs. +#[diagnostic::on_unimplemented( + message = "`{Self}` cannot be used as a primary-key argument for `{E}`", + label = "expected `{E}`'s `PrimaryKey::ValueType` (or something convertible to it), got `{Self}`", + note = "type-safe `Id` wrappers deliberately do not impl `From` to prevent cross-entity ID confusion. Construct ids explicitly with `Id::new(..)` (or the per-entity alias's `::new`), and pass an id belonging to this entity." +)] +pub trait FindByIdArg: Sized { + /// Convert this argument into the entity's primary-key value tuple. + fn into_pk_value(self) -> ::ValueType; +} + +// `do_not_recommend` (stable 1.85) tells rustc not to surface this blanket impl +// in error messages when its where-clause fails. Without it, the user sees a +// confusing message about `From>` not being implemented +// for `Id`, the deeper sub-bound, instead of the +// `on_unimplemented` message on `FindByIdArg` itself. +#[diagnostic::do_not_recommend] +impl FindByIdArg for T +where + T: Into<::ValueType>, +{ + fn into_pk_value(self) -> ::ValueType { + self.into() + } +} + +// === Serde ================================================================== +// +// Transparent: `Id` serializes as just the inner `T`, not as a +// wrapper object. Gated behind `with-json` like the rest of sea-orm's serde +// surface (see `entity/compound/has_one.rs` for the same pattern). + +#[cfg(feature = "with-json")] +impl serde::Serialize for Id { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.value.serialize(serializer) + } +} + +#[cfg(feature = "with-json")] +impl<'de, E: EntityTrait, T: serde::Deserialize<'de>> serde::Deserialize<'de> for Id { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + T::deserialize(deserializer).map(Id::new) + } +} diff --git a/sea-orm-sync/src/entity/mod.rs b/sea-orm-sync/src/entity/mod.rs index b0216be3ac..1f009569a1 100644 --- a/sea-orm-sync/src/entity/mod.rs +++ b/sea-orm-sync/src/entity/mod.rs @@ -102,10 +102,12 @@ mod active_model_ex; mod active_value; #[cfg(feature = "with-arrow")] mod arrow_schema; +mod auto_increment_hint; mod base_entity; pub(crate) mod column; mod column_def; pub mod compound; +mod id; mod identity; mod link; mod model; @@ -125,10 +127,12 @@ pub use active_model_ex::*; pub use active_value::*; #[cfg(feature = "with-arrow")] pub use arrow_schema::*; +pub use auto_increment_hint::*; pub use base_entity::*; pub use column::*; pub use column_def::*; pub use compound::EntityLoaderTrait; +pub use id::*; pub use identity::*; pub use link::*; pub use model::*; diff --git a/sea-orm-sync/src/entity/primary_key.rs b/sea-orm-sync/src/entity/primary_key.rs index d9fbf4fef2..bfae0b2b3e 100644 --- a/sea-orm-sync/src/entity/primary_key.rs +++ b/sea-orm-sync/src/entity/primary_key.rs @@ -47,7 +47,23 @@ pub trait PrimaryKeyTrait: IdenStatic + Iterable { + TryFromU64 + PrimaryKeyArity; - /// Method to call to perform `AUTOINCREMENT` operation on a Primary Key + /// Method to call to perform `AUTOINCREMENT` operation on a Primary Key. + /// + /// The `DeriveEntityModel` macro emits this from one of three sources, in order: + /// + /// 1. An explicit `#[sea_orm(auto_increment = true/false)]` on the primary + /// key column always wins. + /// 2. Composite primary keys (more than one `primary_key` column) always + /// return `false`. + /// 3. Otherwise the default is resolved at trait-resolution time via + /// [`crate::PkAutoIncrementHint`] on the column's type, integer + /// primitives default to `true`, `String` / `Uuid` / `Vec` default + /// to `false`, and `DeriveValueType` wrappers delegate to their inner + /// type (so `RoleId(pub i64)` → `true`, `Token(pub String)` → `false`). + /// + /// For custom primary key types that do not impl `PkAutoIncrementHint`, + /// provide an explicit `auto_increment = ...` on the column or impl the + /// trait on the type. fn auto_increment() -> bool; } diff --git a/sea-orm-sync/src/query/helper.rs b/sea-orm-sync/src/query/helper.rs index 7aef179692..c3b0971722 100644 --- a/sea-orm-sync/src/query/helper.rs +++ b/sea-orm-sync/src/query/helper.rs @@ -909,7 +909,7 @@ pub(crate) fn join_tbl_on_condition( foreign_keys: Identity, ) -> Condition { let mut cond = Condition::all(); - for (owner_key, foreign_key) in owner_keys.into_iter().zip(foreign_keys.into_iter()) { + for (owner_key, foreign_key) in owner_keys.into_iter().zip(foreign_keys) { cond = cond .add(Expr::col((from_tbl.clone(), owner_key)).equals((to_tbl.clone(), foreign_key))); } diff --git a/sea-orm-sync/tests/auto_increment_hint_tests.rs b/sea-orm-sync/tests/auto_increment_hint_tests.rs new file mode 100644 index 0000000000..90b73e2563 --- /dev/null +++ b/sea-orm-sync/tests/auto_increment_hint_tests.rs @@ -0,0 +1,155 @@ +//! Positive tests for `PkAutoIncrementHint` resolution. +//! +//! These pin the contract that `DeriveEntityModel` emits the trait call +//! correctly and that the trait propagates through `DeriveValueType` +//! wrappers and `Id` aliases. + +use sea_orm::{DeriveValueType, Id, PkAutoIncrementHint, PrimaryKeyTrait, entity::prelude::*}; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct IntegerWrapper(pub i64); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct StringWrapper(pub String); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct NestedIntegerWrapper(pub IntegerWrapper); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct NestedStringWrapper(pub StringWrapper); + +mod ent_for_id { + use super::*; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_id")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: i32, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity whose PK is an `Id` alias (the shape +/// `sea-orm-cli generate --with-pk-newtypes` produces). Exercises +/// trait resolution one layer up from raw scalars: the macro emits +/// `::IS_AUTO`, which resolves via the +/// `DelegatesPkAutoIncrementHint` blanket on `Id` down to the +/// inner `i64`. +mod ent_for_typed_pk { + use sea_orm::entity::prelude::*; + pub type EntId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_typed_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: EntId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity with a composite PK whose components are themselves typed +/// `Id` aliases. The macro must short-circuit to `false` +/// regardless of how the trait would resolve for the individual +/// component types. +mod ent_with_composite_pk { + use sea_orm::entity::prelude::*; + pub type LeftId = sea_orm::Id; + pub type RightId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_with_composite_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub left_id: LeftId, + #[sea_orm(primary_key)] + pub right_id: RightId, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity whose PK is a bare `type X = i32` alias (not an `Id`). +/// A transparent alias resolves identically to its target, so the macro +/// emits `::IS_AUTO`, which is the +/// `i32` impl and yields `true`. +mod ent_for_bare_alias { + use super::*; + pub type BareUserId = i32; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_bare_alias")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: BareUserId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +#[test] +fn primitive_integer_defaults_true() { + assert!(::IS_AUTO); + assert!(::IS_AUTO); +} + +#[test] +fn primitive_string_defaults_false() { + assert!(!::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_integer() { + assert!(::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_string() { + assert!(!::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_through_nested() { + assert!(::IS_AUTO); + assert!(!::IS_AUTO); +} + +#[test] +fn id_alias_propagates_through_inner() { + type IntId = Id; + type StrId = Id; + assert!(::IS_AUTO); + assert!(!::IS_AUTO); +} + +#[test] +fn entity_with_i32_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn entity_with_id_alias_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn entity_with_bare_alias_i32_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn composite_pk_is_never_auto_increment() { + assert!(!::auto_increment()); +} + +#[cfg(feature = "with-uuid")] +#[test] +fn uuid_defaults_false() { + assert!(!::IS_AUTO); +} diff --git a/sea-orm-sync/tests/common/features/schema.rs b/sea-orm-sync/tests/common/features/schema.rs index c0e0faf0b3..340deeec89 100644 --- a/sea-orm-sync/tests/common/features/schema.rs +++ b/sea-orm-sync/tests/common/features/schema.rs @@ -775,7 +775,13 @@ pub fn create_value_type_table(db: &DbConn) -> Result { .to_owned(); create_table(db, &general_stmt, value_type::value_type_general::Entity)?; - create_table_from_entity(db, value_type::value_type_pk::Entity) + create_table_from_entity(db, value_type::value_type_pk::Entity)?; + create_table_from_entity(db, value_type::value_type_token_pk::Entity) +} + +#[cfg(feature = "with-uuid")] +pub fn create_value_type_uuid_pk_table(db: &DbConn) -> Result { + create_table_from_entity(db, value_type::value_type_uuid_pk::Entity) } pub fn create_value_type_postgres_table(db: &DbConn) -> Result { diff --git a/sea-orm-sync/tests/common/features/value_type.rs b/sea-orm-sync/tests/common/features/value_type.rs index a58dbc283b..b7705624bb 100644 --- a/sea-orm-sync/tests/common/features/value_type.rs +++ b/sea-orm-sync/tests/common/features/value_type.rs @@ -55,6 +55,45 @@ pub mod value_type_pk { impl ActiveModelBehavior for ActiveModel {} } +/// String-backed newtype PK, exercises `PkAutoIncrementHint` resolving +/// through `DeriveValueType` to `false`. +pub mod value_type_token_pk { + use super::*; + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "value_type_token_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: Token, + pub note: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +#[cfg(feature = "with-uuid")] +pub mod value_type_uuid_pk { + use super::*; + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "value_type_uuid_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: UuidPk, + pub note: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + use sea_orm::entity::prelude::*; #[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] @@ -69,6 +108,13 @@ where } } +#[derive(Clone, Debug, PartialEq, Eq, Hash, DeriveValueType)] +pub struct Token(pub String); + +#[cfg(feature = "with-uuid")] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] +pub struct UuidPk(pub uuid::Uuid); + #[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] pub struct StringVec(pub Vec); diff --git a/sea-orm-sync/tests/id_typed_pk_tests.rs b/sea-orm-sync/tests/id_typed_pk_tests.rs new file mode 100644 index 0000000000..03b32bdeed --- /dev/null +++ b/sea-orm-sync/tests/id_typed_pk_tests.rs @@ -0,0 +1,165 @@ +//! Runtime behaviour tests for the `Id` primary-key newtype. +//! +//! These pin the behaviour the type-safety contract relies on: +//! - Layout: `#[repr(transparent)]` holds, so `Id` and +//! `Option>` are the same size as the raw scalar. +//! - Hashability: typed PKs of different entities coexist in distinct +//! `HashMap`s keyed by their newtype. +//! - Display/Debug: `Display` delegates to the inner value; `Debug` +//! includes the entity tag so different entities render distinctly. +//! - `into_inner` round-trips back to the raw scalar. +//! - Serde shape: `Id` is transparent (a bare number/string, not an +//! object), including `Option>` and a `String`-typed alias. +//! +//! Cross-entity confusion (e.g. comparing or passing the wrong entity's id) +//! is a compile error, pinned separately by the trybuild fixtures under +//! `tests/value_type_pk_compile_fail/`. +//! +//! Two minimal local entities (`post`, `user`) keep this file self-contained. + +use std::collections::HashMap; +use std::mem::size_of; + +mod post { + use sea_orm::entity::prelude::*; + + pub type PostId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +#[test] +fn layout_is_repr_transparent() { + assert_eq!(size_of::(), size_of::()); + assert_eq!(size_of::(), size_of::()); + assert_eq!(size_of::>(), size_of::>()); +} + +#[test] +fn typed_ids_are_hashable_in_separate_maps() { + let mut posts: HashMap = HashMap::new(); + let mut users: HashMap = HashMap::new(); + posts.insert(post::PostId::new(7), "post-7"); + users.insert(user::UserId::new(7), "user-7"); + assert_eq!(posts.get(&post::PostId::new(7)), Some(&"post-7")); + assert_eq!(users.get(&user::UserId::new(7)), Some(&"user-7")); + // Same inner value, different keyspace. + assert_eq!(posts.len(), 1); + assert_eq!(users.len(), 1); +} + +#[test] +fn copy_and_clone_only_when_inner_supports_them() { + // i32 is Copy → PostId is Copy. + let a = post::PostId::new(3); + let b = a; + assert_eq!(a, b); + let c = a.clone(); + assert_eq!(a, c); +} + +#[test] +fn into_inner_round_trip() { + let id = post::PostId::new(42); + assert_eq!(id.into_inner(), 42i32); +} + +#[test] +fn display_delegates_to_inner() { + let id = post::PostId::new(101); + assert_eq!(format!("{}", id), "101"); +} + +#[test] +fn debug_includes_entity_tag() { + // The Debug impl prints the entity tag so that + // `Id(7)` and `Id(7)` don't render + // identically in logs. The tag is the last two `::` segments of + // `type_name::()` to disambiguate entities that all conventionally + // name their inner struct `Entity`. + let post_id = post::PostId::new(7); + let user_id = user::UserId::new(7); + let post_dbg = format!("{post_id:?}"); + let user_dbg = format!("{user_id:?}"); + assert!(post_dbg.contains("post::Entity"), "got: {post_dbg}"); + assert!(user_dbg.contains("user::Entity"), "got: {user_dbg}"); + assert_ne!( + post_dbg, user_dbg, + "Debug output must distinguish post::Entity from user::Entity" + ); + assert!(post_dbg.ends_with("(7)")); +} + +#[cfg(feature = "with-json")] +mod serde_shape { + use super::*; + + #[test] + fn typed_id_serialises_as_bare_number() { + let id = post::PostId::new(7); + let v = serde_json::to_value(&id).expect("serialise"); + assert_eq!(v, serde_json::json!(7)); + } + + #[test] + fn typed_id_deserialises_from_bare_number() { + let v = serde_json::json!(13); + let id: post::PostId = serde_json::from_value(v).expect("deserialise"); + assert_eq!(id, post::PostId::new(13)); + } + + #[test] + fn option_typed_id_serialises_null_and_some() { + let none: Option = None; + assert_eq!( + serde_json::to_value(&none).unwrap(), + serde_json::Value::Null + ); + let some = Some(post::PostId::new(5)); + assert_eq!(serde_json::to_value(&some).unwrap(), serde_json::json!(5)); + } +} + +/// `Id` accepts a `String` payload and survives a serde +/// round-trip. The phantom entity here is `user::Entity`, only the +/// inner-`T` behaviour is under test. +#[cfg(feature = "with-json")] +#[test] +fn string_typed_id_round_trips_through_serde() { + type StringPk = sea_orm::Id; + let id: StringPk = sea_orm::Id::new("abc-xyz".to_string()); + let v = serde_json::to_value(&id).unwrap(); + assert_eq!(v, serde_json::json!("abc-xyz")); + let back: StringPk = serde_json::from_value(v).unwrap(); + assert_eq!(back, sea_orm::Id::new("abc-xyz".to_string())); +} diff --git a/sea-orm-sync/tests/value_type_pk_compile_fail/cross_entity_eq.rs b/sea-orm-sync/tests/value_type_pk_compile_fail/cross_entity_eq.rs new file mode 100644 index 0000000000..1070c20a71 --- /dev/null +++ b/sea-orm-sync/tests/value_type_pk_compile_fail/cross_entity_eq.rs @@ -0,0 +1,41 @@ +//! Comparing two `Id` of DIFFERENT entities must not compile, +//! even when the inner scalar type matches. `PartialEq` is impl'd as +//! `impl PartialEq for Id` (same E), so `Id` +//! and `Id` have no shared `PartialEq` impl. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + pub type UserId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +mod post { + use sea_orm::entity::prelude::*; + pub type PostId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Direct equality across entities, must not compile. + let _ = user::UserId::new(7) == post::PostId::new(7); +} diff --git a/sea-orm-sync/tests/value_type_pk_compile_fail/raw_int_rejected.rs b/sea-orm-sync/tests/value_type_pk_compile_fail/raw_int_rejected.rs new file mode 100644 index 0000000000..124abdfa3a --- /dev/null +++ b/sea-orm-sync/tests/value_type_pk_compile_fail/raw_int_rejected.rs @@ -0,0 +1,29 @@ +//! Passing a raw `i32` to `find_by_id` must not compile when the PK is +//! `Id`. `Id` deliberately does not impl +//! `From`, so `i32: !Into>`. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Raw `i32` not convertible into `Id`. + let _ = user::Entity::find_by_id(1i32); +} diff --git a/sea-orm-sync/tests/value_type_pk_compile_fail/self_ref_role_swap.rs b/sea-orm-sync/tests/value_type_pk_compile_fail/self_ref_role_swap.rs new file mode 100644 index 0000000000..85fb213806 --- /dev/null +++ b/sea-orm-sync/tests/value_type_pk_compile_fail/self_ref_role_swap.rs @@ -0,0 +1,71 @@ +//! Self-ref junction tables (where 2+ FK columns point at the same parent) +//! get per-column "role wrapper" structs around the parent's PK alias. +//! This makes positional swaps (passing one role where the other is +//! expected) fail to compile. + +use sea_orm::entity::prelude::*; +use sea_orm::ActiveValue::Set; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod user_follower { + use sea_orm::entity::prelude::*; + + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + #[sea_orm(try_from_u64)] + pub struct UserFollowerPkUserId(pub super::user::UserId); + + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + #[sea_orm(try_from_u64)] + pub struct UserFollowerPkFollowerId(pub super::user::UserId); + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user_follower")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub user_id: UserFollowerPkUserId, + #[sea_orm(primary_key, auto_increment = false)] + pub follower_id: UserFollowerPkFollowerId, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + let user = user::Model { + id: user::UserId::new(1), + name: "alice".into(), + }; + let follower = user::Model { + id: user::UserId::new(2), + name: "bob".into(), + }; + + // Deliberately swapped roles. The role wrappers make these distinct + // types: `UserFollowerPkUserId` and `UserFollowerPkFollowerId` are not + // inter-convertible, so passing one where the other is expected fails. + let _ = user_follower::ActiveModel { + user_id: Set(user_follower::UserFollowerPkFollowerId(follower.id)), // wrong slot + follower_id: Set(user_follower::UserFollowerPkUserId(user.id)), // wrong slot + }; +} diff --git a/sea-orm-sync/tests/value_type_pk_compile_fail/wrong_id_type.rs b/sea-orm-sync/tests/value_type_pk_compile_fail/wrong_id_type.rs new file mode 100644 index 0000000000..45ec447c85 --- /dev/null +++ b/sea-orm-sync/tests/value_type_pk_compile_fail/wrong_id_type.rs @@ -0,0 +1,48 @@ +//! Passing a `PostId` to `user::Entity::find_by_id` must not compile. +//! `Id` and `Id` are type-distinct +//! despite having the same inner scalar. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod post { + use sea_orm::entity::prelude::*; + + pub type PostId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Cross-entity ID confusion, must not compile. + let _ = user::Entity::find_by_id(post::PostId::new(1)); +} diff --git a/sea-orm-sync/tests/value_type_pk_safety_tests.rs b/sea-orm-sync/tests/value_type_pk_safety_tests.rs new file mode 100644 index 0000000000..4f8d13d763 --- /dev/null +++ b/sea-orm-sync/tests/value_type_pk_safety_tests.rs @@ -0,0 +1,19 @@ +//! Compile-fail harness for `Id`-wrapped primary keys. +//! +//! Each fixture under `tests/value_type_pk_compile_fail/` must fail to +//! compile. This harness asserts only the must-fail invariant; the captured +//! stderr is regenerated on every run via `TRYBUILD=overwrite` and the +//! `.stderr` files are gitignored, so the exact compiler wording is not +//! pinned (it varies across rustc releases). + +#[test] +fn pk_safety() { + // SAFETY: env vars touch global state visible to other threads, but + // this test runs synchronously and the only consumer of `TRYBUILD` + // is trybuild itself, invoked on the line below. + unsafe { + std::env::set_var("TRYBUILD", "overwrite"); + } + let t = trybuild::TestCases::new(); + t.compile_fail("tests/value_type_pk_compile_fail/*.rs"); +} diff --git a/sea-orm-sync/tests/value_type_tests.rs b/sea-orm-sync/tests/value_type_tests.rs index f84b291f41..99954c4042 100644 --- a/sea-orm-sync/tests/value_type_tests.rs +++ b/sea-orm-sync/tests/value_type_tests.rs @@ -9,8 +9,8 @@ pub use common::{ TestContext, features::{ value_type::{ - MyInteger, StringVec, Tag1, Tag2, Tag3, Tag4, Tag5, value_type_general, value_type_pg, - value_type_pk, + MyInteger, StringVec, Tag1, Tag2, Tag3, Tag4, Tag5, Token, value_type_general, + value_type_pg, value_type_pk, value_type_token_pk, }, *, }, @@ -27,12 +27,20 @@ use sea_query::{ArrayType, ColumnType, PostgresQueryBuilder, Value, ValueType, V fn main() -> Result<(), DbErr> { type_test(); conversion_test(); + auto_increment_test(); let ctx = TestContext::new("value_type_tests"); create_value_type_table(&ctx.db)?; insert_value_general(&ctx.db)?; insert_value_pk(&ctx.db)?; + insert_value_token_pk(&ctx.db)?; + + #[cfg(feature = "with-uuid")] + { + create_value_type_uuid_pk_table(&ctx.db)?; + insert_value_uuid_pk(&ctx.db)?; + } if cfg!(feature = "sqlx-postgres") { create_value_type_postgres_table(&ctx.db)?; @@ -79,6 +87,42 @@ pub fn insert_value_pk(db: &DatabaseConnection) -> Result<(), DbErr> { Ok(()) } +#[cfg(feature = "with-uuid")] +pub fn insert_value_uuid_pk(db: &DatabaseConnection) -> Result<(), DbErr> { + use common::features::value_type::{UuidPk, value_type_uuid_pk}; + let the_uuid = uuid::Uuid::new_v4(); + let model = value_type_uuid_pk::Model { + id: UuidPk(the_uuid), + note: "uuid pk round-trip".to_string(), + }; + let result = model.clone().into_active_model().insert(db)?; + assert_eq!(result, model); + + let fetched = value_type_uuid_pk::Entity::find_by_id(UuidPk(the_uuid)) + .one(db)? + .expect("uuid pk row should be readable"); + assert_eq!(fetched, model); + Ok(()) +} + +pub fn insert_value_token_pk(db: &DatabaseConnection) -> Result<(), DbErr> { + let model = value_type_token_pk::Model { + id: Token("abc-123".to_string()), + note: "non-integer PK newtype".to_string(), + }; + let result = model.clone().into_active_model().insert(db)?; + assert_eq!(result, model); + + assert_eq!( + value_type_token_pk::Entity::find_by_id(Token("abc-123".to_string())) + .one(db)? + .unwrap(), + model + ); + + Ok(()) +} + pub fn insert_value_postgres(db: &DatabaseConnection) -> Result<(), DbErr> { let model = value_type_pg::Model { id: 1, @@ -166,3 +210,47 @@ pub fn conversion_test() { .expect_err("should not be ok to convert char to stringvec"); assert_eq!(try_from_string_vec.to_string(), ValueTypeErr.to_string()); } + +/// Asserts that the `PkAutoIncrementHint` trait drives the default for +/// `PrimaryKeyTrait::auto_increment()`. `DeriveValueType` emits a +/// delegating impl on the wrapper that resolves through the inner type, +/// so `MyInteger(i32)` → `true` (via the `i32` impl) and `Token(String)` +/// → `false` (via the `String` impl) without any explicit annotation on +/// the entity. +/// +/// Combined with the delegating `TryFromU64` impl, this lets `Uuid`, +/// `String`, and integer newtype PKs all work end-to-end. +pub fn auto_increment_test() { + use sea_orm::PrimaryKeyTrait; + + // MyInteger(i32), DeriveValueType propagates PkAutoIncrementHint + // through the inner i32 → defaults to true. + assert!( + ::auto_increment(), + "MyInteger(i32) newtype PK should resolve to auto_increment = true" + ); + + // Token(String), same propagation, but inner is String → false. + // No explicit annotation on the entity is required. + assert!( + !::auto_increment(), + "Token(String) PK should resolve to auto_increment = false via PkAutoIncrementHint" + ); + + // `Uuid::try_from_u64` returns Err, confirm the newtype delegates and + // surfaces the same error variant (not a `TryFromIntError`). + #[cfg(feature = "with-uuid")] + { + use common::features::value_type::UuidPk; + use sea_orm::TryFromU64; + let err = UuidPk::try_from_u64(1).unwrap_err(); + assert!(matches!(err, DbErr::ConvertFromU64(_))); + } + + // `String::try_from_u64` returns Ok("n"), confirm the newtype delegates. + { + use sea_orm::TryFromU64; + let token = Token::try_from_u64(42).unwrap(); + assert_eq!(token, Token("42".to_string())); + } +} diff --git a/src/entity/auto_increment_hint.rs b/src/entity/auto_increment_hint.rs new file mode 100644 index 0000000000..cd6b62a324 --- /dev/null +++ b/src/entity/auto_increment_hint.rs @@ -0,0 +1,129 @@ +//! Per-type hint for whether a primary key column defaults to +//! `AUTO_INCREMENT` when the entity declaration doesn't say explicitly. +//! +//! `DeriveEntityModel` consults this trait at trait-resolution time. +//! Integer primitives resolve to `true`; `String`, `Vec` and `Uuid` +//! to `false`. Wrapper types and `Id` aliases resolve through their +//! inner type because each emits a delegating impl of +//! [`DelegatesPkAutoIncrementHint`] (e.g. `Id` -> `true`). +//! +//! For custom PK types outside these categories, either impl this trait +//! on the type or specify `auto_increment` explicitly on the column. + +use crate::EntityTrait; + +/// Default `auto_increment` value used by the entity macro when a +/// primary key column does not specify `#[sea_orm(auto_increment = ...)]` +/// explicitly. +#[diagnostic::on_unimplemented( + message = "`{Self}` cannot be used as a primary key without an explicit `auto_increment` setting", + note = "the entity macro looks up the default via `sea_orm::PkAutoIncrementHint`", + note = "either impl `sea_orm::PkAutoIncrementHint` for `{Self}`, or specify \ + `#[sea_orm(primary_key, auto_increment = false)]` (or `= true`) on the column" +)] +pub trait PkAutoIncrementHint { + /// Whether columns of this type default to `AUTO_INCREMENT` when used + /// as a primary key. + const IS_AUTO: bool; +} + +macro_rules! impl_auto_true { + ($($t:ty),* $(,)?) => { + $( + impl PkAutoIncrementHint for $t { + const IS_AUTO: bool = true; + } + )* + }; +} + +macro_rules! impl_auto_false { + ($($t:ty),* $(,)?) => { + $( + impl PkAutoIncrementHint for $t { + const IS_AUTO: bool = false; + } + )* + }; +} + +impl_auto_true!(i8, i16, i32, i64, u8, u16, u32, u64, isize, usize); +impl_auto_false!(String, Vec); + +#[cfg(feature = "with-uuid")] +impl PkAutoIncrementHint for uuid::Uuid { + const IS_AUTO: bool = false; +} + +#[cfg(feature = "with-uuid")] +mod uuid_fmt_impls { + use super::PkAutoIncrementHint; + use uuid::fmt; + + impl PkAutoIncrementHint for fmt::Braced { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Hyphenated { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Simple { + const IS_AUTO: bool = false; + } + impl PkAutoIncrementHint for fmt::Urn { + const IS_AUTO: bool = false; + } +} + +/// Internal helper trait: marks a wrapper as delegating its +/// `PkAutoIncrementHint` resolution to an inner type. +/// +/// `DeriveValueType` emits an impl of this for every wrapper it generates. +/// The blanket `PkAutoIncrementHint` impl below bridges from it, deferring +/// the inner-type bound to trait-resolution time rather than a concrete +/// `where` clause that would force a compile error on every wrapper whose +/// inner isn't a PK hint, even when the wrapper is never used as a PK. +pub trait DelegatesPkAutoIncrementHint { + /// The inner type whose `PkAutoIncrementHint` impl is delegated to. + type Inner: ?Sized; +} + +impl PkAutoIncrementHint for T +where + T: DelegatesPkAutoIncrementHint, + T::Inner: PkAutoIncrementHint, +{ + const IS_AUTO: bool = ::IS_AUTO; +} + +/// `Id` delegates to its inner `T`. +impl DelegatesPkAutoIncrementHint for crate::Id +where + E: EntityTrait, +{ + type Inner = T; +} + +#[cfg(test)] +mod tests { + use super::PkAutoIncrementHint; + + #[test] + fn integer_primitives_default_true() { + assert!(::IS_AUTO); + assert!(::IS_AUTO); + assert!(::IS_AUTO); + assert!(::IS_AUTO); + } + + #[test] + fn string_and_bytes_default_false() { + assert!(!::IS_AUTO); + assert!(! as PkAutoIncrementHint>::IS_AUTO); + } + + #[cfg(feature = "with-uuid")] + #[test] + fn uuid_defaults_false() { + assert!(!::IS_AUTO); + } +} diff --git a/src/entity/base_entity.rs b/src/entity/base_entity.rs index 110fa91bf3..d34e3e6aca 100644 --- a/src/entity/base_entity.rs +++ b/src/entity/base_entity.rs @@ -1,5 +1,5 @@ use crate::{ - ActiveModelBehavior, ActiveModelTrait, ColumnTrait, Delete, DeleteMany, DeleteOne, + ActiveModelBehavior, ActiveModelTrait, ColumnTrait, Delete, DeleteMany, DeleteOne, FindByIdArg, FromQueryResult, Identity, Insert, InsertMany, ModelTrait, PrimaryKeyArity, PrimaryKeyToColumn, PrimaryKeyTrait, QueryFilter, Related, RelationBuilder, RelationTrait, RelationType, Select, Update, UpdateMany, UpdateOne, ValidatedDeleteOne, @@ -202,7 +202,39 @@ pub trait EntityTrait: EntityName { Select::::new().join_join(JoinType::InnerJoin, R::to(), R::via()) } - /// Find a model by primary key + /// Find a model by primary key. + /// + /// `values` must satisfy [`FindByIdArg`], which is implemented + /// blanket for any `T: Into<::ValueType>`. + /// In practice that means: + /// + /// - For an entity with a raw scalar PK (`pub id: i32`), pass any + /// `T: Into`, including `i32`, `u8`, `&i32`, etc. + /// - For an entity with a typed PK newtype (`pub id: UserId` where + /// `UserId` is a `DeriveValueType` wrapper or `sea_orm::Id` + /// alias), pass the newtype itself: `find_by_id(UserId::new(7))`. + /// Raw scalars are rejected, there is no `From for UserId` + /// to satisfy the `Into` bound. + /// - For composite PKs, pass a tuple of the component types + /// (e.g. `(i32, String)` or `(UserId, RoleId)`). + /// + /// # Type-safe PKs + /// + /// Wrap each entity's primary key in a per-entity newtype to get + /// compile-time protection against passing the wrong id type: + /// + /// ```ignore + /// use sea_orm::entity::prelude::*; + /// + /// #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + /// pub struct UserId(pub i32); + /// + /// // Now `user::Entity::find_by_id` accepts only `UserId`, not raw `i32` + /// // and not any other entity's id type. + /// ``` + /// + /// Do **not** add `impl From for UserId`, that re-opens the door to + /// `find_by_id(1)` and defeats the safety contract. /// /// # Example /// @@ -291,11 +323,11 @@ pub trait EntityTrait: EntityName { /// ``` fn find_by_id(values: T) -> Select where - T: Into<::ValueType>, + T: FindByIdArg, { let mut select = Self::find(); let mut keys = Self::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); select = select.filter(col.eq(v)); @@ -1041,11 +1073,11 @@ pub trait EntityTrait: EntityName { /// ``` fn delete_by_id(values: T) -> ValidatedDeleteOne where - T: Into<::ValueType>, + T: FindByIdArg, { let mut am = Self::ActiveModel::default(); let mut keys = Self::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); am.set(col, v); @@ -1160,7 +1192,7 @@ mod tests { fn delete_by_id(value: T) where - T: Into<<::PrimaryKey as PrimaryKeyTrait>::ValueType>, + T: crate::FindByIdArg, { assert_eq!( hello::Entity::delete_by_id(value) diff --git a/src/entity/compound.rs b/src/entity/compound.rs index 5b07616f34..523e146085 100644 --- a/src/entity/compound.rs +++ b/src/entity/compound.rs @@ -1,8 +1,8 @@ #![allow(missing_docs)] -use super::{ColumnTrait, EntityTrait, PrimaryKeyToColumn, PrimaryKeyTrait}; +use super::{ColumnTrait, EntityTrait, PrimaryKeyToColumn}; use crate::{ - ConnectionTrait, DbErr, IntoSimpleExpr, ItemsAndPagesNumber, Iterable, ModelTrait, QueryFilter, - QueryOrder, + ConnectionTrait, DbErr, FindByIdArg, IntoSimpleExpr, ItemsAndPagesNumber, Iterable, ModelTrait, + QueryFilter, QueryOrder, }; use sea_query::{IntoValueTuple, Order, TableRef}; use std::marker::PhantomData; @@ -21,10 +21,10 @@ pub trait EntityLoaderTrait: QueryFilter + QueryOrder + Clone { /// Find a model by primary key fn filter_by_id(mut self, values: T) -> Self where - T: Into<::ValueType>, + T: FindByIdArg, { let mut keys = E::PrimaryKey::iter(); - for v in values.into().into_value_tuple() { + for v in values.into_pk_value().into_value_tuple() { if let Some(key) = keys.next() { let col = key.into_column(); self.filter_mut(col.eq(v)); diff --git a/src/entity/id.rs b/src/entity/id.rs new file mode 100644 index 0000000000..902bf2d9c7 --- /dev/null +++ b/src/entity/id.rs @@ -0,0 +1,296 @@ +//! Phantom-typed primary-key handle. +//! +//! `Id` wraps a primary-key value of underlying type `T` and tags it +//! with the entity `E` at the type level. Two `Id` types with different +//! entity tags are never inter-convertible, so the compiler rejects +//! cross-entity ID confusion at use sites, e.g. passing a +//! `Id` to `user::Entity::find_by_id` is a type error. +//! +//! `Id` is reachable as `sea_orm::Id` and is intentionally absent from the +//! entity prelude: hand-written aliases spell it `sea_orm::Id`. +//! +//! ## Type parameters +//! +//! `T` is always the raw scalar, `Id::value: T`. Keeping the scalar +//! as an explicit type parameter (rather than projecting it through an +//! associated type on `E`) keeps `PrimaryKey::ValueType = Id` from +//! becoming infinitely recursive (`Id::Inner = Id::Inner = …`), +//! since the alias spells `T` directly: +//! `pub type CakePk = sea_orm::Id;`. +//! +//! ## Usage +//! +//! ```ignore +//! use sea_orm::entity::prelude::*; +//! +//! // Codegen emits this as a one-line alias per entity: +//! pub type CakePk = sea_orm::Id; +//! +//! // The model field uses the alias: +//! pub struct Model { +//! pub id: CakePk, +//! pub name: String, +//! } +//! +//! // Construction is explicit, `Id::new` (no `From` blanket): +//! let id = CakePk::new(7); +//! +//! // Queries use the typed handle: +//! let cake = cake::Entity::find_by_id(id).one(db).await?; +//! ``` +//! +//! ## Safety contract +//! +//! `Id` deliberately does NOT impl `From` for any specific scalar. +//! The only construction path is [`Id::new`]. This is what makes +//! `user::Entity::find_by_id(7_i32)` fail to compile when the entity's PK +//! is `Id`: there's no `i32: Into>` +//! impl. Do not add such a `From` impl; it re-opens the cross-entity hole +//! this type exists to close. + +use crate::{ + ColIdx, DbErr, EntityTrait, PrimaryKeyTrait, QueryResult, TryFromU64, TryGetError, TryGetable, +}; +use sea_query::{ArrayType, ColumnType, Nullable, Value, ValueType, ValueTypeErr}; +use std::fmt; +use std::hash::{Hash, Hasher}; +use std::marker::PhantomData; + +/// Phantom-typed wrapper around a primary-key value. +/// +/// `E` is the entity this id belongs to (a marker, never stored at +/// runtime). `T` is the raw stored value: +/// - For unary PKs, the scalar type (`i32`, `Uuid`, `String`, …). +/// - For composite PKs, a tuple of the typed components +/// (e.g. `(super::cake::CakePk, super::filling::FillingPk)`). +/// +/// See the [module-level docs](self) for usage and the safety contract. +#[repr(transparent)] +pub struct Id { + /// The raw stored value. Public for ergonomic read/unwrap; the + /// no-`From` contract blocks implicit call-site conversion, not field + /// access (the entity tag lives in `_marker`, not here, so reading or + /// mutating `value` cannot turn an `Id` into an `Id`). + pub value: T, + // `PhantomData E>` makes `E` invariant (E appears in both + // parameter and return position), so the compiler never widens an + // `Id` to an `Id`. Function-pointer types are always Send + + // Sync, so this marker preserves those auto-traits on `Id`. + _marker: PhantomData E>, +} + +impl Id { + /// Wrap a raw value as a typed entity ID. This is the only construction + /// path, there is no `From` blanket impl, which is what gives + /// `Id` its type-safety contract. + pub const fn new(value: T) -> Self { + Self { + value, + _marker: PhantomData, + } + } + + /// Unwrap to the raw stored value, consuming the wrapper. + pub fn into_inner(self) -> T { + self.value + } +} + +// Manual impls so the trait bounds land on `T` (the stored value) rather +// than `E` (a phantom). + +impl Clone for Id { + fn clone(&self) -> Self { + Self::new(self.value.clone()) + } +} + +impl Copy for Id {} + +impl fmt::Debug for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Include the entity tag so `Id(7)` and + // `Id(7)` don't look identical in logs, that + // defeats the entire reason this wrapper exists. + // + // Every entity struct is named `Entity` by convention, so the + // disambiguating part is the module that contains it. We render + // `::`, the last two `::`-segments + // of `std::any::type_name::()`. Full paths are too verbose + // for log lines; the trailing two segments preserve the + // disambiguation while staying readable. + let full = std::any::type_name::(); + let mut tail = full.rsplitn(3, "::"); + let last = tail.next().unwrap_or(full); + let prev = tail.next(); + let label = match prev { + Some(p) => format!("{p}::{last}"), + None => last.to_owned(), + }; + write!(f, "Id<{label}>({:?})", self.value) + } +} + +impl PartialEq for Id { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +impl Eq for Id {} + +impl Hash for Id { + fn hash(&self, state: &mut H) { + self.value.hash(state); + } +} + +impl fmt::Display for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.value.fmt(f) + } +} + +// === PrimaryKeyTrait::ValueType bounds ====================================== +// +// All five trait bounds below delegate to `T`. `Into` (unary `T`) +// bridges to sea-query's blanket `From for ValueTuple`, auto-deriving +// `Id: IntoValueTuple`. Composite PKs never use `Id`: each +// component is a unary `Id` and the tuple is just +// `(CakePk, FillingPk)`. + +impl From> for Value +where + T: Into, +{ + fn from(id: Id) -> Self { + id.value.into() + } +} + +// `FromValueTuple` comes from sea-query's blanket impl once `Into` +// (above) and `ValueType` (below) are present; for composite `T` neither +// fires, which is intentional. + +// `TryGetable` (single-column read) also auto-derives `TryGetableMany` for +// `Id` and, via the per-arity macro, for tuples of `Id` (what +// composite PKs need). `Id` does not impl `TryGetable` for tuple `T`: +// composite PKs use tuples of unary `Id`, not `Id`. +impl TryGetable for Id { + fn try_get_by(res: &QueryResult, idx: I) -> Result { + T::try_get_by(res, idx).map(Id::new) + } +} + +impl TryFromU64 for Id { + fn try_from_u64(n: u64) -> Result { + T::try_from_u64(n).map(Id::new) + } +} + +// `PrimaryKeyArity` is auto-derived via the blanket +// `impl PrimaryKeyArity for V { const ARITY = 1 }`. + +// `sea_query::ValueType` lets `DeriveEntityModel` call +// `::column_type()` for the SQL column type. Only when +// `T: ValueType` (a single scalar); composite PKs ask each column instead. +impl ValueType for Id { + fn try_from(v: Value) -> Result { + T::try_from(v).map(Id::new) + } + + fn type_name() -> String { + T::type_name() + } + + fn array_type() -> ArrayType { + T::array_type() + } + + fn column_type() -> ColumnType { + T::column_type() + } +} + +// `Nullable` so the macro can wrap the column in `Option>` for +// nullable FK columns. +impl Nullable for Id { + fn null() -> Value { + T::null() + } +} + +// === FindByIdArg ============================================================ +// +// `find_by_id` / `filter_by_id` accept anything convertible to the entity's +// primary-key value type. We could bound that directly with `Into`, but doing +// so makes the compiler's "this argument is wrong" diagnostic +// incomprehensible: it reads something like +// `the trait bound `Id: From>` +// is not satisfied`, +// burying the two entity types inside generic args of `Into`. +// +// `FindByIdArg` is a thin sea-orm-owned wrapper around that same `Into` +// bound. It exists solely so we can attach `#[diagnostic::on_unimplemented]` +// to it, diagnostics can't be attached to `Into` (a std trait). The blanket +// impl forwards through `Into`, so every existing call site still works +// without change. When the bound *fails*, the user sees a message that names +// the entity and the argument type directly. +// +// MSRV is 1.85; `#[diagnostic::on_unimplemented]` is stable since 1.78. + +/// Helper bound used by `find_by_id` / `filter_by_id`. +/// +/// Implemented for every `T` that converts into `E`'s primary-key value type +/// via `Into`. This trait exists to provide a better compiler error than the +/// raw `Into` bound when the argument doesn't match, see the module docs. +#[diagnostic::on_unimplemented( + message = "`{Self}` cannot be used as a primary-key argument for `{E}`", + label = "expected `{E}`'s `PrimaryKey::ValueType` (or something convertible to it), got `{Self}`", + note = "type-safe `Id` wrappers deliberately do not impl `From` to prevent cross-entity ID confusion. Construct ids explicitly with `Id::new(..)` (or the per-entity alias's `::new`), and pass an id belonging to this entity." +)] +pub trait FindByIdArg: Sized { + /// Convert this argument into the entity's primary-key value tuple. + fn into_pk_value(self) -> ::ValueType; +} + +// `do_not_recommend` (stable 1.85) tells rustc not to surface this blanket impl +// in error messages when its where-clause fails. Without it, the user sees a +// confusing message about `From>` not being implemented +// for `Id`, the deeper sub-bound, instead of the +// `on_unimplemented` message on `FindByIdArg` itself. +#[diagnostic::do_not_recommend] +impl FindByIdArg for T +where + T: Into<::ValueType>, +{ + fn into_pk_value(self) -> ::ValueType { + self.into() + } +} + +// === Serde ================================================================== +// +// Transparent: `Id` serializes as just the inner `T`, not as a +// wrapper object. Gated behind `with-json` like the rest of sea-orm's serde +// surface (see `entity/compound/has_one.rs` for the same pattern). + +#[cfg(feature = "with-json")] +impl serde::Serialize for Id { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.value.serialize(serializer) + } +} + +#[cfg(feature = "with-json")] +impl<'de, E: EntityTrait, T: serde::Deserialize<'de>> serde::Deserialize<'de> for Id { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + T::deserialize(deserializer).map(Id::new) + } +} diff --git a/src/entity/mod.rs b/src/entity/mod.rs index b0216be3ac..1f009569a1 100644 --- a/src/entity/mod.rs +++ b/src/entity/mod.rs @@ -102,10 +102,12 @@ mod active_model_ex; mod active_value; #[cfg(feature = "with-arrow")] mod arrow_schema; +mod auto_increment_hint; mod base_entity; pub(crate) mod column; mod column_def; pub mod compound; +mod id; mod identity; mod link; mod model; @@ -125,10 +127,12 @@ pub use active_model_ex::*; pub use active_value::*; #[cfg(feature = "with-arrow")] pub use arrow_schema::*; +pub use auto_increment_hint::*; pub use base_entity::*; pub use column::*; pub use column_def::*; pub use compound::EntityLoaderTrait; +pub use id::*; pub use identity::*; pub use link::*; pub use model::*; diff --git a/src/entity/primary_key.rs b/src/entity/primary_key.rs index 56897a1126..4add5a021b 100644 --- a/src/entity/primary_key.rs +++ b/src/entity/primary_key.rs @@ -48,7 +48,23 @@ pub trait PrimaryKeyTrait: IdenStatic + Iterable { + TryFromU64 + PrimaryKeyArity; - /// Method to call to perform `AUTOINCREMENT` operation on a Primary Key + /// Method to call to perform `AUTOINCREMENT` operation on a Primary Key. + /// + /// The `DeriveEntityModel` macro emits this from one of three sources, in order: + /// + /// 1. An explicit `#[sea_orm(auto_increment = true/false)]` on the primary + /// key column always wins. + /// 2. Composite primary keys (more than one `primary_key` column) always + /// return `false`. + /// 3. Otherwise the default is resolved at trait-resolution time via + /// [`crate::PkAutoIncrementHint`] on the column's type, integer + /// primitives default to `true`, `String` / `Uuid` / `Vec` default + /// to `false`, and `DeriveValueType` wrappers delegate to their inner + /// type (so `RoleId(pub i64)` → `true`, `Token(pub String)` → `false`). + /// + /// For custom primary key types that do not impl `PkAutoIncrementHint`, + /// provide an explicit `auto_increment = ...` on the column or impl the + /// trait on the type. fn auto_increment() -> bool; } diff --git a/tests/auto_increment_hint_tests.rs b/tests/auto_increment_hint_tests.rs new file mode 100644 index 0000000000..90b73e2563 --- /dev/null +++ b/tests/auto_increment_hint_tests.rs @@ -0,0 +1,155 @@ +//! Positive tests for `PkAutoIncrementHint` resolution. +//! +//! These pin the contract that `DeriveEntityModel` emits the trait call +//! correctly and that the trait propagates through `DeriveValueType` +//! wrappers and `Id` aliases. + +use sea_orm::{DeriveValueType, Id, PkAutoIncrementHint, PrimaryKeyTrait, entity::prelude::*}; + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct IntegerWrapper(pub i64); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct StringWrapper(pub String); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct NestedIntegerWrapper(pub IntegerWrapper); + +#[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] +pub struct NestedStringWrapper(pub StringWrapper); + +mod ent_for_id { + use super::*; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_id")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: i32, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity whose PK is an `Id` alias (the shape +/// `sea-orm-cli generate --with-pk-newtypes` produces). Exercises +/// trait resolution one layer up from raw scalars: the macro emits +/// `::IS_AUTO`, which resolves via the +/// `DelegatesPkAutoIncrementHint` blanket on `Id` down to the +/// inner `i64`. +mod ent_for_typed_pk { + use sea_orm::entity::prelude::*; + pub type EntId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_typed_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: EntId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity with a composite PK whose components are themselves typed +/// `Id` aliases. The macro must short-circuit to `false` +/// regardless of how the trait would resolve for the individual +/// component types. +mod ent_with_composite_pk { + use sea_orm::entity::prelude::*; + pub type LeftId = sea_orm::Id; + pub type RightId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_with_composite_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub left_id: LeftId, + #[sea_orm(primary_key)] + pub right_id: RightId, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +/// Entity whose PK is a bare `type X = i32` alias (not an `Id`). +/// A transparent alias resolves identically to its target, so the macro +/// emits `::IS_AUTO`, which is the +/// `i32` impl and yields `true`. +mod ent_for_bare_alias { + use super::*; + pub type BareUserId = i32; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "ent_for_bare_alias")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: BareUserId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +#[test] +fn primitive_integer_defaults_true() { + assert!(::IS_AUTO); + assert!(::IS_AUTO); +} + +#[test] +fn primitive_string_defaults_false() { + assert!(!::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_integer() { + assert!(::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_string() { + assert!(!::IS_AUTO); +} + +#[test] +fn value_type_wrapper_propagates_through_nested() { + assert!(::IS_AUTO); + assert!(!::IS_AUTO); +} + +#[test] +fn id_alias_propagates_through_inner() { + type IntId = Id; + type StrId = Id; + assert!(::IS_AUTO); + assert!(!::IS_AUTO); +} + +#[test] +fn entity_with_i32_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn entity_with_id_alias_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn entity_with_bare_alias_i32_pk_resolves_true() { + assert!(::auto_increment()); +} + +#[test] +fn composite_pk_is_never_auto_increment() { + assert!(!::auto_increment()); +} + +#[cfg(feature = "with-uuid")] +#[test] +fn uuid_defaults_false() { + assert!(!::IS_AUTO); +} diff --git a/tests/common/features/schema.rs b/tests/common/features/schema.rs index 87c820bc56..2d036d05a1 100644 --- a/tests/common/features/schema.rs +++ b/tests/common/features/schema.rs @@ -777,7 +777,13 @@ pub async fn create_value_type_table(db: &DbConn) -> Result { .to_owned(); create_table(db, &general_stmt, value_type::value_type_general::Entity).await?; - create_table_from_entity(db, value_type::value_type_pk::Entity).await + create_table_from_entity(db, value_type::value_type_pk::Entity).await?; + create_table_from_entity(db, value_type::value_type_token_pk::Entity).await +} + +#[cfg(feature = "with-uuid")] +pub async fn create_value_type_uuid_pk_table(db: &DbConn) -> Result { + create_table_from_entity(db, value_type::value_type_uuid_pk::Entity).await } pub async fn create_value_type_postgres_table(db: &DbConn) -> Result { diff --git a/tests/common/features/value_type.rs b/tests/common/features/value_type.rs index a58dbc283b..b7705624bb 100644 --- a/tests/common/features/value_type.rs +++ b/tests/common/features/value_type.rs @@ -55,6 +55,45 @@ pub mod value_type_pk { impl ActiveModelBehavior for ActiveModel {} } +/// String-backed newtype PK, exercises `PkAutoIncrementHint` resolving +/// through `DeriveValueType` to `false`. +pub mod value_type_token_pk { + use super::*; + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "value_type_token_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: Token, + pub note: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +#[cfg(feature = "with-uuid")] +pub mod value_type_uuid_pk { + use super::*; + use sea_orm::entity::prelude::*; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "value_type_uuid_pk")] + pub struct Model { + #[sea_orm(primary_key)] + pub id: UuidPk, + pub note: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + use sea_orm::entity::prelude::*; #[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] @@ -69,6 +108,13 @@ where } } +#[derive(Clone, Debug, PartialEq, Eq, Hash, DeriveValueType)] +pub struct Token(pub String); + +#[cfg(feature = "with-uuid")] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] +pub struct UuidPk(pub uuid::Uuid); + #[derive(Clone, Debug, PartialEq, Eq, DeriveValueType)] pub struct StringVec(pub Vec); diff --git a/tests/id_typed_pk_tests.rs b/tests/id_typed_pk_tests.rs new file mode 100644 index 0000000000..03b32bdeed --- /dev/null +++ b/tests/id_typed_pk_tests.rs @@ -0,0 +1,165 @@ +//! Runtime behaviour tests for the `Id` primary-key newtype. +//! +//! These pin the behaviour the type-safety contract relies on: +//! - Layout: `#[repr(transparent)]` holds, so `Id` and +//! `Option>` are the same size as the raw scalar. +//! - Hashability: typed PKs of different entities coexist in distinct +//! `HashMap`s keyed by their newtype. +//! - Display/Debug: `Display` delegates to the inner value; `Debug` +//! includes the entity tag so different entities render distinctly. +//! - `into_inner` round-trips back to the raw scalar. +//! - Serde shape: `Id` is transparent (a bare number/string, not an +//! object), including `Option>` and a `String`-typed alias. +//! +//! Cross-entity confusion (e.g. comparing or passing the wrong entity's id) +//! is a compile error, pinned separately by the trybuild fixtures under +//! `tests/value_type_pk_compile_fail/`. +//! +//! Two minimal local entities (`post`, `user`) keep this file self-contained. + +use std::collections::HashMap; +use std::mem::size_of; + +mod post { + use sea_orm::entity::prelude::*; + + pub type PostId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +#[test] +fn layout_is_repr_transparent() { + assert_eq!(size_of::(), size_of::()); + assert_eq!(size_of::(), size_of::()); + assert_eq!(size_of::>(), size_of::>()); +} + +#[test] +fn typed_ids_are_hashable_in_separate_maps() { + let mut posts: HashMap = HashMap::new(); + let mut users: HashMap = HashMap::new(); + posts.insert(post::PostId::new(7), "post-7"); + users.insert(user::UserId::new(7), "user-7"); + assert_eq!(posts.get(&post::PostId::new(7)), Some(&"post-7")); + assert_eq!(users.get(&user::UserId::new(7)), Some(&"user-7")); + // Same inner value, different keyspace. + assert_eq!(posts.len(), 1); + assert_eq!(users.len(), 1); +} + +#[test] +fn copy_and_clone_only_when_inner_supports_them() { + // i32 is Copy → PostId is Copy. + let a = post::PostId::new(3); + let b = a; + assert_eq!(a, b); + let c = a.clone(); + assert_eq!(a, c); +} + +#[test] +fn into_inner_round_trip() { + let id = post::PostId::new(42); + assert_eq!(id.into_inner(), 42i32); +} + +#[test] +fn display_delegates_to_inner() { + let id = post::PostId::new(101); + assert_eq!(format!("{}", id), "101"); +} + +#[test] +fn debug_includes_entity_tag() { + // The Debug impl prints the entity tag so that + // `Id(7)` and `Id(7)` don't render + // identically in logs. The tag is the last two `::` segments of + // `type_name::()` to disambiguate entities that all conventionally + // name their inner struct `Entity`. + let post_id = post::PostId::new(7); + let user_id = user::UserId::new(7); + let post_dbg = format!("{post_id:?}"); + let user_dbg = format!("{user_id:?}"); + assert!(post_dbg.contains("post::Entity"), "got: {post_dbg}"); + assert!(user_dbg.contains("user::Entity"), "got: {user_dbg}"); + assert_ne!( + post_dbg, user_dbg, + "Debug output must distinguish post::Entity from user::Entity" + ); + assert!(post_dbg.ends_with("(7)")); +} + +#[cfg(feature = "with-json")] +mod serde_shape { + use super::*; + + #[test] + fn typed_id_serialises_as_bare_number() { + let id = post::PostId::new(7); + let v = serde_json::to_value(&id).expect("serialise"); + assert_eq!(v, serde_json::json!(7)); + } + + #[test] + fn typed_id_deserialises_from_bare_number() { + let v = serde_json::json!(13); + let id: post::PostId = serde_json::from_value(v).expect("deserialise"); + assert_eq!(id, post::PostId::new(13)); + } + + #[test] + fn option_typed_id_serialises_null_and_some() { + let none: Option = None; + assert_eq!( + serde_json::to_value(&none).unwrap(), + serde_json::Value::Null + ); + let some = Some(post::PostId::new(5)); + assert_eq!(serde_json::to_value(&some).unwrap(), serde_json::json!(5)); + } +} + +/// `Id` accepts a `String` payload and survives a serde +/// round-trip. The phantom entity here is `user::Entity`, only the +/// inner-`T` behaviour is under test. +#[cfg(feature = "with-json")] +#[test] +fn string_typed_id_round_trips_through_serde() { + type StringPk = sea_orm::Id; + let id: StringPk = sea_orm::Id::new("abc-xyz".to_string()); + let v = serde_json::to_value(&id).unwrap(); + assert_eq!(v, serde_json::json!("abc-xyz")); + let back: StringPk = serde_json::from_value(v).unwrap(); + assert_eq!(back, sea_orm::Id::new("abc-xyz".to_string())); +} diff --git a/tests/value_type_pk_compile_fail/cross_entity_eq.rs b/tests/value_type_pk_compile_fail/cross_entity_eq.rs new file mode 100644 index 0000000000..1070c20a71 --- /dev/null +++ b/tests/value_type_pk_compile_fail/cross_entity_eq.rs @@ -0,0 +1,41 @@ +//! Comparing two `Id` of DIFFERENT entities must not compile, +//! even when the inner scalar type matches. `PartialEq` is impl'd as +//! `impl PartialEq for Id` (same E), so `Id` +//! and `Id` have no shared `PartialEq` impl. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + pub type UserId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +mod post { + use sea_orm::entity::prelude::*; + pub type PostId = sea_orm::Id; + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Direct equality across entities, must not compile. + let _ = user::UserId::new(7) == post::PostId::new(7); +} diff --git a/tests/value_type_pk_compile_fail/raw_int_rejected.rs b/tests/value_type_pk_compile_fail/raw_int_rejected.rs new file mode 100644 index 0000000000..124abdfa3a --- /dev/null +++ b/tests/value_type_pk_compile_fail/raw_int_rejected.rs @@ -0,0 +1,29 @@ +//! Passing a raw `i32` to `find_by_id` must not compile when the PK is +//! `Id`. `Id` deliberately does not impl +//! `From`, so `i32: !Into>`. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Raw `i32` not convertible into `Id`. + let _ = user::Entity::find_by_id(1i32); +} diff --git a/tests/value_type_pk_compile_fail/self_ref_role_swap.rs b/tests/value_type_pk_compile_fail/self_ref_role_swap.rs new file mode 100644 index 0000000000..85fb213806 --- /dev/null +++ b/tests/value_type_pk_compile_fail/self_ref_role_swap.rs @@ -0,0 +1,71 @@ +//! Self-ref junction tables (where 2+ FK columns point at the same parent) +//! get per-column "role wrapper" structs around the parent's PK alias. +//! This makes positional swaps (passing one role where the other is +//! expected) fail to compile. + +use sea_orm::entity::prelude::*; +use sea_orm::ActiveValue::Set; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod user_follower { + use sea_orm::entity::prelude::*; + + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + #[sea_orm(try_from_u64)] + pub struct UserFollowerPkUserId(pub super::user::UserId); + + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, DeriveValueType)] + #[sea_orm(try_from_u64)] + pub struct UserFollowerPkFollowerId(pub super::user::UserId); + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user_follower")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub user_id: UserFollowerPkUserId, + #[sea_orm(primary_key, auto_increment = false)] + pub follower_id: UserFollowerPkFollowerId, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + let user = user::Model { + id: user::UserId::new(1), + name: "alice".into(), + }; + let follower = user::Model { + id: user::UserId::new(2), + name: "bob".into(), + }; + + // Deliberately swapped roles. The role wrappers make these distinct + // types: `UserFollowerPkUserId` and `UserFollowerPkFollowerId` are not + // inter-convertible, so passing one where the other is expected fails. + let _ = user_follower::ActiveModel { + user_id: Set(user_follower::UserFollowerPkFollowerId(follower.id)), // wrong slot + follower_id: Set(user_follower::UserFollowerPkUserId(user.id)), // wrong slot + }; +} diff --git a/tests/value_type_pk_compile_fail/wrong_id_type.rs b/tests/value_type_pk_compile_fail/wrong_id_type.rs new file mode 100644 index 0000000000..45ec447c85 --- /dev/null +++ b/tests/value_type_pk_compile_fail/wrong_id_type.rs @@ -0,0 +1,48 @@ +//! Passing a `PostId` to `user::Entity::find_by_id` must not compile. +//! `Id` and `Id` are type-distinct +//! despite having the same inner scalar. + +use sea_orm::entity::prelude::*; + +mod user { + use sea_orm::entity::prelude::*; + + pub type UserId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "user")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: UserId, + pub name: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +mod post { + use sea_orm::entity::prelude::*; + + pub type PostId = sea_orm::Id; + + #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] + #[sea_orm(table_name = "post")] + pub struct Model { + #[sea_orm(primary_key, auto_increment = false)] + pub id: PostId, + pub title: String, + } + + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] + pub enum Relation {} + + impl ActiveModelBehavior for ActiveModel {} +} + +fn main() { + // Cross-entity ID confusion, must not compile. + let _ = user::Entity::find_by_id(post::PostId::new(1)); +} diff --git a/tests/value_type_pk_safety_tests.rs b/tests/value_type_pk_safety_tests.rs new file mode 100644 index 0000000000..4f8d13d763 --- /dev/null +++ b/tests/value_type_pk_safety_tests.rs @@ -0,0 +1,19 @@ +//! Compile-fail harness for `Id`-wrapped primary keys. +//! +//! Each fixture under `tests/value_type_pk_compile_fail/` must fail to +//! compile. This harness asserts only the must-fail invariant; the captured +//! stderr is regenerated on every run via `TRYBUILD=overwrite` and the +//! `.stderr` files are gitignored, so the exact compiler wording is not +//! pinned (it varies across rustc releases). + +#[test] +fn pk_safety() { + // SAFETY: env vars touch global state visible to other threads, but + // this test runs synchronously and the only consumer of `TRYBUILD` + // is trybuild itself, invoked on the line below. + unsafe { + std::env::set_var("TRYBUILD", "overwrite"); + } + let t = trybuild::TestCases::new(); + t.compile_fail("tests/value_type_pk_compile_fail/*.rs"); +} diff --git a/tests/value_type_tests.rs b/tests/value_type_tests.rs index c26f856311..3ab3b0f570 100644 --- a/tests/value_type_tests.rs +++ b/tests/value_type_tests.rs @@ -9,8 +9,8 @@ pub use common::{ TestContext, features::{ value_type::{ - MyInteger, StringVec, Tag1, Tag2, Tag3, Tag4, Tag5, value_type_general, value_type_pg, - value_type_pk, + MyInteger, StringVec, Tag1, Tag2, Tag3, Tag4, Tag5, Token, value_type_general, + value_type_pg, value_type_pk, value_type_token_pk, }, *, }, @@ -27,12 +27,20 @@ use sea_query::{ArrayType, ColumnType, PostgresQueryBuilder, Value, ValueType, V async fn main() -> Result<(), DbErr> { type_test(); conversion_test(); + auto_increment_test(); let ctx = TestContext::new("value_type_tests").await; create_value_type_table(&ctx.db).await?; insert_value_general(&ctx.db).await?; insert_value_pk(&ctx.db).await?; + insert_value_token_pk(&ctx.db).await?; + + #[cfg(feature = "with-uuid")] + { + create_value_type_uuid_pk_table(&ctx.db).await?; + insert_value_uuid_pk(&ctx.db).await?; + } if cfg!(feature = "sqlx-postgres") { create_value_type_postgres_table(&ctx.db).await?; @@ -80,6 +88,44 @@ pub async fn insert_value_pk(db: &DatabaseConnection) -> Result<(), DbErr> { Ok(()) } +#[cfg(feature = "with-uuid")] +pub async fn insert_value_uuid_pk(db: &DatabaseConnection) -> Result<(), DbErr> { + use common::features::value_type::{UuidPk, value_type_uuid_pk}; + let the_uuid = uuid::Uuid::new_v4(); + let model = value_type_uuid_pk::Model { + id: UuidPk(the_uuid), + note: "uuid pk round-trip".to_string(), + }; + let result = model.clone().into_active_model().insert(db).await?; + assert_eq!(result, model); + + let fetched = value_type_uuid_pk::Entity::find_by_id(UuidPk(the_uuid)) + .one(db) + .await? + .expect("uuid pk row should be readable"); + assert_eq!(fetched, model); + Ok(()) +} + +pub async fn insert_value_token_pk(db: &DatabaseConnection) -> Result<(), DbErr> { + let model = value_type_token_pk::Model { + id: Token("abc-123".to_string()), + note: "non-integer PK newtype".to_string(), + }; + let result = model.clone().into_active_model().insert(db).await?; + assert_eq!(result, model); + + assert_eq!( + value_type_token_pk::Entity::find_by_id(Token("abc-123".to_string())) + .one(db) + .await? + .unwrap(), + model + ); + + Ok(()) +} + pub async fn insert_value_postgres(db: &DatabaseConnection) -> Result<(), DbErr> { let model = value_type_pg::Model { id: 1, @@ -167,3 +213,47 @@ pub fn conversion_test() { .expect_err("should not be ok to convert char to stringvec"); assert_eq!(try_from_string_vec.to_string(), ValueTypeErr.to_string()); } + +/// Asserts that the `PkAutoIncrementHint` trait drives the default for +/// `PrimaryKeyTrait::auto_increment()`. `DeriveValueType` emits a +/// delegating impl on the wrapper that resolves through the inner type, +/// so `MyInteger(i32)` → `true` (via the `i32` impl) and `Token(String)` +/// → `false` (via the `String` impl) without any explicit annotation on +/// the entity. +/// +/// Combined with the delegating `TryFromU64` impl, this lets `Uuid`, +/// `String`, and integer newtype PKs all work end-to-end. +pub fn auto_increment_test() { + use sea_orm::PrimaryKeyTrait; + + // MyInteger(i32), DeriveValueType propagates PkAutoIncrementHint + // through the inner i32 → defaults to true. + assert!( + ::auto_increment(), + "MyInteger(i32) newtype PK should resolve to auto_increment = true" + ); + + // Token(String), same propagation, but inner is String → false. + // No explicit annotation on the entity is required. + assert!( + !::auto_increment(), + "Token(String) PK should resolve to auto_increment = false via PkAutoIncrementHint" + ); + + // `Uuid::try_from_u64` returns Err, confirm the newtype delegates and + // surfaces the same error variant (not a `TryFromIntError`). + #[cfg(feature = "with-uuid")] + { + use common::features::value_type::UuidPk; + use sea_orm::TryFromU64; + let err = UuidPk::try_from_u64(1).unwrap_err(); + assert!(matches!(err, DbErr::ConvertFromU64(_))); + } + + // `String::try_from_u64` returns Ok("n"), confirm the newtype delegates. + { + use sea_orm::TryFromU64; + let token = Token::try_from_u64(42).unwrap(); + assert_eq!(token, Token("42".to_string())); + } +}