Skip to content

Commit

Permalink
fix: reload route configuration upon layout changes
Browse files Browse the repository at this point in the history
When a [At]Layout annotated class is modified, the changes are not
propagated to the route registry after hotswap happens.
This change updates Route registry layouts configuration and re-registers
routes potentially impacted by the change to apply the new settings.
It also checks the route target chain for active UIs in order to
trigger a page refresh if the layout changes should be applied.

Fixes #20111
  • Loading branch information
mcollovati committed Oct 3, 2024
1 parent 1ebd400 commit ca5c04a
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 3 deletions.
15 changes: 15 additions & 0 deletions flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.internal.BrowserLiveReload;
import com.vaadin.flow.internal.BrowserLiveReloadAccessor;
import com.vaadin.flow.router.internal.RouteTarget;
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.ServiceDestroyEvent;
import com.vaadin.flow.server.ServiceDestroyListener;
import com.vaadin.flow.server.ServiceException;
Expand Down Expand Up @@ -309,6 +311,19 @@ private UIRefreshStrategy computeRefreshStrategy(UI ui,
refreshStrategy = computeRefreshStrategyForUITree(ui,
changedClasses, targetsChain, route);
}
// A different layout might have been applied after hotswap
if (refreshStrategy == UIRefreshStrategy.SKIP) {
RouteRegistry registry = ui.getInternals().getRouter()
.getRegistry();
RouteTarget routeTarget = registry
.getNavigationRouteTarget(
ui.getActiveViewLocation().getPath())
.getRouteTarget();
if (routeTarget != null && routeTarget.getParentLayouts().stream()
.anyMatch(changedClasses::contains)) {
refreshStrategy = UIRefreshStrategy.PUSH_REFRESH_CHAIN;
}
}

// If push is not enabled we can only request a full page refresh
if (refreshStrategy != UIRefreshStrategy.SKIP && !pushEnabled) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.vaadin.flow.internal.ReflectTools;
import com.vaadin.flow.router.BeforeEnterListener;
import com.vaadin.flow.router.HasErrorParameter;
import com.vaadin.flow.router.Layout;
import com.vaadin.flow.router.Menu;
import com.vaadin.flow.router.MenuData;
import com.vaadin.flow.router.NotFoundException;
Expand All @@ -55,7 +56,6 @@
import com.vaadin.flow.server.auth.NavigationAccessControl;
import com.vaadin.flow.server.auth.NavigationContext;
import com.vaadin.flow.server.auth.ViewAccessChecker;
import com.vaadin.flow.router.Layout;
import com.vaadin.flow.server.menu.MenuRegistry;
import com.vaadin.flow.shared.Registration;

Expand Down Expand Up @@ -610,6 +610,19 @@ public void setLayout(Class<? extends RouterLayout> layout) {
}
}

void updateLayout(Class<? extends RouterLayout> layout) {
if (layout == null) {
return;
}
synchronized (layouts) {
layouts.entrySet()
.removeIf(entry -> layout.equals(entry.getValue()));
if (layout.isAnnotationPresent(Layout.class)) {
layouts.put(layout.getAnnotation(Layout.class).value(), layout);
}
}
}

