Binder - There is no way to setBean and skip writeback

[daniel-pss-prosoft]

Vaadin version: 14.1.2

There is no way to setBean without the binder writing back the "Presentation" values that the bound fields display. This is a serious issue, as it replaces initial Bean values with field "Presentation" values. This causes side effects which affects our application.

It is not acceptable for the Binder to change Bean values without permission. This causes bugs in our application. For example, it replaces NULL values with "0" or "" or whatever a field's default value is.

Because Vaadin put private methods in a library (not to mention FINAL classes), we are forced to use Reflection to deal with this issue (and others in Grid, Column , ColumnEditor, Components, you name it etc...), until hopefully this issue is resolved. We run a production App and are unable to wait 1-2 years until Vaadin fixes issues that require one line of code, or to add getters/setters to a variable, or change access modifiers etc.

The proposed fix is to implement the following method: setBean(BEAN bean, boolean writeBackValues)

Here is the code that should be implemented:

public void setBean(BEAN bean) {
    setBean(bean, true);
}

public void setBean(BEAN bean, boolean writeBackChanges) {
        checkBindingsCompleted("setBean");
        if (bean == null) {
            if (this.bean != null) {
                doRemoveBean(true);
                clearFields();
            }
        } else {
            doRemoveBean(false);
            this.bean = bean;
            getBindings().forEach(b -> b.initFieldValue(bean, writeBackChanges));
            // if there has been field value change listeners that trigger
            // validation, need to make sure the validation errors are cleared
            getValidationStatusHandler().statusChange(
                    BinderValidationStatus.createUnresolvedStatus(this));
            fireStatusChangeEvent(false);
        }
    }

Reporting this bug in the hope that it'll get fixed in the next 12-120 months. We previously had Business Prime/Premium support and it was very disappointing for more than one reason. Warranty / Bug fix priority is an illusion.

Thank you.

[daniel-pss-prosoft]

Same issue as https://github.com/vaadin/framework/issues/12092

[Legioth]

Is the core problem here that Binder updates the bean eagerly rather than only when the user chooses to save their changes, or that the converted presentation value is written to the bean also for fields that the user hasn't (yet) interacted with? What about using readBean and writeBean instead of setBean?

Would you expect the value in the bean to change e.g. from null to "" if the user would have focused the corresponding field in the UI and then immediately focused another field without making any input? What if the user would have made a change and then used ctrl + z to undo that change? What if the user entered a different value and then later manually changed back to the original blank representation?

Reporting this bug in the hope that it'll get fixed in the next 12-120 months.

This is not how open source community support works. First, it's arguable whether this is a bug or a missing feature. Second, if you want certainty that something gets done within a specific time frame, then it's recommended to either sponsor development of that feature or develop your own solution. Otherwise, the product owner will prioritize the suggestion in relationship to all other input based on their judgement of how it aligns with overall strategy and how beneficial it would be to the user base as a whole.

[TatuLund]

Also it is worth of pointing out that if you want to change null representation handling from the default in the binding there is withNullRepresentation(nullRepresentation)method for that

        /**
         * Maps binding value {@code null} to given null representation and back
         * to {@code null} when converting back to model value.
         *
         * @param nullRepresentation
         *            the value to use instead of {@code null}
         * @return a new binding with null representation handling.
         */

[daniel-pss-prosoft]

Regarding this issue: We expect the Binder to change Bean only when a field's value has changed intentionally:

• the user has changed the field manually
• setValue has been called on a field
• Field's initial value -> "ASDF" -> ctrl-z should also change the Bean to ""
• user changed the field to "ASDF" and then to "" means that the bean is set to "", not NULL. For us, NULL means not touched. We need to distinguish between NULL, "", "0".
• anything else which is not a side-effect of the binder, field, anything else

Right now we have one of two options:

• call readBean and manually set the bean field with Reflection afterwards. This isn't as safe as the next option.
• copy-paste the setBean method, call 3 methods with Reflection to prevent writeBackChanges from being called (we currently use this)

The problem is that the Binder has:

• setBean with writeback, which automatically updates fields
• readBean without writeback, which does not automatically update fields

The method setBean without writeback is missing and cannot be easily implemented with a subclass, because certain methods are private. RIP extensions, subclasses, improvements, customization, flexibility.

