Give HeartbeatHandler the highest priority in VaadinPortletService #167
Conversation
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @denis-anisimov, @joheriks, and @mehdi-vaadin)
vaadin-portlet/src/main/java/com/vaadin/flow/portal/VaadinPortletService.java, line 123 at r1 (raw file):
are handles
typo ?
are handled
vaadin-portlet/src/main/java/com/vaadin/flow/portal/VaadinPortletService.java, line 126 at r1 (raw file):
handlers.add(new HeartbeatHandler());
Wont't it added twice ? (also in super.createRequestHandlers() ).
So is it totally forgotten and handled incorrectly by PortletUidlRequestHandler or just
has a wrong order ?
vaadin-portlet-integration-tests/tests-generic/src/test/java/com/vaadin/flow/portal/requesthandlers/HeartbeatHandlerIT.java, line 38 at r1 (raw file):
Thread.sleep(35000);
Looks quite long timeout. Can it be reduced ?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @denis-anisimov, @joheriks, and @mehdi-vaadin)
vaadin-portlet/src/main/java/com/vaadin/flow/portal/VaadinPortletService.java, line 123 at r1 (raw file):
Previously, denis-anisimov (Denis) wrote…
are handlestypo ?
are handled
Done.
vaadin-portlet/src/main/java/com/vaadin/flow/portal/VaadinPortletService.java, line 126 at r1 (raw file):
Previously, denis-anisimov (Denis) wrote…
handlers.add(new HeartbeatHandler());Wont't it added twice ? (also in
super.createRequestHandlers()).
So is it totally forgotten and handled incorrectly byPortletUidlRequestHandleror just
has a wrong order ?
You're right. I removed the extra one.
It's handled by PortletUidlRequestHandler and not by HeartbeatHandler. With changing the order it's handled by the correct handler.
vaadin-portlet-integration-tests/tests-generic/src/test/java/com/vaadin/flow/portal/requesthandlers/HeartbeatHandlerIT.java, line 38 at r1 (raw file):
Previously, denis-anisimov (Denis) wrote…
Thread.sleep(35000);Looks quite long timeout. Can it be reduced ?
I guess it should be fine. I reduced it.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2.
Dismissed @vaadin-bot from a discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @joheriks)
| ButtonElement buttonElement = getFirstPortlet().$(ButtonElement.class) | ||
| .first(); | ||
|
|
||
| Thread.sleep(17000); |
There was a problem hiding this comment.
|
SonarQube analysis reported 3 issues
Watch the comments in this conversation to review them. 2 extra issuesNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
There was a problem hiding this comment.
Dismissed @vaadin-bot from a discussion.
Reviewable status:
Fix #166
This change is