fix #10564: include wells in plate run processing

[mtbc]

When processing a plate run, also process its wells.

Fixes https://trac.openmicroscopy.org.uk/ome/ticket/10564

To test, try deleting the last plate run of a plate that another user owns and that you have permission to delete. Then, try double-clicking on the plate in Insight and see if the data browser shows thumbnail weirdness.

Also, make sure that AdminServiceTest and HierarchyDeleteTest are still both passing; in the latter, testDeletingMultipleRunsWithOrphanedSamples verifies #1430 (comment).

[mtbc]

Actually, I don't like this at all: I think that we might have to include the extra stuff (well reagent, etc.) on the paths in the /Well specification; I don't know if I have to copy them all over into /PlateAcquisition or if there's a better way.

[mtbc]

This might be interesting to try.

[mtbc]

I've attached 6x4y1z1t1c8b-swatch-2s2p4w2ws.ome to the ticket; that's another plate import that should be experimented with. Also attached is an example of the thumbnail weirdness to which the ticket refers.

In testing, watch out for what should or shouldn't be left behind in the database. For instance, after deleting all the plates, is any of the plate stuff (reagents, whatever) wrongly left in a table? Or, after deleting just a run or two, did something else get screwed up?

Also try out things like moving plates between groups to check for any regressions.

[will-moore]

Imported into private group...

$ omero import ...6x4y1z1t1c8b-swatch-2s2p4w2ws.ome

After import I have:

omero=# select * from screen where id in (251,252);
 id  | description | permissions | name | protocoldescription | protocolidentifier | reagentsetdescription | reagentsetidentifier | type | version | creation_id | external_id | group_id | owner_id | update_id 
-----+-------------+-------------+------+---------------------+--------------------+-----------------------+----------------------+------+---------+-------------+-------------+----------+----------+-----------
 251 |             |        -120 |      |                     |                    |                       |                      |      |         |       40140 |             |        3 |        3 |     40140
 252 | twoScreen   |        -120 |      |                     |                    |                       |                      |      |         |       40140 |             |        3 |        3 |     40140
(2 rows)

omero=# select * from screenplatelink where child in (101,102)
;
 id | permissions | version | child | creation_id | external_id | group_id | owner_id | update_id | parent 
----+-------------+---------+-------+-------------+-------------+----------+----------+-----------+--------
 51 |        -120 |       0 |   101 |       40140 |             |        3 |        3 |     40140 |    251
 52 |        -120 |       0 |   101 |       40140 |             |        3 |        3 |     40140 |    252
 53 |        -120 |       0 |   102 |       40140 |             |        3 |        3 |     40140 |    251
(3 rows)

omero=# select * from well where plate in (101,102);
 id  | alpha | blue | column | permissions | externaldescription | externalidentifier | green | red | row | status | type | version | creation_id | external_id | group_id | owner_id | update_id | plate 
-----+-------+------+--------+-------------+---------------------+--------------------+-------+-----+-----+--------+------+---------+-------------+-------------+----------+----------+-----------+-------
 101 |       |      |      2 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   101
 102 |       |      |      1 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 103 |       |      |      1 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 104 |       |      |      2 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 105 |       |      |      2 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 106 |       |      |      2 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   101
 107 |       |      |      1 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   101
 108 |       |      |      1 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   101
(8 rows)

omero=# select * from wellsample where well in (select id from well where plate in (101,102));
 id  | permissions | posx | posy | timepoint | version | creation_id | external_id | group_id | owner_id | update_id | image | plateacquisition | well | well_index 
-----+-------------+------+------+-----------+---------+-------------+-------------+----------+----------+-----------+-------+------------------+------+------------
 120 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1022 |               53 |  101 |          0
 121 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1002 |               51 |  101 |          1
 101 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1004 |                  |  102 |          0
 105 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1003 |               52 |  102 |          1
 102 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1005 |                  |  102 |          2
 103 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1006 |                  |  102 |          3
 104 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1007 |                  |  102 |          4
 106 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1009 |                  |  103 |          0
 107 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1008 |               52 |  103 |          1
 108 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1011 |                  |  104 |          0
 109 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1010 |               52 |  104 |          1
 110 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1013 |                  |  105 |          0
 111 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1012 |               52 |  105 |          1
 112 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1014 |               53 |  106 |          0
 113 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1015 |               51 |  106 |          1
 114 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1016 |               53 |  107 |          0
 115 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1017 |               51 |  107 |          1
 116 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1018 |                  |  107 |          2
 117 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1019 |                  |  107 |          3
 118 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1020 |                  |  107 |          4
 122 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1001 |               53 |  108 |          0
 119 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1021 |               51 |  108 |          1
