Bad design of StateTree.collectDirtyNodes()

[denis-anisimov]

com.vaadin.flow.internal.StateTree.collectDirtyNodes() is a public method which mutates the internal state.
The method cleans up dirty nodes list.
User may call it at any time so it will break the logic which relies on this list to collect changes.

Actually collectChanges() method delegates the call to collectDirtyNodes() snd also causes the same issue.

The dirty nodes list should be cleaned up once (in the end of request handling ?) instead of cleaning up each time when changes are collected (f.e. as a reaction on event). And this logic should not be exposed as API.

[denis-anisimov]

Notes for myself:

• identify all methods that should not be public and breaks the "assumption" (which is : the StateTree state is changed (actually cleaned up) only from expected places).
• if it's possible make "friend" classes so that those methods are not exposed as public API anyhow.
• if it's not possible to tricky then just add javadocs so anyone who is going to call these methods don't call them unexpectedly.

[denis-anisimov]

In the PR :

• collectDirtyNodes doesn't clean the internal state anymore.
• collectChanges is still an issue. It mutates the internal state and has to be called from one place in fact. But it's too tricky to fix this since:
  • there are many tests that rely on it. I can make it private method so only one dedicated caller may call it but it will be too hard to fix existing tests.
  • the problem is also delegated to the StateNode::collectChanges method which also mutates the StateNode state though it should only collect changes. And this method is also used in many places in the tests.

So I've just added warnings to avoid calling this methods from unexpected places.