Regarding using readBean + writeBean: we need all fields to update from the bean, when the bean is changed. When certain fields are changed, we might need to perform calculations based on other fields. We need the Bean to always have the most recent values. On some forms, we perform server-side validation when any of the fields is changed.

@TatuLund this would not fix our issue, and would also require us to manually add withNullRepresentation to all of our bindings, which are approximately 5000....

Regarding the open source part: We understand your position. We've tried providing fixes and having a Prime account and buying UX consultancy services, all at the same time. Unfortunately, all proved to be disappointing experiences.

[Legioth]

When also taking into account use cases that are based on readBean + writeBean rather than only the setBean shorthand, then I suspect "writeback" isn't an appropriate mental model.

Intentional changes should be written back (when they happen when using setBean and when calling writeBean in other cases) while fields for which there are no explicit changes should be ignored regardless of whether writing is managed by setBean or writeBean.

Would a concept like "ignore untouched fields" be more generally applicable while still also covering this use case?

[daniel-pss-prosoft]

@Legioth I'm not sure i understand, but I'm under the impression that we are talking about the same thing.

Additionally: when reading/setting the Bean, values might go through conversions and might become something else. After these conversions, they will be written back to the Bean. This should also be taken into consideration by "ignore untouched fields"

[Legioth]

Additionally: when reading/setting the Bean, values might go through conversions and might become something else. After these conversions, they will be written back to the Bean. This should also be taken into consideration by "ignore untouched fields"

I assume you mean that such conversions should be treated in the same way as the originally discussed conversion from null to "", i.e. make changes to the value in the bean only in case the user has explicitly interacted with the field in the UI? That is how I imagine it would automatically work since the conversion of null into a null representation value isn't (almost) technically any different from any other kind of conversion.

I'm not sure i understand, but I'm under the impression that we are talking about the same thing.

Sounds like a clarification would be due. 😄

Binder provides two ways of managing the data flow from the UI to the bean: "automatic" mode based on setBean and "manual" mode based on writeBean. Being able to retain the original value in the bean if the user hasn't touched the UI field is relevant regardless of which way the application uses Binder.

Modelling the feature around the concept of "writeback" is relatively clear in the context of what happens when setBean is invoked, but it seems quite weird if used in combination with writeBean since the whole point of that method is to write values back to the bean.

My suggestion would thus be to instead focus on a mental model that would help achieve the same end result, but be applicable both with setBean and writeBean. One suggestion for that would be to introduce a configuration parameter on Binder to control whether "untouched" bindings should be ignored when writing to the bean, regardless of how that write is triggered. Usage could then be along the lines of myBinder.setWriteMode(WriteMode.ONLY_TOUCHED_FIELDS);.

[daniel-pss-prosoft]

@Legioth yes, such conversions should be treated in the same way. .

I haven't thought if "Only touched fields" should be a setting of the Binder or a method argument. After giving it some thought, your proposed solution makes more sense (myBinder.setWriteMode(WriteMode.ONLY_TOUCHED_FIELDS);)

What about the private fields and methods in the Binder, Grid, Column, GridEditor? For example, in Grid we cannot manually set "propertySet" as we could in Vaadin 8. In this moment we are in the process of porting a Vaadin 8 App to Vaadin 14. The Vaadin 8 App is using DynaBeans; We are forced to use Reflection to deal with this issue.

This is a serious issue as we cannot extend certain classes or override certain methods. We are forced to clone code to change a single variable or a line of code. We are forced to use Reflection to call methods and set fields, which is very unsafe, likely not forwards-compatible and likely limits our app to a single version of Vaadin.

Thank you

[pleku]

The way the framework has been done for most parts is that only parts that are meant to be extendable for a good reason, are extendable. If we were to expose all parts as something that one can override by e.g. extending the class and overriding a protected method, then everything would be considered public API and we could not really make changes after introducing things or we could be forced to break someones app all the time if we want to improve things. And making things open for extension by default would put more pressure and cost on the initial design of any part, if that were the default. A long experience with the framework has thus proven that API should be opened only when it makes sense, and not by default. Then based on the feedback and use cases, more API can be added or existing made extendable. I'm sure there is valid extension points that we are missing.