(22 rows)

Selected both screens and tried to move to a read-annotate group in Insight - failed.

Blitz log has

2013-09-20 15:03:47,248 WARN  [            ome.services.chgrp.ChgrpStep] (2-thread-2) LogicalChannel:1351 improperly linked by ome.model.core.Channel.logicalChannel: 12
2013-09-20 15:03:47,284 WARN  [            ome.services.chgrp.ChgrpStep] (2-thread-2) LogicalChannel:1351 improperly linked by ome.model.core.Channel.logicalChannel: 11
2013-09-20 15:03:47,332 WARN  [            ome.services.chgrp.ChgrpStep] (2-thread-2) Plate:101 improperly linked by ome.model.screen.ScreenPlateLink.child: 1
2013-09-20 15:03:47,335 WARN  [            ome.services.chgrp.ChgrpStep] (2-thread-2) Reagent:1 improperly linked by ome.model.screen.WellReagentLink.child: 1
2013-09-20 15:03:47,336 INFO  [          ome.services.graphs.GraphState] (2-thread-2) Failed to process Screen/Reagent: 1 due to GraphException: Reagent:1 improperly linked by 1 objects
2013-09-20 15:03:47,336 INFO  [       omero.cmd.graphs.ChgrpI.@18f238fc] (2-thread-2) Cancelled

Tried to delete the plate that is in 2 screens. Deleted OK in Insight.
Then tried to delete the empty screen.
Failed:

2013-09-20 15:07:53,382 INFO  [          ome.services.graphs.GraphState] (2-thread-2) Failed to process Screen/Reagent: 1 due to ConstraintViolation: fkwellreagentlink_child_reagent
2013-09-20 15:07:53,384 INFO  [        omero.cmd.basic.DoAllI.@44ba794d] (2-thread-2) notify cancelled: omero.cmd.ERR@161da59f/omero.cmd.Status@74647510

At this stage DB looks like:

omero=# select * from screen where id in (251,252);
 id  | description | permissions | name | protocoldescription | protocolidentifier | reagentsetdescription | reagentsetidentifier | type | version | creation_id | external_id | group_id | owner_id | update_id 
-----+-------------+-------------+------+---------------------+--------------------+-----------------------+----------------------+------+---------+-------------+-------------+----------+----------+-----------
 251 |             |        -120 |      |                     |                    |                       |                      |      |         |       40140 |             |        3 |        3 |     40140
 252 | twoScreen   |        -120 |      |                     |                    |                       |                      |      |         |       40140 |             |        3 |        3 |     40140
(2 rows)

omero=# select * from screenplatelink where child in (101,102)
;
 id | permissions | version | child | creation_id | external_id | group_id | owner_id | update_id | parent 
----+-------------+---------+-------+-------------+-------------+----------+----------+-----------+--------
 53 |        -120 |       0 |   102 |       40140 |             |        3 |        3 |     40140 |    251
(1 row)

omero=# select * from well where plate in (101,102);
 id  | alpha | blue | column | permissions | externaldescription | externalidentifier | green | red | row | status | type | version | creation_id | external_id | group_id | owner_id | update_id | plate 
-----+-------+------+--------+-------------+---------------------+--------------------+-------+-----+-----+--------+------+---------+-------------+-------------+----------+----------+-----------+-------
 102 |       |      |      1 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 103 |       |      |      1 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 104 |       |      |      2 |        -120 |                     |                    |       |     |   1 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
 105 |       |      |      2 |        -120 |                     |                    |       |     |   2 |        |      |         |       40140 |             |        3 |        3 |     40140 |   102
(4 rows)

omero=# select * from wellsample where well in (select id from well where plate in (101,102));
 id  | permissions | posx | posy | timepoint | version | creation_id | external_id | group_id | owner_id | update_id | image | plateacquisition | well | well_index 
