[infinispan-issues] [JBoss JIRA] (ISPN-3948) Missing logic to add the store to the configuration for custom stores

[Kurt Lehrke (JIRA)]

    [ https://issues.jboss.org/browse/ISPN-3948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12940166#comment-12940166 ] 

Kurt Lehrke edited comment on ISPN-3948 at 1/30/14 8:55 AM:
------------------------------------------------------------

I did some research on possibly fixing this issue, here are some notes:

The PersistenceConfigurationBuilder is a bit misleading with the methods addStore.  It doesn't add stores at all, it expects StoreConfigurationBuilders.  I'm guessing the naming convention came along from the old version of the loaders configuration builder.

The xml defined configuration expects a class to the Store, not the StoreConfigurationBuilder.  Unfortunately, there's no insight into what StoreConfigurationBuilder goes with what Cache Store/Loader.   This makes it impossible to give the PersistenceConfigurationBuilder the means to build the store.  My first reaction was to reuse the BuiltBy annotation for custom cachestores to specify which builder goes with it, that way it does have some means of getting the StoreConfigurationBuilder.  The downfall to this approach is that even for the most simple custom cachestores (ones that might be self contained needing no additional configuration attributes/values), would still require a StoreConfiguration and a StoreConfigurationBuilder.  This makes it especially more difficult because they can't be reused (say if there was a DefaultStoreConfiguration(Builder) )because of the annotations required by the StoreConfiguration (i.e. ConfigurationFor ).  

The other option might be to allow custom cache stores to define the class to the StoreConfigurationBuilder.  This would still require a builder for each store.  Also, this would require a  few changes in the Parser as the first step to the parsingStore method is reflectively creating the Store (even though it isn't used besides a null check).  The upside to this approach is that it wouldn't require any additional annotation and the code would just have two paths for whether the configuration specified an actual store or a StoreConfigurationBuilder.
                
      was (Author: kurtymckurt):
    I did some research on possibly fixing this issue, here are some notes:

The PersistenceConfigurationBuilder is a bit misleading with the methods addStore.  It doesn't add stores at all, it expects StoreConfigurationBuilders.  I'm guessing the naming convention came along from the old version of the loaders configuration builder.

The xml defined configuration expects a class to the Store, not the StoreConfigurationBuilder.  Unfortunately, there's no insight into what StoreConfigurationBuilder goes with what Cache Store/Loader.   This makes it impossible to give the PersistenceConfigurationBuilder the means to build the store.  My first reaction was to reuse the BuiltBy annotation for custom cachestores to specify which builder goes with it, that way it does have some means of getting the StoreConfigurationBuilder.  The downfall to this approach is that even for the most simple custom cachestores (ones that might be self contained needing no additional configuration attributes/values), would still require a StoreConfiguration and a StoreConfigurationBuilder.  This makes it especially more difficult because they can't be reused (say if there was a DefaultStoreConfiguration(Builder) because of the annotations required by the StoreConfiguration (i.e. ConfigurationFor ).  

The other option might be to allow custom cache stores to define the class to the StoreConfigurationBuilder.  This would still require a builder for each store.  Also, this would require a  few changes in the Parser as the first step to the parsingStore method is reflectively creating the Store (even though it isn't used besides a null check).  The upside to this approach is that it wouldn't require any additional annotation and the code would just have two paths for whether the configuration specified an actual store or a StoreConfigurationBuilder.
                  
> Missing logic to add the store to the configuration for custom stores
> ---------------------------------------------------------------------
>
>                 Key: ISPN-3948
>                 URL: https://issues.jboss.org/browse/ISPN-3948
>             Project: Infinispan
>          Issue Type: Bug
>          Components: Configuration, Loaders and Stores
>    Affects Versions: 6.0.0.Final
>            Reporter: Kurt Lehrke
>            Assignee: Dan Berindei
>            Priority: Critical
>
> Looking at the Parser60 class, if you specify a custom store that isn't a singleFileStore or a clusterLoader, the store doesn't get added to the configuration.   I found this when I specified my own CacheLoader and it wasn't instantiated like expected.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira