You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Marcus Crafter <cr...@fztig938.bank.dresdner.net> on 2002/03/15 13:33:01 UTC
Proposal: new style selector
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
' /( &&&
\_&&&&'
&&&&.
&&&&&&&:
Re: Proposal: new style selector
Posted by Marcus Crafter <cr...@fztig938.bank.dresdner.net>.
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
Re: Proposal: new style selector
Posted by Sylvain Wallez <sy...@anyware-tech.com>.
Torsten Curdt wrote:
>While going through my inbox I found this post...
>...did anybody comment on this? If so I must have missed
>that. Anyway, I think this proposal sounds reasonable...
>
>Other comments?
>
We decided to postpone this until 2.0.2 is out, since it has some
impacts on sitemap engine(s).
Sylvain
--
Sylvain Wallez
Anyware Technologies Apache Cocoon
http://www.anyware-tech.com mailto:sylvain@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: Proposal: new style selector
Posted by Torsten Curdt <tc...@dff.st>.
While going through my inbox I found this post...
...did anybody comment on this? If so I must have missed
that. Anyway, I think this proposal sounds reasonable...
Other comments?
--
Torsten
<snip/>
> 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.
<snip/>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org