@Override
public Class<? extends RouterLayout> getLayout(String path) {
Optional<String> first = layouts.keySet().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.vaadin.flow.router.ParentLayout;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouteAlias;
import com.vaadin.flow.router.RouteBaseData;
import com.vaadin.flow.router.RouteConfiguration;
import com.vaadin.flow.router.RoutePathProvider;
import com.vaadin.flow.router.RoutePrefix;
Expand Down Expand Up @@ -125,7 +126,7 @@ public static List<Class<? extends RouterLayout>> getParentLayouts(

if (hasRouteAndPathMatches && !route.get().layout().equals(UI.class)) {
list.addAll(collectRouteParentLayouts(route.get().layout()));
} else if (route.get().autoLayout() && hasRouteAndPathMatches
} else if (hasRouteAndPathMatches && route.get().autoLayout()
&& route.get().layout().equals(UI.class)
&& handledRegistry.hasLayout(path)) {
list.addAll(
Expand Down Expand Up @@ -435,18 +436,39 @@ public static void updateRouteRegistry(RouteRegistry registry,
modifiedClasses.stream()
.filter(clazz -> !Component.class.isAssignableFrom(clazz))
.forEach(nonFlowComponentsToRemove::add);
Set<Class<? extends RouterLayout>> layouts = new HashSet<>();

boolean isSessionRegistry = registry instanceof SessionRouteRegistry;
Predicate<Class<? extends Component>> modifiedClassesRouteRemovalFilter = clazz -> !isSessionRegistry;

if (registry instanceof AbstractRouteRegistry abstractRouteRegistry) {

// update layouts
filterLayoutClasses(deletedClasses).forEach(layouts::add);
filterLayoutClasses(modifiedClasses).forEach(layouts::add);
filterLayoutClasses(addedClasses).forEach(layouts::add);
layouts.forEach(abstractRouteRegistry::updateLayout);
if (!layouts.isEmpty()) {
// Gather routes that don't have a layout or reference a layout
// that has been changed.
// Mark these routes as modified so they can be re-registered
// with the correct layouts applied.
registry.getRegisteredRoutes().stream()
.filter(rd -> rd.getParentLayouts().isEmpty()
|| rd.getParentLayouts().stream()
.anyMatch(layouts::contains))
.map(RouteBaseData::getNavigationTarget)
.forEach(modifiedClasses::add);
}

Map<String, RouteTarget> routesMap = abstractRouteRegistry
.getConfiguration().getRoutesMap();
Map<? extends Class<? extends Component>, RouteTarget> routeTargets = registry
.getRegisteredRoutes().stream()
.map(routeData -> routesMap.get(routeData.getTemplate()))
.filter(Objects::nonNull).collect(Collectors.toMap(
RouteTarget::getTarget, Function.identity()));

modifiedClassesRouteRemovalFilter = modifiedClassesRouteRemovalFilter
.and(clazz -> {
RouteTarget routeTarget = routeTargets.get(clazz);
Expand Down Expand Up @@ -522,6 +544,14 @@ public static void updateRouteRegistry(RouteRegistry registry,
});
}

@SuppressWarnings("unchecked")
private static Stream<Class<? extends RouterLayout>> filterLayoutClasses(
Set<Class<?>> classes) {
return filterComponentClasses(classes)
.filter(RouterLayout.class::isAssignableFrom)
.map(clazz -> (Class<? extends RouterLayout>) clazz);
}

@SuppressWarnings("unchecked")
private static Stream<Class<? extends Component>> filterComponentClasses(
Set<Class<?>> classes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.internal.BrowserLiveReload;
import com.vaadin.flow.internal.BrowserLiveReloadAccessor;
import com.vaadin.flow.router.RouteConfiguration;
import com.vaadin.flow.router.RouterLayout;
import com.vaadin.flow.server.MockVaadinServletService;
import com.vaadin.flow.server.MockVaadinSession;
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.ServiceDestroyEvent;
import com.vaadin.flow.server.ServiceDestroyListener;
import com.vaadin.flow.server.ServiceException;
Expand Down Expand Up @@ -281,6 +283,32 @@ public void onHotswap_pushDisabled_routeLayoutClassChanged_UINotRefreshedButLive
Mockito.verify(liveReload).refresh(anyBoolean());
}

@Test
public void onHotswap_pushDisabled_routeTargetChainChanged_UINotRefreshedButLiveReloadTriggered()
throws ServiceException {
VaadinSession session = createMockVaadinSession();
hotswapper.sessionInit(new SessionInitEvent(service, session, null));

class NewLayout extends Component implements RouterLayout {
}
RefreshTestingUI ui = initUIAndNavigateTo(session, MyRoute.class,
MyLayout.class);
RouteRegistry registry = ui.getInternals().getRouter().getRegistry();
RouteConfiguration routeConfiguration = RouteConfiguration
.forRegistry(registry);
routeConfiguration.update(() -> {
String path = ui.getActiveViewLocation().getPath();
routeConfiguration.removeRoute(path);
routeConfiguration.setRoute(path, MyRoute.class,
List.of(NewLayout.class));
});

hotswapper.onHotswap(new String[] { NewLayout.class.getName() }, true);

ui.assertNotRefreshed();
Mockito.verify(liveReload).refresh(anyBoolean());
}

@Test
public void onHotswap_pushDisabled_routeChildClassChanged_UINotRefreshedButLiveReloadTriggered()
throws ServiceException {
Expand Down Expand Up @@ -389,6 +417,34 @@ public void onHotswap_pushEnabled_routeLayoutClassChanged_activeChainRefreshed()
Mockito.verify(liveReload, never()).refresh(anyBoolean());
}

@Test
public void onHotswap_pushEnabled_routeTargetChainChanged_activeChainRefreshed()
throws ServiceException {
VaadinSession session = createMockVaadinSession();
hotswapper.sessionInit(new SessionInitEvent(service, session, null));

class NewLayout extends Component implements RouterLayout {
}
RefreshTestingUI ui = initUIAndNavigateTo(session, MyRoute.class,
MyLayout.class);
ui.enablePush();
RouteRegistry registry = ui.getInternals().getRouter().getRegistry();
RouteConfiguration routeConfiguration = RouteConfiguration
.forRegistry(registry);
routeConfiguration.update(() -> {
String path = ui.getActiveViewLocation().getPath();
routeConfiguration.removeRoute(path);
routeConfiguration.setRoute(path, MyRoute.class,
List.of(NewLayout.class));
});

hotswapper.onHotswap(new String[] { NewLayout.class.getName() }, true);

ui.assertChainRefreshed();
Mockito.verify(liveReload, never()).reload();
Mockito.verify(liveReload, never()).refresh(anyBoolean());
}

@Test
public void onHotswap_pushEnabled_routeChildrenClassChanged_routeRefreshed()
throws ServiceException {
Expand Down
Loading

0 comments on commit ca5c04a

Please sign in to comment.