diff --git a/build.gradle b/build.gradle index cbe5d95..45918fd 100644 --- a/build.gradle +++ b/build.gradle @@ -68,8 +68,16 @@ 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") + String suffix = spotbugsTask.name.startsWith('spotbugs') ? + spotbugsTask.name.substring('spotbugs'.length()) : '' + String sourceSetName = suffix.isEmpty() ? 'main' : + 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) + } reports { html.required = true xml.required = false 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..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")) ``` @@ -239,6 +240,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` disables the timeout entirely, so the call may block indefinitely. + ### Content Type Detection ```groovy @@ -257,8 +260,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 @@ -299,7 +307,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 @@ -312,5 +320,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..30b7dfe 100644 --- a/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy +++ b/gi-common/src/main/groovy/se/alipsa/gi/AbstractInOut.groovy @@ -15,10 +15,13 @@ 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 { + private static final int MAX_REDIRECTS = 5 + private Parser markdownParser private HtmlRenderer htmlRenderer protected def clipboard @@ -30,24 +33,92 @@ abstract class AbstractInOut implements GuiInteraction { @Override boolean urlExists(String urlString, int timeout) { - HttpURLConnection con = null + if (timeout < 0) { + throw new IllegalArgumentException("timeout cannot be negative") + } try { URL url = new URL(urlString) - con = (HttpURLConnection) url.openConnection() - con.setInstanceFollowRedirects(false) - con.setRequestMethod("HEAD") - con.setConnectTimeout(timeout) - con.setReadTimeout(timeout) - int responseCode = con.getResponseCode() - // Accept 2xx (success) and 3xx (redirect) status codes - return responseCode >= 200 && responseCode < 400 + long deadline = timeout > 0 ? + System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeout) : Long.MAX_VALUE + for (int redirect = 0; redirect <= MAX_REDIRECTS; redirect++) { + if (!isHttpUrl(url) || !hasTimeRemaining(timeout, deadline)) { + return false + } + HttpURLConnection con = null + try { + con = open(url, "HEAD", remainingTimeout(timeout, deadline), false) + int responseCode = con.getResponseCode() + if (shouldFallbackToGet(responseCode)) { + 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() + } + } + if (responseCode >= 300 && responseCode < 400) { + String location = con.getHeaderField("Location") + if (location == null || redirect == MAX_REDIRECTS) { + return false + } + url = new URL(url, location) + continue + } + return responseCode >= 200 && responseCode < 300 + } finally { + if (con != null) { + con.disconnect() + } + } + } } catch (RuntimeException | IOException ignored) { return false - } finally { - if (con != null) { - con.disconnect() - } } + 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 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 + } + long remainingMillis = TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime()) + return (int) Math.max(1, Math.min(Integer.MAX_VALUE, remainingMillis)) + } + + private static boolean hasTimeRemaining(int configuredTimeout, long deadline) { + return configuredTimeout <= 0 || System.nanoTime() < deadline } @Override @@ -91,10 +162,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..c808b92 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,37 +15,96 @@ 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. *

- * 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: *

