ormlite icon indicating copy to clipboard operation
ormlite copied to clipboard

[Feature Request] Implement Typestate Builder with `bon`

Open eboody opened this issue 11 months ago • 3 comments

Summary

The current builder() implementation for ormlite is not typestate-enforced, meaning it's possible to call .build() without ensuring all required fields are set. This can lead to runtime errors that could be prevented at compile time.

This issue proposes integrating bon into ormlite's builder system to leverage typestate tracking, ensuring that all required fields are set before calling .build(). Additionally, #[builder(with)] should be automatically derived for models, handling Join<T> types properly.


Motivation

  • Stronger Compile-Time Guarantees: Prevents incomplete struct construction at runtime.
  • Better Ergonomics: Developers get clear, guided steps to construct models properly.
  • Consistent Handling of Joins: Ensures that Join<T> fields are managed correctly.

Proposed Solution

  1. Modify the Model derive macro to:
    • Automatically add #[builder(with)] to generated models.
    • Ensure Join<T> fields are handled correctly.
  2. Use bon to enforce the required fields via typestate.
  3. Update documentation to reflect the new builder constraints.

Example Usage (Proposed API)

With the new implementation, the following incorrect code would fail at compile time:

let article = Article::builder().build(); // ❌ Error: Missing required fields

Instead, users will need to explicitly set all required fields:

let article = Article::builder()
    .title("Rust & ORMs".to_string())
    .content("Exploring Rust ORM libraries...".to_string())
    .build(); // ✅ Valid

Challenges & Considerations

  • Ensuring backwards compatibility where possible.
  • Handling optional fields gracefully.
  • Managing Join<T> types in the builder in a way that aligns with existing ormlite patterns.

References


Next Steps

  1. Discussion: Gather feedback from maintainers & contributors.
  2. Implementation: Modify Model derive macro.
  3. Testing: Ensure that the builder works correctly with required & optional fields.
  4. Documentation: Update README and examples to reflect changes.

Would love to hear thoughts from @kurtbuilds and other contributors. 🚀

eboody avatar Feb 05 '25 15:02 eboody

Hey, really appreciate the comments here.

I rarely use the builder logic (except for .update_partial(), which is almost all of my update logic), so I'm unbiased about this change.

The ModelBuilder functionality is used both for insertion and for partial updates (.update_partial()). Would the type checking only apply to the insertion struct? There's an argument that the two structs should be different - but that might also negatively impact code size / compile times.

Type safety would be fantastic. My biggest concern about doing this is the impact it would have on compile times. Have you seen benchmarks on what the impact is?

I think if this is done, (hell, even if it isn't done) it'd be nice to put some of the builder logic behind a feature flag to minimize generation of unused code.

kurtbuilds avatar Feb 05 '25 18:02 kurtbuilds

One other note, the Join code is still very alpha (though I increasingly need these APIs, so I'm doing some work to get it over the finish line), so I wouldn't worry too much about how it interplays with this direction. As I've mulled over the API, I think there will need to be a Join<T> for single joins (nullable and not), and a JoinMany which acts like a Vec for one-to-many or many-to-many joins.

kurtbuilds avatar Feb 05 '25 18:02 kurtbuilds

Ah, I didn't think about compile times! That's fair.

I was primarily thinking about addressing the problem of not having a compiler error when referencing a Join<T> that wasn't queried. Implementing bon for this would also provide a lot for free like ergonomics and extensibility which is nice.

The second reason I was tossing this idea around in my head is that there are models that I'd like the ability to derive bon::Builder on, but it conflicts with the ormlite's builder. Short of renaming the method, I can't think of a way around it.

There are a few models I work with that I use partial (which, when derived on a struct, makes a copy with all fields being options) along with bon::Builder so I think that would address the partial_update concern and partial is super small.

After you derive Builder and/or Partial it gives you some powerful options like defining and implementing traits that provide methods only when certain fields are set and/or unset (with IsSet and/or IsUnset) which is useful if you wanted to define methods only when, say, id is set. Or when fields that are Join<T> or have some other attribute.

You can imagine a call to .join on a builder only being available if its join_column field is not None.

The API being even more elegant and powerful is an attractive prospect because I already LOVE ormlite.

But yeah, I think you're 100% right that if this was a thing it would need to be behind a feature flag.

You prefer to define insertion structs over using the builder?

eboody avatar Feb 06 '25 14:02 eboody

Hey. I found some benchmarks for bon: https://bon-rs.com/guide/benchmarks/compilation. Looks like for sensible number of fields it's quite reasonable. typed-builder does not support partial build, so would avoid that. Other benefits of builders worth noting is the ability to add fields to a struct that have some sensible default and if downstream consumers use the builder they won't need to make any API changes at all. bon also supports function builder pattern too.

@eboody may have misunderstood but you can use builder_type or start_fn to change the name of the builder type or the builder "start" function i.e. foo::builder() can be foo::FooBuilder() see https://bon-rs.com/v1/reference/builder#start-fn

alexespencer avatar Nov 09 '25 10:11 alexespencer

Thank you for sharing those benchmarks. That document describes pretty egregious increases in compilation times: 0.1s without macros to 2.3s using bon.

That wouldn't be acceptable for this library, so I'll close this issue for now to keep issues tidy. Happy to discuss more if needed.

kurtbuilds avatar Nov 09 '25 15:11 kurtbuilds