From b6b1a7ad8a1d8e64cf59cf7cb368445420516b4d Mon Sep 17 00:00:00 2001 From: Szabolcs Hubai Date: Wed, 28 Jun 2023 01:20:42 +0200 Subject: [PATCH 1/2] Improve log4j.properties file loading - add failing test In Play! 1.5.x documentation [1] the property "application.log.path" description is the following: Path to a Log4J configuration file, to customise log output. If you do not specify a path, Play will load a log4j.properties file in the conf directory if present. The same can be found in the annotated application.conf: "If you want a very customized log, create a log4j.properties file in the conf directory" I'm unsure if it worked at all in Play!, but it's a nice feature. The init_with_conf_dir() test tries to check the parameters of the newly added "conf/log4j.properties". The init_with_default_config() test deletes that properties file from the test classpath and checks for the Framework's properties file. [1]: https://www.playframework.com/documentation/1.5.x/configuration#application.log.path --- framework/test/conf/log4j.properties | 9 +++++++++ framework/test/play/PlayLoggingSetupTest.java | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 framework/test/conf/log4j.properties diff --git a/framework/test/conf/log4j.properties b/framework/test/conf/log4j.properties new file mode 100644 index 00000000..4b862743 --- /dev/null +++ b/framework/test/conf/log4j.properties @@ -0,0 +1,9 @@ +log4j.rootLogger=DEBUG, PropertiesConsoleAppender + +log4j.appender.PropertiesConsoleAppender=org.apache.log4j.ConsoleAppender +log4j.appender.PropertiesConsoleAppender.layout=org.apache.log4j.PatternLayout +log4j.appender.PropertiesConsoleAppender.layout.ConversionPattern=%d{ISO8601} %-5p [%t] %c: %m%n + + + +log4j.logger.logtest.confdir=ERROR diff --git a/framework/test/play/PlayLoggingSetupTest.java b/framework/test/play/PlayLoggingSetupTest.java index 301b0c46..31e89289 100755 --- a/framework/test/play/PlayLoggingSetupTest.java +++ b/framework/test/play/PlayLoggingSetupTest.java @@ -6,8 +6,10 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import play.libs.Files; import java.io.File; +import java.net.URISyntaxException; import java.util.Properties; import static org.assertj.core.api.Assertions.assertThat; @@ -57,4 +59,21 @@ public void init_with_xml() { Logger log4jLogger = Logger.getLogger("logtest.xml"); assertThat(log4jLogger.getLevel()).isEqualTo(Level.ERROR); } + + @Test + public void init_with_conf_dir() { + loggingSetup.init(); + Logger log4jLogger = Logger.getLogger("logtest.confdir"); + assertThat(log4jLogger.getLevel()).isEqualTo(Level.ERROR); + assertThat(Logger.getRootLogger().getLevel()).isEqualTo(Level.DEBUG); + } + + @Test + public void init_with_default_config() { + // delete the conf/log4j.properties from test classpath and fallback to the framework log4j.properties + File confProp = new File(getClass().getResource("/conf/log4j.properties").getFile()); + assertThat(confProp.delete()).isTrue(); + loggingSetup.init(); + assertThat(Logger.getRootLogger().getLevel()).isEqualTo(Level.INFO); + } } From 214541a20079cc7b9cad8715da35b701be33665c Mon Sep 17 00:00:00 2001 From: Szabolcs Hubai Date: Wed, 28 Jun 2023 01:26:30 +0200 Subject: [PATCH 2/2] Improve log4j.properties file loading - load from conf/ automatically In Play! 1.5.x documentation [1] the property "application.log.path" description is the following: Path to a Log4J configuration file, to customise log output. If you do not specify a path, Play will load a log4j.properties file in the conf directory if present. The same can be found in the annotated application.conf: "If you want a very customized log, create a log4j.properties file in the conf directory" I'm unsure if it worked at all in Play!, but it's a nice feature. [1]: https://www.playframework.com/documentation/1.5.x/configuration#application.log.path --- framework/src/play/PlayLoggingSetup.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/framework/src/play/PlayLoggingSetup.java b/framework/src/play/PlayLoggingSetup.java index 03d59331..33d58de1 100644 --- a/framework/src/play/PlayLoggingSetup.java +++ b/framework/src/play/PlayLoggingSetup.java @@ -12,7 +12,13 @@ public void init() { String log4jPath = Play.configuration.getProperty("application.log.path", "/log4j.xml"); URL log4jConf = PlayLoggingSetup.class.getResource(log4jPath); if (log4jConf == null) { - log4jPath = Play.configuration.getProperty("application.log.path", "/log4j.properties"); + // try log4j.properties from the "conf/" directory/classpath + log4jPath = "/conf/log4j.properties"; + log4jConf = PlayLoggingSetup.class.getResource(log4jPath); + } + if (log4jConf == null) { + // falling back to "/log4j.properties" + log4jPath = "/log4j.properties"; log4jConf = PlayLoggingSetup.class.getResource(log4jPath); } if (log4jConf == null) {