• Views
  • Iteration Report
  • My Iteration Report
  •  
OMERO.server
  • Login
  • Help/Guide
  • About Trac
  • Preferences
  • Wiki
  • Timeline
  • Roadmap
  • Browse Source
  • View Tickets
  • Search

Context Navigation

  • ← Previous Ticket
  • Next Ticket →

Ticket #346 (new defect)

Opened 2 years ago

Last modified 2 years ago

Merge needlessly/dangerously copies immutable fields.

Reported by: jmoore Owned by: jmoore
Priority: critical Milestone: Blocked
Component: Model Version: 3.0-M1
Keywords: collections, links, hibernate, merge, proxies, unloading, corruption Cc: callan

Description (last modified by jmoore) (diff)

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

quick_patch.txt (2.0 kB) - added by jmoore 2 years ago.

Change History

Changed 2 years ago by jmoore

r926 has the failing tests.

Changed 2 years ago by jmoore

#350 blocks the fix for this ticket.

Changed 2 years ago by jmoore

r931 provides a partial fix.

Changed 2 years ago by jmoore

r940 removes debugging.

Changed 2 years ago by jmoore

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

Changed 2 years ago by jmoore

  • cc callan added
  • keywords corruption, iteration5 added; corruption removed

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.

Changed 2 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)

Changed 2 years ago by jmoore

  • keywords iteration6 added; iteration5 removed

Changed 2 years ago by jmoore

  • keywords hibernate, merge, added
  • description modified (diff)
  • 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.

Changed 2 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 2 years ago by jmoore

  • attachment quick_patch.txt added

Changed 2 years ago by jmoore

  • keywords corruption added; corruption, iteration6 removed
Note: See TracTickets for help on using tickets.

Download in other formats:

  • Comma-delimited Text
  • Tab-delimited Text
  • RSS Feed

Trac Powered

Powered by Trac 0.11
By Edgewall Software.

Visit the Trac open source project at
http://trac.edgewall.org/