Jaguar-dart / jaguar_orm

Source-generated ORM with relations (one-to-one, one-to-many, many-to-many), preloading, cascading, polymorphic relations, etc
https://jaguar-dart.github.io
BSD 3-Clause "New" or "Revised" License
217 stars 54 forks source link

Immutability support relations #69

Open jaumard opened 5 years ago

jaumard commented 5 years ago

If model are immutable, all generated code about relations doesn't compile anymore.

Mostly because if generate stuff like this:

void associateCart(CartItem child, Cart parent) {
    child.cartId = parent.id;
  }
Future<Cart> preload(Cart model, {bool cascade: false}) async {
    model.items = await cartItemBean.findByCart(model.id,
        preload: cascade, cascade: cascade);
    return model;
  }

  Future<List<Cart>> preloadAll(List<Cart> models,
      {bool cascade: false}) async {
    models.forEach((Cart model) => model.items ??= []);
    await OneToXHelper.preloadAll<Cart, CartItem>(
        models,
        (Cart model) => [model.id],
        cartItemBean.findByCartList,
        (CartItem model) => [model.cartId],
        (Cart model, CartItem child) => model.items.add(child),
        cascade: cascade);
    return models;
  }

as models are immutable it's not possible to access setters directly

tejainece commented 5 years ago

This will not be supported.

jaumard commented 5 years ago

:( si you can't say #42 is fixed ^^ Why don't you want to support this?

tejainece commented 5 years ago

Jaguar supports immutable fields. Not immutable models. One can have immutable models if there are no relations.

If there are relations, we have to set fields to associate them.

If we come up with a method to achieve this, we can implement it.

jaumard commented 5 years ago

Yes immutable fields are supported since a long time, I already use them. But what I'm interested in is immutable models :)

There is a simple way to achieve this by just returning a full copy of object with the relations instead of settings fields. That's how the all flutter frameworks works, you have a copy method you can call with the possibility to override any fields, that method return a new immutable version of the object. What do you think about that ?

jaumard commented 5 years ago

As it's a breaking changes I think v4 is the perfect time for doing it ^^

tejainece commented 5 years ago

I think in general immutable models are awesome. But I do not agree with making the api and generation complex to get complete immutability. I have never used build_value and its shebang.

Please present a fully implemented, hand written bean example for one-to-one scenario with immutable relations. If it is does not complicate things, let us absorb it into the generator.

What say?

jaumard commented 5 years ago

I don't think we need build_value (never used it too ^^), we just need to generate a copy method on our generator, and change the preload methods to return a new object.

I'll create a one-to-one example ASAP ! I'm reopening the issue for now, if we finally don't do it we'll close it again. But I really think we can do it without to much trouble

tejainece commented 5 years ago

Sounds good.

jaumard commented 5 years ago

Here is my manual implementation of one-to-one association with immutability, all modifications can be done under the generator I think:

https://gist.github.com/jaumard/5e777dbd1f2090a7d859061aa2165f5d

Tested on my Mac with PostgreSQL DB

Let me know what you think @tejainece !

jaumard commented 4 years ago

@tejainece any news on this ? I think it will be a nice addition to v4 :)

tejainece commented 4 years ago

I have not got a clean proposal that does not sacrifice user friendliness for immutability.

jaumard commented 4 years ago

Did you check my gist ? It doesn't sacrifice anything, just give you immutability

tejainece commented 4 years ago

The approach you suggested only supports immutable models. It does not support mutable models.

tejainece commented 4 years ago

I think we should handle immutable models and mutable models differently.

jaumard commented 4 years ago

In fact it does, if you don't put your fields as final they are mutable for the users. What the generated code use is not really a concern imho. Also supporting mutable models it's supporting a bad practice... So not sure it worth supporting both differently (more work, less easy for users and more error prone)

jaumard commented 4 years ago

If you really want to support both we can keep the current way, add a boolean field on the Bean annotation and do the immutable way if that flag is true (I would push for immutable by default but if you don't want it's fine for me ^^)