Commons Configuration
  1. Commons Configuration
  2. CONFIGURATION-104

[configuration] Preserve file structure (line comments) when re-saving properties file

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: Other

      Description

      This bug applies when an application allows both manual editing of its
      configuration files, and manipulation of the configuration via software. When
      saving the configuration file after changing, its file structure - file,
      section and line comments - will all be lost. Next time a human tries to edit
      the configuration file, all property documentation will be lost.

      I have created a fix for this problem, applicable to PropertiesConfiguration
      and XMLPropertiesConfiguration. The fix allows these classes to preserve a
      list of comment lines preceding each property line. By default, comments are
      not preserved; in case the comment preservation is required, it should be
      enabled prior to loading the configuration file using:

      AbstractFileConfiguration.setDefaultPreserveComments(true);

      The fix is implemented and tested, attached as a patch file to this bug.

        Issue Links

          Activity

          Hide
          Oded Noam added a comment -

          Created an attachment (id=18018)
          Comment preservation mechanism for AbstractFileConfiguration and its
          descendants

          Show
          Oded Noam added a comment - Created an attachment (id=18018) Comment preservation mechanism for AbstractFileConfiguration and its descendants
          Hide
          Oliver Heger added a comment -

          Looks interesting. Thank you for your work!

          I think, Emmanuel Bourg also has some ideas in this area (see
          http://issues.apache.org/bugzilla/show_bug.cgi?id=38285#c3). Maybe he can chime
          in and comment about how your approach fits with his ideas?

          However there is one thing I noticed, which is problematic: Our target
          environment is still JDK 1.3. You use some of the new regex facilities that are
          not available in this JDK. So either the affected parts have to be re-written or
          this patch cannot be applied before we drop support for JDK 1.3.

          Show
          Oliver Heger added a comment - Looks interesting. Thank you for your work! I think, Emmanuel Bourg also has some ideas in this area (see http://issues.apache.org/bugzilla/show_bug.cgi?id=38285#c3 ). Maybe he can chime in and comment about how your approach fits with his ideas? However there is one thing I noticed, which is problematic: Our target environment is still JDK 1.3. You use some of the new regex facilities that are not available in this JDK. So either the affected parts have to be re-written or this patch cannot be applied before we drop support for JDK 1.3.
          Hide
          Oded Noam added a comment -

          Created an attachment (id=18067)
          Revised - java1.3 version of the patch

          You are right about the Java1.3 compliance issue. I fixed it.

          Of course, this is not exactly what Emmanuel Bourg dreamt about... But it could
          be expanded to be good enough 90% of the times. Emmanuel Bourg's idea seems
          nice, but I don't think it can be achieved without requiring the user to create
          a formatting definitions file for each properties file. Maybe some enhancement
          of my mechanism would be easier to use.

          Show
          Oded Noam added a comment - Created an attachment (id=18067) Revised - java1.3 version of the patch You are right about the Java1.3 compliance issue. I fixed it. Of course, this is not exactly what Emmanuel Bourg dreamt about... But it could be expanded to be good enough 90% of the times. Emmanuel Bourg's idea seems nice, but I don't think it can be achieved without requiring the user to create a formatting definitions file for each properties file. Maybe some enhancement of my mechanism would be easier to use.
          Hide
          Emmanuel Bourg added a comment -

          Thanks for the patch !

          There are several aspects to consider in order to preserve completly the file
          structure:

          • the comments (property or section comments)
          • the blank lines
          • the order of the properties
          • the proper insertion of the new properties next to the similar properties
          • the format of the properties:
          • multiline or single line lists
          • decimal or hexadecimal numbers
          • text wrapped on multiple lines
          • etc

          To handle this I'd like to introduce a Layout class attached to the
          configuration rather than overloading the class with more methods. This class
          will maintain a List of the elements of the file. This list would look like this:

          <comment: Configuration file for the Foo application>
          <blank line>
          <comment: Main properties>
          property: foo.key.a
          property: foo.key.b
          property: foo.key.bar.a
          <blank line>
          <comment: Secondary properties>
          property: bar.key.a
          property: bar.key.b
          property: bar.key.c

          Every time a property is added or removed from the configuration, the layout
          will be notified and it will update the list accordingly. It will attempt to
          group the properties by prefix. For example, if the foo.key.bar.b property is
          added, it will look for the last property sharing the same prefix, that's
          foo.key.bar.a, and insert it after:

          ...
          property: foo.key.a
          property: foo.key.b
          property: foo.key.bar.a
          property: foo.key.bar.b
          ...

          If no similar property is found the property is added at the end of the list.

          For every property a format is attached, that's what specifies the property
          specific details like the list format or the number format.

          It'll be possible to disable this feature if performance is more important than
          the correctness of the file. Either by adding a flag to the layout, or by using
          a NullLayout that would produce the same result we have today.

          Show
          Emmanuel Bourg added a comment - Thanks for the patch ! There are several aspects to consider in order to preserve completly the file structure: the comments (property or section comments) the blank lines the order of the properties the proper insertion of the new properties next to the similar properties the format of the properties: multiline or single line lists decimal or hexadecimal numbers text wrapped on multiple lines etc To handle this I'd like to introduce a Layout class attached to the configuration rather than overloading the class with more methods. This class will maintain a List of the elements of the file. This list would look like this: <comment: Configuration file for the Foo application> <blank line> <comment: Main properties> property: foo.key.a property: foo.key.b property: foo.key.bar.a <blank line> <comment: Secondary properties> property: bar.key.a property: bar.key.b property: bar.key.c Every time a property is added or removed from the configuration, the layout will be notified and it will update the list accordingly. It will attempt to group the properties by prefix. For example, if the foo.key.bar.b property is added, it will look for the last property sharing the same prefix, that's foo.key.bar.a, and insert it after: ... property: foo.key.a property: foo.key.b property: foo.key.bar.a property: foo.key.bar.b ... If no similar property is found the property is added at the end of the list. For every property a format is attached, that's what specifies the property specific details like the list format or the number format. It'll be possible to disable this feature if performance is more important than the correctness of the file. Either by adding a flag to the layout, or by using a NullLayout that would produce the same result we have today.
          Hide
          Oded Noam added a comment -

          I like the idea for the Layout class. But I see two problems with the fact
          that the Layout object is initialized from a configuration file.

          First, it requires the application developer to create a layout configuration
          file, in addition to default properties file (and, of course, to make sure the
          two are compliant with each other). This is a small issue but I suspect that
          this requirement might discourage 99% of the potential users of this feature.

          Second, since the layout of the file is not derived from the layout it had
          before being parsed, re-saving it may result in re-ordering of the lines in
          the file, thus making it harder to review the changes using diff (or CVS, for
          that matter).

          On the other hand, using a layout configuration file will enable formatting of
          properties added by the user, and accurate formatting of complex properties
          (lists, multi-lines etc). Perhaps you know of applications that make a more
          sophisticated use of properties files than I do, but in the applications I've
          seen, it is rare for an end user to make a big change to the properties file.
          Usually an end user would only modify a couple of properties that are
          specified in the file (or commented out). Same goes for configuration changes
          made via software.

          Maybe the way to go is to create the Layout class, and have it initialized
          from analyzing the parsed properties file. Such layout class would not be
          complete, but in time we can add all kinds of heuristics for it to detect
          things like multiline layout configuration, property groups etc.

          Show
          Oded Noam added a comment - I like the idea for the Layout class. But I see two problems with the fact that the Layout object is initialized from a configuration file. First, it requires the application developer to create a layout configuration file, in addition to default properties file (and, of course, to make sure the two are compliant with each other). This is a small issue but I suspect that this requirement might discourage 99% of the potential users of this feature. Second, since the layout of the file is not derived from the layout it had before being parsed, re-saving it may result in re-ordering of the lines in the file, thus making it harder to review the changes using diff (or CVS, for that matter). On the other hand, using a layout configuration file will enable formatting of properties added by the user, and accurate formatting of complex properties (lists, multi-lines etc). Perhaps you know of applications that make a more sophisticated use of properties files than I do, but in the applications I've seen, it is rare for an end user to make a big change to the properties file. Usually an end user would only modify a couple of properties that are specified in the file (or commented out). Same goes for configuration changes made via software. Maybe the way to go is to create the Layout class, and have it initialized from analyzing the parsed properties file. Such layout class would not be complete, but in time we can add all kinds of heuristics for it to detect things like multiline layout configuration, property groups etc.
          Hide
          Oliver Heger added a comment -

          +1 for the idea of a Layout class that tries to obtain as much information as possible from the properties file. I would like to start with a rather simple one that can be enhanced step by step.

          The collaboration between a PropertiesConfiguration object and its Layout object could be as follows:

          • The load(Reader) method of PropertiesConfiguration delegates to the Layout object.
          • The Layout object reads the file line by line and stores relevant information, e.g. about comments, in internal data structures.
          • Extracted properties are added to the PropertiesConfiguration.
          • The Layout object is also registered as event listener at its PropertiesConfiguration. If events are received, its content is updated accordingly.
          • PropertiesConfiguration.save(Writer) again delegates to the Layout object for writing the data back to file.

          So the interface of the layout class could roughly look as follows:

          public class PropertiesConfigurationLayout implements ConfigurationListener

          { /** * Creates a new instance and sets the associated configuration. */ public PropertiesConfigurationLayou(PropertiesConfiguration config); /** * Reads in a properties file. */ public void load(Reader in) throws ConfigurationException; /** * Writes the internal data into a properties file. */ public void save(Writer out) throws ConfigurationException; // it would also be nice to have access to comments for properties /** * Returns the comment for the specified key. */ public String getComment(String key); /** * Sets the comment for the specified key. */ public void setComment(String key, String comment); /** * Returns the header comment. */ public String getHeaderComment(); /** * Sets the header comment. */ public void setHeaderComment(String comment); }

          Any thoughts?

          Show
          Oliver Heger added a comment - +1 for the idea of a Layout class that tries to obtain as much information as possible from the properties file. I would like to start with a rather simple one that can be enhanced step by step. The collaboration between a PropertiesConfiguration object and its Layout object could be as follows: The load(Reader) method of PropertiesConfiguration delegates to the Layout object. The Layout object reads the file line by line and stores relevant information, e.g. about comments, in internal data structures. Extracted properties are added to the PropertiesConfiguration. The Layout object is also registered as event listener at its PropertiesConfiguration. If events are received, its content is updated accordingly. PropertiesConfiguration.save(Writer) again delegates to the Layout object for writing the data back to file. So the interface of the layout class could roughly look as follows: public class PropertiesConfigurationLayout implements ConfigurationListener { /** * Creates a new instance and sets the associated configuration. */ public PropertiesConfigurationLayou(PropertiesConfiguration config); /** * Reads in a properties file. */ public void load(Reader in) throws ConfigurationException; /** * Writes the internal data into a properties file. */ public void save(Writer out) throws ConfigurationException; // it would also be nice to have access to comments for properties /** * Returns the comment for the specified key. */ public String getComment(String key); /** * Sets the comment for the specified key. */ public void setComment(String key, String comment); /** * Returns the header comment. */ public String getHeaderComment(); /** * Sets the header comment. */ public void setHeaderComment(String comment); } Any thoughts?
          Hide
          Oliver Heger added a comment -

          A new class PropertiesConfigurationLayout was added. Objects of this class are associated with PropertiesConfiguration instances. They are able to preserve many structure related aspects of a properties file.

          Please have a look if this class fits your needs.

          Show
          Oliver Heger added a comment - A new class PropertiesConfigurationLayout was added. Objects of this class are associated with PropertiesConfiguration instances. They are able to preserve many structure related aspects of a properties file. Please have a look if this class fits your needs.

            People

            • Assignee:
              Oliver Heger
              Reporter:
              Oded Noam
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development