On Jan 21, 2010, at 10:32 PM, Thomas Mortagne wrote:
On Thu, Jan 21, 2010 at 22:23, Caleb James DeLisle <[email protected]> wrote:
I am adding a new captcha component, it uses jcaptcha-2.0-alpha1 and supports image captchas and sound captchas (freetts jars must be added for sound captcha support)
There is a captcha plugin in xwiki-core which makes the core depend on jcaptcha-all-1.0-RC3. jcaptcha-all-1.0-RC3 causes conflicts when getting classes from jcaptcha-2.0. The plugin in querstion: http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-core/src/main/j...
Here's my +1 for removing it entirely, breaking backward compatibility, lightening xwiki-core, and making the core no longer depend on jcaptcha. You may review a patch for removing the plugin here: http://jira.xwiki.org/jira/secure/attachment/16508/XWIKI-4779-removeJcaptcha...
-0 for removing captcha plugin
I don't like breaking something when it's not absolutely necessary, would it be too much work (maybe it's not even possible) to refactor xwiki-core captcha plugin to use jcaptcha-2.0 ?
I think there's some misunderstanding thomas. Caleb has developed a component-based captcha module. -1 to not remove the old captcha plugin ;) There's no point of having 2 systems that do the same thing. At worst the old plugin could be moved to retired projects in contrib but I don't even think we should do that since nobody is using the old catpcha plugin AFAIK. Plugins are meant to be removed. They're the old system that has been replaced by components. We should definitely not spend any time trying to create new plugins. In addition it would be a pain and completely unnecessary to maintain 2 modules that do the same thing but differently. Thanks -Vincent
To provide velocity access to the captcha component for checking the results of the captcha, I would like to add a VelocityContextInitializer which will add a binding called $captchaservice which will provide 3 methods:
/** * If this is false, the captcha system will still work. * It is for velocity scripts to determine whether captchas are needed. * * @return Is the captcha service enabled in the configuration. */ boolean isEnabled()
/** @return a List of the names of all registered classes implementing {@link org.xwiki.captcha.CaptchaVerifier}. */ public List<String> listCaptchaNames()
/** * Get a {@link org.xwiki.captcha.CaptchaVerifier} from the service. * * @param captchaName The name of the type of captcha, use {@link #listCaptchaNames()} for a list * @return A captcha object which can be used to check a specific answer for a given challange * @throws Exception If the named captcha doesn't exist. */ public CaptchaVerifier getCaptchaVerifier(String captchaName) throws Exception
Don't you have something more precise than Exception ? It does not seems right to me, it should be a specialized exception (org.xwiki.captcha.InvalidNameException or something). Except for unit tests a methods that throws Exception generally mean there is something wrong in the design.
I realize throwing an Exception to velocity always results in a stack trace but I see it as the lesser evil to returning a non-functional CaptchaVerifier which always returns false. Also I see no use case where the name of the captcha is not given in hardcoded velocity so this Exception is only for the velocity developers. Here's my +1 for adding this as well.
Caleb James DeLisle