Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check the component belongs to UI before task execution #11726

Merged
merged 3 commits into from
Sep 7, 2021
Merged

fix: check the component belongs to UI before task execution #11726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dda0321
fix: check the component belongs to UI before task execution
mshabarov Sep 2, 2021
2042cde
Add IAE to method signature
mshabarov Sep 6, 2021
b600204
Merge remote-tracking branch 'origin/master' into fix/11599-component…
mshabarov Sep 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion flow-server/src/main/java/com/vaadin/flow/component/UI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,9 +995,12 @@ public Router getRouter() {
*
* @return a registration that can be used to cancel the execution of the
* task
* @throws IllegalArgumentException
* if the given component doesn't belong to this UI
*/
public ExecutionRegistration beforeClientResponse(Component component,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method should have throws: javadoc is not necessary but if you extend javadocs then the method should throws .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

SerializableConsumer<ExecutionContext> execution) {
SerializableConsumer<ExecutionContext> execution)
throws IllegalArgumentException {

if (component == null) {
throw new IllegalArgumentException(
Expand All @@ -1008,6 +1011,11 @@ public ExecutionRegistration beforeClientResponse(Component component,
"The 'execution' parameter may not be null");
}

if (component.getUI().isPresent() && component.getUI().get() != this) {
throw new IllegalArgumentException(
"The given component doesn't belong to the UI the task to be executed on");
}

return internals.getStateTree().beforeClientResponse(
component.getElement().getNode(), execution);
}
Expand Down
25 changes: 25 additions & 0 deletions flow-server/src/test/java/com/vaadin/flow/component/UITest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,31 @@ public void beforeClientResponse_withReattachedNodes() {
callCounter.get());
}

@Test
public void beforeClientResponse_componentNotAttachedToUi_noException() {
UI ui = createTestUI();
Component component = new AttachableComponent();
ui.beforeClientResponse(component, context -> {
});
}

@Test()
public void beforeClientResponse_componentBelongsToAnotherUI_throws() {
UI firstUI = createTestUI();
UI anotherUI = createTestUI();
Component component = new AttachableComponent();
anotherUI.add(component);

IllegalArgumentException exception = Assert.assertThrows(
IllegalArgumentException.class,
() -> firstUI.beforeClientResponse(component, context -> {
}));

Assert.assertEquals(
"The given component doesn't belong to the UI the task to be executed on",
exception.getMessage());
}

@ListenerPriority(5)
private static class BeforeEnterListenerFirst
implements BeforeEnterListener {
Expand Down