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

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

@@ -311,15 +311,24 @@ 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) {
+                    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.");
+                }
+                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

@@ -123,6 +123,54 @@ public void appShellConfigurator_collectedAsEndpoint()
             themeDefinition.getTheme());
     }
 
+    @Test
+    public void themeDefiningClassAndName_throwsException()
+        throws ClassNotFoundException {
+        Mockito.when(classFinder.getSubTypesOf(AppShellConfigurator.class))
+            .thenReturn(Collections.singleton(FaultyThemeAnnotation.class));
+        Mockito.when(classFinder.loadClass(FakeLumo.class.getName()))
+            .thenReturn((Class) FakeLumo.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.",
+            exception.getMessage());
+    }
+
+    @Test
+    public void noDefaultThemeAvailable_throwsException()
+        throws ClassNotFoundException {
+        Mockito.when(classFinder.getSubTypesOf(AppShellConfigurator.class))
+            .thenReturn(Collections.singleton(MyAppThemeShell.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.getSubTypesOf(AppShellConfigurator.class))
+            .thenReturn(Collections.singleton(MyAppThemeShell.class));
+
+        FrontendDependencies dependencies = new FrontendDependencies(classFinder, false);
+
+        Assert.assertEquals("Faulty default theme received",
+            FakeLumo.class, dependencies.getThemeDefinition().getTheme());
+
+
+    }
+
     @Test
     public void hasErrorParameterComponent_endpointIsCollected() {
         Mockito.when(classFinder.getSubTypesOf(HasErrorParameter.class))
@@ -257,4 +305,13 @@ public String getThemeUrl() {
     @Theme(themeClass = FakeLumo.class)
     public static class MyAppShell implements AppShellConfigurator {
     }
+
+    @Theme("my-theme")
+    public static class MyAppThemeShell implements AppShellConfigurator {
+    }
+
+
+    @Theme(value = "my-theme", themeClass = FakeLumo.class)
+    public static class FaultyThemeAnnotation implements AppShellConfigurator {
+    }
 }

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
@@ -73,7 +73,7 @@
 * test-servlet
   * Automatic servlet registration test
 * test-themes
-  * Custom Theme tests for NPM and Compatibility modes
+  * Test application theme features
 * test-live-reload
   * Tests the live reload feature in development mode. Run sequentially in their own
     module as live reload affects all open UIs and would cause interference between 

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.JsModule;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.misc.ui.MiscelaneousView.MyTheme;
@@ -42,6 +45,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/src/main/java/com/vaadin/flow/uitest/ui/theme/NpmThemedComponentView.java → flow-tests/test-misc/src/main/java/com/vaadin/flow/misc/ui/NpmThemedComponentView.java

@@ -13,15 +13,14 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package com.vaadin.flow.uitest.ui.theme;
+package com.vaadin.flow.misc.ui;
 
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.component.dependency.JsModule;
 import com.vaadin.flow.router.Route;
-import com.vaadin.flow.theme.Theme;
 
-@Route(value = "com.vaadin.flow.uitest.ui.theme.NpmThemedComponentView")
+@Route(value = "com.vaadin.flow.misc.ui.NpmThemedComponentView")
 @Tag("npm-themed-component")
 // `src/` in component should be replaced by `legacyTheme/my-theme`
 @JsModule("./src/npm-themed-component.js")

flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/NpmThemedComponentIT.java → flow-tests/test-misc/src/test/java/com/vaadin/flow/misc/ui/NpmThemedComponentIT.java

@@ -13,7 +13,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-package com.vaadin.flow.uitest.ui.theme;
+package com.vaadin.flow.misc.ui;
 
 import java.util.List;
 
@@ -26,17 +26,6 @@
 
 public class NpmThemedComponentIT extends ChromeBrowserTest {
 
-    @Test
-    public void applicationTheme_GlobalCss_isUsed() {
-        open();
-        // No exception for bg-image should exist
-        checkLogsForErrors();
-
-        Assert.assertEquals(
-            "url(\"" + getRootURL() + "/theme/app-theme/img/bg.jpg\")",
-            findElement(By.tagName("body")).getCssValue("background-image"));
-    }
-
     @Test
     public void importedClientSideComponentIsThemed() {
         open();
@@ -71,12 +60,7 @@ public void importedClientSideComponentIsThemed() {
     protected String getTestPath() {
         String path = super.getTestPath();
         String view = "view/";
-        String result;
-        if (path.startsWith("/")) {
-            result = path.substring(view.length() + 1);
-        }
-        result = path.substring(view.length());
-        return result;
+        return path.substring(view.length());
     }
 
 }

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/AppShell.java

@@ -20,7 +20,7 @@
 import com.vaadin.flow.server.PWA;
 import com.vaadin.flow.theme.Theme;
 
-@Theme(value ="app-theme", themeClass = MyTheme.class)
+@Theme(value ="app-theme")
 @PWA(name = "Project Base for Vaadin", shortName = "Project Base")
 public class AppShell implements AppShellConfigurator {
 }

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,17 @@
  * 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;
 
-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")
+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"));
     }
 }

flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/ThemeIT.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2000-2020 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.uitest.ui.theme;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+import com.vaadin.flow.testutil.ChromeBrowserTest;
+import com.vaadin.testbench.TestBenchElement;
+
+public class ThemeIT extends ChromeBrowserTest {
+
+    @Test
+    public void applicationTheme_GlobalCss_isUsed() {
+        open();
+        // No exception for bg-image should exist
+        checkLogsForErrors();
+
+        Assert.assertEquals(
+            "url(\"" + getRootURL() + "/theme/app-theme/img/bg.jpg\")",
+            findElement(By.tagName("body")).getCssValue("background-image"));
+    }
+
+    @Override
+    protected String getTestPath() {
+        String path = super.getTestPath();
+        String view = "view/";
+        return path.substring(view.length());
+    }
+
+}