(logging-log4cxx) branch simplify_property_configurator_interface created (now 99344ba2)

[sw...@apache.org]

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

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


      at 99344ba2 Simplify PropertyConfigurator interface

This branch includes the following new commits:

     new 99344ba2 Simplify PropertyConfigurator interface

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(logging-log4cxx) 01/01: Simplify PropertyConfigurator interface

[sw...@apache.org]

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

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

commit 99344ba2333c8e126fc5a6e42d9421d8501226f8
Author: Stephen Webb <sw...@gmail.com>
AuthorDate: Wed Sep 30 14:08:12 2026 +1000

    Simplify PropertyConfigurator interface
---
 src/main/cpp/domconfigurator.cpp                |  14 +--
 src/main/cpp/propertyconfigurator.cpp           | 110 +++++++++++++++++-------
 src/main/include/log4cxx/propertyconfigurator.h |  76 +++-------------
 3 files changed, 100 insertions(+), 100 deletions(-)

diff --git a/src/main/cpp/domconfigurator.cpp b/src/main/cpp/domconfigurator.cpp
index 2ef81d07..b5a2116d 100644
--- a/src/main/cpp/domconfigurator.cpp
+++ b/src/main/cpp/domconfigurator.cpp
@@ -969,16 +969,20 @@ spi::ConfigurationStatus DOMConfigurator::doConfigure
 		}
 	}
 
-	if (!m_priv->appenderAdded)
+	auto result = spi::ConfigurationStatus::NotConfigured;
+	if (m_priv->appenderAdded)
+	{
+		result = spi::ConfigurationStatus::Configured;
+		m_priv->repository->setConfigured(true);
+	}
+	else
 	{
 		LogLog::warn(LOG4CXX_STR("[") + filename.getPath()
 			+ LOG4CXX_STR("] did not add an ") + Appender::getStaticClass().getName()
 			+ LOG4CXX_STR(" to a logger"));
-		return spi::ConfigurationStatus::NotConfigured;
 	}
-
-	m_priv->repository->setConfigured(true);
-	return spi::ConfigurationStatus::Configured;
+	m_priv.reset();
+	return result;
 }
 
 // Read configuration options from \c filename.
diff --git a/src/main/cpp/propertyconfigurator.cpp b/src/main/cpp/propertyconfigurator.cpp
index 7f50f9c6..1c1ac5bc 100644
--- a/src/main/cpp/propertyconfigurator.cpp
+++ b/src/main/cpp/propertyconfigurator.cpp
@@ -53,6 +53,11 @@ using RegistryPtr = std::unique_ptr<RegistryType>;
 
 struct PropertyConfigurator::PrivateData
 {
+public: // Attributes
+	/**
+	The target of configuration directives
+	*/
+	LoggerRepositoryPtr pRepository;
 
 	/**
 	Used internally to keep track of configured appenders.
@@ -68,10 +73,53 @@ struct PropertyConfigurator::PrivateData
 	True if an appender was added to a logger
 	*/
 	bool appenderAdded{ false };
+
+public: // ...structors
+	PrivateData(const LoggerRepositoryPtr& target)
+		: pRepository(target)
+		{}
+
+public: // Methods
+	/**
+	Check the provided <code>Properties</code> object for a LoggerFactory
+	entry specified by *log4j.loggerFactory*.  If such an entry
+	exists, an attempt is made to create an instance using the default
+	constructor.  This instance is used for subsequent Logger creations
+	within this configurator.
+	@see #parseCatsAndRenderers
+	*/
+	void configureLoggerFactory(helpers::Properties& props);
+
+	void configureRootLogger(helpers::Properties& props);
+
+	/**
+	Parse non-root elements, such non-root categories and renderers.
+	*/
+	void parseCatsAndRenderers(helpers::Properties& props);
+
+	/**
+	Parse the additivity option for a non-root logger.
+	*/
+	bool parseAdditivityForLogger(helpers::Properties& props,
+		LoggerPtr& cat, const LogString& loggerName);
+
+	/**
+	This method must work for the root logger as well.
+	*/
+	void parseLogger(
+		helpers::Properties& props, LoggerPtr& logger,
+		const LogString& optionKey, const LogString& loggerName,
+		const LogString& value, bool additivity);
+
+	AppenderPtr parseAppender(
+		helpers::Properties& props, const LogString& appenderName);
+
+	void registryPut(const AppenderPtr& appender);
+	AppenderPtr registryGet(const LogString& name);
+
 };
 
 PropertyConfigurator::PropertyConfigurator()