As already discussed about this particular case, I think this proposed API would be quite good solution for it:

My suggestion would thus be to instead focus on a mental model that would help achieve the same end result, but be applicable both with setBean and writeBean. One suggestion for that would be to introduce a configuration parameter on Binder to control whether "untouched" bindings should be ignored when writing to the bean, regardless of how that write is triggered. Usage could then be along the lines of myBinder.setWriteMode(WriteMode.ONLY_TOUCHED_FIELDS);.

Another alternative or supplement could to provide an override of setBean and writeBean which would have the WriteMode writeMode as an extra parameter, so it could maybe be more discoverable.

[daniel-pss-prosoft]

If a public API is needed, then you could simply use an interface, after which replace all private methods/fields with protected . Programmers are grown-ups which can make their own decisions. If they want to extend some functionality, they should be able to. If they want to fix a bug, they should be able to. If these decisions cause bugs after upgrades, then the bugs are isolated in their extensions. This does not affect how the framework works.

While you mention this, Vaadin doesn't seem to be much concerned about breaking changes.

• See migrating from Vaadin 8-10 to 14.
• See migrating from Vaadin 13 to 14.
• See migrating from Vaadin 14 to the next LTS
• See migrating the next LTS to the next LTS+1 Do you see a pattern here ? Me too. Then why is Vaadin forcing programmers to use Reflection ?

An example of a exposing Grid methods through an Interface:

interface IGrid {
  void vaadinMethod();
}

class Grid implements IGrid {
  protected void notAPrivateMethodInAFramework() {
         System.out.println("This is for grown-ups only!");
   }
  public void vaadinMethod() {
         System.out.println("Hello!");
  }

  public void anotherVaadinMethod() {
         System.out.println("Not accessible, because it's not in the interface!");
  }
}

IGrid grid = new Grid(); // WHOA, i'm using an API !

Now, if the programmer wants to make an extension, he can do this

interface IExtendedGrid extends IGrid {
  void notAPrivateMethodInAFramework();
}

class ExtendedGrid extends Grid implements IExtendedGrid {
    @Override
   public void notAPrivateMethodInAFramework() {
       // **We need to manually run this, to fix some bug**
       // Our app is broken RIGHT NOW and we need a fix!
       // The customer cannot wait 6-12 months for Warranty bug to get fixed
       super.notAPrivateMethodInAFramework();
  }
}

IExtendedGrid grid = new ExtendedGrid(); // WHOA, i'm using an Extended (!!!) API !

[Legioth]

While you mention this, Vaadin doesn't seem to be much concerned about breaking changes.

There is a difference between breaking changes in major version updates and breaking changes in bugfix releases or minor releases that add new features. Breaking changes are needed from time to time to avoid stagnating, but they need to be done in a structured way so that developers can keep using the old version until they have had a chance to update.

Breaking changes outside new major versions would mean that a business cannot easily update to gain access to critical bug fixes or even security updates. Introducing a breaking change in a new major version is less of a problem since the old major version still remains maintained.

That's why reusable libraries and frameworks typically expose a limited API surface based on the use cases that they have chosen to explicilty support, while using encapsulation mechanisms such as private for other parts to make it easy to change those as needed. Every public or protected method is a comittment for stability. Each such comittment makes it more difficult to fix bugs or make performance improvements in a way that doesn't break backwards compatibility.

Programmers are grown-ups which can make their own decisions.

Based on what would the programmes make those decisions? How are they supposed to know which parts of the exposed API are safe to use and which parts are to be avoided if they want to avoid the risk of rework to install a security fix?

• protected is not a suitable way of distinguishing since it also has its role in defining explicitly designed extension points.
• public in the class (e.g. Grid) but not declared in an interface (e.g. IGrid) would only work if developers would primarily use the interface API and only access class API as a calculated risk. That's not something Java developers are used to and it's not how their tooling encourages them to work.

If they want to extend some functionality, they should be able to. If they want to fix a bug, they should be able to.

They are able to do this even if methods and fields are private. They can use reflection. If that is not enough, then Vaadin is open source which means that they can create their own copies of key classes or even maintain their own internal version of Vaadin.

Sure, those options require more effort from the application developer, but that's the whole point. It should require an explicit choice and it shouldn't be something that one might accidentally do without taking the potential consequences into account.

