diff --git a/pom.xml b/pom.xml index d8da67d..ea30cd2 100644 --- a/pom.xml +++ b/pom.xml @@ -16,6 +16,12 @@ ${tomcat.version} provided + + org.junit.jupiter + junit-jupiter + 5.10.2 + test + @@ -28,6 +34,11 @@ 1.8 + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + diff --git a/src/test/java/org/apache/catalina/core/LoadExternalPropertiesListenerTest.java b/src/test/java/org/apache/catalina/core/LoadExternalPropertiesListenerTest.java new file mode 100644 index 0000000..ef5066d --- /dev/null +++ b/src/test/java/org/apache/catalina/core/LoadExternalPropertiesListenerTest.java @@ -0,0 +1,438 @@ +/* + * 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 + * (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.apache.catalina.core; + +import org.apache.catalina.Lifecycle; +import org.apache.catalina.LifecycleEvent; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.LifecycleListener; +import org.apache.catalina.LifecycleState; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.io.PrintWriter; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +class LoadExternalPropertiesListenerTest { + + private static final String KEY_PREFIX = "lepl.test."; + + private LoadExternalPropertiesListener listener; + private final List propertiesToCleanup = new ArrayList<>(); + + @BeforeEach + void setUp() { + listener = new LoadExternalPropertiesListener(); + } + + @AfterEach + void tearDown() { + for (String key : propertiesToCleanup) { + System.clearProperty(key); + } + propertiesToCleanup.clear(); + } + + private void track(String key) { + propertiesToCleanup.add(key); + } + + private Path createPropsFile(Path dir, String filename, String... lines) throws IOException { + Path file = dir.resolve(filename); + try (PrintWriter w = new PrintWriter(file.toFile())) { + for (String line : lines) { + w.println(line); + } + } + return file; + } + + // ------------------------------------------------------------------------- + // setProperty() + // ------------------------------------------------------------------------- + + @Test + void setProperty_acceptsFileDotPrefix() { + assertTrue(listener.setProperty("file.1", "/some/path.properties")); + } + + @Test + void setProperty_rejectsNonFileDotPrefix() { + assertFalse(listener.setProperty("other.key", "/some/path.properties")); + } + + @Test + void setProperty_prefixCheckIsCaseSensitive() { + assertFalse(listener.setProperty("FILE.1", "/some/path.properties")); + } + + @Test + void setProperty_storesValueRetrievableViaGetProperty() { + listener.setProperty("file.1", "/some/path.properties"); + assertEquals("/some/path.properties", listener.getProperty("file.1")); + } + + @Test + void setProperty_returnsFalseAfter99Files() { + for (int i = 1; i <= 99; i++) { + assertTrue(listener.setProperty("file." + i, "/path/file" + i + ".properties"), + "Expected true for file." + i); + } + // The 100th call returns false to signal the limit was exceeded, but the entry + // is still stored in the backing map (put happens before the size check in the + // production code). Callers that act on the false return must be aware of this. + assertFalse(listener.setProperty("file.100", "/path/file100.properties"), + "Should return false when more than 99 files are defined"); + assertNotNull(listener.getProperty("file.100"), + "The 100th entry is stored in the map despite returning false"); + } + + // ------------------------------------------------------------------------- + // loadPropertyFile() + // ------------------------------------------------------------------------- + + @Test + void loadPropertyFile_nullFilename_doesNotThrow() { + assertDoesNotThrow(() -> listener.loadPropertyFile(null)); + assertFalse(listener.isPropertiesLoaded(), + "loadPropertyFile() must not set propertiesLoaded"); + } + + @Test + void loadPropertyFile_emptyFilename_doesNotThrow() { + assertDoesNotThrow(() -> listener.loadPropertyFile("")); + assertFalse(listener.isPropertiesLoaded(), + "loadPropertyFile() must not set propertiesLoaded"); + } + + @Test + void loadPropertyFile_nonExistentFile_doesNotThrow(@TempDir Path tempDir) { + String missing = tempDir.resolve("missing.properties").toString(); + assertDoesNotThrow(() -> listener.loadPropertyFile(missing)); + assertFalse(listener.isPropertiesLoaded(), + "loadPropertyFile() must not set propertiesLoaded"); + } + + @Test + void loadPropertyFile_loadsPropertiesIntoSystem(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "basic"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=hello"); + listener.loadPropertyFile(file.toString()); + + assertEquals("hello", System.getProperty(key)); + } + + @Test + void loadPropertyFile_overwriteTrue_replacesExistingProperty(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "overwrite.true"; + track(key); + System.setProperty(key, "original"); + + Path file = createPropsFile(tempDir, "test.properties", key + "=updated"); + listener.setOverwrite(true); + listener.loadPropertyFile(file.toString()); + + assertEquals("updated", System.getProperty(key)); + } + + @Test + void loadPropertyFile_overwriteFalse_keepsExistingProperty(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "overwrite.false.existing"; + track(key); + System.setProperty(key, "original"); + + Path file = createPropsFile(tempDir, "test.properties", key + "=updated"); + listener.setOverwrite(false); + listener.loadPropertyFile(file.toString()); + + assertEquals("original", System.getProperty(key), + "overwrite=false should not replace an existing system property"); + } + + @Test + void loadPropertyFile_overwriteFalse_setsPropertyWhenAbsent(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "overwrite.false.new"; + track(key); + System.clearProperty(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=newvalue"); + listener.setOverwrite(false); + listener.loadPropertyFile(file.toString()); + + assertEquals("newvalue", System.getProperty(key), + "overwrite=false should still set a property that does not yet exist"); + } + + @Test + void loadPropertyFile_overwriteFalse_emptyExistingPropertyIsOverwritten(@TempDir Path tempDir) + throws IOException { + String key = KEY_PREFIX + "empty.existing"; + track(key); + System.setProperty(key, ""); + + Path file = createPropsFile(tempDir, "test.properties", key + "=filled"); + listener.setOverwrite(false); + listener.loadPropertyFile(file.toString()); + + // NOTE: This documents a known quirk of the implementation. The overwrite guard checks + // !System.getProperty(name).isEmpty(), so an empty-string value is treated as "not set" + // and is overwritten even when overwrite=false. Callers who intentionally set a property + // to "" should be aware of this behaviour. + assertEquals("filled", System.getProperty(key), + "An empty existing property value is treated as absent and overwritten"); + } + + // ------------------------------------------------------------------------- + // loadProperties() + // + // NOTE: The branch that calls Digester.replaceSystemProperties() (guarded by + // system property "org.apache.tomcat.util.digester.REPLACE_SYSTEM_PROPERTIES") + // is intentionally not covered here. Testing it would require invoking a static + // method on Digester with real server-xml state, which is out of scope for unit + // tests and is better handled by integration tests. + // ------------------------------------------------------------------------- + + @Test + void loadProperties_whenNotPreviouslyLoaded_setsPropertiesLoadedTrue() { + assertFalse(listener.isPropertiesLoaded()); + listener.loadProperties(); + assertTrue(listener.isPropertiesLoaded()); + } + + @Test + void loadProperties_skipsWhenAlreadyLoaded(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "skip.if.loaded"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + listener.setPropertiesLoaded(true); + listener.loadProperties(); + + assertNull(System.getProperty(key), + "loadProperties() should be a no-op when propertiesLoaded is already true"); + } + + @Test + void loadProperties_filesLoadedInCaseInsensitiveKeyOrder(@TempDir Path tempDir) throws IOException { + // file.A < file.b (case-insensitive), so file.A loads first and file.b overwrites it + String key = KEY_PREFIX + "order"; + track(key); + + Path fileA = createPropsFile(tempDir, "a.properties", key + "=from-A"); + Path fileB = createPropsFile(tempDir, "b.properties", key + "=from-b"); + + // Intentionally register in reverse order to verify sort is independent of insertion order + listener.setProperty("file.b", fileB.toString()); + listener.setProperty("file.A", fileA.toString()); + + listener.loadProperties(); + + assertEquals("from-b", System.getProperty(key), + "file.A should load before file.b (case-insensitive sort), so file.b's value wins"); + } + + @Test + void loadProperties_skipsEmptyFilePaths(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "skip.empty.path"; + track(key); + + Path file1 = createPropsFile(tempDir, "real.properties", key + "=real-value"); + listener.setProperty("file.1", file1.toString()); + // An empty path models the "file.1 and file.3, but not file.2" scenario described in + // the source comment: loadPropertyFile() silently skips null/empty filenames. + listener.setProperty("file.2", ""); + + assertDoesNotThrow(() -> listener.loadProperties()); + assertEquals("real-value", System.getProperty(key)); + } + + // ------------------------------------------------------------------------- + // loadProperties(Boolean force) + // ------------------------------------------------------------------------- + + @Test + void loadPropertiesForce_true_reloadsEvenIfAlreadyLoaded(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "force.true"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + listener.loadProperties(); + assertTrue(listener.isPropertiesLoaded(), "First loadProperties() should set propertiesLoaded"); + System.clearProperty(key); + + listener.loadProperties(true); + + assertEquals("value", System.getProperty(key), + "force=true should reset propertiesLoaded and reload all files"); + } + + @Test + void loadPropertiesForce_false_doesNotReloadIfAlreadyLoaded(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "force.false"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + listener.loadProperties(); + System.clearProperty(key); + + listener.loadProperties(false); + + assertNull(System.getProperty(key), + "force=false should not reload when propertiesLoaded is already true"); + } + + @Test + void loadPropertiesForce_true_whenNotPreviouslyLoaded_loadsNormally(@TempDir Path tempDir) + throws IOException { + String key = KEY_PREFIX + "force.true.fresh"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + // force=true on a fresh instance where propertiesLoaded is false should behave + // identically to a plain loadProperties() call. + listener.loadProperties(true); + + assertEquals("value", System.getProperty(key), + "force=true on an unloaded instance should load properties normally"); + assertTrue(listener.isPropertiesLoaded()); + } + + // ------------------------------------------------------------------------- + // lifecycleEvent() + // ------------------------------------------------------------------------- + + @Test + void lifecycleEvent_beforeInitEvent_triggersLoad(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "lifecycle.before.init"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=lifecycle-value"); + listener.setProperty("file.1", file.toString()); + + listener.lifecycleEvent(new LifecycleEvent(new StubLifecycle(), Lifecycle.BEFORE_INIT_EVENT, null)); + + assertEquals("lifecycle-value", System.getProperty(key)); + assertTrue(listener.isPropertiesLoaded()); + } + + @Test + void lifecycleEvent_beforeInitEvent_isIdempotentWhenAlreadyLoaded(@TempDir Path tempDir) + throws IOException { + String key = KEY_PREFIX + "lifecycle.idempotent"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=original"); + listener.setProperty("file.1", file.toString()); + + StubLifecycle stub = new StubLifecycle(); + listener.lifecycleEvent(new LifecycleEvent(stub, Lifecycle.BEFORE_INIT_EVENT, null)); + assertEquals("original", System.getProperty(key)); + + // Remove the property and fire the event a second time; the listener must not reload. + System.clearProperty(key); + listener.lifecycleEvent(new LifecycleEvent(stub, Lifecycle.BEFORE_INIT_EVENT, null)); + + assertNull(System.getProperty(key), + "A second BEFORE_INIT_EVENT should be a no-op once propertiesLoaded is true"); + } + + @Test + void lifecycleEvent_otherEvents_doNotTriggerLoad(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "lifecycle.other"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + StubLifecycle stub = new StubLifecycle(); + listener.lifecycleEvent(new LifecycleEvent(stub, Lifecycle.AFTER_START_EVENT, null)); + listener.lifecycleEvent(new LifecycleEvent(stub, Lifecycle.STOP_EVENT, null)); + listener.lifecycleEvent(new LifecycleEvent(stub, Lifecycle.BEFORE_START_EVENT, null)); + + assertFalse(listener.isPropertiesLoaded(), + "Only BEFORE_INIT_EVENT should trigger loadProperties()"); + assertNull(System.getProperty(key)); + } + + // ------------------------------------------------------------------------- + // endSetPropertiesRule() + // ------------------------------------------------------------------------- + + @Test + void endSetPropertiesRule_loadFirstFalse_doesNotTriggerLoad(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "endset.loadfirst.false"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + listener.setLoadFirst(false); + listener.endSetPropertiesRule(); + + assertFalse(listener.isPropertiesLoaded()); + assertNull(System.getProperty(key)); + } + + @Test + void endSetPropertiesRule_loadFirstTrue_triggersLoad(@TempDir Path tempDir) throws IOException { + String key = KEY_PREFIX + "endset.loadfirst.true"; + track(key); + + Path file = createPropsFile(tempDir, "test.properties", key + "=value"); + listener.setProperty("file.1", file.toString()); + + listener.setLoadFirst(true); + listener.endSetPropertiesRule(); + + assertEquals("value", System.getProperty(key)); + assertTrue(listener.isPropertiesLoaded()); + } + + // ------------------------------------------------------------------------- + // Helper: minimal Lifecycle stub for constructing LifecycleEvent instances + // ------------------------------------------------------------------------- + + private static class StubLifecycle implements Lifecycle { + @Override public void addLifecycleListener(LifecycleListener listener) {} + @Override public LifecycleListener[] findLifecycleListeners() { return new LifecycleListener[0]; } + @Override public void removeLifecycleListener(LifecycleListener listener) {} + @Override public void init() throws LifecycleException {} + @Override public void start() throws LifecycleException {} + @Override public void stop() throws LifecycleException {} + @Override public void destroy() throws LifecycleException {} + @Override public LifecycleState getState() { return LifecycleState.NEW; } + @Override public String getStateName() { return LifecycleState.NEW.name(); } + } +}