I have a java web applilication. This application loads its configuration file with singleton according source file below.
public class Configuration {
private static Configuration config;
private static Properties prop = null;
private static InputStream input = null;
private Configuration() {
input = this.getClass().getResourceAsStream("/config.properties");
prop = new Properties();
prop.load(input);
input = new FileInputStream(prop.getProperty("soap.config"));
prop = new Properties(prop);
prop.load(new InputStreamReader(input, StandardCharsets.UTF_8));
}
public static Configuration getInstance() {
if (config == null) {
config = new Configuration();
}
return config;
}
}
config.properties (located into resources folder of java project)
soap.config=/home/wildfly/soap.propeties
Content of soap.properties file:
server=192.168.1.1
user=John
pass=thepass
Server features: Total memory: 8GBram. 30% ram used 1 core, 40gb hard disk Widfly Server linux virtual machine
If I want to change some value in config file, it's necessary to restart the aplication by Wildfly admin console also. I think it's more useful to change config file values without restarting the application.
Additionally, the application receives more than thousands request a day and I see server status is fine.
Questions:
- Is it beneficial or worth to use singleton to load a configuration file?
- The instruction this.getClass().getResourceAsStream("/config.properties") will read the config file and after that it will close it inmediatelly. Is it correct?
CodePudding user response:
The best solution I can think of:
- simply give the config another field:
lastFileEditedTime
that stores the timestamp of the file that was loaded last. - create a static variable:
lastFileUpdateCheckedTime
. this stores the last check time in ms (System.getCurrentMilis
) and if the last check has been made more than x seconds ago, check that file again - use both in the
getInstance()
method. First check againstlastFileUpdateCheckedTime
, and if that triggers, check againstlastFileEditedTime
(you could also make both static or add both to the config, however you like)
This way the system keeps loading updated files, but will not reload too many times per second or do the filesystem timestamp check too often.
And to answer your questions:
yes, singleton is beneficial, because it prevents loading each time, and all parts of your code that use it are (more or less) on the same config
no, getResourceAsStream will give you an open stream, you should close it. Activate compiler warnings, they will clearly show you. Best use try-resource-catch for closing
CodePudding user response:
Let's break this into two pieces: is a singleton ok for a config file? Kind of. Singletons have some major flaws when it comes to testability. It's better in general to use injection, and to pass an instance of the class to every place that needs it. A singleton will work, but it will make your testing far harder.
Is there a better way to handle config files so they handle changes? Yes. Use a WatchService to watch for when the file is changed (https://docs.oracle.com/javase/tutorial/essential/io/notification.html). The Configuration class should handle this. Then when it does change, it should parse the new file and update itself. This does provide a gap for race conditions though where part of the data is fetched from the old file, and part from the new. There's techniques you can use to avoid that however (providing all the data atomically, or allowing a client to lock the configuration file and only updating when its unlocked, etc).