From abc601c60f1fad536873ece0fc7974724cc72f89 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 13:34:50 +0200 Subject: [PATCH 01/11] fix GUI interaction edge cases and build checks --- build.gradle | 5 ++ check.sh | 4 +- docs/API-Guide.md | 2 +- .../groovy/se/alipsa/gi/AbstractInOut.groovy | 26 +++++- .../main/groovy/se/alipsa/gi/FileUtils.groovy | 31 +++++--- .../groovy/se/alipsa/gi/GuiInteraction.groovy | 9 ++- .../se/alipsa/gi/AbstractInOutTest.groovy | 24 ++++++ .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 14 +++- gi-console/README.md | 8 +- .../groovy/se/alipsa/gi/console/InOut.groovy | 45 ++++++++--- .../gi/console/ConsolePromptTest.groovy | 56 +++++++++++++ gi-fx/build.gradle | 2 + .../main/groovy/se/alipsa/gi/fx/InOut.groovy | 42 +++++++--- .../se/alipsa/gi/fx/InOutHeadlessTest.groovy | 20 +++++ gi-swing/build.gradle | 8 ++ .../groovy/se/alipsa/gi/swing/InOut.groovy | 50 ++++++++---- .../alipsa/gi/swing/InOutHeadlessTest.groovy | 20 +++++ gradle/libs.versions.toml | 6 +- release.sh | 79 ++++++++----------- 19 files changed, 333 insertions(+), 118 deletions(-) create mode 100644 gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy create mode 100644 gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy create mode 100644 gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy diff --git a/build.gradle b/build.gradle index cbe5d95..234579e 100644 --- a/build.gradle +++ b/build.gradle @@ -75,6 +75,11 @@ subprojects { xml.required = false } } + afterEvaluate { + tasks.withType(com.github.spotbugs.snom.SpotBugsTask).configureEach { + auxClassPaths.from(configurations.runtimeClasspath) + } + } } } diff --git a/check.sh b/check.sh index 8717b89..40c24bb 100755 --- a/check.sh +++ b/check.sh @@ -1,2 +1,4 @@ #!/usr/bin/env bash -gradlew spotlessApply check --no-configuration-cache --console=plain \ No newline at end of file +set -euo pipefail + +./gradlew spotlessApply check --no-configuration-cache --console=plain diff --git a/docs/API-Guide.md b/docs/API-Guide.md index 7955424..cf72e06 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -312,5 +312,5 @@ if (file == null) { - Best for headless/CI environments - `display()` and `display(Chart)` print messages instead of showing UI -- Password input requires `System.console()` (returns null in IDEs) +- Password input is masked when `System.console()` is available; otherwise stdin input is visible and a warning is logged - Tables displayed as text using Matrix.content() diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index aea24c3..fa87b1e 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -39,6 +39,16 @@ abstract class AbstractInOut implements GuiInteraction { con.setConnectTimeout(timeout) con.setReadTimeout(timeout) int responseCode = con.getResponseCode() + if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || + responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { + con.disconnect() + con = (HttpURLConnection) url.openConnection() + con.setInstanceFollowRedirects(false) + con.setRequestMethod("GET") + con.setConnectTimeout(timeout) + con.setReadTimeout(timeout) + responseCode = con.getResponseCode() + } // Accept 2xx (success) and 3xx (redirect) status codes return responseCode >= 200 && responseCode < 400 } catch (RuntimeException | IOException ignored) { @@ -91,10 +101,18 @@ abstract class AbstractInOut implements GuiInteraction { @Override String prompt(Map namedParams) { - return prompt(String.valueOf(namedParams.getOrDefault("title", "")), - String.valueOf(namedParams.getOrDefault("headerText", "")), - String.valueOf(namedParams.getOrDefault("message", "")), - String.valueOf(namedParams.getOrDefault("defaultValue", ""))) + if (namedParams == null) { + throw new IllegalArgumentException("namedParams cannot be null") + } + return prompt(asPromptValue(namedParams, "title"), + asPromptValue(namedParams, "headerText"), + asPromptValue(namedParams, "message"), + asPromptValue(namedParams, "defaultValue")) + } + + private static String asPromptValue(Map namedParams, String key) { + Object value = namedParams.get(key) + return value == null ? "" : String.valueOf(value) } @Override diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index 9cab7e3..55fe41e 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -30,18 +30,21 @@ class FileUtils { * or {@code null} if the input is {@code null} */ static String baseName(String url) { - if (url == null) return null; - String basename = ""; - url = url.replace('\\', '/'); + if (url == null) return null + url = url.replace('\\', '/') + int queryIndex = url.indexOf('?') + int fragmentIndex = url.indexOf('#') + int suffixIndex = queryIndex >= 0 && fragmentIndex >= 0 ? + Math.min(queryIndex, fragmentIndex) : Math.max(queryIndex, fragmentIndex) + if (suffixIndex >= 0) { + url = url.substring(0, suffixIndex) + } + String basename = "" if (url.contains("/")) { - String filePart = url.substring(url.lastIndexOf('/')+1); - if (filePart.contains("?")) { - basename = filePart.substring(0, filePart.indexOf('?')); - } else { - basename = filePart; - } + String filePart = url.substring(url.lastIndexOf('/')+1) + basename = filePart } - return basename.length() > 0 ? basename : url; + return basename.length() > 0 ? basename : url } /** @@ -59,9 +62,12 @@ class FileUtils { * This method allows loading resources from both the classpath and the file system. * * @param resource the resource path to locate (classpath resource or file path) - * @return the URL of the resource, or {@code null} if not found and path is invalid + * @return the URL of the resource, or {@code null} if it cannot be found */ static URL getResourceUrl(String resource) { + if (resource == null || resource.isEmpty()) { + return null + } final List classLoaders = new ArrayList<>() classLoaders.add(Thread.currentThread().getContextClassLoader()) classLoaders.add(FileUtils.class.getClassLoader()) @@ -82,7 +88,8 @@ class FileUtils { return systemResource } else { try { - return new File(resource).toURI().toURL() + File file = new File(resource) + return file.exists() ? file.toURI().toURL() : null } catch (MalformedURLException e) { return null } diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index 18c33cb..4a1e326 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -102,11 +102,12 @@ interface GuiInteraction { /** * Checks if a URL exists and is accessible. *

