More context for ActiveModelBehaviour

[mohs8421]

Motivation

When designing entities it is pretty common to write constraints into the datalayer. Sea-orm provides the ActiveModelBehaviour to allow users to fill in these pieces.
However, it is usually necessary to do some further analysis in these cases, like dropping some queries to check for further conditions to allow or deny a certain change. It might also be important to know the state of a record before the change that is going to be made, for example to implement a proper state transition, that might only be valid if some more data has been added to the record.

Proposed Solutions

It would be best if the ActiveModelBehaviour's signature would change to be like follows:

pub trait ActiveModelBehavior: ActiveModelTrait {
    /// Create a new ActiveModel with default values. Also used by `Default::default()`.
    fn new() -> Self {
        <Self as ActiveModelTrait>::default()
    }

    /// Will be called before saving
    // Note, that the former insert parameter is replaced by the option to the old_record, which fullfills the same purpose (and more), but is nevertheless a breaking change.
    fn before_save(self, old_record: Option<Model>, connection: &DatabaseConnection) -> Result<Self, DbErr> {
        Ok(self)
    }

    /// Will be called after saving
    fn after_save(
        model: <Self::Entity as EntityTrait>::Model,
        old_record: Option<Model>,
        connection: &DatabaseConnection,
    ) -> Result<<Self::Entity as EntityTrait>::Model, DbErr> {
        Ok(model)
    }

    /// Will be called before deleting
    fn before_delete(self, old_record: Option<Model>, connection: &DatabaseConnection) -> Result<Self, DbErr> {
        Ok(self)
    }

    /// Will be called after deleting
    fn after_delete(self, old_record: Option<Model>, connection: &DatabaseConnection) -> Result<Self, DbErr> {
        Ok(self)
    }
}

It might also be convenient to reduce the amount of parameters and pack them into a parameter-object struct like this:

pub struct SaveContext {
    old_record: …
    connection: …
}

[billy1624]

Hey @mohs8421, thanks for the feature request!

Passing &DatabaseConnection into *_save and *_delete is a straightforward and easy change.

However, providing the old record / model isn't a trivial task. Currently, we don't store old value in ActiveModel.

One approach would be adding a Changed variant to ActiveValue, which keeps a tuple representing the old and new value of a field in ActiveModel

• sea-orm/src/entity/active_model.rs

  Lines 38 to 49 in 10a215d

  	#[derive(Clone, Debug)]
  	pub enum ActiveValue<V>
  	where
  	V: Into<Value>,
  	{
  	/// A defined [Value] actively being set
  	Set(V),
  	/// A defined [Value] remain unchanged
  	Unchanged(V),
  	/// An undefined [Value]
  	NotSet,
  	}

[mohs8421]

That approach actually matches pretty good with a part of what I did before to get the "full" record. That means not only have the changed record I get from the client, but also the data, which is stored in the database:

/// When an ActiveModel is created, it does not necessarily know,
/// whether the values it contains are changed or not, comparing
/// it to the existing Model solves this problem.
pub fn align_changes<E, A>(model: E::Model, mut active_model: A) -> A
where
    E: EntityTrait,
    A: ActiveModelTrait<Entity = E>,
{
    for column in E::Column::iter() {
        let active_value = active_model.get(column);
        if let ActiveValue::Unchanged(value) = active_value {
            if model.get(column) != value {
                active_model.set(column, value);
            }
        }
    }
    // primary keys should never be updated here, so they have to be aligned from the existing model
    for primary in E::PrimaryKey::iter() {
        let col = primary.into_column();
        if model.get(col) != *active_model.get(col).as_ref() {
            active_model.set(col, model.get(col))
        }
    }
    active_model
}

It might even be possible to do something similar to this aligning method by actively providing a filter for the entity, how to get to that record (in case it is not as simple as in my model, where I always have a reliable primary key).

Anyway I think I like your suggestion.

[billy1624]

Hey @mohs8421, sorry, I'm not sure what the snippet above what to do.

Btw...

for column in E::Column::iter() {
    let active_value = active_model.get(column);
    if let ActiveValue::Unchanged(value) = active_value {
        if model.get(column) != value {
            active_model.set(column, value);
            // Shouldn't the line above be...?
            // active_model.set(column, model.get(column));
        }
    }
}

[mohs8421]

The snippet is intended to put the two pieces together.
Let's say I get an active model from my frontend, and match that with the model as it is in the database.

To give you some more context on that snippet: before this is called, I have this method:

pub(crate) async fn deserialize_active_model<T>(request: &mut Request) -> TideResult<T>
where
    T: ActiveModelTrait,
    <<T as ActiveModelTrait>::Entity as EntityTrait>::Model: IntoActiveModel<T>,
    for<'de> <<T as ActiveModelTrait>::Entity as EntityTrait>::Model: serde::de::DeserializeOwned,
{
    let result = match request.content_type() {
        Some(c) if c == mime::FORM => {
            let result: T = request
                .body_form::<<<T as ActiveModelTrait>::Entity as EntityTrait>::Model>()
                .await?
                .into_active_model();
            Ok(result)
        }
        Some(c) if c == mime::JSON => T::from_json(request.body_json::<serde_json::Value>().await?),
        _ => {
            return Err(Error::from_str(
                StatusCode::NotAcceptable,
                "unrecognized content type",
            ));
        }
    };
    result.map_err(|error| Error::new(StatusCode::BadRequest, error))
}

The important part there is, that I can only deserialize into a model, and not into an active model, because that one is generated somewhere inside of sea-orm and I cannot declare it to be Deserialize directly.

Here comes the previously stated function into the game, because it now looks at the model received from the database and compares that with the active_model, which has been translated from a model from the request.
So it only has to specify the correct enum type - which is set for the given active_model, because it should still have the same value, but not as unchanged.

Yes it is a hacky kind of glue code, but it does what it should. Nevertheless I would love to get rid of that at some point in the future though.

[billy1624]

Interesting! Will this be helpful? https://www.sea-ql.org/SeaORM/docs/basic-crud/insert/#set-activemodel-from-json-value

[mohs8421]

Please note the T::from_json(…) part, where I already use what you suggested.

[teenjuna]

Hey @billy1624

Passing &DatabaseConnection into *_save and *_delete is a straightforward and easy change.

What has to be done? Maybe I could help

[mohs8421]

@teenjuna take a look at https://github.com/SeaQL/sea-orm/blob/master/src/entity/active_model.rs#L281 as an example, there you would only need to pass C into the behavior and similar things would need to be done in the other corresponding methods, and probably all related tests need to be adapted. For this part of the change.