Skip to content

Commit

Permalink
chore: improve route registry update logics
Browse files Browse the repository at this point in the history 
Part of #19261
  • Loading branch information
@mcollovati
mcollovati committed May 22, 2024
1 parent 42b76a4 commit 25be21e
Show file tree
Hide file tree
Showing 2 changed files with 319 additions and 21 deletions.
87 changes: 66 additions & 21 deletions flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.vaadin.flow.router.RoutePrefix;
import com.vaadin.flow.router.RouterLayout;
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.SessionRouteRegistry;
import com.vaadin.flow.server.VaadinContext;

/**
Expand Down Expand Up @@ -315,31 +316,75 @@ public static void updateRouteRegistry(RouteRegistry registry,

Logger logger = LoggerFactory.getLogger(RouteUtil.class);

// unhandled cases for modified class
// - modified/removed class is not anymore a Component:
// route will not be removed from registry
// - modified classes and session registry
// routes have been added manually and we do not have enough information
// to decide if the route should be re-added with the path from the
// annotation or if it should be left unchanged
//
// potentially breaking cases
// - not annotated classes manually added to the registry:
// if the class is modified the route will be removed from the registry

boolean isSessionRegistry = registry instanceof SessionRouteRegistry;

// routes for modified classes should be removed
// - @Route annotation has been removed
// - @Route annotation is present but registerAtStartup=false
// for session registries a modified route should never be removed
// because we don't know how it has been registered
Stream<Class<? extends Component>> toRemove = Stream
.concat(filterComponentClasses(deletedClasses),
filterComponentClasses(modifiedClasses)
.filter(clazz -> !isSessionRegistry)
.filter(clazz -> !clazz
.isAnnotationPresent(Route.class)
|| !clazz.getAnnotation(Route.class)
.registerAtStartup()))
.distinct();

Stream<Class<? extends Component>> toAdd;
if (isSessionRegistry) {
// routes on session registry are initialized programmatically so
// new classes should never be added automatically
toAdd = Stream.empty();
} else {
// New classes should be added to the registry only if they have a
// @Route annotation with registerAtStartup=true
toAdd = Stream
.concat(filterComponentClasses(addedClasses),
filterComponentClasses(modifiedClasses))
.filter(clazz -> clazz.isAnnotationPresent(Route.class)
&& clazz.getAnnotation(Route.class)
.registerAtStartup())
.distinct();
}

registry.update(() -> {
// remove deleted classes and classes that lost the annotation from
// registry
Stream.concat(deletedClasses.stream(),
modifiedClasses.stream().filter(
clazz -> !clazz.isAnnotationPresent(Route.class)))
.filter(Component.class::isAssignableFrom)
.forEach(clazz -> {
Class<? extends Component> componentClass = (Class<? extends Component>) clazz;
logger.debug("Removing route to {}", componentClass);
routeConf.removeRoute(componentClass);
});
toRemove.forEach(componentClass -> {
logger.debug("Removing route to {}", componentClass);
routeConf.removeRoute(componentClass);
});
// add new routes to registry
Stream.concat(addedClasses.stream(), modifiedClasses.stream())
.distinct().filter(Component.class::isAssignableFrom)
.filter(clazz -> clazz.isAnnotationPresent(Route.class))
.forEach(clazz -> {
Class<? extends Component> componentClass = (Class<? extends Component>) clazz;
logger.debug(
"Updating route {} to {}", componentClass
.getAnnotation(Route.class).value(),
clazz);
routeConf.removeRoute(componentClass);
routeConf.setAnnotatedRoute(componentClass);
});
toAdd.forEach(componentClass -> {
logger.debug("Updating route {} to {}",
componentClass.getAnnotation(Route.class).value(),
componentClass);
routeConf.removeRoute(componentClass);
routeConf.setAnnotatedRoute(componentClass);
});
});
}

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

}
253 changes: 253 additions & 0 deletions flow-server/src/test/java/com/vaadin/flow/router/internal/RouteUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
import com.vaadin.flow.router.RouterLayout;
import com.vaadin.flow.server.MockVaadinContext;
import com.vaadin.flow.server.MockVaadinServletService;
import com.vaadin.flow.server.SessionRouteRegistry;
import com.vaadin.flow.server.VaadinContext;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.tests.util.AlwaysLockedVaadinSession;

/**
* Test that {@link RouteUtil} route resolving works as intended for both simple
Expand Down Expand Up @@ -459,6 +461,119 @@ public VaadinContext getContext() {
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));
}

@Test
public void newRouteAnnotatedClass_sessionRegistry_updateRouteRegistry_routeIsNotAddedToRegistry() {
// given
@Route("a")
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
SessionRouteRegistry registry = (SessionRouteRegistry) SessionRouteRegistry
.getSessionRegistry(new AlwaysLockedVaadinSession(service));

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.emptySet(), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void newComponentClass_sessionRegistry_updateRouteRegistry_routeIsNotAddedToRegistry() {
// given
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
SessionRouteRegistry registry = (SessionRouteRegistry) SessionRouteRegistry
.getSessionRegistry(new AlwaysLockedVaadinSession(service));

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.emptySet(), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void newLazyRouteAnnotatedClass_updateRouteRegistry_routeIsNotAddedToRegistry() {
// given
@Route(value = "a", registerAtStartup = false)
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.emptySet(), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void newLazyRouteAnnotatedClass_sessionRegistry_updateRouteRegistry_routeIsNotAddedToRegistry() {
// given
@Route(value = "a", registerAtStartup = false)
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
SessionRouteRegistry registry = (SessionRouteRegistry) SessionRouteRegistry
.getSessionRegistry(new AlwaysLockedVaadinSession(service));

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.emptySet(), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void newRouteComponentWithoutRouteAnnotation_updateRouteRegistry_routeIsNotAddedToRegistry() {
// given
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.emptySet(), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void deletedRouteAnnotatedClass_updateRouteRegistry_routeIsRemovedFromRegistry() {
// given
Expand All @@ -481,6 +596,68 @@ class A extends Component {
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void deletedNotAnnotatedRouteClass_updateRouteRegistry_routeIsRemovedFromRegistry() {
// given
class A extends Component {
}

MockVaadinServletService service = new MockVaadinServletService();

ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());
registry.setRoute("a", A.class, Collections.emptyList());
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));

// when
RouteUtil.updateRouteRegistry(registry, Collections.emptySet(),
Collections.emptySet(), Collections.singleton(A.class));

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void deletedRouteAnnotatedClass_sessionRegistry_updateRouteRegistry_routeIsRemovedFromRegistry() {
// given
@Route("a")
class A extends Component {
}

MockVaadinServletService service = new MockVaadinServletService();
SessionRouteRegistry registry = (SessionRouteRegistry) SessionRouteRegistry
.getSessionRegistry(new AlwaysLockedVaadinSession(service));
registry.setRoute("a", A.class, Collections.emptyList());
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));

// when
RouteUtil.updateRouteRegistry(registry, Collections.emptySet(),
Collections.emptySet(), Collections.singleton(A.class));

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void deletedNotAnnotatedRouteClass_sessionRegistry_updateRouteRegistry_routeIsRemovedFromRegistry() {
// given
class A extends Component {
}

MockVaadinServletService service = new MockVaadinServletService();
SessionRouteRegistry registry = (SessionRouteRegistry) SessionRouteRegistry
.getSessionRegistry(new AlwaysLockedVaadinSession(service));
registry.setRoute("a", A.class, Collections.emptyList());
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));

// when
RouteUtil.updateRouteRegistry(registry, Collections.emptySet(),
Collections.emptySet(), Collections.singleton(A.class));

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void renamedRouteAnnotatedClass_updateRouteRegistry_routeIsUpdatedInRegistry() {
// given
Expand All @@ -507,6 +684,31 @@ public VaadinContext getContext() {
Assert.assertTrue(registry.getConfiguration().hasRoute("aa"));
}

@Test
public void changedToLazyRouteAnnotatedClass_updateRouteRegistry_routeIsRemovedInRegistry() {
// given
@Route(value = "a", registerAtStartup = false)
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());
registry.setRoute("a", A.class, Collections.emptyList());
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));

// when
RouteUtil.updateRouteRegistry(registry, Collections.emptySet(),
Collections.singleton(A.class), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

@Test
public void deannotatedRouteClass_updateRouteRegistry_routeIsRemovedFromRegistry() {
// given
Expand All @@ -527,4 +729,55 @@ class A extends Component {
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
}

// Hotswap agent may fire CREATE, MODIFY and REMOVE events for a class
// change. CREATE wins over REMOVE
@Test
public void routeAnnotatedClassAddedModifiedAndRemoved_updateRouteRegistry_routeIsAddedToRegistry() {
// given
@Route("a")
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());

// when
RouteUtil.updateRouteRegistry(registry, Collections.singleton(A.class),
Collections.singleton(A.class), Collections.singleton(A.class));

// then
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));
}

@Test
public void sessionRegistryWithManualRegisteredRouteClass_updateRouteRegistry_routeIsUpdatedInRegistry() {
// given
@Route("aa")
class A extends Component {
}
MockVaadinServletService service = new MockVaadinServletService() {
@Override
public VaadinContext getContext() {
return new MockVaadinContext();
}
};
ApplicationRouteRegistry registry = ApplicationRouteRegistry
.getInstance(service.getContext());
registry.setRoute("a", A.class, Collections.emptyList());
Assert.assertTrue(registry.getConfiguration().hasRoute("a"));

// when
RouteUtil.updateRouteRegistry(registry, Collections.emptySet(),
Collections.singleton(A.class), Collections.emptySet());

// then
Assert.assertFalse(registry.getConfiguration().hasRoute("a"));
Assert.assertTrue(registry.getConfiguration().hasRoute("aa"));
}

}

0 comments on commit 25be21e

Please sign in to comment.