Tapestry
  1. Tapestry
  2. TAPESTRY-967

SpecificationParser doesn't allow custom entities

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 4.0.1
    • Fix Version/s: 3.0.5, 4.2
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      all

      Description

      org.apache.tapestry.parse.SpecificationParser implements the method EntityResolver.resolveEntity() in a way that prohibits resolving any entities but the Tapestry specification DTD (it throws if the public-id does not equal either the 3.0 or the 4.0 spec).

      It would be useful to be able to refer to custom entities. That would allow one to include e.g. specs for base-class properties in all derived page-classes without having to repeat them (and without using annotations).

      suggested patch:
      in EntityResolver.resolveEntity() return null instead of throwing.

        Activity

        Hide
        Andreas Andreou added a comment -

        From a pragmatic POV, i don't see a value for this - it's easy to have specless components
        and not deal with specifications at all...

        Show
        Andreas Andreou added a comment - From a pragmatic POV, i don't see a value for this - it's easy to have specless components and not deal with specifications at all...
        Hide
        Marcus Schulte added a comment -

        oh, please scrap the last line of my previous comment!

        Actually it works nicely in 3.0! It's just plain broken in 4.0.

        Then, I'm in no hurry with the patch, since I need that for a 3.0 app anyway.

        Show
        Marcus Schulte added a comment - oh, please scrap the last line of my previous comment! Actually it works nicely in 3.0! It's just plain broken in 4.0. Then, I'm in no hurry with the patch, since I need that for a 3.0 app anyway.
        Hide
        Marcus Schulte added a comment -

        Ok, I've created a patch.
        It adds two new unit test methods checking for successful inclusion of a custom entity as well as for a sensible error message if an invalid entity ref is given.
        There will still be an error thrown whenever one tries to use an entity with a public id which is not equal to one of the allowed Tapestry dtd's. So nothing's broken in that regard. All unit tests pass.

        Btw., would it be possible to merge this one into the 3.0.x branch? I'd like to use it in a 3.0 app ...

        Show
        Marcus Schulte added a comment - Ok, I've created a patch. It adds two new unit test methods checking for successful inclusion of a custom entity as well as for a sensible error message if an invalid entity ref is given. There will still be an error thrown whenever one tries to use an entity with a public id which is not equal to one of the allowed Tapestry dtd's. So nothing's broken in that regard. All unit tests pass. Btw., would it be possible to merge this one into the 3.0.x branch? I'd like to use it in a 3.0 app ...
        Hide
        Jesse Kuhnert added a comment -

        Sure, patches (combined with updated/new test cases) are always welcome It will make it easier/faster to look at this one. (though it will still be not quite as fast as other patch submissions because this one feels like it might affect a lot of things, or open new doors...so want to make sure those doors should be open/etc...)

        Show
        Jesse Kuhnert added a comment - Sure, patches (combined with updated/new test cases) are always welcome It will make it easier/faster to look at this one. (though it will still be not quite as fast as other patch submissions because this one feels like it might affect a lot of things, or open new doors...so want to make sure those doors should be open/etc...)
        Hide
        Marcus Schulte added a comment -

        I could try my proposal and prepare a patch. Let me know if you want me to do that. I'd have to setup a tapestry build. That's why I didn't do it yet.

        Show
        Marcus Schulte added a comment - I could try my proposal and prepare a patch. Let me know if you want me to do that. I'd have to setup a tapestry build. That's why I didn't do it yet.
        Hide
        Jesse Kuhnert added a comment -

        I like this idea very much, have been wanting to have it in tapestry for a while. Don't have time to review the exact case you describe right now, but if it's feasible I'll try to get it into one of the upcoming 4.1 releases.

        Show
        Jesse Kuhnert added a comment - I like this idea very much, have been wanting to have it in tapestry for a while. Don't have time to review the exact case you describe right now, but if it's feasible I'll try to get it into one of the upcoming 4.1 releases.

          People

          • Assignee:
            Jesse Kuhnert
            Reporter:
            Marcus Schulte
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development