Bug: testChgrpRdef7825 nows fails with rdef fk constraint

[jamoore]

@sprint3-bugs ~/git/components/tools/OmeroPy $ python  test/integration/chgrp.py  TestChgrp.testChgrpRdef7825
F
======================================================================
FAIL: testChgrpRdef7825 (__main__.TestChgrp)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/integration/chgrp.py", line 195, in testChgrpRdef7825
    self.waitOnCmd(owner_g.c, handle)
  File "/Users/moore/git/components/tools/OmeroPy/test/integration/library.py", line 290, in waitOnCmd
    self.assertEquals(passes, is_ok, str(rsp))
AssertionError: object #0 (::omero::cmd::ERR)
{
    category = ::omero::cmd::Delete
    name = constraint-violation
    parameters = 
    {
        key = stacktrace
        value = org.hibernate.exception.ConstraintViolationException: could not execute update query
	at org.hibernate.exception.SQLStateConverter.convert(SQLStateConverter.java:96)
	at org.hibernate.exception.JDBCExceptionHelper.convert(JDBCExceptionHelper.java:66)
	at org.hibernate.hql.ast.exec.BasicExecutor.execute(BasicExecutor.java:110)
	at org.hibernate.hql.ast.QueryTranslatorImpl.executeUpdate(QueryTranslatorImpl.java:421)
	at org.hibernate.engine.query.HQLQueryPlan.performExecuteUpdate(HQLQueryPlan.java:283)
	at org.hibernate.impl.SessionImpl.executeUpdate(SessionImpl.java:1280)
	at org.hibernate.impl.QueryImpl.executeUpdate(QueryImpl.java:117)
	at ome.services.delete.DeleteStep.action(DeleteStep.java:76)
	at ome.services.graphs.GraphState.execute(GraphState.java:332)
	at ome.services.delete.Deletion.execute(Deletion.java:243)
	at omero.cmd.graphs.DeleteI.step(DeleteI.java:106)
	at omero.cmd.basic.DoAllI$X.step(DoAllI.java:100)
	at omero.cmd.basic.DoAllI.step(DoAllI.java:301)
	at omero.cmd.HandleI.steps(HandleI.java:443)
	at omero.cmd.HandleI.doRun(HandleI.java:418)
	at omero.cmd.HandleI$1.doWork(HandleI.java:365)
	at omero.cmd.HandleI$1.doWork(HandleI.java:361)
	at sun.reflect.GeneratedMethodAccessor243.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:307)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
	at ome.services.util.Executor$Impl$Interceptor.invoke(Executor.java:508)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at ome.security.basic.EventHandler.invoke(EventHandler.java:154)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at org.springframework.orm.hibernate3.HibernateInterceptor.invoke(HibernateInterceptor.java:111)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:108)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at ome.tools.hibernate.ProxyCleanupFilter$Interceptor.invoke(ProxyCleanupFilter.java:241)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at ome.services.util.ServiceHandler.invoke(ServiceHandler.java:116)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
	at $Proxy65.doWork(Unknown Source)
	at ome.services.util.Executor$Impl.execute(Executor.java:406)
	at omero.cmd.HandleI.run(HandleI.java:359)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
	at ome.services.util.Executor$Impl$1.call(Executor.java:437)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
	at java.util.concurrent.FutureTask.run(FutureTask.java:138)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
	at java.lang.Thread.run(Thread.java:680)
Caused by: org.postgresql.util.PSQLException: ERROR: update or delete on table "pixels" violates foreign key constraint "fkrenderingdef_pixels_pixels" on table "renderingdef"
  Detail: Key (id)=(903) is still referenced from table "renderingdef".
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2103)
	at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1836)
	at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:257)
	at org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement.java:512)
	at org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(AbstractJdbc2Statement.java:388)
	at org.postgresql.jdbc2.AbstractJdbc2Statement.executeUpdate(AbstractJdbc2Statement.java:334)
	at sun.reflect.GeneratedMethodAccessor260.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at bitronix.tm.resource.jdbc.BaseProxyHandlerClass.invoke(BaseProxyHandlerClass.java:63)
	at $Proxy56.executeUpdate(Unknown Source)
	at org.hibernate.hql.ast.exec.BasicExecutor.execute(BasicExecutor.java:101)
	... 43 more

        
        key = message
        value = could not execute update query
        
        key = name
        value = fkrenderingdef_pixels_pixels
        
        key = Error
        value = ConstraintViolation: fkrenderingdef_pixels_pixels
    }
}

----------------------------------------------------------------------
Ran 1 test in 4.348s

