On Tue, Dec 11, 2012 at 8:02 AM, David Riccitelli <[email protected]> wrote: > Yes, that sounds right. > > As for the engines, I am just not sure that we shall re-open all the > existing ones and update them. I think we shall maintain some kind of > backward compatibility. > > We could probably accomplish that by refactoring the > 'AbstractEnhancementEngine' abstract class (which I believe is used by > almost every engine?): > - adding the two methods: > -- int canEnhance(ContentItem ci, Map<String, Object> > enhancementContext) > -- void computeEnhancements(ContentItem ci, Map<String, Object> > enhancementContext) > > that: > - will store the enhancementContext locally and > - provide access to it via a ' getEnhancementContext' method and > - call the single-argument counterpart ( canEnhance( ci ) and > computeEnhancements( ci ) ). >
This would not work. EnhancementEngine instances are called concurrently by different threads to enhance different ContentItems with different enhancement contexts. So storing those information within a member variable is not feasible. So while we could provide such an adapter like functionality it would need to throw away the parsed enhancement context. > New or existing engines that want to take advantage of the configuration > map, will be able to either override the above, or use the > getEnhancementContext. > > What do you think? > Adapting existing Engines to the proposed API change requires to change the signature and than ignoring the parsed EnhancementContext. So if we do it after a release the impact should be really minimal to Developers that have already implemented their own engines. Because of that I would opt for breaking compatibility rather that adding additional complexity only to keep compatibility with outdated versions best Rupert -- | Rupert Westenthaler [email protected] | Bodenlehenstraße 11 ++43-699-11108907 | A-5500 Bischofshofen
