Skip to content

Commit 1d496aa

Browse files
caaladorcaalador
authored
feat!: No default theme loaded (#21730)
* feat!: No default theme loaded Do not load Lumo as the default theme if not defined. Fixes #21606 * fix themeClassName fix test names
1 parent 7623f4a commit 1d496aa

File tree

7 files changed

+35
-83
lines changed

7 files changed

+35
-83
lines changed

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -582,9 +582,7 @@ private void computeApplicationTheme() throws ClassNotFoundException,
582582
Class<? extends AbstractTheme> theme = null;
583583
String variant = "";
584584
String themeName = "";
585-
if (themes.isEmpty()) {
586-
theme = getDefaultTheme();
587-
} else {
585+
if (!themes.isEmpty()) {
588586
// we have a proper theme or no-theme for the app
589587
ThemeData themeData = themes.iterator().next();
590588
if (!themeData.isNotheme()) {
@@ -598,7 +596,7 @@ private void computeApplicationTheme() throws ClassNotFoundException,
598596
if (themeClass != null) {
599597
theme = getFinder().loadClass(themeClass);
600598
} else {
601-
theme = getDefaultTheme();
599+
theme = getLumoTheme();
602600
if (theme == null) {
603601
throw new IllegalStateException(
604602
"Lumo dependency needs to be available on the classpath when using a theme name.");
@@ -625,16 +623,6 @@ private String getThemeDescription(ThemeData theme) {
625623
return theme.getThemeClass();
626624
}
627625

628-
/**
629-
* Finds the default theme.
630-
*
631-
* @return Lumo
632-
*/
633-
Class<? extends AbstractTheme> getDefaultTheme() throws IOException {
634-
// No theme annotation found by the scanner
635-
return getLumoTheme();
636-
}
637-
638626
/**
639627
* Visit all classes annotated with {@link NpmPackage} and update the list
640628
* of dependencies and their versions.

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,7 @@ private void debug(String label, Set<String> log) {
360360
private void discoverTheme() {
361361
ThemeData data = verifyTheme();
362362

363-
if (data == null) {
364-
setupTheme(getLumoTheme(), "", "");
365-
return;
366-
}
367-
368-
if (data.isNotheme()) {
363+
if (data == null || data.isNotheme()) {
369364
return;
370365
}
371366

@@ -375,7 +370,7 @@ private void discoverTheme() {
375370
setupTheme(theme, data.getVariant(), data.getThemeName());
376371
} catch (ClassNotFoundException exception) {
377372
throw new IllegalStateException(
378-
"Could not load theme class " + data.getThemeClass(),
373+
"Could not load theme class '" + data.getThemeClass() + "'",
379374
exception);
380375
}
381376
}
@@ -404,12 +399,19 @@ private ThemeData verifyTheme() {
404399
Set<ThemeData> themes = annotatedClasses.stream()
405400
.flatMap(clazz -> annotationFinder
406401
.apply(clazz, loadedThemeAnnotation).stream())
407-
.map(theme -> new ThemeData(
408-
((Class<?>) getAnnotationValue(theme, "themeClass"))
409-
.getName(),
410-
getAnnotationValueAsString(theme, "variant"),
411-
getAnnotationValueAsString(theme, VALUE)))
412-
.collect(Collectors.toSet());
402+
.map(theme -> {
403+
String themeClassName = "";
404+
Class<?> themeClass = (Class<?>) getAnnotationValue(
405+
theme, "themeClass");
406+
if (themeClass.equals(AbstractTheme.class)) {
407+
themeClassName = FullDependenciesScanner.LUMO;
408+
} else {
409+
themeClassName = themeClass.getName();
410+
}
411+
return new ThemeData(themeClassName,
412+
getAnnotationValueAsString(theme, "variant"),
413+
getAnnotationValueAsString(theme, VALUE));
414+
}).collect(Collectors.toSet());
413415

414416
Class<? extends Annotation> loadedNoThemeAnnotation = getFinder()
415417
.loadClass(NoTheme.class.getName());

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,7 @@ final class ThemeData implements Serializable {
3535
boolean notheme;
3636

3737
ThemeData(String themeClass, String variant, String themeName) {
38-
if (themeClass.equals(AbstractTheme.class.getName())) {
39-
this.themeClass = FullDependenciesScanner.LUMO;
40-
} else {
41-
this.themeClass = themeClass;
42-
}
38+
this.themeClass = themeClass;
4339
this.variant = variant;
4440
this.themeName = themeName;
4541
}

‎flow-server/src/main/java/com/vaadin/flow/theme/Theme.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@
8585
/**
8686
* The theme translation handler.
8787
*
88-
* Defaults to Lumo, If not specified.
89-
*
9088
* @return theme handler
9189
*/
9290
Class<? extends AbstractTheme> themeClass() default AbstractTheme.class;

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package com.vaadin.flow.server.frontend.scanner;
1717

18-
import java.lang.reflect.InvocationTargetException;
1918
import java.util.Collections;
2019
import java.util.LinkedHashSet;
2120
import java.util.Optional;
@@ -158,7 +157,6 @@ public void appThemeDefined_getsLumoAsTheme() {
158157

159158
Assert.assertEquals("Faulty default theme received", FakeLumo.class,
160159
dependencies.getThemeDefinition().getTheme());
161-
162160
}
163161

164162
@Test
@@ -173,7 +171,6 @@ public void onlyThemeVariantDefined_getsLumoAsTheme_preserveVariant() {
173171
dependencies.getThemeDefinition().getTheme());
174172
Assert.assertEquals("Faulty variant received", "dark",
175173
dependencies.getThemeDefinition().getVariant());
176-
177174
}
178175

179176
@Test
@@ -245,7 +242,7 @@ public void extractsAndScansClassesFromMethodReferences() {
245242
}
246243

247244
@Test
248-
public void defaultThemeIsLoadedForExporters() throws Exception {
245+
public void defaultThemeIsNotLoadedForExporters() throws Exception {
249246
FakeLumo.class.getDeclaredConstructor().newInstance();
250247
Mockito.when(classFinder.getSubTypesOf(WebComponentExporter.class))
251248
.thenReturn(Stream.of(MyExporter.class)
@@ -254,8 +251,8 @@ public void defaultThemeIsLoadedForExporters() throws Exception {
254251
FrontendDependencies dependencies = new FrontendDependencies(
255252
classFinder, true, null, true);
256253

257-
Assert.assertNotNull(dependencies.getTheme());
258-
Assert.assertNotNull(dependencies.getThemeDefinition());
254+
Assert.assertNull(dependencies.getTheme());
255+
Assert.assertNull(dependencies.getThemeDefinition());
259256
}
260257

261258
@Test // #9861

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,20 +96,15 @@ public void setUp() throws ClassNotFoundException {
9696
}
9797

9898
@Test
99-
public void getTheme_noExplicitTheme_lumoThemeIsDiscovered()
99+
public void getTheme_noExplicitTheme_noThemeIsDiscovered()
100100
throws ClassNotFoundException {
101101
FrontendDependenciesScanner scanner = setUpThemeScanner(
102102
Collections.emptySet(), Collections.emptySet(),
103103
(type, annotationType) -> Collections.emptyList());
104104

105105
Mockito.verify(finder).loadClass(AbstractTheme.class.getName());
106106

107-
Assert.assertNotNull(scanner.getTheme());
108-
Assert.assertEquals("foo", scanner.getTheme().getBaseUrl());
109-
Assert.assertEquals(FakeLumoTheme.class,
110-
scanner.getThemeDefinition().getTheme());
111-
Assert.assertEquals("", scanner.getThemeDefinition().getVariant());
112-
107+
Assert.assertNull(scanner.getTheme());
113108
Assert.assertEquals(0, scanner.getClasses().size());
114109
}
115110

@@ -427,10 +422,6 @@ private FullDependenciesScanner setUpThemeScanner(
427422

428423
return new FullDependenciesScanner(finder, annotationFinder, null,
429424
true) {
430-
@Override
431-
protected Class<? extends AbstractTheme> getLumoTheme() {
432-
return FakeLumoTheme.class;
433-
}
434425
};
435426
}
436427

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

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.vaadin.flow.server.frontend.scanner;
22

3-
import java.io.IOException;
43
import java.util.Arrays;
54
import java.util.Collections;
65
import java.util.HashSet;
@@ -18,16 +17,15 @@
1817
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.RootViewWithMultipleTheme;
1918
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.RootViewWithTheme;
2019
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.RootViewWithoutTheme;
21-
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.RootViewWithoutThemeAnnotation;
2220
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.SecondView;
2321
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.Theme1;
2422
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.Theme2;
2523
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.Theme4;
2624
import com.vaadin.flow.server.frontend.scanner.ScannerTestComponents.ThemeExporter;
27-
import com.vaadin.flow.theme.AbstractTheme;
2825

2926
import static com.vaadin.flow.server.frontend.scanner.ScannerDependenciesTest.getFrontendDependencies;
3027
import static org.junit.Assert.assertEquals;
28+
import static org.junit.Assert.assertNull;
3129
import static org.junit.Assert.assertTrue;
3230
import static org.mockito.Mockito.spy;
3331
import static org.mockito.Mockito.times;
@@ -92,30 +90,16 @@ public void should_throw_when_ThemeAndNoTheme() {
9290
}
9391

9492
@Test
95-
public void should_visitDefaultTheme_when_noThemeAnnotationIsGiven()
96-
throws Exception {
97-
98-
DefaultClassFinder finder = spy(new DefaultClassFinder(
99-
Collections.singleton(RootViewWithoutThemeAnnotation.class)));
100-
101-
// we'll do a partial mock here since we want to keep the other
102-
// behavior of the DefaultClassFinder. Theme4 is used as a fake Lumo
103-
// since it has @JsModule annotation which makes it easy to verify
104-
// that the Theme was actually visited and modules collected
93+
public void should_notFindAnyTheme_when_noThemeAnnotationIsGiven() {
94+
DefaultClassFinder finder = spy(
95+
new DefaultClassFinder(Collections.singleton(
96+
ScannerTestComponents.RootViewWithoutThemeAnnotation.class)));
10597

10698
FrontendDependencies deps = new FrontendDependencies(finder, true, null,
107-
true) {
108-
@Override
109-
Class<? extends AbstractTheme> getDefaultTheme()
110-
throws IOException {
111-
return Theme4.class;
112-
}
113-
};
114-
115-
assertEquals(
116-
"Theme4 should have been returned when default theme was selected",
117-
Theme4.class, deps.getThemeDefinition().getTheme());
118-
DepsTests.assertImportsExcludingUI(deps.getModules(), "./theme-4.js");
99+
true);
100+
101+
assertNull("No default theme should have been selected",
102+
deps.getThemeDefinition());
119103
}
120104

121105
@Test
@@ -127,22 +111,18 @@ public void should_takeThemeFromExporter_when_exporterFound() {
127111
}
128112

129113
@Test
130-
public void should_defaultToLumoTheme_when_noThemeDefinedByExporter()
114+
public void should_useExtendedClassTheme_when_noThemeDefinedByExporter()
131115
throws Exception {
132116
// RootViewWithTheme is added to the list just to make sure exporter
133117
// handles theming default, not the other crawlers
134118
DefaultClassFinder finder = spy(new DefaultClassFinder(
135119
new HashSet<>(Arrays.asList(NoThemeExporter.class,
136120
RootViewWithTheme.class))));
137121

138-
Mockito.doReturn(Theme4.class).when(finder)
139-
.loadClass(FrontendDependencies.LUMO);
140-
141122
FrontendDependencies deps = new FrontendDependencies(finder, true, null,
142123
true);
143-
assertEquals(
144-
"Theme4 should have been returned when default theme was selected",
145-
Theme4.class, deps.getThemeDefinition().getTheme());
124+
assertEquals("Theme4 should have been returned as theme", Theme4.class,
125+
deps.getThemeDefinition().getTheme());
146126
DepsTests.assertImportsExcludingUI(deps.getModules(), "./theme-4.js");
147127
}
148128

0 commit comments

Comments
 (0)