FAILED (failures=1)

[jmoore]

This is caused by renderingdefs being allowed in rw---- groups, not not properly handled by delete (and perhaps other methods):

44ga=# select id, owner_id, group_id from renderingdef where pixels = 904;
 id  | owner_id | group_id 
-----+----------+----------
 856 |      665 |      670
 857 |      666 |      670
(2 rows)

44ga=# select *, ome_perms(permissions) from experimentergroup where id = 670;
 id  | description | permissions |                 name                 | version | external_id | ome_perms 
-----+-------------+-------------+--------------------------------------+---------+-------------+-----------
 670 |             |        -120 | 82eaa32a-de10-4892-a561-7de6fac3e0e4 |       0 |             | rw----
(1 row)

[jmoore]

This is not going to be straight-forward. As soon as a new sprint is open, I'll push.

[jmoore]

Won't be able to make any progress on this. We may need to add special testing for this. This should hopefully be my last major issue for 4.4.3

[jburel]

Moved from sprint 2012-09-11 (4)

[jmoore]

Investigating this after seeing failing tests during #9435.

[jmoore]

The issue here is that chgrp is moving rdefs from rwrw to a rw. When the user tries to delete them, they are no longer visible (in fact, they might also not be visible during the validation step). Options:

• prevent moving from rwrw to rw completely
• delete the rdefs on chgrp (probably optimal)
• allow reading of rdef's regardless of rw---- (good, but perhaps too large)

[jmoore]

In a discussion with Chris, we decided to go for the simplest fix for 4.4.4, namely that delete should properly handle the tricky, hidden rdefs. We may have to do more to fix this during the permissions review. Moving to blocker since that work needs to be done for 4.4.4

[jmoore]

Test is now passing. This was achieved by creating a RenderingDefGraphSpec class which uses SQL directly to perform the delete AND by permitting queryBackupIds to run without filters (which may return more objects but will possibly do so faster).

The remaining work which couldn't be done for 4.4.4 should now be done by #9610 for 4.5.0.

The fix for this has beened pushed to ​https://github.com/openmicroscopy/openmicroscopy/pull/354 for review. (Along with work for #9435)

[jmoore <josh@…>]

(In [a704d8cc409ad9fd943b5bcb12f2f82db1f424b7/ome.git] on branch develop) Fix and review all integration/chgrp tests (See #9435, #9496)

~22 tests were failing for various reasons. Here the changes
that were necessary:

• Better handle a NPE in ChgrpI by raising "no-object"

• Added calls to IAdmin.getEventContext to refresh group to fix tests that were failing with "non-member"

• Set enabled=false to tests that were failling with "non-owner"

• Changed one rw---- permissions to rwrw-- to match javadoc

• Added "#9496" to all tests which seem to be failing on annotations or renderingdefs. These are still failing.

[jmoore <josh@…>]

(In [acc9d59a8afe21b185f233afccebc462aa975954/ome.git] on branch develop) Fix delete tests with s/rwrw/rwra/ (See #9496)

Before changing the delete implementaiton for 9496,
a review of the existing test shows many to be failing.
These are failing because deletion by group members
is now possible in rwrw.

[jmoore <josh@…>]

(In [16530d59e4539c478a1ac2db13eb5b794d15c536/ome.git] on branch develop) Reduce HQL filters during querying (Fix #9496)

BaseGraphSpec?.queryBackupIds no longer uses HQL filters at all. This may
tend to return more IDs which will be filtered out by the UPDATE and
DELETE statements but the queries themselves may actually tend to run
faster. At the same time, the filters for RenderingDefs have been
removed by the introduction of RenderingDefGraphSpec which uses raw
SQL, like LightSourceGRaphSpec.

[jmoore <josh@…>]

(In [e71f936b6d24710a7bf4c03e19340c06c25cacf0/ome.git] on branch develop) Permit 'select distinct on' raw SQL (See #11054, #9496)

Previously, only UPDATE and DELETE statements were allowed.
HQL, however, does not properly support distinct on (...)
and therefore it was necessary to include this workaround.

[jmoore <josh@…>]

(In [79ea4922c2a00df1202c9983ac6ee370569b19e0/ome.git] on branch develop) Revert "Permit 'select distinct on' raw SQL (See #11054, #9496)"

This reverts commit e71f936b6d24710a7bf4c03e19340c06c25cacf0.

This workaround was intended only for allowing use of SQL's
"select distinct on (...)". Since another solution was found
for the same problem (namely dropping all the intermediate
IDs from the select list), this need no longer be permitted.