JDO
  1. JDO
  2. JDO-402

checkRemoveGroup - removeGroup should be before getGroups

    Details

      Description

      In org.apache.jdo.tck.api.persistencemanager.fetchplan.FetchPlanInterface, lines 281-282,

      it probably has to be:
      fp.removeGroup("default");
      Collection groups = fp.getGroups();

      Instead of:
      Collection groups = fp.getGroups();
      fp.removeGroup("default");

      1. JDO-402.patch
        0.8 kB
        Craig L Russell

        Activity

        Hide
        Craig L Russell added a comment -

        svn commit -m "JDO-402 Change order of removeGroups and getGroups" src/java/org/apache/jdo/tck/api/persistencemanager/fetchplan/FetchPlanInterface.java
        Sending src/java/org/apache/jdo/tck/api/persistencemanager/fetchplan/FetchPlanInterface.java
        Transmitting file data .
        Committed revision 429180.

        Show
        Craig L Russell added a comment - svn commit -m " JDO-402 Change order of removeGroups and getGroups" src/java/org/apache/jdo/tck/api/persistencemanager/fetchplan/FetchPlanInterface.java Sending src/java/org/apache/jdo/tck/api/persistencemanager/fetchplan/FetchPlanInterface.java Transmitting file data . Committed revision 429180.
        Hide
        Erik Bengtson added a comment -

        It was implicitly understood that the returned Set is immutable because the underlying Set is a reference. If the underlying set is a copy, it does not make sense to be immutable.

        Anyway, I dont have any problem if it's changed to copy. In this case, making the returned set mutable is better for usability.

        Show
        Erik Bengtson added a comment - It was implicitly understood that the returned Set is immutable because the underlying Set is a reference. If the underlying set is a copy, it does not make sense to be immutable. Anyway, I dont have any problem if it's changed to copy. In this case, making the returned set mutable is better for usability.
        Hide
        Andy Jefferson added a comment -

        Understood. The original test does fail with the change to backing store copy (in JPOX CVS). Just need someone to apply the patch to tck20

        Show
        Andy Jefferson added a comment - Understood. The original test does fail with the change to backing store copy (in JPOX CVS). Just need someone to apply the patch to tck20
        Hide
        Craig L Russell added a comment -

        Hi Andy,

        The spec does say the returned Set is immutable but that doesn't necessarily mean that the underlying Set is immutable as well. That is, there is room for the Set to change based on the underlying groups. But I think it makes sense to return an immutable Set that will never change, even if the underlying groups changes.

        Show
        Craig L Russell added a comment - Hi Andy, The spec does say the returned Set is immutable but that doesn't necessarily mean that the underlying Set is immutable as well. That is, there is room for the Set to change based on the underlying groups. But I think it makes sense to return an immutable Set that will never change, even if the underlying groups changes.
        Hide
        Andy Jefferson added a comment -

        The spec isnt totally silent (12.7.5 comment line in the code for getGroups). Erik asked the question on the mailing list on 15th Feb 2006, and a knowledgeable person replied that it is immutable, see
        http://mail-archives.apache.org/mod_mbox/db-jdo-dev/200602.mbox/%3c5EC61AF4-55BA-4B9E-A74A-05ABC2DE451E@Sun.COM%3e

        Show
        Andy Jefferson added a comment - The spec isnt totally silent (12.7.5 comment line in the code for getGroups). Erik asked the question on the mailing list on 15th Feb 2006, and a knowledgeable person replied that it is immutable, see http://mail-archives.apache.org/mod_mbox/db-jdo-dev/200602.mbox/%3c5EC61AF4-55BA-4B9E-A74A-05ABC2DE451E@Sun.COM%3e
        Hide
        Craig L Russell added a comment -

        The spec is silent on whether the return can be a "live" object. I had assumed that it was not shared but copied (all other "value" objects in the specification are defined as copied). I'll raise this with the expert group.

        Show
        Craig L Russell added a comment - The spec is silent on whether the return can be a "live" object. I had assumed that it was not shared but copied (all other "value" objects in the specification are defined as copied). I'll raise this with the expert group.
        Hide
        Ilan Kirsh added a comment -

        Thanks for the fix. I assume that the old code should work fine if the implementation returns from getGroup a shared set that is still used by the FetchPlan.

        Show
        Ilan Kirsh added a comment - Thanks for the fix. I assume that the old code should work fine if the implementation returns from getGroup a shared set that is still used by the FetchPlan.
        Hide
        Craig L Russell added a comment -

        This changes the test as suggested by Ilan.

        I don't understand how the original test could have worked. Still looking.

        Show
        Craig L Russell added a comment - This changes the test as suggested by Ilan. I don't understand how the original test could have worked. Still looking.
        Hide
        Craig L Russell added a comment -

        Need to resolve these issues for maintenance release 1.

        Show
        Craig L Russell added a comment - Need to resolve these issues for maintenance release 1.

          People

          • Assignee:
            Craig L Russell
            Reporter:
            Ilan Kirsh
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development