Bad Binder

Flow
1

When you bind a field, or change the bean, vaadin calls the getter, calls any converters and sets the field value. So far so good.
Then it immediately converts back, and if the value is different it calls the setter

This means that the act of displaying data in an editor can change it.
User gets prompted: “Do you want to save?”, even if she has changed nothing.
IMHO this is objectively bad.

When I bind a form, I believe I can get around this by setting a “ignore writeback” on the bean, and test against that in the setter.
This is really ugly, but it seems to work.

In a grid however, it looks like the binding is set up lazily just before returning to client
So, even if I set a flag around my call to grid.editItem, the flag is off when binder.setBean is actually called and the fields are initialized.

Trying to find a way around this, I see that:

  • Binder.setBean calls Binding.initFieldValue(bean, /* writeBackChangedValues → */ true)
  • Binding.initFieldValue calls setter if writeBackChangedValues, and converted value is different.

This should’ve been trivial to fix: Create MyBinder extends Binder, override setBean, keep all the code, but replace the “true” with “false”.
Unfortunately, as is often the case with Vaadin, even though setBean itself is public, the things it calls are private, so I can’t do it.

I did a quick experiment with copying all of Binder, but it has other internal dependencie, so it looks like I’m stuck.

The only thing I can think of is to not allow vaadin to edit our data directly:
Ie, the getter/setter we give to vaadin is for the finished display value; We can not use Vaadin’s converters.

2

In the upcoming 24.4 are methods for your use case

3

Woo :slightly_smiling_face:

4

I’ve been checking for the release almost every day. Guess I’ll have to try the alpha

5

@quirky-zebra are you thinking about this? https://github.com/vaadin/flow/issues/18583
Doesn’t look like it covers my complaint

6

:thinking: I meant that… but well if it doesn’t help…

I do it like this in our forms:

  • Binder::addValueChangeListener
  • in there check if isClient=true
  • store that info
  • user leaves view… check if valueChangedFromClient = true → do your thing
7

Not sure how that helps. Are you suggesting that I keep track of which fields the user has changed, and on save I only save those?
Unfortunately, we also change bean values from our own code. Maybe when the user sets X, the bean sets Y and Z (and if those are visible, the screen fields are updated)

8

Back to #18583; I’m not sure I understood it, but it covers binder.writeBean. My problem is the writeback that happens already on binder.setBean

9

My listener is globally for the whole form, I don’t care what field was edited - I need the info if something was changed; not what…

If you have a problem that the setters are called to frequently; setBean should not be used but read/writeBean instead.

And you are using it within a grid… not exactly sure how that could interfere, never used it cause of bad UX

10

I let the bean remember the original values, so that I can ask it if it has changed, and which properties have changed. That again is used to generate the update statement. No JPA here.
Also, yes, it is a problem if the user is asked to save when nothing has changed. We might solve that with some global flag. But, it is also a problem if the user has updated something, but we also change fields that the user didn’t touch.

I want actual user changes to go directly back to the bean, so that it can trigger further changes. That rules out readBean I believe?
Also wouldn’t help for grid editor, which is unbuffered btw. All I can do there is to call editor.editItem, which will eventually call binder.setBean

11

Yeah that would sadly rule out readBean

12

So… I’ve looked at various ways of fixing Binder.
The “clean” ways were dead-ends, because Binder has private methods that a subclass can’t reach.
Also, it is not possible to create your own binder copy, since there is no parent class/interface, and there is at least one method that requires a Binder: grid.getEditor().setBinder(binder);
Also, these “clean” ways ended up with copying large amounts of code, and would be a maintenance problem.

Instead I’m now trying a sneaky way: Create a subclass and call vaadin’s private methods with reflection.
I don’t like this, but I like not working even less.
So far I’ve only verified that it runs, and I don’t get any SecurityException

    ...
    private static final Method initFieldValueMethod;
    private static final Field beanField;

    static {
        ...
        initFieldValueMethod = Binder.BindingImpl.class.getDeclaredMethod("initFieldValue", Object.class, boolean.class);
        beanField = Binder.class.getDeclaredField("bean");
        ...
        initFieldValueMethod.setAccessible(true);
        beanField.setAccessible(true);
    }
    
    @Override
    public void setBean(BEAN bean) {

        ...
        //this.bean = bean;
        beanField.set(this, bean);

        getBindings().forEach(b -> {
            // This is the only thing I wanted to override: We call this with *false* to prevent writeback
            //b.initFieldValue(bean, true)
            initFieldValueMethod.invoke(b, bean, false);

        });

        ...

    }

}```
13

I’m wondering how common is it to use the editor functionality in Grid :thinking: I think I have never used it myself, but I have this (WIP) project, related to these a Binder issues and I have not given a thought if it should work with the editor features in Grid at all. Comparing v24...feature/binder-for-the-rest-of-us · viritin/flow-viritin · GitHub

14

@empathetic-hippo there is a method in Binding to disable writeback conversion.

15

See here: flow/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java at main · vaadin/flow · GitHub

16

Binder actually has pretty versatile API, but it sometimes gets a bit time to get full grasp of it. And real life has shown that in different applications very different behaviors are wanted. So the defaults are often not matching. Some of them do, but you may want something to be different and so on. And in different application you want the different things. That is also one reason that has driven us to add new API’s there every now and then, e.g. like the one @quirky-zebra referred to. And when we have altered some behavior, often we have added some API to allow revert it go back to old behavior. And the discussions about different potential behaviors are nearly endless.

17

Literally endless, we discussed it probably since Vaadin 8 intensively every year with different use cases :sweat_smile:

18

@yummy-rhino, hadn’t seen that convertBackToPresentation, but it is only used in BindingImpl.writeFieldValue to prevent an extra update of the field? Don’t get it.
It is not used in initFieldValue, which is the one that bothered me.

I’m curious; Do you have any examples of when it is desirable to ‘write back’ in either of these situations?

Since you say there are many different behaviors wanted, maybe it should be easier to extend or replace it?

19

I think you should go for readBean and update the bean based on some event by using the new methods in Vaadin 24.4. The way setBean works is intentional and serves a purppse, i.e. validate the data coming from model as well. It is just not your scenario, which is something between setBean and readBean, when I now reread your description. The new methods give some new tools to this.