Get rid of ServiceRegistry::getPropertiesService references in Mantis Codebase
sundargates opened this issue · 5 comments
Context
ServiceRegistry is a singleton instance that holds a reference to MantisPropertiesService. Several components within Mantis use this reference to get certain configuration values at runtime. There are several issues with this approach. Firstly, singletons take a lot of work to manage and test. Secondly, the way the MantisPropertiesService gets loaded is not extendable. We need to consider a more long-term approach to deal with dynamic properties within Mantis. Something like archaius2 might be a good idea.
Goals
- Getting rid of ServiceRegistry.
- Clean ways to integrate with various dynamic config implementations.
I would like to pick this. @sundargates, can you please assign this issue to me?
@kriti-sc, let me know if you need any help with this task.
I looked into archaius2 and how it would be used here.
Right now, MantisPropertiesService is used as the main configuration manager, which implements DefaultMantisPropertiesLoader, which actually imports properties from environment variables and parses them.
The Mantis codebase uses the MantisPropertiesService reference through the ServiceRegistery to obtain configuration values.
Now, instead of writing a custom configuration manager, we will use archaius2 as a configuration manager, that would fetch configuration from files or environment (for example). We would still expose it using ServiceRegistry, but the way the configuration is exposed would change.
This is how configuration is exposed currently: ServiceRegistry.INSTANCE.getPropertiesService().getStringValue("mantis.netty.enableHeartBeating", "true");
This would change to:
ServiceRegistry. properties.getRawProperty("mantis.netty.enableHeartBeating", "true");
properties would be the object of type archaius2 config, and getRawProperty is the method of the object to obtain the value of some property. We would not have to worry about maintaining the singleton pattern, because archaius would do that. After this change MantisPropertiesService & MantisPropertiesLoader would become redundant.
This also means that the change would have to be made in all places where ServiceRegistry is used to obtain configuration values.
The above is just an outline of how things will work. @sundargates, please let me know your opinion.
Update – Found an archaius reference within mantis. Will be looking at that and if relevant, take inspiration.
@sundargates
I am assuming ServiceRegistry::getPropertiesService refers to this file: file
It contains a call to DefaultMantisPropertiesLoader here, which performs a System.getenv().
The first thing the new configuration manager will need is getter and setter methods for the config variables. Can you please provide me with a list of all the configuration variables (and their default values) that are loaded from the environment in the above case?