Jetspeed 2
  1. Jetspeed 2
  2. JS2-501

Pluto ObjectID implementation produces invalid map key values

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0-FINAL, 2.1
    • Fix Version/s: 2.1
    • Component/s: Container
    • Labels:
      None

      Description

      The JetspeedObjectID implementation for the Pluto ObjectID interface contains a weird and not reliable algorithm for generating distinct
      long values for String based id's.

      Until now, I never encountered problems with this because in most cases the provided String id value was long enough to produce
      reliable "enough" long keys.
      But, when changing from Castor/file based psml to Database stored psml, the problem with this code suddenly popped up.
      As the DatabasePageManager uses generated (long!) id's which are used as String based id's for seeding the JetspeedObjectID,
      duplicate long values began to pop up. This resulted in very "weird" behaving pages, in which content from one portlet was displayed
      in the portlet window of another. The JetspeedObjectID.equals(Object) method which only compared the generated and not distinct
      long values turned out to be cause behind that.

      Now, the whole idea of providing some kind of "dual" behavior of the JetspeedObjectID: representing both a String and a long value
      as well as being able to convert from the one to the other, seems to come from Pluto itself...

      The Pluto Portal Driver uses an ObjectID implementation with the exact same algorithm in it. More specific, this code has
      already been provided during the first initial code "drop" of Pluto. Jetspeed simply copied this "feature"...

      The whole idea behind an artificial ObjectID interface (adding nothing) for representing some id is beyond me:
      why not just use Object for that?

      Anyway, after evaluating the usage of the ObjectID interface as well as the Portal Driver implementation within Pluto,
      I've come to the conclusion that this "dual" behavior really isn't nor shouldn't be needed.
      There is one Pluto container interface, PortletDefinitionCtrl, which defines a setId(String oid) method.
      Now, that method really should take an ObjectID in my book, not a String!
      But, nowhere within Pluto (1.0.1) itself this method is invoked, and neither is it within Jetspeed.
      Usage within Jetspeed clearly is divided between a "String" based usage, like for PortletEntity/PortletWindow id's, and "Long"
      based usage like for PortletDefinition or LocalizedField.

      So, I've decided to fix this problem by splitting the two behaviors: JetspeedObjectID is going to provide only the "String" based
      behavior, and a new JetspeedLongObjectID the "Long" based behavior.
      That way, these objects now can reliably be used for Map keys.

      I also had to "fix" a few OJB mappings (and internal conversions) for this, but nothing that impacted database definitions itself.
      So, AFAIK, this fix should be without side-effects.

      Note:
      I've removed a few setId(String oid) jetspeed-api interface methods which were I think accidentally copied over from likewise Pluto interfaces.
      But none of those were actually used, nor should they.
      I couldn't remove the Jetspeed implementation for the Pluto PortletDefinitionCtrl.setId(String oid) interface method in
      org.apache.jetspeed.om.portlet.impl.PortletDefinitionImpl of course, but now it will throw a NotSupportedOperationException if invoked.

      Finally, I've decided (for now) to only provide this fix for Jetspeed 2.1, not the 2.0.1 branch as there are a few interface changes which
      normally we shouldn't have in a fix release only. If anyone has a good case for back porting this to the 2.0.1 branch though, do speak
      up (before 2.0.1 is released).

        Activity

        Hide
        Ate Douma added a comment -

        Fix committed

        Show
        Ate Douma added a comment - Fix committed

          People

          • Assignee:
            Ate Douma
            Reporter:
            Ate Douma
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development