-----+-------------+------+------+-----------+---------+-------------+-------------+----------+----------+-----------+-------+------------------+------+------------
 101 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1004 |                  |  102 |          0
 105 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1003 |               52 |  102 |          1
 102 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1005 |                  |  102 |          2
 103 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1006 |                  |  102 |          3
 104 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1007 |                  |  102 |          4
 106 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1009 |                  |  103 |          0
 107 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1008 |               52 |  103 |          1
 108 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1011 |                  |  104 |          0
 109 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1010 |               52 |  104 |          1
 110 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1013 |                  |  105 |          0
 111 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1012 |               52 |  105 |          1
(11 rows)

After deleting the only Run in the remaining Plate (OK in Insight) this doesn't delete any more wells but some wellsamples go:

omero=# select * from wellsample where well in (select id from well where plate in (101,102));
 id  | permissions | posx | posy | timepoint | version | creation_id | external_id | group_id | owner_id | update_id | image | plateacquisition | well | well_index 
-----+-------------+------+------+-----------+---------+-------------+-------------+----------+----------+-----------+-------+------------------+------+------------
 101 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1004 |                  |  102 |          0
 102 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1005 |                  |  102 |          2
 103 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1006 |                  |  102 |          3
 104 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1007 |                  |  102 |          4
 106 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1009 |                  |  103 |          0
 108 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1011 |                  |  104 |          0
 110 |        -120 |      |      |           |         |       40140 |             |        3 |        3 |     40140 |  1013 |                  |  105 |          0

Oooop, just realised I'm supposed to be deleting as NON-owner (all above was as owner).

[will-moore]

Actually, it seems that the final Run delete above didn't delete the Run. Tried deleting the Plate in Insight (OK) and now everything apart from the 2 screens are gone from the DB.

[will-moore]

Re-imported again (private group)...

omero=# select * from plateacquisition where plate in (103,104);
 id | description | permissions |       endtime       | maximumfieldcount | name |      starttime      | version | creation_id | external_id | group_id | owner_id | update_id | plate 
----+-------------+-------------+---------------------+-------------------+------+---------------------+---------+-------------+-------------+----------+----------+-----------+-------
 54 |             |        -120 | 2010-02-23 12:51:29 |                   |      | 2010-02-23 12:50:30 |         |       40303 |             |        3 |        3 |     40303 |   103
 55 |             |        -120 | 2010-02-23 12:51:29 |                   |      | 2010-02-23 12:50:30 |         |       40303 |             |        3 |        3 |     40303 |   104
 56 |             |        -120 | 2010-02-23 12:50:30 |                   |      | 2010-02-23 12:49:30 |         |       40303 |             |        3 |        3 |     40303 |   103

And tried to delete 'run' 56 as "root". This worked (row disappeared from DB) and 4 images were deleted from DB, but now in web when I try to access the Plate from that 'run' (Plate 103) I get

