Adding a script that saves data owner as key-value pair

[Rdornier]

Hello,

As suggested, here is a python script implementing ownership tracability as a key-value pair.

The script accepts as input arguments either image ids, container ids (dataset, project, well, plate, screen) or usernames.
For images and containers, their ID is required and the script adds the owner-keyVal to the specified images/containers and all their children.
For users, their omero-username is required and the script adds the owner-keyVal to all data owned by the users, in all groups they are member of.

If the logged-in user is admin, a sudo connection is established.

I created an excel sheet where I consigned the results of different tests I have done but, as you'll see, there is a scenario I do not really understand.
I thought it was possible to add the key-value on data owned by someone in a private group with a sudo connection but, apparently, it is not the case.

Happy to answer any questions and to improve the script.
Thanks and all the best,

Rémy.

[Rdornier]

In the end, it could be nice to integrate this script directly during the ownership changing process (i.e. when clicking on "change ownership") rather than running the script first to add the owner as KVP and then changing the ownership.
But this is the next step.

[joshmoore]

Hey @Rdornier! Thanks for this, and very sorry for having missed it originally. We're now in a bit of a summer gap, but we'll get this in front of some folks for a review ASAP.

[will-moore]

Hi @Rdornier - Apologies for not looking at this sooner...

The script works well, ran as a regular user...

Then I logged-in as an Admin, and changed the owner of an Image to another member of the group (Admin wasn't a member of this group) and I tried running the script again as an Admin.

This attempted to remove the previous annotation, which failed (see error below), so the annotations didn't get updated at-all.
I'm not sure why the Admin didn't have permission to remove the annotation, but probably the new owner also wouldn't have permission to remove the old annotation (unless in a read-write group).

Stack trace

Traceback (most recent call last):
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 58, in remove_map_annotations
    conn.c.waitOnCmd(handle, loops=10, ms=500, failonerror=True,
  File "/opt/omero/server/venv3/lib/python3.10/site-packages/omero/clients.py", line 1020, in waitOnCmd
    raise omero.CmdError(rsp)
omero.CmdError: 
object #0 (::omero::cmd::GraphException)
{
    category = ::omero::cmd::Delete2
    name = graph-fail
    parameters = 
    {
        key = stacktrace
        value = ome.services.graphs.GraphException(message=not permitted to delete ome.model.annotations.MapAnnotation[2516])
	at ome.services.graphs.GraphTraversal.assertPermissions(GraphTraversal.java:1434)
	at ome.services.graphs.GraphTraversal.assertMayBeDeleted(GraphTraversal.java:1407)
	at ome.services.graphs.GraphTraversal.processTargets(GraphTraversal.java:1661)
	at omero.cmd.graphs.Delete2I.step(Delete2I.java:164)
	at omero.cmd.HandleI.steps(HandleI.java:448)
	at omero.cmd.HandleI$RunSteps.innerWork(HandleI.java:509)
	at omero.cmd.HandleI$2.doWork(HandleI.java:383)
	at omero.cmd.HandleI$2.doWork(HandleI.java:380)
	at jdk.internal.reflect.GeneratedMethodAccessor292.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:333)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
	at ome.services.util.Executor$Impl$Interceptor.invoke(Executor.java:568)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at ome.security.basic.EventHandler.invoke(EventHandler.java:154)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.orm.hibernate3.HibernateInterceptor.invoke(HibernateInterceptor.java:119)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:99)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:282)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at ome.tools.hibernate.ProxyCleanupFilter$Interceptor.invoke(ProxyCleanupFilter.java:249)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at ome.services.util.ServiceHandler.invoke(ServiceHandler.java:121)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213)
	at com.sun.proxy.$Proxy81.doWork(Unknown Source)
	at ome.services.util.Executor$Impl.execute(Executor.java:447)
	at omero.cmd.HandleI.run(HandleI.java:379)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at ome.services.util.Executor$Impl$1.call(Executor.java:488)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

        
        key = message
        value = 
    }
    message = not permitted to delete ome.model.annotations.MapAnnotation[2516]
}

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 406, in <module>
    run_script()
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 398, in run_script
    message = add_owner_as_keyval(conn, script_params)
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 321, in add_owner_as_keyval
    nImage += process_image(user_conn, omero_object, key, owner)
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 115, in process_image
    return (1 if annotate_object(conn, image, key, owner)== True else 0)
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 89, in annotate_object
    remove_map_annotations(conn, obj, namespace)
  File "/opt/omero/server/omero/tmp/omero_omero-server/814/processsn_azv25.dir/./script", line 62, in remove_map_annotations
    print("Failed to delete links: {}".format(ex.message))
AttributeError: 'CmdError' object has no attribute 'message'
!! 07/11/23 07:55:22.847 error: communicator not destroyed during global destruction.

If I commented-out the # remove_map_annotations(conn, obj, namespace) line and ran the script again (as Admin) it combines the ownership history nicely, but we are still left with the previous annotation:

I don't know how the permissions will work in your typical use case, but at the very least you should try/catch the removal of annotation (or test if you can delete) so that the script doesn't fail in this case.
Maybe safer to assume that you can never remove previous annotations and always just add a single annotation.

[Rdornier]

Hello @will-moore

Thanks for having a look to my code and to figure this issue.
I actually struggled a bit with this deletion issue and I thought it was fixed.

Anyway, I pushed some modifications and key-values are not deleted anymore. I only add them in the namespace. So this issue should be corrected now.

I don't know how the permissions will work in your typical use case, but at the very least you should try/catch the removal of annotation (or test if you can delete) so that the script doesn't fail in this case.

Yes, I did it only on SecurityViolation, not on CmdError. I should had done it on Exception

Maybe safer to assume that you can never remove previous annotations and always just add a single annotation.

Done

but we are still left with the previous annotation:

This is corrected now. I only link the MapAnnotationWrapper if it does not exist yet (i.e. link it once to the object)