cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ricardo Rocha <rica...@apache.org>
Subject SQL Logicsheet Bug (was: another bug report)
Date Fri, 24 Mar 2000 15:46:33 GMT
Note:
  prior to writing this answer I took care of examining all
  previous xsp code changes. I also checked thoroughly  
  how current logicsheet utility templates work including,
  of course,  those used in the sql.xsl logicsheet.

  According to my findings, the reported problem does
  not originate in xsp code (whether old or recent) nor in
  the core xsp logicsheets, but in a misunderstanding of
  how nested content processing works in logicsheets.

  I feel partly responsible for this misunderstanding as
  I've failed to produce adequate documentation about
  using xslt for transforming markup to code and also
  because I've been unable to reply to bug reports in a
  timely fashion. I apologize for this inconvenience. I'm
  currently working on such needed documentation as
  well as taking steps to ensure reliable Internet access
  from my current location.

  This note contains both an explanation of the problem
  and a suggestion to avoid it in a general way.

Donald Ball wrote:
>   <sql:query>select * from foo_table where id =
>    <request:get-parameter name="id"/> order by id
>   </sql:query>
> and the java source code would end up looking like this:
>   String.valueOf("select * from foo_table where id = "+
>     request.getParameter(String.valueOf("id"))+
>     " order by id")
> now, all I get is:
>   String.valueOf(
>     request.getParameter(String.valueOf("id"))
>   )

There's a mistake here: the above <sql:query> could have
_never_ produced a string concatenation like:

    "select * from foo_table where id = " +
    request.getParameter(String.valueOf("id")) +
    " order by id"

because it (like most other xsp tags) has always been based
on the "get-nested-content" utility template,  which does _not_
provide for string concatenation.

The perception that this particular markup-to-code transformation
used to work but now does not because of changes to xsp
is incorrect.

The "get-nested-content" utility template (widely used in all
xsp logicsheets) provides a means of recursively expanding
nested dynamic tags so that such tags can be mixed and
interspersed as dictated by the application needs.

Thus, for example:

    <util:get-file-contents>
      <util:name>
        <request:get-parameter name="myFilename"/>
      </util:name>
    </util:get-file-contents>

nests a <request> [dynamic] tag inside a <util> tag resulting
in the following Java code:

    // This comes from <util:get-file-contents>
    XSPUtil.getFileContents(
      XSPUtil.relativeFileName(
        String.valueOf(
          // And this comes from <request:get-parameter>
          String.valueOf(request.getParameter("myFilename")),
           request,
          (ServletContext) context
        )
      )
    );

The "magic" behind this is the "get-nested-content" utility
template, which is used by <util:get-file-contents> as follows:

  <xsl:template match="util:get-file-contents">
    <xsl:variable name="name">
      <xsl:choose>
        <xsl:when test="@name">"<xsl:value-of select="@name"/>"</xsl:when>
        <xsl:when test="util:name">
          <xsl:call-template name="get-nested-content">
            <xsl:with-param name="content" select="util:name"/>
          </xsl:call-template>
        </xsl:when>
      </xsl:choose>
    </xsl:variable>
    <xsp:expr>
      XSPUtil.getFileContents(
        XSPUtil.relativeFilename(
          String.valueOf(<xsl:copy-of select="$name"/>),
            request,
            (ServletContext) context
        )
      )
    </xsp:expr>
  </xsl:template>

Here, the "name" parameter is first checked as a static
attribute to <util:get-file-contents> and, failing that, as a
dynamic, nested element.

This discipline allows xsp authors to use a simpler, more
intuitive attribute notation to pass a constant filename
(when it's known at page authoring time)  or the more
general, nested element notation when the filename
parameter is computed at request time.

It's when parameters themselves are dynamic (that is,
provided at request time) that "get-nested-content" comes
into play: it's used to recursively expand dynamic content
to be passed as a parameter value.

In its current form, though, "get-nested-content" has a 
fundamental limitation: it will process enclosed content as
_either_ nested dynamic tag(s) _or_ as constant string(s).

If parameter values mix constant strings and dynamic tags,
"get-nested-content" will take into account _elements_
only and will discard any intervening text. This can be
appreciated in the current template definition:

  <xsl:template name="get-nested-content">
    <xsl:param name="content"/>
    <xsl:choose>
      <xsl:when test="$content/*">
        <xsl:apply-templates select="$content/*"/>
      </xsl:when>
      <xsl:otherwise>"<xsl:value-of select="$content"/>"</xsl:otherwise>
    </xsl:choose>
  </xsl:template>

Why doesn't "get-nested-content" treat text as constant
strings and nested dynamic tags as string expressions?
Why doesn't it concatenate such strings to yield a single
string expression?

The reason is that content returned by nested dynamic
tags is _not_ limited to the "String" Java type. It can be
_any_ Java type. It would be overly restrictive to limit
arguments to method calls and/or expressions resulting
from dynamic tag expansion to be only of type "String".

Thus, a valid way of rephrasing the above <sql:query>
dynamic tag would be:

    <sql:query>
      <xsp:expr>
        "select *" +
        "from foo_table" +
        "where id = " +
           <request:get-parameter name="id"/> +
         "order by id"
        </xsp:expr>
      </sql:query>

