Tapestry 5
  1. Tapestry 5
  2. TAP5-223

Allow properties files (on classpath or in the context) to be used as SymbolProviders

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0.15
    • Fix Version/s: 5.1.0.5
    • Component/s: tapestry-ioc
    • Labels:
      None

      Description

      add a possibility to resolve symbols from properties file (PropertiesFileSymbolProvider).

      For reference, see:
      http://thread.gmane.org/gmane.comp.java.tapestry.user/65656

      1. TAP5-223.txt
        10 kB
        Ulrich Stärk
      2. PropertiesFileSymbolProvider.patch.txt
        18 kB
        Neeme Praks

        Activity

        Neeme Praks created issue -
        Hide
        Neeme Praks added a comment -

        attached possible implementation for this symbol provider

        Show
        Neeme Praks added a comment - attached possible implementation for this symbol provider
        Neeme Praks made changes -
        Field Original Value New Value
        Attachment PropertiesFileSymbolProvider.java [ 12390481 ]
        Neeme Praks made changes -
        Attachment PropertiesFileSymbolProvider.java [ 12390481 ]
        Hide
        Neeme Praks added a comment -

        fixed the implementation to use tapestry CaseInsensitiveMap instead of HashMap

        Show
        Neeme Praks added a comment - fixed the implementation to use tapestry CaseInsensitiveMap instead of HashMap
        Neeme Praks made changes -
        Attachment PropertiesFileSymbolProvider.java [ 12390482 ]
        Hide
        Howard M. Lewis Ship added a comment -

        No test suite?

        Show
        Howard M. Lewis Ship added a comment - No test suite?
        Hide
        Howard M. Lewis Ship added a comment -

        ... that's just asking me to do even more work. Code doesn't go in without tests.

        Show
        Howard M. Lewis Ship added a comment - ... that's just asking me to do even more work. Code doesn't go in without tests.
        Hide
        Ulrich Stärk added a comment -

        patch against trunk including a test suite.

        Show
        Ulrich Stärk added a comment - patch against trunk including a test suite.
        Ulrich Stärk made changes -
        Attachment TAP5-223.txt [ 12395726 ]
        Hide
        Neeme Praks added a comment -

        attached patch against trunk.

        includes:

        • improved version of original PropertiesFileSymbolProvider
        • test that covers 96% of PropertiesFileSymbolProvider and 100% of MapSymbolProvider (100% of PropertiesFileSymbolProvider is impossible without EasyMock class extension)
        • test data in form of properties file
        Show
        Neeme Praks added a comment - attached patch against trunk. includes: improved version of original PropertiesFileSymbolProvider test that covers 96% of PropertiesFileSymbolProvider and 100% of MapSymbolProvider (100% of PropertiesFileSymbolProvider is impossible without EasyMock class extension) test data in form of properties file
        Neeme Praks made changes -
        Attachment PropertiesFileSymbolProvider.patch.txt [ 12396211 ]
        Neeme Praks made changes -
        Attachment PropertiesFileSymbolProvider.java [ 12390482 ]
        Howard M. Lewis Ship made changes -
        Assignee Howard M. Lewis Ship [ hlship ]
        Howard M. Lewis Ship made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Howard M. Lewis Ship added a comment -

        I've looked over this patch. Although I appreciate the effort and the concept, it does not fulfill my goals for a Tapestry API. Basically put, I'm dubious about class with so many constructors.

        The essential problem is that it is too tied down to the details of where the properties file lives.

        I will look into an improved solution that divides the code into two parts: a factory service that can provide an implementation for any Resource, and an implementation based on reading from a Resource.

        The factory may have additional methods for common cases, such as a file on the classpath. I think it will code quickly.

        Show
        Howard M. Lewis Ship added a comment - I've looked over this patch. Although I appreciate the effort and the concept, it does not fulfill my goals for a Tapestry API. Basically put, I'm dubious about class with so many constructors. The essential problem is that it is too tied down to the details of where the properties file lives. I will look into an improved solution that divides the code into two parts: a factory service that can provide an implementation for any Resource, and an implementation based on reading from a Resource. The factory may have additional methods for common cases, such as a file on the classpath. I think it will code quickly.
        Howard M. Lewis Ship made changes -
        Assignee Howard M. Lewis Ship [ hlship ]
        Hide
        Ulrich Stärk added a comment -

        Howard, would something like this qualify for such a factory method? The only thing I see that will be problematic is that the individual Resource implementations are internal so the user shouldn't contribute them directly. We'd need another service that maps from a list of strings like "cp:org/example/foo", "context:foo/bar" and so on to the respective Resource implementations.

        Uli

        Show
        Ulrich Stärk added a comment - Howard, would something like this qualify for such a factory method? The only thing I see that will be problematic is that the individual Resource implementations are internal so the user shouldn't contribute them directly. We'd need another service that maps from a list of strings like "cp:org/example/foo", "context:foo/bar" and so on to the respective Resource implementations. Uli
        Ulrich Stärk made changes -
        Attachment ResourceSymbolProvider.java [ 12405925 ]
        Hide
        Ulrich Stärk added a comment -

        I now finished an implementation using a ResourceSymbolProvider that reads any Resource and tries to interpret it as symbol=value pairs. Additionally I created a ClasspathResourceSymbolProvider and a ContextResourceSymbolProvider that subclass ResourceSymbolProvider. The user can now contribute a ClasspathResourceSymbolProvider or a ContextResourceSymbolProvider to SymbolSource and access a properties file in the ServletContext or the Classpath.

        I'd like to hear your comments on this, especially from Howard.

        Show
        Ulrich Stärk added a comment - I now finished an implementation using a ResourceSymbolProvider that reads any Resource and tries to interpret it as symbol=value pairs. Additionally I created a ClasspathResourceSymbolProvider and a ContextResourceSymbolProvider that subclass ResourceSymbolProvider. The user can now contribute a ClasspathResourceSymbolProvider or a ContextResourceSymbolProvider to SymbolSource and access a properties file in the ServletContext or the Classpath. I'd like to hear your comments on this, especially from Howard.
        Ulrich Stärk made changes -
        Attachment TAP5-223.txt [ 12406131 ]
        Ulrich Stärk made changes -
        Attachment TAP5-223.txt [ 12395726 ]
        Ulrich Stärk made changes -
        Attachment ResourceSymbolProvider.java [ 12405925 ]
        Hide
        Howard M. Lewis Ship added a comment -

        I'm glad these are going into the internal package because I have some issues with them, primarily with the ContextResourceSymbolProvider ... that is, the order of operations for starting up a Tapestry web application means (to me) that it is highly likely that you will not have a Context available when it is necessary to contribute to the SymbolSource. Perhaps I'm missing something. I'm much more comfortable with the ResourceSymbolProvider.

        Show
        Howard M. Lewis Ship added a comment - I'm glad these are going into the internal package because I have some issues with them, primarily with the ContextResourceSymbolProvider ... that is, the order of operations for starting up a Tapestry web application means (to me) that it is highly likely that you will not have a Context available when it is necessary to contribute to the SymbolSource. Perhaps I'm missing something. I'm much more comfortable with the ResourceSymbolProvider.
        Howard M. Lewis Ship made changes -
        Summary add PropertiesFileSymbolProvider Allow properties files (on classpath or in the context) to be used as SymbolProviders
        Assignee Howard M. Lewis Ship [ hlship ]
        Priority Major [ 3 ] Minor [ 4 ]
        Howard M. Lewis Ship made changes -
        Status In Progress [ 3 ] Closed [ 6 ]
        Fix Version/s 5.1.0.5 [ 12313913 ]
        Resolution Fixed [ 1 ]
        Hide
        Neeme Praks added a comment -

        Howard suspicion about ContextResourceSymbolProvider seems to be "right on the money". We are unable to use it. Can you provide sample code on how to use it?

        Show
        Neeme Praks added a comment - Howard suspicion about ContextResourceSymbolProvider seems to be "right on the money". We are unable to use it. Can you provide sample code on how to use it?
        Neeme Praks made changes -
        Resolution Fixed [ 1 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Hide
        Neeme Praks added a comment -

        some code from my side:

           public static void contributeSymbolSource(
              OrderedConfiguration<SymbolProvider> configuration,
                    @ApplicationDefaults
                    SymbolProvider applicationDefaults,
                    @FactoryDefaults
                    SymbolProvider factoryDefaults,
                    @InjectService("Context") Context context) 
            {
             
             configuration.add("ZpsSymbols",
               new ContextResourceSymbolProvider(context, "WEB-INF/conf/zps-web.properties"));
            }
        

        Gives error:
        Caused by: java.lang.RuntimeException: Exception constructing service 'Context': The constructor for module class org.apache.tapestry5.services.Tapest
        ryModule is recursive: it depends on itself in some way. The constructor, public org.apache.tapestry5.services.TapestryModule(org.apache.tapestry5.ioc
        .services.PipelineBuilder,org.apache.tapestry5.ioc.services.PropertyShadowBuilder,org.apache.tapestry5.services.RequestGlobals,org.apache.tapestry5.se
        rvices.ApplicationGlobals,org.apache.tapestry5.ioc.services.ChainBuilder,org.apache.tapestry5.services.Environment,org.apache.tapestry5.ioc.services.S
        trategyBuilder,org.apache.tapestry5.ioc.services.PropertyAccess,org.apache.tapestry5.services.Request,org.apache.tapestry5.services.Response,org.apach
        e.tapestry5.services.EnvironmentalShadowBuilder,org.apache.tapestry5.internal.services.EndOfRequestEventHub), is in some way is triggering a service b
        uilder, decorator or contribution method within the class.

        And in this case, the context is null:

           public static void contributeSymbolSource(
             OrderedConfiguration<SymbolProvider> configuration,
                   @ApplicationDefaults
                   SymbolProvider applicationDefaults,
                   @FactoryDefaults
                   SymbolProvider factoryDefaults,
                   @InjectService("ApplicationGlobals") ApplicationGlobals globals) 
           {
            
            configuration.add("ZpsSymbols",
              new ContextResourceSymbolProvider(globals.getContext(), "WEB-INF/conf/zps-web.properties"));
           }
        

        Any other ideas are highly appreciated.

        Show
        Neeme Praks added a comment - some code from my side:     public static void contributeSymbolSource(      OrderedConfiguration<SymbolProvider> configuration,            @ApplicationDefaults            SymbolProvider applicationDefaults,            @FactoryDefaults            SymbolProvider factoryDefaults,            @InjectService( "Context" ) Context context)    {         configuration.add( "ZpsSymbols" ,       new ContextResourceSymbolProvider(context, "WEB-INF/conf/zps-web.properties" ));    } Gives error: Caused by: java.lang.RuntimeException: Exception constructing service 'Context': The constructor for module class org.apache.tapestry5.services.Tapest ryModule is recursive: it depends on itself in some way. The constructor, public org.apache.tapestry5.services.TapestryModule(org.apache.tapestry5.ioc .services.PipelineBuilder,org.apache.tapestry5.ioc.services.PropertyShadowBuilder,org.apache.tapestry5.services.RequestGlobals,org.apache.tapestry5.se rvices.ApplicationGlobals,org.apache.tapestry5.ioc.services.ChainBuilder,org.apache.tapestry5.services.Environment,org.apache.tapestry5.ioc.services.S trategyBuilder,org.apache.tapestry5.ioc.services.PropertyAccess,org.apache.tapestry5.services.Request,org.apache.tapestry5.services.Response,org.apach e.tapestry5.services.EnvironmentalShadowBuilder,org.apache.tapestry5.internal.services.EndOfRequestEventHub), is in some way is triggering a service b uilder, decorator or contribution method within the class. And in this case, the context is null: public static void contributeSymbolSource( OrderedConfiguration<SymbolProvider> configuration, @ApplicationDefaults SymbolProvider applicationDefaults, @FactoryDefaults SymbolProvider factoryDefaults, @InjectService( "ApplicationGlobals" ) ApplicationGlobals globals) { configuration.add( "ZpsSymbols" , new ContextResourceSymbolProvider(globals.getContext(), "WEB-INF/conf/zps-web.properties" )); } Any other ideas are highly appreciated.
        Howard M. Lewis Ship made changes -
        Assignee Howard M. Lewis Ship [ hlship ]
        Hide
        Massimo Lusetti added a comment -

        I want to give a look at this one... maybe will be for 5.4

        Show
        Massimo Lusetti added a comment - I want to give a look at this one... maybe will be for 5.4
        Massimo Lusetti made changes -
        Assignee Massimo Lusetti [ mlusetti ]
        Hide
        Massimo Lusetti added a comment -

        This is fine as it is

        Show
        Massimo Lusetti added a comment - This is fine as it is
        Massimo Lusetti made changes -
        Status Reopened [ 4 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Hide
        Neeme Praks added a comment -

        What is "fine as it is"? AFAIK, current Tapestry allows to load properties from classpath, but not from WEB-INF. Is this still the case? And will remain so?

        Show
        Neeme Praks added a comment - What is "fine as it is"? AFAIK, current Tapestry allows to load properties from classpath, but not from WEB-INF. Is this still the case? And will remain so?
        Hide
        Massimo Lusetti added a comment -

        Yep.
        Has Howard said there's a chicken/egg issue here, when you need to Context to provide your contribution to SymbolSource you don't have Context yet so ContextResourceSymbolProvider as is now is almost unusable, despite WEB-INF or not.

        We are in 5.3 release cycle so I don't want to rewind anything.

        If you desperately need this you could use ResourceSymbolProvider which let you provide a Resource which can be inside WEB-INF or wherever you want.

        Show
        Massimo Lusetti added a comment - Yep. Has Howard said there's a chicken/egg issue here, when you need to Context to provide your contribution to SymbolSource you don't have Context yet so ContextResourceSymbolProvider as is now is almost unusable, despite WEB-INF or not. We are in 5.3 release cycle so I don't want to rewind anything. If you desperately need this you could use ResourceSymbolProvider which let you provide a Resource which can be inside WEB-INF or wherever you want.

          People

          • Assignee:
            Massimo Lusetti
            Reporter:
            Neeme Praks
          • Votes:
            3 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development