Hi everyone, I've created a first draft of refactoring chain so that it uses a facade for configuration.
Please see the diff attached to this ticket to get my proposed changes: https://issues.apache.org/jira/browse/CHAIN-72 Here is a summary of what I have done: * Removed the module configuration * Created a new module called noop-configuration * Created a new module called xml-configuration * We now compile the web module against the noop-configuration with noop-configuration being scoped as provided in the pom.xml. * We check on class instantiation inside ChainServlet and ChainListner to validation that we have a valid configuration module present. * The ConfigParser class has a new constructor added called: ConfigParser(String ruleSet, ClassLoader loader) - this allows a ruleset class to be loaded by the classloader without tightly coupling the code against digester. Because we detect if the ruleset classname is specified in the servlet config and if it is we pass it to our constructor, otherwise we use the default constructor. Now we can remove the digester dependency from the web module. Comments: I still don't like how we store data in map with a classloader as a key. I understand that we can an implementation that has a factory per classloader, but this seems like a poor man's dependency injection. See: CatalogFactory.java:181 - public static <K, V, C extends Map<K, V>> CatalogFactory<K, V, C> getInstance() { Anyways, I would love to hear all of your thoughts regarding these changes. Thanks, -Elijah On Tue, Jul 24, 2012 at 12:43 PM, Oliver Heger <oliver.he...@oliver-heger.de> wrote: > Hi Simo and Elijah, > > Am 23.07.2012 21:55, schrieb Simone Tripodi: > >> Thanks a lot Oliver, much more than appreciated! >> >> If you could have a look at current configuration stuff at [chain2] >> and share what you think would be already *great*! > > > I had a look at the current config module. I understand Elijah's concerns > about a redesign of this package because the API indeed seems to be tightly > coupled to Digester. > > IMHO - and IIUC this is also the direction in which you are going - the > underlying library used for parsing XML configurations should not be visible > in the public API of the parser component. So you would have methods like > > Chain parseChain(URL url); > Catalog parseCatalog(URL url); > > Then the parsing library is an implementation detail and can be replaced if > necessary. > > One word about using [configuration]: Note that the philosophy of > [configuration] is pretty much different from [digester]. [digester] is able > - based on its rules - to parse a source file and transform it into a target > in-memory representation in a single step. [configuration] in contrast first > parses the file and creates an internal in-memory representation. Then you > have to evaluate this model (e.g. using XPath or a simplified syntax for > accessing hierarchal structures) and do the conversion yourself. So for the > use case of creating Chain objects from XML documents [digester] may be > better suited because the manual transformation step is not necessary. > > But in any case, the first step is to define the API of the configuration > parser. Then we can think about implementation strategies. > > Oliver > > >> >> then, feel free to put your hands and help us on defining the façade :) >> >> alles gute, >> -Simo >> >> http://people.apache.org/~simonetripodi/ >> http://simonetripodi.livejournal.com/ >> http://twitter.com/simonetripodi >> http://www.99soft.org/ >> >> >> On Mon, Jul 23, 2012 at 9:43 PM, Oliver Heger >> <oliver.he...@oliver-heger.de> wrote: >>> >>> Am 23.07.2012 09:00, schrieb Simone Tripodi: >>> >>>> Good morning all, >>>> >>>> so I continue proposing the already proposed roadmap: let's add the >>>> façade APIs for the [chain] configuration stuff, adapt the existing >>>> XML configuration reader, use the [configuration] in future releases >>>> for new [chain] configurations. >>>> How does it sound? >>> >>> >>> +1 >>> >>> If I can support you, let me know. >>> >>> @Elijah: There is a feature request for adding support for YAML [1]. >>> IIRC, >>> it was planned as a Google Summer of Code project, but it did not >>> succeed. >>> >>> Oliver >>> >>> [1] https://issues.apache.org/jira/browse/CONFIGURATION-201 >>> >>> >>>> >>>> best, >>>> -Simo >>>> >>>> http://people.apache.org/~simonetripodi/ >>>> http://simonetripodi.livejournal.com/ >>>> http://twitter.com/simonetripodi >>>> http://www.99soft.org/ >>>> >>>> >>>> On Mon, Jul 23, 2012 at 7:01 AM, Elijah Zupancic <eli...@apache.org> >>>> wrote: >>>>> >>>>> >>>>> Hi Oliver, >>>>> >>>>> Configuration seems like it might be useful if I end up redoing the >>>>> XML configuration portion. Are there any plans to support YAML? >>>>> >>>>> Thanks, >>>>> -Elijah >>>>> >>>>> On Sun, Jul 22, 2012 at 1:16 PM, Oliver Heger >>>>> <oliver.he...@oliver-heger.de> wrote: >>>>>> >>>>>> >>>>>> Hi Simo, >>>>>> >>>>>> Am 22.07.2012 17:54, schrieb Simone Tripodi: >>>>>> >>>>>>> Good point Oliver, >>>>>>> >>>>>>> I honestly didn't think about [configuration], please apologize! >>>>>>> since >>>>>>> [chain] already had a way to be configured via an XML wrapper around >>>>>>> the Digester, we thought it would have been good having a façade and >>>>>>> plug other textual format... >>>>>>> >>>>>>> Anyway, we are open to suggestions - what would fit better for you? >>>>>>> Do >>>>>>> you already have some hints to share? >>>>>>> >>>>>>> Many thanks in advance, all the best! >>>>>>> -Simo >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> nothing concrete. I saw that you mentioned an XML configuration >>>>>> module, >>>>>> and >>>>>> [configuration] contains a XMLConfiguration class. It also supports >>>>>> other >>>>>> configuration file formats, e.g. properties or ini files which can be >>>>>> accessed through the same Configuration interface. >>>>>> >>>>>> I don't know your concrete use cases. If you already have a working >>>>>> implementation based on Digester, there is probably not much benefit >>>>>> in >>>>>> switching to another API. But if you plan support for other >>>>>> configuration >>>>>> file formats, [configuration] may be worth a look. >>>>>> >>>>>> Oliver >>>>>> >>>>>> >>>>>>> >>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>> http://simonetripodi.livejournal.com/ >>>>>>> http://twitter.com/simonetripodi >>>>>>> http://www.99soft.org/ >>>>>>> >>>>>>> >>>>>>> On Sun, Jul 22, 2012 at 4:49 PM, Oliver Heger >>>>>>> <oliver.he...@oliver-heger.de> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Is there any relation or overlap to [configuration]? >>>>>>>> >>>>>>>> Depending on your concrete requirements, this is probably >>>>>>>> over-sized. >>>>>>>> But >>>>>>>> maybe a source of inspiration? >>>>>>>> >>>>>>>> Oliver >>>>>>>> >>>>>>>> Am 22.07.2012 10:00, schrieb Elijah Zupancic: >>>>>>>> >>>>>>>>> Thanks! >>>>>>>>> >>>>>>>>> On Sat, Jul 21, 2012 at 11:54 PM, Simone Tripodi >>>>>>>>> <simonetrip...@apache.org> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Just tracked CHAIN-72 and assigned to Elijah >>>>>>>>>> >>>>>>>>>> best, >>>>>>>>>> -Simo >>>>>>>>>> >>>>>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>>>>> http://simonetripodi.livejournal.com/ >>>>>>>>>> http://twitter.com/simonetripodi >>>>>>>>>> http://www.99soft.org/ >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Sat, Jul 21, 2012 at 11:27 PM, Simone Tripodi >>>>>>>>>> <simonetrip...@apache.org> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Hi all guys, >>>>>>>>>>> >>>>>>>>>>> Elijah and and I had a chat and we thought that, since chain2 >>>>>>>>>>> hasn't >>>>>>>>>>> released yet, we are still in time to define a façade API for >>>>>>>>>>> textual >>>>>>>>>>> configurations and rename the current XML configuration module to >>>>>>>>>>> xml-configuration (and adapt it to new API) - new formats such as >>>>>>>>>>> YAML >>>>>>>>>>> and JSON will be included in future releases. >>>>>>>>>>> Any objection? >>>>>>>>>>> >>>>>>>>>>> @Elijah: if you have cycles to dedicate to it, I think the façade >>>>>>>>>>> can >>>>>>>>>>> be defined in a o.a.c.chain.config package in the core module, in >>>>>>>>>>> that >>>>>>>>>>> way it should be quick enough going to the first release - WDYT? >>>>>>>>>>> >>>>>>>>>>> TIA! >>>>>>>>>>> -Simo >>>>>>>>>>> >>>>>>>>>>> http://people.apache.org/~simonetripodi/ >>>>>>>>>>> http://simonetripodi.livejournal.com/ >>>>>>>>>>> http://twitter.com/simonetripodi >>>>>>>>>>> http://www.99soft.org/ >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> --------------------------------------------------------------------- >>>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> --------------------------------------------------------------------- >>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> --------------------------------------------------------------------- >>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>> >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>> >>>>>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>> >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
