Test api project dataset update by will-moore · Pull Request #4930 · ome/openmicroscopy

will-moore · 2016-11-08T13:36:59Z

Tests saving of Project with linked Dataset.

Following discussion at ome/omero-marshal#26 (review)
These tests highlight the discussed bug: If we load a Project without child Datasets, then update
the Project and save, all Datasets will get unlinked.
To fix this, we need to call p.unloadDatasetLinks().

Similar tests and logic added for updating of Datasets and Screens.
This now covers all objects that we allow to Save via the json api.

cc @chris-allan @emilroz @joshmoore

joshmoore · 2016-11-09T08:20:56Z

Mostly an informational comment here since I missed the original discussion: with Ice, there's no difference between None and [] so there is an explicit boolean to say that a particular field is unloaded:

      def unloadDatasetLinks(self, current = None):
          self._datasetLinksLoaded = False
          self._datasetLinksSeq = None;

I don't know if it's possible, but if there's a way to represent the difference between "empty set" and "missing set" then it would be possible to track the callers' intent.

will-moore · 2016-11-10T15:22:03Z

It may be that we decide that the web api never breaks links between objects when simply updating those objects. In webclient currently we use /webclient/api/links/ to DELETE or create links between some objects.

will-moore · 2016-11-10T22:34:21Z

Adding an equivalent test for Project Annotation links (instead of Project-Dataset links). This seems to pass without needing to unloadAnnotationLinks() since they are already unloaded, unlike Project-Dataset links.

will-moore · 2016-11-17T13:02:55Z

@chris-allan Thoughts on this PR and how to address this issue for web api?
I think unloading the Datasets for a Project when saving is safest, to avoid breaking links.
Then we can provide other ways of creating/breaking links OR a flag to differentiate loaded from unloaded links in future PRs?

chris-allan · 2016-11-30T09:21:23Z

I haven't really had time to devote to a holistic approach to dealing with unloaded OMERO model objects and am unlikely to for at least another week to ten days. Certainly unloading a collection is a solution to this particular problem, is safe and avoids breaking links. However, unloaded objects are also a problem with "member" objects such as enumerations. The Image-Pixels-Channel hierarchy is littered with examples of this. So, we're going to have to come up with an implementation wide solution to the round-trip problem.

will-moore · 2016-12-05T11:31:39Z

It looks like we're not going to be able to support round-tripping for all the objects that we can read, at least for 5.3.0. I think this is OK as the majority of use-cases initially will be read-only.
Marking some objects (E.g. Images) as "Can't Save" will at least allow us to add support for reading them without exposing round-tripping bugs.

will-moore · 2017-01-10T22:33:54Z

As discussed in meeting http://will-moore.github.io/presentations/2017/Web-Api-Update-Jan-2017/, omero-marshal will need to be smarter to handle decoding of unloaded data so that info is not lost on save.
So the 'fix' from this PR will not be needed but the tests will. This PR can be re-opened when needed.

…ataset_update

jburel · 2017-03-06T11:42:12Z

components/tools/OmeroWeb/test/integration/test_api_containers.py

+        screen_json['Name'] = 'renamed Screen'
+        _csrf_put_json(django_client, save_url, screen_json)
+
+        # Check screen has been updated and still has child Datasets


typo child Plates

jburel · 2017-03-06T11:46:30Z

components/tools/OmeroWeb/test/integration/test_api_projects.py

 import pytest
 from omero.gateway import BlitzGateway
-from omero.model import ProjectI, DatasetI
+from omero.model import ProjectI, DatasetI, TagAnnotationI


alphabetical order

jburel · 2017-03-06T11:48:10Z

components/tools/OmeroWeb/test/integration/test_api_projects.py

+        assert len(list(proj.listChildren())) == dataset_count
+
+        # Get Dataset, update and save back
+        dataset_url = reverse('api_dataset',


I split this test into 2
Projects/Datasets
Datasets/Images

I'll use parametrize to split this into separate tests.

jburel · 2017-03-06T11:48:52Z

components/tools/OmeroWeb/test/integration/test_api_projects.py

+        assert dataset.getName() == 'renamed Dataset'
+        assert len(list(dataset.listChildren())) == image_count
+
+    def test_project_tags_update(self, user1):


This should be tested for other types too

I assumed that Datasets and Screens behave the same way as Projects when being linked to Tags etc. But if we need to test them all, I can parametrize that test.

Better to parametrize, we had surprises before

jburel · 2017-03-07T13:32:59Z

components/tools/OmeroWeb/omeroweb/api/views.py

        # Therefore we ignore any details for now:
        obj.unloadDetails()

+        # Unlink children for Projects, Datasets and Screens to avoid


Probably need a TODO so we can easily find it
and we need a card to review the graph round tripping since we will face it next round with image and roi saving for example

will-moore · 2017-03-08T14:50:06Z

Card created at https://trello.com/c/DrXcE9cH/29-unloaded-objects-round-tripping

will-moore closed this Nov 9, 2016

will-moore reopened this Nov 9, 2016

jburel added the develop label Nov 9, 2016

will-moore added 2 commits November 10, 2016 15:09

Test updating a Project with Datasets

1602175

flake8 fixes

47ca063

will-moore force-pushed the test_api_project_dataset_update branch from 002d80a to 47ca063 Compare November 10, 2016 15:10

Add test_project_tags_update to test_api_projects.py

9358d1f

Remove accidentally-added 'assert False'

99e20f6

will-moore closed this Nov 16, 2016

will-moore reopened this Nov 16, 2016

will-moore mentioned this pull request Nov 28, 2016

Blitz buildQuery params #4950

Merged

will-moore mentioned this pull request Dec 16, 2016

Web API coverage for 5.3.0 ome/design#65

Closed

will-moore closed this Jan 10, 2017

will-moore reopened this Feb 28, 2017

will-moore added 2 commits February 28, 2017 23:48

Merge remote-tracking branch 'origin/develop' into test_api_project_d…

690ac6f

…ataset_update

flake8 and test fixes

6d3555a

will-moore closed this Mar 1, 2017

will-moore reopened this Mar 1, 2017

will-moore added 3 commits March 1, 2017 13:50

Test children not unlinked on update of Dataset & Screen

5799485

Don't unlink children on save of Dataset or Screen

5b3bc6c

flake8 fix

3305099

will-moore closed this Mar 2, 2017

will-moore reopened this Mar 2, 2017

will-moore closed this Mar 5, 2017

will-moore reopened this Mar 5, 2017

jburel reviewed Mar 6, 2017

View reviewed changes

will-moore added 2 commits March 6, 2017 15:43

Various updates to api tests from PR ome#4930

571724d

flake8 fixes

4352b3a

will-moore closed this Mar 6, 2017

will-moore reopened this Mar 6, 2017

jburel reviewed Mar 7, 2017

View reviewed changes

Add TODO flag to comment

240ef76

jburel merged commit 9f4eda3 into ome:develop Mar 14, 2017

jburel added this to the 5.3.0 milestone Mar 29, 2017

will-moore deleted the test_api_project_dataset_update branch February 18, 2019 04:16

Conversation

will-moore commented Nov 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests saving of Project with linked Dataset.

Uh oh!

joshmoore commented Nov 9, 2016

Uh oh!

will-moore commented Nov 10, 2016

Uh oh!

will-moore commented Nov 10, 2016

Uh oh!

will-moore commented Nov 17, 2016

Uh oh!

chris-allan commented Nov 30, 2016

Uh oh!

will-moore commented Dec 5, 2016

Uh oh!

will-moore commented Jan 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

will-moore commented Mar 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

will-moore commented Nov 8, 2016 •

edited

Loading