* * @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; - String basename = ""; - url = url.replace('\\', '/'); + if (url == null) return null + url = url.replace('\\', '/') + int queryIndex = 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) { + 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 + } + + /** + * 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. + */ + 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 basename.length() > 0 ? basename : url; + String decoded = new String(content, charset) + return decoded.startsWith('\uFEFF') ? decoded.substring(1) : decoded + } + + /** + * 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()) { + return decodeXml(input.readAllBytes()) + } } /** @@ -59,9 +122,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 +148,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..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,11 +102,15 @@ 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 for retryable method/authorization + * responses (400, 401, 403, 405, or 501), + * 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 + * @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} for non-HTTP(S) URLs or any other unsuccessful response + * @throws IllegalArgumentException if {@code timeout} is negative */ boolean urlExists(String urlString, int timeout) @@ -136,7 +140,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) @@ -150,7 +154,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 */ @@ -198,8 +203,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); @@ -211,7 +215,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); @@ -221,7 +225,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); @@ -233,8 +237,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, 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); @@ -243,7 +247,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); @@ -251,8 +255,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..56a78b1 100644 --- a/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy +++ b/gi-common/src/test/groovy/se/alipsa/gi/AbstractInOutTest.groovy @@ -4,12 +4,20 @@ 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 import javax.swing.JComponent 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 java.util.stream.Stream import static org.junit.jupiter.api.Assertions.* @@ -179,6 +187,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 @@ -191,6 +206,159 @@ class AbstractInOutTest { assertFalse(inOut.urlExists("http://localhost:59999/", 1000)) } + @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 + } else { + rangeHeader.set(exchange.requestHeaders.getFirst("Range")) + responseCode = 200 + } + exchange.sendResponseHeaders(responseCode, -1) + exchange.close() + } + server.start() + try { + 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) + } + } + + 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 + 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) + } + } + + @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) + 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 faae125..6b9cd62 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,74 @@ 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 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 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())) + 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 + 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 testReadXmlReadsAndDecodesClasspathSvg() { + String svg = FileUtils.readXml(FileUtils.getResourceUrl('svgplot.svg')) + + assertTrue(svg.startsWith(' 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) { + return null + } + if (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 +195,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..3f9d5c7 --- /dev/null +++ b/gi-console/src/test/groovy/se/alipsa/gi/console/ConsolePromptTest.groovy @@ -0,0 +1,64 @@ +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 rangedYearMonthFallsBackForOutOfRangeValues() { + InOut inOut = new InOut() + 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))) + } + + @Test + void fullSelectionValidatesOptionsAndHandlesEof() { + InOut inOut = new InOut() + inOut.sysin = new BufferedReader(new StringReader('')) + + 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..394cf9b 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. 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/build.gradle b/gi-fx/build.gradle index 06e119e..4d79165 100644 --- a/gi-fx/build.gradle +++ b/gi-fx/build.gradle @@ -53,15 +53,18 @@ 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 { - 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 2687c52..4b71844 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 @@ -46,20 +45,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 +68,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 +212,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) { @@ -310,24 +313,40 @@ 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) { - 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) } } - Image img = new Image(url.toExternalForm()) - display(img, title) + 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) + } } void display(Image img, String... title) { @@ -335,6 +354,25 @@ class InOut extends AbstractInOut { display(node, title) } + private void displaySvg(URL url, String... title) { + String svgContent + try { + svgContent = FileUtils.readXml(url) + } catch (IOException e) { + log.error("Failed to read SVG {}", url, e) + return + } + String windowTitle = title.length > 0 ? title[0] : '' + Platform.runLater(() -> { + try { + Node node = ChartToJfx.export(svgContent) + showNow(node, windowTitle) + } catch (RuntimeException e) { + log.error("Failed to parse SVG {}", url, e) + } + }) + } + @Override void display(File file, String... title) { if (file == null || !file.exists()) { @@ -389,49 +427,57 @@ 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 } void saveToClipboard(String string) { - Platform.runLater(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putString(string) getClipboard().setContent(content) + return null }); } void saveToClipboard(File file) { - Platform.runLater(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putFiles(List.of(file)) getClipboard().setContent(content) + return null }); } void saveToClipboard(Image img) { - Platform.runLater(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.putImage(img) getClipboard().setContent(content) + return null }); } void saveToClipboard(Object obj, DataFormat format) { - Platform.runLater(() -> { + runOnFxThreadAsync(() -> { ClipboardContent content = new ClipboardContent() content.put(format, obj) getClipboard().setContent(content) + return null }); } @@ -493,6 +539,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 new file mode 100644 index 0000000..2756b6f --- /dev/null +++ b/gi-fx/src/test/groovy/se/alipsa/gi/fx/InOutHeadlessTest.groovy @@ -0,0 +1,15 @@ +package se.alipsa.gi.fx + +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertThrows + +class InOutHeadlessTest { + + @Test + void headlessConstructionReportsTheExpectedException() { + assertThrows(UnsupportedOperationException) { + new InOut() + } + } +} diff --git a/gi-swing/build.gradle b/gi-swing/build.gradle index d7bf266..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 @@ -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 + testRuntimeOnly libs.junit.platform.launcher } repositories { @@ -61,6 +65,11 @@ repositories { } } +test { + useJUnitPlatform() + systemProperty 'java.awt.headless', 'true' +} + 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..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 @@ -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 @@ -32,15 +33,12 @@ 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 +134,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 +199,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,39 +298,59 @@ 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 = 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) 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 (FileUtils.isSvgResource(resource)) { + 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) } /** - * Displays an SVG file using Apache Batik's JSVGCanvas. + * Displays an SVG file using matrix-charts' JSVG-backed panel. */ - private void displaySvg(File svgFile, String... title) { + private void displaySvg(URL svgUrl, String... title) { String svg try { - svg = svgFile.getText("UTF-8") + svg = FileUtils.readXml(svgUrl) } 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..0a9a920 --- /dev/null +++ b/gi-swing/src/test/groovy/se/alipsa/gi/swing/InOutHeadlessTest.groovy @@ -0,0 +1,15 @@ +package se.alipsa.gi.swing + +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertThrows + +class InOutHeadlessTest { + + @Test + void headlessConstructionReportsTheExpectedException() { + 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.md b/release.md index ac698e3..5b5c7bc 100644 --- a/release.md +++ b/release.md @@ -1,6 +1,13 @@ # 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. +- 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. ## 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 7ed5db7..642557b 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 @@ -180,43 +181,40 @@ 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 - RELEASE_VERSION="${CURRENT_VERSION%-SNAPSHOT}" - echo -e "Stripping SNAPSHOT suffix: ${YELLOW}${CURRENT_VERSION}${NC} -> ${GREEN}${RELEASE_VERSION}${NC}" +commit_release_version() { + local release_version=$1 + update_version "$release_version" + update_readme_version "$release_version" + generate_changelog "$release_version" - 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}" + 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}" -- 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 - CURRENT_VERSION=$RELEASE_VERSION +} + +# 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}" 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}" - 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 +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 +229,34 @@ 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 +# 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 - echo -e "${YELLOW}[DRY RUN] Would update version to ${NEW_VERSION}${NC}" + 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 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 + 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 - - CURRENT_VERSION=$NEW_VERSION fi - echo "" echo -e "Releasing version: ${GREEN}${CURRENT_VERSION}${NC}" echo ""