From 8adb3a425373a6c16ea61a0022b2401dd9b1c514 Mon Sep 17 00:00:00 2001 From: Rob Platt <9541175+docrjp@users.noreply.github.com> Date: Sun, 10 Jan 2021 15:38:32 +0000 Subject: [PATCH] Fixed exception about missing custom css file (#7292) --- CHANGELOG.md | 1 + .../preferences/AppearanceTabViewModel.java | 4 +- .../org/jabref/gui/preview/PreviewViewer.java | 22 +- .../gui/util/DefaultFileUpdateMonitor.java | 7 +- src/main/java/org/jabref/gui/util/Theme.java | 190 +++++++++++-- .../org/jabref/model/strings/StringUtil.java | 23 -- .../jabref/preferences/JabRefPreferences.java | 2 +- .../java/org/jabref/gui/util/ThemeTest.java | 258 ++++++++++++++++++ .../org.mockito.plugins.MockMaker | 2 + 9 files changed, 438 insertions(+), 71 deletions(-) create mode 100644 src/test/java/org/jabref/gui/util/ThemeTest.java create mode 100644 src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/CHANGELOG.md b/CHANGELOG.md index d8f89f60f5d..faf7f5758a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue with the style of highlighted check boxes while searching in preferences. [#7226](https://github.com/JabRef/jabref/issues/7226) - We fixed an issue where the option "Move file to file directory" was disabled in the entry editor for all files [#7194](https://github.com/JabRef/jabref/issues/7194) - We fixed an issue where application dialogs were opening in the wrong display when using multiple screens [#7273](https://github.com/JabRef/jabref/pull/7273) +- We fixed an issue where an exception would be displayed for previewing and preferences when a custom theme has been configured but is missing [#7177](https://github.com/JabRef/jabref/issues/7177) ### Removed diff --git a/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java b/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java index e2edc7a78cb..d66b6d5ab3f 100644 --- a/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java @@ -93,7 +93,7 @@ public void setValues() { themeLightProperty.setValue(false); themeDarkProperty.setValue(false); themeCustomProperty.setValue(true); - customPathToThemeProperty.setValue(currentTheme.getPath().toString()); + customPathToThemeProperty.setValue(currentTheme.getCssPathString()); } } @@ -116,7 +116,7 @@ public void storeSettings() { restartWarnings.add(Localization.lang("Theme changed to dark theme.")); newTheme = new Theme(EMBEDDED_DARK_THEME_CSS, preferences); } else if (themeCustomProperty.getValue() && - (!initialAppearancePreferences.getTheme().getPath().toString() + (!initialAppearancePreferences.getTheme().getCssPathString() .equalsIgnoreCase(customPathToThemeProperty.getValue()) || initialAppearancePreferences.getTheme().getType() != Theme.Type.CUSTOM)) { restartWarnings.add(Localization.lang("Theme changed to a custom theme:") + " " diff --git a/src/main/java/org/jabref/gui/preview/PreviewViewer.java b/src/main/java/org/jabref/gui/preview/PreviewViewer.java index 68b71fe7ca6..b0949ee9a5d 100644 --- a/src/main/java/org/jabref/gui/preview/PreviewViewer.java +++ b/src/main/java/org/jabref/gui/preview/PreviewViewer.java @@ -1,8 +1,5 @@ package org.jabref.gui.preview; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.Base64; import java.util.Objects; import java.util.Optional; import java.util.regex.Pattern; @@ -19,7 +16,6 @@ import org.jabref.gui.ClipBoardManager; import org.jabref.gui.DialogService; import org.jabref.gui.Globals; -import org.jabref.gui.JabRefFrame; import org.jabref.gui.StateManager; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.TaskExecutor; @@ -30,7 +26,6 @@ import org.jabref.logic.search.SearchQuery; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; -import org.jabref.model.strings.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -126,22 +121,7 @@ public PreviewViewer(BibDatabaseContext database, DialogService dialogService, S } public void setTheme(Theme theme) { - if (theme.getType() == Theme.Type.DARK) { - // We need to load the css file manually, due to a bug in the jdk - // https://bugs.openjdk.java.net/browse/JDK-8240969 - // TODO: Remove this workaround as soon as openjfx 16 is released - URL url = JabRefFrame.class.getResource(theme.getPath().getFileName().toString()); - String dataUrl = "data:text/css;charset=utf-8;base64," + - Base64.getEncoder().encodeToString(StringUtil.getResourceFileAsString(url).getBytes()); - - previewView.getEngine().setUserStyleSheetLocation(dataUrl); - } else if (theme.getType() != Theme.Type.LIGHT) { - try { - previewView.getEngine().setUserStyleSheetLocation(theme.getPath().toUri().toURL().toExternalForm()); - } catch (MalformedURLException ex) { - LOGGER.error("Cannot set custom theme, invalid url", ex); - } - } + theme.getAdditionalStylesheet().ifPresent(location -> previewView.getEngine().setUserStyleSheetLocation(location)); } private void highlightSearchPattern() { diff --git a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java index 7526dd55792..0dc66b038f8 100644 --- a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java +++ b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java @@ -32,7 +32,7 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor { private static final Logger LOGGER = LoggerFactory.getLogger(DefaultFileUpdateMonitor.class); private final Multimap listeners = ArrayListMultimap.create(20, 4); - private WatchService watcher; + private volatile WatchService watcher; private final AtomicBoolean notShutdown = new AtomicBoolean(true); private Optional filesystemMonitorFailure; @@ -102,7 +102,10 @@ public void removeListener(Path path, FileUpdateListener listener) { public void shutdown() { try { notShutdown.set(false); - watcher.close(); + WatchService watcher = this.watcher; + if (watcher != null) { + watcher.close(); + } } catch (IOException e) { LOGGER.error("error closing watcher", e); } diff --git a/src/main/java/org/jabref/gui/util/Theme.java b/src/main/java/org/jabref/gui/util/Theme.java index 8577e879e88..6009459ccff 100644 --- a/src/main/java/org/jabref/gui/util/Theme.java +++ b/src/main/java/org/jabref/gui/util/Theme.java @@ -1,13 +1,18 @@ package org.jabref.gui.util; import java.io.IOException; +import java.io.InputStream; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.net.URLConnection; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.util.Base64; import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; import javafx.scene.Scene; @@ -25,6 +30,14 @@ * Installs the style file and provides live reloading. * JabRef provides two inbuilt themes and a user customizable one: Light, Dark and Custom. The Light theme is basically * the base.css theme. Every other theme is loaded as an addition to base.css. + * + * For type Custom, Theme will protect against removal of the CSS file, degrading as gracefully as possible. If the file + * becomes unavailable while the application is running, some Scenes that have not yet had the CSS installed may not be + * themed. The PreviewViewer, which uses WebEngine, supports data URLs and so generally are not affected by removal + * of the file; however Theme will not attempt to URL-encode large style sheets so as to protect + * memory usage (see {@link Theme#MAX_IN_MEMORY_CSS_LENGTH}. + * + * @see Custom themes in the Jabref documentation. */ public class Theme { public enum Type { @@ -33,45 +46,153 @@ public enum Type { public static final String BASE_CSS = "Base.css"; + /** + * A size limit above which Theme will not attempt to keep a data-embedded URL in memory for the CSS. + * + * It's tolerable for CSS to exceed this limit; the functional benefit of the encoded CSS is in some edge + * case error handling. Specifically, having a reference to a data-embedded URL means that the Preview Viewer + * isn't impacted if the source CSS file is removed while the application is running. + * + * If the CSS is over this limit, then the user won't see any functional impact, as long as the file exists. Only if + * it becomes unavailable, might there be some impact. First, the Preview Viewer when created might not be themed. + * Second, there is a very small chance of uncaught exceptions. Theme makes a best effort to avoid this: + * it checks for CSS file existence before passing it to the Preview Viewer for theming. Still, as file existence + * checks are immediately out of date, it can't be perfectly ruled out. + * + * At the time of writing this comment: + * + * + * + * So realistic custom themes will fit comfortably within 48k, even if they are modified copies of the base theme. + * + * Note that Base-64 encoding will increase the memory footprint of the URL by a third. + */ + private static final int MAX_IN_MEMORY_CSS_LENGTH = 48000; + private static final Logger LOGGER = LoggerFactory.getLogger(Theme.class); private final Type type; - private final Path pathToCss; - private final Optional additionalCssToLoad; + + /* String, Path, and URL formats of the path to the css. These are determined at construction. + Path and URL are only set if they are relevant and valid (i.e. no illegal characters). + In general, use additionalCssToLoad(), to also ensure file existence checks are performed + */ + private final String cssPathString; + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + private final Optional cssUrl; + + private final AtomicReference> cssDataUrlString = new AtomicReference<>(Optional.empty()); + private final PreferencesService preferencesService; public Theme(String path, PreferencesService preferencesService) { - this.pathToCss = Path.of(path); + this.cssPathString = path; this.preferencesService = preferencesService; if (StringUtil.isBlank(path) || BASE_CSS.equalsIgnoreCase(path)) { // Light theme this.type = Type.LIGHT; - this.additionalCssToLoad = Optional.empty(); + this.cssUrl = Optional.empty(); } else { - Optional cssResource = Optional.ofNullable(JabRefFrame.class.getResource(path)); - if (cssResource.isPresent()) { + URL url = JabRefFrame.class.getResource(path); + if (url != null) { // Embedded dark theme this.type = Type.DARK; } else { // Custom theme this.type = Type.CUSTOM; - if (Files.exists(pathToCss)) { - try { - cssResource = Optional.of(pathToCss.toUri().toURL()); - } catch (MalformedURLException e) { - cssResource = Optional.empty(); - } + + try { + url = Path.of(path).toUri().toURL(); + } catch (InvalidPathException e) { + LOGGER.warn("Cannot load additional css {} because it is an invalid path: {}", path, e.getLocalizedMessage()); + url = null; + } catch (MalformedURLException e) { + LOGGER.warn("Cannot load additional css url {} because it is a malformed url: {}", path, e.getLocalizedMessage()); + url = null; } } - if (cssResource.isPresent()) { - additionalCssToLoad = cssResource; - LOGGER.debug("Using css {}", path); - } else { - additionalCssToLoad = Optional.empty(); - LOGGER.warn("Cannot load css {}", path); + this.cssUrl = Optional.ofNullable(url); + LOGGER.debug("Theme is {}, additional css url is {}", this.type, cssUrl.orElse(null)); + } + + additionalCssToLoad().ifPresent(this::loadCssToMemory); + } + + /** + * Returns the additional CSS file or resource, after checking that it is accessible. + * + * Note that the file checks are immediately out of date, i.e. the CSS file could become unavailable between + * the check and attempts to use that file. The checks are just on a best-effort basis. + * + * @return an optional providing the URL of the CSS file/resource, or empty + */ + private Optional additionalCssToLoad() { + // Check external sources of CSS to make sure they are available: + if (isAdditionalCssExternal()) { + Optional cssPath = cssUrl.map(url -> Path.of(URI.create(url.toExternalForm()))); + // No need to return explicitly return Optional.empty() if Path is invalid; the URL will be empty anyway + if (cssPath.isPresent()) { + // When we have a valid file system path, check that the CSS file is readable + Path path = cssPath.get(); + + if (!Files.exists(path)) { + LOGGER.warn("Not loading additional css file {} because it could not be found", cssPath.get()); + return Optional.empty(); + } + + if (Files.isDirectory(path)) { + LOGGER.warn("Not loading additional css file {} because it is a directory", cssPath.get()); + return Optional.empty(); + } + } + } + + return cssUrl; + } + + private boolean isAdditionalCssExternal() { + return cssUrl.isPresent() && "file".equals(cssUrl.get().getProtocol()); + } + + /** + * Creates a data-embedded URL from a file (or resource) URL. + * + * TODO: this is only desirable for file URLs, as protection against the file being removed (see + * {@link #MAX_IN_MEMORY_CSS_LENGTH} for details). However, there is a bug in OpenJFX, in that it does not + * recognise jrt URLs (modular java runtime URLs). This is detailed in + * JDK-8240969. + * When we upgrade to OpenJFX 16, we should limit loadCssToMemory to external URLs i.e. check + * {@link #isAdditionalCssExternal()}. Also rename to loadExternalCssToMemory() and reword the + * javadoc, for clarity. + * + * @param url the URL of the resource to convert into a data: url + */ + private void loadCssToMemory(URL url) { + try { + URLConnection conn = url.openConnection(); + conn.connect(); + + try (InputStream inputStream = conn.getInputStream()) { + byte[] data = inputStream.readNBytes(MAX_IN_MEMORY_CSS_LENGTH); + if (data.length < MAX_IN_MEMORY_CSS_LENGTH) { + String embeddedDataUrl = "data:text/css;charset=utf-8;base64," + + Base64.getEncoder().encodeToString(data); + LOGGER.debug("Embedded css in data URL of length {}", embeddedDataUrl.length()); + cssDataUrlString.set(Optional.of(embeddedDataUrl)); + } else { + LOGGER.debug("Not embedding css in data URL as the length is >= {}", MAX_IN_MEMORY_CSS_LENGTH); + cssDataUrlString.set(Optional.empty()); + } } + } catch (IOException e) { + LOGGER.warn("Could not load css url {} into memory", url, e); } } @@ -83,7 +204,7 @@ public void installCss(Scene scene, FileUpdateMonitor fileUpdateMonitor) { AppearancePreferences appearancePreferences = preferencesService.getAppearancePreferences(); addAndWatchForChanges(scene, JabRefFrame.class.getResource(BASE_CSS), fileUpdateMonitor, 0); - additionalCssToLoad.ifPresent(file -> addAndWatchForChanges(scene, file, fileUpdateMonitor, 1)); + additionalCssToLoad().ifPresent(file -> addAndWatchForChanges(scene, file, fileUpdateMonitor, 1)); if (appearancePreferences.shouldOverrideDefaultFontSize()) { scene.getRoot().setStyle("-fx-font-size: " + appearancePreferences.getMainFontSize() + "pt;"); @@ -113,9 +234,10 @@ private void addAndWatchForChanges(Scene scene, URL cssFile, FileUpdateMonitor f LOGGER.debug("CSS URI {}", cssUri); Path cssPath = Path.of(cssUri).toAbsolutePath(); - LOGGER.info("Enabling live reloading of {}", cssPath); + LOGGER.info("Enabling live reloading of css file {}", cssPath); fileUpdateMonitor.addListenerForFile(cssPath, () -> { LOGGER.info("Reload css file {}", cssFile); + additionalCssToLoad().ifPresent(this::loadCssToMemory); DefaultTaskExecutor.runInJavaFXThread(() -> { scene.getStylesheets().remove(cssFile.toExternalForm()); scene.getStylesheets().add(index, cssFile.toExternalForm()); @@ -127,11 +249,35 @@ private void addAndWatchForChanges(Scene scene, URL cssFile, FileUpdateMonitor f } } + /** + * @return the Theme type + */ public Type getType() { return type; } - public Path getPath() { - return pathToCss; + /** + * Provides the raw, configured custom CSS location. This should be a file system path, but the raw string is + * returned even if it is not valid in some way. For this reason, the main use case for this getter is to + * storing or display the user preference, rather than to read and use the CSS file. + * + * @return the raw configured CSS location + */ + public String getCssPathString() { + return cssPathString; + } + + /** + * This method allows callers to obtain the theme's additional stylesheet. + * + * @return called with the stylesheet location if there is an additional stylesheet present and available. The + * location will be a local URL. Typically it will be a {@code 'data:'} URL where the CSS is embedded. However for + * large themes it can be {@code 'file:'}. + */ + public Optional getAdditionalStylesheet() { + if (cssDataUrlString.get().isEmpty()) { + additionalCssToLoad().ifPresent(this::loadCssToMemory); + } + return cssDataUrlString.get().or(() -> additionalCssToLoad().map(URL::toExternalForm)); } } diff --git a/src/main/java/org/jabref/model/strings/StringUtil.java b/src/main/java/org/jabref/model/strings/StringUtil.java index 4debf557b4b..cc428a54455 100644 --- a/src/main/java/org/jabref/model/strings/StringUtil.java +++ b/src/main/java/org/jabref/model/strings/StringUtil.java @@ -1,12 +1,5 @@ package org.jabref.model.strings; -import java.io.BufferedInputStream; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.net.URL; -import java.net.URLConnection; import java.text.Normalizer; import java.util.ArrayList; import java.util.Arrays; @@ -17,7 +10,6 @@ import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; import org.jabref.architecture.ApacheCommonsLang3Allowed; @@ -743,19 +735,4 @@ public static boolean containsIgnoreCase(String text, String searchString) { public static String substringBetween(String str, String open, String close) { return StringUtils.substringBetween(str, open, close); } - - public static String getResourceFileAsString(URL resource) { - try { - URLConnection conn = resource.openConnection(); - conn.connect(); - - try (InputStream inputStream = new BufferedInputStream(conn.getInputStream()); - InputStreamReader inputStreamReader = new InputStreamReader(inputStream); - BufferedReader reader = new BufferedReader(inputStreamReader)) { - return reader.lines().collect(Collectors.joining(System.lineSeparator())); - } - } catch (IOException e) { - throw new RuntimeException(e); - } - } } diff --git a/src/main/java/org/jabref/preferences/JabRefPreferences.java b/src/main/java/org/jabref/preferences/JabRefPreferences.java index 44f6e0571be..5629628b9eb 100644 --- a/src/main/java/org/jabref/preferences/JabRefPreferences.java +++ b/src/main/java/org/jabref/preferences/JabRefPreferences.java @@ -1989,7 +1989,7 @@ public AppearancePreferences getAppearancePreferences() { public void storeAppearancePreference(AppearancePreferences preferences) { putBoolean(OVERRIDE_DEFAULT_FONT_SIZE, preferences.shouldOverrideDefaultFontSize()); putInt(MAIN_FONT_SIZE, preferences.getMainFontSize()); - put(FX_THEME, preferences.getTheme().getPath().toString()); + put(FX_THEME, preferences.getTheme().getCssPathString()); } //************************************************************************************************************* diff --git a/src/test/java/org/jabref/gui/util/ThemeTest.java b/src/test/java/org/jabref/gui/util/ThemeTest.java new file mode 100644 index 00000000000..08471166620 --- /dev/null +++ b/src/test/java/org/jabref/gui/util/ThemeTest.java @@ -0,0 +1,258 @@ +package org.jabref.gui.util; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.Optional; + +import javafx.collections.FXCollections; +import javafx.scene.Scene; + +import org.jabref.preferences.AppearancePreferences; +import org.jabref.preferences.PreferencesService; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ThemeTest { + + private Path tempFolder; + + private PreferencesService preferencesMock; + + @BeforeEach + void setUp(@TempDir Path tempFolder) { + this.tempFolder = tempFolder; + this.preferencesMock = mock(PreferencesService.class); + AppearancePreferences appearancePreferences = mock(AppearancePreferences.class); + when(this.preferencesMock.getAppearancePreferences()).thenReturn(appearancePreferences); + when(appearancePreferences.shouldOverrideDefaultFontSize()).thenReturn(false); + } + + @Test + public void lightThemeUsedWhenPathIsBlank() { + Theme blankTheme = new Theme("", preferencesMock); + assertEquals(Theme.Type.LIGHT, blankTheme.getType()); + assertEquals(Optional.empty(), blankTheme.getAdditionalStylesheet(), + "didn't expect additional stylesheet to be available"); + } + + @Test + public void lightThemeUsedWhenPathIsBaseCss() { + Theme baseTheme = new Theme("Base.css", preferencesMock); + assertEquals(Theme.Type.LIGHT, baseTheme.getType()); + assertEquals(Optional.empty(), baseTheme.getAdditionalStylesheet(), + "didn't expect additional stylesheet to be available"); + } + + @Test + public void darkThemeUsedWhenPathIsDarkCss() { + // Dark theme is detected by name: + Theme theme = new Theme("Dark.css", preferencesMock); + assertEquals(Theme.Type.DARK, theme.getType()); + assertTrue(theme.getAdditionalStylesheet().isPresent(), + "expected dark theme stylesheet to be available"); + } + + @Test + public void customThemeIgnoredIfDirectory() { + Theme baseTheme = new Theme(tempFolder.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, baseTheme.getType()); + assertEquals(Optional.empty(), baseTheme.getAdditionalStylesheet(), + "didn't expect additional stylesheet to be available when location is a directory"); + } + + @Test + public void customThemeIgnoredIfInvalidPath() { + Theme baseTheme = new Theme("\0\0\0", preferencesMock); + assertEquals(Theme.Type.CUSTOM, baseTheme.getType()); + assertEquals(Optional.empty(), baseTheme.getAdditionalStylesheet(), + "didn't expect additional stylesheet when CSS location is just some null terminators!"); + } + + @Test + public void customThemeAvailableEvenWhenDeleted() throws IOException { + + /* Create a temporary custom theme that is just a small snippet of CSS. There is no CSS + validation (at the moment) but by making a valid CSS block we don't preclude adding validation later */ + Path testCss = tempFolder.resolve("test.css"); + Files.writeString(testCss, + "/* Biblatex Source Code */\n" + + ".code-area .text {\n" + + " -fx-font-family: monospace;\n" + + "}", StandardOpenOption.CREATE); + + // This is detected as a custom theme: + Theme theme = new Theme(testCss.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, theme.getType()); + assertEquals(testCss.toString(), theme.getCssPathString()); + + Optional testCssLocation1 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation1.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQmlibGF0ZXggU291cmNlIENvZGUgKi8KLmNvZGUtYXJlYSAudGV4dCB7CiAgICAtZngtZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsKfQ==", + testCssLocation1.get()); + + Files.delete(testCss); + + // Consumer passed to additionalStylesheet() should still return the data url even though file is deleted. + // It shouldn't matter whether the file existed at the time the Theme object was created (before or after) + + Optional testCssLocation2 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation2.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQmlibGF0ZXggU291cmNlIENvZGUgKi8KLmNvZGUtYXJlYSAudGV4dCB7CiAgICAtZngtZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsKfQ==", + testCssLocation2.get()); + + Theme themeCreatedWhenAlreadyMissing = new Theme(testCss.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, theme.getType()); + assertEquals(testCss.toString(), theme.getCssPathString()); + assertEquals(Optional.empty(), themeCreatedWhenAlreadyMissing.getAdditionalStylesheet(), + "didn't expect additional stylesheet to be available because it didn't exist when theme was created"); + + // Check that the consumer is called once more, if the file is restored + Files.writeString(testCss, + "/* Biblatex Source Code */\n" + + ".code-area .text {\n" + + " -fx-font-family: monospace;\n" + + "}", StandardOpenOption.CREATE); + + Optional testCssLocation3 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation3.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQmlibGF0ZXggU291cmNlIENvZGUgKi8KLmNvZGUtYXJlYSAudGV4dCB7CiAgICAtZngtZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsKfQ==", + testCssLocation3.get()); + + Optional testCssLocation4 = themeCreatedWhenAlreadyMissing.getAdditionalStylesheet(); + assertTrue(testCssLocation4.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQmlibGF0ZXggU291cmNlIENvZGUgKi8KLmNvZGUtYXJlYSAudGV4dCB7CiAgICAtZngtZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsKfQ==", + testCssLocation4.get()); + } + + @Test + public void largeCustomThemeNotHeldInMemory() throws IOException { + + /* Create a temporary custom theme that is just a large comment over 48 kilobytes in size. There is no CSS + validation (at the moment) but by making a valid CSS comment we don't preclude adding validation later */ + Path testCss = tempFolder.resolve("test.css"); + Files.createFile(testCss); + Files.writeString(testCss, "/* ", StandardOpenOption.CREATE); + final String testString = "ALL WORK AND NO PLAY MAKES JACK A DULL BOY\n"; + for (int i = 0; i <= (48000 / testString.length()); i++) { + Files.writeString(testCss, testString, StandardOpenOption.APPEND); + } + Files.writeString(testCss, " */", StandardOpenOption.APPEND); + + // This is detected as a custom theme: + Theme theme = new Theme(testCss.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, theme.getType()); + assertEquals(testCss.toString(), theme.getCssPathString()); + + Optional testCssLocation1 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation1.isPresent(), "expected custom theme location to be available"); + assertTrue(testCssLocation1.get().startsWith("file:"), "expected large custom theme to be a file"); + + Files.move(testCss, testCss.resolveSibling("renamed.css")); + + // additionalStylesheet() will no longer offer the deleted stylesheet, because it's not been held in memory + + assertEquals(Optional.empty(), theme.getAdditionalStylesheet(), + "didn't expect additional stylesheet after css was deleted"); + + Theme themeCreatedWhenAlreadyMissing = new Theme(testCss.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, theme.getType()); + assertEquals(testCss.toString(), theme.getCssPathString()); + assertEquals(Optional.empty(), themeCreatedWhenAlreadyMissing.getAdditionalStylesheet(), + "didn't expect additional stylesheet to be available because it didn't exist when theme was created"); + + // Check that it is available once more, if the file is restored + + Files.move(testCss.resolveSibling("renamed.css"), testCss); + + Optional testCssLocation2 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation2.isPresent(), "expected custom theme location to be available"); + assertTrue(testCssLocation2.get().startsWith("file:"), "expected large custom theme to be a file"); + + Optional testCssLocation3 = themeCreatedWhenAlreadyMissing.getAdditionalStylesheet(); + assertTrue(testCssLocation3.isPresent(), "expected custom theme location to be available"); + assertTrue(testCssLocation3.get().startsWith("file:"), "expected large custom theme to be a file"); + } + + /* + TODO this test works great on a local Windows development machine, but currently fails in the github CI pipeline. + Investigate why, and when resolved remove the @EnabledOnOs annotation that limits this test + */ + @Test + @EnabledOnOs(OS.WINDOWS) + public void liveReloadCssDataUrl() throws IOException, InterruptedException { + + /* Create a temporary custom theme that is just a small snippet of CSS. There is no CSS + validation (at the moment) but by making a valid CSS block we don't preclude adding validation later */ + Path testCss = tempFolder.resolve("reload.css"); + Files.writeString(testCss, + "/* Biblatex Source Code */\n" + + ".code-area .text {\n" + + " -fx-font-family: monospace;\n" + + "}", StandardOpenOption.CREATE); + + Theme theme = new Theme(testCss.toString(), preferencesMock); + assertEquals(Theme.Type.CUSTOM, theme.getType()); + assertEquals(testCss.toString(), theme.getCssPathString()); + + Optional testCssLocation1 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation1.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQmlibGF0ZXggU291cmNlIENvZGUgKi8KLmNvZGUtYXJlYSAudGV4dCB7CiAgICAtZngtZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsKfQ==", + testCssLocation1.get()); + + DefaultFileUpdateMonitor fileUpdateMonitor = null; + Thread thread = null; + try { + Scene scene = mock(Scene.class); + when(scene.getStylesheets()).thenReturn(FXCollections.observableArrayList()); + fileUpdateMonitor = new DefaultFileUpdateMonitor(); + thread = new Thread(fileUpdateMonitor); + thread.start(); + try { + theme.installCss(scene, fileUpdateMonitor); + } catch (NullPointerException ex) { + fail("Possible mocking issue due to NPE in installCss", ex); + } + + Thread.sleep(2000); + + Files.writeString(testCss, + "/* And now for something slightly different */\n" + + ".code-area .text {\n" + + " -fx-font-family: serif;\n" + + "}", StandardOpenOption.CREATE); + + Thread.sleep(2000); + } finally { + if (fileUpdateMonitor != null) { + fileUpdateMonitor.shutdown(); + } + if (thread != null) { + thread.join(); + } + } + + Optional testCssLocation2 = theme.getAdditionalStylesheet(); + assertTrue(testCssLocation2.isPresent(), "expected custom theme location to be available"); + assertEquals( + "data:text/css;charset=utf-8;base64,LyogQW5kIG5vdyBmb3Igc29tZXRoaW5nIHNsaWdodGx5IGRpZmZlcmVudCAqLwouY29kZS1hcmVhIC50ZXh0IHsKICAgIC1meC1mb250LWZhbWlseTogc2VyaWY7Cn0=", + testCssLocation2.get(), + "stylesheet embedded in data: url should have reloaded"); + } +} diff --git a/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000000..83088d4ec16 --- /dev/null +++ b/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1,2 @@ +# This Mockito extension allows us to mock final methods, which are very common in OpenJFX +mock-maker-inline