From 48cf46b7093366cfa0683a2bd78c46b1d25b4ad1 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Mon, 7 Sep 2026 14:44:52 -0400 Subject: [PATCH 1/2] Improve installer form initialization and preview navigation --- .../business/startup/DatabaseInstaller.java | 6 +++ .../ui/core/filters/LoadSaltFilter.java | 1 + .../weblogger/ui/struts2/core/Install.java | 9 ++++ .../ui/struts2/editor/EntryEdit.java | 2 +- .../WEB-INF/jsps/core/DatabaseError.jsp | 2 +- .../startup/DatabaseInstallerUpgradeTest.java | 38 +++++++++++++++ .../ui/core/filters/LoadSaltFilterTest.java | 35 ++++++++++++++ .../ui/struts2/core/InstallTest.java | 31 ++++++++++++ .../struts2/editor/EntryEditPreviewTest.java | 47 +++++++++++++++++++ 9 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/InstallTest.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditPreviewTest.java 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 1917c24f03..ab2819aa0c 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,6 +226,8 @@ public void upgradeDatabase(boolean runScripts) throws StartupException { log.info("Database is old, beginning upgrade to version "+myVersion); + boolean schemaChangesRequired = dbversion < 610; + // 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() @@ -254,6 +256,10 @@ public void upgradeDatabase(boolean runScripts) throws StartupException { // make sure the database version is the exact version // we are upgrading too. updateDatabaseVersion(con, myVersion); + if (!schemaChangesRequired) { + successMessage("No table changes were required."); + } + successMessage("Database version updated to " + myVersion + "."); } catch (SQLException e) { throw new StartupException("ERROR obtaining connection"); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java index b2e63915d1..8a1c830fa3 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java @@ -37,6 +37,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha throws IOException, ServletException { HttpServletRequest httpReq = (HttpServletRequest) request; + httpReq.getSession(true); RollerSession rollerSession = RollerSession.getRollerSession(httpReq); if (rollerSession != null) { String userId = rollerSession.getAuthenticatedUser() != null ? rollerSession.getAuthenticatedUser().getId() : ""; diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Install.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Install.java index 355239519f..feed9b0b6b 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Install.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Install.java @@ -54,6 +54,15 @@ public class Install extends UIAction { private String databaseName = "Unknown"; + @Override + public void setPageTitle(String pageTitle) { + this.pageTitle = pageTitle; + } + + public String getRootCauseExceptionName() { + return rootCauseException == null ? "" : rootCauseException.getClass().getName(); + } + @Override public boolean isUserRequired() { return false; diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java index d4e0af71b7..8fda6fcbdc 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEdit.java @@ -412,7 +412,7 @@ public String getPreviewURL() { .getUrlStrategy() .getPreviewURLStrategy(null) .getWeblogEntryURL(getActionWeblog(), null, - getEntry().getAnchor(), true); + getEntry().getAnchor(), false); } /** diff --git a/app/src/main/webapp/WEB-INF/jsps/core/DatabaseError.jsp b/app/src/main/webapp/WEB-INF/jsps/core/DatabaseError.jsp index 9bd1133536..2f1398277e 100644 --- a/app/src/main/webapp/WEB-INF/jsps/core/DatabaseError.jsp +++ b/app/src/main/webapp/WEB-INF/jsps/core/DatabaseError.jsp @@ -32,7 +32,7 @@

- [] + []

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 new file mode 100644 index 0000000000..a55191b53c --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/business/startup/DatabaseInstallerUpgradeTest.java @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0. + */ +package org.apache.roller.weblogger.business.startup; + +import java.sql.*; +import org.apache.roller.weblogger.business.DatabaseProvider; +import org.junit.jupiter.api.Test; +import static org.mockito.Mockito.*; +import static org.junit.jupiter.api.Assertions.*; + +class DatabaseInstallerUpgradeTest { + @Test + void versionOnlyUpgradeReportsCompletion() 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("610"); + when(con.prepareStatement(anyString())).thenReturn(update); + DatabaseInstaller installer = new DatabaseInstaller(db, scripts); + installer.upgradeDatabase(true); + verify(update).setString(1, "616"); + verify(update).executeUpdate(); + verifyNoInteractions(scripts); + 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 616."))); + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilterTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilterTest.java index 5ace927a27..2087a005b3 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilterTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilterTest.java @@ -74,6 +74,41 @@ public void testDoFilterWithNullRollerSession() throws Exception { } } + @Test + void firstFormHasAUsableSalt() throws Exception { + javax.servlet.http.HttpSession session = mock(javax.servlet.http.HttpSession.class); + java.util.Map attributes = new java.util.HashMap<>(); + java.util.Map sessionAttributes = new java.util.HashMap<>(); + when(request.getSession(true)).thenAnswer(invocation -> { + when(request.getSession(false)).thenReturn(session); + return session; + }); + when(session.getAttribute(anyString())).thenAnswer(i -> sessionAttributes.get(i.getArgument(0))); + doAnswer(i -> { sessionAttributes.put(i.getArgument(0), i.getArgument(1)); return null; }) + .when(session).setAttribute(anyString(), any()); + doAnswer(i -> { attributes.put(i.getArgument(0), i.getArgument(1)); return null; }) + .when(request).setAttribute(anyString(), any()); + java.util.Map salts = new java.util.HashMap<>(); + try (MockedStatic cache = mockStatic(SaltCache.class)) { + cache.when(SaltCache::getInstance).thenReturn(saltCache); + doAnswer(i -> { salts.put(i.getArgument(0), i.getArgument(1)); return null; }) + .when(saltCache).put(anyString(), anyString()); + when(saltCache.get(anyString())).thenAnswer(i -> salts.get(i.getArgument(0))); + doAnswer(i -> { salts.remove(i.getArgument(0)); return null; }) + .when(saltCache).remove(anyString()); + filter.doFilter(request, response, chain); + String salt = (String) attributes.get("salt"); + org.junit.jupiter.api.Assertions.assertNotNull(salt); + when(request.getParameter("salt")).thenReturn(null); + org.junit.jupiter.api.Assertions.assertFalse(SaltValidator.consumeSubmittedSalt(request)); + when(request.getParameter("salt")).thenReturn("unknown"); + org.junit.jupiter.api.Assertions.assertFalse(SaltValidator.consumeSubmittedSalt(request)); + when(request.getParameter("salt")).thenReturn(salt); + org.junit.jupiter.api.Assertions.assertTrue(SaltValidator.consumeSubmittedSalt(request)); + org.junit.jupiter.api.Assertions.assertFalse(SaltValidator.consumeSubmittedSalt(request)); + } + } + private static class TestUser extends User { private final String id; diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/InstallTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/InstallTest.java new file mode 100644 index 0000000000..de2d340eb0 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/InstallTest.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0. + */ +package org.apache.roller.weblogger.ui.struts2.core; + +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.business.startup.*; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import static org.mockito.Mockito.*; +import static org.junit.jupiter.api.Assertions.*; + +class InstallTest { + @Test + void installerExposesItsTitleAndExceptionName() { + Install action = spy(new Install()); + doAnswer(i -> i.getArgument(0)).when(action).getText(anyString()); + assertEquals("", action.getRootCauseExceptionName()); + try (MockedStatic factory = mockStatic(WebloggerFactory.class); + MockedStatic startup = mockStatic(WebloggerStartup.class)) { + startup.when(WebloggerStartup::getDatabaseProviderException) + .thenReturn(new StartupException("Connection failed", new IllegalStateException("offline"))); + assertEquals("database_error", action.execute()); + assertEquals("installer.error.connection.pageTitle", action.getPageTitle()); + assertEquals("java.lang.IllegalStateException", action.getRootCauseExceptionName()); + } + } +} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditPreviewTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditPreviewTest.java new file mode 100644 index 0000000000..ab73893383 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/editor/EntryEditPreviewTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0. + */ +package org.apache.roller.weblogger.ui.struts2.editor; + +import java.net.URI; +import org.apache.roller.weblogger.business.*; +import org.apache.roller.weblogger.config.WebloggerRuntimeConfig; +import org.apache.roller.weblogger.pojos.*; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import static org.mockito.Mockito.*; +import static org.junit.jupiter.api.Assertions.*; + +class EntryEditPreviewTest { + @Test + void previewUsesTheEditorsOrigin() { + Weblogger weblogger = mock(Weblogger.class); + when(weblogger.getUrlStrategy()).thenReturn(new MultiWeblogURLStrategy()); + try (MockedStatic factory = mockStatic(WebloggerFactory.class); + MockedStatic config = mockStatic(WebloggerRuntimeConfig.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(weblogger); + config.when(WebloggerRuntimeConfig::getAbsoluteContextURL).thenReturn("http://other.example/roller"); + Weblog weblog = new Weblog(); + weblog.setHandle("mainpage"); + WeblogEntry entry = new WeblogEntry(); + entry.setAnchor("test entry"); + EntryEdit action = new EntryEdit(); + action.setActionWeblog(weblog); + action.setEntry(entry); + for (String context : new String[]{"", "/roller"}) { + config.when(WebloggerRuntimeConfig::getRelativeContextURL).thenReturn(context); + String preview = action.getPreviewURL(); + assertEquals(context + "/roller-ui/authoring/preview/mainpage/?previewEntry=test+entry", preview); + for (String scheme : new String[]{"http", "https"}) { + URI editor = URI.create(scheme + "://example.org:8443" + context + "/roller-ui/authoring/entryEdit.rol"); + URI target = editor.resolve(preview); + assertEquals(editor.getScheme(), target.getScheme()); + assertEquals(editor.getAuthority(), target.getAuthority()); + } + } + } + } +} From 4a89f02f45884cfd0e4ed3cdfc4700bbb40a86e0 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Mon, 7 Sep 2026 16:45:10 -0400 Subject: [PATCH 2/2] Derive upgrade test version expectation from roller-version.properties DatabaseInstallerUpgradeTest pinned the expected database version to the current release constant ("616"), so the test would fail on the next version bump even though the behavior it checks is unchanged. The test now derives its expectation from the same source the installer uses -- the ro.version property in roller-version.properties, parsed with the same DatabaseInstaller.parseVersionString -- so the widening of that pure helper to package-private static is the only production change. JDK 11 app verify: 301 tests, 0 failures, 1 skipped, unchanged. --- .../business/startup/DatabaseInstaller.java | 3 ++- .../startup/DatabaseInstallerUpgradeTest.java | 13 +++++++++++-- 2 files changed, 13 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 ab2819aa0c..1267d4d26e 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 @@ -853,7 +853,8 @@ private int getDatabaseVersion() throws StartupException { } - private int parseVersionString(String vstring) { + // package-private so tests can parse versions exactly as the installer does + static int parseVersionString(String vstring) { int myversion = 0; // NOTE: this assumes a maximum of 3 digits for the version number 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 a55191b53c..280dde3ee7 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 @@ -7,6 +7,7 @@ package org.apache.roller.weblogger.business.startup; import java.sql.*; +import java.util.Properties; import org.apache.roller.weblogger.business.DatabaseProvider; import org.junit.jupiter.api.Test; import static org.mockito.Mockito.*; @@ -28,11 +29,19 @@ void versionOnlyUpgradeReportsCompletion() throws Exception { when(rows.getString(1)).thenReturn("610"); when(con.prepareStatement(anyString())).thenReturn(update); DatabaseInstaller installer = new DatabaseInstaller(db, scripts); + + // the version the installer writes is the one it reads from + // /roller-version.properties, so derive the expectation from the + // same source and parser instead of pinning a release constant + Properties props = new Properties(); + props.load(getClass().getResourceAsStream("/roller-version.properties")); + int expectedVersion = DatabaseInstaller.parseVersionString(props.getProperty("ro.version", "UNKNOWN")); + installer.upgradeDatabase(true); - verify(update).setString(1, "616"); + verify(update).setString(1, String.valueOf(expectedVersion)); verify(update).executeUpdate(); verifyNoInteractions(scripts); 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 616."))); + assertTrue(installer.getMessages().stream().anyMatch(m -> m.contains("Database version updated to " + expectedVersion + "."))); } }