RFE: Smart properties save() method

[gitblit]

I use properties files as config files in Gitblit so your project is very interesting to me. Issue 15 (hot reload) is definitely a requirement for me to adopt your library, but another thing I would be missing is an enhanced write feature that replaces a property value in the file without destroying the file. I currently document config settings within the properties file (example https://github.com/gitblit/gitblit/blob/master/distrib/gitblit.properties). Unfortunately, the stock Properties class destroys the source file when you save. It is not very smart. :(

To work around this I implemented a smarter save. You can see my implementation of this here: https://github.com/gitblit/gitblit/blob/master/src/com/gitblit/FileSettings.java#L86

That capability is actually pretty small and I think would make a nice addition to your library.

[lviggiano]

The intent, in your case, is that you want to keep the original file structure (comments and the order of the file properties), correct?

The library, can load properties from a single property file (the first available from a list of @Sources) or load the properties from multiple @Sources (then merge them in memory following the order), the property replacement shall be done in the same file from where it has been loaded - if the file is writable-, or the narrowest writeable one (and throw an exception if none is writeable?).

I find the RFE interesting; I need to think a little bit on how to implement this in the correct way.

[gitblit]

Exactly. Preserve original file structure but update the specified (or all) property values.

Your override capability is of interest to me too. That would allow PHP-like behavior where there is a base config file and then personal setting overrides. Perhaps it would make sense to have an annotation attribute to specify preferred write destination in the event of multiple sources?

I'm not in a hurry. Thanks for your consideration.

[lviggiano]

I do agree that this would be a nice addition. I will focus on automatic properties reload first (I'll try to take example from commons-config), and a reload() method that can be triggered manually as well if you modify the file by yourself. So, at first you can update the file as you are doing, then trigger the reload().

The update mechanism may not be trivial due to the fact of the loading happens from multiple resources (which can be a file, but also an URL resource), in different ways (MERGE, FIRST), and (as soon as I can) will involve also different file formats.

The new annotation attribute to specify preferred write destination doesn't convince me.
I think that the preferred destination can be guessed by the framework itself:

• if the property is already available in one of the files (which is writable), that's where it should be overwritten, the narrowest should be preferred. But if the narrowest is not writable, the property will not take effect... (and should it throw an exception maybe)
• if the property is not available in any of the writable files specified in @Sources, then the narrowest writable one should apply. If no files are writable, then we have an exception.

But once determined the path to be updated, this can be delegated to a specific method like

public void update(URL target);  // updates sounds better than save, since the 'smart' save is 
                                 // keeping what was there before; but if the file doesn't exists, 
                                 // then the file will be created.

That can be made public (and available earlier). This will read the file then do the smart update as you described.
I don't like very much to specify a java.io.File inside a method signature; I would prefer OutputStream or a Writer, but since the content must be read first then updated later... URL may be better? since also an FTP url can be updatable and a URL is more flexible to specify a generic source that can be read and eventually updated...

So, at the end the methods that could be added to the Config interface can be something like:

public interface UpdatableConfig extends Config { 
    void reload();
    void setProperty(String key, String value);
    void setProperties(Map<String, String> newProperties)
    void update(); // will guess the resource where update
    void update(URL target);  // or string?
}

The update() methods will also imply a reload().

Implementing UpdatableConfig will add the support for updating the properties in memory or on the specified @Sources or to a specified target.

This is what it comes to my mind at the moment, but I'll think more about it.

I think the above methods could satisfy your RFE. You can do:

Map<String, String> updatedProps = ...
cfg.setProperties(updatedProps)
cfg.update(new URL("file:${baseFolder}/gitblit.properties"));

And this should make the changes on the file and reload the properties immediately.

It's not a single API call, but this allows people to change properties in memory without saving them to a file, that can also be desirable in some circumstances.

Since the setProperties and the update() are separated calls, I'll probably need to keep trac of which properties have been changed since last load (to update only those in the 'smart way'), also to minimize the lookup on the resources where things should be updated in several sources.

Comments and suggestions, are welcome.

[gitblit]

At first I didn't think that save(destination) would work for me, but that actually would be fine. I currently only call that in one place for the server.

You see that I too use the ${} notation in my properties. However, I don't resolve that against the properties file. I resolve those properties based on runtime parameters (e.g. baseFolder is a command-line parameter or it is set by the web.xml). Is there a way to disable OWNER's resolution of ${} properties?

[lviggiano]

It's not possible to disable properties resolution, if a variable ${} is not set, an empty string is replaced. There is no way to avoid this.

But, you can pass an external context for your variables resolution (I called this "importing properties")

Example:

public interface MyConfig extends Config {
    @DefaultValue("Hello ${world}.")
    String foo();
}

@Test
public void testPropertyNotSet() {
    MyConfig cfg = ConfigFactory.create(MyConfig.class);
    assertEquals("Hello .", cfg.foo());
}

@Test
public void testPropertySet() {
    Map<String, String> ctx = new HashMap<String, String>() {{
        put("world", "Earth");
    }};
    MyConfig cfg = ConfigFactory.create(MyConfig.class, ctx);    // here
    assertEquals("Hello Earth.", cfg.foo());
}

The second parameter in ConfigFactory.create() is a vararg, so you can specify more than one map to resolve the properties. So, for example you can do ConfigFactory.create(MyConfig.class, ctx1, ctx2, System.getenv(), System.getProperties(), ...);

Hope this helps.

[gitblit]

Do you you resolve these properties at property retrieval or at source load?

[lviggiano]

When ConfigFactory.create(class, map) is invoked, all the values from map are copied inside an internal properties list.

So this won't work:

@Test
public void testPropertyChanged() {
    Map<String, String> ctx = new HashMap<String, String>() {{
        put("world", "Earth");
    }};
    MyConfig cfg = ConfigFactory.create(MyConfig.class, ctx);

    ctx.put("world", "Mars");
    assertEquals("Hello Mars.", cfg.foo());
}

// output
org.junit.ComparisonFailure:  
Expected :Hello Mars.
Actual   :Hello Earth.

The variables are substituted when the method foo() is called (at retrieval), but the map specified on ConfigFactory.create(class, map) are copied when the method is invoked.

[gitblit]

Actually, I wanted to know when a value of "${baseFolder}/data" is resolved? At getting that property or at loading/copying the map into OWNER?

[lviggiano]

The value is substituted when you call the method to retrieve the property (when you do cfg.foo() in the above examples).

See PropertiesInvokationHandler.resolveProperty(), line 72

When you call MyConfig.foo() (that is actually a dynamic proxy), the PropertiesInvokationHandler.invoke() is called, and this calls resolveProperty() that calls substitutor.replace().

[lviggiano]

James, I've not forgot these things. I've been busy these days with other issues but I count to get back on this asap. Thanks for the patience.

[gitblit]

No problem, it will be a while before I would be ready to integrate OWNER. No hurry, really.

[lviggiano]

Hi James.

About this question:

Is there a way to disable OWNER's resolution of ${} properties?

I just committed a new annotation that allows to disable the ${} and the parameter formatters, as described in the issue #20 .

Documentation is updated and - as example - the junit test can be seen.

This annotation, when specified on class level is not inherited by subclasses by design.

This annotation will be available in version 1.0.4, and at the moment is available on the latest snapshot.

If you need this to be released into 1.0.4 final, I'll release it.

[lviggiano]

I've implemented a Mutable interface which allows to modify the config properties (above is described as UpdatableConfig), the request related is the #31
And modifying the file from which the configuration was loaded, it's possible to programmatically invoke the method reload().

Now I want to make some progress in the automatic reload (changing the file on the disk, the configuration is reloaded automatically in the Config object)

The smart update of files as you indicated here is not available yet. And I feel I need to think more on how this should be designed in the API interfaces.

[lviggiano]

Hi James.

I will release 1.0.4 soon, and it will include the hot reload feature, as well the @DisableFeatures that allow you to disable variable expansion that was causing you some issues.

Regarding the smart properties save, I thought long time how this should be implemented, and - they way I feel it should be - is very complicated, as I described above.

I think it may be just simpler to include your method in a utility method that is implemeted as you already did, but I quite don't feel it fitting in the rest of the API.

Any suggestion?

[gwinner]

I am also interested in a smart save() method as described. You can see an implementation of such in Apache Commons Configuration using the PropertiesConfigurationLayout helper class:

http://commons.apache.org/proper/commons-configuration/apidocs/org/apache/commons/configuration/PropertiesConfigurationLayout.html

Have you had additional thoughts since the original posts a year ago?

Thank you for your excellent work on OWNER.

[lviggiano]

I didn't do any work on this, but after initial (complicated) idea I had, I thought I would do something like the implementation you pointed out from commons-configuration. Actually I was thinking to use some layout engine like velocity or freemarker (in a separate maven module).
Unfortunately lately I have been very busy and I am not dedicating much time to this project. And in the meantime I was focusing on other features.

I also think this would be a nice feature, and it's something that I would like to add.

Thanks.

L.

[gwinner]

Hi Luigi,

Thanks for the information and for your work on this project. The design of
OWNER web site is very creative, visually appealing and practical.

All the best,

Gary

On Tue, Mar 18, 2014 at 8:00 AM, Luigi R. Viggiano <notifications@github.com

wrote:

I didn't do any work on this, but after initial (complicated) idea I had,
I thought I would do something like the implementation you pointed out from
commons-configuration. Actually I was thinking to use some layout engine
like velocity or freemarker (in a separate maven module).

Unfortunately lately I have been very busy and I am not dedicating much
time to this project. And in the meantime I was focusing on other features.

I also think this would be a nice feature, and it's something that I would
like to add.

Thanks.

L.

Reply to this email directly or view it on GitHubhttps://github.com//issues/16#issuecomment-37935253
.