http://localhost:4080/webgateway/plate/103/0/
("error in call","Traceback (most recent call last):
  File "/Users/will/Desktop/OMERO/dist/lib/python/omeroweb/webgateway/views.py", line 1025, in wrap
    rv = f(request, *args, **kwargs)
  File "/Users/will/Desktop/OMERO/dist/lib/python/omeroweb/webgateway/views.py", line 1189, in plateGrid_json
    i = e.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5056, in getImage
    return wellsample.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5131, in getImage
    return self._getChildWrapper()(self._conn, self._obj.image)
AttributeError: 'NoneType' object has no attribute 'image'

But Insight looks OK, without 'run'

Now deleted the other 'run' using "root" in web. Delete went OK. None of the 'runs' above remain in DB.
Only have 10 of the original 22 images:

omero=# select * from wellsample where well in (select id from well where plate in (103,104));
 id  | permissions | posx | posy | timepoint | version | creation_id | external_id | group_id | owner_id | update_id | image | plateacquisition | well | well_index 
-----+-------------+------+------+-----------+---------+-------------+-------------+----------+----------+-----------+-------+------------------+------+------------
 125 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1028 |                  |  111 |          0
 127 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1030 |                  |  112 |          0
 140 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1042 |                  |  109 |          2
 141 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1043 |                  |  109 |          3
 142 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1044 |                  |  109 |          4
 123 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1026 |                  |  110 |          0
 128 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1031 |                  |  112 |          2
 129 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1032 |                  |  112 |          3
 130 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1033 |                  |  112 |          4
 132 |        -120 |      |      |           |         |       40303 |             |        3 |        3 |     40303 |  1035 |                  |  113 |          0
(10 rows)

Insight has problems - E.g. on plate 103:

[mtbc]

Thanks, @will-moore.

1. Have now added a commit that should correct that deletion error that you saw.
2. We expect some well samples to stay behind after deleting the runs because some of them aren't actually in a run. Could it be that the web client gets improperly upset by well samples that aren't in runs? (Or wells without samples?)
3. The Insight error looks to be how it responds to wells without samples. I don't know if that dialog ought to be reworded.

[mtbc]

I should add, hopefully the chgrp you tried should now work too, let me know. And, thanks very much for the psql and Blitz log extracts, they were most helpful.

[will-moore]

The chgrp proceeds without error in web when I choose to move both Screens, BUT the Plate that was in 2 Screens after import (see above) is only in 1 screen after chgrp.

omero=# select * from screenplatelink where child in (1,2);
 id | permissions | version | child | creation_id | external_id | group_id | owner_id | update_id | parent 
----+-------------+---------+-------+-------------+-------------+----------+----------+-----------+--------
  3 |        -120 |       0 |     2 |          34 |             |        5 |        3 |        34 |      1
  2 |        -120 |       0 |     1 |          34 |             |        5 |        3 |        34 |      2
(2 rows)

Before:

After:

[will-moore]

omero=# select * from plateacquisition where plate in (1,2);
 id | description | permissions |       endtime       | maximumfieldcount | name |      starttime      | version | creation_id | external_id | group_id | owner_id | update_id | plate 
----+-------------+-------------+---------------------+-------------------+------+---------------------+---------+-------------+-------------+----------+----------+-----------+-------
  2 |             |        -120 | 2010-02-23 12:51:29 |                   |      | 2010-02-23 12:50:30 |         |          34 |             |        5 |        3 |        34 |     2
  1 |             |        -120 | 2010-02-23 12:51:29 |                   |      | 2010-02-23 12:50:30 |         |          34 |             |        5 |        3 |        34 |     1
  3 |             |        -120 | 2010-02-23 12:50:30 |                   |      | 2010-02-23 12:49:30 |         |          34 |             |        5 |        3 |        34 |     1
(3 rows)

Deleted the Run under Plate: twoName in Insight. Worked OK and looks good in Insight and same in Web.

After deleting Run 3 in web (worked OK) I get the error above when clicking on the remaining sibling Run:

("error in call","Traceback (most recent call last):
  File "/Users/will/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/webgateway/views.py", line 1025, in wrap
    rv = f(request, *args, **kwargs)
  File "/Users/will/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/webgateway/views.py", line 1189, in plateGrid_json
    i = e.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5056, in getImage
    return wellsample.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5131, in getImage
    return self._getChildWrapper()(self._conn, self._obj.image)
AttributeError: 'NoneType' object has no attribute 'image'

But looks fine in Insight:

Now in Insight I delete the remaining Run as Owner of the read-ann group. Delete works OK, but I am left with only a single well in the Plate:

omero=# select * from well where plate=1;
 id | alpha | blue | column | permissions | externaldescription | externalidentifier | green | red | row | status | type | version | creation_id | external_id | group_id | owner_id | update_id | plate 
----+-------+------+--------+-------------+---------------------+--------------------+-------+-----+-----+--------+------+---------+-------------+-------------+----------+----------+-----------+-------
  7 |       |      |      1 |        -120 |                     |                    |       |     |   2 |        |      |         |          34 |             |        5 |        3 |        34 |     1

Probably not what was expected?

[will-moore]

After discussion with @jburel @mtbc we think that deleting an Acquisition/Run should delete all the linked WellSamples and their linked Images. If any Well becomes empty (no Well Samples) then it should be deleted too.
Any WellSamples (and Wells) that were not in any Run (visible directly under Plate) will not be affected by this delete.

[mtbc]

@will-moore, we do expect one well to be left on plate 1 after all the runs are deleted because one of that plate's wells after initial import has "orphaned" samples. (On the plate with different wells, I think they all do.)

Still, not enough wells are being deleted. @joshmoore, I added /PlateAcquisition/WellSample/Well;SOFT into the /PlateAcquisition bean and the /Well bean has a /Well/WellReagentLink;FORCE, so why, when I delete plate acquisitions and the well samples get deleted, do the wells remain? The only thing keeping the wells are the well-reagent links: if I delete that link in the DB, I can then delete the well, no constraint violations. Is there an easy tweak to the graph traversal spec that will fix that?

[mtbc]

The loss of a plate under chgrp: could that simply be because the two screens aren't moved by the client in the same DoAll? If that's so, it's probably an entirely separate issue.

[joshmoore]

@mtbc: have you tried moving the reagent link deletion earlier in the /Screen spec? (Specifically before the plate and therefore before the plate acquisition)

[mtbc]

I had guessed that the /Screen spec would be irrelevant because I am deleting runs, but I can give it a try. (Unless you're thinking it might fix the chgrp, I'll try that too.)

[joshmoore]

Sorry, the screen spec would be irrelevant then, I was not following the thread apparently. One would certainly think that the Well would get rid of its own reagents (unless they belonged to someone else or similar). A possible bug but I don't have any other evidence for this is that the Well's SOFT is overriding the FORCE of reagent, which could cause this to happen.

Likely unrelatedly, I was wondering why the Well entry to PlateAcquisition is so low down when it's so high up for Plate?

[mtbc]

I just put it low down because we have to get rid of the sample before we get rid of the well so it made sense to me to think of it like that: we delete the sample, then we delete the well only if it has no samples from other runs.

[mtbc]

With a /Foo;SOFT do we actually still try to recurse into the list of "path"s in the /Foo bean?

And, if there were a /Foo without paths, would have to bother making a bean for it? (I'm curious about the annotation link support at the end of spec.xml.) I've tried writing things like,

<bean parent="delSpec" name="/WellReagentLink">
    <constructor-arg>
        <list>
            <value>/WellReagentLink/Reagent;SOFT</value>
            <value>/WellReagentLink</value>
        </list>
    </constructor-arg>
</bean>

but I still come back to the basic problem that we do,

tag[omero.graph.query.PlateAcquisition.WellSample.Well]

but never,

tag[omero.graph.query.PlateAcquisition.WellSample.Well.WellReagentLink]

even though I have a /Well/WellReagentLink;FORCE in the /Well bean and (after deleting the run in the client) I seem to be able to delete the well-reagent-links then wells just fine in psql, and I did try moving the PlateAcquisition/WellSample/Well;SOFT path up in /PlateAcquisition.

I am wondering if I should give up on this one. Separately, I am wondering to what extent the model can be analyzed and rules like "if I have an X I may not actually also want to delete the related Y" could be used to generate a spec.xml and integration tests that make sure the database responds as expected to deletions; at present, given that wells were hanging around even worse before, and the screen fix I added, I wonder to what extent the file is a somewhat ad hoc what-we-thought-of kind of thing without model-driven mechanical guarantees behind its intent or action, and if so how large the job is of fixing that.

[joshmoore]

• We definitely would recurse into SOFT Foos.
• Ah, of course. But only if you re-arrange things. smacks head

Try:

<value>/Well;SOFT;/PlateAcquisition/WellSample</value> 

[mtbc]

Oh! I hadn't realized that I could write /PlateAcquisition after /Well like that with the model not directly linking them.

[mtbc]

(Sorry, where am I meant to be writing this? (-:)

[joshmoore]

Instead of /PlateAcq/WellSample/Well. It's saying, "Use the Well spec which you can lookup by following PlateAcquisition to WellSample"

[mtbc]

Hmm, now it's deleting the wells too successfully: even if they have orphaned samples they go if I delete the run.

[mtbc]

Unless it turns out to be some regression caused by this PR, I'd suggest that the disappearing duplicate plate is a separate DoAll client issue for which I could file an RFE.

[joshmoore]

Code changes look good. I'll leave this for UI testing though. (Do any integration tests need updating?)

[mtbc]

The integration tests looked okay but we should wait for http://hudson.openmicroscopy.org.uk/view/2.%20Stable/job/OmeroJava-integration-stable/ to turn green.

[mtbc]

(I have a grander vision for integration tests for the longer term, though: to the last part of my comment #1430 (comment) I'd add that it'd be great if such tests could somehow, from the analysis of the model, generate sufficiently rich data to really test out that graph traversal: I'd guess that that line I added to /Screen in this PR is just one of several omissions that simply don't get exercised by what we usually import for testing.)

[joshmoore]

The generation of small .ome files which have the content you want has been a pretty good way to do this. We certainly don't have a test for each type of rich graph, but it would certainly be possible to add the specific graphs you're thinking about in this PR.

[will-moore]

Deleted the Plate that is only in a single Screen - this removed half of the images & wellsamples (11/22) from the DB. All OK.
Then deleted one of the Runs from the other Plate.

omero=# select * from wellsample;
 id | permissions | posx | posy | timepoint | version | creation_id | external_id | group_id | owner_id | update_id | image | plateacquisition | well | well_index 
----+-------------+------+------+-----------+---------+-------------+-------------+----------+----------+-----------+-------+------------------+------+------------
 21 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |     2 |                1 |    1 |          1
 12 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    14 |                1 |    6 |          1
 14 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    16 |                1 |    7 |          1
 15 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    17 |                  |    7 |          2
 16 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    18 |                  |    7 |          3
 17 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    19 |                  |    7 |          4
 19 |        -120 |      |      |           |         |          34 |             |        3 |        3 |        34 |    21 |                1 |    8 |          1

At this point, web viewing of the remaining Run fails as before:

("error in call","Traceback (most recent call last):
  File "/Users/will/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/../omeroweb/webgateway/views.py", line 1025, in wrap
    rv = f(request, *args, **kwargs)
  File "/Users/will/Desktop/OMERO/components/tools/OmeroWeb/omeroweb/../omeroweb/webgateway/views.py", line 1189, in plateGrid_json
    i = e.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5060, in getImage
    return wellsample.getImage()
  File "/Users/will/Desktop/OMERO/dist/lib/python/omero/gateway/__init__.py", line 5135, in getImage
    return self._getChildWrapper()(self._conn, self._obj.image)
AttributeError: 'NoneType' object has no attribute 'image'
")

because the WellWrapper._obj has some null WellSamples, so copyWellSample() returns a list of length 2, one item is None. [None, WellSample].

    _wellSamplesSeq = 
    {
        [0] = <nil>
        [1] = object #92 (::omero::model::WellSample)
        {
            _id = object #93 (::omero::RLong)
            {
                _val = 12
            }

This is because the Wells loaded by

query = "select well from Well as well "\
                    "join fetch well.details.creationEvent "\
                    "join fetch well.details.owner join fetch well.details.group " \
                    "left outer join fetch well.plate as pt "\
                    "left outer join fetch well.wellSamples as ws " \
                    "left outer join fetch ws.image as img "\
                    "where well.plate.id = :oid"

each have an additional [None] in their copyWellSample() list E.g. print [ws.id.val for ws in well.copyWellSamples() if ws is not None]

Plate._listChildren...
   well ID 6  copyWellSamples() len 2
    [12L]
   well ID 7  copyWellSamples() len 5
    [14L, 15L, 16L, 17L]
   well ID 1  copyWellSamples() len 2
    [21L]
   well ID 8  copyWellSamples() len 2
    [19L]

[will-moore]

After deleting the last Run, we are left with a single Well that has 3 WellSamples (no Runs). Insight displays this fine, but web fails as before:

http://localhost:8000/webgateway/plate/1/0/

Plate._listChildren...
   well ID 7  copyWellSamples() len 5
    [15L, 16L, 17L]

[will-moore]

I think this PR is now working. Everything is deleted as expected, but we have some strange behaviour in web.
Good to merge!

[mtbc]

Great! Filed http://trac.openmicroscopy.org.uk/ome/ticket/11492 about the chgrp lost-duplicate issue.

[mtbc]

HierarchyDeleteTest.testDeletingScreen() is probably a good start for seeing how to write the integration tests.

[joshmoore]

@mtbc: do you intend for the tests to be added here/now? or are you planning on another PR for that?

[mtbc]

I'm hoping to push a decent test today.

[mtbc]

Had to rebase and force-push to use a recently merged superclass method.

[bpindelski]

Did basic chgrp and plate acquisition deletes in a private group on howe (using user-1 and user-2). No errors in Insight and Web. Also http://hudson.openmicroscopy.org.uk/job/OmeroJava-integration-stable/122/testngreports/integration.delete/integration.delete.HierarchyDeleteTest/testDeletingMultipleRunsWithOrphanedSamples%20on%20instance%20null%28integration.delete.HierarchyDeleteTest%29/? is passing. The peculiar .ome file from the ticket has been used during testing. I've not been able to run into orphaned well samples after deleting a parent plate acquisition. Looks ok to merge.

[mtbc]

Thank you, @bpindelski. Will rebase after #1595 is merged.

[joshmoore]

Thanks all.

[mtbc]

--rebased-to #1600