ome / omero-blitz

Gradle project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
0 stars 15 forks source link

Explicitly include details.externalInfo when relinking duplicate objects #129

Closed sbesson closed 1 year ago

sbesson commented 1 year ago

The current implementation will duplicate objects with ExternalInfo objects associated to them and create both the original object as well as a duplicated ExternalInfo object. However, the linking between the duplicated objects is lost since the externalInfo link is stored under details.externalInfo.

This commit should allow to properly handle this scenario and relink the duplicated objects.

/cc @emilroz @muhanadz

Proposed tag: 5.5.13

sbesson commented 1 year ago

https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/1253/ was green with this included so I think it's ready for a round of review

pwalczysko commented 1 year ago

Sorry, a bit lost. What is the scenario we are talking here about ? Is that duplicating objects ? i.e. should omero duplicate... plugin be run to test it ? If so, confused that the change is not in https://github.com/ome/omero-cli-duplicate (which might be as expected, but it does not help to clarify :)

sbesson commented 1 year ago

What is the scenario we are talking here about ? Is that duplicating objects ? i.e. should omero duplicate... plugin be run to test it ?

Yes, the minimal scenario is as follows:

  1. create an object e.g. import an Image but any object in the DB will do
  2. attach an ExternalInfo object to it. I don't think there is any UI allowing you to do that and you might have to use the .getDetails().setExternalInfo() API
  3. duplicate the original object using omero duplicate
  4. the duplication command will return that the ExternalInfo is also duplicated but if you introspect the duplicated object using the .getDetails().getExternalInfo() API, you will find that the duplicated ExternalInfo is not linked to the duplicated object

The linked integration tests should capture this scenario and demonstrate the API calls required to read/write external info

If so, confused that the change is not in https://github.com/ome/omero-cli-duplicate (which might be as expected, but it does not help to clarify :)

The CLI plugin is constructing the duplication command that is sent server-side and returning the result. In that case, the issue is at the level of the server-side code as the linking between the duplicated objects is missed. In other terms, a server upgrade is required to fix the issue

pwalczysko commented 1 year ago

Thank you @sbesson . So am I right to think that this PR is solving https://github.com/ome/omero-cli-duplicate/issues/20 ?

Edit: Actually , most probably the answer to my question is No. It seems that only one bit of the graph which should have been duplicated does not get duplicated, and this PR is fixing it ?

Edit 2: Aha, no, the duplication is happening atm, but only the link between the ExternalInfo duplicate and the object duplicate is lost... I think I got it. ta

pwalczysko commented 1 year ago

the duplication command will return that the ExternalInfo is also duplicated

I am not sure that I can confirm this. I have successfully set ExternalInfo on an image and dataset, and in neither case the duplication command mentions the ExternalInfo in the output, such as

python
... #establish connection
>>> dataset = conn.getObject("Dataset", 16505)
>>> dataset.getDetails().setExternalInfo("huhly")
>>> print(dataset.getDetails().getExternalInfo())
huhly

omero duplicate Dataset:16505  --report
Using session for trainer-1@outreach.openmicroscopy.org:4064. Idle timeout: 10 min. Current group: Lab1
omero.cmd.Duplicate Dataset:16505 ok
Steps: 14
Elapsed time: 0.019 secs.
Flags: []
Duplicates
  Dataset:16506

Edit Indeed, it rather seems that there is no ExternalInfo set on the duplicate at all. So either I misunderstand, or the situation is worse from the outset than the description suggests ?

>>> dat2 = conn.getObject("Dataset", 16506)
>>> 
>>> print(dat2.getDetails().getExternalInfo())
None
>>> print(dat2.getName())
test
sbesson commented 1 year ago

@pwalczysko ExternalInfo is an object - see https://docs.openmicroscopy.org/omero-blitz/5.5.12/slice2html/omero/model/ExternalInfo.html. dataset.getDetails().setExternalInfo("huhly") will set an invalid value which will be ignored when creating the object server-side using the update service.

Try something along the lines of

e=omero.model.ExternalInfoI()
e.setEntityType(omero.rtypes.rstring("petr"))
e.setEntityId(omero.rtypes.rlong(2))
dataset.getDetails().setExternalInfo(e)
pwalczysko commented 1 year ago
...
e=omero.model.ExternalInfoI()
e.setEntityType(omero.rtypes.rstring("petr"))
e.setEntityId(omero.rtypes.rlong(2))
dat2.getDetails().setExternalInfo(e)
dat2.save()
conn.close()

in another terminal

...
dat2 = conn.getObject("Dataset", 16506)
print(dat2.getDetails().getExternalInfo().getEntityType())
petr

So this seems to persist, thanks

pwalczysko commented 1 year ago
omero duplicate Dataset:16506  --report
Using session for trainer-1@outreach.openmicroscopy.org:4064. Idle timeout: 10 min. Current group: Lab1
omero.cmd.Duplicate Dataset:16506 ok
Steps: 14
Elapsed time: 1.343 secs.
Flags: []
Duplicates
  Dataset:16509
  ExternalInfo:2

Indeed, now I have the ExternalInfo reported by the duplicate cmd.

But, when inspecting the duplicated object, it does not have the ExternalInfo attached, as indicated by @sbesson

dat2 = conn.getObject("Dataset", 16509)
print(dat2.getDetails().getExternalInfo().getEntityType())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'NoneType' object has no attribute 'getEntityType'
print(dat2.getDetails().getExternalInfo())
None

So this matches the expectation for the status without this PR.

pwalczysko commented 1 year ago
omero duplicate Dataset:39469 --report
Using session for user-3@merge-ci-devspace.openmicroscopy.org:4064. Idle timeout: 10 min. Current group: read-only-1
omero.cmd.Duplicate Dataset:39469 ok
Steps: 14
Elapsed time: 1.929 secs.
Flags: []
Duplicates
  Dataset:62869
  ExternalInfo:52
....

dat2 = conn.getObject("Dataset", 62869)
print(dat2.getDetails().getExternalInfo().getEntityType())
petr

See above for the situation on merge-ci, i.e. with this PR, now, the ExternalInfo is present on the duplicate. LGTM