Skip to content

Commit

Permalink
fix: forward/reroute when query parameters changed (#17659) (CP: 24.1) (
Browse files Browse the repository at this point in the history 
#17691)

* fix: forward/reroute when query parameters changed (#17659)

Forward and reroute should run even when targeting the same target
but with different query parameters.

Fixes #17639

* replace not back-ported api in test

---------

Co-authored-by: Marco Collovati <marco@vaadin.com>
  • Loading branch information
@vaadin-bot @mcollovati
vaadin-bot and mcollovati committed Sep 22, 2023
1 parent 2703d04 commit de5f7c9
Show file tree
Hide file tree
Showing 2 changed files with 250 additions and 15 deletions.
11 changes: 9 additions & 2 deletions ...server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,18 +658,25 @@ protected Optional<Integer> handleTriggeredBeforeEvent(
if (beforeEvent.hasExternalForwardUrl()) {
return Optional.of(forwardToExternalUrl(event, beforeEvent));
}

boolean queryParameterChanged = beforeEvent.hasRedirectQueryParameters()
&& !beforeEvent.getRedirectQueryParameters()
.equals(event.getLocation().getQueryParameters());

if (beforeEvent.hasForwardTarget() && (!isSameNavigationState(
beforeEvent.getForwardTargetType(),
beforeEvent.getForwardTargetRouteParameters())
|| queryParameterChanged
|| !(navigationState.getResolvedPath() != null
&& navigationState.getResolvedPath()
.equals(beforeEvent.getForwardUrl())))) {
return Optional.of(forward(event, beforeEvent));
}

if (beforeEvent.hasRerouteTarget()
&& !isSameNavigationState(beforeEvent.getRerouteTargetType(),
beforeEvent.getRerouteTargetRouteParameters())) {
&& (!isSameNavigationState(beforeEvent.getRerouteTargetType(),
beforeEvent.getRerouteTargetRouteParameters())
|| queryParameterChanged)) {
return Optional.of(reroute(event, beforeEvent));
}

Expand Down
254 changes: 241 additions & 13 deletions flow-server/src/test/java/com/vaadin/flow/router/RouterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import net.jcip.annotations.NotThreadSafe;

import org.hamcrest.MatcherAssert;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.ComponentUtil;
import com.vaadin.flow.component.HasComponents;
Expand All @@ -63,9 +52,18 @@
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.flow.shared.Registration;

import elemental.json.Json;
import elemental.json.JsonObject;
import net.jcip.annotations.NotThreadSafe;
import org.hamcrest.MatcherAssert;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import static com.vaadin.flow.router.internal.RouteModelTest.parameters;
import static com.vaadin.flow.router.internal.RouteModelTest.varargs;
import static org.hamcrest.CoreMatchers.is;
Expand Down Expand Up @@ -466,6 +464,70 @@ public void beforeEnter(BeforeEnterEvent event) {
}
}

@Route("rerouteWithQueryParams")
@Tag(Tag.DIV)
public static class RerouteWithQueryParams extends RouteParametersBase
implements BeforeEnterObserver {

static int events = 0;

static void clear() {
RouteParametersBase.clear();
events = 0;
}

@Override
public void beforeEnter(BeforeEnterEvent event) {
events++;
super.beforeEnter(event);
QueryParameters queryParameters = event.getLocation()
.getQueryParameters();
String updateQueryParams = queryParameters.getParameters()
.getOrDefault("updateQueryParams", Collections.emptyList())
.stream().findFirst().orElse(null);
QueryParameters newParams = QueryParameters.of("newParam", "hello");
if (queryParameters.getParameters().isEmpty()) {
event.rerouteTo("show", newParams);
} else if ("true".equals(updateQueryParams)) {
event.rerouteTo("rerouteWithQueryParams", newParams);
} else if ("false".equals(updateQueryParams)) {
event.rerouteTo("rerouteWithQueryParams", queryParameters);
}
}
}

@Route("forwardWithQueryParams")
@Tag(Tag.DIV)
public static class ForwardWithQueryParams extends RouteParametersBase
implements BeforeEnterObserver {

static int events = 0;

static void clear() {
RouteParametersBase.clear();
events = 0;
}

@Override
public void beforeEnter(BeforeEnterEvent event) {
events++;
super.beforeEnter(event);
QueryParameters queryParameters = event.getLocation()
.getQueryParameters();
String updateQueryParams = queryParameters.getParameters()
.getOrDefault("updateQueryParams", Collections.emptyList())
.stream().findFirst().orElse(null);
QueryParameters newParams = QueryParameters.of("newParam", "hello");
if (queryParameters.getParameters().isEmpty()) {
event.forwardTo("show", newParams);
} else if ("true".equals(updateQueryParams)) {
event.forwardTo("forwardWithQueryParams", newParams);
} else if ("false".equals(updateQueryParams)) {
event.forwardTo("forwardWithQueryParams", queryParameters);
}
}
}

@Route("redirect/to/param")
@Tag(Tag.DIV)
public static class RedirectToRouteWithParamInUrl extends Component
Expand Down Expand Up @@ -1270,7 +1332,7 @@ public void afterNavigation(AfterNavigationEvent event) {
* children components used in the assertion of the event order, as being
* children of the layout in the chain instead of being part of the layout
* chain itself.
*
* <p>
* So any children of an instance of this class should receive the
* navigation events right after the instance of this class receives them
* and in the order they are added.
Expand Down Expand Up @@ -1736,6 +1798,8 @@ public void init() throws NoSuchFieldException, SecurityException,
@After
public void tearDown() {
CurrentInstance.clearAll();
ForwardWithQueryParams.clear();
RerouteWithQueryParams.clear();
}

@Rule
Expand Down Expand Up @@ -4159,6 +4223,170 @@ public void forwardToExternalUrl_forwardsToUrl() {

}

@Test
public void reroute_queryParameters()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(RerouteWithQueryParams.class, ShowAllView.class);

router.navigate(ui, new Location("rerouteWithQueryParams"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected reroute to ShowAll view but was "
+ RouteParametersBase.target,
ShowAllView.class, RouteParametersBase.target);
Assert.assertEquals("Expecting reroute view to be entered once", 1,
RerouteWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("newParam", Collections.emptyList()).stream()
.findFirst().orElse(null);
Assert.assertEquals("Missing parameter after reroute", "hello",
singleParameter);
}

@Test
public void reroute_queryParameters_sameNavigationTarget()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(RerouteWithQueryParams.class, ShowAllView.class);

router.navigate(ui,
new Location("rerouteWithQueryParams?updateQueryParams=true"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected reroute to same view but was "
+ RouteParametersBase.target,
RerouteWithQueryParams.class, RouteParametersBase.target);
Assert.assertEquals("Expecting reroute view to be entered twice", 2,
RerouteWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("newParam", Collections.emptyList()).stream()
.findFirst().orElse(null);
Assert.assertEquals("Missing parameter after reroute", "hello",
singleParameter);
Assert.assertTrue(
"Expecting original parameter not be present after reroute",
RouteParametersBase.queryParameters.getParameters()
.getOrDefault("updateQueryParams",
Collections.emptyList())
.isEmpty());
}

@Test
public void reroute_sameQueryParameters_sameNavigationTarget()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(RerouteWithQueryParams.class, ShowAllView.class);

router.navigate(ui,
new Location("rerouteWithQueryParams?updateQueryParams=false"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected reroute to same view but was "
+ RouteParametersBase.target,
RerouteWithQueryParams.class, RouteParametersBase.target);
Assert.assertEquals("Expecting reroute view to be entered once", 1,
RerouteWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("updateQueryParams", Collections.emptyList())
.stream().findFirst().orElse(null);
Assert.assertEquals("Expecting original parameter after reroute",
"false", singleParameter);
Assert.assertTrue(
"Expecting new parameter not to be present after reroute",
RouteParametersBase.queryParameters.getParameters()
.getOrDefault("newParam", Collections.emptyList())
.isEmpty());
}

@Test
public void forward_queryParameters()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(ForwardWithQueryParams.class, ShowAllView.class);

router.navigate(ui, new Location("forwardWithQueryParams"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected forward to ShowAll view but was "
+ RouteParametersBase.target,
ShowAllView.class, RouteParametersBase.target);
Assert.assertEquals("Expecting forward view to be entered once", 1,
ForwardWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("newParam", Collections.emptyList()).stream()
.findFirst().orElse(null);
Assert.assertEquals("Missing query parameter after forward", "hello",
singleParameter);
}

@Test
public void forward_queryParameters_sameNavigationTarget()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(ForwardWithQueryParams.class, ShowAllView.class);

router.navigate(ui,
new Location("forwardWithQueryParams?updateQueryParams=true"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected forward to same view but was "
+ RouteParametersBase.target,
ForwardWithQueryParams.class, RouteParametersBase.target);
Assert.assertEquals("Expecting forward view to be entered twice", 2,
ForwardWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("newParam", Collections.emptyList()).stream()
.findFirst().orElse(null);
Assert.assertEquals("Missing query parameter after forward", "hello",
singleParameter);
Assert.assertTrue(
"Expecting original parameter not be present after forward",
RouteParametersBase.queryParameters.getParameters()
.getOrDefault("updateQueryParams",
Collections.emptyList())
.isEmpty());
}

@Test
public void forward_sameQueryParameters_sameNavigationTarget()
throws InvalidRouteConfigurationException {
RouteParametersBase.clear();
setNavigationTargets(ForwardWithQueryParams.class, ShowAllView.class);

router.navigate(ui,
new Location("forwardWithQueryParams?updateQueryParams=false"),
NavigationTrigger.PROGRAMMATIC);

Assert.assertEquals(
"Expected forward to same view but was "
+ RouteParametersBase.target,
ForwardWithQueryParams.class, RouteParametersBase.target);
Assert.assertEquals("Expecting forward view to be entered once", 1,
ForwardWithQueryParams.events);
String singleParameter = RouteParametersBase.queryParameters
.getParameters()
.getOrDefault("updateQueryParams", Collections.emptyList())
.stream().findFirst().orElse(null);
Assert.assertEquals("Missing original query parameter after forward",
"false", singleParameter);
Assert.assertTrue(
"Expecting new parameter not be present after forward",
RouteParametersBase.queryParameters.getParameters()
.getOrDefault("newParam", Collections.emptyList())
.isEmpty());
}

private void assertWrongRouteParametersRedirect() {
assertRouteParameters("show/wrong", null, null);
}
Expand Down

0 comments on commit de5f7c9

Please sign in to comment.