Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
91 stars 13 forks source link

serde skip_serializing_if and Many #31

Open jayvdb opened 1 year ago

jayvdb commented 1 year ago

If I have a struct which I want to serialize using serde, and wish to have a skip_serializing_if on a field which is a relation, I can use Option<ForeignKey<..>>, and use #[serde(default, skip_serializing_if = "Option::is_none")] .

If I am not using butane, a "many" field would be Vec<OtherModel> and I can use skip_serializing_if = "Vec::is_empty" to achieve the same. Many doesnt have an is_empty, and it probably have anything like that as it needs a data connection in order to know whether the relation has zero members.

When attempting to use Option<Many<...>>, the result is:

error[E0277]: the trait bound `Many<OtherModel>: ToSql` is not satisfied
   --> rust/models/src/lib.rs:714:5
    |
714 |     #[model]
    |     ^^^^^^^^ the trait `ToSql` is not implemented for `Many<OtherModel>`
    |
    = help: the following other types implement trait `ToSql`:
              &Polygon
              &...
            and 45 others
    = note: required for `std::option::Option<Many<OtherModel>>` to implement `ToSql`
    = note: required for `SqlVal` to implement `From<std::option::Option<Many<OtherModel>>>`
    = note: required for `std::option::Option<Many<OtherModel>>` to implement `Into<SqlVal>`
note: required by a bound in `FieldExpr`
   --> /home/jayvdb/.cargo/git/checkouts/butane-5043582c88b692d2/ca571a9/butane_core/src/query/fieldexpr.rs:42:8
    |
42  |     T: Into<SqlVal>,
    |        ^^^^^^^^^^^^ required by this bound in `FieldExpr`
    = note: this error originates in the attribute macro `model` (in Nightly builds, run with -Z macro-backtrace for more info)

Building my own Option<Many<OtherModel>> (i.e. struct OptionManyOtherModel(Option<Many<OtherModel>>) doesnt seem to help, because the inner member Many doesnt seem to have something that would help with to_sql/into_sql/etc - hmm, maybe the owner / ower_type could be used here, but I feel like I might be going in the wrong direction, and I need to be doing something in butane_codegen or butane_core/src/codegen to make this work .

Electron100 commented 1 year ago

The error reporting here could probably be improved, but Many is definitely a special case in Butane because it indicates linkage without actually having an associated database column in the same table. Under the hood it uses a separate table to manage the relation. I'm not sure Option<Many> would even mean semantically because a Many is already allowed to be an empty relation.

Given the current design of Butane, I'm not sure it's going to be possible to do exactly what you want smoothly because as you note Many doesn't necessarily have all the related entities loaded into memory. I think the most straightforward way would be to use an active Connection to transform into a different struct with everything in memory and then serialize that. If you don't want to do that, you'd still have to make sure each Many was loaded and we'd have to add a method to Many like fn is_empty(&self) -> Result<boolean> which fails if the Many is not loaded. Application code could then add a wrapper which ignores errors (because you know you loaded it) to transform into a straight boolean result and then skip serialization with that function.

jayvdb commented 1 year ago

Having a different set of structs which include an active Connection feels like a lot of baggage just to know whether a member is null or not. IMO it takes the joy out of the Many.

A Many.is_empty which fails if not already loaded may be an option, but feels brittle. In my use case, I would prefer to have a Many.is_not_loaded_or_is_empty so the struct metadata indicates the default is to not serialise the Many relationships, and there is no need to load the Many - in my case, those Many have their own Many, several layers deep. Requiring the app developer to be explicit about the loading and serialising of those Many is a good thing for performance.

Perhaps Many could be accompanied by a Multiple , which would work like a Many except it guarantees there is at least one item, which implies it has done the lookup to the additional table, and then Option<Multiple> makes more sense? The benefit of this (maybe only to some users?) is that then the "is empty"-ness is exposed in the Rust type system.

Removing the last item from a Multiple might be prevented, as the correct way to achieve that would be to change the Option to be None, which due to ownership will force de-allocating of the Multiple and its last item - hopefully this deallocation can be caught and result in a db update to remove the last item.