tabish121 commented on code in PR #6205:
URL: https://github.com/apache/artemis/pull/6205#discussion_r2760717891
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
##########
@@ -4682,22 +4682,28 @@ public void reloadConfigurationFile() throws Exception {
}
private void reloadConfigurationFile(URL xmlConfigUri) throws Exception {
+ Configuration config = new ConfigurationImpl();
if (xmlConfigUri != null) {
- Configuration config = new
FileConfigurationParser().parseMainConfig(xmlConfigUri.openStream());
+ config = new
FileConfigurationParser().parseMainConfig(xmlConfigUri.openStream());
LegacyJMSConfiguration legacyJMSConfiguration = new
LegacyJMSConfiguration(config);
legacyJMSConfiguration.parseConfiguration(xmlConfigUri.openStream());
- configuration.setSecurityRoles(config.getSecurityRoles());
- configuration.setAddressSettings(config.getAddressSettings());
-
configuration.setDivertConfigurations(config.getDivertConfigurations());
-
configuration.setAddressConfigurations(config.getAddressConfigurations());
- configuration.setQueueConfigs(config.getQueueConfigs());
-
configuration.setBridgeConfigurations(config.getBridgeConfigurations());
-
configuration.setConnectorConfigurations(config.getConnectorConfigurations());
-
configuration.setAcceptorConfigurations(config.getAcceptorConfigurations());
-
configuration.setAMQPConnectionConfigurations(config.getAMQPConnection());
- configuration.setPurgePageFolders(config.isPurgePageFolders());
}
- configuration.parseProperties(propertiesFileUrl);
+ config.parseProperties(propertiesFileUrl);
+ configuration.setStatus(config.getStatus());
+
Review Comment:
This seems to disregard any configuration that was assigned programmatically
via an initial configuration object when it sets the configuration from the
reload of the possible XML file and then the properties file. It creates an
empty configuration vessel which it might fill in from XML and or a properties
file(s) set and then hard sets those entries into the existing configuration
regardless of the differences between old and new which could lose original
data. Its not clear when or why these differing configuration sets take
precedence over each other.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java:
##########
@@ -1040,6 +1040,19 @@ public void addJaasConfig(JaasAppConfiguration config) {
jaasConfigs.put(config.getName(), config);
}
+ @Override
+ public Configuration setJaasConfigs(Map<String, JaasAppConfiguration>
configs) {
Review Comment:
I would normally expect a `set` method like this to clear the old values
before writing the provided new set of entries, this one is just confuses me
and seems to run counter to anything I'd expect a set method to do. The other
set methods here just replace the old collection with the new one, why it this
one different?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
