You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by sw...@apache.org on 2022/12/18 05:22:07 UTC

[logging-log4cxx] 01/01: Prevent spurious warnings when confguring RollingFileAppender using MaxBackupIndex and MaxFileSize options

This is an automated email from the ASF dual-hosted git repository.

swebb2066 pushed a commit to branch prevent_spurious_warnings
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git

commit 478e06ee735879a0204729c2ed2d5bb78a34fad2
Author: Stephen Webb <st...@sabreautonomous.com.au>
AuthorDate: Sun Dec 18 16:20:52 2022 +1100

    Prevent spurious warnings when confguring RollingFileAppender using MaxBackupIndex and MaxFileSize options
---
 src/main/cpp/propertyconfigurator.cpp              | 65 +++++++++++++---------
 src/main/cpp/rollingfileappender.cpp               | 28 +++++++---
 .../include/log4cxx/rolling/rollingfileappender.h  | 27 +++++++--
 3 files changed, 83 insertions(+), 37 deletions(-)

diff --git a/src/main/cpp/propertyconfigurator.cpp b/src/main/cpp/propertyconfigurator.cpp
index ea5c340a..2406608e 100644
--- a/src/main/cpp/propertyconfigurator.cpp
+++ b/src/main/cpp/propertyconfigurator.cpp
@@ -433,8 +433,6 @@ AppenderPtr PropertyConfigurator::parseAppender(
 	// Appender was not previously initialized.
 	LogString prefix = APPENDER_PREFIX + appenderName;
 	LogString layoutPrefix = prefix + LOG4CXX_STR(".layout");
-	LogString rollingPolicyKey = prefix + LOG4CXX_STR(".rollingPolicy");
-	LogString triggeringPolicyKey = prefix + LOG4CXX_STR(".triggeringPolicy");
 
 	std::shared_ptr<Object> obj =
 		OptionConverter::instantiateByKey(
@@ -485,38 +483,55 @@ AppenderPtr PropertyConfigurator::parseAppender(
 		}
 
 		RollingFileAppenderPtr rolling = log4cxx::cast<rolling::RollingFileAppender>(appender);
-		if(rolling)
+		if (rolling)
 		{
-			RollingPolicyPtr rollingPolicy;
-			std::shared_ptr<Object> rolling_obj =
-				OptionConverter::instantiateByKey(
-					props, rollingPolicyKey, RollingPolicy::getStaticClass(), 0);
-			rollingPolicy = log4cxx::cast<RollingPolicy>( rolling_obj );
-			if(rollingPolicy)
+			LogString rollingPolicyKey = prefix + LOG4CXX_STR(".rollingPolicy");
+			if (!OptionConverter::findAndSubst(rollingPolicyKey, props).empty())
 			{
-				rolling->setRollingPolicy(rollingPolicy);
-
-				LogLog::debug((LogString) LOG4CXX_STR("Parsing rolling policy options for \"")
-					+ appenderName + LOG4CXX_STR("\"."));
-				PropertySetter::setProperties(rollingPolicy, props, rollingPolicyKey + LOG4CXX_STR("."), p);
+				RollingPolicyPtr rollingPolicy;
+				std::shared_ptr<Object> rolling_obj =
+					OptionConverter::instantiateByKey(
+						props, rollingPolicyKey, RollingPolicy::getStaticClass(), 0);
+				rollingPolicy = log4cxx::cast<RollingPolicy>( rolling_obj );
+				if(rollingPolicy)
+				{
+					rolling->setRollingPolicy(rollingPolicy);
+
+					LogLog::debug((LogString) LOG4CXX_STR("Parsing rolling policy options for \"")
+						+ appenderName + LOG4CXX_STR("\"."));
+					PropertySetter::setProperties(rollingPolicy, props, rollingPolicyKey + LOG4CXX_STR("."), p);
+				}
 			}
 
-			TriggeringPolicyPtr triggeringPolicy;
-			std::shared_ptr<Object> triggering_obj =
-				OptionConverter::instantiateByKey(
-					props, triggeringPolicyKey, TriggeringPolicy::getStaticClass(), 0);
-			triggeringPolicy = log4cxx::cast<TriggeringPolicy>( triggering_obj );
-			if(triggeringPolicy)
+			LogString triggeringPolicyKey = prefix + LOG4CXX_STR(".triggeringPolicy");
+			if (!OptionConverter::findAndSubst(triggeringPolicyKey, props).empty())
 			{
-				rolling->setTriggeringPolicy(triggeringPolicy);
-
-				LogLog::debug((LogString) LOG4CXX_STR("Parsing triggering policy options for \"")
-					+ appenderName + LOG4CXX_STR("\"."));
-				PropertySetter::setProperties(triggeringPolicy, props, triggeringPolicyKey + LOG4CXX_STR("."), p);
+				TriggeringPolicyPtr triggeringPolicy;
+				std::shared_ptr<Object> triggering_obj =
+					OptionConverter::instantiateByKey(
+						props, triggeringPolicyKey, TriggeringPolicy::getStaticClass(), 0);
+				triggeringPolicy = log4cxx::cast<TriggeringPolicy>( triggering_obj );
+				if(triggeringPolicy)
+				{
+					rolling->setTriggeringPolicy(triggeringPolicy);
+
+					LogLog::debug((LogString) LOG4CXX_STR("Parsing triggering policy options for \"")
+						+ appenderName + LOG4CXX_STR("\"."));
+					PropertySetter::setProperties(triggeringPolicy, props, triggeringPolicyKey + LOG4CXX_STR("."), p);
+				}
 			}
 		}
 
 		PropertySetter::setProperties(appender, props, prefix + LOG4CXX_STR("."), p);
+		if (!rolling)
+			;
+		else if (!rolling->hasRollingPolicy())
+			LogLog::warn((LogString) LOG4CXX_STR("Missing rolling policy for \"")
+				+ appenderName + LOG4CXX_STR("\"."));
+		else if (!rolling->hasTriggeringPolicy())
+			LogLog::warn((LogString) LOG4CXX_STR("Missing triggering policy for \"")
+				+ appenderName + LOG4CXX_STR("\"."));
+
 		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 fa543fcb..dff03fb9 100644
--- a/src/main/cpp/rollingfileappender.cpp
+++ b/src/main/cpp/rollingfileappender.cpp
@@ -482,8 +482,15 @@ void RollingFileAppender::subAppend(const LoggingEventPtr& event, Pool& p)
 }
 
 /**
- * Get rolling policy.
- * @return rolling policy.
+ Does this have a scheme for rolling over log files?
+*/
+bool RollingFileAppender::hasRollingPolicy() const
+{
+	return !!_priv->rollingPolicy;
+}
+
+/**
+ * TThe policy that implements the scheme for rolling over a log file.
  */
 RollingPolicyPtr RollingFileAppender::getRollingPolicy() const
 {
@@ -491,8 +498,15 @@ RollingPolicyPtr RollingFileAppender::getRollingPolicy() const
 }
 
 /**
- * Get triggering policy.
- * @return triggering policy.
+ Does this have a basis for triggering log file rollover?
+*/
+bool RollingFileAppender::hasTriggeringPolicy() const
+{
+	return !!_priv->triggeringPolicy;
+}
+
+/**
+ * The policy that determine when to trigger a log file rollover.
  */
 TriggeringPolicyPtr RollingFileAppender::getTriggeringPolicy() const
 {
@@ -500,8 +514,7 @@ TriggeringPolicyPtr RollingFileAppender::getTriggeringPolicy() const
 }
 
 /**
- * Sets the rolling policy.
- * @param policy rolling policy.
+ * Set the scheme for rolling over log files.
  */
 void RollingFileAppender::setRollingPolicy(const RollingPolicyPtr& policy)
 {
@@ -509,8 +522,7 @@ void RollingFileAppender::setRollingPolicy(const RollingPolicyPtr& policy)
 }
 
 /**
- * Set triggering policy.
- * @param policy triggering policy.
+ * Set policy that determine when to trigger a log file rollover.
  */
 void RollingFileAppender::setTriggeringPolicy(const TriggeringPolicyPtr& policy)
 {
diff --git a/src/main/include/log4cxx/rolling/rollingfileappender.h b/src/main/include/log4cxx/rolling/rollingfileappender.h
index b44f7b8d..bb582089 100644
--- a/src/main/include/log4cxx/rolling/rollingfileappender.h
+++ b/src/main/include/log4cxx/rolling/rollingfileappender.h
@@ -158,19 +158,38 @@ class LOG4CXX_EXPORT RollingFileAppender : public FileAppender
 		bool rolloverInternal(log4cxx::helpers::Pool& p);
 
 	public:
+		/**
+		 Does this have a scheme for rolling over log files?
+		*/
+		bool hasRollingPolicy() const;
+
+		/**
+		 Does this have a basis for triggering log file rollover?
+		*/
+		bool hasTriggeringPolicy() const;
 
+		/**
+		 * The policy that implements the scheme for rolling over a log file.
+		 */
 		RollingPolicyPtr getRollingPolicy() const;
 
+		/**
+		 * The policy that determine when to trigger a log file rollover.
+		 */
 		TriggeringPolicyPtr getTriggeringPolicy() const;
 
 		/**
-		 * Sets the rolling policy. In case the 'policy' argument also implements
-		 * {@link TriggeringPolicy}, then the triggering policy for this appender
-		 * is automatically set to be the policy argument.
-		 * @param policy
+		 * Use \c policy as the scheme for rolling over log files.
+		 *
+		 * Where the \c policy also implements
+		 * {@link TriggeringPolicy}, then \c policy
+		 * will be used to determine when to trigger a log file rollover.
 		 */
 		void setRollingPolicy(const RollingPolicyPtr& policy);
 
+		/**
+		 * Use \c policy to determine when to trigger a log file rollover.
+		 */
 		void setTriggeringPolicy(const TriggeringPolicyPtr& policy);
 
 	public: