JDO
  1. JDO
  2. JDO-345

MethodsAndObjectConstructionNotSupported (A14.6.2-8) should allow getX()

    Details

      Description

      According to the spec:

      "Methods, including object construction, are not supported, except for Collection, String, and Map methods documented below. Implementations might choose to support non-mutating method calls as non-standard extensions."

      However, MethodsAndObjectConstructionNotSupported expects an exception for method getX().

      1. JDO-345.patch
        2 kB
        Michael Bouschen

        Activity

        Hide
        Craig L Russell added a comment -

        I agree that the test is invalid. There is nothing else of value in the test, so I propose to drop the test from the TCK.

        Show
        Craig L Russell added a comment - I agree that the test is invalid. There is nothing else of value in the test, so I propose to drop the test from the TCK.
        Hide
        Michael Bouschen added a comment -

        I agree we should drop the test from the TCK.

        It might make sense to have a negative test case with a query calling a mutating method such as Collection.add. Calling a mutating method is not allowed in JDOQL.

        Show
        Michael Bouschen added a comment - I agree we should drop the test from the TCK. It might make sense to have a negative test case with a query calling a mutating method such as Collection.add. Calling a mutating method is not allowed in JDOQL.
        Hide
        Ilan Kirsh added a comment -

        I also think that this test should be dropped. A negative test might cause a new problem - how the implementation can check if a method is mutating in the general case? I think we already have enough byte code processing in the enhancer.

        Show
        Ilan Kirsh added a comment - I also think that this test should be dropped. A negative test might cause a new problem - how the implementation can check if a method is mutating in the general case? I think we already have enough byte code processing in the enhancer.
        Hide
        Craig L Russell added a comment -

        I think a word about why we used the term "mutating method" is in order here.

        The expert group did not want to allow the user and implementation to use query as a back door to a generalized UPDATE facility, so we said specifically that mutating methods were not allowed, even in vendor extensions. That said, there are only a small number of standard mutating methods that return a value, and hence even could be used in a filter.

        So I think we are safe in dropping this test and we can debate the value of a new test later on if someone wants to raise the issue again.

        Show
        Craig L Russell added a comment - I think a word about why we used the term "mutating method" is in order here. The expert group did not want to allow the user and implementation to use query as a back door to a generalized UPDATE facility, so we said specifically that mutating methods were not allowed, even in vendor extensions. That said, there are only a small number of standard mutating methods that return a value, and hence even could be used in a filter. So I think we are safe in dropping this test and we can debate the value of a new test later on if someone wants to raise the issue again.
        Hide
        Ilan Kirsh added a comment -

        On a second thought, maybe the test should be preserved as a negative test but refer to non existing methods? In this case an implementation that supports additional method as an extension should still throw an exception.

        Show
        Ilan Kirsh added a comment - On a second thought, maybe the test should be preserved as a negative test but refer to non existing methods? In this case an implementation that supports additional method as an extension should still throw an exception.
        Hide
        Ilan Kirsh added a comment -

        Can this be resolved for maintenance release 1? Simply by discarding the test or by switching to non existing methods?

        Show
        Ilan Kirsh added a comment - Can this be resolved for maintenance release 1? Simply by discarding the test or by switching to non existing methods?
        Hide
        Michael Bouschen added a comment -

        Attached you find a patch for review. I changed the test to call methods Collection.add and Collection.remove that are known to be mutating.

        Show
        Michael Bouschen added a comment - Attached you find a patch for review. I changed the test to call methods Collection.add and Collection.remove that are known to be mutating.
        Hide
        Ilan Kirsh added a comment -

        Thanks. A tiny comment - the import in line 24 (import org.apache.jdo.tck.JDO_Test) is not needed.

        Show
        Ilan Kirsh added a comment - Thanks. A tiny comment - the import in line 24 (import org.apache.jdo.tck.JDO_Test) is not needed.
        Hide
        Craig L Russell added a comment -

        The patch looks good.

        Show
        Craig L Russell added a comment - The patch looks good.
        Hide
        Michael Bouschen added a comment -

        Cleaned up the imports and checked in the patch into the trunk and into branch 2.0.1 (see revision 453263).

        Show
        Michael Bouschen added a comment - Cleaned up the imports and checked in the patch into the trunk and into branch 2.0.1 (see revision 453263).

          People

          • Assignee:
            Michael Bouschen
            Reporter:
            Ilan Kirsh
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development