Click
  1. Click
  2. CLK-73

CayenneForm.getDataObject() handling of new objects problem

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: core
    • Labels:
      None
    • Environment:
      Click 0.18 (but I believe it is the same in current CVS version)

      Description

      Calling CayenneForm.getDataObject() more than once for the NEW object results in duplicate insert. I have a case when I need to manipulate a DataObject before saving it, so the action method code looks like this:

      // 'getDataObject() - call #1
      JobPost post = (JobPost) form.getDataObject();
      // do something to POST
      ...
      // 'getDataObject() - call #2
      form.saveChanges()

      here two records are inserted instead of one.

      So maybe we should store DataObject in a transient field of the form for the duration of request, so that consecutive calls to getDataObject do not instantiate it again?

        Activity

        Hide
        Malcolm Edgar added a comment -

        Fix checked in will be available in release 1.3

        Show
        Malcolm Edgar added a comment - Fix checked in will be available in release 1.3
        Hide
        Bob Schellink added a comment -

        Hi

        This still seems like an issue. If no dataObject is associated with a form, multiple calls to cayenneForm.getDataObject() will create and register a new DataObject in the DataContext each time. Also the saveChanges() method calls getDataObject(), potentially registering another object.

        Perhaps getDataObject() should return the cached dataObject on subsequent calls?

        Not sure if the issue above raised by Malcolm about the user moving away without committing the form is still a problem, since the Cayenne filter rolls back any uncommitted object each request. Also it seems that Cayenne supports nested contexts now as well.

        Show
        Bob Schellink added a comment - Hi This still seems like an issue. If no dataObject is associated with a form, multiple calls to cayenneForm.getDataObject() will create and register a new DataObject in the DataContext each time. Also the saveChanges() method calls getDataObject(), potentially registering another object. Perhaps getDataObject() should return the cached dataObject on subsequent calls? Not sure if the issue above raised by Malcolm about the user moving away without committing the form is still a problem, since the Cayenne filter rolls back any uncommitted object each request. Also it seems that Cayenne supports nested contexts now as well.
        Hide
        Andrus Adamchik added a comment -

        +1 on ObjectId serialization. This won't work for appending to URLs, but this should be the best solution for the forms.

        Re new object registration. I realize the problem that you describe, but sometimes there is a need to carry uncommitted state across multiple pages. Also there is a limitation in Cayenne - relationships between transient objects won't work. Now I am designing nested DataContext feature that should hopefully address all your concerns regarding uncomitted state. So we may revisit this discussion later.

        Show
        Andrus Adamchik added a comment - +1 on ObjectId serialization. This won't work for appending to URLs, but this should be the best solution for the forms. Re new object registration. I realize the problem that you describe, but sometimes there is a need to carry uncommitted state across multiple pages. Also there is a limitation in Cayenne - relationships between transient objects won't work. Now I am designing nested DataContext feature that should hopefully address all your concerns regarding uncomitted state. So we may revisit this discussion later.
        Hide
        Malcolm Edgar added a comment -

        I may be demonstrating my Cayenne ignorance here.

        Regarding storing a DataObject's objectId as the PK, this shouldn't be a problem, we just serialize the ObjectId in the HiddenField. This would be better than the current schema which is predicated on an numeric PK for an DataObject.

        However with new DataObject I would think that they shouldn't be registered with the DataContext as the user may never commit the form operation. For example a user may navigate away from a new DataObject form, and we dont want this temporary DataObject in the session based DataContext, as a latter commitChanges() operation would insert this object.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - I may be demonstrating my Cayenne ignorance here. Regarding storing a DataObject's objectId as the PK, this shouldn't be a problem, we just serialize the ObjectId in the HiddenField. This would be better than the current schema which is predicated on an numeric PK for an DataObject. However with new DataObject I would think that they shouldn't be registered with the DataContext as the user may never commit the form operation. For example a user may navigate away from a new DataObject form, and we dont want this temporary DataObject in the session based DataContext, as a latter commitChanges() operation would insert this object. regards Malcolm Edgar
        Hide
        Andrus Adamchik added a comment -

        sorry - overlooked that one. It would be nice if we could also store temp object id binary key in forms to be able to keep a reference to the same new DataObject across requests, but that's a topic of another issue

        Show
        Andrus Adamchik added a comment - sorry - overlooked that one. It would be nice if we could also store temp object id binary key in forms to be able to keep a reference to the same new DataObject across requests, but that's a topic of another issue
        Hide
        Malcolm Edgar added a comment -

        Hi Andrus,

        this issue has already been raised: http://www.avoka.com/jira/browse/CLK-63

        I just need to check the fix into CVS.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Hi Andrus, this issue has already been raised: http://www.avoka.com/jira/browse/CLK-63 I just need to check the fix into CVS. regards Malcolm Edgar
        Hide
        Andrus Adamchik added a comment -

        here is a patch against HEAD that fixes the issue for me

        Show
        Andrus Adamchik added a comment - here is a patch against HEAD that fixes the issue for me

          People

          • Assignee:
            Malcolm Edgar
            Reporter:
            Andrus Adamchik
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development