From 912a2b7cd8dfe14462c696891d3012b40d8b667d Mon Sep 17 00:00:00 2001
From: "David M. Johnson" <snoopdave@apache.org>
Date: Tue, 8 Sep 2026 08:03:46 -0400
Subject: [PATCH] Track schema upgrades with a flag instead of a hardcoded
 version check

upgradeDatabase decided whether to report "No table changes were required."
from a hardcoded `dbversion < 610`, which has to be bumped by hand every time
a new migration is added. Set a schemaUpgraded flag inside each upgrade block
instead, so the message is derived from whether a step actually ran. Behavior
is unchanged: at that point dbversion < 610 is true exactly when one of the
upgrade blocks runs.

Adds a test covering the schema-change path (the flag suppresses the "no
changes" message) alongside the existing version-only case.

Addresses review feedback from Michael Bien on #182.

Claude-Session: https://claude.ai/code/session_015X69HHQ5XnjRkJP8ymzwFf
---
 .../business/startup/DatabaseInstaller.java   | 15 +++++++--
 .../startup/DatabaseInstallerUpgradeTest.java | 32 +++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/app/src/main/java/org/apache/roller/weblogger/business/startup/DatabaseInstaller.java b/app/src/main/java/org/apache/roller/weblogger/business/startup/DatabaseInstaller.java
index 1267d4d26..684a8aaa3 100644
--- a/app/src/main/java/org/apache/roller/weblogger/business/startup/DatabaseInstaller.java
+++ b/app/src/main/java/org/apache/roller/weblogger/business/startup/DatabaseInstaller.java
@@ -226,37 +226,46 @@ public void upgradeDatabase(boolean runScripts) throws StartupException {
 
             log.info("Database is old, beginning upgrade to version "+myVersion);
 
-            boolean schemaChangesRequired = dbversion < 610;
+            // track whether any upgrade step actually ran, so the
+            // "no table changes" message stays correct without a
+            // hardcoded version constant
+            boolean schemaUpgraded = false;
 
             // iterate through each upgrade as needed
             // to add to the upgrade sequence simply add a new "if" statement
-            // for whatever version needed and then define a new method upgradeXXX()
+            // for whatever version needed, define a new method upgradeXXX(),
+            // and set schemaUpgraded = true
 
             if(dbversion < 400) {
                 upgradeTo400(con, runScripts);
                 dbversion = 400;
+                schemaUpgraded = true;
             }
             if(dbversion < 500) {
                 upgradeTo500(con, runScripts);
                 dbversion = 500;
+                schemaUpgraded = true;
             }
             if(dbversion < 510) {
                 upgradeTo510(con, runScripts);
                 dbversion = 510;
+                schemaUpgraded = true;
             }
             if(dbversion < 520) {
                 upgradeTo520(con, runScripts);
                 dbversion = 520;
+                schemaUpgraded = true;
             }
             if(dbversion < 610) {
                 upgradeTo610(con, runScripts);
                 dbversion = 610;
+                schemaUpgraded = true;
             }
 
             // make sure the database version is the exact version
             // we are upgrading too.
             updateDatabaseVersion(con, myVersion);
-            if (!schemaChangesRequired) {
+            if (!schemaUpgraded) {
                 successMessage("No table changes were required.");
             }
             successMessage("Database version updated to " + myVersion + ".");
diff --git a/app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java b/app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java
index 280dde3ee..94dbb0129 100644
--- a/app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java
+++ b/app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java
@@ -44,4 +44,36 @@ void versionOnlyUpgradeReportsCompletion() throws Exception {
         assertTrue(installer.getMessages().stream().anyMatch(m -> m.contains("No table changes were required.")));
         assertTrue(installer.getMessages().stream().anyMatch(m -> m.contains("Database version updated to " + expectedVersion + ".")));
     }
+
+    @Test
+    void schemaUpgradeSuppressesNoChangesMessage() throws Exception {
+        DatabaseProvider db = mock(DatabaseProvider.class);
+        DatabaseScriptProvider scripts = mock(DatabaseScriptProvider.class);
+        Connection con = mock(Connection.class);
+        Statement query = mock(Statement.class);
+        ResultSet rows = mock(ResultSet.class);
+        PreparedStatement update = mock(PreparedStatement.class);
+        when(db.getConnection()).thenReturn(con);
+        when(con.createStatement()).thenReturn(query);
+        when(query.executeQuery(anyString())).thenReturn(rows);
+        when(rows.next()).thenReturn(true);
+        when(rows.getString(1)).thenReturn("520");
+        when(con.prepareStatement(anyString())).thenReturn(update);
+        DatabaseInstaller installer = new DatabaseInstaller(db, scripts);
+
+        Properties props = new Properties();
+        props.load(getClass().getResourceAsStream("/roller-version.properties"));
+        int expectedVersion = DatabaseInstaller.parseVersionString(props.getProperty("ro.version", "UNKNOWN"));
+
+        // a 520 database upgrades through the 520->610 schema step; run with
+        // runScripts=false so the step does its bookkeeping without executing a
+        // migration script. Because a schema step ran, the "no table changes"
+        // message must be suppressed.
+        installer.upgradeDatabase(false);
+
+        verify(update).setString(1, String.valueOf(expectedVersion));
+        verify(update).executeUpdate();
+        assertTrue(installer.getMessages().stream().anyMatch(m -> m.contains("Database version updated to " + expectedVersion + ".")));
+        assertFalse(installer.getMessages().stream().anyMatch(m -> m.contains("No table changes were required.")));
+    }
 }