Warning: Can't synchronize with repository "(default)" (/home/git/ome.git does not appear to be a Git repository.). Look in the Trac log for more information.
Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #346 (closed)

Opened 15 years ago

Closed 6 years ago

Merge needlessly/dangerously copies immutable fields.

Reported by: jamoore Owned by: jamoore
Priority: critical Milestone: Blocked
Component: Model Version: 3.0-M1
Keywords: collections, links, hibernate, merge, proxies, unloading, corruption Cc: cxallan
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description (last modified by jmoore)

Was: Passing non-proxied one-to-manys to server can return empty (non-null) collection (See comments below)

Several model objects have methods for adding items to collection-valued fields:

 * Pixels.addThumbnail(Thumbnail)
 * Project.linkDataset(Dataset)

These calls add the argument to their own collection and then call the reverse method on the arguments:

 * Thumnail.setPixels(Pixels)
 * ProjectDatasetLink.setProject(Project)

The collections are ignored by Hibernate, and only the reverse methods have an effect. This is known behavior.

However, when one uses only the reverse methods (e.g. setPixels(Pixels)) and the Pixels instance is not a proxy (e.g. pixels.isLoaded()==true) then the Pixels instance which is returned by the IUpdate.saveAndReturnObject() method is neither unloaded, nor is the collection nulled, nor does it contain the newly added Thumbnail which could easily lead to data corruption.

Attachments (1)

quick_patch.txt (2.0 KB) - added by jmoore 15 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 15 years ago by jmoore

r926 has the failing tests.

comment:2 Changed 15 years ago by jmoore

#350 blocks the fix for this ticket.

comment:3 Changed 15 years ago by jmoore

r931 provides a partial fix.

comment:4 Changed 15 years ago by jmoore

r940 removes debugging.

comment:5 Changed 15 years ago by jmoore

r942 reverts the patrial fix. Strange Hibernate bugs arose which couldn't easily be tracked down.

comment:6 Changed 15 years ago by jmoore

  • Cc callan added
  • Keywords iteration5 added

r955 has another non-collection-valued example of this. Specifically, that if a user attempts to set the creation event to some invalid value (any other value is invalid since creation event is immutable) for an IObject, though the database is unaffected, the value returned to the user will be the invalid one.

comment:7 Changed 15 years ago by jmoore

Hibernate Docs 10.11:
"It doesn't usually make sense to enable cascade on a <many-to-one> or <many-to-many> association. Cascade is often useful for <one-to-one> and <one-to-many> associations."

This may be the cause of our problem. Have to think about what the repercussions are of turning off cascades on many-to-one in our scenario. (Along the lines of always having to pass in the one-to-many side of relationships (e.g. Image for Image<->Pixels to have everything saved)

comment:8 Changed 15 years ago by jmoore

  • Keywords iteration6 added; iteration5 removed

comment:9 Changed 15 years ago by jmoore

  • Description modified (diff)
  • Keywords hibernate merge added
  • Summary changed from Passing non-proxied one-to-manys to server can return empty (non-null) collection to Merge needlessly/dangerously copies immutable fields.

So, this has nothing to do with cascading. It actually is a mirkier question of the specification of "merge()". What's happening is thus:

  • Object A (detached on client) has field B set to C.
  • Field B, however, is immutable and won't be set in SQL or dirty detection. (i.e. no errors)
  • Session.merge(), however, copies all values from detached A to persistent A, which gets returned to user.

Working on a workaround.

comment:10 Changed 15 years ago by jmoore

  • Milestone changed from 3.0-M3 to Blocked

Moving to milestone:Blocked. Hibernate does not seem provide access for me to write our workaround.

See r990 for the workaround. (A patch for MergeEventListener? is also necessary)

Changed 15 years ago by jmoore

comment:11 Changed 15 years ago by jmoore

  • Keywords iteration6 removed

comment:12 Changed 6 years ago by jamoore

  • Resolution set to wontfix
  • Status changed from new to closed

Unless profiling shows this to be a considerable issue, we won't be working on this.

Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.69642 sec.)

We're Hiring!