Hi All,
Now that the release is out the door, I'd like to re-raise the
extended selector proposal I brought up with Sylvain a few weeks
back. It's in Bugzilla under #7244, and the initial email I sent
out is attached below.
Any thoughts, comments, questions ?
Cheers,
Marcus
On Fri, Mar 15, 2002 at 01:33:01PM +0100, Marcus Crafter wrote:
> Hi All,
>
> Hope all is well.
>
> Sylvain and I have recently had a discussion about the current
> selector implementation. Here's the first email from the
> conversation:
>
> --------------------------------------------------------------------------------
>
> Hi Sylvain!
>
> Hope all is well mate.
>
> I remember a while back (>6 months) on cocoon-dev we discussed the
> semantics of map:select and it's current implementation.
>
> I remember you comments were similar to mine, and I'm
> trying to catch up with where this discussion went (if
> anywhere), as I've been off in project land for the past few
> months.
>
> My current (perhaps naive) opinion is that map:select is implemented
> wrong, and causes confusion among new cocoon application developers.
>
> The current implementation converts:
>
> <map:select type="myselector">
> <map:when test="this">...</map:when>
> <map:when test="that">...</map:when>
> </map:select>
>
> to:
>
> if (myselector.select("this"...)) {
> ...
> } else if (myselector.select("that"...)) {
> ...
> }
>
> This seems to be contrary to the semantics the xml implies (and
> can cause unwanted behaviour) :
>
> I think many might expect generated java code to behave like:
>
> Object result = myselector.select(...);
>
> if (result.equals("this")) {
> ...
> } else if (result.equals("that")) {
> ...
> }
>
> more like a expanded switch statement.
>
> ie. the test case is done only once, and its return value is
> checked multiple times. Not only is this better performance
> wise, but prevents unwanted behaviour. eg:
>
> I found this out when writing a login selector.
>
> I had something like:
>
> <map:select type="login">
> <map:when test="permitted">...</>
> <map:when test="denied">...</>
> <map:otherwise>...</>
> </map:select>
>
> which in practice tried to log denied users in 2 times!
>
> This means one has to do something like:
>
> <map:act type="login"/>
> <map:select type="userstatus">
> <map:when test="permitted">...</>
> <map:when test="denied">...</>
> <map:otherwise>...</>
> </map:select>>
> </map:act>
>
> ie. combination of action and then selector, which I find to be
> a hack.
>
> What do you think ? Has this already been covered on cocoon-dev in
> recent times ?
>
> Cheers,
>
> Marcus
>
> --------------------------------------------------------------------------------
>
> After some thinking we came up with an idea to create
> an extended selector interface which supports switch-style semantics
> rather than if-if-else semantics.
>
> The newer interface supports switch-style semantics by allowing
> a developer to create a 'testable' context before any of the
> <map:when> tests have been done. This context can then be used in
> select() to see if the current test yield a true value.
>
> The newer interface looks like:
>
> public interface ContextualizableSelector extends Selector, ThreadSafe
> {
> Object getSelectorContext(Map objectModel, Parameters parameters);
> boolean select(String expression, Object selectorContext);
> }
>
> There's an Abstract class to support the older style:
>
> public abstract class AbstractContextualizableSelector extends AbstractLoggable
> implements ContextualizableSelector
> {
> public boolean select(
> String expr, Map objectModel, Parameters params
> )
> {
> return select(expr, getSelectorContext(objectModel, params));
> }
> }
>
> A sample implementation would be like:
>
> public class RequestParameterSelector extends AbstractContextualizableSelector
> implements Configurable
> {
> public Object getSelectorContext(Map objectModel, Parameters params)
> {
> <snip>
> String name = params.getParameter("parameter-name", this.defaultName);
> return ObjectModelHelper.getRequest(objectModel).getParameter(name);
> }
>
> public boolean select(String expression, Object selectorContext)
> {
> return selectorContext.equals(expression);
> }
> }
>
> Ok, it's a simple example, but for selectors that calculate more
> complex conditionals, or do some thing like attempt to log in a
> user for example, the getSelectorContext(...) would give them a
> huge saving.
>
> The current files use Contextualizable as the prefix which we're
> not so happy with as a name as it might confuse people with
> avalon's context or the cocoon environment context. If anyone has a
> better idea please let us know. So far the only alternative name has
> been SwitchStyleSelector (?)
>
> I've put together some diffs which are attached (updated
> sitemap.xsl, interfaces and abstract class, and a modified
> RequestParameterSelector as a sample). We'd be interested in your
> comments.
>
> Cheers,
>
> Marcus
>
> --
> .....
> ,,$$$$$$$$$, Marcus Crafter
> ;$' '$$$$: Computer Systems Engineer
> $: $$$$: ManageSoft GmbH
> $ o_)$$$: 82-84 Mainzer Landstrasse
> ;$, _/\ &&:' 60327 Frankfurt Germany
> ' /( &&&
> \_&&&&'
> &&&&.
> &&&&&&&:
> ? src/java/org/apache/cocoon/selection/AbstractContextualizableSelector.java
> ? src/java/org/apache/cocoon/selection/ContextualizableSelector.java
> Index: src/java/org/apache/cocoon/components/language/markup/sitemap/java/sitemap.xsl
> ===================================================================
> RCS file:
>/home/cvspublic/xml-cocoon2/src/java/org/apache/cocoon/components/language/markup/sitemap/java/sitemap.xsl,v
> retrieving revision 1.10
> diff -u -r1.10 sitemap.xsl
> --- src/java/org/apache/cocoon/components/language/markup/sitemap/java/sitemap.xsl
> 1 Mar 2002 15:09:04 -0000 1.10
> +++ src/java/org/apache/cocoon/components/language/markup/sitemap/java/sitemap.xsl
> 15 Mar 2002 11:36:14 -0000
> @@ -193,6 +193,7 @@
> import org.apache.cocoon.matching.Matcher;
> import org.apache.cocoon.matching.PreparableMatcher;
> import org.apache.cocoon.selection.Selector;
> + import org.apache.cocoon.selection.ContextualizableSelector;
> import org.apache.cocoon.sitemap.AbstractSitemap;
> import org.apache.cocoon.components.pipeline.StreamPipeline;
> import org.apache.cocoon.components.pipeline.EventPipeline;
> @@ -242,16 +243,36 @@
> /**
> * Method that handles selectors.
> */
> - private boolean isSelected(String hint, String testValue, Parameters params,
>Map objectModel) throws Exception {
> - Selector selector = (Selector)this.selectors.select(hint);
> + private boolean isSelected(String hint, String testValue, Parameters params,
>Map objectModel, Object selectorContext) throws Exception {
> + Component selector = (Selector)this.selectors.select(hint);
> try {
> - return selector.select(testValue, objectModel, params);
> + if (selector instanceof ContextualizableSelector)
> + return ((ContextualizableSelector) selector).select(
> + testValue, selectorContext
> + );
> + else // support normal selector
> + return ((Selector) selector).select(testValue, objectModel, params);
> } finally {
> this.selectors.release(selector);
> }
> }
>
> /**
> + * Method that handles selectors.
> + */
> + private Object getSelectorContext(String hint, Map objectModel, Parameters
>params) throws Exception {
> + Component selector = this.selectors.select(hint);
> + try {
> + if (selector instanceof ContextualizableSelector)
> + return ((ContextualizableSelector)
>selector).getSelectorContext(objectModel, params);
> + } finally {
> + this.selectors.release(selector);
> + }
> + // non-ContextualizableSelector
> + return null;
> + }
> +
> + /**
> * Method that handles matchers.
> */
> private Map matches(String hint, Object preparedPattern, String pattern,
>Parameters params, Map objectModel) throws Exception {
> @@ -959,6 +980,11 @@
> </xsl:choose>
> </xsl:variable>
>
> + Object selectorContext =
> + getSelectorContext(
> + "<xsl:value-of select="$selector-type"/>", objectModel, <xsl:value-of
>select="$component-param"/>
> + );
> +
> <!-- loop through all the when cases -->
> <xsl:for-each select="map:when">
> <!-- remove all invalid chars from the test expression. The result is used to
>form the name of the generated method
> @@ -974,7 +1000,7 @@
> <xsl:call-template name="line-number"/>
> <xsl:if test="position() > 1">
> else </xsl:if>
> - if (isSelected("<xsl:value-of select="$selector-type"/>",
><xsl:apply-templates select="@test"/>, <xsl:value-of select="$component-param"/>,
>objectModel)) {
> + if (isSelected("<xsl:value-of select="$selector-type"/>",
><xsl:apply-templates select="@test"/>, <xsl:value-of select="$component-param"/>,
>objectModel, selectorContext)) {
> if (debug_enabled) getLogger().debug("Select <xsl:value-of
>select="$selector-type"/> Test <xsl:value-of
>select="XSLTFactoryLoader:escape($factory-loader, @test)"/>");
> <xsl:apply-templates/>
> }
> Index: src/java/org/apache/cocoon/selection/RequestParameterSelector.java
> ===================================================================
> RCS file:
>/home/cvspublic/xml-cocoon2/src/java/org/apache/cocoon/selection/RequestParameterSelector.java,v
> retrieving revision 1.4
> diff -u -r1.4 RequestParameterSelector.java
> --- src/java/org/apache/cocoon/selection/RequestParameterSelector.java 22 Feb
>2002 07:03:54 -0000 1.4
> +++ src/java/org/apache/cocoon/selection/RequestParameterSelector.java 15 Mar
>2002 11:36:15 -0000
> @@ -53,7 +53,6 @@
> import org.apache.avalon.framework.configuration.Configurable;
> import org.apache.avalon.framework.configuration.Configuration;
> import org.apache.avalon.framework.configuration.ConfigurationException;
> -import org.apache.avalon.framework.logger.AbstractLoggable;
> import org.apache.avalon.framework.parameters.Parameters;
> import org.apache.avalon.framework.thread.ThreadSafe;
>
> @@ -74,10 +73,11 @@
> * @author <a href="mailto:[EMAIL PROTECTED]";>Christian Haul</a>
> * @author <a href="mailto:[EMAIL PROTECTED]";>Sylvain Wallez</a>
> * @author <a href="mailto:[EMAIL PROTECTED]";>Vadim Gritsenko</a>
> + * @author <a href="mailto:[EMAIL PROTECTED]";>Marcus Crafter</a>
> * @version CVS $Id: RequestParameterSelector.java,v 1.4 2002/02/22 07:03:54
>cziegeler Exp $
> */
> -public class RequestParameterSelector extends AbstractLoggable
> - implements Configurable, ThreadSafe, Selector {
> +public class RequestParameterSelector extends AbstractContextualizableSelector
> + implements Configurable {
>
> protected String defaultName;
>
> @@ -85,20 +85,24 @@
> this.defaultName = config.getChild("parameter-name").getValue(null);
> }
>
> - public boolean select(String expression, Map objectModel, Parameters
>parameters) {
> + public Object getSelectorContext(Map objectModel, Parameters parameters) {
> +
> String name = parameters.getParameter("parameter-name", this.defaultName);
>
> if (name == null) {
> getLogger().warn("No parameter name given -- failing.");
> - return false;
> + return null;
> }
>
> - String value = ObjectModelHelper.getRequest(objectModel).getParameter(name);
> - if (value == null) {
> - getLogger().debug("Request parameter '" + name + "' not set --
>failing.");
> + return ObjectModelHelper.getRequest(objectModel).getParameter(name);
> + }
> +
> + public boolean select(String expression, Object selectorContext) {
> + if (selectorContext == null) {
> + getLogger().debug("Request parameter '" + selectorContext + "' not set
>-- failing.");
> return false;
> }
>
> - return value.equals(expression);
> + return selectorContext.equals(expression);
> }
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]
--
.....
,,$$$$$$$$$, Marcus Crafter
;$' '$$$$: Computer Systems Engineer
$: $$$$: ManageSoft GmbH
$ o_)$$$: 82-84 Mainzer Landstrasse
;$, _/\ &&:' 60327 Frankfurt Germany
' /( &&&
\_&&&&'
&&&&.
&&&&&&&:
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
