Tapestry 5
  1. Tapestry 5
  2. TAP5-713

Change template parser to not use StAX, as it is not (yet) compatible with Google App Engine

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.1.0.5
    • Fix Version/s: 5.2.0
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      The StAX APIs are not on the GAE "white list".

      Should be reasonable ot change the code, by using a SAX parser that parses the template into a list of tokens, and then iterate down the token list as we do today using StAX. End result will be fewer dependencies to boot.

      1. SaxTemplateParserImpl.java
        21 kB
        Christian Köberl
      2. SaxTemplateParserImpl.java
        23 kB
        Christian Köberl
      3. SaxTemplateParserImpl.java
        30 kB
        Christian Köberl
      4. SaxTemplateParserImpl.java
        30 kB
        Christian Köberl
      5. SaxTemplateParserImpl.java
        31 kB
        Christian Köberl

        Activity

        Hide
        Igor Drobiazko added a comment -

        +1 for it.
        The woodstox's approach to FactoryFinder is a mess. It causes a lot of class loading problems in OSGi. I spent a lot of time migrating 5.0 Tapestry-OSGi bundle to 5.1. We should drop this dependency.

        Show
        Igor Drobiazko added a comment - +1 for it. The woodstox's approach to FactoryFinder is a mess. It causes a lot of class loading problems in OSGi. I spent a lot of time migrating 5.0 Tapestry-OSGi bundle to 5.1. We should drop this dependency.
        Hide
        Tatu Saloranta added a comment - - edited

        Woodstox does not use FactoryFinder for anything. Only thing that does is the Stax API itself.
        Nothing to do with Woodstox; same would apply to JDK6-bundled Stax implementation (Sun Sjsxp). Ditto for OSGi: Stax/SAX/JAXP all use introspection mechanism that does not work well with OSGi. This is the same for all implementations.

        One thought regarding nature of dependency: maybe it'd be good to make stax implementation a "provided" dependency: provided by platform or added by users. That way it could use whichever implementation user chose; either the best, or one that container or JDK provides.

        And finally – if and when there are concerns about Woodstox, would it kill you to maybe quickly contact Woodstox project team? Who knows, that could even help resolve issues.
        It is rather frustrating to find that there's enough time to complain, and hack around things, but apparently not enough time to collaborate. Most open source packages have competent and friendly maintainers, who can be concated to help resolve issues (or point out that issues are elsewhere as the case may be); or at least would be interested in learning of such issues.

        Show
        Tatu Saloranta added a comment - - edited Woodstox does not use FactoryFinder for anything. Only thing that does is the Stax API itself. Nothing to do with Woodstox; same would apply to JDK6-bundled Stax implementation (Sun Sjsxp). Ditto for OSGi: Stax/SAX/JAXP all use introspection mechanism that does not work well with OSGi. This is the same for all implementations. One thought regarding nature of dependency: maybe it'd be good to make stax implementation a "provided" dependency: provided by platform or added by users. That way it could use whichever implementation user chose; either the best, or one that container or JDK provides. And finally – if and when there are concerns about Woodstox, would it kill you to maybe quickly contact Woodstox project team? Who knows, that could even help resolve issues. It is rather frustrating to find that there's enough time to complain, and hack around things, but apparently not enough time to collaborate. Most open source packages have competent and friendly maintainers, who can be concated to help resolve issues (or point out that issues are elsewhere as the case may be); or at least would be interested in learning of such issues.
        Hide
        Robert Zeigler added a comment -

        Sadly, using a "provided" dependency is practically impossible since there's no reliable, implementation-independent manner to get at the DTD, as you well know.

        Show
        Robert Zeigler added a comment - Sadly, using a "provided" dependency is practically impossible since there's no reliable, implementation-independent manner to get at the DTD, as you well know.
        Hide
        Christian Köberl added a comment -

        I'm currently working on a SAX implementation of the template parser - here's my first (not complete!!!) version. It passes 30 out of the 62 test cases for the template parser. I will try to finish the parser until the first week of August.

        Show
        Christian Köberl added a comment - I'm currently working on a SAX implementation of the template parser - here's my first (not complete!!!) version. It passes 30 out of the 62 test cases for the template parser. I will try to finish the parser until the first week of August.
        Hide
        Christian Köberl added a comment -

        My second version (still not complete!!!)
        It passes 48 out of the 62 test cases for the TemplateParserImplTest

        Show
        Christian Köberl added a comment - My second version (still not complete!!!) It passes 48 out of the 62 test cases for the TemplateParserImplTest
        Hide
        Christian Köberl added a comment -

        First, good news: the parser works (my 3rd attachment)! We have it running in our test environment.

        Bad news: there are still some integration tests failing, namely:

        • extend_without_base_template(org.apache.tapestry5.integration.IntegrationTests) Time elapsed: 0.328 sec <<< FAILURE!
          java.lang.AssertionError: Page did not contain 'Component org.apache.tapestry5.integration.app1.pages.InvalidTemplateExtend uses an extension template, but does not have a parent component.'.
        • component_extends_parent_template(org.apache.tapestry5.integration.IntegrationTests) Time elapsed: 0.344 sec <<< FAILURE!
          com.thoughtworks.selenium.SeleniumException: ERROR: Element title not found

        Then, I get 2 errors in the TemplateParserImplTest when running the whole suite from command line. When I run just single test from Eclipse (for debugging) it works fine:

        • parse_failure(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE!
          java.lang.AssertionError: expected:<2> but was:<4>
        • just_HTML(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE!
          java.lang.AssertionError: expected:<6> but was:<8>

        And finally I my template parser reports on the following test line number 3 (which I think should be correct):

        { "content_within_body_element.tml", "Content inside a Tapestry body element is not allowed", 2 }

        ,

        I will investigate on this the next days - help welcome.

        Show
        Christian Köberl added a comment - First, good news: the parser works (my 3rd attachment)! We have it running in our test environment. Bad news: there are still some integration tests failing, namely: extend_without_base_template(org.apache.tapestry5.integration.IntegrationTests) Time elapsed: 0.328 sec <<< FAILURE! java.lang.AssertionError: Page did not contain 'Component org.apache.tapestry5.integration.app1.pages.InvalidTemplateExtend uses an extension template, but does not have a parent component.'. component_extends_parent_template(org.apache.tapestry5.integration.IntegrationTests) Time elapsed: 0.344 sec <<< FAILURE! com.thoughtworks.selenium.SeleniumException: ERROR: Element title not found Then, I get 2 errors in the TemplateParserImplTest when running the whole suite from command line. When I run just single test from Eclipse (for debugging) it works fine: parse_failure(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE! java.lang.AssertionError: expected:<2> but was:<4> just_HTML(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE! java.lang.AssertionError: expected:<6> but was:<8> And finally I my template parser reports on the following test line number 3 (which I think should be correct): { "content_within_body_element.tml", "Content inside a Tapestry body element is not allowed", 2 } , I will investigate on this the next days - help welcome.
        Hide
        Christian Köberl added a comment -

        Here is my final version - everthing seems to work (except see below).

        To use it in your 5.1 version you'll have to:

        1. Remove Woodstox Depdency:
        <dependency>
        <groupId>org.apache.tapestry</groupId>
        <artifactId>tapestry-core</artifactId>
        <version>$

        {tapestry-version}

        </version>
        <exclusions>
        <exclusion>
        <groupId>org.codehaus.woodstox</groupId>
        <artifactId>woodstox-core-asl</artifactId>
        </exclusion>
        </exclusions>
        </dependency>

        2. Add this to your AppModule:

        public static void bind(ServiceBinder binder)

        { binder.bind(TemplateParser.class, SaxTemplateParserImpl.class).withId("TemplateParserOverride"); }

        public static void contributeTemplateParserOverride(MappedConfiguration<String, URL> config)

        { // Any class inside the internal module would do. Or we could move all these // files to o.a.t.services. Class<UpdateListenerHubImpl> c = UpdateListenerHubImpl.class; config.add("-//W3C//DTD XHTML 1.0 Strict//EN", c.getResource("xhtml1-strict.dtd")); config.add("-//W3C//DTD XHTML 1.0 Transitional//EN", c .getResource("xhtml1-transitional.dtd")); config.add("-//W3C//DTD XHTML 1.0 Frameset//EN", c.getResource("xhtml1-frameset.dtd")); config.add("-//W3C//DTD HTML 4.01//EN", c.getResource("xhtml1-strict.dtd")); config.add("-//W3C//DTD HTML 4.01 Transitional//EN", c .getResource("xhtml1-transitional.dtd")); config.add("-//W3C//DTD HTML 4.01 Frameset//EN", c.getResource("xhtml1-frameset.dtd")); config.add("-//W3C//ENTITIES Latin 1 for XHTML//EN", c.getResource("xhtml-lat1.ent")); config.add("-//W3C//ENTITIES Symbols for XHTML//EN", c.getResource("xhtml-symbol.ent")); config.add("-//W3C//ENTITIES Special for XHTML//EN", c.getResource("xhtml-special.ent")); }

        public static void contributeServiceOverride(MappedConfiguration<Class, Object> configuration,
        @Local TemplateParser override)

        { configuration.add(TemplateParser.class, override); }

        Voila - you have a non-Stax Tapestry.

        I still get the 2 errors in the TemplateParserImplTest when running the whole suite from command line. When I run just single test from Eclipse (for debugging) it works fine:

        • parse_failure(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE!
          java.lang.AssertionError: expected:<2> but was:<4>
        • just_HTML(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE!
          java.lang.AssertionError: expected:<6> but was:<8>
          These problems are just line numbers that don't match - so I will not investigate any further.
        Show
        Christian Köberl added a comment - Here is my final version - everthing seems to work (except see below). To use it in your 5.1 version you'll have to: 1. Remove Woodstox Depdency: <dependency> <groupId>org.apache.tapestry</groupId> <artifactId>tapestry-core</artifactId> <version>$ {tapestry-version} </version> <exclusions> <exclusion> <groupId>org.codehaus.woodstox</groupId> <artifactId>woodstox-core-asl</artifactId> </exclusion> </exclusions> </dependency> 2. Add this to your AppModule: public static void bind(ServiceBinder binder) { binder.bind(TemplateParser.class, SaxTemplateParserImpl.class).withId("TemplateParserOverride"); } public static void contributeTemplateParserOverride(MappedConfiguration<String, URL> config) { // Any class inside the internal module would do. Or we could move all these // files to o.a.t.services. Class<UpdateListenerHubImpl> c = UpdateListenerHubImpl.class; config.add("-//W3C//DTD XHTML 1.0 Strict//EN", c.getResource("xhtml1-strict.dtd")); config.add("-//W3C//DTD XHTML 1.0 Transitional//EN", c .getResource("xhtml1-transitional.dtd")); config.add("-//W3C//DTD XHTML 1.0 Frameset//EN", c.getResource("xhtml1-frameset.dtd")); config.add("-//W3C//DTD HTML 4.01//EN", c.getResource("xhtml1-strict.dtd")); config.add("-//W3C//DTD HTML 4.01 Transitional//EN", c .getResource("xhtml1-transitional.dtd")); config.add("-//W3C//DTD HTML 4.01 Frameset//EN", c.getResource("xhtml1-frameset.dtd")); config.add("-//W3C//ENTITIES Latin 1 for XHTML//EN", c.getResource("xhtml-lat1.ent")); config.add("-//W3C//ENTITIES Symbols for XHTML//EN", c.getResource("xhtml-symbol.ent")); config.add("-//W3C//ENTITIES Special for XHTML//EN", c.getResource("xhtml-special.ent")); } public static void contributeServiceOverride(MappedConfiguration<Class, Object> configuration, @Local TemplateParser override) { configuration.add(TemplateParser.class, override); } Voila - you have a non-Stax Tapestry. I still get the 2 errors in the TemplateParserImplTest when running the whole suite from command line. When I run just single test from Eclipse (for debugging) it works fine: parse_failure(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE! java.lang.AssertionError: expected:<2> but was:<4> just_HTML(org.apache.tapestry5.internal.services.TemplateParserImplTest) Time elapsed: 0 sec <<< FAILURE! java.lang.AssertionError: expected:<6> but was:<8> These problems are just line numbers that don't match - so I will not investigate any further.
        Hide
        Christian Köberl added a comment -

        Here's the newest version of SaxTemplateParserImpl. I fixed the mustBeRoot check - DTD tokens are allowed before root element.

        Show
        Christian Köberl added a comment - Here's the newest version of SaxTemplateParserImpl. I fixed the mustBeRoot check - DTD tokens are allowed before root element.
        Hide
        Christian Köberl added a comment -

        Here is the implementation in a maven project (for those who need it in 5.1):
        http://wiki.github.com/derkoe/tapestry-sax-parser

        Show
        Christian Köberl added a comment - Here is the implementation in a maven project (for those who need it in 5.1): http://wiki.github.com/derkoe/tapestry-sax-parser
        Hide
        Howard M. Lewis Ship added a comment -

        And the extra bonus is to reduce the number of dependencies in Tapestry.

        Show
        Howard M. Lewis Ship added a comment - And the extra bonus is to reduce the number of dependencies in Tapestry.

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Howard M. Lewis Ship
          • Votes:
            7 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development