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

Update with a variable set of fields using MaybeUnset #35

Open
kakserpom opened this issue May 29, 2024 · 5 comments
Open

Update with a variable set of fields using MaybeUnset #35

kakserpom opened this issue May 29, 2024 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kakserpom
Copy link

kakserpom commented May 29, 2024

Here's how I am doing it with the native driver:

use scylla::frame::value::MaybeUnset::{Set, Unset};
    scylla
        .query(
            "UPDATE places
            SET name = ?, phone = ?, email = ?, address = ?, google_maps_link = ?, short_description = ?, description = ?, tags = ?
            WHERE id = ? IF owner_telegram_id = ?",
            (
                name.map_or(Unset, Set),
                phone.map_or(Unset, Set),
                email.map_or(Unset, Set),
                address.map_or(Unset, Set),
                google_maps_link.map_or(Unset, Set),
                short_description.map_or(Unset, Set),
                description.map_or(Unset, Set),
                tags.map_or(Unset, |x| {
                    Set(x.split(',')
                        .map(|s| s.trim().to_string())
                        .collect::<Vec<String>>())
                }),
                id.clone(),
                telegram_id.clone(),
            ),
        )
        .await?;

Is there a Charybdis alternative to this?

@GoranBrkuljan
Copy link
Member

GoranBrkuljan commented May 29, 2024

In Charybdis we make use of partial_<model_name> to update partial fields. In our app we usually have partial model for each action e.g.

Lets say we have model user:

#[charybdis_model(
    table_name = users,
    partition_keys = [id],
    clustering_keys = [],
)]
#[derive(Serialize, Deserialize, Default, Clone)]
#[serde(rename_all = "camelCase")]
pub struct User {
    pub id: Uuid,
    pub username: Text,
    pub email: Text,
    pub password: Text,
    pub first_name: Text,
    pub last_name: Text,
    pub bio: Option<Text>,

    #[serde(default = "chrono::Utc::now")]
    pub created_at: Timestamp,

    #[serde(default = "chrono::Utc::now")]
    pub updated_at: Timestamp,
}

and we want to update bio field, we would create partial_user model that has primary key and fields that we want to update:

partial_user!(UpdateBioUser, id, bio, updated_at);

Now we have UpdateBioUser struct that has same functionality as native User struct but only with provided fields.

partial_user is automatically generated by charybdis_model macro.

If you use actix web you can define action like this:

#[put("/bio")]
pub async fn update_bio(data: RequestData, client_session: Session, mut user: web::Json<UpdateBioUser>) -> Response {
    user.auth_update(&data).await?;
     
    user.update_cb(&data).execute(data.db_session()).await?;
    // or if you don't use callbacks
    // user.update().execute(data.db_session().await?;

    Ok(HttpResponse::Ok().json(user))
}

so you can utilize partial model UpdateBioUser for both deserializing data from client and updating model fields.

Note that UpdateBioUser implements all derives and field attributes defined on native model. In general, partial model will implement all derives that are defined bellow charybdis_model.

@kakserpom
Copy link
Author

@GoranBrkuljan I am familiar with partial_<model_name> macro, but I cannot see how it qualifies as a solution, because as you can see in my code snippet, the set of fields is variable and unknown at the compile time.

@kakserpom
Copy link
Author

kakserpom commented May 29, 2024

Here is my full server function (Leptos with actix_web). If an argument is None, its corresponding column in Scylla remains untouched.

#[server(EditBusinessPlace, "/api")]
pub async fn edit_business_place(
    id: String,
    name: Option<String>,
    phone: Option<String>,
    email: Option<String>,
    address: Option<String>,
    google_maps_link: Option<String>,
    short_description: Option<String>,
    description: Option<String>,
    tags: Option<String>,
) -> Result<String, ServerFnError> {
    use actix_session::Session;
    use actix_web::web::Data;
    use leptos_actix::extract;
    let session: Session = extract().await.unwrap();
    let scylla: Data<scylla::SessionBuilder> = extract().await?;
    let scylla = scylla.build().await.unwrap();
    let telegram_id = crate::utils::extractors::user_telegram_id_or_fail().await?;
    use scylla::frame::value::MaybeUnset::{Set, Unset};
    scylla
        .query(
            "UPDATE business_places
            SET name = ?, phone = ?, email = ?, address = ?, google_maps_link = ?, short_description = ?, description = ?, tags = ?
            WHERE id = ? IF owner_telegram_id = ?",
            (
                name.map_or(Unset, |x| Set(x)),
                phone.map_or(Unset, |x| Set(x)),
                email.map_or(Unset, |x| Set(x)),
                address.map_or(Unset, |x| Set(x)),
                google_maps_link.map_or(Unset, |x| Set(x)),
                short_description.map_or(Unset, |x| Set(x)),
                description.map_or(Unset, |x| Set(x)),
                tags.map_or(Unset, |x| {
                    Set(x.split(',')
                        .map(|s| s.trim().to_string())
                        .collect::<Vec<String>>())
                }),
                id.clone(),
                telegram_id.clone(),
            ),
        )
        .await?;
    Ok(id)
}

@GoranBrkuljan
Copy link
Member

GoranBrkuljan commented May 29, 2024

Well, we don't support that ATM. In our platform we have endpoint for each set of fields that we want to update at once, and we know them in advance. That is why I recommended partial models. Here you would like to have single action for many possible updates on the same model without knowing in advance what set of fields are going to be updated. We could maybe introduce update_unset or something similar, but question remains on how would it be implemented as this requires fields to be defined as Option<T> and native model might be just pure T.

Other option is to create helper macro, like we have for find

find_user!("username = ?", (username,)).execute(db).await?;

So if implemented we could do:

update_user!(
    "first_name  = ?, last_name = ?",
    (
       first_name.map_or(Unset, |x| Set(x)),
       last_name.map_or(Unset, |x| Set(x)),
    )
)
.execute(db)
.await?;

Let me know if something like this would be helpful.

@kakserpom
Copy link
Author

kakserpom commented May 29, 2024

I guess a macro like that (update_user!("first_name = ?, last_name = ?",...)) would be a little better than nothing, but it won't change much.

A better approach in my view is implement a macro update_user!(UpdateUser); (similar to partial_<model_name>). It would look like this:

#[charybdis_model(
    table_name = users,
    partition_keys = [id],
    clustering_keys = [],
)]
#[derive(Serialize, Deserialize, Default, Clone)]
#[serde(rename_all = "camelCase")]
pub struct User {
    pub id: Uuid,
    pub username: Text,
    pub email: Text,
    pub password: Text,
    pub first_name: Text,
    pub last_name: Text,
    pub bio: Option<Text>,

    #[serde(default = "chrono::Utc::now")]
    pub created_at: Timestamp,

    #[serde(default = "chrono::Utc::now")]
    pub updated_at: Timestamp,
}

update_user!(UpdateUser);

#[put("/bio")]
pub async fn update_bio(data: RequestData, client_session: Session) -> Response {
   // @TODO: define id and bio with values from the request
    let user = UpdateUser {id, bio, ..Default::default()};
    user.auth_update(&data).await?;
    user.save().execute(data.db_session().await?;

    Ok(HttpResponse::Ok().json(user))
}

update_user!(UpdateUser); would wrap all non-PK fields into MaybeUnset<T> and default them to MaybeUnset::Unset. And cast all values into MaybeUnset::Set(x)

This would also be really cool if implemeted:
user.update_if("updated_at = ?", (updated_at)).execute(data.db_session().await?;

@GoranBrkuljan GoranBrkuljan added enhancement New feature or request help wanted Extra attention is needed labels Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants