Skip to content

Commit

Permalink
test: unignore tests in PopStateHandlerIT (#10750)
Browse files Browse the repository at this point in the history 
* fix: enable HistoryStateChangeHandler in client-side bootstrap mode

* test: ignore a test for excessive HistoryStateChangeEvent occurences

* Update flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/PopStateHandlerIT.java

Co-authored-by: Haijian Wang <[email protected]>

* fix: restore consistent URL trimming behaviour

* refactor: remove extra nested else

* chore: fix formatting

Co-authored-by: Haijian Wang <[email protected]>
  • Loading branch information
@platosha @haijian-vaadin
platosha and haijian-vaadin authored May 5, 2021
1 parent df4b798 commit c0ce21b
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 82 deletions.
3 changes: 2 additions & 1 deletion flow-client/src/main/frontend/Flow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ export class Flow {
this.container.localName,
this.container.id,
this.getFlowRoute(ctx),
this.appShellTitle
this.appShellTitle,
history.state
);
});
} else {
Expand Down
103 changes: 60 additions & 43 deletions flow-server/src/main/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.vaadin.flow.component.HasElement;
import com.vaadin.flow.component.Tag;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.page.History;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.internal.nodefeature.NodeProperties;
import com.vaadin.flow.router.ErrorNavigationEvent;
Expand All @@ -43,10 +44,12 @@
import com.vaadin.flow.router.internal.PathUtil;
import com.vaadin.flow.server.communication.JavaScriptBootstrapHandler;

import elemental.json.JsonValue;

/**
* Custom UI for {@link JavaScriptBootstrapHandler}. This class is intended for
* internal use in client side bootstrapping.
*
*
* <p>
* For internal use only. May be renamed or removed in a future release.
*/
Expand Down Expand Up @@ -105,13 +108,25 @@ public String getForwardToClientUrl() {
* client side element id
* @param flowRoute
* flow route that should be attached to the client element
* @param appShellTitle
* client side title of the application shell
* @param historyState
* client side history state value
*/
@ClientCallable
public void connectClient(String clientElementTag, String clientElementId,
String flowRoute, String appShellTitle) {
String flowRoute, String appShellTitle, JsonValue historyState) {
if (appShellTitle != null && !appShellTitle.isEmpty()) {
getInternals().setAppShellTitle(appShellTitle);
}

final String trimmedRoute = PathUtil.trimPath(flowRoute);
if (!trimmedRoute.equals(flowRoute)) {
// See InternalRedirectHandler invoked via Router.
getPage().getHistory().replaceState(null, trimmedRoute);
}
final Location location = new Location(trimmedRoute);

if (wrapperElement == null) {
// Create flow reference for the client outlet element
wrapperElement = new Element(clientElementTag);
Expand All @@ -120,23 +135,25 @@ public void connectClient(String clientElementTag, String clientElementId,
getElement().getStateProvider().appendVirtualChild(
getElement().getNode(), wrapperElement,
NodeProperties.INJECT_BY_ID, clientElementId);
}

final String trimmedRoute = PathUtil.trimPath(flowRoute);
if (!trimmedRoute.equals(flowRoute)) {
// See InternalRedirectHandler invoked via Router.
getPage().getHistory().replaceState(null, trimmedRoute);
}
getPage().getHistory().setHistoryStateChangeHandler(
event -> renderViewForRoute(event.getLocation(),
NavigationTrigger.CLIENT_SIDE));

// Render the flow view that the user wants to navigate to.
renderViewForRoute(new Location(trimmedRoute),
NavigationTrigger.CLIENT_SIDE);
// Render the flow view that the user wants to navigate to.
renderViewForRoute(location, NavigationTrigger.CLIENT_SIDE);
} else {
History.HistoryStateChangeHandler handler = getPage().getHistory()
.getHistoryStateChangeHandler();
handler.onHistoryStateChange(new History.HistoryStateChangeEvent(
getPage().getHistory(), historyState, location,
NavigationTrigger.CLIENT_SIDE));
}

// true if the target is client-view and the push mode is disable
if (getForwardToClientUrl() != null) {
navigateToClient(getForwardToClientUrl());
acknowledgeClient();

} else if (isPostponed()) {
cancelClient();
} else {
Expand Down Expand Up @@ -181,41 +198,41 @@ public void navigate(String pathname, QueryParameters queryParameters) {
if (Boolean.TRUE.equals(getSession().getAttribute(SERVER_ROUTING))) {
// server-side routing
renderViewForRoute(location, NavigationTrigger.UI_NAVIGATE);
} else {
// client-side routing
return;
}

// There is an in-progress navigation or there are no changes,
// prevent looping
if (navigationInProgress || getInternals().hasLastHandledLocation()
&& sameLocation(getInternals().getLastHandledLocation(),
location)) {
return;
}
// client-side routing

// There is an in-progress navigation or there are no changes,
// prevent looping
if (navigationInProgress
|| getInternals().hasLastHandledLocation() && sameLocation(
getInternals().getLastHandledLocation(), location)) {
return;
}

navigationInProgress = true;
try {
Optional<NavigationState> navigationState = getInternals()
.getRouter().resolveNavigationTarget(location);

if (navigationState.isPresent()) {
// Navigation can be done in server side without extra
// round-trip
handleNavigation(location, navigationState.get(),
NavigationTrigger.UI_NAVIGATE);
if (getForwardToClientUrl() != null) {
// Server is forwarding to a client route from a
// BeforeEnter.
navigateToClient(getForwardToClientUrl());
}
} else {
// Server cannot resolve navigation, let client-side to
// handle it.
navigateToClient(location.getPathWithQueryParameters());
navigationInProgress = true;
try {
Optional<NavigationState> navigationState = getInternals()
.getRouter().resolveNavigationTarget(location);

if (navigationState.isPresent()) {
// Navigation can be done in server side without extra
// round-trip
handleNavigation(location, navigationState.get(),
NavigationTrigger.UI_NAVIGATE);
if (getForwardToClientUrl() != null) {
// Server is forwarding to a client route from a
// BeforeEnter.
navigateToClient(getForwardToClientUrl());
}
} finally {
navigationInProgress = false;
} else {
// Server cannot resolve navigation, let client-side to
// handle it.
navigateToClient(location.getPathWithQueryParameters());
}

} finally {
navigationInProgress = false;
}
}

Expand Down
8 changes: 6 additions & 2 deletions flow-server/src/main/java/com/vaadin/flow/component/page/History.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,13 @@ public void pushState(JsonValue state, String location) {
* to only change the JSON state
*/
public void pushState(JsonValue state, Location location) {
final String pathWithQueryParameters = Optional.ofNullable(location)
.map(Location::getPathWithQueryParameters).orElse(null);
// Second parameter is title which is currently ignored according to
// https://developer.mozilla.org/en-US/docs/Web/API/History_API
ui.getPage().executeJs(
"setTimeout(() => window.history.pushState($0, '', $1))", state,
location.getPathWithQueryParameters());
pathWithQueryParameters);
}

/**
Expand Down Expand Up @@ -219,11 +221,13 @@ public void replaceState(JsonValue state, String location) {
* to only change the JSON state
*/
public void replaceState(JsonValue state, Location location) {
final String pathWithQueryParameters = Optional.ofNullable(location)
.map(Location::getPathWithQueryParameters).orElse(null);
// Second parameter is title which is currently ignored according to
// https://developer.mozilla.org/en-US/docs/Web/API/History_API
ui.getPage().executeJs(
"setTimeout(() => window.history.replaceState($0, '', $1))",
state, location.getPathWithQueryParameters());
state, pathWithQueryParameters);
}

/**
Expand Down
46 changes: 24 additions & 22 deletions flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,37 +207,37 @@ public void cleanup() {

@Test
public void should_allow_navigation() {
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
ui.wrapperElement.getChild(0).getChild(0).getTag());

// Dirty view is allowed after clean view
ui.connectClient("foo", "bar", "/dirty", "");
ui.connectClient("foo", "bar", "/dirty", "", null);
assertEquals(Tag.SPAN, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H1,
ui.wrapperElement.getChild(0).getChild(0).getTag());
}

@Test
public void should_navigate_when_endingSlash() {
ui.connectClient("foo", "bar", "/clean/", "");
ui.connectClient("foo", "bar", "/clean/", "", null);
assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
ui.wrapperElement.getChild(0).getChild(0).getTag());
}

@Test
public void getChildren_should_notReturnAnEmptyList() {
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(1, ui.getChildren().count());
}

@Test
public void addRemoveComponent_clientSideRouting_addsToBody() {
final Element uiElement = ui.getElement();

ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
// router outlet is a virtual child that is not reflected on element
// level
assertEquals(1, ui.getChildren().count());
Expand Down Expand Up @@ -293,7 +293,7 @@ public void addRemoveComponent_serverSideRouting_addsDirectlyToUI() {
public void addComponent_clientSideRouterAndNavigation_componentsRemain() {
final Element uiElement = ui.getElement();
// trigger route via client
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
final RouterLink routerLink = new RouterLink();
ui.add(routerLink);

Expand Down Expand Up @@ -331,25 +331,25 @@ public void addComponent_serverSideRouterAndNavigation_componentsRemain() {

@Test
public void should_prevent_navigation_on_dirty() {
ui.connectClient("foo", "bar", "/dirty", "");
ui.connectClient("foo", "bar", "/dirty", "", null);
assertEquals(Tag.SPAN, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H1,
ui.wrapperElement.getChild(0).getChild(0).getTag());

// clean view cannot be rendered after dirty
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(Tag.H1,
ui.wrapperElement.getChild(0).getChild(0).getTag());

// an error route cannot be rendered after dirty
ui.connectClient("foo", "bar", "/errr", "");
ui.connectClient("foo", "bar", "/errr", "", null);
assertEquals(Tag.H1,
ui.wrapperElement.getChild(0).getChild(0).getTag());
}

@Test
public void should_remove_content_on_leaveNavigation() {
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
ui.wrapperElement.getChild(0).getChild(0).getTag());
Expand All @@ -361,7 +361,7 @@ public void should_remove_content_on_leaveNavigation() {

@Test
public void should_keep_content_on_leaveNavigation_postpone() {
ui.connectClient("foo", "bar", "/dirty", "");
ui.connectClient("foo", "bar", "/dirty", "", null);
assertEquals(Tag.SPAN, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H1,
ui.wrapperElement.getChild(0).getChild(0).getTag());
Expand All @@ -375,30 +375,31 @@ public void should_keep_content_on_leaveNavigation_postpone() {
@Test
public void should_handle_forward_to_client_side_view_on_beforeEnter() {
ui.connectClient("foo", "bar", "/forwardToClientSideViewOnBeforeEnter",
"");
"", null);

assertEquals("client-view", ui.getForwardToClientUrl());
}

@Test
public void should_not_handle_forward_to_client_side_view_on_beforeLeave() {
ui.connectClient("foo", "bar", "/forwardToClientSideViewOnBeforeLeave",
"");
"", null);

assertNull(ui.getForwardToClientUrl());
}

@Test
public void should_not_handle_forward_to_client_side_view_on_reroute() {
ui.connectClient("foo", "bar", "/forwardToClientSideViewOnReroute", "");
ui.connectClient("foo", "bar", "/forwardToClientSideViewOnReroute", "",
null);

assertNull(ui.getForwardToClientUrl());
}

@Test
public void should_handle_forward_to_server_side_view_on_beforeEnter_and_update_url() {
ui.connectClient("foo", "bar", "/forwardToServerSideViewOnBeforeEnter",
"");
"", null);

assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
Expand All @@ -417,7 +418,7 @@ public void should_handle_forward_to_server_side_view_on_beforeEnter_and_update_

@Test
public void should_show_error_page() {
ui.connectClient("foo", "bar", "/err", "");
ui.connectClient("foo", "bar", "/err", "", null);
assertEquals(Tag.DIV, ui.wrapperElement.getChild(0).getTag());
assertTrue(ui.wrapperElement.toString().contains("Available routes:"));
}
Expand All @@ -433,7 +434,7 @@ public void should_initializeUI_when_wrapperElement_null() {

@Test
public void should_navigate_when_server_routing() {
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
ui.wrapperElement.getChild(0).getChild(0).getTag());
Expand Down Expand Up @@ -538,7 +539,7 @@ public void should_not_notify_clientRoute_when_navigatingToTheSame() {

@Test
public void server_should_not_doClientRoute_when_navigatingToServer() {
ui.connectClient("foo", "bar", "/clean", "");
ui.connectClient("foo", "bar", "/clean", "", null);
assertEquals(Tag.HEADER, ui.wrapperElement.getChild(0).getTag());
assertEquals(Tag.H2,
ui.wrapperElement.getChild(0).getChild(0).getTag());
Expand Down Expand Up @@ -581,22 +582,23 @@ public void should_removeTitle_when_noAppShellTitle() {

@Test
public void should_restoreIndexHtmlTitle() {
ui.connectClient("foo", "bar", "empty", "app-shell-title");
ui.connectClient("foo", "bar", "empty", "app-shell-title", null);
assertEquals("", ui.getInternals().getTitle());
ui.connectClient("foo", "bar", "dirty", "app-shell-title");
ui.connectClient("foo", "bar", "dirty", "app-shell-title", null);
assertEquals("app-shell-title", ui.getInternals().getTitle());
}

@Test
public void should_not_share_dynamic_app_title_for_different_UIs() {
String dynamicTitle = UUID.randomUUID().toString();
ui.connectClient("foo", "bar", "clean", dynamicTitle);
ui.connectClient("foo", "bar", "clean", dynamicTitle, null);
assertEquals(dynamicTitle, ui.getInternals().getTitle());

String anotherDynamicTitle = UUID.randomUUID().toString();
JavaScriptBootstrapUI anotherUI = new JavaScriptBootstrapUI();
anotherUI.getInternals().setSession(mocks.getSession());
anotherUI.connectClient("foo", "bar", "clean", anotherDynamicTitle);
anotherUI.connectClient("foo", "bar", "clean", anotherDynamicTitle,
null);
assertEquals(anotherDynamicTitle, anotherUI.getInternals().getTitle());

ui.navigate("dirty");
Expand Down
2 changes: 1 addition & 1 deletion ...erver/src/test/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ public void should_use_client_routing_when_there_is_a_router_call()
Boolean.FALSE);

((JavaScriptBootstrapUI) UI.getCurrent()).connectClient("foo", "bar",
"/foo", "");
"/foo", "", null);

Mockito.verify(session, Mockito.times(1)).setAttribute(SERVER_ROUTING,
Boolean.FALSE);
Expand Down
2 changes: 1 addition & 1 deletion ...er/src/test/java/com/vaadin/flow/server/communication/JavaScriptBootstrapHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void should_attachViewTo_UiContainer() throws Exception {
jsInitHandler.handleRequest(session, request, response);

JavaScriptBootstrapUI ui = (JavaScriptBootstrapUI) UI.getCurrent();
ui.connectClient("a-tag", "an-id", "a-route", "");
ui.connectClient("a-tag", "an-id", "a-route", "", null);

TestNodeVisitor visitor = new TestNodeVisitor(true);
BasicElementStateProvider.get().visit(ui.getElement().getNode(),
Expand Down
Loading

0 comments on commit c0ce21b

Please sign in to comment.