Bug: Chgrp target specified

[jburel]

When moving data between group, specifying the link to save does not seem to work. I am using the doAll method.
I have added 2 tests under OmeroJava integration/chgrp/HierarchyMoveTest
The second one links to a project to be created in the target group.

[jburel]

The second test is probably not allowed but it will be nice to have. I can always create the object before generating the link.

[jmoore]

Tracking this down was far harder than expected, but has lead, I think, to a substantial improvement in the DoAll? and in general submit/handle functionality all together.

Problems included:

• client-side unregistered SaveI
• difference between client-side and server-side tests
• weird tx interactions between commands
• etc.

Now almost all tests pass:

@sprint16-bugfixes $ ./build.py -f components/tools/OmeroJava/build.xml test -DTEST=HierarchyMoveTest -Dtestng.useDefaultListeners=true
...
test:
Entering /Users/moore/NoBackup/_develop/components/tools/OmeroJava...

test-single:
[TestNG] Running:
  Ant suite
PASSED: testMoveBasicImage on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveDatasetToNewProject on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveDatasetToProject on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveDatasetWithSharedImage on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveImage on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveImageWithROIs on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMovePlate on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMovePlateNoPlateAcquisition on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMovePlateWithReagent on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveProject on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveScreen on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveScreenWithReagent on instance null(integration.chgrp.HierarchyMoveTest)
PASSED: testMoveScreenWithSharedPlate on instance null(integration.chgrp.HierarchyMoveTest)
FAILED: testDeletePlateWithROIMeasurements on instance null(integration.chgrp.HierarchyMoveTest)
java.lang.NullPointerException
	at integration.chgrp.HierarchyMoveTest.testDeletePlateWithROIMeasurements(HierarchyMoveTest.java:828)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:74)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:673)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:846)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1170)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:109)
	at org.testng.TestRunner.runWorkers(TestRunner.java:1125)
	at org.testng.TestRunner.privateRun(TestRunner.java:749)
	at org.testng.TestRunner.run(TestRunner.java:600)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:317)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:312)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:274)
	at org.testng.SuiteRunner.run(SuiteRunner.java:223)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1007)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:932)
	at org.testng.TestNG.run(TestNG.java:868)
	at org.testng.TestNG.privateMain(TestNG.java:1150)
	at org.testng.TestNG.main(TestNG.java:1114)
===============================================
    Ant test
    Tests run: 14, Failures: 1, Skips: 0
===============================================
===============================================
Ant suite
Total tests run: 14, Failures: 1, Skips: 0
===============================================
The tests failed.

BUILD SUCCESSFUL
Total time: 36 seconds

The last test failure has nothing to do with chgrp so I'm not going to worry about it for the minute.

Jean-Marie, you might want to review these commits for how we might need to update other integration tests:

