From c2dcf226d8cd6b65985c8c535b6813b81d29b013 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 14 Aug 2026 17:12:07 -0700 Subject: [PATCH 1/8] Add audit log for successful LK version changes --- api/src/org/labkey/api/ApiModule.java | 7 + .../provider/SystemUpgradeAuditProvider.java | 474 ++++++++++++++++++ .../org/labkey/api/module/ModuleLoader.java | 22 +- .../org/labkey/api/util/VersionNumber.java | 122 ++++- 4 files changed, 603 insertions(+), 22 deletions(-) create mode 100644 api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 4457534ada3..4c06c05a3d6 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -32,6 +32,8 @@ import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.ImageServlet; import org.labkey.api.attachments.LookAndFeelResourceType; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.SystemUpgradeAuditProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.cache.BlockingCache; import org.labkey.api.collections.ArrayListMap; @@ -184,6 +186,7 @@ import org.labkey.api.util.SystemMaintenanceStartupListener; import org.labkey.api.util.URIUtil; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.VersionNumber; import org.labkey.api.util.XmlBeansUtil; import org.labkey.api.util.emailTemplate.EmailTemplate; import org.labkey.api.view.ActionURL; @@ -298,9 +301,11 @@ protected void doStartup(ModuleContext moduleContext) ContentSecurityPolicyFilter.registerMetricsProvider(); ApiKeyManager.get().handleStartupProperties(); MailHelper.init(); + AuditLogService.get().registerAuditType(new SystemUpgradeAuditProvider()); // Handle system maintenance startup properties as late as possible; we want all system maintenance tasks to be registered first ContextListener.addStartupListener(new SystemMaintenanceStartupListener()); ContextListener.addStartupListener(new StartupPropertyStartupListener()); + ContextListener.addStartupListener(new SystemUpgradeAuditProvider.SystemUpgradeStartupListener()); } @Override @@ -485,12 +490,14 @@ public void registerServlets(ServletContext servletCtx) SubfolderWriter.TestCase.class, SvgUtil.TestCase.class, SwapQueue.TestCase.class, + SystemUpgradeAuditProvider.TestCase.class, TSVMapWriter.Tests.class, TSVWriter.TestCase.class, TabLoader.HeaderMatchTest.class, Table.IsSelectTestCase.class, URIUtil.TestCase.class, ValidEmail.TestCase.class, + VersionNumber.TestCase.class, XmlBeansUtil.TestCase.class ); } diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java new file mode 100644 index 00000000000..8d6046b43b2 --- /dev/null +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -0,0 +1,474 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.audit.provider; + +import jakarta.servlet.ServletContext; +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.audit.AuditTypeProvider; +import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.Sort; +import org.labkey.api.exp.PropertyDescriptor; +import org.labkey.api.exp.PropertyType; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.query.FieldKey; +import org.labkey.api.security.User; +import org.labkey.api.settings.AppProps; +import org.labkey.api.util.StartupListener; +import org.labkey.api.util.VersionNumber; +import org.labkey.api.util.logging.LogHelper; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +/** + * Records one event each time the server comes up running a different release version or build than the previous boot. + * Written by the startup listener registered in ApiModule.doStartup, which runs after the audit providers have been + * initialized. + */ +public class SystemUpgradeAuditProvider extends AbstractAuditTypeProvider implements AuditTypeProvider +{ + private static final Logger LOG = LogHelper.getLogger(SystemUpgradeAuditProvider.class, "Recording of server version changes"); + + public static final String AUDIT_EVENT_TYPE = "SystemUpgradeAuditEvent"; + + public static final String COLUMN_NAME_RELEASE_VERSION = "ReleaseVersion"; + public static final String COLUMN_NAME_PREVIOUS_RELEASE_VERSION = "PreviousReleaseVersion"; + public static final String COLUMN_NAME_BUILD_TIME = "BuildTime"; + public static final String COLUMN_NAME_PREVIOUS_BUILD_TIME = "PreviousBuildTime"; + public static final String COLUMN_NAME_CHANGE_TYPE = "ChangeType"; + public static final String COLUMN_NAME_HAS_SCHEMA_UPGRADE = "HasSchemaUpgrade"; + public static final String COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE = "HasExternalSchemaUpgrade"; + + private static final List defaultVisibleColumns = new ArrayList<>(); + + static + { + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CHANGE_TYPE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_PREVIOUS_RELEASE_VERSION)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_RELEASE_VERSION)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_BUILD_TIME)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_HAS_SCHEMA_UPGRADE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); + } + + public SystemUpgradeAuditProvider() + { + super(new SystemUpgradeAuditDomainKind()); + } + + @Override + public String getEventName() + { + return AUDIT_EVENT_TYPE; + } + + @Override + public String getLabel() + { + return "System Upgrade events"; + } + + @Override + public String getDescription() + { + return "Displays information about changes to the server's release version and build."; + } + + @Override + public Class getEventClass() + { + return SystemUpgradeAuditEvent.class; + } + + @Override + public List getDefaultVisibleColumns() + { + return defaultVisibleColumns; + } + + public enum ChangeType + { + /** First boot of a new installation */ + Install, + /** Release version moved forward */ + Upgrade, + /** Release version moved backward */ + Downgrade, + /** Same release/snapshot version, different build */ + Rebuild, + /** Baseline for a server that predates this feature, or a version string we can't parse */ + Unknown + } + + /** + * @return the change to record, or null if this boot matches the previous event and nothing should be recorded + */ + static @Nullable ChangeType determineChangeType(@Nullable String prevVersion, @Nullable String prevBuildTime, @Nullable String newVersion, @Nullable String newBuildTime, boolean newInstall) + { + // No prior event: either a fresh install or an existing server picking up this feature. We don't backfill, so + // the latter gets a baseline event with no direction. + if (prevVersion == null && prevBuildTime == null) + return newInstall ? ChangeType.Install : ChangeType.Unknown; + + if (Objects.equals(prevVersion, newVersion) && Objects.equals(prevBuildTime, newBuildTime)) + return null; + + Integer comparison = compareVersions(prevVersion, newVersion); + + if (comparison == null) + return ChangeType.Unknown; + if (comparison < 0) + return ChangeType.Upgrade; + if (comparison > 0) + return ChangeType.Downgrade; + + return ChangeType.Rebuild; + } + + /** + * @return the sign of prevVersion compared to newVersion, or null if either is missing or unparseable + */ + private static @Nullable Integer compareVersions(@Nullable String prevVersion, @Nullable String newVersion) + { + if (prevVersion == null || newVersion == null) + return null; + + try + { + return new VersionNumber(prevVersion).compareTo(new VersionNumber(newVersion)); + } + catch (RuntimeException e) + { + return null; + } + } + + static String buildComment(ChangeType changeType, @Nullable String prevVersion, @Nullable String newVersion) + { + if (ChangeType.Install == changeType) + return "Server installed at version " + newVersion; + if (ChangeType.Rebuild == changeType) + return "Server rebuilt at version " + newVersion; + if (null == prevVersion) + return "Server running version " + newVersion; + + return "Server version changed from " + prevVersion + " to " + newVersion; + } + + /** + * Must be registered from a Module.doStartup(), not init(). The provisioned audit table is created by + * AuditLogImpl's own StartupListener, which registers itself from AuditModule.init(); registering here guarantees + * we run after it, when the table exists and the queued events have been flushed. + */ + public static class SystemUpgradeStartupListener implements StartupListener + { + @Override + public String getName() + { + return "System Upgrade Audit"; + } + + @Override + public void moduleStartupComplete(ServletContext servletContext) + { + try + { + recordVersionChange(); + } + catch (Exception e) + { + // Treat a failure to record as a non-fatal startup problem + LOG.error("Failed to record system upgrade audit event", e); + } + } + } + + private static void recordVersionChange() + { + AuditLogService auditLog = AuditLogService.get(); + ModuleLoader moduleLoader = ModuleLoader.getInstance(); + + if (!moduleLoader.shouldInsertData()) + return; + + // No audit module, so the service is DefaultAuditProvider, which discards whatever we hand it + if (!auditLog.isViewable()) + return; + + User user = User.getAdminServiceUser(); + Container root = ContainerManager.getRoot(); + + List priorEvents = auditLog.getAuditEvents(root, user, AUDIT_EVENT_TYPE, null, new Sort("-Created,-RowId")); + SystemUpgradeAuditEvent prior = priorEvents.isEmpty() ? null : priorEvents.getFirst(); + + String prevVersion = null != prior ? prior.getReleaseVersion() : null; + String prevBuildTime = null != prior ? prior.getBuildTime() : null; + String newVersion = AppProps.getInstance().getReleaseVersion(); + String newBuildTime = moduleLoader.getCoreModule().getBuildTime(); + + ChangeType changeType = determineChangeType(prevVersion, prevBuildTime, newVersion, newBuildTime, moduleLoader.isNewInstall()); + + // Same version and build as the last event: an ordinary restart + if (null == changeType) + return; + + SystemUpgradeAuditEvent event = new SystemUpgradeAuditEvent(root, buildComment(changeType, prevVersion, newVersion)); + event.setChangeType(changeType.name()); + event.setReleaseVersion(newVersion); + event.setPreviousReleaseVersion(prevVersion); + event.setBuildTime(newBuildTime); + event.setPreviousBuildTime(prevBuildTime); + event.setHasSchemaUpgrade(moduleLoader.hasSchemaUpgrade()); + event.setHasExternalSchemaUpgrade(moduleLoader.hasExternalSchemaUpgrade()); + + auditLog.addEvent(user, event); + LOG.info(event.getComment()); + } + + public static class SystemUpgradeAuditEvent extends AuditTypeEvent + { + private String _releaseVersion; + private String _previousReleaseVersion; + private String _buildTime; + private String _previousBuildTime; + private String _changeType; + private boolean _hasSchemaUpgrade; + private boolean _hasExternalSchemaUpgrade; + + /** Important for reflection-based instantiation */ + @SuppressWarnings("unused") + public SystemUpgradeAuditEvent() {} + + public SystemUpgradeAuditEvent(Container container, String comment) + { + super(AUDIT_EVENT_TYPE, container, comment); + } + + public String getReleaseVersion() + { + return _releaseVersion; + } + + public void setReleaseVersion(String releaseVersion) + { + _releaseVersion = releaseVersion; + } + + public String getPreviousReleaseVersion() + { + return _previousReleaseVersion; + } + + public void setPreviousReleaseVersion(String previousReleaseVersion) + { + _previousReleaseVersion = previousReleaseVersion; + } + + public String getBuildTime() + { + return _buildTime; + } + + public void setBuildTime(String buildTime) + { + _buildTime = buildTime; + } + + public String getPreviousBuildTime() + { + return _previousBuildTime; + } + + public void setPreviousBuildTime(String previousBuildTime) + { + _previousBuildTime = previousBuildTime; + } + + public String getChangeType() + { + return _changeType; + } + + public void setChangeType(String changeType) + { + _changeType = changeType; + } + + public boolean isHasSchemaUpgrade() + { + return _hasSchemaUpgrade; + } + + public void setHasSchemaUpgrade(boolean hasSchemaUpgrade) + { + _hasSchemaUpgrade = hasSchemaUpgrade; + } + + public boolean isHasExternalSchemaUpgrade() + { + return _hasExternalSchemaUpgrade; + } + + public void setHasExternalSchemaUpgrade(boolean hasExternalSchemaUpgrade) + { + _hasExternalSchemaUpgrade = hasExternalSchemaUpgrade; + } + + @Override + public Map getAuditLogMessageElements() + { + Map elements = new LinkedHashMap<>(); + + elements.put("changeType", getChangeType()); + elements.put("previousReleaseVersion", getPreviousReleaseVersion()); + elements.put("releaseVersion", getReleaseVersion()); + elements.put("previousBuildTime", getPreviousBuildTime()); + elements.put("buildTime", getBuildTime()); + elements.put("hasSchemaUpgrade", isHasSchemaUpgrade()); + elements.put("hasExternalSchemaUpgrade", isHasExternalSchemaUpgrade()); + elements.putAll(super.getAuditLogMessageElements()); + return elements; + } + } + + public static class SystemUpgradeAuditDomainKind extends AbstractAuditDomainKind + { + public static final String NAME = "SystemUpgradeAuditDomain"; + public static String NAMESPACE_PREFIX = "Audit-" + NAME; + + private final Set _fields; + + public SystemUpgradeAuditDomainKind() + { + super(AUDIT_EVENT_TYPE); + + Set fields = new LinkedHashSet<>(); + fields.add(createPropertyDescriptor(COLUMN_NAME_RELEASE_VERSION, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_PREVIOUS_RELEASE_VERSION, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_BUILD_TIME, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_PREVIOUS_BUILD_TIME, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_CHANGE_TYPE, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_HAS_SCHEMA_UPGRADE, PropertyType.BOOLEAN)); + fields.add(createPropertyDescriptor(COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE, PropertyType.BOOLEAN)); + _fields = Collections.unmodifiableSet(fields); + } + + @Override + public Set getProperties() + { + return _fields; + } + + @Override + protected String getNamespacePrefix() + { + return NAMESPACE_PREFIX; + } + + @Override + public String getKindName() + { + return NAME; + } + } + + public static class TestCase extends Assert + { + private static final String BUILD_1 = "Aug 11, 2026, 4:49:29 PM"; + private static final String BUILD_2 = "Aug 12, 2026, 9:01:02 AM"; + + @Test + public void testNoPriorEvent() + { + assertEquals(ChangeType.Install, determineChangeType(null, null, "26.9.0", BUILD_1, true)); + assertEquals(ChangeType.Unknown, determineChangeType(null, null, "26.9.0", BUILD_1, false)); + } + + @Test + public void testNoChange() + { + assertNull(determineChangeType("26.9.0", BUILD_1, "26.9.0", BUILD_1, false)); + assertNull(determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9-SNAPSHOT", BUILD_1, false)); + } + + @Test + public void testUpgrade() + { + assertEquals(ChangeType.Upgrade, determineChangeType("26.7.1", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Upgrade, determineChangeType("26.9.0", BUILD_1, "26.9.1", BUILD_2, false)); + assertEquals(ChangeType.Upgrade, determineChangeType("25.11.0", BUILD_1, "26.3.0", BUILD_2, false)); + assertEquals(ChangeType.Upgrade, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9.0", BUILD_2, false)); + } + + @Test + public void testDowngrade() + { + assertEquals(ChangeType.Downgrade, determineChangeType("26.9.0", BUILD_1, "26.7.1", BUILD_2, false)); + assertEquals(ChangeType.Downgrade, determineChangeType("26.9.1", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Downgrade, determineChangeType("26.9.0", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); + } + + /** Regression guard for VersionNumber.getVersionInt(), which maps both 26.11 and 26.1 to 261 */ + @Test + public void testDoubleDigitMinor() + { + assertEquals(ChangeType.Upgrade, determineChangeType("26.1.0", BUILD_1, "26.11.0", BUILD_2, false)); + assertEquals(ChangeType.Downgrade, determineChangeType("26.11.0", BUILD_1, "26.1.0", BUILD_2, false)); + } + + @Test + public void testRebuild() + { + assertEquals(ChangeType.Rebuild, determineChangeType("26.9.0", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Rebuild, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); + assertEquals(ChangeType.Rebuild, determineChangeType("26.9.0", null, "26.9.0", BUILD_2, false)); + } + + @Test + public void testUnparseableVersion() + { + assertEquals(ChangeType.Unknown, determineChangeType("bogus", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Unknown, determineChangeType("26.9.0", BUILD_1, "", BUILD_2, false)); + assertEquals(ChangeType.Unknown, determineChangeType("26.9.0", BUILD_1, null, BUILD_2, false)); + } + + @Test + public void testComments() + { + assertEquals("Server installed at version 26.9.0", buildComment(ChangeType.Install, null, "26.9.0")); + assertEquals("Server running version 26.9.0", buildComment(ChangeType.Unknown, null, "26.9.0")); + assertEquals("Server rebuilt at version 26.9.0", buildComment(ChangeType.Rebuild, "26.9.0", "26.9.0")); + assertEquals("Server version changed from 26.7.1 to 26.9.0", buildComment(ChangeType.Upgrade, "26.7.1", "26.9.0")); + assertEquals("Server version changed from 26.9.0 to 26.7.1", buildComment(ChangeType.Downgrade, "26.9.0", "26.7.1")); + } + } +} diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 5920b0a99d2..783cf953f07 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -241,6 +241,10 @@ public enum ModuleState */ private final List _modulesImmutable = Collections.unmodifiableList(_modules); + // Whether this startup ran any schema scripts, used by the system upgrade audit event + private volatile boolean _hasSchemaUpgrade = false; + private volatile boolean _hasExternalSchemaUpgrade = false; + // Allow multiple StartupPropertyHandlers with the same scope as long as the StartupProperty impl class is different. private final Set> _startupPropertyHandlers = new ConcurrentSkipListSet<>(Comparator.comparing((StartupPropertyHandler sph) -> sph.getScope(), String.CASE_INSENSITIVE_ORDER).thenComparing(StartupPropertyHandler::getStartupPropertyClassName)); private final MultiValuedMap _startupPropertyMap = new CaseInsensitiveKeyedHashSetValuedMap<>(); @@ -663,7 +667,8 @@ boolean isTooOld() // Now that we know if this is a new install... setDatabaseMigrationConfiguration(labkeyRoot); - upgradeCoreModule(lockFile); + // Core is upgraded before modulesRequiringUpgrade is built, so a core-only upgrade shows up nowhere else + boolean coreUpgraded = upgradeCoreModule(lockFile); // Issue 40422 - log server and session GUIDs during startup. Do it after the core module has // been bootstrapped/upgraded to ensure that AppProps is ready @@ -776,6 +781,9 @@ public void addStaticWarnings(@NotNull Warnings warnings, boolean showAllWarning }); } + _hasSchemaUpgrade = coreUpgraded || !modulesRequiringUpgrade.isEmpty(); + _hasExternalSchemaUpgrade = !additionalSchemasRequiringUpgrade.isEmpty(); + if (!modulesRequiringUpgrade.isEmpty()) _log.info("Modules requiring upgrade: {}", modulesRequiringUpgrade); @@ -2071,6 +2079,18 @@ public boolean isNewInstall() return _newInstall; } + /** Did any module, including core, run schema scripts during this startup? */ + public boolean hasSchemaUpgrade() + { + return _hasSchemaUpgrade; + } + + /** Did any schema in an external data source get installed or upgraded during this startup? */ + public boolean hasExternalSchemaUpgrade() + { + return _hasExternalSchemaUpgrade; + } + private void setDatabaseMigrationConfiguration(FileLike labkeyRoot) { _databaseMigrationConfiguration = DatabaseMigrationService.get().getDatabaseMigrationConfiguration(labkeyRoot, _migration); diff --git a/api/src/org/labkey/api/util/VersionNumber.java b/api/src/org/labkey/api/util/VersionNumber.java index 1c4db051a6a..c50b192cfd8 100644 --- a/api/src/org/labkey/api/util/VersionNumber.java +++ b/api/src/org/labkey/api/util/VersionNumber.java @@ -22,7 +22,13 @@ * Time: 11:41:14 AM */ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; + import java.io.Serializable; +import java.util.Objects; import static org.labkey.api.util.IntegerUtils.asIntegerElseNull; @@ -31,9 +37,9 @@ * based on the format "major.minor.revision". Revision can be either an integer * or a string. */ -public class VersionNumber implements Serializable +public class VersionNumber implements Serializable, Comparable { - private int _major; + private final int _major; private int _minor = 0; private Object _revision = null; @@ -128,27 +134,101 @@ public String toString() return _major + "." + _minor; } - // - // Returns major & minor version numbers (but not revision) in an int form that's easier for range checking: - // - // major * 10 + minor - // - // Examples: - // - // 8.3 --> 83 - // 8.4 --> 84 - // 9.0 --> 90 - // - // Requires (and validates that) minor version is a single-digit (0 <= minor <= 9) - // + /** + * Packs major and minor version into an int that is easy to range check, ignoring revision: {@code major * 10 + minor}. + * For example, 8.3 gives 83, 8.4 gives 84, and 9.0 gives 90. + *