-	: m_priv{ std::make_unique<PrivateData>() }
 {
 }
 
@@ -106,7 +154,7 @@ spi::ConfigurationStatus PropertyConfigurator::doConfigure
 	try
 	{
 		result = doConfigure(props, repository ? repository : LogManager::getLoggerRepository());
-		if (!m_priv->appenderAdded)
+		if (spi::ConfigurationStatus::NotConfigured == result)
 		{
 			LogLog::warn(LOG4CXX_STR("[") + configFileName.getPath()
 				+ LOG4CXX_STR("] did not add an ") + Appender::getStaticClass().getName()
@@ -139,8 +187,10 @@ spi::ConfigurationStatus PropertyConfigurator::configureAndWatch(
 	return FileWatchdog::startWatching(configFilename, std::make_shared<PropertyConfigurator>(), LogManager::getLoggerRepository(), delay);
 }
 
-spi::ConfigurationStatus PropertyConfigurator::doConfigure(helpers::Properties& properties,
-	spi::LoggerRepositoryPtr hierarchy)
+spi::ConfigurationStatus PropertyConfigurator::doConfigure
+	( helpers::Properties&            properties
+	, const spi::LoggerRepositoryPtr& hierarchy
+	)
 {
 	LogString debugValue(properties.getProperty(LOG4CXX_STR("log4j.debug")));
 	if (!debugValue.empty())
@@ -187,20 +237,22 @@ spi::ConfigurationStatus PropertyConfigurator::doConfigure(helpers::Properties&
 		helpers::ThreadUtility::configure( ThreadConfigurationType::BlockSignalsAndNameThread );
 	}
 
-	configureRootLogger(properties, hierarchy);
-	configureLoggerFactory(properties);
-	parseCatsAndRenderers(properties, hierarchy);
+	auto result = spi::ConfigurationStatus::NotConfigured;
+	m_priv = std::make_unique<PrivateData>(hierarchy);
+	m_priv->configureRootLogger(properties);
+	m_priv->configureLoggerFactory(properties);
+	m_priv->parseCatsAndRenderers(properties);
 	LogLog::debug(LOG4CXX_STR("Finished configuring."));
-	auto result = !m_priv->appenderAdded
-		? spi::ConfigurationStatus::NotConfigured
-		: spi::ConfigurationStatus::Configured;
-
-	if (spi::ConfigurationStatus::Configured == result)
+	if (m_priv->appenderAdded)
+	{
 		hierarchy->setConfigured(true);
+		result = spi::ConfigurationStatus::Configured;
+	}
+	m_priv.reset();
 	return result;
 }
 
-void PropertyConfigurator::configureLoggerFactory(helpers::Properties& props)
+void PropertyConfigurator::PrivateData::configureLoggerFactory(helpers::Properties& props)
 {
 	LogString factoryClassName =
 		OptionConverter::findAndSubst(LOG4CXX_STR("log4j.loggerFactory"), props);
@@ -213,13 +265,12 @@ void PropertyConfigurator::configureLoggerFactory(helpers::Properties& props)
 			, std::make_shared<LoggerFactory>()
 			);
 
-		m_priv->loggerFactory = LOG4CXX_NS::cast<LoggerFactory>( instance );
-		PropertySetter::setProperties(m_priv->loggerFactory, props, LOG4CXX_STR("log4j.factory."));
+		this->loggerFactory = LOG4CXX_NS::cast<LoggerFactory>( instance );
+		PropertySetter::setProperties(this->loggerFactory, props, LOG4CXX_STR("log4j.factory."));
 	}
 }
 
-void PropertyConfigurator::configureRootLogger(helpers::Properties& props,
-	spi::LoggerRepositoryPtr& hierarchy)
+void PropertyConfigurator::PrivateData::configureRootLogger(helpers::Properties& props)
 {
 	LogString effectivePrefix(LOG4CXX_STR("log4j.rootLogger"));
 	LogString value = OptionConverter::findAndSubst(effectivePrefix, props);
@@ -236,13 +287,12 @@ void PropertyConfigurator::configureRootLogger(helpers::Properties& props,
 	}
 	else
 	{
-		LoggerPtr root = hierarchy->getRootLogger();
+		LoggerPtr root = this->pRepository->getRootLogger();
 		parseLogger(props, root, effectivePrefix, LOG4CXX_STR("root"), value, true);
 	}
 }
 
-void PropertyConfigurator::parseCatsAndRenderers(helpers::Properties& props,
-	spi::LoggerRepositoryPtr& hierarchy)
+void PropertyConfigurator::PrivateData::parseCatsAndRenderers(helpers::Properties& props)
 {
 	for (auto key : props.propertyNames())
 	{
@@ -256,7 +306,7 @@ void PropertyConfigurator::parseCatsAndRenderers(helpers::Properties& props,
 				).length();
 			auto loggerName = key.substr(prefixLength);
 			auto value = OptionConverter::findAndSubst(key, props);
-			auto logger = hierarchy->getLogger(loggerName, m_priv->loggerFactory);
+			auto logger = this->pRepository->getLogger(loggerName, this->loggerFactory);
 			auto additivity = parseAdditivityForLogger(props, logger, loggerName);
 			parseLogger(props, logger, key, loggerName, value, additivity);
 
@@ -264,7 +314,7 @@ void PropertyConfigurator::parseCatsAndRenderers(helpers::Properties& props,
 	}
 }
 
-bool PropertyConfigurator::parseAdditivityForLogger(helpers::Properties& props,
+bool PropertyConfigurator::PrivateData::parseAdditivityForLogger(helpers::Properties& props,
 	LoggerPtr& cat, const LogString& loggerName)
 {
 	LogString value(OptionConverter::findAndSubst(LOG4CXX_STR("log4j.additivity.") + loggerName, props));
@@ -287,7 +337,7 @@ bool PropertyConfigurator::parseAdditivityForLogger(helpers::Properties& props,
 /**
         This method must work for the root logger as well.
 */
-void PropertyConfigurator::parseLogger(
+void PropertyConfigurator::PrivateData::parseLogger(
 	helpers::Properties& props, LoggerPtr& logger, const LogString& /* optionKey */,
 	const LogString& loggerName, const LogString& value, bool additivity)
 {
@@ -374,7 +424,7 @@ void PropertyConfigurator::parseLogger(
 		}
 	}
 	if (!newappenders.empty())
-		m_priv->appenderAdded = true;
+		this->appenderAdded = true;
 	if (async && !newappenders.empty())
 	{
 		if (LogLog::isDebugEnabled())
@@ -388,7 +438,7 @@ void PropertyConfigurator::parseLogger(
 		logger->reconfigure( newappenders, additivity );
 }
 
-AppenderPtr PropertyConfigurator::parseAppender(
+AppenderPtr PropertyConfigurator::PrivateData::parseAppender(
 	helpers::Properties& props, const LogString& appenderName)
 {
 	AppenderPtr appender = registryGet(appenderName);
@@ -519,13 +569,13 @@ AppenderPtr PropertyConfigurator::parseAppender(
 	return appender;
 }
 
-void PropertyConfigurator::registryPut(const AppenderPtr& appender)
+void PropertyConfigurator::PrivateData::registryPut(const AppenderPtr& appender)
 {
-	(*m_priv->registry)[appender->getName()] = appender;
+	(*this->registry)[appender->getName()] = appender;
 }
 
-AppenderPtr PropertyConfigurator::registryGet(const LogString& name)
+AppenderPtr PropertyConfigurator::PrivateData::registryGet(const LogString& name)
 {
-	auto it = m_priv->registry->find(name);
-	return (it == m_priv->registry->end()) ? AppenderPtr() : it->second;
+	auto it = this->registry->find(name);
+	return (it == this->registry->end()) ? AppenderPtr() : it->second;
 }
diff --git a/src/main/include/log4cxx/propertyconfigurator.h b/src/main/include/log4cxx/propertyconfigurator.h
index d0817a2f..8a329ae8 100644
--- a/src/main/include/log4cxx/propertyconfigurator.h
+++ b/src/main/include/log4cxx/propertyconfigurator.h
@@ -27,25 +27,11 @@
 
 namespace LOG4CXX_NS
 {
-class Logger;
-typedef std::shared_ptr<Logger> LoggerPtr;
-
-class Appender;
-typedef std::shared_ptr<Appender> AppenderPtr;
-
 namespace helpers
 {
 class Properties;
 }
 
-
-namespace spi
-{
-class LoggerFactory;
-typedef std::shared_ptr<LoggerFactory> LoggerFactoryPtr;
-}
-
-class PropertyWatchdog;
 /**
 Allows the configuration of log4cxx from an external file.
 
@@ -273,7 +259,7 @@ class LOG4CXX_EXPORT PropertyConfigurator
 		*/
 		spi::ConfigurationStatus doConfigure
 			( const File&                     configFileName
-			, const spi::LoggerRepositoryPtr& repository = spi::LoggerRepositoryPtr()
+			, const spi::LoggerRepositoryPtr& target = {}
 			) override;
 
 		/**
@@ -288,17 +274,19 @@ class LOG4CXX_EXPORT PropertyConfigurator
 		A thread will be created that periodically checks
 		whether \c configFilename has been created or modified.
 		A period of log4cxx::helpers::FileWatchdog#DEFAULT_DELAY
-		is used if \c delay is not a positive number.
+		is used if \c millisecondDelay is not a positive number.
 		If a change or file creation is detected,
 		then \c configFilename is read to configure Log4cxx.
 
 		The thread will be stopped by a LogManager::shutdown call.
 
 		@param configFilename A file in key=value format.
-		@param delay The delay in milliseconds to wait between each check.
+		@param millisecondDelay The duration to wait between each check.
 		*/
-		static spi::ConfigurationStatus configureAndWatch(const File& configFilename,
-			long delay = 0);
+		static spi::ConfigurationStatus configureAndWatch
+			( const File& configFilename
+			, long        millisecondDelay = 0
+			);
 
 		/**
 		Read configuration options from <code>properties</code>.
@@ -313,52 +301,10 @@ class LOG4CXX_EXPORT PropertyConfigurator
 		See the \ref PropertyConfigurator_details "detailed description"
 		for the expected format.
 		*/
-		spi::ConfigurationStatus doConfigure(helpers::Properties& properties,
-			spi::LoggerRepositoryPtr hierarchy);
-
-		// --------------------------------------------------------------------------
-		// Internal stuff
-		// --------------------------------------------------------------------------
-	protected:
-		/**
-		Check the provided <code>Properties</code> object for a LoggerFactory
-		entry specified by *log4j.loggerFactory*.  If such an entry
-		exists, an attempt is made to create an instance using the default
-		constructor.  This instance is used for subsequent Logger creations
-		within this configurator.
-		@see #parseCatsAndRenderers
-		*/
-		void configureLoggerFactory(helpers::Properties& props);
-
-		void configureRootLogger(helpers::Properties& props,
-			spi::LoggerRepositoryPtr& hierarchy);
-
-		/**
-		Parse non-root elements, such non-root categories and renderers.
-		*/
-		void parseCatsAndRenderers(helpers::Properties& props,
-			spi::LoggerRepositoryPtr& hierarchy);
-
-		/**
-		Parse the additivity option for a non-root logger.
-		*/
-		bool parseAdditivityForLogger(helpers::Properties& props,
-			LoggerPtr& cat, const LogString& loggerName);
-
-		/**
-		This method must work for the root logger as well.
-		*/
-		void parseLogger(
-			helpers::Properties& props, LoggerPtr& logger,
-			const LogString& optionKey, const LogString& loggerName,
-			const LogString& value, bool additivity);
-
-		AppenderPtr parseAppender(
-			helpers::Properties& props, const LogString& appenderName);
-
-		void registryPut(const AppenderPtr& appender);
-		AppenderPtr registryGet(const LogString& name);
-
+		spi::ConfigurationStatus doConfigure
+			( helpers::Properties&            properties
+			, const spi::LoggerRepositoryPtr& target
+			);
 	private:
 		PropertyConfigurator(const PropertyConfigurator&);
 		PropertyConfigurator& operator=(const PropertyConfigurator&);