fb18d9c - Test fix: Create new project (See #8979) (2 minutes ago) <jmoore>
402ec62 - Test fix: log in to target group on doChange (See #8979) (2 minutes ago) <jmoore>
fe9e41a - Test fix: typo in query (See #8979) (2 minutes ago) <jmoore>
d41bfc9 - Test fix: refresh after group add (See #8979) (2 minutes ago) <jmoore>

Note that both of the forms of move-to-target that you wanted are supported.

[jmoore]

Jean-Marie found another issue and wrote a new test. Re-opening.

[jburel <j.burel@…>]

(In [d32a161c71a91e6904d6da98f7e7182a2a037b12/ome.git] on branch develop) Add test for chgrp see #8979.

[jmoore <josh@…>]

(In [dff981f36f07dab9ff2a2f13f40796938d102846/ome.git] on branch develop) Fix registration of SaveI command (See #8979)

[jmoore <josh@…>]

(In [ca878a64289ab4fe25fda3f38266b019380ad454/ome.git] on branch develop) Improve cancel logic of HandleI (See #8979)

Cancellations should now not overlap since a
catch block should always be present to let
previous Cancel exceptions pass through. Also,
an ERR response should always now be set on
cancellation.

[jmoore <josh@…>]

(In [876543a2f06969b3ceaee2b49315c01e788b9d19/ome.git] on branch develop) Refactor DoAllI for fail-fast (See #8979)

This commit continues the previous work on improving
cancellation and makes all substeps fail-fast, so that
any error produced by a substep immediately propagates
to the client. This makes finding the issues with the
tests *much* simpler.

Tests in DoAllITest are still failing due to Security-
violations and similar.

[jmoore <josh@…>]

(In [a20b1cc49ce7ddc5e918dfadf56cbf50337984a8/ome.git] on branch develop) Attempt at fixing DoAllI with SaveI (See #8979)

This attempt tries to use explicitly setting the group
on the to save objects. It fails with a MixedGroupSecVio?
since the link is left with a group=0 for some reason.

[jmoore <josh@…>]

(In [86b382747898e936ea380516fc277ae0d187f4bc/ome.git] on branch develop) Revert "Attempt at fixing DoAllI with SaveI (See #8979)"

This reverts commit a20b1cc49ce7ddc5e918dfadf56cbf50337984a8.

Other options to be considered:

• Set an explicit group in SaveI
• Use the multiple contexts of DoAllI
• Make ChgrpI -1, and use the new group on submit

[jmoore <josh@…>]

(In [85c4fecd2d3828a739920d33538504cbed40d40e/ome.git] on branch develop) Add ContextMessage?.Push/Pop? for interim login (See #8979)

EventHandle? and CurrentDetails? have long had the ability
to maintain a stack of login information, but the functionality
has been unused.

Now by publishing a ContextMessage? internal services can
either add a login to the stack or pop one off. This will
allow each individual command in a DoAll? to have its own
call context.

This commit also includes a refactoring of DoAllI's state
maintenance in order to have a single method ("substep")
from which login and logout can be managed.

[jmoore <josh@…>]

(In [d280af9e1f8bdcd0c056c6af9cdbd46b659eb76b/ome.git] on branch develop) Better propagation of cancel state to DoAll? (See #8979)

Includes more logging fixes and wholesale copy
from the substatus items to the DoAll? status.

[jmoore <josh@…>]

(In [37efa1b471880c38218bffb2ed348d950f6b3c8b/ome.git] on branch develop) Make ctx a require ctor arg for DoAll? (See #8979)

[jmoore <josh@…>]

(In [6df3bed686f01146d7960c5d8726b72ffc3b56d4/ome.git] on branch develop) Refresh loaded (and stale) chgrp object (See #8979)

In order to check the details on the target object,
ChgrpI first loads the object. Then, that object is
changed at the SQL level so that Hibernate knows
nothing of the change. This causes later commands to
fail since they see the old group id.

[jmoore <josh@…>]

(In [7f7859c6fec20c09828fd3be8258b2f9e6d66411/ome.git] on branch develop) Enable proper security filter for each subcommand (See #8979)

When a command needs to create a sub-login, it's necessary
to disable the reader *before* any changes to the context
so that the filter found by SecurityFilterHolder?.choose()
will have the same name. Similarly, enabling should only
happen *after* the context has been updated.

[jmoore <josh@…>]

(In [d41bfc9318d4230ff73094c658e8c034bcc6886e/ome.git] on branch develop) Test fix: refresh after group add (See #8979)

[jmoore <josh@…>]

(In [fe9e41a62f0ce76137767cc924d2c7d87a571617/ome.git] on branch develop) Test fix: typo in query (See #8979)

[jmoore <josh@…>]

(In [402ec62201a4603e6b9c0f2135d5aad4ec238d9c/ome.git] on branch develop) Test fix: log in to target group on doChange (See #8979)

[jmoore <josh@…>]

(In [fb18d9ce41d12bf678dd9c48954a20554be89139/ome.git] on branch develop) Test fix: Create new project (See #8979)

[jmoore <josh@…>]

(In [14a3f15ca8e9d9da34c8b9da95d2201f22d04c2f/ome.git] on branch develop) Pass call context to HandleI on initialize (See #8979)

Server-side tests were using the the HandleI(ctx) ctor,
which guaranteed a non-null call context for the cmd
execution. Client-side tests were not. This likely was
the cause of a good deal of confusion in testing. Now
the ctor argument has been removed in favor of using
the initialize() method.

[jmoore <josh@…>]

(In [247a8f99aa8846d999af49ad6c0672a79d47f66b/ome.git] on branch develop) Merge branch '8979-doall-chgrp-save' into sprint16-bugfixes (Fix #8979)

[jburel <j.burel@…>]

(In [52d5950cc926e56a9cfc4a93f1a0f205328f9380/ome.git] on branch develop) Review chgrp tests (see #8979)