feat: Do not allow themeClass and themeName in same annotation (#9389)

Theme Class and Theme Name is not supported in
the same Theme annotation as theme name builds
on the Lumo theme.

test-themes is now for testing the Application theme only.
Old theme test was moved into test-misc.

Fixes #9370
# Conflicts:
#	flow-server/src/test/java/com/vaadin/flow/server/frontend/scanner/FrontendDependenciesTest.java
#	flow-tests/README.md
#	flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/MiscelaneousView.java
#	flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/NpmThemedComponentView.java
#	flow-tests/test-themes-legacy/src/main/java/com/vaadin/flow/uitest/ui/theme/MyComponent.java

flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/FrontendDependencies.java

@@ -305,15 +305,26 @@ private void computeApplicationTheme() throws ClassNotFoundException,
             // we have a proper theme or no-theme for the app
             ThemeData themeData = themes.iterator().next();
             if (!themeData.isNotheme()) {
-                variant = themeData.getVariant();
                 String themeClass = themeData.getThemeClass();
-                if (themeClass == null) {
-                    themeClass = LUMO;
+                if (!themeData.getThemeName().isEmpty() && (themeClass != null
+                && !getDefaultTheme().isAssignableFrom(getFinder().loadClass(themeClass)))) {
+                    throw new IllegalStateException(
+                        "Theme name and theme class can not both be specified. "
+                            + "Theme name uses Lumo and can not be used in combination "
+                            + "with custom theme class that doesn't extend Lumo.");
+                }
+                variant = themeData.getVariant();
+                if (themeClass != null) {
+                    theme = getFinder().loadClass(themeClass);
+                } else {
+                    theme = getDefaultTheme();
+                    if (theme == null) {
+                        throw new IllegalStateException(
+                            "Lumo dependency needs to be available on the classpath when using a theme name.");
+                    }
                 }
-                theme = getFinder().loadClass(themeClass);
                 themeName = themeData.getThemeName();
             }
-
         }
 
         // theme could be null when lumo is not found or when a NoTheme found

flow-server/src/test/java/com/vaadin/flow/server/frontend/scanner/FrontendDependenciesTest.java

@@ -44,6 +44,7 @@
 import com.vaadin.flow.server.frontend.scanner.samples.RouteComponentWithLayout;
 import com.vaadin.flow.server.frontend.scanner.samples.RouteComponentWithMethodReference;
 import com.vaadin.flow.theme.AbstractTheme;
+import com.vaadin.flow.theme.Theme;
 
 import static org.hamcrest.CoreMatchers.is;
 
@@ -73,16 +74,15 @@ public void setUp() throws ClassNotFoundException {
         Mockito.when(classFinder.loadClass(FrontendDependencies.LUMO))
                 .thenReturn((Class) FakeLumo.class);
 
-        Mockito.doAnswer(invocation -> {
-            return FrontendDependenciesTest.class.getClassLoader()
-                    .getResource(invocation.getArgumentAt(0, String.class));
-        }).when(classFinder).getResource(Mockito.anyString());
+        Mockito.doAnswer(
+            invocation -> FrontendDependenciesTest.class.getClassLoader()
+                .getResource(invocation.getArgumentAt(0, String.class)))
+            .when(classFinder).getResource(Mockito.anyString());
 
     }
 
     @Test
-    public void routedComponent_endpointsAreCollected()
-            throws ClassNotFoundException {
+    public void routedComponent_endpointsAreCollected() {
         Mockito.when(classFinder.getAnnotatedClasses(Route.class))
                 .thenReturn(Collections.singleton(RouteComponent.class));
         FrontendDependencies dependencies = new FrontendDependencies(
@@ -97,8 +97,7 @@ public void routedComponent_endpointsAreCollected()
     }
 
     @Test
-    public void hasErrorParameterComponent_endpointIsCollected()
-            throws ClassNotFoundException {
+    public void hasErrorParameterComponent_endpointIsCollected() {
         Mockito.when(classFinder.getSubTypesOf(HasErrorParameter.class))
                 .thenReturn(Collections.singleton(ErrorComponent.class));
         FrontendDependencies dependencies = new FrontendDependencies(
@@ -113,8 +112,7 @@ public void hasErrorParameterComponent_endpointIsCollected()
     }
 
     @Test
-    public void componentInsideUiInitListener_endpointsAreCollected()
-            throws ClassNotFoundException {
+    public void componentInsideUiInitListener_endpointsAreCollected() {
         Mockito.when(classFinder.getSubTypesOf(UIInitListener.class))
                 .thenReturn(Collections.singleton(MyUIInitListener.class));
         FrontendDependencies dependencies = new FrontendDependencies(
@@ -129,8 +127,7 @@ public void componentInsideUiInitListener_endpointsAreCollected()
     }
 
     @Test
-    public void componentInsideUiInitListenerInsideServiceInitListener_endpointsAreCollected()
-            throws ClassNotFoundException {
+    public void componentInsideUiInitListenerInsideServiceInitListener_endpointsAreCollected() {
         Mockito.when(classFinder.getSubTypesOf(VaadinServiceInitListener.class))
                 .thenReturn(Collections.singleton(MyServiceListener.class));
         FrontendDependencies dependencies = new FrontendDependencies(
@@ -145,7 +142,7 @@ public void componentInsideUiInitListenerInsideServiceInitListener_endpointsAreC
     }
 
     @Test
-    public void jsScriptOrderIsPreserved() throws ClassNotFoundException {
+    public void jsScriptOrderIsPreserved() {
         Mockito.when(classFinder.getAnnotatedClasses(Route.class))
                 .thenReturn(Collections.singleton(JsOrderComponent.class));
         FrontendDependencies dependencies = new FrontendDependencies(
@@ -201,6 +198,91 @@ public void defaultThemeIsLoadedForExporters() throws Exception {
         Assert.assertNotNull(dependencies.getThemeDefinition());
     }
 
+    @Test
+    public void themeDefiningNonLumoClassAndName_throwsException()
+        throws ClassNotFoundException {
+        Mockito.when(classFinder.getAnnotatedClasses(Route.class))
+            .thenReturn(Collections.singleton(FaultyThemeAnnotation.class));
+        Mockito.when(classFinder.loadClass(FakeLumo.class.getName()))
+            .thenReturn((Class) FakeLumo.class);
+
+        Mockito.when(classFinder.loadClass(MyTheme.class.getName()))
+            .thenReturn((Class) MyTheme.class);
+
+        IllegalStateException exception = Assert
+            .assertThrows(IllegalStateException.class,
+                () -> new FrontendDependencies(classFinder, false));
+
+        Assert.assertEquals("Unexpected message for the thrown exception",
+            "Theme name and theme class can not both be specified. "
+                + "Theme name uses Lumo and can not be used in combination with"
+                + " custom theme class that doesn't extend Lumo.",
+            exception.getMessage());
+    }
+
+    @Test
+    public void themeDefiningLumoClassAndName_loadsLumo()
+        throws ClassNotFoundException {
+        Mockito.when(classFinder.getAnnotatedClasses(Route.class))
+            .thenReturn(Collections.singleton(OkClassExtension.class));
+        Mockito.when(classFinder.loadClass(FakeLumo.class.getName()))
+            .thenReturn((Class) FakeLumo.class);
+
+        Mockito.when(classFinder.loadClass(MyLumoTheme.class.getName()))
+            .thenReturn((Class) MyLumoTheme.class);
+
+        FrontendDependencies dependencies = new FrontendDependencies(classFinder, false);
+
+        Assert.assertEquals("Faulty default theme received",
+            MyLumoTheme.class, dependencies.getThemeDefinition().getTheme());
+    }
+
+    @Test
+    public void noDefaultThemeAvailable_throwsException()
+        throws ClassNotFoundException {
+        Mockito.when(classFinder.getAnnotatedClasses(Route.class))
+            .thenReturn(Collections.singleton(MyAppTheme.class));
+        Mockito.when(classFinder.loadClass(FrontendDependencies.LUMO))
+            .thenThrow(ClassNotFoundException.class);
+
+        IllegalStateException exception = Assert
+            .assertThrows(IllegalStateException.class,
+                () -> new FrontendDependencies(classFinder, false));
+
+        Assert.assertEquals("Thrown exception didn't contain correct message",
+            "Lumo dependency needs to be available on the classpath when using a theme name.",
+            exception.getMessage());
+    }
+
+    @Test
+    public void appThemeDefined_getsLumoAsTheme() {
+        Mockito.when(classFinder.getAnnotatedClasses(Route.class))
+            .thenReturn(Collections.singleton(MyAppTheme.class));
+
+        FrontendDependencies dependencies = new FrontendDependencies(classFinder, false);
+
+        Assert.assertEquals("Faulty default theme received",
+            FakeLumo.class, dependencies.getThemeDefinition().getTheme());
+
+
+    }
+
+    @Theme(FakeLumo.class)
+    public static class MyApp extends Component {
+    }
+
+    @Theme(themeFolder ="my-theme")
+    public static class MyAppTheme extends Component {
+    }
+
+    @Theme(themeFolder = "my-theme", value = MyTheme.class)
+    public static class FaultyThemeAnnotation extends Component {
+    }
+
+    @Theme(themeFolder = "my-theme", value = MyLumoTheme.class)
+    public static class OkClassExtension extends Component {
+    }
+
     public static class MyComponent extends Component {
     }
 
@@ -229,4 +311,34 @@ public String getThemeUrl() {
             return null;
         }
     }
+
+    public static class MyTheme implements AbstractTheme {
+        public MyTheme() {
+        }
+
+        @Override
+        public String getBaseUrl() {
+            return null;
+        }
+
+        @Override
+        public String getThemeUrl() {
+            return null;
+        }
+    }
+
+    public static class MyLumoTheme extends FakeLumo {
+        public MyLumoTheme() {
+        }
+
+        @Override
+        public String getBaseUrl() {
+            return null;
+        }
+
+        @Override
+        public String getThemeUrl() {
+            return null;
+        }
+    }
 }

flow-tests/README.md

@@ -28,8 +28,8 @@
 * test-memory-leaks
   * Verification test that no memory leaks happen during war redeployment.
 * test-misc
-  * Uncategorized tests in both npm and bower modes
-    * Contains custom theme functionality
+  * Uncategorized tests
+    * Contains custom theme tests
 * test-mixed
   * Test maven builds in all 4 run modes
 * test-multi-war
@@ -75,6 +75,8 @@
 * test-subcontext
   * `context://` tests for Compatibility mode
 * test-themes
+  * improved theming support tests
+* test-themes-legacy
   * Custom Theme tests for NPM and Compatibility modes
 
 ### Common test resource modules

flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/MiscelaneousView.java

@@ -15,6 +15,9 @@
  */
 package com.vaadin.flow.misc.ui;
 
+import java.util.Collections;
+import java.util.List;
+
 import com.vaadin.flow.component.dependency.HtmlImport;
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.component.html.Div;
@@ -47,6 +50,12 @@ public String getBaseUrl() {
         public String getThemeUrl() {
             return "legacyTheme/my-theme";
         }
+
+        @Override
+        public List<String> getHeaderInlineContents() {
+            return Collections.singletonList("<custom-style>\n <style>\n   html {\n"
+                + "      font-size: 20px;\n  color:red;  }\n <style>\n </custom-style>");
+        }
     }
 
     public MiscelaneousView() {

flow-tests/test-themes/pom.xml

@@ -25,6 +25,11 @@
             <artifactId>flow-html-components-testbench</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>com.vaadin</groupId>
+            <artifactId>vaadin-lumo-theme</artifactId>
+            <version>${project.version}</version>
+        </dependency>
 
     </dependencies>
 

flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/MyTheme.java → flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/ThemeView.java

@@ -13,27 +13,19 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package com.vaadin.flow.uitest.ui.theme;
 
-import java.util.Collections;
-import java.util.List;
+package com.vaadin.flow.uitest.ui.theme;
 
-import com.vaadin.flow.theme.AbstractTheme;
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.Span;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.theme.Theme;
 
-public class MyTheme implements AbstractTheme {
-    @Override
-    public String getBaseUrl() {
-        return "src/";
-    }
-
-    @Override
-    public String getThemeUrl() {
-        return "legacyTheme/myTheme/";
-    }
+@Route("com.vaadin.flow.uitest.ui.theme.Theme")
+@Theme(themeFolder = "app-theme")
+public class ThemeView extends Div {
 
-    @Override
-    public List<String> getHeaderInlineContents() {
-        return Collections.singletonList("<custom-style>\n <style>\n   html {\n"
-                + "      font-size: 20px;\n  color:red;  }\n <style>\n </custom-style>");
+    public ThemeView() {
+        add(new Span("This is the theme test view"));
     }
 }