- * Performs an HTTP HEAD request to verify the URL is reachable. + * Performs an HTTP HEAD request, falling back to GET when HEAD is not supported, + * to verify the URL is reachable. * * @param urlString the URL to check * @param timeout connection timeout in milliseconds - * @return {@code true} if the URL returns HTTP 200, {@code false} otherwise + * @return {@code true} if the URL returns a 2xx or 3xx response, {@code false} otherwise */ boolean urlExists(String urlString, int timeout) @@ -251,8 +252,8 @@ interface GuiInteraction { /** * Prompts the user for a password with masked input. *

- * In console mode, this may return {@code null} if no console is available - * (e.g., when running in an IDE or CI environment). + * In console mode, a visible-input fallback is used if no system console is + * available (e.g., when running in an IDE or CI environment). * * @param title the dialog title * @param message the prompt message to display diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 16a6e08..50cd249 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -10,6 +10,8 @@ import se.alipsa.matrix.core.Matrix import javax.swing.JComponent import java.time.LocalDate import java.time.YearMonth +import com.sun.net.httpserver.HttpServer +import java.net.InetSocketAddress import static org.junit.jupiter.api.Assertions.* @@ -191,6 +193,28 @@ class AbstractInOutTest { assertFalse(inOut.urlExists("http://localhost:59999/", 1000)) } + @Test + void urlExistsFallsBackToGetWhenHeadIsUnsupported() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/health") { exchange -> + if (exchange.requestMethod == "HEAD") { + exchange.sendResponseHeaders(405, -1) + } else { + byte[] response = "ok".bytes + exchange.sendResponseHeaders(200, response.length) + exchange.responseBody.write(response) + exchange.responseBody.close() + } + exchange.close() + } + server.start() + try { + assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) + } finally { + server.stop(0) + } + } + /** * Concrete implementation of AbstractInOut for testing purposes. * Provides minimal stub implementations for abstract methods. diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index faae125..769cffd 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -35,6 +35,11 @@ class FileUtilsTest { assertEquals("file.txt", FileUtils.baseName("http://example.com/path/file.txt?param=value")) } + @Test + void testBaseNameWithQueryStringAndNoPath() { + assertEquals("file.txt", FileUtils.baseName("file.txt?param=value")) + } + @Test void testBaseNameWithNull() { assertNull(FileUtils.baseName(null)) @@ -76,9 +81,14 @@ class FileUtilsTest { @Test void testGetResourceUrlWithNonExistentResource() { - // Should still return a URL (file URL) even for non-existent paths URL url = FileUtils.getResourceUrl("/nonexistent/path/file.txt") - assertNotNull(url, "Should return file URL for non-existent path") + assertNull(url, "Non-existent resources should not resolve") + } + + @Test + void testGetResourceUrlWithNullOrEmptyResource() { + assertNull(FileUtils.getResourceUrl(null)) + assertNull(FileUtils.getResourceUrl("")) } @Test diff --git a/gi-console/README.md b/gi-console/README.md index 159f75f..c944987 100644 --- a/gi-console/README.md +++ b/gi-console/README.md @@ -86,20 +86,18 @@ dependencies { ## Limitations - **Clipboard**: Clipboard access is unavailable in headless environments; clipboard methods log the issue and return `null` or no-op. -- **Password Input**: Requires `System.console()` to be available. Returns `null` in IDEs and some CI environments where console is unavailable. +- **Password Input**: Uses masked input through `System.console()` when available. In IDEs and some CI environments, it falls back to visible stdin input and logs a warning. - **Charts/Images**: `display(Chart)` and `display(File)` for images print a message instead of showing graphics - **Swing Components**: `display(JComponent)` is not supported - **File Choosers**: User manually types file paths instead of browsing ## Console Availability -The password prompt requires a system console: +The password prompt uses a system console when available: ```groovy def password = io.promptPassword("Login", "Enter password") -if (password == null) { - println("Console not available - cannot read password") -} +// If no system console is available, stdin input is visible. ``` To ensure console availability: diff --git a/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy b/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy index ed593f9..bb7a036 100644 --- a/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy +++ b/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy @@ -27,6 +27,7 @@ class InOut extends AbstractInOut { String read(String prompt) { print(prompt) + System.out.flush() return sysin.readLine() } @@ -69,36 +70,59 @@ class InOut extends AbstractInOut { @Override YearMonth promptYearMonth(String message) { String yearMonth = read(message) - return YearMonth.parse(yearMonth) + if (yearMonth == null || yearMonth.trim().isEmpty()) { + return null + } + return YearMonth.parse(yearMonth.trim()) } @Override YearMonth promptYearMonth(String title, String message, YearMonth from, YearMonth to, YearMonth initial) { String yearMonth = read("$title: $message") - return YearMonth.parse(yearMonth) + if (yearMonth == null) { + return null + } + if (yearMonth.trim().isEmpty()) { + return initial + } + YearMonth selected = YearMonth.parse(yearMonth.trim()) + if (from != null && selected.isBefore(from) || to != null && selected.isAfter(to)) { + throw new IllegalArgumentException("Selected year-month $selected is outside the range $from to $to") + } + return selected } @Override LocalDate promptDate(String title, String message, LocalDate defaultValue) { - String yearMonth = read("$title: $message") - return LocalDate.parse(yearMonth) + String date = read("$title: $message") + if (date == null) { + return null + } + if (date.trim().isEmpty()) { + return defaultValue + } + return LocalDate.parse(date.trim()) } @Override Object promptSelect(String title, String headerText, String message, Collection options, Object defaultValue) { + if (options == null || options.isEmpty()) { + throw new IllegalArgumentException("Options collection cannot be null or empty") + } + List optionList = new ArrayList<>(options) println title println headerText int i = 0 - for (def option : options) { + for (Object option : optionList) { println "${i}. $option" i++ } println "Default value is $defaultValue" String input = read(message) - if (input.isInteger()) { - int index = input.toInteger() - if (index >= 0 && index < options.size()) { - return options[index] + if (input != null && input.trim().isInteger()) { + int index = input.trim().toInteger() + if (index >= 0 && index < optionList.size()) { + return optionList[index] } else { println("Invalid index $input. Returning default value $defaultValue") return defaultValue @@ -148,6 +172,9 @@ class InOut extends AbstractInOut { println title println headerText String input = read(message) + if (input == null) { + return null + } if (input.isEmpty()) { return defaultValue } else { diff --git a/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy new file mode 100644 index 0000000..0d5117e --- /dev/null +++ b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy @@ -0,0 +1,56 @@ +package se.alipsa.gi.console + +import org.junit.jupiter.api.Test + +import java.time.LocalDate +import java.time.YearMonth + +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertNull +import static org.junit.jupiter.api.Assertions.assertThrows + +class ConsolePromptTest { + + @Test + void eofIsCancellationForPrompts() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('')) + + assertNull(inOut.prompt('message')) + assertNull(inOut.prompt('title', 'header', 'message', 'default')) + assertNull(inOut.promptYearMonth('month')) + assertNull(inOut.promptDate('date', 'message', LocalDate.of(2026, 1, 2))) + } + + @Test + void blankDateAndMonthUseDefaults() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('\n\n')) + + assertEquals(YearMonth.of(2026, 1), + inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), YearMonth.of(2026, 1))) + assertEquals(LocalDate.of(2026, 1, 2), + inOut.promptDate('date', 'message', LocalDate.of(2026, 1, 2))) + } + + @Test + void rangedYearMonthRejectsValuesOutsideRange() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('2027-01\n')) + + assertThrows(IllegalArgumentException) { + inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), null) + } + } + + @Test + void fullSelectionValidatesOptionsAndHandlesEof() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('')) + + assertEquals('first', inOut.promptSelect('title', '', 'choice', ['first', 'second'], 'first')) + assertThrows(IllegalArgumentException) { + inOut.promptSelect('title', '', 'choice', [], null) + } + } +} diff --git a/gi-fx/build.gradle b/gi-fx/build.gradle index 06e119e..e1107c2 100644 --- a/gi-fx/build.gradle +++ b/gi-fx/build.gradle @@ -53,11 +53,13 @@ dependencies { testImplementation platform(libs.junit.bom) testImplementation libs.junit.jupiter + testImplementation libs.groovy testImplementation "org.openjfx:javafx-graphics:${javaFxVersion}:$qualifier" testImplementation "org.openjfx:javafx-base:${javaFxVersion}:$qualifier" testImplementation "org.openjfx:javafx-controls:${javaFxVersion}:$qualifier" testImplementation "org.openjfx:javafx-swing:${javaFxVersion}:$qualifier" testImplementation "org.openjfx:javafx-web:${javaFxVersion}:$qualifier" + testRuntimeOnly libs.junit.platform.launcher } test { diff --git a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy index 2687c52..3202730 100644 --- a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy +++ b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy @@ -46,20 +46,17 @@ class InOut extends AbstractInOut { private static final Logger log = Logger.getLogger(InOut.class) - static { - if (GraphicsEnvironment.isHeadless()) { - throw new UnsupportedOperationException( - "gi-fx InOut requires a graphical environment. " + - "Use gi-console for headless environments.") - } - } - Window ownerWindow = null ObservableList styleSheetUrls = null //Clipboard clipboard InOut() { - new JFXPanel() + if (GraphicsEnvironment.isHeadless()) { + throw new UnsupportedOperationException( + "gi-fx InOut requires a graphical environment. " + + "Use gi-console for headless environments.") + } + initializeToolkit() } InOut(Window owner) { @@ -72,6 +69,10 @@ class InOut extends AbstractInOut { setStyleSheetUrls(styleSheets) } + private static void initializeToolkit() { + new JFXPanel() + } + @Override File chooseFile(String title, File initialDirectory, String description, String... extensions) { return runOnFxThread(() -> { @@ -212,6 +213,9 @@ class InOut extends AbstractInOut { @Override Object promptSelect(String title, String headerText, String message, Collection options, Object defaultValue) { + if (options == null || options.isEmpty()) { + throw new IllegalArgumentException("Options collection cannot be null or empty") + } List opt = options as List int defaultIndex = opt.indexOf(defaultValue) if (defaultIndex == -1) { @@ -326,6 +330,14 @@ class InOut extends AbstractInOut { log.error("Failed to detect image content type", e) } } + if (url.toExternalForm().toLowerCase(Locale.ROOT).contains('.svg')) { + Platform.runLater(() -> { + final WebView browser = new WebView() + browser.getEngine().load(url.toExternalForm()) + display(browser, title) + }); + return + } Image img = new Image(url.toExternalForm()) display(img, title) } @@ -404,34 +416,38 @@ class InOut extends AbstractInOut { } void saveToClipboard(String string) { - Platform.runLater(() -> { + runOnFxThread(() -> { ClipboardContent content = new ClipboardContent() content.putString(string) getClipboard().setContent(content) + return null }); } void saveToClipboard(File file) { - Platform.runLater(() -> { + runOnFxThread(() -> { ClipboardContent content = new ClipboardContent() content.putFiles(List.of(file)) getClipboard().setContent(content) + return null }); } void saveToClipboard(Image img) { - Platform.runLater(() -> { + runOnFxThread(() -> { ClipboardContent content = new ClipboardContent() content.putImage(img) getClipboard().setContent(content) + return null }); } void saveToClipboard(Object obj, DataFormat format) { - Platform.runLater(() -> { + runOnFxThread(() -> { ClipboardContent content = new ClipboardContent() content.put(format, obj) getClipboard().setContent(content) + return null }); } diff --git a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy new file mode 100644 index 0000000..d1a258e --- /dev/null +++ b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy @@ -0,0 +1,20 @@ +package se.alipsa.gi.fx + +import org.junit.jupiter.api.Test + +import java.awt.GraphicsEnvironment + +import static org.junit.jupiter.api.Assertions.assertThrows +import static org.junit.jupiter.api.Assumptions.assumeTrue + +class InOutHeadlessTest { + + @Test + void headlessConstructionReportsTheExpectedException() { + assumeTrue(GraphicsEnvironment.isHeadless()) + + assertThrows(UnsupportedOperationException) { + new InOut() + } + } +} diff --git a/gi-swing/build.gradle b/gi-swing/build.gradle index d7bf266..6dbec6e 100644 --- a/gi-swing/build.gradle +++ b/gi-swing/build.gradle @@ -50,6 +50,10 @@ dependencies { compileOnly "org.openjfx:javafx-controls:${javaFxVersion}:$qualifier" // SVG rendering is handled by matrix-charts SvgPanel + testImplementation platform(libs.junit.bom) + testImplementation libs.junit.jupiter + testImplementation libs.groovy.all + testRuntimeOnly libs.junit.platform.launcher } repositories { @@ -61,6 +65,10 @@ repositories { } } +test { + useJUnitPlatform() +} + publishing { publications { maven(MavenPublication) { diff --git a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy index 9156b38..52fcee8 100644 --- a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy +++ b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy @@ -9,6 +9,7 @@ import se.alipsa.gi.ImageTransferable import java.awt.Image import se.alipsa.gi.AbstractInOut +import se.alipsa.gi.FileUtils import se.alipsa.matrix.core.Matrix import se.alipsa.symp.YearMonthPicker @@ -26,21 +27,19 @@ import se.alipsa.matrix.core.util.Logger import java.awt.GraphicsEnvironment import javax.swing.filechooser.FileNameExtensionFilter import java.util.concurrent.ExecutionException +import java.util.Locale @CompileStatic class InOut extends AbstractInOut { private static final Logger log = Logger.getLogger(InOut.class) - static { - if (GraphicsEnvironment.isHeadless()) { - throw new UnsupportedOperationException( - "gi-swing InOut requires a graphical environment. " + - "Use gi-console for headless environments.") - } - } - InOut() { + if (GraphicsEnvironment.isHeadless()) { + throw new UnsupportedOperationException( + "gi-swing InOut requires a graphical environment. " + + "Use gi-console for headless environments.") + } // This is needed due to timing issues to ensure swing UI starts properly // Note: attempts with invokeLater to start the EDT did not work on MacOs JFrame frame = new JFrame() @@ -136,6 +135,9 @@ class InOut extends AbstractInOut { @Override Object promptSelect(String title, String headerText, String message, Collection options, Object defaultValue) { + if (options == null || options.isEmpty()) { + throw new IllegalArgumentException("Options collection cannot be null or empty") + } JPanel content = new JPanel(new BorderLayout()) content.add(new JLabel(headerText), BorderLayout.NORTH) JPanel messagePanel = new JPanel(new FlowLayout()) @@ -198,6 +200,10 @@ class InOut extends AbstractInOut { @Override void view(File file, String... title) { + if (file == null || !file.exists()) { + log.warn("Cannot view file: Failed to find {}", file) + return + } JEditorPane jep = new JEditorPane() jep.setPage(file.toURI().toURL()) JScrollPane scrollPane = new JScrollPane(jep) @@ -293,20 +299,32 @@ class InOut extends AbstractInOut { @Override void display(String fileName, String... title) { - File file = new File(fileName) - if (file.exists()) { + URL resource = FileUtils.getResourceUrl(fileName) + if (resource == null) { + log.warn("Cannot display image: Failed to find {}", fileName) + return + } + File file = resource.protocol == 'file' ? new File(resource.toURI()) : null + if (file != null && file.exists()) { try { String contentType = getContentType(file) if ("image/svg+xml" == contentType) { - displaySvg(file, title) + displaySvg(resource, title) return } } catch (IOException e) { log.error("Error detecting content type", e) return } + } else if (resource.toExternalForm().toLowerCase(Locale.ROOT).contains('.svg')) { + displaySvg(resource, title) + return + } + ImageIcon img = new ImageIcon(resource) + if (img.getIconWidth() < 0 || img.getIconHeight() < 0) { + log.warn("Cannot display image: Failed to load {}", fileName) + return } - ImageIcon img = new ImageIcon(fileName) JLabel label = new JLabel(img) display(label, title) } @@ -314,18 +332,18 @@ class InOut extends AbstractInOut { /** * Displays an SVG file using Apache Batik's JSVGCanvas. */ - private void displaySvg(File svgFile, String... title) { + private void displaySvg(URL svgUrl, String... title) { String svg try { - svg = svgFile.getText("UTF-8") + svg = svgUrl.getText("UTF-8") } catch (IOException e) { - log.error("Failed to read svg file {}", svgFile, e) + log.error("Failed to read svg resource {}", svgUrl, e) return } def svgPanel = ChartToSwing.export(svg) svgPanel.setPreferredSize(new Dimension(800, 600)) - JFrame frame = new JFrame(title.length > 0 ? title[0] : svgFile.getName()) + JFrame frame = new JFrame(title.length > 0 ? title[0] : FileUtils.baseName(svgUrl.toExternalForm())) frame.getContentPane().add(new JScrollPane(svgPanel)) frame.setSize(800, 600) frame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE) diff --git a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy new file mode 100644 index 0000000..518ad47 --- /dev/null +++ b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy @@ -0,0 +1,20 @@ +package se.alipsa.gi.swing + +import org.junit.jupiter.api.Test + +import java.awt.GraphicsEnvironment + +import static org.junit.jupiter.api.Assertions.assertThrows +import static org.junit.jupiter.api.Assumptions.assumeTrue + +class InOutHeadlessTest { + + @Test + void headlessConstructionReportsTheExpectedException() { + assumeTrue(GraphicsEnvironment.isHeadless()) + + assertThrows(UnsupportedOperationException) { + new InOut() + } + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6f2421b..dc5c93b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,13 +1,13 @@ [versions] groovy = "5.0.6" tika = "3.3.0" -matrixBom = "2.5.0-SNAPSHOT" +matrixBom = "2.5.1" commonmark = "0.28.0" jsoup = "1.22.2" junit = "6.0.3" javafx = "23.0.2" fxYearMonthPicker = "1.1.0" -swingWidgets = "1.1.0" +swingWidgets = "1.1.2" [libraries] groovy = { module = "org.apache.groovy:groovy", version.ref = "groovy" } @@ -34,4 +34,4 @@ javafx-swing = { module = "org.openjfx:javafx-swing", version.ref = "javafx" } javafx-web = { module = "org.openjfx:javafx-web", version.ref = "javafx" } fx-yearmonth-picker = { module = "se.alipsa:fx-yearmonth-picker", version.ref = "fxYearMonthPicker" } -swing-widgets = { module = "se.alipsa:swing-widgets", version.ref = "swingWidgets" } \ No newline at end of file +swing-widgets = { module = "se.alipsa:swing-widgets", version.ref = "swingWidgets" } diff --git a/release.sh b/release.sh index 7ed5db7..b3f6f8f 100755 --- a/release.sh +++ b/release.sh @@ -180,31 +180,33 @@ echo "" CURRENT_VERSION=$(get_version) echo -e "Current version: ${YELLOW}${CURRENT_VERSION}${NC}" -# Handle SNAPSHOT version - strip -SNAPSHOT suffix for release -if echo "$CURRENT_VERSION" | grep -q '\-SNAPSHOT'; then +commit_release_version() { + local release_version=$1 + update_version "$release_version" + update_readme_version "$release_version" + generate_changelog "$release_version" + + if ! git add build.gradle README.md CHANGELOG.md; then + echo -e "${RED}Error: Failed to add files to git. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi + if ! git commit -m "Release version ${release_version}"; then + echo -e "${RED}Error: Failed to commit version change. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi +} + +# A requested bump determines the release version, including when the current +# version is a snapshot. This avoids first committing the unbumped version and +# then publishing a different version. +if [ -n "$BUMP_TYPE" ]; then + RELEASE_VERSION=$(bump_version "$CURRENT_VERSION" "$BUMP_TYPE") + echo -e "Bumping version to: ${GREEN}${RELEASE_VERSION}${NC}" +elif echo "$CURRENT_VERSION" | grep -q '\-SNAPSHOT'; then RELEASE_VERSION="${CURRENT_VERSION%-SNAPSHOT}" echo -e "Stripping SNAPSHOT suffix: ${YELLOW}${CURRENT_VERSION}${NC} -> ${GREEN}${RELEASE_VERSION}${NC}" - - if [ "$DRY_RUN" = false ]; then - update_version "$RELEASE_VERSION" - update_readme_version "$RELEASE_VERSION" - generate_changelog "$RELEASE_VERSION" - - # Commit version changes - if ! git add build.gradle README.md CHANGELOG.md; then - echo -e "${RED}Error: Failed to add files to git. Please resolve the issue and try again.${NC}" >&2 - exit 1 - fi - if ! git commit -m "Release version ${RELEASE_VERSION}"; then - echo -e "${RED}Error: Failed to commit version change. Please resolve the issue and try again.${NC}" >&2 - exit 1 - fi - else - echo -e "${YELLOW}[DRY RUN] Would update build.gradle and README.md to ${RELEASE_VERSION}${NC}" - fi - CURRENT_VERSION=$RELEASE_VERSION else - # No SNAPSHOT - verify README.md has the correct version + RELEASE_VERSION="$CURRENT_VERSION" if ! check_readme_version "$CURRENT_VERSION"; then if [ "$DRY_RUN" = true ]; then echo -e "${YELLOW}[DRY RUN] Would update README.md to match version ${CURRENT_VERSION}${NC}" @@ -217,6 +219,13 @@ else fi fi +if [ "$DRY_RUN" = false ] && [ "$RELEASE_VERSION" != "$CURRENT_VERSION" ]; then + commit_release_version "$RELEASE_VERSION" +elif [ "$DRY_RUN" = true ] && [ "$RELEASE_VERSION" != "$CURRENT_VERSION" ]; then + echo -e "${YELLOW}[DRY RUN] Would update build.gradle and README.md to ${RELEASE_VERSION}${NC}" +fi +CURRENT_VERSION="$RELEASE_VERSION" + # Check if version has already been released (git tag exists) TAG="v${CURRENT_VERSION}" if git rev-parse "$TAG" >/dev/null 2>&1 || git ls-remote --tags origin | grep -q "refs/tags/$TAG$"; then @@ -231,32 +240,6 @@ if git rev-parse "$TAG" >/dev/null 2>&1 || git ls-remote --tags origin | grep -q fi fi fi -# Handle version bump -if [ -n "$BUMP_TYPE" ]; then - NEW_VERSION=$(bump_version "$CURRENT_VERSION" "$BUMP_TYPE") - echo -e "Bumping version to: ${GREEN}${NEW_VERSION}${NC}" - - if [ "$DRY_RUN" = false ]; then - update_version "$NEW_VERSION" - update_readme_version "$NEW_VERSION" - generate_changelog "$NEW_VERSION" - - # Commit version change - if ! git add build.gradle README.md CHANGELOG.md; then - echo -e "${RED}Error: Failed to add files to git. Please resolve the issue and try again.${NC}" >&2 - exit 1 - fi - if ! git commit -m "Release version ${NEW_VERSION}"; then - echo -e "${RED}Error: Failed to commit version change. Please resolve the issue and try again.${NC}" >&2 - exit 1 - fi - else - echo -e "${YELLOW}[DRY RUN] Would update version to ${NEW_VERSION}${NC}" - fi - - CURRENT_VERSION=$NEW_VERSION -fi - echo "" echo -e "Releasing version: ${GREEN}${CURRENT_VERSION}${NC}" echo "" From efd6e4a0a71226f712dbe7c6c93a1701a3426bb4 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 14:09:29 +0200 Subject: [PATCH 02/11] address GUI interaction review feedback --- build.gradle | 9 +-- docs/API-Guide.md | 5 ++ .../groovy/se/alipsa/gi/AbstractInOut.groovy | 13 +++-- .../main/groovy/se/alipsa/gi/FileUtils.groovy | 10 +++- .../groovy/se/alipsa/gi/GuiInteraction.groovy | 21 +++---- .../se/alipsa/gi/AbstractInOutTest.groovy | 24 ++++++++ .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 10 ++++ .../groovy/se/alipsa/gi/console/InOut.groovy | 33 +++++++++-- .../gi/console/ConsolePromptTest.groovy | 13 +++-- gi-fx/README.md | 4 +- gi-fx/build.gradle | 3 +- .../main/groovy/se/alipsa/gi/fx/InOut.groovy | 55 +++++++++++++------ .../se/alipsa/gi/fx/InOutHeadlessTest.groovy | 5 -- .../se/alipsa/gi/fx/InOutSvgTest.groovy | 16 ++++++ gi-swing/build.gradle | 5 +- .../groovy/se/alipsa/gi/swing/InOut.groovy | 18 +++++- .../alipsa/gi/swing/InOutHeadlessTest.groovy | 5 -- .../se/alipsa/gi/swing/InOutSvgTest.groovy | 16 ++++++ release.md | 6 ++ release.sh | 12 ++-- 20 files changed, 208 insertions(+), 75 deletions(-) create mode 100644 gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy create mode 100644 gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy diff --git a/build.gradle b/build.gradle index 234579e..e4c2247 100644 --- a/build.gradle +++ b/build.gradle @@ -68,18 +68,15 @@ subprojects { reportLevel = 'medium' excludeFilter = rootProject.file('config/spotbugs-exclude.xml') } - tasks.withType(com.github.spotbugs.snom.SpotBugsTask).configureEach { + tasks.withType(com.github.spotbugs.snom.SpotBugsTask).configureEach { spotbugsTask -> notCompatibleWithConfigurationCache("SpotBugs does not support configuration cache yet") + def runtimeConfigurationName = spotbugsTask.name == 'spotbugsTest' ? 'testRuntimeClasspath' : 'runtimeClasspath' + auxClassPaths.from(project.configurations.named(runtimeConfigurationName)) reports { html.required = true xml.required = false } } - afterEvaluate { - tasks.withType(com.github.spotbugs.snom.SpotBugsTask).configureEach { - auxClassPaths.from(configurations.runtimeClasspath) - } - } } } diff --git a/docs/API-Guide.md b/docs/API-Guide.md index cf72e06..b6d99b3 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -257,8 +257,13 @@ if (url != null) { def content = url.text } +// Missing paths return null; use File directly for a file that will be created later. +def output = new File("build/report.html") + // Extract filename from path def filename = FileUtils.baseName("/path/to/data.csv") // "data.csv" +// A # in a local filename is preserved; URL fragments are stripped. +assert FileUtils.baseName("/tmp/report#2.pdf") == "report#2.pdf" ``` ## Gade Compatibility diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index fa87b1e..5914846 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -13,6 +13,7 @@ import java.awt.datatransfer.ClipboardOwner import java.awt.datatransfer.DataFlavor import java.awt.datatransfer.StringSelection import java.awt.datatransfer.Transferable +import java.io.InputStream import java.nio.file.Paths import java.util.concurrent.ExecutionException; @@ -34,7 +35,7 @@ abstract class AbstractInOut implements GuiInteraction { try { URL url = new URL(urlString) con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) + con.setInstanceFollowRedirects(true) con.setRequestMethod("HEAD") con.setConnectTimeout(timeout) con.setReadTimeout(timeout) @@ -43,14 +44,18 @@ abstract class AbstractInOut implements GuiInteraction { responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { con.disconnect() con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) + con.setInstanceFollowRedirects(true) con.setRequestMethod("GET") + con.setRequestProperty("Range", "bytes=0-0") con.setConnectTimeout(timeout) con.setReadTimeout(timeout) responseCode = con.getResponseCode() + try (InputStream response = con.getInputStream()) { + response.readNBytes(1) + } } - // Accept 2xx (success) and 3xx (redirect) status codes - return responseCode >= 200 && responseCode < 400 + // Redirects are followed, so only a successful final response counts. + return responseCode >= 200 && responseCode < 300 } catch (RuntimeException | IOException ignored) { return false } finally { diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index 55fe41e..a556ee8 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -14,26 +14,30 @@ class FileUtils { /** * Extracts the base filename from a path or URL string. *

- * Handles both Unix and Windows path separators, and strips query strings from URLs. + * Handles both Unix and Windows path separators, and strips query strings from paths or URLs. + * URL fragments are stripped only from URL-shaped input; {@code #} remains valid in local filenames. *

* Examples: *

    *
  • {@code baseName("/path/to/file.txt")} returns {@code "file.txt"}
    • *
  • {@code baseName("C:\\path\\to\\file.txt")} returns {@code "file.txt"}
    • *
  • {@code baseName("http://example.com/file.txt?param=1")} returns {@code "file.txt"}
    • + *
  • {@code baseName("/tmp/report#2.pdf")} returns {@code "report#2.pdf"}
    • *
  • {@code baseName("filename")} returns {@code "filename"}
    • *
  • {@code baseName("/path/to/dir/")} returns {@code "/path/to/dir/"} (empty basename)
    • *
* * @param url the path or URL string to extract the filename from - * @return the base filename, or the original string if no path separator is found, + * @return the base filename, or the original path if it ends with a separator, * or {@code null} if the input is {@code null} */ static String baseName(String url) { if (url == null) return null url = url.replace('\\', '/') int queryIndex = url.indexOf('?') - int fragmentIndex = url.indexOf('#') + boolean urlLike = url ==~ /^[a-zA-Z][a-zA-Z0-9+.-]*:\/\/.*$/ || + url.startsWith('file:') || url.startsWith('jar:') + int fragmentIndex = urlLike ? url.indexOf('#') : -1 int suffixIndex = queryIndex >= 0 && fragmentIndex >= 0 ? Math.min(queryIndex, fragmentIndex) : Math.max(queryIndex, fragmentIndex) if (suffixIndex >= 0) { diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index 4a1e326..5052881 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -107,7 +107,8 @@ interface GuiInteraction { * * @param urlString the URL to check * @param timeout connection timeout in milliseconds - * @return {@code true} if the URL returns a 2xx or 3xx response, {@code false} otherwise + * @return {@code true} if the URL returns a successful 2xx response after redirects, + * {@code false} otherwise */ boolean urlExists(String urlString, int timeout) @@ -137,7 +138,7 @@ interface GuiInteraction { * absolute path of the file system (not only the classpath's). * * @param resource the path to the resource - * @return the URL representation of the resource + * @return the URL representation of the resource, or {@code null} if it does not exist */ URL getResourceUrl(String resource) @@ -151,7 +152,8 @@ interface GuiInteraction { * ) * * @param namedParams a key/value map with the parameter name and its value - * @return the user input prompted for + * @return the user input prompted for, or {@code null} if cancelled + * @throws IllegalArgumentException if {@code namedParams} is {@code null} * @throws ExecutionException if a threading issue occurs * @throws InterruptedException if a threading interrupt issue occurs */ @@ -199,8 +201,7 @@ interface GuiInteraction { * Prompts the user to select a year and month. * * @param message the prompt message to display - * @return the selected YearMonth - * @throws java.time.format.DateTimeParseException if input cannot be parsed + * @return the selected YearMonth, or {@code null} if cancelled or input cannot be parsed */ YearMonth promptYearMonth(String message); @@ -212,7 +213,7 @@ interface GuiInteraction { * @param from the earliest selectable YearMonth * @param to the latest selectable YearMonth * @param initial the initially selected YearMonth - * @return the selected YearMonth + * @return the selected YearMonth, the initial value for invalid input, or {@code null} if cancelled */ YearMonth promptYearMonth(String title, String message, YearMonth from, YearMonth to, YearMonth initial); @@ -222,7 +223,7 @@ interface GuiInteraction { * @param title the dialog title * @param message the prompt message to display * @param defaultValue the initially selected date - * @return the selected LocalDate + * @return the selected LocalDate, the default value for invalid input, or {@code null} if cancelled */ LocalDate promptDate(String title, String message, LocalDate defaultValue); @@ -234,8 +235,8 @@ interface GuiInteraction { * @param message the prompt message to display * @param options the collection of options to choose from (must not be empty) * @param defaultValue the initially selected option - * @return the selected option - * @throws IllegalArgumentException if options collection is empty + * @return the selected option, or {@code null} if cancelled + * @throws IllegalArgumentException if options collection is null or empty */ Object promptSelect(String title, String headerText, String message, Collection options, Object defaultValue); @@ -244,7 +245,7 @@ interface GuiInteraction { * * @param message the prompt message to display * @param options the collection of options to choose from (must not be null or empty) - * @return the selected option + * @return the selected option, or {@code null} if cancelled * @throws IllegalArgumentException if options collection is null or empty */ Object promptSelect(String message, Collection options); diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 50cd249..2990333 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -12,6 +12,7 @@ import java.time.LocalDate import java.time.YearMonth import com.sun.net.httpserver.HttpServer import java.net.InetSocketAddress +import java.util.concurrent.atomic.AtomicReference import static org.junit.jupiter.api.Assertions.* @@ -195,11 +196,13 @@ class AbstractInOutTest { @Test void urlExistsFallsBackToGetWhenHeadIsUnsupported() { + AtomicReference rangeHeader = new AtomicReference<>() HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) server.createContext("/health") { exchange -> if (exchange.requestMethod == "HEAD") { exchange.sendResponseHeaders(405, -1) } else { + rangeHeader.set(exchange.requestHeaders.getFirst("Range")) byte[] response = "ok".bytes exchange.sendResponseHeaders(200, response.length) exchange.responseBody.write(response) @@ -210,6 +213,27 @@ class AbstractInOutTest { server.start() try { assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) + assertEquals("bytes=0-0", rangeHeader.get()) + } finally { + server.stop(0) + } + } + + @Test + void urlExistsFollowsRedirectsAndChecksTheFinalResponse() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/redirect") { exchange -> + exchange.responseHeaders.add("Location", "/missing") + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + server.createContext("/missing") { exchange -> + exchange.sendResponseHeaders(404, -1) + exchange.close() + } + server.start() + try { + assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/redirect", 2000)) } finally { server.stop(0) } diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index 769cffd..67eb6e5 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -40,6 +40,16 @@ class FileUtilsTest { assertEquals("file.txt", FileUtils.baseName("file.txt?param=value")) } + @Test + void testBaseNamePreservesFragmentCharactersInLocalPaths() { + assertEquals("report#2.pdf", FileUtils.baseName("/tmp/report#2.pdf")) + } + + @Test + void testBaseNameStripsUrlFragments() { + assertEquals("file.txt", FileUtils.baseName("https://example.com/file.txt#section")) + } + @Test void testBaseNameWithNull() { assertNull(FileUtils.baseName(null)) diff --git a/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy b/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy index bb7a036..a8e5fb9 100644 --- a/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy +++ b/gi-console/src/main/groovy/se/alipsa/gi/console/InOut.groovy @@ -16,6 +16,7 @@ import java.awt.datatransfer.Clipboard import java.awt.datatransfer.DataFlavor import java.time.LocalDate import java.time.YearMonth +import java.time.format.DateTimeParseException import java.util.concurrent.ExecutionException @CompileStatic @@ -73,7 +74,12 @@ class InOut extends AbstractInOut { if (yearMonth == null || yearMonth.trim().isEmpty()) { return null } - return YearMonth.parse(yearMonth.trim()) + try { + return YearMonth.parse(yearMonth.trim()) + } catch (DateTimeParseException e) { + println("Invalid year-month '$yearMonth'. Returning null") + return null + } } @Override @@ -85,9 +91,18 @@ class InOut extends AbstractInOut { if (yearMonth.trim().isEmpty()) { return initial } - YearMonth selected = YearMonth.parse(yearMonth.trim()) + YearMonth selected + try { + selected = YearMonth.parse(yearMonth.trim()) + } catch (DateTimeParseException e) { + println("Invalid year-month '$yearMonth'. Returning initial value $initial") + return initial + } if (from != null && selected.isBefore(from) || to != null && selected.isAfter(to)) { - throw new IllegalArgumentException("Selected year-month $selected is outside the range $from to $to") + String range = from != null && to != null ? "$from to $to" : + from != null ? "on or after $from" : "on or before $to" + println("Selected year-month $selected is outside the range $range. Returning initial value $initial") + return initial } return selected } @@ -101,7 +116,12 @@ class InOut extends AbstractInOut { if (date.trim().isEmpty()) { return defaultValue } - return LocalDate.parse(date.trim()) + try { + return LocalDate.parse(date.trim()) + } catch (DateTimeParseException e) { + println("Invalid date '$date'. Returning default value $defaultValue") + return defaultValue + } } @Override @@ -119,7 +139,10 @@ class InOut extends AbstractInOut { } println "Default value is $defaultValue" String input = read(message) - if (input != null && input.trim().isInteger()) { + if (input == null) { + return null + } + if (input.trim().isInteger()) { int index = input.trim().toInteger() if (index >= 0 && index < optionList.size()) { return optionList[index] diff --git a/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy index 0d5117e..c846ca7 100644 --- a/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy +++ b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy @@ -34,13 +34,14 @@ class ConsolePromptTest { } @Test - void rangedYearMonthRejectsValuesOutsideRange() { + void rangedYearMonthFallsBackForInvalidValues() { InOut inOut = new InOut() - inOut.sysin = new BufferedReader(new StringReader('2027-01\n')) + inOut.sysin = new BufferedReader(new StringReader('2027-01\nnot-a-month\n')) - assertThrows(IllegalArgumentException) { - inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), null) - } + assertEquals(YearMonth.of(2026, 12), + inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), YearMonth.of(2026, 12))) + assertEquals(YearMonth.of(2026, 12), + inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), YearMonth.of(2026, 12))) } @Test @@ -48,7 +49,7 @@ class ConsolePromptTest { InOut inOut = new InOut() inOut.sysin = new BufferedReader(new StringReader('')) - assertEquals('first', inOut.promptSelect('title', '', 'choice', ['first', 'second'], 'first')) + assertNull(inOut.promptSelect('title', '', 'choice', ['first', 'second'], 'first')) assertThrows(IllegalArgumentException) { inOut.promptSelect('title', '', 'choice', [], null) } diff --git a/gi-fx/README.md b/gi-fx/README.md index 6ef65be..acecc59 100644 --- a/gi-fx/README.md +++ b/gi-fx/README.md @@ -15,7 +15,7 @@ This module provides JavaFX-based dialogs and viewers for user interaction. It o - Native JavaFX dialogs for file/directory selection - Rich date and year-month pickers with calendar UI -- WebView-based HTML and SVG rendering +- WebView-based HTML rendering - Markdown viewing with full formatting support - Chart display via Matrix Charts integration @@ -82,7 +82,7 @@ dependencies { - JavaFX must be available at runtime (either bundled in JDK or added as dependencies) - Thread safety: UI operations are automatically dispatched to the FX Application Thread -- SVG files are rendered using WebView +- SVG files are parsed and rendered as JavaFX nodes without WebView ## API Documentation diff --git a/gi-fx/build.gradle b/gi-fx/build.gradle index e1107c2..4d79165 100644 --- a/gi-fx/build.gradle +++ b/gi-fx/build.gradle @@ -63,7 +63,8 @@ dependencies { } test { - useJUnitPlatform() + useJUnitPlatform() + systemProperty 'java.awt.headless', 'true' } tasks.named('javadocJar', Jar) { diff --git a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy index 3202730..f98f29f 100644 --- a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy +++ b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy @@ -19,7 +19,6 @@ import javafx.scene.input.Clipboard import javafx.scene.input.ClipboardContent import javafx.scene.input.DataFormat import javafx.scene.layout.FlowPane -import javafx.scene.web.WebView import javafx.stage.DirectoryChooser import javafx.stage.FileChooser import javafx.stage.Modality @@ -35,6 +34,7 @@ import se.alipsa.matrix.core.util.Logger import javax.swing.JComponent import java.awt.GraphicsEnvironment +import java.nio.charset.StandardCharsets import java.time.LocalDate import java.time.YearMonth import java.util.concurrent.Callable @@ -319,27 +319,23 @@ class InOut extends AbstractInOut { try { String contentType = getContentType(file) if ("image/svg+xml" == contentType) { - Platform.runLater(() -> { - final WebView browser = new WebView() - browser.getEngine().load(url.toExternalForm()) - display(browser, title) - }); + displaySvg(url, title) return } } catch (IOException e) { log.error("Failed to detect image content type", e) } } - if (url.toExternalForm().toLowerCase(Locale.ROOT).contains('.svg')) { - Platform.runLater(() -> { - final WebView browser = new WebView() - browser.getEngine().load(url.toExternalForm()) - display(browser, title) - }); + if (isSvgResource(url)) { + displaySvg(url, title) return } - Image img = new Image(url.toExternalForm()) - display(img, title) + try { + Image img = new Image(url.toExternalForm()) + display(img, title) + } catch (RuntimeException e) { + log.error("Failed to display image {}", fileName, e) + } } void display(Image img, String... title) { @@ -347,6 +343,21 @@ class InOut extends AbstractInOut { display(node, title) } + private void displaySvg(URL url, String... title) { + Platform.runLater(() -> { + try (InputStream input = url.openStream()) { + String svgContent = new String(input.readAllBytes(), StandardCharsets.UTF_8) + display(ChartToJfx.export(svgContent), title) + } catch (IOException | RuntimeException e) { + log.error("Failed to display SVG {}", url, e) + } + }) + } + + static boolean isSvgResource(URL url) { + return url != null && FileUtils.baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') + } + @Override void display(File file, String... title) { if (file == null || !file.exists()) { @@ -416,7 +427,7 @@ class InOut extends AbstractInOut { } void saveToClipboard(String string) { - runOnFxThread(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putString(string) getClipboard().setContent(content) @@ -425,7 +436,7 @@ class InOut extends AbstractInOut { } void saveToClipboard(File file) { - runOnFxThread(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putFiles(List.of(file)) getClipboard().setContent(content) @@ -434,7 +445,7 @@ class InOut extends AbstractInOut { } void saveToClipboard(Image img) { - runOnFxThread(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putImage(img) getClipboard().setContent(content) @@ -443,7 +454,7 @@ class InOut extends AbstractInOut { } void saveToClipboard(Object obj, DataFormat format) { - runOnFxThread(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.put(format, obj) getClipboard().setContent(content) @@ -509,6 +520,14 @@ class InOut extends AbstractInOut { } } + private static void runOnFxThreadAsync(Runnable action) { + if (Platform.isFxApplicationThread()) { + action.run() + } else { + Platform.runLater(action) + } + } + private static T runOnFxThreadChecked(Callable action) throws ExecutionException, InterruptedException { if (Platform.isFxApplicationThread()) { diff --git a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy index d1a258e..2756b6f 100644 --- a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy +++ b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy @@ -2,17 +2,12 @@ package se.alipsa.gi.fx import org.junit.jupiter.api.Test -import java.awt.GraphicsEnvironment - import static org.junit.jupiter.api.Assertions.assertThrows -import static org.junit.jupiter.api.Assumptions.assumeTrue class InOutHeadlessTest { @Test void headlessConstructionReportsTheExpectedException() { - assumeTrue(GraphicsEnvironment.isHeadless()) - assertThrows(UnsupportedOperationException) { new InOut() } diff --git a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy new file mode 100644 index 0000000..205c636 --- /dev/null +++ b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy @@ -0,0 +1,16 @@ +package se.alipsa.gi.fx + +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertFalse +import static org.junit.jupiter.api.Assertions.assertTrue + +class InOutSvgTest { + + @Test + void detectsOnlySvgResourceNames() { + assertTrue(InOut.isSvgResource(new URL('file:/tmp/report.svg'))) + assertFalse(InOut.isSvgResource(new URL('file:/tmp/report.svg.png'))) + assertFalse(InOut.isSvgResource(new URL('file:/tmp/svgs.svg/logo.png'))) + } +} diff --git a/gi-swing/build.gradle b/gi-swing/build.gradle index 6dbec6e..b5083c1 100644 --- a/gi-swing/build.gradle +++ b/gi-swing/build.gradle @@ -40,7 +40,7 @@ dependencies { def javaFxVersion = libs.versions.javafx.get() implementation project(':gi-common') - compileOnly libs.groovy.all + compileOnly libs.groovy implementation libs.swing.widgets // JavaFX is needed at compile-time only for matrix-charts type resolution @@ -52,7 +52,7 @@ dependencies { // SVG rendering is handled by matrix-charts SvgPanel testImplementation platform(libs.junit.bom) testImplementation libs.junit.jupiter - testImplementation libs.groovy.all + testImplementation libs.groovy testRuntimeOnly libs.junit.platform.launcher } @@ -67,6 +67,7 @@ repositories { test { useJUnitPlatform() + systemProperty 'java.awt.headless', 'true' } publishing { diff --git a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy index 52fcee8..029e101 100644 --- a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy +++ b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy @@ -304,7 +304,15 @@ class InOut extends AbstractInOut { log.warn("Cannot display image: Failed to find {}", fileName) return } - File file = resource.protocol == 'file' ? new File(resource.toURI()) : null + File file = null + if (resource.protocol == 'file') { + try { + file = new File(resource.toURI()) + } catch (URISyntaxException e) { + log.warn("Cannot display image: Invalid resource URL {}", resource, e) + return + } + } if (file != null && file.exists()) { try { String contentType = getContentType(file) @@ -316,7 +324,7 @@ class InOut extends AbstractInOut { log.error("Error detecting content type", e) return } - } else if (resource.toExternalForm().toLowerCase(Locale.ROOT).contains('.svg')) { + } else if (isSvgResource(resource)) { displaySvg(resource, title) return } @@ -330,7 +338,7 @@ class InOut extends AbstractInOut { } /** - * Displays an SVG file using Apache Batik's JSVGCanvas. + * Displays an SVG file using matrix-charts' JSVG-backed panel. */ private void displaySvg(URL svgUrl, String... title) { String svg @@ -350,6 +358,10 @@ class InOut extends AbstractInOut { frame.setVisible(true) } + static boolean isSvgResource(URL url) { + return url != null && FileUtils.baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') + } + @Override void display(File file, String... title) { if (file == null || !file.exists()) { diff --git a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy index 518ad47..0a9a920 100644 --- a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy +++ b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy @@ -2,17 +2,12 @@ package se.alipsa.gi.swing import org.junit.jupiter.api.Test -import java.awt.GraphicsEnvironment - import static org.junit.jupiter.api.Assertions.assertThrows -import static org.junit.jupiter.api.Assumptions.assumeTrue class InOutHeadlessTest { @Test void headlessConstructionReportsTheExpectedException() { - assumeTrue(GraphicsEnvironment.isHeadless()) - assertThrows(UnsupportedOperationException) { new InOut() } diff --git a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy new file mode 100644 index 0000000..1f0fb56 --- /dev/null +++ b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy @@ -0,0 +1,16 @@ +package se.alipsa.gi.swing + +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertFalse +import static org.junit.jupiter.api.Assertions.assertTrue + +class InOutSvgTest { + + @Test + void detectsOnlySvgResourceNames() { + assertTrue(InOut.isSvgResource(new URL('file:/tmp/report.svg'))) + assertFalse(InOut.isSvgResource(new URL('file:/tmp/report.svg.png'))) + assertFalse(InOut.isSvgResource(new URL('file:/tmp/svgs.svg/logo.png'))) + } +} diff --git a/release.md b/release.md index ac698e3..036f32d 100644 --- a/release.md +++ b/release.md @@ -1,6 +1,12 @@ # Gui Interaction Release Notes ## 0.4.0 - in progress +- Breaking: `FileUtils.getResourceUrl` returns `null` for paths that do not exist; use `File` for output targets that will be created later. +- Breaking: Swing and JavaFX display resolution now prefers classpath resources when a name exists both on the classpath and in the working directory. +- Common: local filenames preserve `#`; URL fragments are stripped only from URL-shaped inputs. +- Console: EOF consistently returns `null`; malformed or out-of-range date input falls back to the documented default/initial value. +- JavaFX: SVG files are rendered directly as JavaFX nodes; SVG display no longer creates a WebView. +- Build: Matrix BOM updated to 2.5.1 and Swing Widgets to 1.1.2. ## 0.3.0 - 2026-02-03 - Build: introduced Gradle version catalog `gradle/libs.versions.toml` to centralize dependency versions across modules. diff --git a/release.sh b/release.sh index b3f6f8f..4ae72fd 100755 --- a/release.sh +++ b/release.sh @@ -219,11 +219,6 @@ else fi fi -if [ "$DRY_RUN" = false ] && [ "$RELEASE_VERSION" != "$CURRENT_VERSION" ]; then - commit_release_version "$RELEASE_VERSION" -elif [ "$DRY_RUN" = true ] && [ "$RELEASE_VERSION" != "$CURRENT_VERSION" ]; then - echo -e "${YELLOW}[DRY RUN] Would update build.gradle and README.md to ${RELEASE_VERSION}${NC}" -fi CURRENT_VERSION="$RELEASE_VERSION" # Check if version has already been released (git tag exists) @@ -240,6 +235,13 @@ if git rev-parse "$TAG" >/dev/null 2>&1 || git ls-remote --tags origin | grep -q fi fi fi + +# Only mutate and commit release files after the tag check has passed. +if [ "$DRY_RUN" = false ] && [ "$RELEASE_VERSION" != "$(get_version)" ]; then + commit_release_version "$RELEASE_VERSION" +elif [ "$DRY_RUN" = true ] && [ "$RELEASE_VERSION" != "$(get_version)" ]; then + echo -e "${YELLOW}[DRY RUN] Would update build.gradle and README.md to ${RELEASE_VERSION}${NC}" +fi echo "" echo -e "Releasing version: ${GREEN}${CURRENT_VERSION}${NC}" echo "" From d5395e63d68c04b24d4db543ab567f70f58e1a01 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 14:48:16 +0200 Subject: [PATCH 03/11] fix review regressions --- build.gradle | 10 ++- .../groovy/se/alipsa/gi/AbstractInOut.groovy | 82 +++++++++++++------ .../main/groovy/se/alipsa/gi/FileUtils.groovy | 9 ++ .../groovy/se/alipsa/gi/GuiInteraction.groovy | 2 +- .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 7 ++ .../gi/console/ConsolePromptTest.groovy | 11 ++- gi-fx/README.md | 2 +- .../main/groovy/se/alipsa/gi/fx/InOut.groovy | 28 ++++--- .../se/alipsa/gi/fx/InOutSvgTest.groovy | 16 ---- .../groovy/se/alipsa/gi/swing/InOut.groovy | 7 +- .../se/alipsa/gi/swing/InOutSvgTest.groovy | 16 ---- release.md | 2 +- release.sh | 23 ++++-- 13 files changed, 126 insertions(+), 89 deletions(-) delete mode 100644 gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy delete mode 100644 gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy diff --git a/build.gradle b/build.gradle index e4c2247..dbce296 100644 --- a/build.gradle +++ b/build.gradle @@ -70,8 +70,14 @@ subprojects { } tasks.withType(com.github.spotbugs.snom.SpotBugsTask).configureEach { spotbugsTask -> notCompatibleWithConfigurationCache("SpotBugs does not support configuration cache yet") - def runtimeConfigurationName = spotbugsTask.name == 'spotbugsTest' ? 'testRuntimeClasspath' : 'runtimeClasspath' - auxClassPaths.from(project.configurations.named(runtimeConfigurationName)) + String suffix = spotbugsTask.name.startsWith('spotbugs') ? + spotbugsTask.name.substring('spotbugs'.length()) : '' + String sourceSetName = suffix.isEmpty() ? 'main' : + suffix.substring(0, 1).toLowerCase() + suffix.substring(1) + def sourceSet = project.sourceSets.findByName(sourceSetName) + if (sourceSet != null) { + auxClassPaths.from(sourceSet.compileClasspath, sourceSet.runtimeClasspath) + } reports { html.required = true xml.required = false diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index 5914846..fed4f0d 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -13,13 +13,14 @@ import java.awt.datatransfer.ClipboardOwner import java.awt.datatransfer.DataFlavor import java.awt.datatransfer.StringSelection import java.awt.datatransfer.Transferable -import java.io.InputStream import java.nio.file.Paths import java.util.concurrent.ExecutionException; @CompileStatic abstract class AbstractInOut implements GuiInteraction { + private static final int MAX_REDIRECTS = 5 + private Parser markdownParser private HtmlRenderer htmlRenderer protected def clipboard @@ -31,37 +32,68 @@ abstract class AbstractInOut implements GuiInteraction { @Override boolean urlExists(String urlString, int timeout) { - HttpURLConnection con = null try { URL url = new URL(urlString) - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(true) - con.setRequestMethod("HEAD") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) - int responseCode = con.getResponseCode() - if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || - responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { - con.disconnect() - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(true) - con.setRequestMethod("GET") - con.setRequestProperty("Range", "bytes=0-0") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) - responseCode = con.getResponseCode() - try (InputStream response = con.getInputStream()) { - response.readNBytes(1) + for (int redirect = 0; redirect <= MAX_REDIRECTS; redirect++) { + HttpURLConnection con = null + try { + con = (HttpURLConnection) url.openConnection() + con.setInstanceFollowRedirects(false) + con.setRequestMethod("HEAD") + con.setConnectTimeout(timeout) + con.setReadTimeout(timeout) + int responseCode = con.getResponseCode() + if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || + responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { + con.disconnect() + con = (HttpURLConnection) url.openConnection() + con.setInstanceFollowRedirects(false) + con.setRequestMethod("GET") + con.setRequestProperty("Range", "bytes=0-0") + con.setConnectTimeout(timeout) + con.setReadTimeout(timeout) + responseCode = con.getResponseCode() + if (responseCode == 416) { + con.disconnect() + con = (HttpURLConnection) url.openConnection() + con.setInstanceFollowRedirects(false) + con.setRequestMethod("GET") + con.setConnectTimeout(timeout) + con.setReadTimeout(timeout) + responseCode = con.getResponseCode() + } + closeResponseBody(con, responseCode) + } + if (responseCode >= 300 && responseCode < 400) { + String location = con.getHeaderField("Location") + if (location == null || redirect == MAX_REDIRECTS) { + return location == null + } + url = new URL(url, location) + continue + } + return responseCode >= 200 && responseCode < 300 + } finally { + if (con != null) { + con.disconnect() + } } } - // Redirects are followed, so only a successful final response counts. - return responseCode >= 200 && responseCode < 300 } catch (RuntimeException | IOException ignored) { return false - } finally { - if (con != null) { - con.disconnect() + } + return false + } + + private static void closeResponseBody(HttpURLConnection con, int responseCode) { + try { + InputStream response = responseCode >= 200 && responseCode < 400 ? + con.getInputStream() : con.getErrorStream() + if (response != null) { + response.close() } + } catch (IOException ignored) { + // The response code is authoritative for this existence check. } } diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index a556ee8..853d0b7 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -51,6 +51,15 @@ class FileUtils { return basename.length() > 0 ? basename : url } + /** + * Returns whether a URL identifies an SVG resource by its final path segment. + * Query strings and URL fragments are ignored, while names such as {@code report.svg.png} + * and {@code /svgs.svg/logo.png} are not treated as SVG resources. + */ + public static boolean isSvgResource(URL url) { + return url != null && baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') + } + /** * Finds a resource using multiple classloader strategies. *

diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index 5052881..9b95eba 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -235,7 +235,7 @@ interface GuiInteraction { * @param message the prompt message to display * @param options the collection of options to choose from (must not be empty) * @param defaultValue the initially selected option - * @return the selected option, or {@code null} if cancelled + * @return the selected option, the default value for invalid console input, or {@code null} if cancelled * @throws IllegalArgumentException if options collection is null or empty */ Object promptSelect(String title, String headerText, String message, Collection options, Object defaultValue); diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index 67eb6e5..a82053d 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -50,6 +50,13 @@ class FileUtilsTest { assertEquals("file.txt", FileUtils.baseName("https://example.com/file.txt#section")) } + @Test + void testSvgResourceDetectionUsesTheFinalPathSegment() { + assertTrue(FileUtils.isSvgResource(URI.create('file:/tmp/report.svg').toURL())) + assertFalse(FileUtils.isSvgResource(URI.create('file:/tmp/report.svg.png').toURL())) + assertFalse(FileUtils.isSvgResource(URI.create('file:/tmp/svgs.svg/logo.png').toURL())) + } + @Test void testBaseNameWithNull() { assertNull(FileUtils.baseName(null)) diff --git a/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy index c846ca7..3f9d5c7 100644 --- a/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy +++ b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy @@ -34,12 +34,19 @@ class ConsolePromptTest { } @Test - void rangedYearMonthFallsBackForInvalidValues() { + void rangedYearMonthFallsBackForOutOfRangeValues() { InOut inOut = new InOut() - inOut.sysin = new BufferedReader(new StringReader('2027-01\nnot-a-month\n')) + inOut.sysin = new BufferedReader(new StringReader('2027-01\n')) assertEquals(YearMonth.of(2026, 12), inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), YearMonth.of(2026, 12))) + } + + @Test + void rangedYearMonthFallsBackForUnparseableValues() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('not-a-month\n')) + assertEquals(YearMonth.of(2026, 12), inOut.promptYearMonth('title', 'month', YearMonth.of(2025, 1), YearMonth.of(2026, 12), YearMonth.of(2026, 12))) } diff --git a/gi-fx/README.md b/gi-fx/README.md index acecc59..394cf9b 100644 --- a/gi-fx/README.md +++ b/gi-fx/README.md @@ -82,7 +82,7 @@ dependencies { - JavaFX must be available at runtime (either bundled in JDK or added as dependencies) - Thread safety: UI operations are automatically dispatched to the FX Application Thread -- SVG files are parsed and rendered as JavaFX nodes without WebView +- SVG files are parsed and rendered as JavaFX nodes without WebView. This uses a lighter SVG backend than a browser, so advanced CSS, filters, and scripting may render differently or be unsupported. ## API Documentation diff --git a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy index f98f29f..f47a085 100644 --- a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy +++ b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy @@ -326,12 +326,16 @@ class InOut extends AbstractInOut { log.error("Failed to detect image content type", e) } } - if (isSvgResource(url)) { + if (FileUtils.isSvgResource(url)) { displaySvg(url, title) return } try { Image img = new Image(url.toExternalForm()) + if (img.isError()) { + log.error("Failed to display image {}", fileName, img.getException()) + return + } display(img, title) } catch (RuntimeException e) { log.error("Failed to display image {}", fileName, e) @@ -344,20 +348,24 @@ class InOut extends AbstractInOut { } private void displaySvg(URL url, String... title) { + byte[] svgBytes + try (InputStream input = url.openStream()) { + svgBytes = input.readAllBytes() + } catch (IOException e) { + log.error("Failed to read SVG {}", url, e) + return + } + String windowTitle = title.length > 0 ? title[0] : '' Platform.runLater(() -> { - try (InputStream input = url.openStream()) { - String svgContent = new String(input.readAllBytes(), StandardCharsets.UTF_8) - display(ChartToJfx.export(svgContent), title) - } catch (IOException | RuntimeException e) { - log.error("Failed to display SVG {}", url, e) + try { + Node node = ChartToJfx.export(new String(svgBytes, StandardCharsets.UTF_8)) + show(node, windowTitle) + } catch (RuntimeException e) { + log.error("Failed to parse SVG {}", url, e) } }) } - static boolean isSvgResource(URL url) { - return url != null && FileUtils.baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') - } - @Override void display(File file, String... title) { if (file == null || !file.exists()) { diff --git a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy deleted file mode 100644 index 205c636..0000000 --- a/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutSvgTest.groovy +++ /dev/null @@ -1,16 +0,0 @@ -package se.alipsa.gi.fx - -import org.junit.jupiter.api.Test - -import static org.junit.jupiter.api.Assertions.assertFalse -import static org.junit.jupiter.api.Assertions.assertTrue - -class InOutSvgTest { - - @Test - void detectsOnlySvgResourceNames() { - assertTrue(InOut.isSvgResource(new URL('file:/tmp/report.svg'))) - assertFalse(InOut.isSvgResource(new URL('file:/tmp/report.svg.png'))) - assertFalse(InOut.isSvgResource(new URL('file:/tmp/svgs.svg/logo.png'))) - } -} diff --git a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy index 029e101..fc51751 100644 --- a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy +++ b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy @@ -27,7 +27,6 @@ import se.alipsa.matrix.core.util.Logger import java.awt.GraphicsEnvironment import javax.swing.filechooser.FileNameExtensionFilter import java.util.concurrent.ExecutionException -import java.util.Locale @CompileStatic class InOut extends AbstractInOut { @@ -324,7 +323,7 @@ class InOut extends AbstractInOut { log.error("Error detecting content type", e) return } - } else if (isSvgResource(resource)) { + } else if (FileUtils.isSvgResource(resource)) { displaySvg(resource, title) return } @@ -358,10 +357,6 @@ class InOut extends AbstractInOut { frame.setVisible(true) } - static boolean isSvgResource(URL url) { - return url != null && FileUtils.baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') - } - @Override void display(File file, String... title) { if (file == null || !file.exists()) { diff --git a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy deleted file mode 100644 index 1f0fb56..0000000 --- a/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutSvgTest.groovy +++ /dev/null @@ -1,16 +0,0 @@ -package se.alipsa.gi.swing - -import org.junit.jupiter.api.Test - -import static org.junit.jupiter.api.Assertions.assertFalse -import static org.junit.jupiter.api.Assertions.assertTrue - -class InOutSvgTest { - - @Test - void detectsOnlySvgResourceNames() { - assertTrue(InOut.isSvgResource(new URL('file:/tmp/report.svg'))) - assertFalse(InOut.isSvgResource(new URL('file:/tmp/report.svg.png'))) - assertFalse(InOut.isSvgResource(new URL('file:/tmp/svgs.svg/logo.png'))) - } -} diff --git a/release.md b/release.md index 036f32d..d81f773 100644 --- a/release.md +++ b/release.md @@ -5,7 +5,7 @@ - Breaking: Swing and JavaFX display resolution now prefers classpath resources when a name exists both on the classpath and in the working directory. - Common: local filenames preserve `#`; URL fragments are stripped only from URL-shaped inputs. - Console: EOF consistently returns `null`; malformed or out-of-range date input falls back to the documented default/initial value. -- JavaFX: SVG files are rendered directly as JavaFX nodes; SVG display no longer creates a WebView. +- JavaFX: SVG files are rendered directly as JavaFX nodes; SVG display no longer creates a WebView. The SVG backend supports a subset of browser SVG features, so advanced CSS, filters, and scripting may render differently or be unsupported. - Build: Matrix BOM updated to 2.5.1 and Swing Widgets to 1.1.2. ## 0.3.0 - 2026-02-03 diff --git a/release.sh b/release.sh index 4ae72fd..35e4705 100755 --- a/release.sh +++ b/release.sh @@ -33,6 +33,7 @@ fi PROJECT=$(basename "$PWD") DRY_RUN=false BUMP_TYPE="" +README_NEEDS_UPDATE=false # Parse arguments while [[ $# -gt 0 ]]; do @@ -208,14 +209,7 @@ elif echo "$CURRENT_VERSION" | grep -q '\-SNAPSHOT'; then else RELEASE_VERSION="$CURRENT_VERSION" if ! check_readme_version "$CURRENT_VERSION"; then - if [ "$DRY_RUN" = true ]; then - echo -e "${YELLOW}[DRY RUN] Would update README.md to match version ${CURRENT_VERSION}${NC}" - else - read -p "Update README.md to version ${CURRENT_VERSION}? [Y/n]: " update_readme - if [[ ! "$update_readme" =~ ^[Nn]$ ]]; then - update_readme_version "$CURRENT_VERSION" - fi - fi + README_NEEDS_UPDATE=true fi fi @@ -236,11 +230,22 @@ if git rev-parse "$TAG" >/dev/null 2>&1 || git ls-remote --tags origin | grep -q fi fi -# Only mutate and commit release files after the tag check has passed. +# Only mutate release files after the tag check has passed. if [ "$DRY_RUN" = false ] && [ "$RELEASE_VERSION" != "$(get_version)" ]; then commit_release_version "$RELEASE_VERSION" elif [ "$DRY_RUN" = true ] && [ "$RELEASE_VERSION" != "$(get_version)" ]; then echo -e "${YELLOW}[DRY RUN] Would update build.gradle and README.md to ${RELEASE_VERSION}${NC}" +elif [ "$README_NEEDS_UPDATE" = true ]; then + if [ "$DRY_RUN" = true ]; then + echo -e "${YELLOW}[DRY RUN] Would update README.md to match version ${CURRENT_VERSION}${NC}" + else + read -p "Update README.md to version ${CURRENT_VERSION}? [Y/n]: " update_readme + if [[ ! "$update_readme" =~ ^[Nn]$ ]]; then + update_readme_version "$CURRENT_VERSION" + git add README.md + git commit -m "Update README version to ${CURRENT_VERSION}" + fi + fi fi echo "" echo -e "Releasing version: ${GREEN}${CURRENT_VERSION}${NC}" From 18cb02025847020fdfd0cddafb15a491816a3c68 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 15:03:58 +0200 Subject: [PATCH 04/11] cover redirect and rendering edge cases --- build.gradle | 2 +- .../groovy/se/alipsa/gi/AbstractInOut.groovy | 54 ++++++++++++------ .../main/groovy/se/alipsa/gi/FileUtils.groovy | 35 +++++++++++- .../groovy/se/alipsa/gi/GuiInteraction.groovy | 2 +- .../se/alipsa/gi/AbstractInOutTest.groovy | 55 +++++++++++++++++++ .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 10 ++++ .../main/groovy/se/alipsa/gi/fx/InOut.groovy | 35 ++++++++---- .../groovy/se/alipsa/gi/swing/InOut.groovy | 4 +- 8 files changed, 162 insertions(+), 35 deletions(-) diff --git a/build.gradle b/build.gradle index dbce296..45918fd 100644 --- a/build.gradle +++ b/build.gradle @@ -73,7 +73,7 @@ subprojects { String suffix = spotbugsTask.name.startsWith('spotbugs') ? spotbugsTask.name.substring('spotbugs'.length()) : '' String sourceSetName = suffix.isEmpty() ? 'main' : - suffix.substring(0, 1).toLowerCase() + suffix.substring(1) + suffix.substring(0, 1).toLowerCase(Locale.ROOT) + suffix.substring(1) def sourceSet = project.sourceSets.findByName(sourceSetName) if (sourceSet != null) { auxClassPaths.from(sourceSet.compileClasspath, sourceSet.runtimeClasspath) diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index fed4f0d..e287c13 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -15,6 +15,7 @@ import java.awt.datatransfer.StringSelection import java.awt.datatransfer.Transferable import java.nio.file.Paths import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit @CompileStatic abstract class AbstractInOut implements GuiInteraction { @@ -34,40 +35,33 @@ abstract class AbstractInOut implements GuiInteraction { boolean urlExists(String urlString, int timeout) { try { URL url = new URL(urlString) + long deadline = timeout > 0 ? + System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeout) : Long.MAX_VALUE for (int redirect = 0; redirect <= MAX_REDIRECTS; redirect++) { + if (!isHttpUrl(url) || timeout > 0 && System.nanoTime() >= deadline) { + return false + } HttpURLConnection con = null try { - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) - con.setRequestMethod("HEAD") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) + int remaining = remainingTimeout(timeout, deadline) + con = open(url, "HEAD", remaining, false) int responseCode = con.getResponseCode() if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { con.disconnect() - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) - con.setRequestMethod("GET") - con.setRequestProperty("Range", "bytes=0-0") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) + con = open(url, "GET", remainingTimeout(timeout, deadline), true) responseCode = con.getResponseCode() if (responseCode == 416) { con.disconnect() - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) - con.setRequestMethod("GET") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) + con = open(url, "GET", remainingTimeout(timeout, deadline), false) responseCode = con.getResponseCode() } - closeResponseBody(con, responseCode) } + closeResponseBody(con, responseCode) if (responseCode >= 300 && responseCode < 400) { String location = con.getHeaderField("Location") if (location == null || redirect == MAX_REDIRECTS) { - return location == null + return false } url = new URL(url, location) continue @@ -85,6 +79,30 @@ abstract class AbstractInOut implements GuiInteraction { return false } + private static HttpURLConnection open(URL url, String method, int timeout, boolean range) { + HttpURLConnection con = (HttpURLConnection) url.openConnection() + con.setInstanceFollowRedirects(false) + con.setRequestMethod(method) + if (range) { + con.setRequestProperty("Range", "bytes=0-0") + } + con.setConnectTimeout(timeout) + con.setReadTimeout(timeout) + return con + } + + private static boolean isHttpUrl(URL url) { + return "http".equalsIgnoreCase(url.protocol) || "https".equalsIgnoreCase(url.protocol) + } + + private static int remainingTimeout(int configuredTimeout, long deadline) { + if (configuredTimeout <= 0) { + return configuredTimeout + } + long remainingMillis = TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime()) + return (int) Math.max(1, Math.min(Integer.MAX_VALUE, remainingMillis)) + } + private static void closeResponseBody(HttpURLConnection con, int responseCode) { try { InputStream response = responseCode >= 200 && responseCode < 400 ? diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index 853d0b7..25878dd 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -2,6 +2,10 @@ package se.alipsa.gi import groovy.transform.CompileStatic +import java.nio.charset.Charset +import java.nio.charset.StandardCharsets +import java.util.regex.Pattern + /** * Utility class for file and resource operations. *

@@ -11,6 +15,9 @@ import groovy.transform.CompileStatic @CompileStatic class FileUtils { + private static final Pattern XML_ENCODING = Pattern.compile( + "(?i)<\\?xml[^>]*encoding\\s*=\\s*['\"]([^'\"]+)['\"]") + /** * Extracts the base filename from a path or URL string. *

@@ -56,10 +63,36 @@ class FileUtils { * Query strings and URL fragments are ignored, while names such as {@code report.svg.png} * and {@code /svgs.svg/logo.png} are not treated as SVG resources. */ - public static boolean isSvgResource(URL url) { + static boolean isSvgResource(URL url) { return url != null && baseName(url.toExternalForm()).toLowerCase(Locale.ROOT).endsWith('.svg') } + /** + * Decodes XML bytes using the encoding declared in the XML prolog when present. + * UTF-8 is used when no declaration or byte-order mark is available. + */ + static String decodeXml(byte[] content) { + if (content == null || content.length == 0) { + return '' + } + Charset charset = StandardCharsets.UTF_8 + if (content.length >= 2 && content[0] == (byte) 0xFF && content[1] == (byte) 0xFE) { + charset = StandardCharsets.UTF_16LE + } else if (content.length >= 2 && content[0] == (byte) 0xFE && content[1] == (byte) 0xFF) { + charset = StandardCharsets.UTF_16BE + } + String prefix = new String(content, 0, Math.min(content.length, 512), charset) + def matcher = XML_ENCODING.matcher(prefix) + if (matcher.find()) { + try { + charset = Charset.forName(matcher.group(1)) + } catch (IllegalArgumentException ignored) { + // Keep the UTF-8/BOM-derived fallback for an unknown declaration. + } + } + return new String(content, charset) + } + /** * Finds a resource using multiple classloader strategies. *

diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index 9b95eba..d94117b 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -106,7 +106,7 @@ interface GuiInteraction { * to verify the URL is reachable. * * @param urlString the URL to check - * @param timeout connection timeout in milliseconds + * @param timeout overall timeout budget in milliseconds, including redirect hops and retries * @return {@code true} if the URL returns a successful 2xx response after redirects, * {@code false} otherwise */ diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 2990333..85b4f2b 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -239,6 +239,61 @@ class AbstractInOutTest { } } + @Test + void urlExistsRejectsRedirectsWithoutLocation() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/bare-redirect") { exchange -> + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + server.start() + try { + assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/bare-redirect", 2000)) + } finally { + server.stop(0) + } + } + + @Test + void urlExistsStopsAtTheRedirectLimit() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/loop") { exchange -> + exchange.responseHeaders.add("Location", "/loop") + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + server.start() + try { + assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/loop", 2000)) + } finally { + server.stop(0) + } + } + + @Test + void urlExistsRetriesWithoutRangeAfterA416Response() { + AtomicReference rangeHeader = new AtomicReference<>() + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/range") { exchange -> + if (exchange.requestMethod == "HEAD") { + exchange.sendResponseHeaders(405, -1) + } else if (exchange.requestHeaders.getFirst("Range") != null) { + rangeHeader.set(exchange.requestHeaders.getFirst("Range")) + exchange.sendResponseHeaders(416, -1) + } else { + exchange.sendResponseHeaders(200, -1) + } + exchange.close() + } + server.start() + try { + assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/range", 2000)) + assertEquals("bytes=0-0", rangeHeader.get()) + } finally { + server.stop(0) + } + } + /** * Concrete implementation of AbstractInOut for testing purposes. * Provides minimal stub implementations for abstract methods. diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index a82053d..c42c104 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -55,6 +55,16 @@ class FileUtilsTest { assertTrue(FileUtils.isSvgResource(URI.create('file:/tmp/report.svg').toURL())) assertFalse(FileUtils.isSvgResource(URI.create('file:/tmp/report.svg.png').toURL())) assertFalse(FileUtils.isSvgResource(URI.create('file:/tmp/svgs.svg/logo.png').toURL())) + assertTrue(FileUtils.isSvgResource(URI.create('https://x/a.svg?v=2').toURL())) + assertFalse(FileUtils.isSvgResource(URI.create('https://x/download?file=a.svg').toURL())) + } + + @Test + void testDecodeXmlUsesTheDeclaredEncoding() { + byte[] content = 'caf\u00e9' + .getBytes('ISO-8859-1') + + assertTrue(FileUtils.decodeXml(content).contains('café')) } @Test diff --git a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy index f47a085..e33836a 100644 --- a/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy +++ b/gi-fx/src/main/groovy/se/alipsa/gi/fx/InOut.groovy @@ -34,7 +34,6 @@ import se.alipsa.matrix.core.util.Logger import javax.swing.JComponent import java.awt.GraphicsEnvironment -import java.nio.charset.StandardCharsets import java.time.LocalDate import java.time.YearMonth import java.util.concurrent.Callable @@ -314,8 +313,16 @@ class InOut extends AbstractInOut { log.warn("Cannot display image, Failed to find {}", fileName) return } - File file = new File(fileName); - if (file.exists()) { + File file = null + if ('file' == url.protocol) { + try { + file = new File(url.toURI()) + } catch (URISyntaxException e) { + log.warn("Cannot display image: Invalid resource URL {}", url, e) + return + } + } + if (file != null && file.exists()) { try { String contentType = getContentType(file) if ("image/svg+xml" == contentType) { @@ -358,8 +365,8 @@ class InOut extends AbstractInOut { String windowTitle = title.length > 0 ? title[0] : '' Platform.runLater(() -> { try { - Node node = ChartToJfx.export(new String(svgBytes, StandardCharsets.UTF_8)) - show(node, windowTitle) + Node node = ChartToJfx.export(FileUtils.decodeXml(svgBytes)) + showNow(node, windowTitle) } catch (RuntimeException e) { log.error("Failed to parse SVG {}", url, e) } @@ -420,16 +427,20 @@ class InOut extends AbstractInOut { private static void show(Node node, String title) { Platform.runLater { - Alert alert = new Alert(Alert.AlertType.INFORMATION) - alert.setHeaderText(null) - alert.setContentText(null) - alert.setTitle(title) - alert.getDialogPane().setContent(node) - alert.initModality(Modality.NONE) - alert.showAndWait() + showNow(node, title) } } + private static void showNow(Node node, String title) { + Alert alert = new Alert(Alert.AlertType.INFORMATION) + alert.setHeaderText(null) + alert.setContentText(null) + alert.setTitle(title) + alert.getDialogPane().setContent(node) + alert.initModality(Modality.NONE) + alert.showAndWait() + } + void setStyleSheetUrls(ObservableList styleSheetUrls) { this.styleSheetUrls = styleSheetUrls } diff --git a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy index fc51751..1565a4d 100644 --- a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy +++ b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy @@ -341,8 +341,8 @@ class InOut extends AbstractInOut { */ private void displaySvg(URL svgUrl, String... title) { String svg - try { - svg = svgUrl.getText("UTF-8") + try (InputStream input = svgUrl.openStream()) { + svg = FileUtils.decodeXml(input.readAllBytes()) } catch (IOException e) { log.error("Failed to read svg resource {}", svgUrl, e) return From d1c330a356790d5d55dd5f3d668e25f0c0025957 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 15:49:34 +0200 Subject: [PATCH 05/11] finish URL and encoding review fixes --- .../groovy/se/alipsa/gi/AbstractInOut.groovy | 27 ++++++------ .../main/groovy/se/alipsa/gi/FileUtils.groovy | 3 +- .../se/alipsa/gi/AbstractInOutTest.groovy | 43 +++++++++++++++++++ .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 28 ++++++++++++ release.md | 1 + release.sh | 12 ++++-- 6 files changed, 96 insertions(+), 18 deletions(-) diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index e287c13..2685f42 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -33,31 +33,38 @@ abstract class AbstractInOut implements GuiInteraction { @Override boolean urlExists(String urlString, int timeout) { + if (timeout < 0) { + throw new IllegalArgumentException("timeout cannot be negative") + } try { URL url = new URL(urlString) long deadline = timeout > 0 ? System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeout) : Long.MAX_VALUE for (int redirect = 0; redirect <= MAX_REDIRECTS; redirect++) { - if (!isHttpUrl(url) || timeout > 0 && System.nanoTime() >= deadline) { + if (!isHttpUrl(url) || !hasTimeRemaining(timeout, deadline)) { return false } HttpURLConnection con = null try { - int remaining = remainingTimeout(timeout, deadline) - con = open(url, "HEAD", remaining, false) + con = open(url, "HEAD", remainingTimeout(timeout, deadline), false) int responseCode = con.getResponseCode() if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { + if (!hasTimeRemaining(timeout, deadline)) { + return false + } con.disconnect() con = open(url, "GET", remainingTimeout(timeout, deadline), true) responseCode = con.getResponseCode() if (responseCode == 416) { + if (!hasTimeRemaining(timeout, deadline)) { + return false + } con.disconnect() con = open(url, "GET", remainingTimeout(timeout, deadline), false) responseCode = con.getResponseCode() } } - closeResponseBody(con, responseCode) if (responseCode >= 300 && responseCode < 400) { String location = con.getHeaderField("Location") if (location == null || redirect == MAX_REDIRECTS) { @@ -103,16 +110,8 @@ abstract class AbstractInOut implements GuiInteraction { return (int) Math.max(1, Math.min(Integer.MAX_VALUE, remainingMillis)) } - private static void closeResponseBody(HttpURLConnection con, int responseCode) { - try { - InputStream response = responseCode >= 200 && responseCode < 400 ? - con.getInputStream() : con.getErrorStream() - if (response != null) { - response.close() - } - } catch (IOException ignored) { - // The response code is authoritative for this existence check. - } + private static boolean hasTimeRemaining(int configuredTimeout, long deadline) { + return configuredTimeout <= 0 || System.nanoTime() < deadline } @Override diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index 25878dd..db99f51 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -90,7 +90,8 @@ class FileUtils { // Keep the UTF-8/BOM-derived fallback for an unknown declaration. } } - return new String(content, charset) + String decoded = new String(content, charset) + return decoded.startsWith('\uFEFF') ? decoded.substring(1) : decoded } /** diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 85b4f2b..5f46d82 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -182,6 +182,13 @@ class AbstractInOutTest { assertFalse(inOut.urlExists("not-a-valid-url", 1000)) } + @Test + void testUrlExistsRejectsNegativeTimeout() { + assertThrows(IllegalArgumentException) { + inOut.urlExists("http://127.0.0.1", -1) + } + } + @Test void testUrlExistsWithUnreachableHost() { // Non-existent hosts should return false with timeout @@ -239,6 +246,42 @@ class AbstractInOutTest { } } + @Test + void urlExistsResolvesRelativeRedirects() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/redirect") { exchange -> + exchange.responseHeaders.add("Location", "/ok") + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + server.createContext("/ok") { exchange -> + exchange.sendResponseHeaders(200, -1) + exchange.close() + } + server.start() + try { + assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/redirect", 2000)) + } finally { + server.stop(0) + } + } + + @Test + void urlExistsRejectsNonHttpRedirectTargets() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/redirect") { exchange -> + exchange.responseHeaders.add("Location", "file:/etc/hostname") + exchange.sendResponseHeaders(302, -1) + exchange.close() + } + server.start() + try { + assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/redirect", 2000)) + } finally { + server.stop(0) + } + } + @Test void urlExistsRejectsRedirectsWithoutLocation() { HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index c42c104..273070b 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -67,6 +67,34 @@ class FileUtilsTest { assertTrue(FileUtils.decodeXml(content).contains('café')) } + @Test + void testDecodeXmlStripsUtf8AndUtf16ByteOrderMarks() { + String xml = 'ok' + byte[] utf8 = prepend([0xEF, 0xBB, 0xBF] as byte[], xml.getBytes('UTF-8')) + byte[] utf16le = prepend([0xFF, 0xFE] as byte[], + xml.replace('UTF-8', 'UTF-16LE').getBytes('UTF-16LE')) + byte[] utf16be = prepend([0xFE, 0xFF] as byte[], + xml.replace('UTF-8', 'UTF-16BE').getBytes('UTF-16BE')) + + assertTrue(FileUtils.decodeXml(utf8).startsWith('ok'.bytes + + assertTrue(FileUtils.decodeXml(content).contains('ok')) + } + @Test void testBaseNameWithNull() { assertNull(FileUtils.baseName(null)) diff --git a/release.md b/release.md index d81f773..8575982 100644 --- a/release.md +++ b/release.md @@ -5,6 +5,7 @@ - Breaking: Swing and JavaFX display resolution now prefers classpath resources when a name exists both on the classpath and in the working directory. - Common: local filenames preserve `#`; URL fragments are stripped only from URL-shaped inputs. - Console: EOF consistently returns `null`; malformed or out-of-range date input falls back to the documented default/initial value. +- Breaking: `urlExists` now follows HTTP(S) redirects manually with a bounded overall timeout budget and returns `true` only for a successful final 2xx response; negative timeouts are rejected. - JavaFX: SVG files are rendered directly as JavaFX nodes; SVG display no longer creates a WebView. The SVG backend supports a subset of browser SVG features, so advanced CSS, filters, and scripting may render differently or be unsupported. - Build: Matrix BOM updated to 2.5.1 and Swing Widgets to 1.1.2. diff --git a/release.sh b/release.sh index 35e4705..b13008b 100755 --- a/release.sh +++ b/release.sh @@ -191,7 +191,7 @@ commit_release_version() { echo -e "${RED}Error: Failed to add files to git. Please resolve the issue and try again.${NC}" >&2 exit 1 fi - if ! git commit -m "Release version ${release_version}"; then + if ! git commit -m "Release version ${release_version}" -- build.gradle README.md CHANGELOG.md; then echo -e "${RED}Error: Failed to commit version change. Please resolve the issue and try again.${NC}" >&2 exit 1 fi @@ -242,8 +242,14 @@ elif [ "$README_NEEDS_UPDATE" = true ]; then read -p "Update README.md to version ${CURRENT_VERSION}? [Y/n]: " update_readme if [[ ! "$update_readme" =~ ^[Nn]$ ]]; then update_readme_version "$CURRENT_VERSION" - git add README.md - git commit -m "Update README version to ${CURRENT_VERSION}" + if ! git add README.md; then + echo -e "${RED}Error: Failed to add README.md to git. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi + if ! git commit -m "Update README version to ${CURRENT_VERSION}" -- README.md; then + echo -e "${RED}Error: Failed to commit README.md version change. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi fi fi fi From 4424e89691080d8a5c597cc1a1211d4e9dba5fa4 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 16:00:28 +0200 Subject: [PATCH 06/11] document URL contract and test SVG loading --- docs/API-Guide.md | 2 ++ .../groovy/se/alipsa/gi/AbstractInOut.groovy | 3 +-- .../main/groovy/se/alipsa/gi/FileUtils.groovy | 9 +++++++++ .../groovy/se/alipsa/gi/GuiInteraction.groovy | 5 +++-- .../se/alipsa/gi/AbstractInOutTest.groovy | 19 +++++++++++++++++++ .../groovy/se/alipsa/gi/FileUtilsTest.groovy | 8 ++++++++ .../main/groovy/se/alipsa/gi/fx/InOut.groovy | 8 ++++---- .../groovy/se/alipsa/gi/swing/InOut.groovy | 4 ++-- release.md | 2 +- release.sh | 18 +++++++++++------- 10 files changed, 60 insertions(+), 18 deletions(-) diff --git a/docs/API-Guide.md b/docs/API-Guide.md index b6d99b3..33e7c27 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -239,6 +239,8 @@ if (io.urlExists("https://example.com/api/health", 5000)) { } ``` +`urlExists` accepts HTTP and HTTPS URLs, follows redirects within the supplied overall timeout budget, and returns `false` unless the final response is 2xx. A negative timeout throws `IllegalArgumentException`; `0` retains the JVM connection timeout behavior. + ### Content Type Detection ```groovy diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index 2685f42..39c2155 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -48,8 +48,7 @@ abstract class AbstractInOut implements GuiInteraction { try { con = open(url, "HEAD", remainingTimeout(timeout, deadline), false) int responseCode = con.getResponseCode() - if (responseCode == HttpURLConnection.HTTP_BAD_METHOD || - responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED) { + if (responseCode >= 400 && responseCode < 500) { if (!hasTimeRemaining(timeout, deadline)) { return false } diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index db99f51..21f303c 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -94,6 +94,15 @@ class FileUtils { return decoded.startsWith('\uFEFF') ? decoded.substring(1) : decoded } + /** + * Reads and decodes XML content from a URL, honoring its XML declaration and byte-order mark. + */ + static String readXml(URL url) throws IOException { + try (InputStream input = url.openStream()) { + return decodeXml(input.readAllBytes()) + } + } + /** * Finds a resource using multiple classloader strategies. *

diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index d94117b..7b844e5 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -102,13 +102,14 @@ interface GuiInteraction { /** * Checks if a URL exists and is accessible. *

- * Performs an HTTP HEAD request, falling back to GET when HEAD is not supported, + * Performs an HTTP HEAD request, falling back to GET when HEAD returns a 4xx response, * to verify the URL is reachable. * * @param urlString the URL to check * @param timeout overall timeout budget in milliseconds, including redirect hops and retries * @return {@code true} if the URL returns a successful 2xx response after redirects, - * {@code false} otherwise + * {@code false} for non-HTTP(S) URLs or any other unsuccessful response + * @throws IllegalArgumentException if {@code timeout} is negative */ boolean urlExists(String urlString, int timeout) diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 5f46d82..07817c1 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -226,6 +226,25 @@ class AbstractInOutTest { } } + @Test + void urlExistsFallsBackToGetForAnyHeadClientError() { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/health") { exchange -> + if (exchange.requestMethod == "HEAD") { + exchange.sendResponseHeaders(403, -1) + } else { + exchange.sendResponseHeaders(200, -1) + } + exchange.close() + } + server.start() + try { + assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) + } finally { + server.stop(0) + } + } + @Test void urlExistsFollowsRedirectsAndChecksTheFinalResponse() { HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) diff --git a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy index 273070b..6b9cd62 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/FileUtilsTest.groovy @@ -95,6 +95,14 @@ class FileUtilsTest { assertTrue(FileUtils.decodeXml(content).contains('ok')) } + @Test + void testReadXmlReadsAndDecodesClasspathSvg() { + String svg = FileUtils.readXml(FileUtils.getResourceUrl('svgplot.svg')) + + assertTrue(svg.startsWith(' 0 ? title[0] : '' Platform.runLater(() -> { try { - Node node = ChartToJfx.export(FileUtils.decodeXml(svgBytes)) + Node node = ChartToJfx.export(svgContent) showNow(node, windowTitle) } catch (RuntimeException e) { log.error("Failed to parse SVG {}", url, e) diff --git a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy index 1565a4d..a211a25 100644 --- a/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy +++ b/gi-swing/src/main/groovy/se/alipsa/gi/swing/InOut.groovy @@ -341,8 +341,8 @@ class InOut extends AbstractInOut { */ private void displaySvg(URL svgUrl, String... title) { String svg - try (InputStream input = svgUrl.openStream()) { - svg = FileUtils.decodeXml(input.readAllBytes()) + try { + svg = FileUtils.readXml(svgUrl) } catch (IOException e) { log.error("Failed to read svg resource {}", svgUrl, e) return diff --git a/release.md b/release.md index 8575982..5b5c7bc 100644 --- a/release.md +++ b/release.md @@ -5,7 +5,7 @@ - Breaking: Swing and JavaFX display resolution now prefers classpath resources when a name exists both on the classpath and in the working directory. - Common: local filenames preserve `#`; URL fragments are stripped only from URL-shaped inputs. - Console: EOF consistently returns `null`; malformed or out-of-range date input falls back to the documented default/initial value. -- Breaking: `urlExists` now follows HTTP(S) redirects manually with a bounded overall timeout budget and returns `true` only for a successful final 2xx response; negative timeouts are rejected. +- Breaking: `urlExists` now follows HTTP(S) redirects manually with a bounded overall timeout budget and returns `true` only for a successful final 2xx response; non-HTTP(S) URLs return `false` and negative timeouts are rejected. - JavaFX: SVG files are rendered directly as JavaFX nodes; SVG display no longer creates a WebView. The SVG backend supports a subset of browser SVG features, so advanced CSS, filters, and scripting may render differently or be unsupported. - Build: Matrix BOM updated to 2.5.1 and Swing Widgets to 1.1.2. diff --git a/release.sh b/release.sh index b13008b..a306c6c 100755 --- a/release.sh +++ b/release.sh @@ -242,13 +242,17 @@ elif [ "$README_NEEDS_UPDATE" = true ]; then read -p "Update README.md to version ${CURRENT_VERSION}? [Y/n]: " update_readme if [[ ! "$update_readme" =~ ^[Nn]$ ]]; then update_readme_version "$CURRENT_VERSION" - if ! git add README.md; then - echo -e "${RED}Error: Failed to add README.md to git. Please resolve the issue and try again.${NC}" >&2 - exit 1 - fi - if ! git commit -m "Update README version to ${CURRENT_VERSION}" -- README.md; then - echo -e "${RED}Error: Failed to commit README.md version change. Please resolve the issue and try again.${NC}" >&2 - exit 1 + if ! git diff --quiet -- README.md; then + if ! git add README.md; then + echo -e "${RED}Error: Failed to add README.md to git. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi + if ! git commit -m "Update README version to ${CURRENT_VERSION}" -- README.md; then + echo -e "${RED}Error: Failed to commit README.md version change. Please resolve the issue and try again.${NC}" >&2 + exit 1 + fi + else + echo -e "${YELLOW}README.md already matches version ${CURRENT_VERSION}; no commit needed.${NC}" fi fi fi From 6afc0cabae95a4dbd43e45cdcb8c5ed912ef4ca4 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 16:07:09 +0200 Subject: [PATCH 07/11] refine HEAD fallback behavior --- docs/API-Guide.md | 2 +- .../groovy/se/alipsa/gi/AbstractInOut.groovy | 10 ++++- .../main/groovy/se/alipsa/gi/FileUtils.groovy | 4 ++ .../groovy/se/alipsa/gi/GuiInteraction.groovy | 3 +- .../se/alipsa/gi/AbstractInOutTest.groovy | 41 +++++++++++++++++++ release.sh | 2 +- 6 files changed, 58 insertions(+), 4 deletions(-) diff --git a/docs/API-Guide.md b/docs/API-Guide.md index 33e7c27..ad3a835 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -239,7 +239,7 @@ if (io.urlExists("https://example.com/api/health", 5000)) { } ``` -`urlExists` accepts HTTP and HTTPS URLs, follows redirects within the supplied overall timeout budget, and returns `false` unless the final response is 2xx. A negative timeout throws `IllegalArgumentException`; `0` retains the JVM connection timeout behavior. +`urlExists` accepts HTTP and HTTPS URLs, follows redirects within the supplied overall timeout budget, and returns `false` unless the final response is 2xx. A negative timeout throws `IllegalArgumentException`; `0` disables the timeout entirely, so the call may block indefinitely. ### Content Type Detection diff --git a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy index 39c2155..30b7dfe 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -48,7 +48,7 @@ abstract class AbstractInOut implements GuiInteraction { try { con = open(url, "HEAD", remainingTimeout(timeout, deadline), false) int responseCode = con.getResponseCode() - if (responseCode >= 400 && responseCode < 500) { + if (shouldFallbackToGet(responseCode)) { if (!hasTimeRemaining(timeout, deadline)) { return false } @@ -101,6 +101,14 @@ abstract class AbstractInOut implements GuiInteraction { return "http".equalsIgnoreCase(url.protocol) || "https".equalsIgnoreCase(url.protocol) } + private static boolean shouldFallbackToGet(int responseCode) { + return responseCode == HttpURLConnection.HTTP_BAD_REQUEST || + responseCode == HttpURLConnection.HTTP_UNAUTHORIZED || + responseCode == HttpURLConnection.HTTP_FORBIDDEN || + responseCode == HttpURLConnection.HTTP_BAD_METHOD || + responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED + } + private static int remainingTimeout(int configuredTimeout, long deadline) { if (configuredTimeout <= 0) { return configuredTimeout diff --git a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy index 21f303c..c808b92 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/FileUtils.groovy @@ -96,6 +96,10 @@ class FileUtils { /** * Reads and decodes XML content from a URL, honoring its XML declaration and byte-order mark. + * + * @param url the URL containing XML content + * @return the decoded XML content + * @throws IOException if the URL cannot be opened or read */ static String readXml(URL url) throws IOException { try (InputStream input = url.openStream()) { diff --git a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy index 7b844e5..cad60a4 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/GuiInteraction.groovy @@ -102,7 +102,8 @@ interface GuiInteraction { /** * Checks if a URL exists and is accessible. *

- * Performs an HTTP HEAD request, falling back to GET when HEAD returns a 4xx response, + * Performs an HTTP HEAD request, falling back to GET for retryable method/authorization + * responses (400, 401, 403, 405, or 501), * to verify the URL is reachable. * * @param urlString the URL to check diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 07817c1..dff22c4 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -12,6 +12,7 @@ import java.time.LocalDate import java.time.YearMonth import com.sun.net.httpserver.HttpServer import java.net.InetSocketAddress +import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference import static org.junit.jupiter.api.Assertions.* @@ -245,6 +246,46 @@ class AbstractInOutTest { } } + @Test + void urlExistsFallsBackToGetWhenHeadReturns501() { + AtomicInteger requests = new AtomicInteger() + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/health") { exchange -> + requests.incrementAndGet() + if (exchange.requestMethod == "HEAD") { + exchange.sendResponseHeaders(501, -1) + } else { + exchange.sendResponseHeaders(200, -1) + } + exchange.close() + } + server.start() + try { + assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) + assertEquals(2, requests.get()) + } finally { + server.stop(0) + } + } + + @Test + void urlExistsDoesNotRetryAuthoritativeNotFoundResponses() { + AtomicInteger requests = new AtomicInteger() + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) + server.createContext("/missing") { exchange -> + requests.incrementAndGet() + exchange.sendResponseHeaders(404, -1) + exchange.close() + } + server.start() + try { + assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/missing", 2000)) + assertEquals(1, requests.get()) + } finally { + server.stop(0) + } + } + @Test void urlExistsFollowsRedirectsAndChecksTheFinalResponse() { HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) diff --git a/release.sh b/release.sh index a306c6c..642557b 100755 --- a/release.sh +++ b/release.sh @@ -242,7 +242,7 @@ elif [ "$README_NEEDS_UPDATE" = true ]; then read -p "Update README.md to version ${CURRENT_VERSION}? [Y/n]: " update_readme if [[ ! "$update_readme" =~ ^[Nn]$ ]]; then update_readme_version "$CURRENT_VERSION" - if ! git diff --quiet -- README.md; then + if ! git diff --quiet HEAD -- README.md; then if ! git add README.md; then echo -e "${RED}Error: Failed to add README.md to git. Please resolve the issue and try again.${NC}" >&2 exit 1 From ee9a8f9e899f7535568e8ffcc497a667d193913e Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 16:46:52 +0200 Subject: [PATCH 08/11] tighten HEAD fallback test coverage --- .../se/alipsa/gi/AbstractInOutTest.groovy | 73 +++++++------------ 1 file changed, 25 insertions(+), 48 deletions(-) diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index dff22c4..8f8e757 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -4,6 +4,9 @@ import groovy.transform.CompileStatic import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource import se.alipsa.groovy.svg.Svg import se.alipsa.matrix.core.Matrix @@ -14,6 +17,7 @@ import com.sun.net.httpserver.HttpServer import java.net.InetSocketAddress import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference +import java.util.stream.Stream import static org.junit.jupiter.api.Assertions.* @@ -202,70 +206,43 @@ class AbstractInOutTest { assertFalse(inOut.urlExists("http://localhost:59999/", 1000)) } - @Test - void urlExistsFallsBackToGetWhenHeadIsUnsupported() { + @ParameterizedTest(name = 'HEAD {0}, fallback expected: {1}') + @MethodSource('headFallbackCases') + void urlExistsAppliesTheHeadFallbackAllowList(int headStatus, boolean fallbackExpected) { AtomicReference rangeHeader = new AtomicReference<>() HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) server.createContext("/health") { exchange -> + int responseCode if (exchange.requestMethod == "HEAD") { - exchange.sendResponseHeaders(405, -1) + responseCode = headStatus } else { rangeHeader.set(exchange.requestHeaders.getFirst("Range")) - byte[] response = "ok".bytes - exchange.sendResponseHeaders(200, response.length) - exchange.responseBody.write(response) - exchange.responseBody.close() + responseCode = 200 } + exchange.sendResponseHeaders(responseCode, -1) exchange.close() } server.start() try { - assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) - assertEquals("bytes=0-0", rangeHeader.get()) + assertEquals(fallbackExpected, + inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) + assertEquals(fallbackExpected ? "bytes=0-0" : null, rangeHeader.get()) } finally { server.stop(0) } } - @Test - void urlExistsFallsBackToGetForAnyHeadClientError() { - HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) - server.createContext("/health") { exchange -> - if (exchange.requestMethod == "HEAD") { - exchange.sendResponseHeaders(403, -1) - } else { - exchange.sendResponseHeaders(200, -1) - } - exchange.close() - } - server.start() - try { - assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) - } finally { - server.stop(0) - } - } - - @Test - void urlExistsFallsBackToGetWhenHeadReturns501() { - AtomicInteger requests = new AtomicInteger() - HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) - server.createContext("/health") { exchange -> - requests.incrementAndGet() - if (exchange.requestMethod == "HEAD") { - exchange.sendResponseHeaders(501, -1) - } else { - exchange.sendResponseHeaders(200, -1) - } - exchange.close() - } - server.start() - try { - assertTrue(inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) - assertEquals(2, requests.get()) - } finally { - server.stop(0) - } + static Stream headFallbackCases() { + Stream.of( + Arguments.of(400, true), + Arguments.of(401, true), + Arguments.of(403, true), + Arguments.of(405, true), + Arguments.of(501, true), + Arguments.of(404, false), + Arguments.of(410, false), + Arguments.of(500, false) + ) } @Test From d60ac461f5be3e03dbc5adb6b77f5c95b6d54f1e Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 16:51:38 +0200 Subject: [PATCH 09/11] restore URL request count assertions --- .../se/alipsa/gi/AbstractInOutTest.groovy | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy index 8f8e757..56a78b1 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -209,9 +209,11 @@ class AbstractInOutTest { @ParameterizedTest(name = 'HEAD {0}, fallback expected: {1}') @MethodSource('headFallbackCases') void urlExistsAppliesTheHeadFallbackAllowList(int headStatus, boolean fallbackExpected) { + AtomicInteger requests = new AtomicInteger() AtomicReference rangeHeader = new AtomicReference<>() HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) server.createContext("/health") { exchange -> + requests.incrementAndGet() int responseCode if (exchange.requestMethod == "HEAD") { responseCode = headStatus @@ -227,6 +229,7 @@ class AbstractInOutTest { assertEquals(fallbackExpected, inOut.urlExists("http://127.0.0.1:${server.address.port}/health", 2000)) assertEquals(fallbackExpected ? "bytes=0-0" : null, rangeHeader.get()) + assertEquals(fallbackExpected ? 2 : 1, requests.get()) } finally { server.stop(0) } @@ -245,24 +248,6 @@ class AbstractInOutTest { ) } - @Test - void urlExistsDoesNotRetryAuthoritativeNotFoundResponses() { - AtomicInteger requests = new AtomicInteger() - HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) - server.createContext("/missing") { exchange -> - requests.incrementAndGet() - exchange.sendResponseHeaders(404, -1) - exchange.close() - } - server.start() - try { - assertFalse(inOut.urlExists("http://127.0.0.1:${server.address.port}/missing", 2000)) - assertEquals(1, requests.get()) - } finally { - server.stop(0) - } - } - @Test void urlExistsFollowsRedirectsAndChecksTheFinalResponse() { HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0) From 14278d6bf674d3c8a16a5f8aa9631e092e44f6da Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 16:56:14 +0200 Subject: [PATCH 10/11] correct FX SVG documentation --- docs/API-Guide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/API-Guide.md b/docs/API-Guide.md index ad3a835..fe2fab5 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -306,7 +306,7 @@ if (file == null) { ### gi-fx (JavaFX) - Requires a JVM with JavaFX support -- Full SVG rendering support via WebView +- SVG rendering via matrix-charts JavaFX integration (a lighter subset than a browser renderer) - Rich date pickers with calendar UI ### gi-swing From cc104ec30848bfb7e07d3d00bbca1081132cf336 Mon Sep 17 00:00:00 2001 From: per Date: Mon, 3 Aug 2026 17:07:04 +0200 Subject: [PATCH 11/11] clarify module-specific file display --- docs/API-Guide.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/API-Guide.md b/docs/API-Guide.md index fe2fab5..9fb3cdb 100644 --- a/docs/API-Guide.md +++ b/docs/API-Guide.md @@ -203,7 +203,8 @@ io.view([ // Display an image io.display("/path/to/chart.png", "Chart") -// Display a file (opens with system default for unknown types) +// gi-console opens unknown files with the system default application; +// gi-swing and gi-fx expect a displayable image resource instead. io.display(new File("document.pdf")) ```