thedodd / wither

An ODM for MongoDB built on the official MongoDB Rust driver.
https://docs.rs/wither
Other
325 stars 40 forks source link

_id is lost when fetching data from MongoDB and using serde flatten attribute #32

Closed brianwk closed 5 years ago

thedodd commented 5 years ago

@brianwk any additional details on this issue? Any way you can show me what the struct looks like which you are using?

Perhaps it is just an issue with structuring the attributes a bit differently.

Also, I’m pretty stoked because this is the first actual bug opened against this project.

brianwk commented 5 years ago

@thedodd This is my struct -- maybe it isn't a bug and I am just ignorant about something.

#[derive(Model, Clone, Serialize, Deserialize)]
pub struct Submission {
    #[serde(rename = "_id", skip_serializing_if = "Option::is_none", flatten)]
    pub id: Option<ObjectId>,
    pub email: String,
}

When I insert a new document using the save() method of the model, I correctly get a flattened document with $oid as the key and the new key as the value.

thedodd commented 5 years ago

Hmm ... so, what is the desired outcome of using the flatten attribute? If you remove it, then a new ObjectId will be generated for you by the DB. No flatten needed.

thedodd commented 5 years ago

The reason it does not work when deserializing is quite likely because the inner value, the “$oid” field will attempt to be flattened into the container struct, and such a field does not exists on the struct you showed.

brianwk commented 5 years ago

Thanks for the response @thedodd. I want to flatten it because when I serialize to JSON I don't want the ID to be nested in my API response. How do other developers typically handle this?

Also, I'm kind of new to Rust (and I don't have a strong C++ background), so any insight you can provide is definitely appreciated. This crate is surely going to make my life easier once I get a hang of the Rust way to do things I think.

brianwk commented 5 years ago

By the way, this is how I'm fetching the data in my Rocket handler:

#[get("/")]
fn index(db: PitchDb) -> Json<SubmissionCollection> {
    let submissions = Submission::find(db.clone(), None, None).unwrap();
    Json(SubmissionCollection { submissions })
}

Output (without serde flatten attribute):

{"submissions":[{"_id":{"$oid":"5caad06e65636334009055da"},"email":"test@example.com"}

Desired Output (I thought using serde flatten attribute would do this, but I guess not):

{"submissions":[{"id":"5caad06e65636334009055da","email":"test@example.com"}

At first I thought I had to make my own serializer and deserializer, but i quickly realized that is probably not the best approach because I would have to reimplement the serializer and deserializers that are already implemented in the bson crate.

thedodd commented 5 years ago

Ah, I see. So the difficulty is that serialization behavior needs to be specialized based on the output type. Hmm

I’ll do some experimentation with a few of the other serde field attrs to see if we can produce the desired behavior. However, in the meanwhile, you could use a separate struct for JSON output. I know it is repetitive, but until I can do some experimentation I don’t have a better solution off the top of my head.

You can use the From trait to setup a simple transformation. The new struct could just have the id field as an optional string. The From impl could map the option onto a closure which will call the OID to_hex method. It wouldn’t need any of the Wither attrs either.

thedodd commented 5 years ago

The concept of dividing your data models into public & private variants is definitely pretty pervasive (regardless of language). Per my comment above, the following is a pretty simple way to do the From pattern:

#[derive(Clone, Serialize, Deserialize)]
pub struct SubmissionJson {
    pub id: Option<String>,
    pub email: String,
}

impl From<Submission> for SubmissionJson {
    fn from(sub: Submission) -> Self {
        SubmissionJson{
            id: sub.id.map(|oid| oid.to_hex()),
            email: sub.email,
        }
    }
}

That will do the trick. To apply that directly to the example that you gave for your rocket handler:

#[get("/")]
fn index(db: PitchDb) -> Json<SubmissionCollection> {
    let submissions = Submission::find(db.clone(), None, None).unwrap()
        .into_iter().map(SubmissionJson::from).collect();
    Json(SubmissionCollection { submissions })
}

NOTE: you will have to update your SubmissionCollection type for the new SubmissionJson type.

brianwk commented 5 years ago

I already did something similar @thedodd. However, for the oid I am calling id: submission.id.unwrap().to_hex(),

I tried to use map as you did, however the method does not exist on Submission.

thedodd commented 5 years ago

Cool. So, you may already know, but if you don’t want to risk a panic, instead of unwrap, you can do unwrap_or_default. It will give you an empty string if the option is None.

thedodd commented 5 years ago

@brianwk if you think that the issue has been resolved, I'll go ahead and close out this issue.

As far as the original concern, I haven't quite been able to get serde to do what we have been discussing. I would definitely classify this as a serde issue, and not necessarily an issue with this repo, as we are just leveraging serde as is (nothing custom under the hood).