Jetspeed 2
  1. Jetspeed 2
  2. JS2-692

Fragment ids are not automatically created, causing runtime errors

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.1.2
    • Component/s: PSML
    • Labels:
      None

      Description

      When deploying PSML pages, it is currently required to enter unique ids for each fragment in the PSML file.
      This is an error prone process. If a fragment id is not supplied, the following stacktrace occurs in the browser:

      java.lang.StackOverflowError
      java.util.regex.Pattern.atom(Pattern.java:1863)
      java.util.regex.Pattern.sequence(Pattern.java:1743)
      java.util.regex.Pattern.expr(Pattern.java:1687)
      java.util.regex.Pattern.compile(Pattern.java:1397)
      java.util.regex.Pattern.<init>(Pattern.java:1124)
      java.util.regex.Pattern.compile(Pattern.java:817)
      java.lang.String.split(String.java:2103)
      java.lang.String.split(String.java:2145)
      org.apache.jetspeed.util.Path.parsePathSegments(Path.java:116)
      org.apache.jetspeed.util.Path.<init>(Path.java:100)
      org.apache.jetspeed.util.Path.getSubPath(Path.java:253)
      org.apache.jetspeed.util.Path.getChild(Path.java:447)
      org.apache.jetspeed.decoration.BaseDecoration.getResource(BaseDecoration.java:125)
      org.apache.jetspeed.decoration.BaseDecoration.getStyleSheet(BaseDecoration.java:180)
      org.apache.jetspeed.decoration.PageTheme.setupFragmentDecorations(PageTheme.java:90)
      org.apache.jetspeed.decoration.PageTheme.setupFragmentDecorations(PageTheme.java:125)
      org.apache.jetspeed.decoration.PageTheme.setupFragmentDecorations(PageTheme.java:125)
      ...

      This bug fix will make fragment ids optional on deploy, and automatically create the ids if they do not exist.

      1. castor-page-manager.patch
        24 kB
        Dennis Dam
      2. castor-test-pages.patch
        6 kB
        Dennis Dam
      3. fs-page-manager.patch
        50 kB
        Dennis Dam

        Activity

        David Sean Taylor created issue -
        Hide
        Dennis Dam added a comment -

        This patch addresses several things:

        • automatic ID generation for fragments
        • update to latest Cstor version (1.1.1)
        • performance improvements for Castor filesystem page manager

        The automatic ID generation was solved by specifying the ID field as a constructor argument for the FragmentImpl class in the Castor page mapping file. If the ID is null or empty, a new ID is generated, in a similar way as the JetspeedIDGenerator generates IDs.

        To be able to use constructor arguments, I had to switch to the latest version of Castor (1.1.1). The only change I had to make to make the new version work is remove the jetspeed namespaces when parsing PSML pages. This is ok, because previously namespaces were also ignored. On a side note , it might be "cleaner" to just support the namespaces, because the namespaces are specified in most / all PSML files within Jetspeed, but I was afraid it might break customer implementations if we make the namespace mandatory.

        About the performance improvements, unmarshalling / marshalling PSML pages should be a lot faster than before. I tested it on my local machine by creating 1000 pages first and then reading them in again, in between resetting the test environment. Unmarshalling took 1.3 seconds, with the old version it took about 2.4 seconds.
        What I did basically was applying some performance best practices from the Castor website, and remove some deprecated code ( replacing old SAX 1 DocumentHandlers with SAX 2 ContentHandlers)

        Show
        Dennis Dam added a comment - This patch addresses several things: automatic ID generation for fragments update to latest Cstor version (1.1.1) performance improvements for Castor filesystem page manager The automatic ID generation was solved by specifying the ID field as a constructor argument for the FragmentImpl class in the Castor page mapping file. If the ID is null or empty, a new ID is generated, in a similar way as the JetspeedIDGenerator generates IDs. To be able to use constructor arguments, I had to switch to the latest version of Castor (1.1.1). The only change I had to make to make the new version work is remove the jetspeed namespaces when parsing PSML pages. This is ok, because previously namespaces were also ignored. On a side note , it might be "cleaner" to just support the namespaces, because the namespaces are specified in most / all PSML files within Jetspeed, but I was afraid it might break customer implementations if we make the namespace mandatory. About the performance improvements, unmarshalling / marshalling PSML pages should be a lot faster than before. I tested it on my local machine by creating 1000 pages first and then reading them in again, in between resetting the test environment. Unmarshalling took 1.3 seconds, with the old version it took about 2.4 seconds. What I did basically was applying some performance best practices from the Castor website, and remove some deprecated code ( replacing old SAX 1 DocumentHandlers with SAX 2 ContentHandlers)
        Dennis Dam made changes -
        Field Original Value New Value
        Attachment castor-page-manager.patch [ 12359996 ]
        David Sean Taylor made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        David Sean Taylor added a comment -

        Patch reviewed and looks good.
        Unfortunately we are missing two files causing the unit test to fail:

        pageWithoutIds.psml
        pageWithSomeIds.psml

        Could you please send us the two test files, thanks

        Show
        David Sean Taylor added a comment - Patch reviewed and looks good. Unfortunately we are missing two files causing the unit test to fail: pageWithoutIds.psml pageWithSomeIds.psml Could you please send us the two test files, thanks
        Hide
        Dennis Dam added a comment -

        overlooked those.. sorry about that!

        Show
        Dennis Dam added a comment - overlooked those.. sorry about that!
        Dennis Dam made changes -
        Attachment castor-test-pages.patch [ 12360181 ]
        Hide
        Dennis Dam added a comment -

        all encompassing patch, includes:

        • castor upgrade
        • automatic id generation
        • testcases
        Show
        Dennis Dam added a comment - all encompassing patch, includes: castor upgrade automatic id generation testcases
        Dennis Dam made changes -
        Attachment fs-page-manager.patch [ 12360693 ]
        David Sean Taylor made changes -
        Resolution Fixed [ 1 ]
        Status In Progress [ 3 ] Resolved [ 5 ]
        Ate Douma made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        45d 22h 51m 1 David Sean Taylor 19/Jun/07 16:45
        In Progress In Progress Resolved Resolved
        9d 22m 1 David Sean Taylor 28/Jun/07 17:08
        Resolved Resolved Closed Closed
        1559d 5h 29m 1 Ate Douma 04/Oct/11 22:37

          People

          • Assignee:
            David Sean Taylor
            Reporter:
            David Sean Taylor
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development