Granted, granted!!! This is way too verbose and forces
non-programmers to understand and use Java syntax.
Put briefly: it sucks!

In order to support the cleaner form:

  <sql:query>
    select *
    from    foo_table
    where id =
      <request:get-parameter name="id"/>
    order by id
  </sql:query>

the logicsheet author must go the extra mile and use
a modified version of the "get-nested-content" that
not only provides for recursive expansion, but also
ensures that all inner nodes are treated as strings and
properly concatenated.

There are many ways to achieve this. For example, a
string-only, modified template may look like:

  <xsl:template name="get-nested-string">
    <xsl:param name="content"/>
    <xsl:choose>
      <xsl:when test="$content/*">
        ""
        <xsl:for-each select="$content/node()">
          <xsl:choose>
            <xsl:when test="name(.)">
              + <xsl:apply-templates select="."/>
            </xsl:when>
	<xsl:otherwise> <!-- text()? -->
             + "<xsl:value-of select="normalize-space(translate(.,'&#9;&#10;&#13;',
'   '))"/>" 
            </xsl:otherwise>
          </xsl:choose>
        </xsl:for-each>
      </xsl:when>
      <xsl:otherwise>"<xsl:value-of select="$content"/>"</xsl:otherwise>
    </xsl:choose>
  </xsl:template>

Here, we test each nested node to determine whether it's an element
(presumably a dynamic tag) or not (presumably, constant text). Note that
we map tabs, newlines and carriage returns to blanks to ensure that
syntactically valid string constants are generated. Note also the leading
empty string: it guarantees that a "correct" string expression is generated
even in absence of actual nested content (btw, it also relieves us from the
burden of checking whether a concatenation operator should be generated
or not, :-))

Donald Ball wrote:
> this is a pretty big bug since it makes it almost impossible for taglibs
> to play with each other. Anyone got a clue?

As explained above, this is not a bug but the result of "get-nested-content"
results not being coerced to String.

Stefano Mazzocchi wrote:
> Ouch, damn it.
> Could you backtrace the XSP commits and see what broke it? Damn, we need
> to rewrite that code... without Ricardo around, it's impossible to
> continue this way...

Again, this problem does not arise from bugs or changes in xsp
code. I do apologize, though, for not being around lately, :-(

Donald Ball wrote:
> Gee, looking over it recently, all I see is Ricardo doing some
> changes, adding XSPLogicSheet and related classes, modified DOM handler
> methods... that's about it.

These changes were made to provide request-time logicsheet
reloading and are not related to dynamic tag expansion.

A _big_ problem with the previous xsp version was that the servlet
engine had to be restarted for logicsheet changes to be visible,
clearly a pain in the ass (tm).

It's fixed now: logicsheet changes are now detected and result in
the automatic recompilation and reloading of affected xsp pages.

Btw, in addition to the original namespace-based logicsheet
inclusion mechanism, logicsheets can now be declared by
means of a processing instruction (a la xslt):

  <?xml-logicsheet href="logicsheet.xsl"?>

> It seems like the problem should be in xsp-java.xsl, but that
> stylesheet hasn't been touched. Looking at it carefully, I can't
> really see how it _ever_ worked, but hey, what do I
> know?

I know that xslt-based markup-to-code translation is a tricky 
business, but once you understand its ramifications it's really
easy to understand how it works.

> All I know is that it's darned frustrating trying to work with
> unmaintained code that's not really documented at all.

You're right, Donald. We need more documentation on
xslt-based code generation. My fault, :-(

> Actually, I'm lying... upon further inspection, it seems that the
> XSPLogicSheet stuff is clearly related to the taglib/namespace reaction,
> since each taglib has its own logicsheet that does the transformation.  

Btw, logicsheets are not tied to namespaces anymore.

Logicsheets explicitly declared by means of a <?xml-logicsheet?>
pi are not restricted to any given namespace.

Logicsheets declared in "cocoon.properties" as associated
with a given namespace continue to be pre-loaded an will be
applied according to the original rules.

> Looks like the bug crept in here:
> XSPProcessor.java
> revision 1.12
> date: 2000/03/13 21:26:27;  author: ricardo;  state: Exp;  lines: +262
> -119
> Added support for request-time logicsheet reloading. Not final: a lot of
> refactoring follows

No. There's no bug, there were no adverse changes. Only lack
of communication. Again, my fault :-((

> damned if i can figure out how to fix it, though, except by rolling back
> the changes. i think i could tease it apart given a few hours, but that's
> not a luxury i have.

Please, don't rollback any changes. Xsp is working fine!

> maybe if we all pool our money together we can buy ricardo a citizenship
> in belize or switzerland so he can get back on the 'net and fix this. :)

Hmmm... I tend to think of myself as a citizen of the world. A "netizen" sounds
even better, :-)

Now, I'm a Colombian, too. It seems this isn't a fashionable nationality these
days...

But, believe me, Colombia is a country full of honest, hard-working and
intelligent people. Even in the middle of this absurd war, I wouldn't buy
another citizenship, thanks, :-)

Regards,

Ricardo

Mime
View raw message