This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch restore_rfa_properties in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit e57784d92b2f6b0844eb94d0b00d89532eb7cb6b Author: Stephen Webb <[email protected]> AuthorDate: Sun Oct 30 13:18:36 2022 +1100 Restore documented RollingFileAppender configuration properties file MaxBackupIndex and MaxFileSize options --- src/main/cpp/fixedwindowrollingpolicy.cpp | 8 +- src/main/cpp/propertyconfigurator.cpp | 2 - src/main/cpp/rollingfileappender.cpp | 80 +++++++++++++++- .../include/log4cxx/rolling/rollingfileappender.h | 34 ++++++- .../rolling/rollingfileappenderpropertiestest.cpp | 105 ++++++++++++++++++++- .../input/rolling/obsoleteRFA1.properties | 1 + 6 files changed, 213 insertions(+), 17 deletions(-) diff --git a/src/main/cpp/fixedwindowrollingpolicy.cpp b/src/main/cpp/fixedwindowrollingpolicy.cpp index 9e992834..8534c5df 100644 --- a/src/main/cpp/fixedwindowrollingpolicy.cpp +++ b/src/main/cpp/fixedwindowrollingpolicy.cpp @@ -14,11 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#if defined(_MSC_VER) - #pragma warning ( disable: 4231 4251 4275 4786 ) -#endif - - #include <log4cxx/logstring.h> #include <log4cxx/rolling/fixedwindowrollingpolicy.h> #include <log4cxx/helpers/pool.h> @@ -45,7 +40,8 @@ struct FixedWindowRollingPolicy::FixedWindowRollingPolicyPrivate : public Rollin FixedWindowRollingPolicyPrivate() : RollingPolicyBasePrivate(), minIndex(1), - maxIndex(7) + maxIndex(7), + explicitActiveFile(false) {} int minIndex; diff --git a/src/main/cpp/propertyconfigurator.cpp b/src/main/cpp/propertyconfigurator.cpp index f688d931..f0293a2b 100644 --- a/src/main/cpp/propertyconfigurator.cpp +++ b/src/main/cpp/propertyconfigurator.cpp @@ -473,7 +473,6 @@ AppenderPtr PropertyConfigurator::parseAppender( LogLog::debug((LogString) LOG4CXX_STR("Parsing layout options for \"") + appenderName + LOG4CXX_STR("\".")); - //configureOptionHandler(layout, layoutPrefix + ".", props); PropertySetter::setProperties(layout, props, layoutPrefix + LOG4CXX_STR("."), p); LogLog::debug((LogString) LOG4CXX_STR("End of parsing for \"") + appenderName + LOG4CXX_STR("\".")); @@ -512,7 +511,6 @@ AppenderPtr PropertyConfigurator::parseAppender( } } - //configureOptionHandler((OptionHandler) appender, prefix + _T("."), props); PropertySetter::setProperties(appender, props, prefix + LOG4CXX_STR("."), p); LogLog::debug((LogString) LOG4CXX_STR("Parsed \"") + appenderName + LOG4CXX_STR("\" options.")); diff --git a/src/main/cpp/rollingfileappender.cpp b/src/main/cpp/rollingfileappender.cpp index a24a8e49..96c79c88 100644 --- a/src/main/cpp/rollingfileappender.cpp +++ b/src/main/cpp/rollingfileappender.cpp @@ -24,8 +24,10 @@ #include <log4cxx/rolling/rolloverdescription.h> #include <log4cxx/helpers/fileoutputstream.h> #include <log4cxx/helpers/bytebuffer.h> +#include <log4cxx/helpers/optionconverter.h> +#include <log4cxx/helpers/stringhelper.h> #include <log4cxx/rolling/fixedwindowrollingpolicy.h> -#include <log4cxx/rolling/manualtriggeringpolicy.h> +#include <log4cxx/rolling/sizebasedtriggeringpolicy.h> #include <log4cxx/helpers/transcoder.h> #include <log4cxx/private/fileappender_priv.h> #include <mutex> @@ -75,12 +77,80 @@ RollingFileAppender::RollingFileAppender() : { } +void RollingFileAppender::setOption(const LogString& option, const LogString& value) +{ + if (StringHelper::equalsIgnoreCase(option, + LOG4CXX_STR("MAXFILESIZE"), LOG4CXX_STR("maxfilesize")) + || StringHelper::equalsIgnoreCase(option, + LOG4CXX_STR("MAXIMUMFILESIZE"), LOG4CXX_STR("maximumfilesize"))) + { + setMaxFileSize(value); + } + else if (StringHelper::equalsIgnoreCase(option, + LOG4CXX_STR("MAXBACKUPINDEX"), LOG4CXX_STR("maxbackupindex")) + || StringHelper::equalsIgnoreCase(option, + LOG4CXX_STR("MAXIMUMBACKUPINDEX"), LOG4CXX_STR("maximumbackupindex"))) + { + setMaxBackupIndex(StringHelper::toInt(value)); + } + else + { + FileAppender::setOption(option, value); + } +} + +int RollingFileAppender::getMaxBackupIndex() const +{ + int result = 1; + if (auto fwrp = log4cxx::cast<FixedWindowRollingPolicy>(_priv->rollingPolicy)) + result = fwrp->getMaxIndex(); + return result; +} + +void RollingFileAppender::setMaxBackupIndex(int maxBackups) +{ + if (!_priv->rollingPolicy) + { + auto fwrp = std::make_shared<FixedWindowRollingPolicy>(); + fwrp->setFileNamePattern(getFile() + LOG4CXX_STR(".%i")); + fwrp->setMaxIndex(maxBackups); + _priv->rollingPolicy = fwrp; + } + else if (auto fwrp = log4cxx::cast<FixedWindowRollingPolicy>(_priv->rollingPolicy)) + fwrp->setMaxIndex(maxBackups); +} + +size_t RollingFileAppender::getMaximumFileSize() const +{ + size_t result = 10 * 1024 * 1024; + if (auto sbtp = log4cxx::cast<SizeBasedTriggeringPolicy>(_priv->triggeringPolicy)) + result = sbtp->getMaxFileSize(); + return result; +} + +void RollingFileAppender::setMaximumFileSize(size_t maxFileSize) +{ + if (!_priv->triggeringPolicy) + { + auto sbtp = std::make_shared<SizeBasedTriggeringPolicy>(); + sbtp->setMaxFileSize(maxFileSize); + _priv->triggeringPolicy = sbtp; + } + else if (auto sbtp = log4cxx::cast<SizeBasedTriggeringPolicy>(_priv->triggeringPolicy)) + sbtp->setMaxFileSize(maxFileSize); +} + +void RollingFileAppender::setMaxFileSize(const LogString& value) +{ + setMaximumFileSize(OptionConverter::toFileSize(value, long(getMaximumFileSize() + 1))); +} + /** * Prepare instance of use. */ void RollingFileAppender::activateOptions(Pool& p) { - if (_priv->rollingPolicy == NULL) + if (!_priv->rollingPolicy) { auto fwrp = std::make_shared<FixedWindowRollingPolicy>(); fwrp->setFileNamePattern(getFile() + LOG4CXX_STR(".%i")); @@ -90,7 +160,7 @@ void RollingFileAppender::activateOptions(Pool& p) // // if no explicit triggering policy and rolling policy is both. // - if (_priv->triggeringPolicy == NULL) + if (!_priv->triggeringPolicy) { TriggeringPolicyPtr trig = log4cxx::cast<TriggeringPolicy>(_priv->rollingPolicy); @@ -100,9 +170,9 @@ void RollingFileAppender::activateOptions(Pool& p) } } - if (_priv->triggeringPolicy == NULL) + if (!_priv->triggeringPolicy) { - _priv->triggeringPolicy = std::make_shared<ManualTriggeringPolicy>(); + _priv->triggeringPolicy = std::make_shared<SizeBasedTriggeringPolicy>(); } { diff --git a/src/main/include/log4cxx/rolling/rollingfileappender.h b/src/main/include/log4cxx/rolling/rollingfileappender.h index 603dffe4..31af81a3 100644 --- a/src/main/include/log4cxx/rolling/rollingfileappender.h +++ b/src/main/include/log4cxx/rolling/rollingfileappender.h @@ -89,8 +89,40 @@ class LOG4CXX_EXPORT RollingFileAppender : public FileAppender public: RollingFileAppender(); - void activateOptions(log4cxx::helpers::Pool&); + /** Returns the value of the <b>MaxBackupIndex</b> option. */ + int getMaxBackupIndex() const; + /** Get the maximum size that the output file is allowed to reach before being rolled over to backup files. */ + size_t getMaximumFileSize() const; + + + /** + Set the maximum number of backup files to keep around. + + <p>The <b>MaxBackupIndex</b> option determines how many backup + files are kept before the oldest is erased. This option takes + a positive integer value. If set to zero, then there will be no + backup files and the log file will be truncated when it reaches <code>MaxFileSize</code>. + */ + void setMaxBackupIndex( int maxBackupIndex ); + + /** + Set the maximum size that the output file is allowed to reach before being rolled over to backup files. + + <p>In configuration files, the <b>MaxFileSize</b> option takes an + long integer in the range 0 - 2^63. You can specify the value with the suffixes "KB", "MB" or "GB" so that the integer is + interpreted being expressed respectively in kilobytes, megabytes + or gigabytes. For example, the value "10KB" will be interpreted as 10240. + */ + void setMaxFileSize( const LogString& value ); + + void setMaximumFileSize( size_t value ); + + + void setOption( const LogString& option, const LogString& value ) override; + + /** Prepares RollingFileAppender for use. */ + void activateOptions( log4cxx::helpers::Pool& pool ); /** Implements the usual roll over behaviour. diff --git a/src/test/cpp/rolling/rollingfileappenderpropertiestest.cpp b/src/test/cpp/rolling/rollingfileappenderpropertiestest.cpp index 3240f9f7..fe4729c6 100644 --- a/src/test/cpp/rolling/rollingfileappenderpropertiestest.cpp +++ b/src/test/cpp/rolling/rollingfileappenderpropertiestest.cpp @@ -46,6 +46,9 @@ using namespace log4cxx::rolling; LOGUNIT_CLASS(RollingFileAppenderPropertiesTest) { LOGUNIT_TEST_SUITE(RollingFileAppenderPropertiesTest); + LOGUNIT_TEST(test1); + LOGUNIT_TEST(test2); + LOGUNIT_TEST(testIsOptionHandler); LOGUNIT_TEST(testRollingFromProperties); LOGUNIT_TEST_SUITE_END(); @@ -56,6 +59,102 @@ public: void tearDown() { + LogManager::shutdown(); + } + + /** + * Test basic rolling functionality. + */ + void test1() + { + PropertyConfigurator::configure(File("input/rolling/obsoleteRFA1.properties")); + + // Make sure that the configured values are what we expect + auto appender = LogManager::getRootLogger()->getAppender(LOG4CXX_STR("testAppender")); + LOGUNIT_ASSERT(appender); + + auto rfa = cast<RollingFileAppender>(appender); + LOGUNIT_ASSERT(rfa); + + LOGUNIT_ASSERT_EQUAL(3, rfa->getMaxBackupIndex()); + LOGUNIT_ASSERT_EQUAL(100, static_cast<int>(rfa->getMaximumFileSize())); + + logchar msg[] = { 'H', 'e', 'l', 'l', 'o', '-', '-', '-', '?', 0}; + auto logger = Logger::getLogger("RollingFileAppenderTest"); + + // Write exactly 10 bytes with each log + for (int i = 0; i < 25; i++) + { + apr_sleep(100000); + + if (i < 10) + { + msg[8] = (logchar) ('0' + i); + LOG4CXX_DEBUG(logger, msg); + } + else if (i < 100) + { + msg[7] = (logchar) ('0' + i / 10); + msg[8] = (logchar) ('0' + i % 10); + LOG4CXX_DEBUG(logger, msg); + } + } + + Pool p; + LOGUNIT_ASSERT_EQUAL(true, File("output/obsoleteRFA-test1.log").exists(p)); + LOGUNIT_ASSERT_EQUAL(true, File("output/obsoleteRFA-test1.log.1").exists(p)); + } + + /** + * Test basic rolling functionality. + */ + void test2() + { + auto layout = std::make_shared <PatternLayout>(LOG4CXX_STR("%m\n")); + auto rfa = std::make_shared<RollingFileAppender>(); + rfa->setName(LOG4CXX_STR("ROLLING")); + rfa->setLayout(layout); + rfa->setOption(LOG4CXX_STR("append"), LOG4CXX_STR("false")); + rfa->setMaximumFileSize(100); + rfa->setFile(LOG4CXX_STR("output/obsoleteRFA-test2.log")); + Pool p; + rfa->activateOptions(p); + auto root = Logger::getRootLogger(); + root->addAppender(rfa); + + logchar msg[] = { 'H', 'e', 'l', 'l', 'o', '-', '-', '-', '?', 0}; + auto logger = Logger::getLogger("org.apache.logj4.ObsoleteRollingFileAppenderTest"); + + // Write exactly 10 bytes with each log + for (int i = 0; i < 25; i++) + { + apr_sleep(100000); + + if (i < 10) + { + msg[8] = (logchar) ('0' + i); + LOG4CXX_DEBUG(logger, msg); + } + else if (i < 100) + { + msg[7] = (logchar) ('0' + i / 10); + msg[8] = (logchar) ('0' + i % 10); + LOG4CXX_DEBUG(logger, msg); + } + } + + LOGUNIT_ASSERT_EQUAL(true, File("output/obsoleteRFA-test2.log").exists(p)); + LOGUNIT_ASSERT_EQUAL(true, File("output/obsoleteRFA-test2.log.1").exists(p)); + } + + /** + * Tests if class is declared to support the OptionHandler interface. + * See LOGCXX-136. + */ + void testIsOptionHandler() + { + auto rfa = std::make_shared<RollingFileAppender>(); + LOGUNIT_ASSERT_EQUAL(true, rfa->instanceof(log4cxx::spi::OptionHandler::getStaticClass())); } void testRollingFromProperties(){ @@ -63,16 +162,16 @@ public: // what we expect PropertyConfigurator::configure(LOG4CXX_FILE("input/rolling/rollingFileAppenderFromProperties.properties")); - AppenderPtr appender = LogManager::getRootLogger()->getAppender(LOG4CXX_STR("FILE")); + auto appender = LogManager::getRootLogger()->getAppender(LOG4CXX_STR("FILE")); LOGUNIT_ASSERT(appender); - RollingFileAppenderPtr rfa = cast<RollingFileAppender>(appender); + auto rfa = cast<RollingFileAppender>(appender); LOGUNIT_ASSERT(rfa); FixedWindowRollingPolicyPtr fixedWindowRolling = cast<FixedWindowRollingPolicy>(rfa->getRollingPolicy()); LOGUNIT_ASSERT(fixedWindowRolling); - SizeBasedTriggeringPolicyPtr sizeBasedPolicy = cast<SizeBasedTriggeringPolicy>(rfa->getTriggeringPolicy()); + auto sizeBasedPolicy = cast<SizeBasedTriggeringPolicy>(rfa->getTriggeringPolicy()); LOGUNIT_ASSERT(sizeBasedPolicy); LOGUNIT_ASSERT_EQUAL(3, fixedWindowRolling->getMaxIndex()); diff --git a/src/test/resources/input/rolling/obsoleteRFA1.properties b/src/test/resources/input/rolling/obsoleteRFA1.properties index 1255605c..6fcc8dbe 100644 --- a/src/test/resources/input/rolling/obsoleteRFA1.properties +++ b/src/test/resources/input/rolling/obsoleteRFA1.properties @@ -20,6 +20,7 @@ log4j.appender.testAppender.Append=false log4j.appender.testAppender.layout=org.apache.log4j.PatternLayout log4j.appender.testAppender.layout.ConversionPattern=%m\n log4j.appender.testAppender.maxFileSize=100 +log4j.appender.testAppender.maxBackupIndex=3 # Prevent internal log4j DEBUG messages from polluting the output. log4j.logger.org.apache.log4j.PropertyConfigurator=INFO
