Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ActiveEnum field as composite primary key #1414

Merged
merged 3 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/entity/active_enum.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{ColumnDef, DbErr, Iterable, QueryResult, TryGetError, TryGetable};
use crate::{ColumnDef, DbErr, Iterable, QueryResult, TryFromU64, TryGetError, TryGetable};
use sea_query::{DynIden, Expr, Nullable, SimpleExpr, Value, ValueType};

/// A Rust representation of enum defined in database.
Expand Down Expand Up @@ -160,6 +160,17 @@ where
}
}

impl<T> TryFromU64 for T
where
T: ActiveEnum,
{
fn try_from_u64(_: u64) -> Result<Self, DbErr> {
Err(DbErr::ConvertFromU64(
"Fail to construct ActiveEnum from a u64, if your primary key consist of a ActiveEnum field, its auto increment should be set to false."
))
}
}

#[cfg(test)]
mod tests {
use crate as sea_orm;
Expand Down
2 changes: 2 additions & 0 deletions tests/common/features/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub mod satellite;
pub mod schema;
pub mod sea_orm_active_enums;
pub mod self_join;
pub mod teas;
pub mod transaction_log;
pub mod uuid_fmt;

Expand All @@ -36,5 +37,6 @@ pub use satellite::Entity as Satellite;
pub use schema::*;
pub use sea_orm_active_enums::*;
pub use self_join::Entity as SelfJoin;
pub use teas::Entity as Teas;
pub use transaction_log::Entity as TransactionLog;
pub use uuid_fmt::Entity as UuidFmt;
17 changes: 17 additions & 0 deletions tests/common/features/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub async fn create_tables(db: &DatabaseConnection) -> Result<(), DbErr> {
create_pi_table(db).await?;
create_uuid_fmt_table(db).await?;
create_edit_log_table(db).await?;
create_teas_table(db).await?;

if DbBackend::Postgres == db_backend {
create_collection_table(db).await?;
Expand Down Expand Up @@ -498,3 +499,19 @@ pub async fn create_edit_log_table(db: &DbConn) -> Result<ExecResult, DbErr> {

create_table(db, &stmt, EditLog).await
}

pub async fn create_teas_table(db: &DbConn) -> Result<ExecResult, DbErr> {
let create_table_stmt = sea_query::Table::create()
.table(teas::Entity.table_ref())
.col(
ColumnDef::new(teas::Column::Id)
.enumeration(TeaEnum, [TeaVariant::EverydayTea, TeaVariant::BreakfastTea])
.not_null()
.primary_key(),
)
.col(ColumnDef::new(teas::Column::Category).string_len(1))
.col(ColumnDef::new(teas::Column::Color).integer())
.to_owned();

create_table(db, &create_table_stmt, Teas).await
}
16 changes: 16 additions & 0 deletions tests/common/features/teas.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use super::sea_orm_active_enums::*;
use sea_orm::entity::prelude::*;

#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
#[sea_orm(table_name = "teas")]
pub struct Model {
#[sea_orm(primary_key, auto_increment = false)]
pub id: Tea,
pub category: Option<Category>,
pub color: Option<Color>,
}

#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
pub enum Relation {}

impl ActiveModelBehavior for ActiveModel {}
104 changes: 104 additions & 0 deletions tests/enum_primary_key_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
pub mod common;

pub use common::{features::*, setup::*, TestContext};
use pretty_assertions::assert_eq;
use sea_orm::{
entity::prelude::*,
entity::*,
sea_query::{BinOper, Expr},
ActiveEnum as ActiveEnumTrait, DatabaseConnection,
};

#[sea_orm_macros::test]
#[cfg(any(
feature = "sqlx-mysql",
feature = "sqlx-sqlite",
feature = "sqlx-postgres"
))]
async fn main() -> Result<(), DbErr> {
let ctx = TestContext::new("enum_primary_key_tests").await;
create_tables(&ctx.db).await?;
insert_teas(&ctx.db).await?;
ctx.delete().await;

Ok(())
}

pub async fn insert_teas(db: &DatabaseConnection) -> Result<(), DbErr> {
use teas::*;

let model = Model {
id: Tea::EverydayTea,
category: None,
color: None,
};

assert_eq!(
model,
ActiveModel {
id: Set(Tea::EverydayTea),
category: Set(None),
color: Set(None),
..Default::default()
}
.insert(db)
.await?
);
assert_eq!(model, Entity::find().one(db).await?.unwrap());
assert_eq!(
model,
Entity::find()
.filter(Column::Id.is_not_null())
.filter(Column::Category.is_null())
.filter(Column::Color.is_null())
.one(db)
.await?
.unwrap()
);

let _ = ActiveModel {
category: Set(Some(Category::Big)),
color: Set(Some(Color::Black)),
..model.into_active_model()
}
.save(db)
.await?;
Comment on lines +81 to +87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the intention of this bit, and I think it SHOULD fail to insert instead of passing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does an update instead of insert.

// The `..model.into_active_model()` expands into

let _ = ActiveModel {
        category: Set(Some(Category::Big)),
        color: Set(Some(Color::Black)),
        id: Unchanged( ... )
    }
    .save(db)
    .await?;

Copy link
Member

@tyt2y3 tyt2y3 Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh Right

Copy link
Member

@tyt2y3 tyt2y3 Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add an insert conflict test case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can insert a row with duplicate ID. But I cannot assert the exact error, since it's database dependent. 66cffd4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, is_err would do

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, good to go


let model = Entity::find().one(db).await?.unwrap();
assert_eq!(
model,
Model {
id: Tea::EverydayTea,
category: Some(Category::Big),
color: Some(Color::Black),
}
);
assert_eq!(
model,
Entity::find()
.filter(Column::Id.eq(Tea::EverydayTea))
.filter(Column::Category.eq(Category::Big))
.filter(Column::Color.eq(Color::Black))
.one(db)
.await?
.unwrap()
);
assert_eq!(
model,
Entity::find()
.filter(
Expr::col(Column::Id)
.binary(BinOper::In, Expr::tuple([Tea::EverydayTea.as_enum()]))
)
.one(db)
.await?
.unwrap()
);

let res = model.delete(db).await?;

assert_eq!(res.rows_affected, 1);
assert_eq!(Entity::find().one(db).await?, None);

Ok(())
}