thedodd / wither

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

Support models which have generics. #37

Closed magiclen closed 4 years ago

magiclen commented 5 years ago

To support models like the following struct,

#[derive(Debug, Serialize, Deserialize, Model)]
struct Person<'a> {
    #[serde(rename = "_id", skip_serializing_if = "Option::is_none")]
    id: Option<ObjectId>,
    name: Cow<'a, str>
}
magiclen commented 5 years ago

The reason for the failure is that I changed the lifetime 'a to '_de because 'a is too often to be used by developers. I think it is not appropriate to be an internal-generated lifetime. Anyway, the test case has been fixed.

thedodd commented 5 years ago

@magiclen so an additional request for you here. It would be great to add an additional compile test which covers the exact example that you have in the description for this PR. That way we know the compiler is accepting the original model patterns, in addition to the new model pattern you've introduced here.

Should be as simple as defining the new model type here: https://github.com/thedodd/wither/blob/master/wither_tests/tests/modelgen.rs#L80 along with a minimal number of asserts (most will be covered by previous tests). That will ensure that the model's code-gen is working as needed and that such models will be usable.

thedodd commented 5 years ago

@magiclen this is a copy/paste from #35:

I've been leaning towards using the DeserializeOwned bound on these models, and given the nature of a data model, I'm inclined to say that it should not be generic. Instead ... perhaps if you need a generic model, instead of using generics, you could use a bson field type. That would allow you to use the same model, and then just deserialize the field type as needed. You could even use an enum type with serde's internally tagged serialization model, which would remove the need for generic types.

As far as generic lifetime bounds ... well, when taking data directly out of the database, it doesn't make much sense. I am inclined to keep generic types & lifetimes out of the models in order to keep them simple and direct.

Thoughts?

thedodd commented 4 years ago

Closing in favor of using non-generic enum patterns which work quite nicely with this system already.