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. fs-page-manager.patch
        50 kB
        Dennis Dam
      2. castor-test-pages.patch
        6 kB
        Dennis Dam
      3. castor-page-manager.patch
        24 kB
        Dennis Dam

        Activity

        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)
        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!
        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

          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