Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segregate nested objects #295

Merged
merged 10 commits into from
Dec 27, 2018
Merged

Segregate nested objects #295

merged 10 commits into from
Dec 27, 2018

Conversation

vddesai1871
Copy link
Contributor

Fixes #248

Checklist

  • My branch is up-to-date with upstream/develop branch.
  • Everything works and tested for Python 3.5.2 and above.

Description

Segregates objects returned in a response

Test Logs

(h) vishal@vishal:~/oss/hydrus$ pytest hydrus/tests/
============================================================ test session starts ============================================================
platform linux -- Python 3.5.2, pytest-3.4.2, py-1.5.2, pluggy-0.6.0
rootdir: /home/vishal/oss/hydrus, inifile:
collected 51 items                                                                                                                          

hydrus/tests/test_app.py ................                                                                                             [ 31%]
hydrus/tests/test_auth.py ..........                                                                                                  [ 50%]
hydrus/tests/test_crud.py ............                                                                                                [ 74%]
hydrus/tests/test_doc_writer.py ....                                                                                                  [ 82%]
hydrus/tests/test_parser.py .........                                                                                                 [100%]

========================================================= 51 passed in 3.50 seconds =========================================================
(h) vishal@vishal:~/oss/hydrus$ 

Changes log

Added a new resource names 'NestedItem' in app.py which handles the requests for nested objects

Added

  • Feature 1
    Segregates objects returned in a response.
    Only segregates the objects nested at the first level, if the nested object has other nested objects in it then they will be returned in the same response.

Changed

Fixed

Removed

@vddesai1871
Copy link
Contributor Author

vddesai1871 commented Nov 28, 2018

Response for Drone object

drone_ex

Response for State object

state_ex

@vddesai1871
Copy link
Contributor Author

Should we add POST method in NestedItem so clients can modify the particular nested object directly?
Regarding to changes in flock-demo documentation, some primarily nested classes like State do not have any operations defined, I should define GET as supported operation for those classes and check in app.py if the nested class supports the GET operation before returning it. What other chnages need to be made in ApiDoc?

@chrizandr
Copy link
Member

@chrizandr
Copy link
Member

As far as I can understand, if there are mutliple nestings the old method of retrieving objects is followed right?

Please read the comment I posted, I think we can handle multiple nestings and single level nestings using only the modifications in crud, we might not need to add new endpoints for it.

@Mec-iS
Copy link
Contributor

Mec-iS commented Dec 8, 2018

This improvement may save different reads as in the place of the full object should appear only its link to the proper URI. Any Hydra Resource has by definition its own endpoint, so State does.

@vddesai1871
Copy link
Contributor Author

vddesai1871 commented Dec 14, 2018

Response for Drone object

dc

Response for State object

sc

@vddesai1871
Copy link
Contributor Author

(h) vishal@vishal:~/oss/hydrus$ pytest hydrus/tests
============================================================ test session starts ============================================================
platform linux -- Python 3.5.2, pytest-3.4.2, py-1.5.2, pluggy-0.6.0
rootdir: /home/vishal/oss/hydrus, inifile:
collected 51 items                                                                                                                          

hydrus/tests/test_app.py ................                                                                                             [ 31%]
hydrus/tests/test_auth.py ..........                                                                                                  [ 50%]
hydrus/tests/test_crud.py ............                                                                                                [ 74%]
hydrus/tests/test_doc_writer.py ....                                                                                                  [ 82%]
hydrus/tests/test_parser.py .........                                                                                                 [100%]

========================================================= 51 passed in 3.67 seconds =========================================================
(h) vishal@vishal:~/oss/hydrus$ 

@vddesai1871
Copy link
Contributor Author

Other classes besides State in flock-demo-api-doc, which can be returned as nested object are Datastream and Command, both already have collections defined over them and support GET requests. So I have to just add collection for State and define GET operation for State objects in flock-demo-vocab, right?

@Mec-iS
Copy link
Contributor

Mec-iS commented Dec 14, 2018

Every object that is a Resource and it's present inside another Resource should be referencing via its URI. So yes for Datastream and Command in flock-demo and in general for any other object that is instance of HydraResource.
This is the defining part of an hypermedia API, nested objects have to be accessed via URI dereferencing.

@vddesai1871
Copy link
Contributor Author

vddesai1871 commented Dec 15, 2018

This is the defining part of an hypermedia API, nested objects have to be accessed via URI dereferencing.

Yes I know that, and maybe that's why we prefer HTTP-APIs as org name instead of REST-APIs cause REST constraint of hypermedia as the engine of application state is not fulfilled by many self-proclaimed RESTful APIs.
As you can see from the screenshot now server just returns a URI for the nested object, I have also checked it for other examples as well. So the changes needed to be made in the server are done and I have to update the flock-vocab in hydra-flock-vocab repo.
The question I am having is about the changes I have to make in flock-vocab. The repo for drone vocab hydra-flock-vocab has different vocab than one available in the hydrus/examples/drones folder in this repo. Different in the sense that it doesn't define classes like Area, LogEntry, Message and their collections. Why is this the case?

@Mec-iS
Copy link
Contributor

Mec-iS commented Dec 15, 2018

the official vocabulary is the one in the server, nobody updated the version in Github.
HTTP-APIs/hydra-flock-vocab#1 is meant for that.

REST patadigm has a principle called HATEOAS that is rarely applied.

@Mec-iS
Copy link
Contributor

Mec-iS commented Dec 26, 2018

@vddesai1871 I dont see tests for this feature in th ePR.

@vddesai1871
Copy link
Contributor Author

vddesai1871 commented Dec 27, 2018

Initially I was only handling collection classes as nested objects. After following your comment

Every object that is a Resource and it's present inside another Resource should be referencing via its URI.

Now code also handlescollection classes and non collection classes alike which may be present at nested classes in api_doc.
Extended sample doc to have both collection class and non collection class as nested classes (property).
Wrote tests and ran with sample doc and flock-drone doc.

@Mec-iS Mec-iS merged commit 7822454 into HTTP-APIs:develop Dec 27, 2018
@vddesai1871 vddesai1871 deleted the seg_obj branch January 5, 2019 08:26
@Mec-iS Mec-iS mentioned this pull request Mar 24, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants