Sylvain Wallez wrote: > Two suggestions regarding sourceResolver and redirector: > > SourceResolver (the Cocoon one) is a legacy requirement because we can't > change the interface of actions, generators, etc, and is used by the > Processor (for Actions) and the pipelines (for SitemapModelComponents). > Now since 2.1, the Excalibur resolver is available to every Serviceable > component. > > So why don't we simply write a wrapper that implement the legacy > interface around the looked-up excalibur resolver? That way, there's no > need to store the legacy resolver neither in Environment nor > EnvironmentHelper (and even less in the object model!)
Yes, we have this wrapper in 2.2. So, if you use the ServiceManager to lookup the SourceResolver you get this wrapper. So, we *could* lookup the SourceResolver e.g. in the ActNode and pass it to the action and have a clean solution. But :) i tried to make the environment handling (and source resolving) as fast as possible and avoid any unnecessary lookups. if you use the source resolver you get from the service manager, this source resolver has to get the current "processor" for resolving in order to get the current location of the sitemap. So, this is one lookup per resolving a source. Therefore I added the EnvironmentHelper object, that implements the SourceResolver interface as well to each sitemap processor. This instance directly resolves to the location of its sitemap. So if you pass this object into the sitemap components like actions etc. you avoid this one lookup. Now, of course this lookup doesn't cost that much, but if we can avoid it, it would be great. I still think, the easiest solution would be if the each Node that needs a resolver, knows the processor or gets this EnvironmentHelper object via IoC on tree creation. Then we have both, a clean and a fast solution. > > The Redirector is needed only within the Processor, so we can simply add > it as a property of the InvokeContext. Again, no need to clutter other > classes with processor-only concerns. > Sounds good! Carsten