+ * Caveat: a minor version above 9 is divided by ten first, so 26.11 and 26.1 both give 261. Use + * {@link #compareTo(VersionNumber)} to order versions whose minor may reach two digits. + * + * @throws IllegalStateException if the minor version is still outside 0-9 after that adjustment + */ public int getVersionInt() { - // Temporary fix for SQL Server 2008 R2 (10.50.1600.1). TODO: Support two-digit version ints (MMmm) - if (_minor > 9) - _minor = _minor / 10; - if (_minor > 9 || _minor < 0) - throw new IllegalStateException("Bad minor version: " + _minor); + // Temporary fix for SQL Server 2008 R2 (10.50.1600.1). TODO: Support two-digit version ints (MMmm) + int minor = _minor > 9 ? _minor / 10 : _minor; + if (minor > 9 || minor < 0) + throw new IllegalStateException("Bad minor version: " + minor); + + return _major * 10 + minor; + } + + /** + * Orders by major, then minor, then revision, so unlike {@link #getVersionInt()} this is safe for minor versions + * above 9. A missing revision sorts first, which puts "26.9-SNAPSHOT" (parsed as 26.9 with no revision) ahead of + * the released "26.9.0". + *

+ * Not consistent with equals(), which this class does not override. + */ + @Override + public int compareTo(@NotNull VersionNumber o) + { + int result = Integer.compare(_major, o._major); + if (result != 0) + return result; + + result = Integer.compare(_minor, o._minor); + if (result != 0) + return result; + + return compareRevisions(_revision, o._revision); + } + + private static int compareRevisions(@Nullable Object revision1, @Nullable Object revision2) + { + if (Objects.equals(revision1, revision2)) + return 0; + if (null == revision1) + return -1; + if (null == revision2) + return 1; + if (revision1 instanceof Integer int1 && revision2 instanceof Integer int2) + return Integer.compare(int1, int2); + + // Mixed or non-numeric revisions, e.g. PostgreSQL's "4beta1" + return revision1.toString().compareTo(revision2.toString()); + } - return _major * 10 + _minor; + public static class TestCase extends Assert + { + @Test + public void testOrdering() + { + assertBefore("26.7.1", "26.9.0"); + assertBefore("26.9.0", "26.9.1"); + assertBefore("25.11.0", "26.3.0"); + assertBefore("26.9", "26.9.0"); + assertBefore("26.9-SNAPSHOT", "26.9.0"); + + assertEquals(0, new VersionNumber("26.9.0").compareTo(new VersionNumber("26.9.0"))); + assertEquals(0, new VersionNumber("26.9-SNAPSHOT").compareTo(new VersionNumber("26.9-SNAPSHOT"))); + } + + /** getVersionInt() maps both to 261, so ordering must not be built on it */ + @Test + public void testDoubleDigitMinor() + { + assertBefore("26.1.0", "26.11.0"); + assertEquals(261, new VersionNumber("26.1").getVersionInt()); + assertEquals(261, new VersionNumber("26.11").getVersionInt()); + } + + /** getVersionInt() used to divide the minor version in place, which would corrupt later comparisons */ + @Test + public void testGetVersionIntDoesNotMutate() + { + VersionNumber version = new VersionNumber("10.50.1600"); + assertEquals(105, version.getVersionInt()); + assertEquals(50, version.getMinor()); + assertEquals(105, version.getVersionInt()); + } + + private void assertBefore(String earlier, String later) + { + VersionNumber first = new VersionNumber(earlier); + VersionNumber second = new VersionNumber(later); + assertTrue(earlier + " should sort before " + later, first.compareTo(second) < 0); + assertTrue(later + " should sort after " + earlier, second.compareTo(first) > 0); + } } } From bcd314845800e4be956c025b0f3c53fc6d66490b Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 14 Aug 2026 17:57:42 -0700 Subject: [PATCH 2/8] Claude review --- .../org/labkey/api/audit/AuditLogService.java | 9 ++- .../api/audit/DefaultAuditProvider.java | 2 +- .../provider/SystemUpgradeAuditProvider.java | 72 +++++++++++-------- .../org/labkey/api/module/ModuleLoader.java | 8 --- .../org/labkey/api/util/VersionNumber.java | 21 ++++-- audit/src/org/labkey/audit/AuditLogImpl.java | 4 +- .../org/labkey/audit/model/LogManager.java | 6 ++ 7 files changed, 75 insertions(+), 47 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditLogService.java b/api/src/org/labkey/api/audit/AuditLogService.java index 4850bfa92aa..f6764fb3180 100644 --- a/api/src/org/labkey/api/audit/AuditLogService.java +++ b/api/src/org/labkey/api/audit/AuditLogService.java @@ -24,6 +24,7 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; +import org.labkey.api.data.Table; import org.labkey.api.module.ModuleLoader; import org.labkey.api.query.QueryService; import org.labkey.api.query.UserSchema; @@ -114,7 +115,13 @@ default void addEvents(@Nullable User user, List e List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort); - List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf); + default List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf) + { + return getAuditEvents(container, user, eventType, filter, sort, cf, Table.ALL_ROWS); + } + + /** @param maxRows a positive count, or {@link Table#ALL_ROWS} */ + List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf, int maxRows); UserSchema createSchema(User user, Container container); diff --git a/api/src/org/labkey/api/audit/DefaultAuditProvider.java b/api/src/org/labkey/api/audit/DefaultAuditProvider.java index e5649b19e9d..a407d9fe831 100644 --- a/api/src/org/labkey/api/audit/DefaultAuditProvider.java +++ b/api/src/org/labkey/api/audit/DefaultAuditProvider.java @@ -75,7 +75,7 @@ public List getAuditEvents(Container container, Us } @Override - public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf) + public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf, int maxRows) { return Collections.emptyList(); } diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java index 8d6046b43b2..33d65663ac0 100644 --- a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -16,6 +16,7 @@ package org.labkey.api.audit.provider; import jakarta.servlet.ServletContext; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.junit.Assert; @@ -64,7 +65,6 @@ public class SystemUpgradeAuditProvider extends AbstractAuditTypeProvider implem public static final String COLUMN_NAME_PREVIOUS_BUILD_TIME = "PreviousBuildTime"; public static final String COLUMN_NAME_CHANGE_TYPE = "ChangeType"; public static final String COLUMN_NAME_HAS_SCHEMA_UPGRADE = "HasSchemaUpgrade"; - public static final String COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE = "HasExternalSchemaUpgrade"; private static final List defaultVisibleColumns = new ArrayList<>(); @@ -76,7 +76,6 @@ public class SystemUpgradeAuditProvider extends AbstractAuditTypeProvider implem defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_RELEASE_VERSION)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_BUILD_TIME)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_HAS_SCHEMA_UPGRADE)); - defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); } @@ -125,7 +124,7 @@ public enum ChangeType Downgrade, /** Same release/snapshot version, different build */ Rebuild, - /** Baseline for a server that predates this feature, or a version string we can't parse */ + /** Baseline for a server that predates this feature, a version we can't parse, or a change with no knowable direction */ Unknown } @@ -142,29 +141,37 @@ public enum ChangeType if (Objects.equals(prevVersion, newVersion) && Objects.equals(prevBuildTime, newBuildTime)) return null; - Integer comparison = compareVersions(prevVersion, newVersion); + VersionNumber prev = parseVersion(prevVersion); + VersionNumber current = parseVersion(newVersion); - if (comparison == null) + if (null == prev || null == current) return ChangeType.Unknown; - if (comparison < 0) - return ChangeType.Upgrade; - if (comparison > 0) - return ChangeType.Downgrade; - return ChangeType.Rebuild; + int comparison = prev.compareTo(current); + + if (0 == comparison) + return ChangeType.Rebuild; + + // A releaseXX.Y-SNAPSHOT build keeps that version string for the entire patch line, so it can be either older + // or newer than a given XX.Y.Z release and the strings alone establish no direction. + if (prev.getMajor() == current.getMajor() && prev.getMinor() == current.getMinor() + && isSnapshot(prevVersion) != isSnapshot(newVersion)) + return ChangeType.Unknown; + + return comparison < 0 ? ChangeType.Upgrade : ChangeType.Downgrade; } /** - * @return the sign of prevVersion compared to newVersion, or null if either is missing or unparseable + * @return the parsed version, or null if it's missing or unparseable */ - private static @Nullable Integer compareVersions(@Nullable String prevVersion, @Nullable String newVersion) + private static @Nullable VersionNumber parseVersion(@Nullable String version) { - if (prevVersion == null || newVersion == null) + if (null == version) return null; try { - return new VersionNumber(prevVersion).compareTo(new VersionNumber(newVersion)); + return new VersionNumber(version); } catch (RuntimeException e) { @@ -172,6 +179,11 @@ public enum ChangeType } } + private static boolean isSnapshot(String version) + { + return StringUtils.containsIgnoreCase(version, "SNAPSHOT"); + } + static String buildComment(ChangeType changeType, @Nullable String prevVersion, @Nullable String newVersion) { if (ChangeType.Install == changeType) @@ -227,7 +239,7 @@ private static void recordVersionChange() User user = User.getAdminServiceUser(); Container root = ContainerManager.getRoot(); - List priorEvents = auditLog.getAuditEvents(root, user, AUDIT_EVENT_TYPE, null, new Sort("-Created,-RowId")); + List priorEvents = auditLog.getAuditEvents(root, user, AUDIT_EVENT_TYPE, null, new Sort("-Created,-RowId"), null, 1); SystemUpgradeAuditEvent prior = priorEvents.isEmpty() ? null : priorEvents.getFirst(); String prevVersion = null != prior ? prior.getReleaseVersion() : null; @@ -248,7 +260,6 @@ private static void recordVersionChange() event.setBuildTime(newBuildTime); event.setPreviousBuildTime(prevBuildTime); event.setHasSchemaUpgrade(moduleLoader.hasSchemaUpgrade()); - event.setHasExternalSchemaUpgrade(moduleLoader.hasExternalSchemaUpgrade()); auditLog.addEvent(user, event); LOG.info(event.getComment()); @@ -262,7 +273,6 @@ public static class SystemUpgradeAuditEvent extends AuditTypeEvent private String _previousBuildTime; private String _changeType; private boolean _hasSchemaUpgrade; - private boolean _hasExternalSchemaUpgrade; /** Important for reflection-based instantiation */ @SuppressWarnings("unused") @@ -333,16 +343,6 @@ public void setHasSchemaUpgrade(boolean hasSchemaUpgrade) _hasSchemaUpgrade = hasSchemaUpgrade; } - public boolean isHasExternalSchemaUpgrade() - { - return _hasExternalSchemaUpgrade; - } - - public void setHasExternalSchemaUpgrade(boolean hasExternalSchemaUpgrade) - { - _hasExternalSchemaUpgrade = hasExternalSchemaUpgrade; - } - @Override public Map getAuditLogMessageElements() { @@ -354,7 +354,6 @@ public Map getAuditLogMessageElements() elements.put("previousBuildTime", getPreviousBuildTime()); elements.put("buildTime", getBuildTime()); elements.put("hasSchemaUpgrade", isHasSchemaUpgrade()); - elements.put("hasExternalSchemaUpgrade", isHasExternalSchemaUpgrade()); elements.putAll(super.getAuditLogMessageElements()); return elements; } @@ -378,7 +377,6 @@ public SystemUpgradeAuditDomainKind() fields.add(createPropertyDescriptor(COLUMN_NAME_PREVIOUS_BUILD_TIME, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_CHANGE_TYPE, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_HAS_SCHEMA_UPGRADE, PropertyType.BOOLEAN)); - fields.add(createPropertyDescriptor(COLUMN_NAME_HAS_EXTERNAL_SCHEMA_UPGRADE, PropertyType.BOOLEAN)); _fields = Collections.unmodifiableSet(fields); } @@ -426,7 +424,10 @@ public void testUpgrade() assertEquals(ChangeType.Upgrade, determineChangeType("26.7.1", BUILD_1, "26.9.0", BUILD_2, false)); assertEquals(ChangeType.Upgrade, determineChangeType("26.9.0", BUILD_1, "26.9.1", BUILD_2, false)); assertEquals(ChangeType.Upgrade, determineChangeType("25.11.0", BUILD_1, "26.3.0", BUILD_2, false)); - assertEquals(ChangeType.Upgrade, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9.0", BUILD_2, false)); + + // Crossing a release number is unambiguous even when one side is a snapshot + assertEquals(ChangeType.Upgrade, determineChangeType("26.7.1", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); + assertEquals(ChangeType.Upgrade, determineChangeType("26.7-SNAPSHOT", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); } @Test @@ -434,7 +435,16 @@ public void testDowngrade() { assertEquals(ChangeType.Downgrade, determineChangeType("26.9.0", BUILD_1, "26.7.1", BUILD_2, false)); assertEquals(ChangeType.Downgrade, determineChangeType("26.9.1", BUILD_1, "26.9.0", BUILD_2, false)); - assertEquals(ChangeType.Downgrade, determineChangeType("26.9.0", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); + assertEquals(ChangeType.Downgrade, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.7.1", BUILD_2, false)); + } + + /** release26.9-SNAPSHOT keeps that version string for the whole patch line, so it may precede or follow 26.9.0 */ + @Test + public void testSnapshotWithinRelease() + { + assertEquals(ChangeType.Unknown, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Unknown, determineChangeType("26.9.0", BUILD_1, "26.9-SNAPSHOT", BUILD_2, false)); + assertEquals(ChangeType.Unknown, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.9.4", BUILD_2, false)); } /** Regression guard for VersionNumber.getVersionInt(), which maps both 26.11 and 26.1 to 261 */ diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 783cf953f07..5fd34410896 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -243,7 +243,6 @@ public enum ModuleState // Whether this startup ran any schema scripts, used by the system upgrade audit event private volatile boolean _hasSchemaUpgrade = false; - private volatile boolean _hasExternalSchemaUpgrade = false; // Allow multiple StartupPropertyHandlers with the same scope as long as the StartupProperty impl class is different. private final Set> _startupPropertyHandlers = new ConcurrentSkipListSet<>(Comparator.comparing((StartupPropertyHandler sph) -> sph.getScope(), String.CASE_INSENSITIVE_ORDER).thenComparing(StartupPropertyHandler::getStartupPropertyClassName)); @@ -782,7 +781,6 @@ public void addStaticWarnings(@NotNull Warnings warnings, boolean showAllWarning } _hasSchemaUpgrade = coreUpgraded || !modulesRequiringUpgrade.isEmpty(); - _hasExternalSchemaUpgrade = !additionalSchemasRequiringUpgrade.isEmpty(); if (!modulesRequiringUpgrade.isEmpty()) _log.info("Modules requiring upgrade: {}", modulesRequiringUpgrade); @@ -2085,12 +2083,6 @@ public boolean hasSchemaUpgrade() return _hasSchemaUpgrade; } - /** Did any schema in an external data source get installed or upgraded during this startup? */ - public boolean hasExternalSchemaUpgrade() - { - return _hasExternalSchemaUpgrade; - } - private void setDatabaseMigrationConfiguration(FileLike labkeyRoot) { _databaseMigrationConfiguration = DatabaseMigrationService.get().getDatabaseMigrationConfiguration(labkeyRoot, _migration); diff --git a/api/src/org/labkey/api/util/VersionNumber.java b/api/src/org/labkey/api/util/VersionNumber.java index c50b192cfd8..99ad42bd10c 100644 --- a/api/src/org/labkey/api/util/VersionNumber.java +++ b/api/src/org/labkey/api/util/VersionNumber.java @@ -156,7 +156,7 @@ public int getVersionInt() /** * Orders by major, then minor, then revision, so unlike {@link #getVersionInt()} this is safe for minor versions * above 9. A missing revision sorts first, which puts "26.9-SNAPSHOT" (parsed as 26.9 with no revision) ahead of - * the released "26.9.0". + * the released "26.9.0"; a non-numeric revision sorts last. *

* Not consistent with equals(), which this class does not override. */ @@ -182,10 +182,13 @@ private static int compareRevisions(@Nullable Object revision1, @Nullable Object return -1; if (null == revision2) return 1; - if (revision1 instanceof Integer int1 && revision2 instanceof Integer int2) - return Integer.compare(int1, int2); + // Non-numeric revisions, e.g. PostgreSQL's "4beta1", all sort after the numeric ones. Comparing them lexically + // against numbers instead would be intransitive: 2 > "1x" > 10 > 2. + if (revision1 instanceof Integer int1) + return revision2 instanceof Integer int2 ? Integer.compare(int1, int2) : -1; + if (revision2 instanceof Integer) + return 1; - // Mixed or non-numeric revisions, e.g. PostgreSQL's "4beta1" return revision1.toString().compareTo(revision2.toString()); } @@ -213,6 +216,16 @@ public void testDoubleDigitMinor() assertEquals(261, new VersionNumber("26.11").getVersionInt()); } + /** Lexical comparison against numeric revisions would be intransitive, so non-numeric revisions sort last */ + @Test + public void testNonNumericRevision() + { + assertBefore("8.4.2", "8.4.4beta1"); + assertBefore("8.4.10", "8.4.1x"); + assertBefore("8.4.4beta1", "8.4.4rc1"); + assertEquals(0, new VersionNumber("8.4.4beta1").compareTo(new VersionNumber("8.4.4beta1"))); + } + /** getVersionInt() used to divide the minor version in place, which would corrupt later comparisons */ @Test public void testGetVersionIntDoesNotMutate() diff --git a/audit/src/org/labkey/audit/AuditLogImpl.java b/audit/src/org/labkey/audit/AuditLogImpl.java index 9c7e82bea13..5e9c3729fe6 100644 --- a/audit/src/org/labkey/audit/AuditLogImpl.java +++ b/audit/src/org/labkey/audit/AuditLogImpl.java @@ -238,9 +238,9 @@ public List getAuditEvents(Container container, Us } @Override - public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf) + public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf, int maxRows) { - return LogManager.get().getAuditEvents(container, user, eventType, filter, sort, cf); + return LogManager.get().getAuditEvents(container, user, eventType, filter, sort, cf, maxRows); } @Override diff --git a/audit/src/org/labkey/audit/model/LogManager.java b/audit/src/org/labkey/audit/model/LogManager.java index 5349ade7eb8..d0c54b92f4d 100644 --- a/audit/src/org/labkey/audit/model/LogManager.java +++ b/audit/src/org/labkey/audit/model/LogManager.java @@ -212,6 +212,11 @@ public List getAuditEvents(Container container, Us return getAuditEvents(container, user, eventType, filter, sort, null); } public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf) + { + return getAuditEvents(container, user, eventType, filter, sort, cf, Table.ALL_ROWS); + } + + public List getAuditEvents(Container container, User user, String eventType, @Nullable SimpleFilter filter, @Nullable Sort sort, @Nullable ContainerFilter cf, int maxRows) { AuditTypeProvider provider = AuditLogService.get().getAuditProvider(eventType); if (provider != null) @@ -222,6 +227,7 @@ public List getAuditEvents(Container container, Us { TableInfo table = schema.getTable(provider.getEventName(), cf); TableSelector selector = new TableSelector(table, filter, sort); + selector.setMaxRows(maxRows); return selector.getArrayList(provider.getEventClass()); } From bddd8413409a5f765b592c3febfe16fadaaa7458 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 17 Aug 2026 15:19:08 -0700 Subject: [PATCH 3/8] Introduce AppProps.getBuildTime() --- api/src/org/labkey/api/admin/AdminBean.java | 2 +- .../api/audit/provider/SystemUpgradeAuditProvider.java | 2 +- api/src/org/labkey/api/settings/AppProps.java | 8 ++++++++ api/src/org/labkey/api/settings/AppPropsImpl.java | 7 +++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/admin/AdminBean.java b/api/src/org/labkey/api/admin/AdminBean.java index 4c655762208..d47ffa94769 100644 --- a/api/src/org/labkey/api/admin/AdminBean.java +++ b/api/src/org/labkey/api/admin/AdminBean.java @@ -78,7 +78,7 @@ public static class RecentUser public static final String servletContainer = ModuleLoader.getServletContext().getServerInfo(); public static final String servletConfiguration = "Embedded"; public static final String sessionTimeout = Formats.commaf0.format(ModuleLoader.getServletContext().getSessionTimeout()); - public static final String buildTime = ModuleLoader.getInstance().getCoreModule().getBuildTime(); + public static final @Nullable String buildTime = AppProps.getInstance().getBuildTime(); public static final String serverStartupTime = DateUtil.formatDateTime(ContainerManager.getRoot()); public static String asserts = "disabled"; diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java index 33d65663ac0..c4bf75d7daf 100644 --- a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -245,7 +245,7 @@ private static void recordVersionChange() String prevVersion = null != prior ? prior.getReleaseVersion() : null; String prevBuildTime = null != prior ? prior.getBuildTime() : null; String newVersion = AppProps.getInstance().getReleaseVersion(); - String newBuildTime = moduleLoader.getCoreModule().getBuildTime(); + String newBuildTime = AppProps.getInstance().getBuildTime(); ChangeType changeType = determineChangeType(prevVersion, prevBuildTime, newVersion, newBuildTime, moduleLoader.isNewInstall()); diff --git a/api/src/org/labkey/api/settings/AppProps.java b/api/src/org/labkey/api/settings/AppProps.java index a8a65fa02cb..8be7bc2a18f 100644 --- a/api/src/org/labkey/api/settings/AppProps.java +++ b/api/src/org/labkey/api/settings/AppProps.java @@ -130,6 +130,14 @@ static WriteableAppProps getWriteableInstance() @NotNull String getReleaseVersion(); + /** + * Returns the core module's build time. Null if the module was built without one, which is why this has no + * "unknown" sentinel like {@link #getReleaseVersion()}: callers compare build times across restarts, and a sentinel + * would compare equal to a genuinely unknown build. + */ + @Nullable + String getBuildTime(); + /** * Convenience method for getting the core schema version, returning 0.0 instead of null */ diff --git a/api/src/org/labkey/api/settings/AppPropsImpl.java b/api/src/org/labkey/api/settings/AppPropsImpl.java index ac90bf67310..53518ad1623 100644 --- a/api/src/org/labkey/api/settings/AppPropsImpl.java +++ b/api/src/org/labkey/api/settings/AppPropsImpl.java @@ -533,6 +533,13 @@ public String getReleaseVersion() return ObjectUtils.defaultIfNull(ModuleLoader.getInstance().getCoreModule().getReleaseVersion(), UNKNOWN_VERSION); } + @Nullable + @Override + public String getBuildTime() + { + return ModuleLoader.getInstance().getCoreModule().getBuildTime(); + } + @Override public double getSchemaVersion() { From ecc51f1f2a147bf4ba9f88bdc7ce9df1a69db84d Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 17 Aug 2026 15:23:44 -0700 Subject: [PATCH 4/8] Comments --- .../api/audit/provider/SystemUpgradeAuditProvider.java | 2 +- api/src/org/labkey/api/settings/AppProps.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java index c4bf75d7daf..2e86b553ab1 100644 --- a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -187,7 +187,7 @@ private static boolean isSnapshot(String version) static String buildComment(ChangeType changeType, @Nullable String prevVersion, @Nullable String newVersion) { if (ChangeType.Install == changeType) - return "Server installed at version " + newVersion; + return "Server bootstrapped at version " + newVersion; if (ChangeType.Rebuild == changeType) return "Server rebuilt at version " + newVersion; if (null == prevVersion) diff --git a/api/src/org/labkey/api/settings/AppProps.java b/api/src/org/labkey/api/settings/AppProps.java index 8be7bc2a18f..a182527d9b5 100644 --- a/api/src/org/labkey/api/settings/AppProps.java +++ b/api/src/org/labkey/api/settings/AppProps.java @@ -131,9 +131,8 @@ static WriteableAppProps getWriteableInstance() String getReleaseVersion(); /** - * Returns the core module's build time. Null if the module was built without one, which is why this has no - * "unknown" sentinel like {@link #getReleaseVersion()}: callers compare build times across restarts, and a sentinel - * would compare equal to a genuinely unknown build. + * Returns the core module's build time. Null if the module was built without one, unlike the sentinel from + * {@link #getReleaseVersion()}. */ @Nullable String getBuildTime(); From fe362fb0348d722f1ca939f4f3c426a9aefeec70 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Tue, 18 Aug 2026 09:18:46 -0700 Subject: [PATCH 5/8] Update api/src/org/labkey/api/module/ModuleLoader.java Co-authored-by: Susan Hert --- api/src/org/labkey/api/module/ModuleLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 5fd34410896..f94570a92c9 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -241,7 +241,7 @@ public enum ModuleState */ private final List _modulesImmutable = Collections.unmodifiableList(_modules); - // Whether this startup ran any schema scripts, used by the system upgrade audit event + // Whether this startup ran any schema scripts. Used by the system upgrade audit event private volatile boolean _hasSchemaUpgrade = false; // Allow multiple StartupPropertyHandlers with the same scope as long as the StartupProperty impl class is different. From 7a4c347840e207c294fa0925958e5fb7b5e51e31 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Tue, 18 Aug 2026 09:18:59 -0700 Subject: [PATCH 6/8] Update api/src/org/labkey/api/util/VersionNumber.java Co-authored-by: Susan Hert --- api/src/org/labkey/api/util/VersionNumber.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/util/VersionNumber.java b/api/src/org/labkey/api/util/VersionNumber.java index 99ad42bd10c..be5fd725d97 100644 --- a/api/src/org/labkey/api/util/VersionNumber.java +++ b/api/src/org/labkey/api/util/VersionNumber.java @@ -156,7 +156,7 @@ public int getVersionInt() /** * Orders by major, then minor, then revision, so unlike {@link #getVersionInt()} this is safe for minor versions * above 9. A missing revision sorts first, which puts "26.9-SNAPSHOT" (parsed as 26.9 with no revision) ahead of - * the released "26.9.0"; a non-numeric revision sorts last. + * the released "26.9.0". A non-numeric revision sorts last. *

* Not consistent with equals(), which this class does not override. */ From a687528dc870ec2ce1df5e1dce744fbac24fd949 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Tue, 18 Aug 2026 09:33:41 -0700 Subject: [PATCH 7/8] Code review feedback --- .../api/audit/provider/SystemUpgradeAuditProvider.java | 8 +++++--- api/src/org/labkey/api/util/VersionNumber.java | 1 - 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java index 2e86b553ab1..5f4822ff1df 100644 --- a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -16,7 +16,7 @@ package org.labkey.api.audit.provider; import jakarta.servlet.ServletContext; -import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.junit.Assert; @@ -181,7 +181,7 @@ && isSnapshot(prevVersion) != isSnapshot(newVersion)) private static boolean isSnapshot(String version) { - return StringUtils.containsIgnoreCase(version, "SNAPSHOT"); + return Strings.CI.endsWith(version, "-SNAPSHOT"); } static String buildComment(ChangeType changeType, @Nullable String prevVersion, @Nullable String newVersion) @@ -423,6 +423,7 @@ public void testUpgrade() { assertEquals(ChangeType.Upgrade, determineChangeType("26.7.1", BUILD_1, "26.9.0", BUILD_2, false)); assertEquals(ChangeType.Upgrade, determineChangeType("26.9.0", BUILD_1, "26.9.1", BUILD_2, false)); + assertEquals(ChangeType.Upgrade, determineChangeType("26.9.0", BUILD_1, "26.9.1", BUILD_1, false)); assertEquals(ChangeType.Upgrade, determineChangeType("25.11.0", BUILD_1, "26.3.0", BUILD_2, false)); // Crossing a release number is unambiguous even when one side is a snapshot @@ -435,6 +436,7 @@ public void testDowngrade() { assertEquals(ChangeType.Downgrade, determineChangeType("26.9.0", BUILD_1, "26.7.1", BUILD_2, false)); assertEquals(ChangeType.Downgrade, determineChangeType("26.9.1", BUILD_1, "26.9.0", BUILD_2, false)); + assertEquals(ChangeType.Downgrade, determineChangeType("26.10.0", BUILD_1, "26.9.0", BUILD_2, false)); assertEquals(ChangeType.Downgrade, determineChangeType("26.9-SNAPSHOT", BUILD_1, "26.7.1", BUILD_2, false)); } @@ -474,7 +476,7 @@ public void testUnparseableVersion() @Test public void testComments() { - assertEquals("Server installed at version 26.9.0", buildComment(ChangeType.Install, null, "26.9.0")); + assertEquals("Server bootstrapped at version 26.9.0", buildComment(ChangeType.Install, null, "26.9.0")); assertEquals("Server running version 26.9.0", buildComment(ChangeType.Unknown, null, "26.9.0")); assertEquals("Server rebuilt at version 26.9.0", buildComment(ChangeType.Rebuild, "26.9.0", "26.9.0")); assertEquals("Server version changed from 26.7.1 to 26.9.0", buildComment(ChangeType.Upgrade, "26.7.1", "26.9.0")); diff --git a/api/src/org/labkey/api/util/VersionNumber.java b/api/src/org/labkey/api/util/VersionNumber.java index be5fd725d97..90f195f3a1c 100644 --- a/api/src/org/labkey/api/util/VersionNumber.java +++ b/api/src/org/labkey/api/util/VersionNumber.java @@ -226,7 +226,6 @@ public void testNonNumericRevision() assertEquals(0, new VersionNumber("8.4.4beta1").compareTo(new VersionNumber("8.4.4beta1"))); } - /** getVersionInt() used to divide the minor version in place, which would corrupt later comparisons */ @Test public void testGetVersionIntDoesNotMutate() { From 56a7cd8e43618ea8d2a9c2f1e979607c83a3effe Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 20 Aug 2026 11:45:57 -0500 Subject: [PATCH 8/8] consistent label casing between app and LKS --- .../labkey/api/audit/provider/SystemUpgradeAuditProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java index 5f4822ff1df..2372d6666e0 100644 --- a/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/SystemUpgradeAuditProvider.java @@ -93,7 +93,7 @@ public String getEventName() @Override public String getLabel() { - return "System Upgrade events"; + return "System Upgrade Events"; } @Override