fix: unwrap router target class in getRouteTarget (#19151)

AbstractNavigationStateRenderer.getRouteTarget filters the active router
targets chain comparing collection items class with the route target type.
The filter may fail if routes instances are proxy objects created by frameworks
like spring, since the class will not match the required one.
This cahnge ensures that synthetic and proxy classes are discarded,
allowing for more accurate comparison when filtering objects based on their
real application class.

Fixes #19100

flow-server/src/main/java/com/vaadin/flow/di/Instantiator.java

@@ -16,6 +16,7 @@
 package com.vaadin.flow.di;
 
 import java.io.Serializable;
+import java.util.Objects;
 import java.util.ServiceLoader;
 import java.util.stream.Stream;
 
@@ -116,6 +117,38 @@ default Stream<DependencyFilter> getDependencyFilters(
      */
     <T> T getOrCreate(Class<T> type);
 
+    /**
+     * Return the application-defined class for the given instance: usually
+     * simply the class of the given instance, but the original class in case of
+     * a runtime generated subclass.
+     *
+     * @param instance
+     *            the instance to check
+     * @return the user-defined class
+     */
+    default Class<?> getApplicationClass(Object instance) {
+        Objects.requireNonNull(instance, "Instance cannot be null");
+        return getApplicationClass(instance.getClass());
+    }
+
+    /**
+     * Return the application-defined class for the given class: usually simply
+     * the given class, but the original class in case of a runtime generated
+     * subclass.
+     *
+     * @param clazz
+     *            the class to check
+     * @return the user-defined class
+     */
+    default Class<?> getApplicationClass(Class<?> clazz) {
+        Class<?> appClass = clazz;
+        while (appClass != null && appClass != Object.class
+                && appClass.isSynthetic()) {
+            appClass = appClass.getSuperclass();
+        }
+        return appClass;
+    }
+
     /**
      * Creates an instance of a navigation target or router layout. This method
      * is not called in cases when a component instance is reused when

flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java

@@ -123,13 +123,14 @@ public NavigationState getNavigationState() {
     static <T extends HasElement> T getRouteTarget(Class<T> routeTargetType,
             NavigationEvent event) {
         UI ui = event.getUI();
+        Instantiator instantiator = Instantiator.get(ui);
         Optional<HasElement> currentInstance = ui.getInternals()
                 .getActiveRouterTargetsChain().stream()
-                .filter(component -> component.getClass()
+                .filter(component -> instantiator.getApplicationClass(component)
                         .equals(routeTargetType))
                 .findAny();
-        return (T) currentInstance.orElseGet(() -> Instantiator.get(ui)
-                .createRouteTarget(routeTargetType, event));
+        return (T) currentInstance.orElseGet(
+                () -> instantiator.createRouteTarget(routeTargetType, event));
     }
 
     @Override

flow-server/src/test/java/com/vaadin/flow/di/DefaultInstantiatorTest.java

@@ -18,6 +18,10 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import net.bytebuddy.ByteBuddy;
+import net.bytebuddy.description.modifier.SyntheticState;
+import net.bytebuddy.description.modifier.Visibility;
+import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -75,6 +79,43 @@ public void getOrCreate_lookupHasNoObject_createNewObject() {
         Assert.assertNotNull(component);
     }
 
+    @Test
+    public void getApplicationClass_regularClass_getsSameClass() {
+        VaadinService service = Mockito.mock(VaadinService.class);
+        mockLookup(service);
+
+        DefaultInstantiator instantiator = new DefaultInstantiator(service);
+
+        TestComponent instance = instantiator.getOrCreate(TestComponent.class);
+        Assert.assertSame(TestComponent.class,
+                instantiator.getApplicationClass(instance));
+        Assert.assertSame(TestComponent.class,
+                instantiator.getApplicationClass(instance.getClass()));
+    }
+
+    @Test
+    public void getApplicationClass_syntheticClass_getsApplicationClass()
+            throws Exception {
+        VaadinService service = Mockito.mock(VaadinService.class);
+        mockLookup(service);
+        DefaultInstantiator instantiator = new DefaultInstantiator(service);
+
+        Class<? extends TestComponent> syntheticClass = new ByteBuddy()
+                .subclass(TestComponent.class)
+                .modifiers(Visibility.PUBLIC, SyntheticState.SYNTHETIC).make()
+                .load(getClass().getClassLoader(),
+                        ClassLoadingStrategy.Default.WRAPPER)
+                .getLoaded();
+        TestComponent instance = syntheticClass.getDeclaredConstructor()
+                .newInstance();
+
+        Assert.assertNotSame(TestComponent.class, instance.getClass());
+        Assert.assertSame(TestComponent.class,
+                instantiator.getApplicationClass(instance));
+        Assert.assertSame(TestComponent.class,
+                instantiator.getApplicationClass(instance.getClass()));
+    }
+
     private Lookup mockLookup(VaadinService service) {
         VaadinContext context = Mockito.mock(VaadinContext.class);
         Mockito.when(service.getContext()).thenReturn(context);

flow-server/src/test/java/com/vaadin/flow/router/internal/NavigationStateRendererTest.java

@@ -25,6 +25,10 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 
+import net.bytebuddy.ByteBuddy;
+import net.bytebuddy.description.modifier.SyntheticState;
+import net.bytebuddy.description.modifier.Visibility;
+import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
 import net.jcip.annotations.NotThreadSafe;
 import org.junit.Assert;
 import org.junit.Before;
@@ -43,6 +47,7 @@
 import com.vaadin.flow.component.page.Page;
 import com.vaadin.flow.component.page.PendingJavaScriptResult;
 import com.vaadin.flow.dom.Element;
+import com.vaadin.flow.internal.ReflectTools;
 import com.vaadin.flow.router.AfterNavigationEvent;
 import com.vaadin.flow.router.AfterNavigationObserver;
 import com.vaadin.flow.router.BeforeEnterEvent;
@@ -131,6 +136,13 @@ private static class PreservedNestedView extends Text {
         }
     }
 
+    @Route(value = "proxyable")
+    public static class ProxyableView extends Text {
+        public ProxyableView() {
+            super("");
+        }
+    }
+
     private Router router;
 
     @Rule
@@ -217,6 +229,51 @@ public <T extends HasElement> T createRouteTarget(
         UI.setCurrent(null);
     }
 
+    @Test
+    public void getRouteTarget_supportsProxyClasses() {
+        try {
+            Class<? extends ProxyableView> routeProxyClass = new ByteBuddy()
+                    .subclass(ProxyableView.class)
+                    .modifiers(Visibility.PUBLIC, SyntheticState.SYNTHETIC)
+                    .make().load(ProxyableView.class.getClassLoader(),
+                            ClassLoadingStrategy.Default.WRAPPER)
+                    .getLoaded();
+
+            AtomicInteger routeCreationCounter = new AtomicInteger(0);
+            MockVaadinServletService service = new MockVaadinServletService();
+            service.init(new MockInstantiator() {
+                @Override
+                public <T extends HasElement> T createRouteTarget(
+                        Class<T> routeTargetType, NavigationEvent event) {
+                    Assert.assertEquals(ProxyableView.class, routeTargetType);
+                    routeCreationCounter.incrementAndGet();
+                    return (T) ReflectTools.createInstance(routeProxyClass);
+                }
+            });
+            MockUI ui = new MockUI(new AlwaysLockedVaadinSession(service));
+
+            NavigationEvent event = new NavigationEvent(
+                    new Router(new TestRouteRegistry()), new Location("child"),
+                    ui, NavigationTrigger.UI_NAVIGATE);
+            NavigationStateRenderer renderer = new NavigationStateRenderer(
+                    navigationStateFromTarget(ProxyableView.class));
+            renderer.handle(event);
+            HasElement view = ui.getInternals().getActiveRouterTargetsChain()
+                    .get(0);
+
+            Component routeTarget = renderer.getRouteTarget(ProxyableView.class,
+                    event);
+
+            // Getting route target should not create a new instance
+            Assert.assertEquals(
+                    "Only one view instance should have been created", 1,
+                    routeCreationCounter.get());
+            Assert.assertSame(view, routeTarget);
+        } finally {
+            UI.setCurrent(null);
+        }
+    }
+
     @Route("parent")
     private static class RouteParentLayout extends Component
             implements RouterLayout {

flow-tests/vaadin-spring-tests/test-spring-boot/pom.xml

@@ -135,6 +135,7 @@
                                     <artifactId>vaadin-test-spring-common</artifactId>
                                     <version>${project.version}</version>
                                     <type>test-jar</type>
+                                    <classifier>tests</classifier>
                                     <outputDirectory>${project.build.directory}/test-classes</outputDirectory>
                                 </artifactItem>
                             </artifactItems>

flow-tests/vaadin-spring-tests/test-spring-boot/src/main/java/com/vaadin/flow/spring/test/TestServletInitializer.java

@@ -19,11 +19,13 @@
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
+import org.springframework.scheduling.annotation.EnableAsync;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 
 @SpringBootApplication
 @Configuration
 @EnableWebSecurity
+@EnableAsync(proxyTargetClass = true)
 @Import(DummyOAuth2Server.class)
 public class TestServletInitializer {
 

flow-tests/vaadin-spring-tests/test-spring-common/src/main/java/com/vaadin/flow/spring/test/ProxiedNavigationTarget.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2000-2024 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.spring.test;
+
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.springframework.context.annotation.Scope;
+import org.springframework.context.annotation.ScopedProxyMode;
+import org.springframework.scheduling.annotation.Async;
+import org.springframework.stereotype.Component;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.BeforeEvent;
+import com.vaadin.flow.router.HasUrlParameter;
+import com.vaadin.flow.router.OptionalParameter;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.router.RouterLink;
+
+import static com.vaadin.flow.spring.scopes.VaadinUIScope.VAADIN_UI_SCOPE_NAME;
+
+@Route("proxied")
+public class ProxiedNavigationTarget extends Div
+        implements HasUrlParameter<Integer> {
+
+    private final String uuid = UUID.randomUUID().toString();
+    private final AtomicInteger counter = new AtomicInteger();
+    private final RouterLink routerLink;
+    private final Div clickCounter;
+
+    public ProxiedNavigationTarget() {
+        Div uuid = new Div(this.uuid);
+        uuid.setId("COMPONENT_ID");
+        add(uuid);
+
+        clickCounter = new Div("P:0, C:0");
+        clickCounter.setId("CLICK_COUNTER");
+        add(clickCounter);
+
+        // Self navigation should use the same view instance
+        routerLink = new RouterLink("Self Link", ProxiedNavigationTarget.class,
+                counter.incrementAndGet());
+        add(routerLink);
+    }
+
+    // @Async annotation should cause Spring to create a proxy for the
+    // bean instance
+    @Async
+    public void asyncMethod() {
+
+    }
+
+    @Override
+    public void setParameter(BeforeEvent event,
+            @OptionalParameter Integer parameter) {
+        if (parameter != null) {
+            clickCounter.setText("P:" + parameter + ", C:" + counter.get());
+            routerLink.setRoute(ProxiedNavigationTarget.class,
+                    counter.incrementAndGet());
+        }
+    }
+}

flow-tests/vaadin-spring-tests/test-spring-common/src/test/java/com/vaadin/flow/spring/test/RouteBasicIT.java

@@ -16,9 +16,12 @@
 
 package com.vaadin.flow.spring.test;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.junit.Assert;
 import org.junit.Test;
 import org.openqa.selenium.By;
+import org.slf4j.LoggerFactory;
 
 public class RouteBasicIT extends AbstractSpringTest {
 
@@ -37,4 +40,36 @@ public void testServletDeployed() throws Exception {
 
         waitUntil(driver -> isElementPresent(By.id("singleton-in-ui")));
     }
+
+    @Test
+    public void uiScopedProxiedTargetView_shouldUseSameViewInstance()
+            throws Exception {
+        getDriver().get(getTestURL() + "proxied");
+        waitForDevServer();
+
+        String prevUuid = null;
+        AtomicReference<String> prevCounter = new AtomicReference<>("");
+        for (int i = 0; i < 5; i++) {
+            String uuid = waitUntil(d -> d.findElement(By.id("COMPONENT_ID")))
+                    .getText();
+
+            waitUntil(d -> !prevCounter.get()
+                    .equals(d.findElement(By.id("CLICK_COUNTER")).getText()));
+
+            if (prevUuid != null) {
+                Assert.assertEquals("UUID should not have been changed",
+                        prevUuid, uuid);
+            }
+            String counter = findElement(By.id("CLICK_COUNTER")).getText();
+            Assert.assertEquals(
+                    "Parameter and counter should have the same value",
+                    "P:" + i + ", C:" + i, counter);
+
+            prevUuid = uuid;
+            prevCounter.set(counter);
+
+            $("a").first().click();
+        }
+
+    }
 }

flow-tests/vaadin-spring-tests/test-spring/pom.xml

@@ -93,6 +93,7 @@
                                     <artifactId>vaadin-test-spring-common</artifactId>
                                     <version>${project.version}</version>
                                     <type>test-jar</type>
+                                    <classifier>tests</classifier>
                                     <outputDirectory>${project.build.directory}/test-classes</outputDirectory>
                                 </artifactItem>
                             </artifactItems>

vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringInstantiator.java

@@ -24,6 +24,7 @@
 import org.springframework.boot.SpringBootVersion;
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.SpringVersion;
+import org.springframework.util.ClassUtils;
 
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.di.DefaultInstantiator;
@@ -135,4 +136,9 @@ public <T> T getOrCreate(Class<T> type) {
             return context.getAutowireCapableBeanFactory().createBean(type);
         }
     }
+
+    @Override
+    public Class<?> getApplicationClass(Class<?> clazz) {
+        return ClassUtils.getUserClass(clazz);
+    }
 }