It would be great if there would be some other way of making such workarounds possible that wouldn't be so "expensive", but I have so far not heard about any such alternative that wouldn't also have a significant risk of accidentally being used unknowingly.

[daniel-pss-prosoft]

Breaking changes are needed from time to time to avoid stagnating I understand that this is the current flavor of the month way of approaching developing frameworks.... at the cost of stability, predictability, upgrade cost etc... "Minor" things that affect the whole life-cycle of a product. Believe it or not, this is not as important as the discussed problem: final classes and private methods in frameworks (Vaadin, specifically)

First of all: No one is suggesting to make breaking changes in a minor version.

You said: reusable libraries and frameworks typically expose a limited API surface based on the use cases that they have chosen to explicilty support ... And i completely agree. This is exactly how things are supposed to work! The framework developer offers some kind of support (community/paid) for the API. Extensions are the responsibility of the person that made them.

any such alternative that wouldn't also have a significant risk of accidentally being used unknowingly. Please leave the grown-up programmers to make their own choices. You really don't need to hold their hand while they're working or worse, impede their progress and generate frustration. The framework developer does not need to be concerned of the reasoning behind the programmers' choices. The programmers have experience as well. They are architects, engineers etc. The framework developer provides an API, documentation for the API and that's about it.

If the framework developer goes beyond that, and locks the framework for extension, then they are interfering in a bad way with the development process of the applications using the framework. They are constraining the framework users to use the framework with the default bugs and feature restrictions. I'm sure that Vaadin doesn't want users to have bugs in their app, but if Vaadin sets fields and methods as private to protect the Pro version, then that's pretty sad. I'm hoping that this isn't the case.

Vaadin is open source which means that they can create their own copies of key classes or even maintain their own internal version of Vaadin. What is the point of forcing the programmer to copy-paste a 3000 line (!!!) class to change one access modifier or to override a method? Don't you see a problem here? This is forcing developers to jump through obstacles without a proper reason...

Right now, 06.09.2020, the cost of fixing a bug in Grid is 3000 lines of duplicate code, ( hypothetically, if Reflection can't be used ) How are the developers protected when they have 3000 lines of duplicate code, from just fixing a bug in one class?

[Legioth]

Your suggestion seems to be that the framework should avoid using private modifiers for internal methods and fields, but instead use protected everywhere. Would a change to the contract (e.g. signature or key aspects of how it behaves) of a protected internal method be considerd a breaking change?

If that is seen as a breaking change, then it would be much more difficult for Vaadin to fix issues in maintenance releases or add new features in minor releases without also introducing breaking changes.

If changes to the contract of a protected method is not seen as a breaking change, then the main challenge is that it's contrary to the general convention in the Java ecosystem. Developers have learned to expect that protected methods are part of the official API of the library or framework and that a change to the contract of a protected method is a seen as a breaking change. Java itself is leading the way here with how protected is treated in the JRE.

It would be quite confusing for developers if Vaadin would follow a completely different convention. Why is Vaadin so different that it shouldn't follow typical Java conventions in this case? How could we ensure that all developers who use Vaadin are aware of the exceptions?

Another challenge is that there would be two different types of protected API in the framework. Additional documentation would have to be added to make each case clear.

• Some protected methods belong to explicitly designed extension points (e.g. Composite.initContent) that are equivalent with public methods when it comes to breaking changes.
• Some protected methods are internal implementation details that might be changed without warning even in maintenance or minor releases.

What is the point of forcing the programmer to copy-paste a 3000 line (!!!) class to change one access modifier or to override a method? Don't you see a problem here?

Copying is cheap (!!!). The main challenge is to maintain the copy. To deal with that, a Git clone could be used instead of a verbatim copy so that upstream changes can automatically be merged in for the parts that you haven't changed for you own extension.

That approach gives you the possibilty to create your own extensions as needed while also making it 100% clear to you which parst are supported by Vaadin and which parts are your own responsibilty. The cost of it is slightly more process overhead to merge in changes and build your own internal version, but that's mostly a one-time task to set up and then it will require manual intervention only when there are merge conflicts, which corresponds to the occasions when there would be changes in protected method signatures if all private methods would be changed to unsupported protected methods.