Projected image: pixelsService.copyAndResizeImage() workaround

[jburel]

Not possible to move source image/projected image due to channels linkage.

Current Workaround: Delete the projections manually or ask the other user to if not possible.

Until we have deep copy on demand (!), for new projections it could be good if pixelsService.copyAndResizeImage() did *not* share instrument, objective settings, etc. between images.

[mtbcarroll]

Is it clear what should be done about this? "Everything is in exactly one group" has all kinds of interesting linkage-caused implications.

[jamoore]

@mtbc: not really. At the moment the projected image is in a different MIF, so to speak, though it re-uses some internal objects. So options include:

1. make them part of the same MIF
2. make them *not* re-use the same objects
3. add extra handling for finding and removing the projected images on chgrp/delete
4. like 3, but leave them orphaned.
5. ???

[jburel]

graph problem moved Beta3

[jburel]

see ​https://trac.openmicroscopy.org.uk/ome/ticket/11529

[jburel]

see ​https://trac.openmicroscopy.org.uk/ome/ticket/11307 and ​https://trac.openmicroscopy.org.uk/ome/ticket/10262

[mtbcarroll]

The core problem is that images owned by different users have some model objects in common -- probably instrument, objective settings -- and those objects may be in only one group? We won't be creating a copy of the shared objects at chgrp time for 5.1.0. A difficulty is deciding which image is the original: without special ID-comparing logic, we'd not want the owner of the projection to move their image and cause the original to be deleted. Of course, it may be that we can have the projection script not re-use the objects, and OMERO does tend to be less permissive than the OME core object model, but people will already have legacy projections in their systems anyway. Not sure if we ought to try to fix this soon or simply bring it into a larger discussion of the way forward permissions-and-model-wise.

[jburel]

Problem is not only related to projected image as discussed with Mark
Any "derive" image will lead to similar error e.g. images from ROis.

[jamoore]

Petr: there's some question if the state of "move projected image" is sufficiently good for you at the moment. If yes, then we can push this to "5.x". Otherwise, someone can you let us know the steps to badness.

[pwalczysko]

Yes, atm., I am kinda happy for it to move to 5x. Steps to badness:

• create a Projection
• move it somewhere where you forgot the place of
• try to move the original image -> move fails, no explanation why, no idea where the projection is anyway -> badness

[mtbcarroll]

If it would help clients to handle this, the container service could offer a getImagesBySplitInstruments or somesuch.

[wmoore]

I would like it if pixelsService.copyAndResizeImage() created new objects in the DB instead of linking the new image to existing objects.
That would solve this issue for new projected images, channel offsets script etc, although if we still wanted to fix existing images then we'd also need another approach.

[jamoore]

wmoore: the DB upgrade could possibly handle existing images.

[jamoore]

Referencing ticket #11752 has changed sprint.

[jamoore]

Referencing ticket #11752 has changed sprint.

[jamoore]

Are we attempting anything here?

[pwalczysko]

Not me. The satatus to my knowledge is as reported by me couple of comments above.

[pwalczysko]

(I guess this is not something we will attempt for 5.1.0)

[jamoore]

Think this will have to go along with the "Graphs clean" (roughly with "Permissions" and "Export", followed by "Deep Copy").

Timing to be discussed.

[mtbcarroll]

Naturally, timing depends on the chosen solution. For 5.1.1 we can at least provide some kind of getImagesBySplitObjects (looks at instruments, channels, objective settings) that is like checking if the images are in the same fileset, though ownership within the set may vary. For anything more like deep copy, even 5.1.x is optimistic, especially if we also have to duplicate annotations on the instrument, etc. You're right, definitely needs discussion.

[wmoore]

Just found another bug associated with this issue:

• I create a share and put image into it.
• Other members of the share can view image OK etc.
• Then I create a new image ('Channel Offset script OR Projection etc)
• Now, other members of the share try to view the image that I previously added to the share:

Traceback:
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/django/core/handlers/base.py" in get_response
  114.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/Users/wmoore/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/decorators.py" in wrapped
  469.             retval = f(request, *args, **kwargs)
File "/Users/wmoore/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/decorators.py" in wrapper
  519.             context = f(request, *args, **kwargs)
File "/Users/wmoore/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/webclient/views.py" in load_metadata_preview
  967.     rdefId = manager.image.getRenderingDefId()
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py" in wrapped
  6695.                 if not self._prepareRenderingEngine() \
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py" in _prepareRenderingEngine
  6920.                 self._re = self._prepareRE(rdid=rdid)
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py" in _prepareRE
  6889.         re.lookupPixels(pid, ctx)
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py" in __call__
  4120.             return self.handle_exception(e, *args, **kwargs)
File "/Users/wmoore/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/webclient/webclient_gateway.py" in handle_exception
  2069.                 e, *args, **kwargs)
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py" in __call__
  4117.             return self.f(*args, **kwargs)
