cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus Crafter <craft...@fztig938.bank.dresdner.net>
Subject Re: Proposal: new style selector
Date Mon, 08 Apr 2002 13:56:09 GMT
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:haul@informatik.tu-darmstadt.de">Christian Haul</a>
>   * @author <a href="mailto:sylvain@apache.org">Sylvain Wallez</a>
>   * @author <a href="mailto:vgritsenko@apache.org">Vadim Gritsenko</a>
> + * @author <a href="mailto:crafterm@managesoft.com">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: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org

-- 
        .....
     ,,$$$$$$$$$,      Marcus Crafter
    ;$'      '$$$$:    Computer Systems Engineer
    $:         $$$$:   ManageSoft GmbH
     $       o_)$$$:   82-84 Mainzer Landstrasse
     ;$,    _/\ &&:'   60327 Frankfurt Germany
       '     /( &&&
           \_&&&&'
          &&&&.
    &&&&&&&:

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Mime
View raw message