Return back preventing disallowed property update #4774
Conversation
denis-anisimov
changed the title
| (ListModelType<?>) propertyType); | ||
| } | ||
| if (beanType != null) { | ||
| ((BeanModelType<?>) beanType).collectAllowedProperties( |
There was a problem hiding this comment.
| return updateFromClientFilter.test(key); | ||
| } else { | ||
| return false; | ||
| private AllowUpdate isUpdateFromClientAllowedByFilter(StateNode node, |
There was a problem hiding this comment.
denis-anisimov
changed the title
There was a problem hiding this comment.
Reviewed 2 of 4 files at r1, 2 of 2 files at r3, 5 of 5 files at r4.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained
flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java, line 298 at r4 (raw file):
} return AllowUpdate.NO_EXPLICIT_STATUS; }
Seems right, but would benefit from having a flowchart as this is really hard to follow when it goes through the same methods multiple times. As at first the new rounds seem unnecessary.
Perhaps clarify the flow for deferredUpdateFromClient.
Add the flowchart to comment or as it's own page to the wiki.
flow-server/src/main/java/com/vaadin/flow/templatemodel/BeanModelType.java, line 682 at r4 (raw file):
((ListModelType<?>) type)
Un needed cast as we only take in ListModelType<?>
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained
flow-server/src/main/java/com/vaadin/flow/templatemodel/BeanModelType.java, line 682 at r4 (raw file):
Previously, caalador wrote…
((ListModelType<?>) type)Un needed cast as we only take in ListModelType<?>
Right, leftover from refactoring.
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained
flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java, line 298 at r4 (raw file):
Previously, caalador wrote…
Seems right, but would benefit from having a flowchart as this is really hard to follow when it goes through the same methods multiple times. As at first the new rounds seem unnecessary.
Perhaps clarify the flow for deferredUpdateFromClient.
Add the flowchart to comment or as it's own page to the wiki.
I agree that this is complicated logic.
I've extracted private methods and added comments with ascii chart.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r5.
Reviewable status: 3 unresolved discussions, 1 of 1 LGTMs obtained
flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java, line 298 at r4 (raw file):
Previously, denis-anisimov (Denis) wrote…
I agree that this is complicated logic.
I've extracted private methods and added comments with ascii chart.
Nice.
| return allowUpdateFromClient(key, value); | ||
| } | ||
|
|
||
| private boolean allowUpdateFromClient(String key, Serializable value) { |
There was a problem hiding this comment.
| * | ||
| * Here is the logic flow: | ||
| * | ||
| * <pre> |
There was a problem hiding this comment.
|
SonarQube analysis reported 4 issues
Watch the comments in this conversation to review them. |
There was a problem hiding this comment.
Dismissed @vaadin-bot from 4 discussions.
Reviewable status:complete! all discussions resolved, 1 of 1 LGTMs obtained
# Conflicts: # flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java
This change is