File "/Users/wmoore/Desktop/OMERO/dist/lib/python/omero_api_RenderingEngine_ice.py" in lookupPixels
  337.             return _M_omero.api.RenderingEngine._op_lookupPixels.invoke(self, ((pixelsId, ), _ctx))

Exception Type: SecurityViolation at /webclient/metadata_preview/image/8102/57579/
Exception Value: exception ::omero::SecurityViolation
{
    serverStackTrace = ome.conditions.SecurityViolation: ome.model.core.Channel:Id_8813 not contained in share
	at ome.security.sharing.SharingACLVoter.throwLoadViolation(SharingACLVoter.java:79)
	at ome.security.CompositeACLVoter.throwLoadViolation(CompositeACLVoter.java:92)
	at ome.security.ACLEventListener.onPostLoad(ACLEventListener.java:104)
	at org.hibernate.engine.TwoPhaseLoad.initializeEntity(TwoPhaseLoad.java:250)
	at org.hibernate.loader.Loader.initializeEntitiesAndCollections(Loader.java:898)
	at org.hibernate.loader.Loader.doQuery(Loader.java:773)

[mtbcarroll]

This ticket is largely superseded by ​https://trello.com/c/muXgN0XR/213-deep-copy but is there some other ticket regarding the other share permissions issues we've been seeing that covers Will's latest?

[mtbcarroll]

Will move deep copy issues to the Trello card: this ticket becomes devoted to the API behavior workaround.

[wmoore]

As discussed with Mark, "deep copy" is a much bigger issue than the dual linking of new images to Channels, Instrument etc caused by pixelsService.copyAndResizeImage().
So lets keep a ticket for that smaller issue.
This issue is also a problem for shares: New ticket for that today:
​https://trac.openmicroscopy.org.uk/ome/ticket/12832

[wmoore]

Another use case that is broken by this bug - Can't project an image belonging to another user (read-ann group) #12806

[jamoore]

See also: ​https://trac.openmicroscopy.org.uk/ome/ticket/12806

[jamoore]

Splitting 5.1.4 due to milestone decoupling

[jamoore]

Note: it might suffice for 5.1.4 to simply comment out the re-use of instrument etc. See ​https://github.com/openmicroscopy/openmicroscopy/blob/v5.1.3/components/server/src/ome/logic/PixelsImpl.java#L238

[mtbcarroll]

If OMERO 5.2.0 code is to be largely open in PRs by mid-October then, with my preparing for the BioImage Informatics Conference, the acquisition object duplication for this ticket certainly isn't happening. If anyone wants to try getting some quick fix in in the meantime, feel free to steal this ticket, otherwise I'll still keep it on my priority list: happy to see it in 5.2.1 once that milestone exists.

[mtbcarroll]

Fixing this actually fits in with ROI work.

[mtbcarroll]

So, let's say there's a deep-copy graph request that can be submitted from client-side scripts. How to submit such requests from inside the server itself, i.e. from PixelsImpl? Have we any simple examples of this?

[jamoore]

I think this best example of this is Deletion.java. i.e. encapsulate the logic in the server/ component so it can be used internally. If that proves too costly (in terms of refactoring), the next option would be to write a DuplicateMessage and put it on the event bus.

[mtbcarroll]

Thank you! The more I pursue the first option, the more I think it's best to choose the latter.

[mtbcarroll]

The DuplicateMessage approach works well, except for that the duplication happens in a separate transaction because a different thread is needed to avoid blocking message dispatch.

However, it helps us to fix PixelsImpl rather less than I'd originally hoped: pixels need an image and channels need a pixels so they have to be duplicated attached to other things, and they also link via other model objects to detectors and instruments and stuff, so if any of those are to be duplicated then the whole set should probably be in the same operation, yet the pixels service offers ability to mess with the pixels object and its channels and whatnot, and to create extra pixels on the original image, so a whole subgraph duplicate isn't what's really wanted.

To avoid multiply duplicating subgraph fragments through reaching them via required properties from different objects that do need duplicating, it might actually be easier to duplicate the whole lot then delete or graft as appropriate. (For instance, to delete channels not in the provided channelList.) However, that requires addition of a DeleteMessage and yet more transactions (I assume we can't move PixelsImpl to Blitz), and pretty much a reimplementation of the pixels service.

I'm thus tempted to push this again, I'm afraid; I hadn't realized how awkwardly featureful the pixels service is. I wonder how well our integration testing covers those features.

[jamoore]

Happy both to discuss possible workarounds to these issues as well as to let someone else specify the priority for the changes.

[jburel]

Milestone OMERO-5.2.1 deleted

[jburel]

Milestone OMERO-5.2.2 deleted

[mtbcarroll]

Pushing again, I'm afraid. I won't forget about this, but the model change work has to take priority at present.

[mtbcarroll]

I hadn't realized how awkwardly featureful the pixels service is.

One question is how featureful it needs to be. If it requires major work for this bug, perhaps it can be made simpler without anybody minding.

[mtbcarroll]

Also see #12580.

[mtbcarroll]

Note #1217: projection should have different sha1.