From 9e3a273d5ace47499ddc50fa8d85d763f82cb72d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 13 May 2026 04:27:20 +0300 Subject: [PATCH 1/2] Fix Dark/Light Mode toggle: preserve app theme and stop growing the canvas Two regressions in the simulator's Dark/Light Mode menu (and the adjacent Larger Text menu), plus a related stale-compile bug behind "my localization edits don't take effect until I run mvn clean": 1. JavaSEPort: dark-mode / larger-text toggles now route through a new refreshThemeOnly() helper instead of refreshSkin. refreshSkin called Display.installNativeTheme(), which is UIManager.setThemeProps( nativeProps) and **replaces** the installed theme wholesale -- so a CSS-compiled app theme (custom UIID fonts, custom margins, overridden colors) was wiped to the bare native theme after one click. Visually the user saw "fonts are completely wrong" until the simulator was relaunched. refreshSkin also recomputed zoomLevel as canvas.width * retinaScale / skin.width, which equals retinaScale on a HiDPI display whose skin already fits the screen. The canvas then grew to skin * retinaScale while subsequent drawing still ran at zoomLevel == 1, so the simulator appeared to "activate zoom mode" for no reason after a Dark/Light click. refreshThemeOnly() uses UIManager.refreshTheme() (re-applies the currently-installed themeProps, clears the styles cache so shouldUseDarkStyle resolves $Dark correctly) and leaves the canvas alone. 2. CompileCSSMojo: edits to a .properties localization bundle now invalidate the up-to-date check. The shared AbstractCN1Mojo.getCSSSourcesModificationTime only walks src/main/css, but the CSS compiler also reads the l10n directory (passed via -l) and bakes the bundle into the same theme.res. With the old check, l10n edits looked like "no change", the mojo skipped the compiler, and the stale theme.res hid the .properties update until a mvn clean. The new getLocalizationModificationTime() rolls the l10n directory's recursive modtime into the comparison. Tests: - CompileCSSMojoTest gains three cases: recompilesWhenLocalizationFileNewerThanThemeRes (the actual reproducer -- failed before the fix, passes after); skipsCompilationWhenAllInputsOlderThanThemeRes (guards against an over-eager fix that just disables the up-to-date check); recompilesWhenCssFileNewerThanThemeRes (locks down the CSS path so it can't regress alongside the l10n fix). A new setupMultiModuleCommonProject helper builds the canonical parent/common/ layout the production check is written against; the pre-existing single-module helper put CSS at project/src/main/css which left getCSSSourcesModificationTime returning zero and silently masked the staleness bug. - UIManagerDarkModeStyleTest gains two cases: testRefreshThemeAppliesDarkStyleAfterCachedLightLookup (the cached light style must not win after a dark-mode flip + refreshTheme); testRefreshThemePreservesAppThemeCustomizationsAcrossDarkModeFlip (app-only UIIDs and overridden colors must survive the refresh -- this is the contract refreshThemeOnly() relies on). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/codename1/impl/javase/JavaSEPort.java | 48 ++++++- .../com/codename1/maven/CompileCSSMojo.java | 22 ++- .../codename1/maven/CompileCSSMojoTest.java | 127 ++++++++++++++++++ .../ui/plaf/UIManagerDarkModeStyleTest.java | 77 +++++++++++ 4 files changed, 269 insertions(+), 5 deletions(-) diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index b50eca6335..6b5649d85c 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -4678,7 +4678,7 @@ public void actionPerformed(ActionEvent ae) { public void actionPerformed(ActionEvent e) { JavaSEPort.this.darkMode = Boolean.TRUE; pref.put("cn1.simulator.darkMode", "dark"); - refreshSkin(frm); + refreshThemeOnly(); } }); @@ -4687,7 +4687,7 @@ public void actionPerformed(ActionEvent e) { public void actionPerformed(ActionEvent e) { JavaSEPort.this.darkMode = Boolean.FALSE; pref.put("cn1.simulator.darkMode", "light"); - refreshSkin(frm); + refreshThemeOnly(); } }); @@ -4696,7 +4696,7 @@ public void actionPerformed(ActionEvent e) { public void actionPerformed(ActionEvent e) { JavaSEPort.this.darkMode = null; pref.put("cn1.simulator.darkMode", "unsupported"); - refreshSkin(frm); + refreshThemeOnly(); } }); darkLightModeMenu.add(darkModeItem); @@ -5793,7 +5793,9 @@ public void actionPerformed(ActionEvent e) { largerTextScale = scale; largerTextEnabled = scale > 1.0f + 0.001f; pref.putFloat(PREF_LARGER_TEXT_SCALE, scale); - refreshSkin(frm); + // Theme-only refresh so the app's CSS-compiled theme survives; see + // refreshThemeOnly() for why refreshSkin breaks the app theme + canvas size. + refreshThemeOnly(); } }); group.add(item); @@ -5904,6 +5906,44 @@ static Rectangle parsePersistedBounds(String s) { } } + /// Theme-only refresh used by the Dark/Light Mode and Larger Text menu actions. + /// + /// These toggles change how the *current* theme should be resolved, but the skin (the + /// phone-shell image, the canvas pixel size, the screen-fit zoom) hasn't moved. Routing them + /// through {@link #refreshSkin} caused two visible regressions: + /// + /// 1. `refreshSkin` calls `Display.installNativeTheme()`, which delegates to + /// `UIManager.setThemeProps(nativeProps)`. `setThemeProps` **replaces** the installed + /// theme wholesale, so any CSS-compiled app theme (custom UIID fonts, custom margins, + /// app-only style keys) is wiped to the bare native theme. Visually this read as "fonts + /// are completely wrong" until the simulator was relaunched. + /// + /// 2. `refreshSkin` recomputes the canvas size from `canvas.getWidth() * retinaScale / + /// skin.getWidth()`, which equals `retinaScale` on a HiDPI display whose skin already + /// fits the screen. The canvas then grows to `skin * retinaScale` while subsequent + /// drawing still uses `zoomLevel == 1`, so the simulator appeared to "activate zoom mode" + /// after a Dark/Light click. + /// + /// Master PR #4935 (`isUsableWindowBounds`/`parsePersistedBounds` above) adds defense in + /// depth against the resulting bad window-bounds prefs from the Larger Text path. This + /// helper removes the trigger entirely by leaving the canvas alone -- + /// `UIManager.refreshTheme()` re-applies the currently-installed themeProps (so the app + /// theme survives) and clears the style cache (so `shouldUseDarkStyle` resolves correctly + /// against the new `Display.isDarkMode()`). + private void refreshThemeOnly() { + Display.getInstance().callSerially(new Runnable() { + public void run() { + UIManager.getInstance().refreshTheme(); + Form curr = Display.getInstance().getCurrent(); + if (curr != null) { + deepRevaliate(curr); + curr.revalidate(); + curr.repaint(); + } + } + }); + } + private ArrayList deinitializeHooks = new ArrayList<>(); public void addDeinitializeHook(Runnable r) { diff --git a/maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/CompileCSSMojo.java b/maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/CompileCSSMojo.java index 25599bf105..de742e5b91 100644 --- a/maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/CompileCSSMojo.java +++ b/maven/codenameone-maven-plugin/src/main/java/com/codename1/maven/CompileCSSMojo.java @@ -74,6 +74,24 @@ protected void executeImpl() throws MojoExecutionException, MojoFailureException } } + /** + * The localization directory is bundled into the same `theme.res` as the CSS rules + * (see the `-l` argument passed to `CN1CSSCLI` below). The shared + * {@link AbstractCN1Mojo#getCSSSourcesModificationTime} only walks `src/main/css`, + * so it would happily treat l10n edits as "no change" and let the up-to-date check + * at the top of {@link #executeImpl(String)} skip recompilation, leaving the user's + * `.properties` updates trapped in the stale `theme.res` until a `mvn clean` forces + * a rebuild. Roll the l10n directory's recursive modtime into the comparison so + * touching any `Messages.properties` invalidates the cached output. + */ + protected long getLocalizationModificationTime() { + File localizationDir = findLocalizationDirectory(); + if (localizationDir == null || !localizationDir.exists()) { + return 0L; + } + return lastModifiedRecursive(localizationDir, ALL_FILES_FILTER); + } + /** * Gets the source CSS directory (src/main/css). The theme.css file should be inside this directory. * @return @@ -168,7 +186,9 @@ private void executeImpl(String themePrefix) throws MojoExecutionException, Mojo File mergeFile = new File(cssBuildDir, themePrefix + "theme.css"); mergeFile.getParentFile().mkdirs(); try { - if (themeResOutput.exists() && getCSSSourcesModificationTime() < themeResOutput.lastModified()) { + long sourcesModTime = Math.max(getCSSSourcesModificationTime(), + getLocalizationModificationTime()); + if (themeResOutput.exists() && sourcesModTime < themeResOutput.lastModified()) { getLog().info("CSS sources unchanged since last compile. Skipping CSS compilation"); return; } diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/maven/CompileCSSMojoTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/maven/CompileCSSMojoTest.java index 2c67e79e04..c571338317 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/maven/CompileCSSMojoTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/maven/CompileCSSMojoTest.java @@ -20,6 +20,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; class CompileCSSMojoTest { @@ -78,6 +80,94 @@ void addsI18nDirectoryToDesignerInvocation(@TempDir Path tempDir) throws Excepti assertEquals(projectDir.resolve("src/main/i18n").toFile().getAbsolutePath(), args.get(index + 1)); } + /// Reproduces the staleness bug where edits to a `.properties` localization bundle do not + /// trigger CSS re-compilation. `getCSSSourcesModificationTime` only walks `src/main/css`, but + /// the CSS compiler also reads the l10n directory (it bakes the resource bundle into the same + /// `theme.res`), so a newer l10n file with an older CSS file must still recompile. + @Test + void recompilesWhenLocalizationFileNewerThanThemeRes(@TempDir Path tempDir) throws Exception { + Path projectDir = setupMultiModuleCommonProject(tempDir, "l10n"); + Path themeRes = projectDir.resolve("target/classes/theme.res"); + Path l10nFile = projectDir.resolve("src/main/l10n/Messages.properties"); + + Files.createDirectories(themeRes.getParent()); + Files.write(themeRes, new byte[]{0x42}); + + long base = System.currentTimeMillis() - 60_000L; + ageAllInputs(projectDir, base); + assertTrue(themeRes.toFile().setLastModified(base + 5_000L)); + assertTrue(l10nFile.toFile().setLastModified(base + 10_000L)); + + TestCompileCSSMojo mojo = createMojo(projectDir); + mojo.executeImpl(); + + assertNotNull(mojo.getRecordingJava(), + "CSS compiler should have been invoked because the l10n file is newer than theme.res"); + } + + /// Sanity-check: when only the merged-output theme.res is newer than every CSS/l10n input, + /// the mojo correctly skips the subprocess. This guards against an over-eager fix that just + /// disables the up-to-date check entirely. + @Test + void skipsCompilationWhenAllInputsOlderThanThemeRes(@TempDir Path tempDir) throws Exception { + Path projectDir = setupMultiModuleCommonProject(tempDir, "l10n"); + Path themeRes = projectDir.resolve("target/classes/theme.res"); + + Files.createDirectories(themeRes.getParent()); + Files.write(themeRes, new byte[]{0x42}); + + long base = System.currentTimeMillis() - 60_000L; + ageAllInputs(projectDir, base); + assertTrue(themeRes.toFile().setLastModified(base + 10_000L)); + + TestCompileCSSMojo mojo = createMojo(projectDir); + mojo.executeImpl(); + + assertNull(mojo.getRecordingJava(), + "CSS compiler should be skipped when neither CSS nor l10n changed since the last build"); + } + + /// Companion to the l10n test: CSS edits already invalidate the cache today, but lock the + /// behavior down so it does not regress alongside the l10n fix. + @Test + void recompilesWhenCssFileNewerThanThemeRes(@TempDir Path tempDir) throws Exception { + Path projectDir = setupMultiModuleCommonProject(tempDir, null); + Path themeRes = projectDir.resolve("target/classes/theme.res"); + Path cssFile = projectDir.resolve("src/main/css/theme.css"); + + Files.createDirectories(themeRes.getParent()); + Files.write(themeRes, new byte[]{0x42}); + + long base = System.currentTimeMillis() - 60_000L; + ageAllInputs(projectDir, base); + assertTrue(themeRes.toFile().setLastModified(base)); + assertTrue(cssFile.toFile().setLastModified(base + 10_000L)); + + TestCompileCSSMojo mojo = createMojo(projectDir); + mojo.executeImpl(); + + assertNotNull(mojo.getRecordingJava(), + "CSS compiler should have been invoked because the CSS file is newer than theme.res"); + } + + /// `getCSSSourcesModificationTime` walks `src/main/css` recursively (directory mtimes count too) + /// and also stats the project pom and codenameone_settings. Files just created by the test + /// helper land with `now` as their mtime, which would always win the comparison and force + /// compilation regardless of the CSS/l10n setup we want to assert against. Walk every file + /// and directory under the project and age them to a known baseline so each test can drive + /// the modtime ordering it cares about. + private static void ageAllInputs(Path projectDir, long baseMillis) throws IOException { + Files.walk(projectDir).forEach(p -> { + // The target/ directory is set up per-test (theme.res gets an explicit mtime later), + // so leave it alone here. + if (p.startsWith(projectDir.resolve("target"))) { + return; + } + assertTrue(p.toFile().setLastModified(baseMillis), + "Failed to set mtime on " + p); + }); + } + private TestCompileCSSMojo createMojo(Path projectDir) throws IOException { MavenProject mavenProject = new MavenProject(); mavenProject.setFile(projectDir.resolve("pom.xml").toFile()); @@ -129,6 +219,43 @@ private Path setupProject(Path tempDir, String localizationDirName) throws IOExc return projectDir; } + /// Sets up the canonical multi-module Codename One layout: a parent dir containing a `common/` + /// child that owns the CSS and (optionally) the localization. `getCSSSourcesModificationTime` + /// derives `root = projectBaseDir.getParentFile()` and then looks for `root/common/src/main/css`, + /// so naming the basedir literally "common" is what makes that lookup resolve back to this + /// project's CSS directory. The single-module helper above puts CSS at `project/src/main/css`, + /// which leaves `getCSSSourcesModificationTime` returning zero and silently masks the staleness + /// bug we want to test. + private Path setupMultiModuleCommonProject(Path tempDir, String localizationDirName) throws IOException { + Path parentDir = tempDir.resolve("parent"); + Files.createDirectories(parentDir); + Path commonDir = parentDir.resolve("common"); + Files.createDirectories(commonDir); + Files.createDirectories(commonDir.resolve("src/main/java")); + Path cssDir = Files.createDirectories(commonDir.resolve("src/main/css")); + Files.write(cssDir.resolve("theme.css"), Arrays.asList("/* test css */")); + Files.write(commonDir.resolve("codenameone_settings.properties"), Arrays.asList("codename1.cssTheme=true")); + Files.createDirectories(commonDir.resolve("target/classes")); + Files.createFile(commonDir.resolve("designer.jar")); + Files.write(commonDir.resolve("pom.xml"), Arrays.asList( + "", + " 4.0.0", + " com.codename1", + " test-common", + " 1.0-SNAPSHOT", + "" + )); + + if (localizationDirName != null) { + Path localizationDir = Files.createDirectories(commonDir.resolve("src/main").resolve(localizationDirName)); + Files.write(localizationDir.resolve("Messages.properties"), Arrays.asList("greeting=Hello")); + } + + return commonDir; + } + private static class TestCompileCSSMojo extends CompileCSSMojo { private final File designerJar; private RecordingJava recordingJava; diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerDarkModeStyleTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerDarkModeStyleTest.java index 51268cc211..9beafd9a39 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerDarkModeStyleTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/plaf/UIManagerDarkModeStyleTest.java @@ -74,4 +74,81 @@ public void testFallsBackToRegularStyleWhenNoDarkStyleExists() { assertEquals(0xaabbcc, style.getFgColor()); } + + /// Regression test for the simulator's Dark/Light Mode toggle. When the user flips dark mode + /// after styles have already been resolved once (the common case: the toggle lives on a + /// running simulator), the next style lookup must reflect the dark variant -- not the cached + /// light style. The bug surfaced because `getComponentStyleImpl` caches results by UIID + /// without keying the cache on dark-mode state, so without an explicit cache invalidation + /// the cached light-mode style would win even after `Display.setDarkMode(TRUE)`. The + /// simulator's documented escape hatch for users is `UIManager.refreshTheme()`, which + /// clears the cache. + @Test + public void testRefreshThemeAppliesDarkStyleAfterCachedLightLookup() { + UIManager manager = UIManager.getInstance(); + Hashtable theme = new Hashtable(); + theme.put("Button.fgColor", "112233"); + theme.put("Button.bgColor", "445566"); + theme.put("$DarkButton.bgColor", "000000"); + manager.setThemeProps(theme); + + display.setDarkMode(Boolean.FALSE); + Style lightStyle = manager.getComponentStyle("Button"); + assertEquals(0x445566, lightStyle.getBgColor(), + "Sanity: light-mode lookup populates the styles cache"); + + display.setDarkMode(Boolean.TRUE); + manager.refreshTheme(); + Style darkStyle = manager.getComponentStyle("Button"); + + assertEquals(0x112233, darkStyle.getFgColor(), + "Dark variant should inherit fgColor from the base Button rule"); + assertEquals(0x000000, darkStyle.getBgColor(), + "Dark variant bgColor must win after refreshTheme; otherwise the simulator's" + + " Dark/Light toggle silently keeps the light-cached bgColor."); + } + + /// Regression test for the simulator's Dark/Light Mode menu path. The toggle handler used + /// to call `JavaSEPort.refreshSkin`, which itself calls `Display.installNativeTheme()`. + /// `installNativeTheme` calls `UIManager.setThemeProps(nativeProps)`, and `setThemeProps` + /// blows away the previously-installed theme entirely -- so a project that had loaded an + /// app theme (CSS-generated `theme.res`) with custom font sizes, custom margins, or + /// overridden UIID styling would, after one click on Dark/Light/Unsupported, find every + /// app-level customization gone. Visually that read as "fonts are completely wrong" until + /// the user re-launched the simulator. + /// + /// `UIManager.refreshTheme()` re-applies the *current* themeProps (so app customizations + /// survive) and clears the style cache (so dark/light variants resolve correctly). This + /// test pins that contract: after a refresh, app-theme keys are still effective. + @Test + public void testRefreshThemePreservesAppThemeCustomizationsAcrossDarkModeFlip() { + UIManager manager = UIManager.getInstance(); + Hashtable appTheme = new Hashtable(); + // Pretend this is the CSS-generated user theme: it sets app-specific colors on Button + // and a separate UIID the native theme would never define. + appTheme.put("Button.fgColor", "ff8800"); + appTheme.put("Button.bgColor", "eeeeee"); + appTheme.put("$DarkButton.bgColor", "111111"); + appTheme.put("AppOnlyLabel.fgColor", "abcdef"); + manager.setThemeProps(appTheme); + + display.setDarkMode(Boolean.FALSE); + // Warm the style cache the way a real simulator session would. + manager.getComponentStyle("Button"); + manager.getComponentStyle("AppOnlyLabel"); + + display.setDarkMode(Boolean.TRUE); + manager.refreshTheme(); + + Style buttonDark = manager.getComponentStyle("Button"); + assertEquals(0xff8800, buttonDark.getFgColor(), + "App theme's Button.fgColor must survive the dark-mode refresh"); + assertEquals(0x111111, buttonDark.getBgColor(), + "Dark Button.bgColor override must apply after refreshTheme"); + + Style appOnly = manager.getComponentStyle("AppOnlyLabel"); + assertEquals(0xabcdef, appOnly.getFgColor(), + "App-only UIIDs not present in any native theme must still resolve after a " + + "dark-mode refresh; if this fails the toggle is wiping the app theme."); + } } From a229d8c5c9c71b0e6d1ca59692e036222cc4f1c4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 13 May 2026 06:25:52 +0300 Subject: [PATCH 2/2] AutoLocalizationBundle: preserve mtime so CSS mojo doesn't recompile every run Follow-up to the l10n staleness fix in this PR. Reviewer caught a feedback loop: when the user opts in to "Auto Update Default Bundle", AutoLocalizationBundle persists to disk on every missing-key lookup (see `get` -> `storeEntry` -> `persistIfNeeded(true)` -> `persist`). Each persist advanced the bundle file's mtime, so CompileCSSMojo.getLocalizationModificationTime -- which now factors l10n mtime into the staleness comparison -- would treat the runtime cache flush as a fresh user edit and force a ~30s CSS recompile on every subsequent `cn1:run`. Reproducer: opt in to auto-bundle, add `getString("hello")` to the app, run twice. The second `cn1:run` recompiles CSS even though the user changed nothing. Fix: snapshot the bundle file's mtime before each persist and restore it after the write. If an external editor advanced the mtime since our last restoration (i.e. current mtime != preservedMtime), adopt the user's mtime as the new preserved target so genuine user edits still propagate through the staleness check. `setLastModified` failure is treated as best-effort -- worst case is one extra CSS recompile, which is the pre-fix behavior, not a regression. Why neither the original code review nor the test plan caught this: - I scoped my review to CompileCSSMojo in isolation and reasoned "the compiler reads `-l `, so l10n edits must invalidate the cache." That logic is correct for user-driven edits. I never asked who else writes to `src/main/l10n/*.properties` at runtime. - The auto-bundle lives in Ports/JavaSE while the staleness check lives in maven/codenameone-maven-plugin -- different modules, so the cross-cutting writer didn't surface naturally during the mojo-level review. - The new CompileCSSMojoTest cases are pure mtime arithmetic (Files.write + setLastModified). The auto-bundle never executes in those tests, so its runtime write pattern can't show up. There was no integration test of "compile -> simulate -> compile" that would have asserted the second compile is a no-op. Tests: - AutoLocalizationBundleMtimeTest is new and covers four scenarios: runtimePersistKeepsOriginalFileMtime (the actual reproducer); multipleRuntimePersistsKeepOriginalFileMtime (no drift across many persists); externalUserEditPropagatesToMtimeEvenAfterAutoBundleWrite (a user edit between auto-bundle writes still propagates); and freshFilePersistEstablishesPreservedMtime (no baseline yet -> first write becomes the baseline). All four fail before this commit and pass after. The bundle is a private inner class of JavaSEPort so the new test uses reflection, mirroring the established pattern in tests/core/test/com/codename1/impl/javase/AutoLocalizationBundleTest. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/codename1/impl/javase/JavaSEPort.java | 40 +++++ .../AutoLocalizationBundleMtimeTest.java | 168 ++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 maven/javase/src/test/java/com/codename1/impl/javase/AutoLocalizationBundleMtimeTest.java diff --git a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java index 6b5649d85c..bac4798a75 100644 --- a/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java +++ b/Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java @@ -14714,6 +14714,21 @@ private static class AutoLocalizationBundle extends Hashtable { private File bundleFile; private final java.util.Properties properties = new java.util.Properties(); private boolean dirty; + /// Mtime to restore on the bundle file after each {@link #persist}. + /// + /// The auto-bundle persists every time the app reads a missing key (see + /// `get`/`storeEntry`), so during normal development the file is rewritten on most + /// simulator launches even when no human ever edited it. Without this restoration, + /// `CompileCSSMojo.getLocalizationModificationTime` -- which trusts the file mtime as + /// a proxy for "user edited" -- would treat each runtime persist as a fresh + /// localization edit and force a ~30s CSS recompile on every subsequent `cn1:run`. + /// + /// Strategy: before each persist, sample the file's current mtime. If it matches the + /// last value we restored (i.e. the file is exactly as we left it, no external editor + /// touched it), restore to that value after the write. If it differs, the user edited + /// outside the simulator -- capture their mtime as the new target so their edit still + /// flows into the staleness comparison. + private long preservedMtime = -1L; AutoLocalizationBundle(File bundleFile, Map base) { this.bundleFile = bundleFile; @@ -14784,12 +14799,37 @@ private void ensureParentExists() { private void persist() { ensureParentExists(); + // Sample the file's mtime BEFORE writing so we can carry the user's last-edit + // timestamp forward. If the file doesn't exist yet (first persist of a fresh + // project), there's no original mtime to preserve -- fall through and let the + // post-write mtime become the new baseline. + long mtimeBeforeWrite = bundleFile.exists() ? bundleFile.lastModified() : -1L; + if (mtimeBeforeWrite > 0L + && (preservedMtime <= 0L || mtimeBeforeWrite != preservedMtime)) { + // Either this is the first persist of the session, or the user edited the + // file externally since our last restoration. Adopt the current mtime as the + // new preserved target so their edit propagates to the staleness check. + preservedMtime = mtimeBeforeWrite; + } try (OutputStream out = new FileOutputStream(bundleFile)) { properties.store(out, "Codename One auto-generated bundle"); } catch (IOException err) { Log.e(err); } dirty = false; + if (preservedMtime > 0L) { + // Best-effort: setLastModified can fail on read-only filesystems or unusual + // platforms. If it does, the worst case is the user gets one extra CSS + // recompile, which is the pre-fix behavior -- not a regression. + if (!bundleFile.setLastModified(preservedMtime)) { + // Capture the post-write mtime so subsequent persists in this session + // don't treat the failure as an external edit. + preservedMtime = bundleFile.lastModified(); + } + } else { + // File was freshly created: use this write's mtime as the future baseline. + preservedMtime = bundleFile.lastModified(); + } } private void persistIfNeeded(boolean force) { diff --git a/maven/javase/src/test/java/com/codename1/impl/javase/AutoLocalizationBundleMtimeTest.java b/maven/javase/src/test/java/com/codename1/impl/javase/AutoLocalizationBundleMtimeTest.java new file mode 100644 index 0000000000..5fcfbc8882 --- /dev/null +++ b/maven/javase/src/test/java/com/codename1/impl/javase/AutoLocalizationBundleMtimeTest.java @@ -0,0 +1,168 @@ +package com.codename1.impl.javase; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.OutputStream; +import java.lang.reflect.Constructor; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// Regression coverage for the feedback loop between {@code AutoLocalizationBundle} and the +/// CSS staleness check. +/// +/// The auto-bundle persists to disk every time the running app reads a missing key (see +/// `AutoLocalizationBundle.get`). Without the mtime-preservation contract these tests pin, +/// each persist advanced the bundle file's modified-time, which would in turn make +/// `CompileCSSMojo.getLocalizationModificationTime` treat the runtime cache flush as a fresh +/// user edit and force a ~30s CSS recompile on every subsequent `cn1:run`. The reproducer is +/// the obvious dev-loop: opt in to "Auto Update Default Bundle", add `getString("hello")` to +/// the app, run twice -- the second `cn1:run` should not recompile CSS because the user +/// changed nothing. +/// +/// `AutoLocalizationBundle` is a private inner class of `JavaSEPort`. The pre-existing +/// `AutoLocalizationBundleTest` in `tests/core/...` already drives it via reflection, so we +/// follow the same pattern here rather than widening the visibility just for tests. +class AutoLocalizationBundleMtimeTest { + + @Test + void runtimePersistKeepsOriginalFileMtime(@TempDir Path tempDir) throws Exception { + Path bundleFile = setupExistingBundle(tempDir, "hello=world\n"); + long originalMtime = ageFile(bundleFile, 60_000L); + + Object bundle = newBundle(bundleFile.toFile(), null); + @SuppressWarnings("unchecked") + Map bundleMap = (Map) bundle; + + // Trigger a runtime auto-fabrication: missing key -> persist. + bundleMap.get("missingKey"); + + assertEquals(originalMtime, bundleFile.toFile().lastModified(), + "Auto-bundle persist must restore the file mtime so CompileCSSMojo's" + + " getLocalizationModificationTime doesn't treat a runtime cache flush" + + " as a fresh user edit"); + + // And the actual content must still be written through. + Properties on_disk = loadProperties(bundleFile); + assertEquals("missingKey", on_disk.getProperty("missingKey"), + "Mtime restoration must not skip the content write"); + } + + @Test + void multipleRuntimePersistsKeepOriginalFileMtime(@TempDir Path tempDir) throws Exception { + Path bundleFile = setupExistingBundle(tempDir, "hello=world\n"); + long originalMtime = ageFile(bundleFile, 60_000L); + + Object bundle = newBundle(bundleFile.toFile(), null); + @SuppressWarnings("unchecked") + Map bundleMap = (Map) bundle; + + // The simulator session reads many missing keys; each one persists. None of them is a + // user edit, so none of them should advance the file mtime. + bundleMap.get("one"); + bundleMap.get("two"); + bundleMap.get("three"); + bundleMap.put("four", "fourValue"); + + assertEquals(originalMtime, bundleFile.toFile().lastModified(), + "Successive auto-bundle persists must not drift the mtime forward"); + } + + @Test + void externalUserEditPropagatesToMtimeEvenAfterAutoBundleWrite(@TempDir Path tempDir) throws Exception { + Path bundleFile = setupExistingBundle(tempDir, "hello=world\n"); + ageFile(bundleFile, 60_000L); + + Object bundle = newBundle(bundleFile.toFile(), null); + @SuppressWarnings("unchecked") + Map bundleMap = (Map) bundle; + + bundleMap.get("first"); + long mtimeAfterFirstPersist = bundleFile.toFile().lastModified(); + + // Simulate the user opening their IDE and adding a new translation while the + // simulator is still running. The auto-bundle does NOT know about this edit + // a priori; the next persist must detect it (mtime drifted from what we last + // restored) and adopt the user's mtime as the new preserved baseline, so the + // edit propagates through `CompileCSSMojo.getLocalizationModificationTime`. + long userEditMtime = mtimeAfterFirstPersist + 30_000L; + assertTrue(bundleFile.toFile().setLastModified(userEditMtime), + "Test setup precondition: the host filesystem must accept setLastModified"); + + bundleMap.get("triggersPersistAfterUserEdit"); + + long mtimeAfterSecondPersist = bundleFile.toFile().lastModified(); + assertNotEquals(mtimeAfterFirstPersist, mtimeAfterSecondPersist, + "After a user edit, the next auto-bundle persist must surface the user's mtime"); + assertEquals(userEditMtime, mtimeAfterSecondPersist, + "The preserved mtime should follow the user's edit, not snap back to ours"); + } + + @Test + void freshFilePersistEstablishesPreservedMtime(@TempDir Path tempDir) throws Exception { + // Project with no Bundle.properties yet -- the constructor creates an empty file. + Path l10nDir = Files.createDirectories(tempDir.resolve("l10n")); + File bundleFile = new File(l10nDir.toFile(), "Bundle.properties"); + assertTrue(!bundleFile.exists()); + + Object bundle = newBundle(bundleFile, null); + @SuppressWarnings("unchecked") + Map bundleMap = (Map) bundle; + assertTrue(bundleFile.exists(), "Constructor should have created the empty bundle"); + + long mtimeAfterCreation = bundleFile.lastModified(); + // Force the OS to record a clearly later "now" so a drift-forward bug would be visible. + Thread.sleep(50L); + + bundleMap.get("alpha"); + bundleMap.get("beta"); + + assertEquals(mtimeAfterCreation, bundleFile.lastModified(), + "Once a baseline mtime is established (here, the empty-file creation timestamp)," + + " subsequent auto-bundle persists must not drift it forward."); + } + + private Path setupExistingBundle(Path tempDir, String contents) throws Exception { + Path l10nDir = Files.createDirectories(tempDir.resolve("l10n")); + Path bundleFile = l10nDir.resolve("Bundle.properties"); + try (OutputStream out = new FileOutputStream(bundleFile.toFile())) { + out.write(contents.getBytes("UTF-8")); + } + return bundleFile; + } + + /// Files freshly created by the test land with "now" as their mtime, which makes it + /// impossible to tell a real restoration from a no-op (current mtime == "now" either way). + /// Push the mtime back by `millisInPast` so any drift-forward bug is unambiguous. + private long ageFile(Path file, long millisInPast) { + long target = System.currentTimeMillis() - millisInPast; + assertTrue(file.toFile().setLastModified(target), + "Test setup precondition: setLastModified must be honored on the host FS"); + return target; + } + + private Object newBundle(File bundleFile, Map base) throws Exception { + Class bundleClass = Class.forName("com.codename1.impl.javase.JavaSEPort$AutoLocalizationBundle"); + Constructor ctor = bundleClass.getDeclaredConstructor(File.class, Map.class); + ctor.setAccessible(true); + return ctor.newInstance(bundleFile, base == null ? new HashMap() : base); + } + + private Properties loadProperties(Path file) throws Exception { + Properties props = new Properties(); + try (FileInputStream in = new FileInputStream(file.toFile())) { + props.load(in); + } + return props; + } +}