Commons Configuration
  1. Commons Configuration
  2. CONFIGURATION-118

[configuration] Loading a configuration twice creates duplicate properties

    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: All
      Platform: All

      Description

      The file configurations are not cleared when the source is loaded, thus if the
      configuration is loaded twice, we end up with duplicate properties.

      public void testDoubleLoading() throws Exception

      { FileConfiguration config = new PropertiesConfiguration(); config.setFileName("conf/test.properties"); config.load(); config.load(); assertEquals("Number of elements in the 'test.integer.array' list", 2, config.getList("test.integer.array").size()); }

        Activity

        Emmanuel Bourg created issue -
        Hide
        Oliver Heger added a comment -

        We could add an abstract append(Reader) method to AbstractFileConfiguration. In
        the sub classes we move the code from the load(Reader) method to append(Reader).
        AbstractFileConfiguration.load(Reader) can then be implemented by calling
        clear() and then delegating to append(Reader). XMLConfiguration needs an extra
        work because it does not inherit from AbstractFileConfiguration.

        This way we would also gain a new append feature, which might be useful for
        easily creating union configurations. WDYT?

        Is anybody already working on this? Otherwise I would have a look at it during
        the weekend.

        Show
        Oliver Heger added a comment - We could add an abstract append(Reader) method to AbstractFileConfiguration. In the sub classes we move the code from the load(Reader) method to append(Reader). AbstractFileConfiguration.load(Reader) can then be implemented by calling clear() and then delegating to append(Reader). XMLConfiguration needs an extra work because it does not inherit from AbstractFileConfiguration. This way we would also gain a new append feature, which might be useful for easily creating union configurations. WDYT? Is anybody already working on this? Otherwise I would have a look at it during the weekend.
        Hide
        Emmanuel Bourg added a comment -

        I see 2 more solutions:

        • change nothing and document the load() method to make it clear that properties
          are added to the configuration and do not replace the current values.
        • change the load() method to include a call to clear(), but not the other
          methods loading from a stream, a file, a url, etc.
        Show
        Emmanuel Bourg added a comment - I see 2 more solutions: change nothing and document the load() method to make it clear that properties are added to the configuration and do not replace the current values. change the load() method to include a call to clear(), but not the other methods loading from a stream, a file, a url, etc.
        Hide
        Emmanuel Bourg added a comment -

        Anyway this is a minor issue, it should not block the 1.1 release.

        Show
        Emmanuel Bourg added a comment - Anyway this is a minor issue, it should not block the 1.1 release.
        Hide
        Oliver Heger added a comment -

        (In reply to comment #2)
        > I see 2 more solutions:
        >
        > - change nothing and document the load() method to make it clear that properties
        > are added to the configuration and do not replace the current values.
        >
        > - change the load() method to include a call to clear(), but not the other
        > methods loading from a stream, a file, a url, etc.

        The first solution is fine with me, especially after cross checking that
        java.util.Properties.load() behaves exactly the same way.

        I added some notes to the javadoc of AbstractFileConfiguration and the load()
        methods of PropertiesConfiguration and XMLConfiguration. I also added two new
        test cases that show that a second load() call inserts new properties rather
        than replacing the old content.

        For XMLConfiguration it was a little harder because I found out that the output
        of the save() method is broken when multiple files were loaded. I could fix
        this, which is also checked in the corresponding test class.

        So I suppose this bug can be closed now?

        Show
        Oliver Heger added a comment - (In reply to comment #2) > I see 2 more solutions: > > - change nothing and document the load() method to make it clear that properties > are added to the configuration and do not replace the current values. > > - change the load() method to include a call to clear(), but not the other > methods loading from a stream, a file, a url, etc. The first solution is fine with me, especially after cross checking that java.util.Properties.load() behaves exactly the same way. I added some notes to the javadoc of AbstractFileConfiguration and the load() methods of PropertiesConfiguration and XMLConfiguration. I also added two new test cases that show that a second load() call inserts new properties rather than replacing the old content. For XMLConfiguration it was a little harder because I found out that the output of the save() method is broken when multiple files were loaded. I could fix this, which is also checked in the corresponding test class. So I suppose this bug can be closed now?
        Hide
        Emmanuel Bourg added a comment -

        Yes that's fine, thank you Oliver

        Show
        Emmanuel Bourg added a comment - Yes that's fine, thank you Oliver
        Henri Yandell made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 33814 12342086
        Henri Yandell made changes -
        Affects Version/s unspecified [ 12311647 ]
        Component/s Configuration [ 12311107 ]
        Key COM-1934 CONFIGURATION-118
        Project Commons [ 12310458 ] Commons Configuration [ 12310467 ]
        Assignee Jakarta Commons Developers Mailing List [ commons-dev@jakarta.apache.org ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Emmanuel Bourg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development