[xwiki-devs] [Proposal] Velocity Tools refactoring

Hi devs, = Introduction = With the recent addition of the CookieTool [1] as a velocity tool and the recent upgrade to velocity-tools 2.0 [2] we`ve been having some issues with the build. More precisely, the usage of CookieTool requires servlet-api to be used as dependency for xwiki-commons-velocity and requires that all modules that use it to specify servlet-api as a dependency as well. This has the negative impact of making xwiki-rendering depend on servlet-api and we don`t want that. We had this dependency before, since the pom of velocity-tools 1.4 was empty and we would only be hit by this problem if we tried to use some class from inside it that needed servlet-api in a non-web environment. For 2.0 the pom [4] is correct so either way (1.4 or 2.0) we can not fully say that xwiki-rendering does not depend on servlet-api. = Current state = Now that we added this cookietool, our current manual handling of velocity tools is to get all the registered tools, create a context, instantiate all the tools and set them into this cotext (key -> instance) and then reuse this context as base for every new Velocity context we create. This way we cache the tools across velocity contexts. This works well for singleton tools (stringtool, escapetool, etc), but not so well for per-request tools (like this cookietool is). = Proposal details = Velocity tools 2.0 has done a lot of rewriting regarding configuration and tool management so now it offers a tool manager that handles your tool instances (cached and instantiated when first needed, not a priori). Also it defines 3 scopes (application, request and session) to support various types of tools, like explained previously. Initially wanting to fix our problem with the servlet-api runtime dependency, I had hoped that this new tool manager instantiates tools only when they are first used, so it will not fail for example when xwiki-rendering uses xwiki-commons-velocity to do its job, but without calling the cookietool, even if it does not have servlet-api set up as dependency. I integrated it in XWiki [5][6] (not on master), but when I got the chance to test it, it turns out that the tool manager does a deep verification of each configured tool to see if it is properly configured and if it can be used (instantiated) so if cookietool is configured and servlet-api is not present, we still end up with a runtime exception. Not wanting to throw away the work I already did, I came up with a (not too nice) hack (line 78 [7] to 133) that basically entirely skips the verification of the configuration and takes it as it is. Of course, at runtime it would fail if it is used without the dependency, but this was intended. Now, in the meantime, Vincent came up with a workaround [8] for the problem the cookietool was causing to the xwiki-rendering build (tests actually) by simply not using the tool in the default tool configuration if (the class needed from) servlet-api was not present in the classpath. = Actual proposal = So we should end up with: 1) The current system where all tools are global (i.e. similar to tools 2.0's "application" scope) and a few tools (e.g. cookietool) might behave unexpectedly, but to fix the xwiki-rendering runtime problem: 1.1) With Vincent's less ugly fix that just does not include cookietool in the list of default tools OR 2) My refactoring that adds the benefits of velocity tools 2.0's tool manager with scopes, tool configuration and instance handling, but to fix the xwiki-rendering runtime problem: 2.1) With my ulgy fix that does not perform configuration validation 2.2) With Vincent's less ugly fix that just does not include cookietool in the list of default tools Some things to consider is whether or not we *want* to: - support *full*/*standard* velocity tool configuration. Maybe we just want to pass the tool's key and class like we do now? - support tool scopes. Maybe we just want to use singleton tools? - go ahead with velocity tools and support additional ones or just focus more on using script services instead? In our case, we can simply not use the standard cookietool and make our own script service for the things we need. However, this proposal is at xwiki-commons level so maybe it's not *only* about what we do with it in XWiki but maybe in other environments as well. I know this is a bit too much for maybe a not-that-important subject, but I`d appreciate your opinion on where we should take this. Thanks, Eduard ---------- [1] http://jira.xwiki.org/browse/XCOMMONS-627 [2] http://jira.xwiki.org/browse/XWIKI-5347 [3] http://repo1.maven.org/maven2/velocity-tools/velocity-tools/1.4/velocity-to… [4] http://repo1.maven.org/maven2/org/apache/velocity/velocity-tools/2.0/veloci… [5] https://github.com/xwiki/xwiki-commons/compare/master...velocity-tools-refa… [6] https://github.com/xwiki/xwiki-platform/compare/master...velocity-tools-ref… [7] https://github.com/xwiki/xwiki-commons/compare/master...velocity-tools-refa… [8] https://github.com/xwiki/xwiki-commons/commit/6f8509e0ac3d5250331a6d2073300…