Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1
    • Fix Version/s: 1.2
    • Component/s: General
    • Labels:
      None
    • Number of attachments :
      3
    1. NewConfiguration.java
      22 kB
      Ralf Joachim
    2. patch-C1927-20070627.txt
      65 kB
      Ralf Joachim
    3. patch-C1927-20070725-01.txt
      78 kB
      Ralf Joachim

      Activity

      Hide
      Ralf Joachim added a comment -

      The modul configurations (CoreConfiguration, XMLConfiguration, CPAConfiguration) are responsible for the modul specific predefined default values defined in castor.|modul|.properties files in one of the Castor jars. The CastorConfiguration class on the other side loads the user specific configuration from classpath root or current working directory. This design allows to maintain the cofigurations of the modules independend of each other. As you don't want to use a separate configuration for each modul the different configurations are linked with the parent property to be searched hierarchically. This is modelled with the same pattern used at java.lang.Properties.

      If you look at the newInstance() methods of CPAConfiguration (XMLConfiguration) you will see how the hierarchy is created. From top to bottom:

      CastorConfiguration (as user should be able to overwrite everything)
      CPAConfiguration (as some XML properties may need to be overwritten to allow loading of JDO mapping or config)
      XMLConfiguration
      CoreConfiguration

      As you see the concrete configuration returned will always be a CastorConfiguration holding the user properties. You won't have access to an additional getSAXParser() method defined at XMLConfiguration. The only chance to add other getters would be the abstract Configuration class where all the default getters are defined.

      Apart of the limitation of the design I personally think that a configuration class should not be responsible to handle such specific tasks like construction of a SAX parser. A better approach in my opinon would be to create the SAX parser somewhere else and put it in the configuration programatically for later retrieval by the getObject() method. Having said that I am not sure if things like a SAX parser instance should be put into a configuration at all.

      Show
      Ralf Joachim added a comment - The modul configurations (CoreConfiguration, XMLConfiguration, CPAConfiguration) are responsible for the modul specific predefined default values defined in castor.|modul|.properties files in one of the Castor jars. The CastorConfiguration class on the other side loads the user specific configuration from classpath root or current working directory. This design allows to maintain the cofigurations of the modules independend of each other. As you don't want to use a separate configuration for each modul the different configurations are linked with the parent property to be searched hierarchically. This is modelled with the same pattern used at java.lang.Properties. If you look at the newInstance() methods of CPAConfiguration (XMLConfiguration) you will see how the hierarchy is created. From top to bottom: CastorConfiguration (as user should be able to overwrite everything) CPAConfiguration (as some XML properties may need to be overwritten to allow loading of JDO mapping or config) XMLConfiguration CoreConfiguration As you see the concrete configuration returned will always be a CastorConfiguration holding the user properties. You won't have access to an additional getSAXParser() method defined at XMLConfiguration. The only chance to add other getters would be the abstract Configuration class where all the default getters are defined. Apart of the limitation of the design I personally think that a configuration class should not be responsible to handle such specific tasks like construction of a SAX parser. A better approach in my opinon would be to create the SAX parser somewhere else and put it in the configuration programatically for later retrieval by the getObject() method. Having said that I am not sure if things like a SAX parser instance should be put into a configuration at all.
      Hide
      Joachim Grüneis added a comment -

      Hello Ralf,

      I just had a look onto patch-C1927-20070627.txt and I had a lot of problems applying it against HEAD...
      So I looked onto the patch file instead - the changes I requested are in (no direct RuntimeException, getObject, getObjects...) so I'm fine with it.

      Show
      Joachim Grüneis added a comment - Hello Ralf, I just had a look onto patch-C1927-20070627.txt and I had a lot of problems applying it against HEAD... So I looked onto the patch file instead - the changes I requested are in (no direct RuntimeException, getObject, getObjects...) so I'm fine with it.
      Hide
      Ralf Joachim added a comment -

      Final patch.

      Show
      Ralf Joachim added a comment - Final patch.
      Hide
      Werner Guttmann added a comment -

      For the sake of reference, I have been through this patch twice over the last few days, and I really dig both the effort and the quality delivered (especially in terms of separation of concerns). I'll make some small changes to Javadocs over the next few days to make a few things more explicit, hoping that this will help us (everybody who touches code to integrate this new code) in our efforts.

      Here's one final question: is the HOME directory still included in the search for a user-defined castor.properties file ?

      Show
      Werner Guttmann added a comment - For the sake of reference, I have been through this patch twice over the last few days, and I really dig both the effort and the quality delivered (especially in terms of separation of concerns). I'll make some small changes to Javadocs over the next few days to make a few things more explicit, hoping that this will help us (everybody who touches code to integrate this new code) in our efforts. Here's one final question: is the HOME directory still included in the search for a user-defined castor.properties file ?
      Hide
      Ralf Joachim added a comment -

      The Castor JAR and Java HOME directory is search for:

      • castor.core.properties
      • castor.cpa.properties
      • castor.xml.properties
        but not for castor.properties. This is only searched for under:
      • classpath root
      • current working directory
        If one placed his castor.properties under:
      • classpath org.exolab.castor
      • Java HOME
        it will not be recognized any more. Having said that we can change that. If you have objections according to packages or class hierarchy im also open for ideas how to further improve this.
      Show
      Ralf Joachim added a comment - The Castor JAR and Java HOME directory is search for: castor.core.properties castor.cpa.properties castor.xml.properties but not for castor.properties. This is only searched for under: classpath root current working directory If one placed his castor.properties under: classpath org.exolab.castor Java HOME it will not be recognized any more. Having said that we can change that. If you have objections according to packages or class hierarchy im also open for ideas how to further improve this.

        People

        • Assignee:
          Ralf Joachim
          Reporter:
          Ralf Joachim
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved: