Skip to content

Commit

Permalink
feat(pagination-Bundle): remove _iter()__ form Bundle class
Browse files Browse the repository at this point in the history
Simplify url sanitization, adjust testse

Replace redundant code on perform()

Add tests perform-iter and perform_resources_iter via Response
  • Loading branch information
LanaNYC committed Oct 28, 2024
1 parent c0da201 commit bd60dc3
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 255 deletions.
2 changes: 1 addition & 1 deletion fhir-parser
40 changes: 14 additions & 26 deletions fhirclient/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ def _fetch_next_page(bundle: 'Bundle') -> Optional['Bundle']:
Returns:
Optional[Bundle]: The next page of results as a FHIR Bundle, or None if no "next" link is found.
"""
next_link = _get_next_link(bundle)
if next_link:
if next_link := _get_next_link(bundle):
sanitized_next_link = _sanitize_next_link(next_link)
return _execute_pagination_request(sanitized_next_link)
return None
Expand All @@ -42,23 +41,28 @@ def _get_next_link(bundle: 'Bundle') -> Optional[str]:

for link in bundle.link:
if link.relation == "next":
return link.url
return _sanitize_next_link(link.url)
return None


def _sanitize_next_link(next_link: str) -> str:
"""
Sanitize the `next` link to ensure it is safe to use.
Sanitize the `next` link by validating its scheme and hostname against the origin server.
This function ensures the `next` link URL uses a valid scheme (`http` or `https`) and that it contains a
hostname. This provides a basic safeguard against malformed URLs without overly restricting flexibility.
Args:
next_link (str): The raw `next` link URL.
origin_server (str): The expected origin server hostname.
Returns:
str: The sanitized URL.
str: The validated URL.
Raises:
ValueError: If the URL scheme or domain is invalid.
ValueError: If the URL's scheme is not `http` or `https`, or if the hostname does not match the origin server.
"""

parsed_url = urllib.parse.urlparse(next_link)

# Validate scheme and netloc (domain)
Expand All @@ -67,26 +71,10 @@ def _sanitize_next_link(next_link: str) -> str:
if not parsed_url.netloc:
raise ValueError("Invalid URL domain in `next` link.")

# Additional sanitization if necessary, e.g., removing dangerous query parameters
query_params = urllib.parse.parse_qs(parsed_url.query)
sanitized_query = {k: v for k, v in query_params.items()}

# Rebuild the sanitized URL
sanitized_url = urllib.parse.urlunparse(
(
parsed_url.scheme,
parsed_url.netloc,
parsed_url.path,
parsed_url.params,
urllib.parse.urlencode(sanitized_query, doseq=True),
parsed_url.fragment,
)
)

return sanitized_url
return next_link


def _execute_pagination_request(sanitized_url: str) -> Optional['Bundle']:
def _execute_pagination_request(sanitized_url: str) -> 'Bundle':
"""
Execute the request to retrieve the next page using the sanitized URL.
Expand All @@ -110,9 +98,9 @@ def _execute_pagination_request(sanitized_url: str) -> Optional['Bundle']:

return next_bundle

except requests.exceptions.HTTPError as e:
except requests.exceptions.HTTPError:
# Handle specific HTTP errors as needed, possibly including retry logic
raise e
raise


def iter_pages(first_bundle: 'Bundle') -> Iterable['Bundle']:
Expand Down
3 changes: 0 additions & 3 deletions fhirclient/models/bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ def __init__(self, jsondict=None, strict=True):

super(Bundle, self).__init__(jsondict=jsondict, strict=strict)

def __iter__(self):
return iter(self.entry or [])

def elementProperties(self):
js = super(Bundle, self).elementProperties()
js.extend([
Expand Down
11 changes: 4 additions & 7 deletions fhirclient/models/fhirsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,9 @@ def perform(self, server) -> 'Bundle':

if server is None:
raise Exception("Need a server to perform search")

from . import bundle as bundle_module
res = server.request_json(self.construct())
bundle = bundle_module.Bundle(res)
bundle.origin_server = server
return bundle

from .bundle import Bundle
return Bundle.read_from(self.construct(), server)

# Use forward references to avoid circular imports
def perform_iter(self, server) -> Iterator['Bundle']:
Expand Down Expand Up @@ -174,7 +171,7 @@ def perform_resources_iter(self, server) -> Iterator['Resource']:
:returns: An iterator of Resource instances
"""
for bundle in self.perform_iter(server):
for entry in bundle:
for entry in bundle.entry:
yield entry.resource


Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ testpaths = "tests"
tests = [
"pytest >= 2.5",
"pytest-cov",
"responses",
]
22 changes: 0 additions & 22 deletions tests/models/bundle_iterator_test.py

This file was deleted.

Loading

0 comments on commit bd60dc3

Please sign in to comment.