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

$everything not working properly #674

Closed
vlad-ignatov opened this Issue Jun 15, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@vlad-ignatov

vlad-ignatov commented Jun 15, 2017

I'm having a lot of problems with the $everything query. In fact I cannot find any reasonable solution so far that would allow me to fetch the patient and everything related to him. Here are the issues that I have encountered so far:

  1. The documentation states that unless the client requests otherwise, the server returns the entire result set in a single bundle (rather than using paging). However, my $everything results are often paginated!

  2. Then I had to write a function that will walk and fetch all the available pages, just to see if that will work. That is tricky, because I have to follow the link[rel="next].url which also contains a hardcoded _format=xml parameter (even though I work with json).

  3. Even if I fetch all the pages, the end result is a mess containing many duplicates. For example the patient and his organization are repeated on every page. Other things (like conditions) might be repeated twice...

Here are some links to see this in action
Patient:
https://sb-fhir-stu3.smarthealthit.org/smartstu3/open/Patient/eab7ef31-1a76-446c-a870-981992760738
Patient/$everything:
https://sb-fhir-stu3.smarthealthit.org/smartstu3/open/Patient/eab7ef31-1a76-446c-a870-981992760738/$everything
Patient/$everything page 2:
https://sb-fhir-stu3.smarthealthit.org/smartstu3/open?_getpages=c55aa040-8687-4731-b8a2-1acb652df764&_getpagesoffset=10&_count=10&_pretty=true&_bundletype=searchset

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Jun 19, 2017

Owner

Hi Vlad,

I'm going to check in a fix to this shortly. Thanks for reporting, and providing a demonostration of the issue!

Unfortunately item 1 on your list won't be fixed though. HAPI's design is not built around streaming results directly from the database (pages are loaded into memory) so an unbounded request is an easy way for a malicious client to overwhelm the server and essentially DOS it. The administrators of the SMART server could certainly increase the page size to something quite large though if they feel that is appropriate.

Items 2 and 3 are fixed on a working branch, which will be merged once the CI passes.

Owner

jamesagnew commented Jun 19, 2017

Hi Vlad,

I'm going to check in a fix to this shortly. Thanks for reporting, and providing a demonostration of the issue!

Unfortunately item 1 on your list won't be fixed though. HAPI's design is not built around streaming results directly from the database (pages are loaded into memory) so an unbounded request is an easy way for a malicious client to overwhelm the server and essentially DOS it. The administrators of the SMART server could certainly increase the page size to something quite large though if they feel that is appropriate.

Items 2 and 3 are fixed on a working branch, which will be merged once the CI passes.

jamesagnew added a commit that referenced this issue Jun 19, 2017

@vlad-ignatov

This comment has been minimized.

Show comment
Hide comment
@vlad-ignatov

vlad-ignatov Jun 19, 2017

Thank you for the quick fix. Maybe here is not the right place but since you made some changes to the prev/next links recently, I wanted to mention one more issue with them: they had urls that are always in http which is a big issue for client-side apps working on https because the browsers will simply refuse to follow those links.

I cannot give you an example because our backend guys had to fix that with some apache module that would overwrite it and also, I can't reproduce it on http://fhirtest.uhn.ca/baseDstu3. Maybe it is fixed already but I wanted to mention it just in case.

vlad-ignatov commented Jun 19, 2017

Thank you for the quick fix. Maybe here is not the right place but since you made some changes to the prev/next links recently, I wanted to mention one more issue with them: they had urls that are always in http which is a big issue for client-side apps working on https because the browsers will simply refuse to follow those links.

I cannot give you an example because our backend guys had to fix that with some apache module that would overwrite it and also, I can't reproduce it on http://fhirtest.uhn.ca/baseDstu3. Maybe it is fixed already but I wanted to mention it just in case.

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Jun 19, 2017

Owner

Hi @vlad-ignatov - We did put some fixes in that may well have resolved this issue (although I don't recall specifically having an "http vs. https" issue in the generated URLs so I'm not totally sure).

One thing to check is whether you're using the HardcodedServerAddressStrategy to fix the server's base URL (details here: http://hapifhir.io/doc_rest_server.html#Configure_the_Servers_IdentityWeb_Address ). If not, this would be the first thing to add.

If this problem persists for you, please do feel free to open a new ticket.

Owner

jamesagnew commented Jun 19, 2017

Hi @vlad-ignatov - We did put some fixes in that may well have resolved this issue (although I don't recall specifically having an "http vs. https" issue in the generated URLs so I'm not totally sure).

One thing to check is whether you're using the HardcodedServerAddressStrategy to fix the server's base URL (details here: http://hapifhir.io/doc_rest_server.html#Configure_the_Servers_IdentityWeb_Address ). If not, this would be the first thing to add.

If this problem persists for you, please do feel free to open a new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment