From ab069100bacabff3f51a8239f1180487260dcc56 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Thu, 25 Feb 2021 07:29:34 +0000 Subject: [PATCH 01/18] Remove obsolete reference to `dicomweb_client.error` module from `package.rst`. --- docs/package.rst | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/package.rst b/docs/package.rst index 2ccb856..ba63eef 100644 --- a/docs/package.rst +++ b/docs/package.rst @@ -28,14 +28,6 @@ dicomweb\_client.cli module .. autoprogram:: dicomweb_client.cli:_get_parser() :prog: dicomweb_client -dicomweb\_client.error module -+++++++++++++++++++++++++++++ - -.. automodule:: dicomweb_client.error - :members: - :undoc-members: - :show-inheritance: - dicomweb\_client.log module +++++++++++++++++++++++++++ From 1297dccb4cf26d03e603ee54ea4bf40752a8f7c9 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Thu, 25 Feb 2021 09:01:57 +0000 Subject: [PATCH 02/18] Add `create_chc_uri()` to `uri.py`. --- src/dicomweb_client/uri.py | 60 ++++++++++++++++++++++++++++++++++++++ tests/test_uri.py | 31 +++++++++++++++++++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 644e59a..9df3684 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -438,6 +438,66 @@ def from_string(cls, return uri +def create_chc_uri( + project_id: str, + location: str, + dataset_id: str, + dicom_store_id: str, + study_instance_uid: Optional[str] = None, + series_instance_uid: Optional[str] = None, + sop_instance_uid: Optional[str] = None, + frames: Optional[Sequence[int]] = None, + suffix: Optional[URISuffix] = None) -> URI: + """Creates a :py:class:`URI` based on the `Google Cloud Healthcare (CHC) API`_ DICOMweb Service. + + The :py:attr:`URI.base_url` in the returned instance corresponds to the v1_ + API. + + .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide + .. _Google Cloud Healthcare (CHC) API: https://cloud.google.com/healthcare + + Parameters + ---------- + project_id: str + The ID of the `GCP Project + `_ + that contains the CHC DICOM Store (see ``dicom_store_id`` below). + location: str + The `Region name + `_ of the + geographic location configured for the CHC Dataset (see ``dataset_id`` + below). + dataset_id: str + The ID of the `CHC Dataset + `_ + that contains the CHC DICOM Store (see ``dicom_store_id`` below). + dicom_store_id: str + The ID of the `CHC DICOM Store + `_ + housing the DICOMs referenced by the returned :py:class:`URI`. + study_instance_uid: str, optional + Identical to its :py:class:`URI` counterpart. + series_instance_uid: str, optional + Identical to its :py:class:`URI` counterpart. + sop_instance_uid: str, optional + Identical to its :py:class:`URI` counterpart. + frames: Sequence[int], optional + Identical to its :py:class:`URI` counterpart. + suffix: URISuffix, optional + Identical to its :py:class:`URI` counterpart. + + Returns + ------- + URI + The newly constructed `URI` object. + """ + base_url = ('https://healthcare.googleapis.com/v1/' + f'projects/{project_id}/locations/{location}/' + f'datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb') + return URI(base_url, study_instance_uid, series_instance_uid, + sop_instance_uid, frames, suffix) + + def _validate_base_url(url: str) -> None: """Validates the Base (DICOMweb service) URL supplied to `URI`.""" parse_result = urlparse.urlparse(url) diff --git a/tests/test_uri.py b/tests/test_uri.py index 1875bd0..cf8558d 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -1,4 +1,4 @@ -from dicomweb_client.uri import URI, URISuffix, URIType +from dicomweb_client.uri import create_chc_uri, URI, URISuffix, URIType import pytest @@ -14,6 +14,21 @@ _INSTANCE_URI = f'{_SERIES_URI}/instances/{_INSTANCE_UID}' _FRAME_URI = f'{_INSTANCE_URI}/frames/{",".join(str(f) for f in _FRAMES)}' +# CHC DICOMweb URI parameters. +_PROJECT_ID = 'my-project' +_LOCATION = 'us-central1' +_DATASET_ID = 'my-dataset' +_DICOM_STORE_ID = 'my_dicom_store' +_CHC_BASE_URL = ( + 'https://healthcare.googleapis.com/v1/' + f'projects/{_PROJECT_ID}/locations/{_LOCATION}/' + f'datasets/{_DATASET_ID}/dicomStores/{_DICOM_STORE_ID}/dicomWeb') +_CHC_STUDY_URI = f'{_CHC_BASE_URL}/studies/{_STUDY_UID}' +_CHC_SERIES_URI = f'{_CHC_STUDY_URI}/series/{_SERIES_UID}' +_CHC_INSTANCE_URI = f'{_CHC_SERIES_URI}/instances/{_INSTANCE_UID}' +_CHC_FRAME_URI = ( + f'{_CHC_INSTANCE_URI}/frames/{",".join(str(f) for f in _FRAMES)}') + @pytest.mark.parametrize('illegal_char', ['/', '@', 'a', 'A']) def test_uid_illegal_character(illegal_char): @@ -433,3 +448,17 @@ def test_update_error(uri_args, update_args, error_msg): """Tests for failure if the `URI` returned by `update()` is invalid.""" with pytest.raises(ValueError, match=error_msg): URI(*uri_args).update(*update_args) + + +@pytest.mark.parametrize('args,expected_uri', [ + ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID), _CHC_BASE_URL), + ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID), + _CHC_STUDY_URI), + ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID, + _SERIES_UID), _CHC_SERIES_URI), + ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID, + _SERIES_UID, _INSTANCE_UID), _CHC_INSTANCE_URI), +]) +def test_create_chc_uri(args, expected_uri): + """Locks down implementation of `create_chc_uri()`.""" + assert str(create_chc_uri(*args)) == expected_uri From 9dd6c818f1d3a5e0fe46baa1ebcfd36514b9954f Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 07:02:50 +0000 Subject: [PATCH 03/18] Repackage `create_chc_uri()` under new class `CloudHealthcareDICOMStore`. --- src/dicomweb_client/uri.py | 107 ++++++++++++++++++------------------- tests/test_uri.py | 31 +++++------ 2 files changed, 65 insertions(+), 73 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 9df3684..4fdfb2f 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -1,4 +1,5 @@ """Utilities for DICOMweb URI manipulation.""" +import attr import enum import re from typing import Optional, Sequence, Tuple @@ -24,6 +25,11 @@ class URISuffix(enum.Enum): # For DICOM Standard spec validation of UID components in `URI`. _MAX_UID_LENGTH = 64 _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') +# Used for Project ID and Location validation in `CloudHealthcareDICOMStore`. +_ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(r'[\w-]+') +# Used for Dataset ID and DICOM Store ID validation in +# `CloudHealthcareDICOMStore`. +_ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(r'[\w.-]+') class URI: @@ -438,64 +444,55 @@ def from_string(cls, return uri -def create_chc_uri( - project_id: str, - location: str, - dataset_id: str, - dicom_store_id: str, - study_instance_uid: Optional[str] = None, - series_instance_uid: Optional[str] = None, - sop_instance_uid: Optional[str] = None, - frames: Optional[Sequence[int]] = None, - suffix: Optional[URISuffix] = None) -> URI: - """Creates a :py:class:`URI` based on the `Google Cloud Healthcare (CHC) API`_ DICOMweb Service. - - The :py:attr:`URI.base_url` in the returned instance corresponds to the v1_ - API. +@attr.s(frozen=True) +class CloudHealthcareDICOMStore: + """Base URL helper for DICOM Stores under the `Google Cloud Healthcare API`_. + + This class facilitates interaction of :py:class:`URI` instances with + URLs derived from attributes specific to the v1_ API. + + The string representation of its instances may be used as + :py:attr:`URI.base_url`. The returned URL is of the form: + + ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` + + .. _Google Cloud Healthcare API: https://cloud.google.com/healthcare .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide - .. _Google Cloud Healthcare (CHC) API: https://cloud.google.com/healthcare - - Parameters - ---------- - project_id: str - The ID of the `GCP Project - `_ - that contains the CHC DICOM Store (see ``dicom_store_id`` below). - location: str - The `Region name - `_ of the - geographic location configured for the CHC Dataset (see ``dataset_id`` - below). - dataset_id: str - The ID of the `CHC Dataset - `_ - that contains the CHC DICOM Store (see ``dicom_store_id`` below). - dicom_store_id: str - The ID of the `CHC DICOM Store - `_ - housing the DICOMs referenced by the returned :py:class:`URI`. - study_instance_uid: str, optional - Identical to its :py:class:`URI` counterpart. - series_instance_uid: str, optional - Identical to its :py:class:`URI` counterpart. - sop_instance_uid: str, optional - Identical to its :py:class:`URI` counterpart. - frames: Sequence[int], optional - Identical to its :py:class:`URI` counterpart. - suffix: URISuffix, optional - Identical to its :py:class:`URI` counterpart. - - Returns - ------- - URI - The newly constructed `URI` object. + + Attributes: + project_id: str + The ID of the `GCP Project + `_ + that contains the DICOM Store. + location: str + The `Region name + `_ of the + geographic location configured for the Dataset to which the DICOM + Store belongs. + dataset_id: str + The ID of the `Dataset + `_ + that contains the DICOM Store. + dicom_store_id: str + The ID of the `DICOM Store + `_. """ - base_url = ('https://healthcare.googleapis.com/v1/' - f'projects/{project_id}/locations/{location}/' - f'datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb') - return URI(base_url, study_instance_uid, series_instance_uid, - sop_instance_uid, frames, suffix) + project_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_1) + location = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_1) + dataset_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) + dicom_store_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) + + # The URL for the CHC API endpoint. + _API_URL = 'https://healthcare.googleapis.com/v1' + + def __str__(self) -> str: + """Returns a string URL for use as :py:attr:`URI.base_url`.""" + return (f'{self._API_URL}/' + f'projects/{self.project_id}/' + f'locations/{self.location}/' + f'datasets/{self.dataset_id}/' + f'dicomStores/{self.dicom_store_id}/dicomWeb') def _validate_base_url(url: str) -> None: diff --git a/tests/test_uri.py b/tests/test_uri.py index cf8558d..622ed9d 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -1,4 +1,8 @@ -from dicomweb_client.uri import create_chc_uri, URI, URISuffix, URIType +from dicomweb_client.uri import ( + CloudHealthcareDICOMStore, + URI, + URISuffix, + URIType) import pytest @@ -23,11 +27,6 @@ 'https://healthcare.googleapis.com/v1/' f'projects/{_PROJECT_ID}/locations/{_LOCATION}/' f'datasets/{_DATASET_ID}/dicomStores/{_DICOM_STORE_ID}/dicomWeb') -_CHC_STUDY_URI = f'{_CHC_BASE_URL}/studies/{_STUDY_UID}' -_CHC_SERIES_URI = f'{_CHC_STUDY_URI}/series/{_SERIES_UID}' -_CHC_INSTANCE_URI = f'{_CHC_SERIES_URI}/instances/{_INSTANCE_UID}' -_CHC_FRAME_URI = ( - f'{_CHC_INSTANCE_URI}/frames/{",".join(str(f) for f in _FRAMES)}') @pytest.mark.parametrize('illegal_char', ['/', '@', 'a', 'A']) @@ -450,15 +449,11 @@ def test_update_error(uri_args, update_args, error_msg): URI(*uri_args).update(*update_args) -@pytest.mark.parametrize('args,expected_uri', [ - ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID), _CHC_BASE_URL), - ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID), - _CHC_STUDY_URI), - ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID, - _SERIES_UID), _CHC_SERIES_URI), - ((_PROJECT_ID, _LOCATION, _DATASET_ID, _DICOM_STORE_ID, _STUDY_UID, - _SERIES_UID, _INSTANCE_UID), _CHC_INSTANCE_URI), -]) -def test_create_chc_uri(args, expected_uri): - """Locks down implementation of `create_chc_uri()`.""" - assert str(create_chc_uri(*args)) == expected_uri +def test_chc_dicom_store_str(): + """Locks down `CloudHealthcareDICOMStore.__str__()`.""" + assert str( + CloudHealthcareDICOMStore( + _PROJECT_ID, + _LOCATION, + _DATASET_ID, + _DICOM_STORE_ID)) == _CHC_BASE_URL From 54612848a861f219a2ce94f3296ab9ff349a1aa1 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 07:15:44 +0000 Subject: [PATCH 04/18] Update documentation for `CloudHealthcareDICOMStore`. --- src/dicomweb_client/uri.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 4fdfb2f..835e9fe 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -449,13 +449,9 @@ def from_string(cls, class CloudHealthcareDICOMStore: """Base URL helper for DICOM Stores under the `Google Cloud Healthcare API`_. - This class facilitates interaction of :py:class:`URI` instances with - URLs derived from attributes specific to the v1_ API. + This class facilitates the parsing and creation of :py:attr:`URI.base_url` + corresponding to DICOMweb API Service URLs under the v1_ API. - The string representation of its instances may be used as - :py:attr:`URI.base_url`. The returned URL is of the form: - - ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` .. _Google Cloud Healthcare API: https://cloud.google.com/healthcare .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide @@ -468,8 +464,8 @@ class CloudHealthcareDICOMStore: location: str The `Region name `_ of the - geographic location configured for the Dataset to which the DICOM - Store belongs. + geographic location configured for the Dataset that contains the + DICOM Store. dataset_id: str The ID of the `Dataset `_ @@ -487,7 +483,12 @@ class CloudHealthcareDICOMStore: _API_URL = 'https://healthcare.googleapis.com/v1' def __str__(self) -> str: - """Returns a string URL for use as :py:attr:`URI.base_url`.""" + """Returns a string URL for use as :py:attr:`URI.base_url`. + + The returned URL is of the form: + + ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` + """ return (f'{self._API_URL}/' f'projects/{self.project_id}/' f'locations/{self.location}/' From ea9bc22399f18865f813531dbc6554ba56ff2488 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 08:14:40 +0000 Subject: [PATCH 05/18] Add `CloudHealthcareDICOMStore.from_url()`. --- src/dicomweb_client/uri.py | 59 +++++++++++++++++++++++++++++--------- tests/test_uri.py | 37 +++++++++++++++++++++++- 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 835e9fe..a5ed5b8 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -26,10 +26,12 @@ class URISuffix(enum.Enum): _MAX_UID_LENGTH = 64 _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') # Used for Project ID and Location validation in `CloudHealthcareDICOMStore`. -_ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(r'[\w-]+') +_REGEX_ID_1 = r'[\w-]+' +_ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(_REGEX_ID_1) # Used for Dataset ID and DICOM Store ID validation in # `CloudHealthcareDICOMStore`. -_ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(r'[\w.-]+') +_REGEX_ID_2 = r'[\w.-]+' +_ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(_REGEX_ID_2) class URI: @@ -444,18 +446,18 @@ def from_string(cls, return uri - @attr.s(frozen=True) class CloudHealthcareDICOMStore: """Base URL helper for DICOM Stores under the `Google Cloud Healthcare API`_. - + This class facilitates the parsing and creation of :py:attr:`URI.base_url` - corresponding to DICOMweb API Service URLs under the v1_ API. - - + corresponding to DICOMweb API Service URLs under the v1_ API. The URLs are + of the form: + ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` + .. _Google Cloud Healthcare API: https://cloud.google.com/healthcare .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide - + Attributes: project_id: str The ID of the `GCP Project @@ -478,16 +480,14 @@ class CloudHealthcareDICOMStore: location = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_1) dataset_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) dicom_store_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) - + # The URL for the CHC API endpoint. _API_URL = 'https://healthcare.googleapis.com/v1' - + def __str__(self) -> str: """Returns a string URL for use as :py:attr:`URI.base_url`. - The returned URL is of the form: - - ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` + See class docstring for the returned URL format. """ return (f'{self._API_URL}/' f'projects/{self.project_id}/' @@ -495,6 +495,39 @@ def __str__(self) -> str: f'datasets/{self.dataset_id}/' f'dicomStores/{self.dicom_store_id}/dicomWeb') + @classmethod + def from_url(cls, base_url: str) -> 'CloudHealthcareDICOMStore': + """Creates an instance from ``base_url``. + + Parameters + ---------- + base_url: str + The URL for the DICOMweb API Service endpoint corresponding to a + `CHC API DICOM Store + `_. + See class docstring for supported formats. + + Raises + ------ + ValueError + If ``base_url`` does not match the specifications in the class + docstring. + """ + if not base_url.startswith(f'{cls._API_URL}/'): + raise ValueError('Invalid CHC API v1 URL: {base_url!r}') + resource_suffix = base_url[len(cls._API_URL) + 1:] + + store_regex = (r'projects/(%s)/locations/(%s)/datasets/(%s)/' + r'dicomStores/(%s)/dicomWeb$') % ( + _REGEX_ID_1, _REGEX_ID_1, _REGEX_ID_2, _REGEX_ID_2) + store_match = re.match(store_regex, resource_suffix) + if store_match is None: + raise ValueError( + 'Invalid CHC API v1 DICOM Store name: {resource_suffix!r}') + + return cls(store_match.group(1), store_match.group(2), + store_match.group(3), store_match.group(4)) + def _validate_base_url(url: str) -> None: """Validates the Base (DICOMweb service) URL supplied to `URI`.""" diff --git a/tests/test_uri.py b/tests/test_uri.py index 622ed9d..7757a61 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -23,8 +23,9 @@ _LOCATION = 'us-central1' _DATASET_ID = 'my-dataset' _DICOM_STORE_ID = 'my_dicom_store' +_CHC_API_URL = 'https://healthcare.googleapis.com/v1' _CHC_BASE_URL = ( - 'https://healthcare.googleapis.com/v1/' + f'{_CHC_API_URL}/' f'projects/{_PROJECT_ID}/locations/{_LOCATION}/' f'datasets/{_DATASET_ID}/dicomStores/{_DICOM_STORE_ID}/dicomWeb') @@ -457,3 +458,37 @@ def test_chc_dicom_store_str(): _LOCATION, _DATASET_ID, _DICOM_STORE_ID)) == _CHC_BASE_URL + + +@pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) +def test_chc_dicom_store_from_url_invalid_api(url): + """Tests for bad API URL error`CloudHealthcareDICOMStore.from_url()`.""" + with pytest.raises(ValueError, match='v1 URL'): + CloudHealthcareDICOMStore.from_url(url) + + +@pytest.mark.parametrize('url', [ + f'{_CHC_BASE_URL}/', # Trailing slash disallowed. + f'{_CHC_API_URL}/project/p/locations/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/location/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/dataset/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomStore/ds/dicomWeb', + f'{_CHC_API_URL}/locations/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' +]) +def test_chc_dicom_store_from_url_invalid_store_name(url): + """Tests for bad Store name `CloudHealthcareDICOMStore.from_url()`.""" + with pytest.raises(ValueError, match='v1 DICOM'): + CloudHealthcareDICOMStore.from_url(url) + + +def test_chc_dicom_from_url_success(): + """Locks down `CloudHealthcareDICOMStore.from_url()`.""" + store = CloudHealthcareDICOMStore.from_url(_CHC_BASE_URL) + assert store.project_id == _PROJECT_ID + assert store.location == _LOCATION + assert store.dataset_id == _DATASET_ID + assert store.dicom_store_id == _DICOM_STORE_ID From 69204ad7a2ae51160c51de637cc3e21ee7e601da Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 08:18:18 +0000 Subject: [PATCH 06/18] Rename `CloudHealthcareDICOMStore` to `GoogleCloudHealthcare`. --- src/dicomweb_client/uri.py | 8 ++++---- tests/test_uri.py | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index a5ed5b8..922fbd2 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -25,11 +25,11 @@ class URISuffix(enum.Enum): # For DICOM Standard spec validation of UID components in `URI`. _MAX_UID_LENGTH = 64 _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') -# Used for Project ID and Location validation in `CloudHealthcareDICOMStore`. +# Used for Project ID and Location validation in `GoogleCloudHealthcare`. _REGEX_ID_1 = r'[\w-]+' _ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(_REGEX_ID_1) # Used for Dataset ID and DICOM Store ID validation in -# `CloudHealthcareDICOMStore`. +# `GoogleCloudHealthcare`. _REGEX_ID_2 = r'[\w.-]+' _ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(_REGEX_ID_2) @@ -447,7 +447,7 @@ def from_string(cls, @attr.s(frozen=True) -class CloudHealthcareDICOMStore: +class GoogleCloudHealthcare: """Base URL helper for DICOM Stores under the `Google Cloud Healthcare API`_. This class facilitates the parsing and creation of :py:attr:`URI.base_url` @@ -496,7 +496,7 @@ def __str__(self) -> str: f'dicomStores/{self.dicom_store_id}/dicomWeb') @classmethod - def from_url(cls, base_url: str) -> 'CloudHealthcareDICOMStore': + def from_url(cls, base_url: str) -> 'GoogleCloudHealthcare': """Creates an instance from ``base_url``. Parameters diff --git a/tests/test_uri.py b/tests/test_uri.py index 7757a61..aeb389b 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -1,5 +1,5 @@ from dicomweb_client.uri import ( - CloudHealthcareDICOMStore, + GoogleCloudHealthcare, URI, URISuffix, URIType) @@ -451,9 +451,9 @@ def test_update_error(uri_args, update_args, error_msg): def test_chc_dicom_store_str(): - """Locks down `CloudHealthcareDICOMStore.__str__()`.""" + """Locks down `GoogleCloudHealthcare.__str__()`.""" assert str( - CloudHealthcareDICOMStore( + GoogleCloudHealthcare( _PROJECT_ID, _LOCATION, _DATASET_ID, @@ -462,9 +462,9 @@ def test_chc_dicom_store_str(): @pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) def test_chc_dicom_store_from_url_invalid_api(url): - """Tests for bad API URL error`CloudHealthcareDICOMStore.from_url()`.""" + """Tests for bad API URL error`GoogleCloudHealthcare.from_url()`.""" with pytest.raises(ValueError, match='v1 URL'): - CloudHealthcareDICOMStore.from_url(url) + GoogleCloudHealthcare.from_url(url) @pytest.mark.parametrize('url', [ @@ -480,14 +480,14 @@ def test_chc_dicom_store_from_url_invalid_api(url): f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' ]) def test_chc_dicom_store_from_url_invalid_store_name(url): - """Tests for bad Store name `CloudHealthcareDICOMStore.from_url()`.""" + """Tests for bad Store name `GoogleCloudHealthcare.from_url()`.""" with pytest.raises(ValueError, match='v1 DICOM'): - CloudHealthcareDICOMStore.from_url(url) + GoogleCloudHealthcare.from_url(url) def test_chc_dicom_from_url_success(): - """Locks down `CloudHealthcareDICOMStore.from_url()`.""" - store = CloudHealthcareDICOMStore.from_url(_CHC_BASE_URL) + """Locks down `GoogleCloudHealthcare.from_url()`.""" + store = GoogleCloudHealthcare.from_url(_CHC_BASE_URL) assert store.project_id == _PROJECT_ID assert store.location == _LOCATION assert store.dataset_id == _DATASET_ID From 81dc4c5a8427e951a900195ae40c0c90f210862c Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 08:41:21 +0000 Subject: [PATCH 07/18] Add tests for `GoogleCloudHealthcare` validators. --- tests/test_uri.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/test_uri.py b/tests/test_uri.py index aeb389b..75855aa 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -19,10 +19,10 @@ _FRAME_URI = f'{_INSTANCE_URI}/frames/{",".join(str(f) for f in _FRAMES)}' # CHC DICOMweb URI parameters. -_PROJECT_ID = 'my-project' +_PROJECT_ID = 'my-project44' _LOCATION = 'us-central1' -_DATASET_ID = 'my-dataset' -_DICOM_STORE_ID = 'my_dicom_store' +_DATASET_ID = 'my-44.dataset' +_DICOM_STORE_ID = 'my.d1com_store' _CHC_API_URL = 'https://healthcare.googleapis.com/v1' _CHC_BASE_URL = ( f'{_CHC_API_URL}/' @@ -460,8 +460,26 @@ def test_chc_dicom_store_str(): _DICOM_STORE_ID)) == _CHC_BASE_URL +@pytest.mark.parametrize('name', ['hmmm.1', '#95', '43/']) +def test_chc_invalid_project_or_location(name): + """Tests `GoogleCloudHealthcare` for bad `project_id`, `location`.""" + with pytest.raises(ValueError): + GoogleCloudHealthcare(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) + with pytest.raises(ValueError): + GoogleCloudHealthcare(_PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + + +@pytest.mark.parametrize('name', ['hmmm.!', '#95', '43/']) +def test_chc_invalid_dataset_or_store(name): + """Tests `GoogleCloudHealthcare` for bad `dataset_id`, `dicom_store_id`.""" + with pytest.raises(ValueError): + GoogleCloudHealthcare(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) + with pytest.raises(ValueError): + GoogleCloudHealthcare(_PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + + @pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) -def test_chc_dicom_store_from_url_invalid_api(url): +def test_chc_from_url_invalid_api(url): """Tests for bad API URL error`GoogleCloudHealthcare.from_url()`.""" with pytest.raises(ValueError, match='v1 URL'): GoogleCloudHealthcare.from_url(url) @@ -479,13 +497,13 @@ def test_chc_dicom_store_from_url_invalid_api(url): f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomWeb', f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' ]) -def test_chc_dicom_store_from_url_invalid_store_name(url): +def test_chc_from_url_invalid_store_name(url): """Tests for bad Store name `GoogleCloudHealthcare.from_url()`.""" with pytest.raises(ValueError, match='v1 DICOM'): GoogleCloudHealthcare.from_url(url) -def test_chc_dicom_from_url_success(): +def test_chc_from_url_success(): """Locks down `GoogleCloudHealthcare.from_url()`.""" store = GoogleCloudHealthcare.from_url(_CHC_BASE_URL) assert store.project_id == _PROJECT_ID From e288e90848b62aaebdcadd7856c790add64b03d9 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 08:50:31 +0000 Subject: [PATCH 08/18] Cosmetic internal change in `GoogleCloudHealthcare`. --- src/dicomweb_client/uri.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 922fbd2..a6c5af3 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -32,6 +32,8 @@ class URISuffix(enum.Enum): # `GoogleCloudHealthcare`. _REGEX_ID_2 = r'[\w.-]+' _ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(_REGEX_ID_2) +# The URL for the Google Cloud Healthcare API endpoint. +_CHC_API_URL = 'https://healthcare.googleapis.com/v1' class URI: @@ -481,15 +483,12 @@ class GoogleCloudHealthcare: dataset_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) dicom_store_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) - # The URL for the CHC API endpoint. - _API_URL = 'https://healthcare.googleapis.com/v1' - def __str__(self) -> str: """Returns a string URL for use as :py:attr:`URI.base_url`. See class docstring for the returned URL format. """ - return (f'{self._API_URL}/' + return (f'{_CHC_API_URL}/' f'projects/{self.project_id}/' f'locations/{self.location}/' f'datasets/{self.dataset_id}/' @@ -513,9 +512,9 @@ def from_url(cls, base_url: str) -> 'GoogleCloudHealthcare': If ``base_url`` does not match the specifications in the class docstring. """ - if not base_url.startswith(f'{cls._API_URL}/'): + if not base_url.startswith(f'{_CHC_API_URL}/'): raise ValueError('Invalid CHC API v1 URL: {base_url!r}') - resource_suffix = base_url[len(cls._API_URL) + 1:] + resource_suffix = base_url[len(_CHC_API_URL) + 1:] store_regex = (r'projects/(%s)/locations/(%s)/datasets/(%s)/' r'dicomStores/(%s)/dicomWeb$') % ( From 29b7547f6ee3e6ade102c4d955e00d3d3df80d98 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 09:06:49 +0000 Subject: [PATCH 09/18] Rename `from_url()` to `from_string()` in `GoogleCloudHealthcare`. --- src/dicomweb_client/uri.py | 4 ++-- tests/test_uri.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index a6c5af3..2db6f2a 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -450,7 +450,7 @@ def from_string(cls, @attr.s(frozen=True) class GoogleCloudHealthcare: - """Base URL helper for DICOM Stores under the `Google Cloud Healthcare API`_. + """Base URL container for DICOM Stores under the `Google Cloud Healthcare API`_. This class facilitates the parsing and creation of :py:attr:`URI.base_url` corresponding to DICOMweb API Service URLs under the v1_ API. The URLs are @@ -495,7 +495,7 @@ def __str__(self) -> str: f'dicomStores/{self.dicom_store_id}/dicomWeb') @classmethod - def from_url(cls, base_url: str) -> 'GoogleCloudHealthcare': + def from_string(cls, base_url: str) -> 'GoogleCloudHealthcare': """Creates an instance from ``base_url``. Parameters diff --git a/tests/test_uri.py b/tests/test_uri.py index 75855aa..ddf2d22 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -479,10 +479,10 @@ def test_chc_invalid_dataset_or_store(name): @pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) -def test_chc_from_url_invalid_api(url): - """Tests for bad API URL error`GoogleCloudHealthcare.from_url()`.""" +def test_chc_from_string_invalid_api(url): + """Tests for bad API URL error`GoogleCloudHealthcare.from_string()`.""" with pytest.raises(ValueError, match='v1 URL'): - GoogleCloudHealthcare.from_url(url) + GoogleCloudHealthcare.from_string(url) @pytest.mark.parametrize('url', [ @@ -497,15 +497,15 @@ def test_chc_from_url_invalid_api(url): f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomWeb', f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' ]) -def test_chc_from_url_invalid_store_name(url): - """Tests for bad Store name `GoogleCloudHealthcare.from_url()`.""" +def test_chc_from_string_invalid_store_name(url): + """Tests for bad Store name `GoogleCloudHealthcare.from_string()`.""" with pytest.raises(ValueError, match='v1 DICOM'): - GoogleCloudHealthcare.from_url(url) + GoogleCloudHealthcare.from_string(url) -def test_chc_from_url_success(): - """Locks down `GoogleCloudHealthcare.from_url()`.""" - store = GoogleCloudHealthcare.from_url(_CHC_BASE_URL) +def test_chc_from_string_success(): + """Locks down `GoogleCloudHealthcare.from_string()`.""" + store = GoogleCloudHealthcare.from_string(_CHC_BASE_URL) assert store.project_id == _PROJECT_ID assert store.location == _LOCATION assert store.dataset_id == _DATASET_ID From 15e8fe4e08cad762a20842d1ccce21a26af7ecc2 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 8 Mar 2021 09:09:47 +0000 Subject: [PATCH 10/18] Rename `GoogleCloudHealthcare` to `GoogleCloudHealthcareURL`. --- src/dicomweb_client/uri.py | 8 ++++---- tests/test_uri.py | 32 +++++++++++++++++--------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 2db6f2a..4dca937 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -25,11 +25,11 @@ class URISuffix(enum.Enum): # For DICOM Standard spec validation of UID components in `URI`. _MAX_UID_LENGTH = 64 _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') -# Used for Project ID and Location validation in `GoogleCloudHealthcare`. +# Used for Project ID and Location validation in `GoogleCloudHealthcareURL`. _REGEX_ID_1 = r'[\w-]+' _ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(_REGEX_ID_1) # Used for Dataset ID and DICOM Store ID validation in -# `GoogleCloudHealthcare`. +# `GoogleCloudHealthcareURL`. _REGEX_ID_2 = r'[\w.-]+' _ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(_REGEX_ID_2) # The URL for the Google Cloud Healthcare API endpoint. @@ -449,7 +449,7 @@ def from_string(cls, @attr.s(frozen=True) -class GoogleCloudHealthcare: +class GoogleCloudHealthcareURL: """Base URL container for DICOM Stores under the `Google Cloud Healthcare API`_. This class facilitates the parsing and creation of :py:attr:`URI.base_url` @@ -495,7 +495,7 @@ def __str__(self) -> str: f'dicomStores/{self.dicom_store_id}/dicomWeb') @classmethod - def from_string(cls, base_url: str) -> 'GoogleCloudHealthcare': + def from_string(cls, base_url: str) -> 'GoogleCloudHealthcareURL': """Creates an instance from ``base_url``. Parameters diff --git a/tests/test_uri.py b/tests/test_uri.py index ddf2d22..5e44372 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -1,5 +1,5 @@ from dicomweb_client.uri import ( - GoogleCloudHealthcare, + GoogleCloudHealthcareURL, URI, URISuffix, URIType) @@ -451,9 +451,9 @@ def test_update_error(uri_args, update_args, error_msg): def test_chc_dicom_store_str(): - """Locks down `GoogleCloudHealthcare.__str__()`.""" + """Locks down `GoogleCloudHealthcareURL.__str__()`.""" assert str( - GoogleCloudHealthcare( + GoogleCloudHealthcareURL( _PROJECT_ID, _LOCATION, _DATASET_ID, @@ -462,27 +462,29 @@ def test_chc_dicom_store_str(): @pytest.mark.parametrize('name', ['hmmm.1', '#95', '43/']) def test_chc_invalid_project_or_location(name): - """Tests `GoogleCloudHealthcare` for bad `project_id`, `location`.""" + """Tests for bad `project_id`, `location`.""" with pytest.raises(ValueError): - GoogleCloudHealthcare(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) + GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) with pytest.raises(ValueError): - GoogleCloudHealthcare(_PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + GoogleCloudHealthcareURL( + _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) @pytest.mark.parametrize('name', ['hmmm.!', '#95', '43/']) def test_chc_invalid_dataset_or_store(name): - """Tests `GoogleCloudHealthcare` for bad `dataset_id`, `dicom_store_id`.""" + """Tests for bad `dataset_id`, `dicom_store_id`.""" with pytest.raises(ValueError): - GoogleCloudHealthcare(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) + GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) with pytest.raises(ValueError): - GoogleCloudHealthcare(_PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + GoogleCloudHealthcareURL( + _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) @pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) def test_chc_from_string_invalid_api(url): - """Tests for bad API URL error`GoogleCloudHealthcare.from_string()`.""" + """Tests for bad API URL error`GoogleCloudHealthcareURL.from_string()`.""" with pytest.raises(ValueError, match='v1 URL'): - GoogleCloudHealthcare.from_string(url) + GoogleCloudHealthcareURL.from_string(url) @pytest.mark.parametrize('url', [ @@ -498,14 +500,14 @@ def test_chc_from_string_invalid_api(url): f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' ]) def test_chc_from_string_invalid_store_name(url): - """Tests for bad Store name `GoogleCloudHealthcare.from_string()`.""" + """Tests for bad Store name `GoogleCloudHealthcareURL.from_string()`.""" with pytest.raises(ValueError, match='v1 DICOM'): - GoogleCloudHealthcare.from_string(url) + GoogleCloudHealthcareURL.from_string(url) def test_chc_from_string_success(): - """Locks down `GoogleCloudHealthcare.from_string()`.""" - store = GoogleCloudHealthcare.from_string(_CHC_BASE_URL) + """Locks down `GoogleCloudHealthcareURL.from_string()`.""" + store = GoogleCloudHealthcareURL.from_string(_CHC_BASE_URL) assert store.project_id == _PROJECT_ID assert store.location == _LOCATION assert store.dataset_id == _DATASET_ID From 32588e21c2b24f823253137481107b63d723bedd Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Sat, 17 Apr 2021 01:01:53 +0000 Subject: [PATCH 11/18] Change base class of `GoogleCloudHealthcareURL` from `attr` to `dataclass`. --- src/dicomweb_client/uri.py | 50 +++++++++++++++++++++++++++----------- tests/test_uri.py | 12 ++++----- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index aec7b01..838d545 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -1,5 +1,5 @@ """Utilities for DICOMweb URI manipulation.""" -import attr +import dataclasses import enum import re from typing import Optional, Sequence, Tuple @@ -27,14 +27,22 @@ class URISuffix(enum.Enum): _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') _REGEX_PERMISSIVE_UID = re.compile(r'[^/@]+') # Used for Project ID and Location validation in `GoogleCloudHealthcareURL`. -_REGEX_ID_1 = r'[\w-]+' -_ATTR_VALIDATOR_ID_1 = attr.validators.matches_re(_REGEX_ID_1) +_REGEX_ID_1 = re.compile(r'[\w-]+') # Used for Dataset ID and DICOM Store ID validation in # `GoogleCloudHealthcareURL`. -_REGEX_ID_2 = r'[\w.-]+' -_ATTR_VALIDATOR_ID_2 = attr.validators.matches_re(_REGEX_ID_2) +_REGEX_ID_2 = re.compile(r'[\w.-]+') +# Regex for the DICOM Store suffix for the Google Cloud Healthcare API endpoint. +_STORE_REGEX = re.compile( + (r'projects/(%s)/locations/(%s)/datasets/(%s)/' + r'dicomStores/(%s)/dicomWeb$') % (_REGEX_ID_1.pattern, + _REGEX_ID_1.pattern, + _REGEX_ID_2.pattern, + _REGEX_ID_2.pattern)) # The URL for the Google Cloud Healthcare API endpoint. _CHC_API_URL = 'https://healthcare.googleapis.com/v1' +# Cloud Healthcare validation error. +_CHC_API_ERROR_TMPL = ('`{attribute}` must match regex {regex}. Actual value: ' + '{value!r}') class URI: @@ -476,7 +484,7 @@ def from_string(cls, return uri -@attr.s(frozen=True) +@dataclasses.dataclass(eq=True, frozen=True) class GoogleCloudHealthcareURL: """Base URL container for DICOM Stores under the `Google Cloud Healthcare API`_. @@ -506,10 +514,27 @@ class GoogleCloudHealthcareURL: The ID of the `DICOM Store `_. """ - project_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_1) - location = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_1) - dataset_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) - dicom_store_id = attr.ib(type=str, validator=_ATTR_VALIDATOR_ID_2) + project_id: str + location: str + dataset_id: str + dicom_store_id: str + + def __post_init__(self) -> None: + """Performs input sanity checks.""" + if _REGEX_ID_1.fullmatch(self.project_id) is None: + raise ValueError(_CHC_API_ERROR_TMPL.format( + attribute='project_id', regex=_REGEX_ID_1, value=self.project_id)) + if _REGEX_ID_1.fullmatch(self.location) is None: + raise ValueError(_CHC_API_ERROR_TMPL.format( + attribute='location', regex=_REGEX_ID_1, value=self.location)) + if _REGEX_ID_2.fullmatch(self.dataset_id) is None: + raise ValueError(_CHC_API_ERROR_TMPL.format( + attribute='dataset_id', regex=_REGEX_ID_2, value=self.dataset_id)) + if _REGEX_ID_2.fullmatch(self.dicom_store_id) is None: + raise ValueError(_CHC_API_ERROR_TMPL.format( + attribute='dicom_store_id', + regex=_REGEX_ID_2, + value=self.dicom_store_id)) def __str__(self) -> str: """Returns a string URL for use as :py:attr:`URI.base_url`. @@ -544,10 +569,7 @@ def from_string(cls, base_url: str) -> 'GoogleCloudHealthcareURL': raise ValueError('Invalid CHC API v1 URL: {base_url!r}') resource_suffix = base_url[len(_CHC_API_URL) + 1:] - store_regex = (r'projects/(%s)/locations/(%s)/datasets/(%s)/' - r'dicomStores/(%s)/dicomWeb$') % ( - _REGEX_ID_1, _REGEX_ID_1, _REGEX_ID_2, _REGEX_ID_2) - store_match = re.match(store_regex, resource_suffix) + store_match = _STORE_REGEX.match(resource_suffix) if store_match is None: raise ValueError( 'Invalid CHC API v1 DICOM Store name: {resource_suffix!r}') diff --git a/tests/test_uri.py b/tests/test_uri.py index ca56b63..61112e5 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -499,9 +499,9 @@ def test_chc_dicom_store_str(): @pytest.mark.parametrize('name', ['hmmm.1', '#95', '43/']) def test_chc_invalid_project_or_location(name): """Tests for bad `project_id`, `location`.""" - with pytest.raises(ValueError): + with pytest.raises(ValueError, match='project_id'): GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match='location'): GoogleCloudHealthcareURL( _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) @@ -509,11 +509,11 @@ def test_chc_invalid_project_or_location(name): @pytest.mark.parametrize('name', ['hmmm.!', '#95', '43/']) def test_chc_invalid_dataset_or_store(name): """Tests for bad `dataset_id`, `dicom_store_id`.""" - with pytest.raises(ValueError): - GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match='dataset_id'): + GoogleCloudHealthcareURL(_PROJECT_ID, _LOCATION, name, _DICOM_STORE_ID) + with pytest.raises(ValueError, match='dicom_store_id'): GoogleCloudHealthcareURL( - _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + _PROJECT_ID, _LOCATION, _DATASET_ID, name) @pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) From 2683abbfe4308ce158715aa509fc2a3fff251468 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Sat, 17 Apr 2021 01:10:35 +0000 Subject: [PATCH 12/18] Fix `flake8` errors in `uri.py`. --- src/dicomweb_client/uri.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 838d545..828c31a 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -521,20 +521,14 @@ class GoogleCloudHealthcareURL: def __post_init__(self) -> None: """Performs input sanity checks.""" - if _REGEX_ID_1.fullmatch(self.project_id) is None: - raise ValueError(_CHC_API_ERROR_TMPL.format( - attribute='project_id', regex=_REGEX_ID_1, value=self.project_id)) - if _REGEX_ID_1.fullmatch(self.location) is None: - raise ValueError(_CHC_API_ERROR_TMPL.format( - attribute='location', regex=_REGEX_ID_1, value=self.location)) - if _REGEX_ID_2.fullmatch(self.dataset_id) is None: - raise ValueError(_CHC_API_ERROR_TMPL.format( - attribute='dataset_id', regex=_REGEX_ID_2, value=self.dataset_id)) - if _REGEX_ID_2.fullmatch(self.dicom_store_id) is None: - raise ValueError(_CHC_API_ERROR_TMPL.format( - attribute='dicom_store_id', - regex=_REGEX_ID_2, - value=self.dicom_store_id)) + for regex, attribute, value in ( + (_REGEX_ID_1, 'project_id', self.project_id), + (_REGEX_ID_1, 'location', self.location), + (_REGEX_ID_2, 'dataset_id', self.dataset_id), + (_REGEX_ID_2, 'dicom_store_id', self.dicom_store_id)): + if regex.fullmatch(value) is None: + raise ValueError(_CHC_API_ERROR_TMPL.format( + attribute=attribute, regex=regex, value=value)) def __str__(self) -> str: """Returns a string URL for use as :py:attr:`URI.base_url`. From 21ee6040b8725adcb8241e41d92fdc6d5d64fbfd Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Sat, 17 Apr 2021 01:36:47 +0000 Subject: [PATCH 13/18] Add `dataclasses` backport for Python 3.6. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index e055280..65cb191 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,7 @@ }, python_requires='>=3.6', install_requires=[ + 'dataclasses; python_version=="3.6"', 'pydicom>=2.0', 'requests>=2.18', 'retrying>=1.3.3', From f40bdf5b9434cdc72c7645d45fb6c6e0f557123b Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 24 May 2021 04:51:00 +0000 Subject: [PATCH 14/18] Move `uri.GoogleCloudHealthcareURL` to new module `ext.gcp.uri`. --- docs/installation.rst | 9 ++ docs/package.rst | 8 ++ setup.py | 2 +- src/dicomweb_client/ext/__init__.py | 1 + src/dicomweb_client/ext/gcp/__init__.py | 10 +++ src/dicomweb_client/ext/gcp/uri.py | 113 ++++++++++++++++++++++++ src/dicomweb_client/uri.py | 106 ---------------------- tests/test_ext_gcp_uri.py | 78 ++++++++++++++++ tests/test_uri.py | 70 +-------------- 9 files changed, 221 insertions(+), 176 deletions(-) create mode 100644 src/dicomweb_client/ext/__init__.py create mode 100644 src/dicomweb_client/ext/gcp/__init__.py create mode 100644 src/dicomweb_client/ext/gcp/uri.py create mode 100644 tests/test_ext_gcp_uri.py diff --git a/docs/installation.rst b/docs/installation.rst index 49fa157..2159057 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -28,6 +28,15 @@ Pre-build package available at PyPi: pip install dicomweb-client +Additional dependencies required for extensions compatible with +`Google Cloud Platform (GCP)`_ may be installed as: + +.. _Google Cloud Platform (GCP): https://cloud.google.com + +.. code-block:: none + + pip install dicomweb-client[gcp] + Source code available at Github: .. code-block:: none diff --git a/docs/package.rst b/docs/package.rst index ba63eef..adf3979 100644 --- a/docs/package.rst +++ b/docs/package.rst @@ -51,3 +51,11 @@ dicomweb\_client.uri module :members: :undoc-members: :show-inheritance: + +dicomweb\_client.ext.gcp.uri module ++++++++++++++++++++++++++++++++++++++ + +.. automodule:: dicomweb_client.ext.gcp.uri + :members: + :undoc-members: + :show-inheritance: diff --git a/setup.py b/setup.py index 65cb191..42fe3c9 100644 --- a/setup.py +++ b/setup.py @@ -42,13 +42,13 @@ package_dir={'': 'src'}, extras_require={ 'gcp': [ + 'dataclasses>=0.8; python_version=="3.6"', 'google-auth>=1.6', 'google-oauth>=1.0', ], }, python_requires='>=3.6', install_requires=[ - 'dataclasses; python_version=="3.6"', 'pydicom>=2.0', 'requests>=2.18', 'retrying>=1.3.3', diff --git a/src/dicomweb_client/ext/__init__.py b/src/dicomweb_client/ext/__init__.py new file mode 100644 index 0000000..2e98b57 --- /dev/null +++ b/src/dicomweb_client/ext/__init__.py @@ -0,0 +1 @@ +"""Vendor-specific extensions of the `dicomweb_client` package.""" diff --git a/src/dicomweb_client/ext/gcp/__init__.py b/src/dicomweb_client/ext/gcp/__init__.py new file mode 100644 index 0000000..dd7869e --- /dev/null +++ b/src/dicomweb_client/ext/gcp/__init__.py @@ -0,0 +1,10 @@ +"""Google Cloud Platform (GCP) compatible extensions of `dicomweb_client`. + +Modules under this package may require additional dependencies. Instructions for +installation are available in the Installation Guide here: +https://dicomweb-client.readthedocs.io/en/latest/installation.html#installation-guide + +For further details about GCP, visit: https://cloud.google.com +""" + +from dicomweb_client.ext.gcp.uri import GoogleCloudHealthcareURL diff --git a/src/dicomweb_client/ext/gcp/uri.py b/src/dicomweb_client/ext/gcp/uri.py new file mode 100644 index 0000000..55c9fdd --- /dev/null +++ b/src/dicomweb_client/ext/gcp/uri.py @@ -0,0 +1,113 @@ +"""Utilities for Google Cloud Healthcare DICOMweb API URI manipulation. + +For details, visit: https://cloud.google.com/healthcare +""" +import dataclasses +import re + + +# Used for Project ID and Location validation in `GoogleCloudHealthcareURL`. +_REGEX_ID_1 = re.compile(r'[\w-]+') +# Used for Dataset ID and DICOM Store ID validation in +# `GoogleCloudHealthcareURL`. +_REGEX_ID_2 = re.compile(r'[\w.-]+') +# Regex for the DICOM Store suffix for the Google Cloud Healthcare API endpoint. +_STORE_REGEX = re.compile( + (r'projects/(%s)/locations/(%s)/datasets/(%s)/' + r'dicomStores/(%s)/dicomWeb$') % (_REGEX_ID_1.pattern, + _REGEX_ID_1.pattern, + _REGEX_ID_2.pattern, + _REGEX_ID_2.pattern)) +# The URL for the Google Cloud Healthcare API endpoint. +_CHC_API_URL = 'https://healthcare.googleapis.com/v1' +# GCP resource name validation error. +_GCP_RESOURCE_ERROR_TMPL = ('`{attribute}` must match regex {regex}. Actual ' + 'value: {value!r}') + + +@dataclasses.dataclass(eq=True, frozen=True) +class GoogleCloudHealthcareURL: + """Base URL container for DICOM Stores under the `Google Cloud Healthcare API`_. + + This class facilitates the parsing and creation of :py:attr:`URI.base_url` + corresponding to DICOMweb API Service URLs under the v1_ API. The URLs are + of the form: + ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` + + .. _Google Cloud Healthcare API: https://cloud.google.com/healthcare + .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide + + Attributes: + project_id: str + The ID of the `GCP Project + `_ + that contains the DICOM Store. + location: str + The `Region name + `_ of the + geographic location configured for the Dataset that contains the + DICOM Store. + dataset_id: str + The ID of the `Dataset + `_ + that contains the DICOM Store. + dicom_store_id: str + The ID of the `DICOM Store + `_. + """ + project_id: str + location: str + dataset_id: str + dicom_store_id: str + + def __post_init__(self) -> None: + """Performs input sanity checks.""" + for regex, attribute, value in ( + (_REGEX_ID_1, 'project_id', self.project_id), + (_REGEX_ID_1, 'location', self.location), + (_REGEX_ID_2, 'dataset_id', self.dataset_id), + (_REGEX_ID_2, 'dicom_store_id', self.dicom_store_id)): + if regex.fullmatch(value) is None: + raise ValueError(_GCP_RESOURCE_ERROR_TMPL.format( + attribute=attribute, regex=regex, value=value)) + + def __str__(self) -> str: + """Returns a string URL for use as :py:attr:`URI.base_url`. + + See class docstring for the returned URL format. + """ + return (f'{_CHC_API_URL}/' + f'projects/{self.project_id}/' + f'locations/{self.location}/' + f'datasets/{self.dataset_id}/' + f'dicomStores/{self.dicom_store_id}/dicomWeb') + + @classmethod + def from_string(cls, base_url: str) -> 'GoogleCloudHealthcareURL': + """Creates an instance from ``base_url``. + + Parameters + ---------- + base_url: str + The URL for the DICOMweb API Service endpoint corresponding to a + `CHC API DICOM Store + `_. + See class docstring for supported formats. + + Raises + ------ + ValueError + If ``base_url`` does not match the specifications in the class + docstring. + """ + if not base_url.startswith(f'{_CHC_API_URL}/'): + raise ValueError('Invalid CHC API v1 URL: {base_url!r}') + resource_suffix = base_url[len(_CHC_API_URL) + 1:] + + store_match = _STORE_REGEX.match(resource_suffix) + if store_match is None: + raise ValueError( + 'Invalid CHC API v1 DICOM Store name: {resource_suffix!r}') + + return cls(store_match.group(1), store_match.group(2), + store_match.group(3), store_match.group(4)) diff --git a/src/dicomweb_client/uri.py b/src/dicomweb_client/uri.py index 828c31a..889ecd7 100644 --- a/src/dicomweb_client/uri.py +++ b/src/dicomweb_client/uri.py @@ -1,5 +1,4 @@ """Utilities for DICOMweb URI manipulation.""" -import dataclasses import enum import re from typing import Optional, Sequence, Tuple @@ -26,23 +25,6 @@ class URISuffix(enum.Enum): _MAX_UID_LENGTH = 64 _REGEX_UID = re.compile(r'[0-9]+([.][0-9]+)*') _REGEX_PERMISSIVE_UID = re.compile(r'[^/@]+') -# Used for Project ID and Location validation in `GoogleCloudHealthcareURL`. -_REGEX_ID_1 = re.compile(r'[\w-]+') -# Used for Dataset ID and DICOM Store ID validation in -# `GoogleCloudHealthcareURL`. -_REGEX_ID_2 = re.compile(r'[\w.-]+') -# Regex for the DICOM Store suffix for the Google Cloud Healthcare API endpoint. -_STORE_REGEX = re.compile( - (r'projects/(%s)/locations/(%s)/datasets/(%s)/' - r'dicomStores/(%s)/dicomWeb$') % (_REGEX_ID_1.pattern, - _REGEX_ID_1.pattern, - _REGEX_ID_2.pattern, - _REGEX_ID_2.pattern)) -# The URL for the Google Cloud Healthcare API endpoint. -_CHC_API_URL = 'https://healthcare.googleapis.com/v1' -# Cloud Healthcare validation error. -_CHC_API_ERROR_TMPL = ('`{attribute}` must match regex {regex}. Actual value: ' - '{value!r}') class URI: @@ -484,94 +466,6 @@ def from_string(cls, return uri -@dataclasses.dataclass(eq=True, frozen=True) -class GoogleCloudHealthcareURL: - """Base URL container for DICOM Stores under the `Google Cloud Healthcare API`_. - - This class facilitates the parsing and creation of :py:attr:`URI.base_url` - corresponding to DICOMweb API Service URLs under the v1_ API. The URLs are - of the form: - ``https://healthcare.googleapis.com/v1/projects/{project_id}/locations/{location}/datasets/{dataset_id}/dicomStores/{dicom_store_id}/dicomWeb`` - - .. _Google Cloud Healthcare API: https://cloud.google.com/healthcare - .. _v1: https://cloud.google.com/healthcare/docs/how-tos/transition-guide - - Attributes: - project_id: str - The ID of the `GCP Project - `_ - that contains the DICOM Store. - location: str - The `Region name - `_ of the - geographic location configured for the Dataset that contains the - DICOM Store. - dataset_id: str - The ID of the `Dataset - `_ - that contains the DICOM Store. - dicom_store_id: str - The ID of the `DICOM Store - `_. - """ - project_id: str - location: str - dataset_id: str - dicom_store_id: str - - def __post_init__(self) -> None: - """Performs input sanity checks.""" - for regex, attribute, value in ( - (_REGEX_ID_1, 'project_id', self.project_id), - (_REGEX_ID_1, 'location', self.location), - (_REGEX_ID_2, 'dataset_id', self.dataset_id), - (_REGEX_ID_2, 'dicom_store_id', self.dicom_store_id)): - if regex.fullmatch(value) is None: - raise ValueError(_CHC_API_ERROR_TMPL.format( - attribute=attribute, regex=regex, value=value)) - - def __str__(self) -> str: - """Returns a string URL for use as :py:attr:`URI.base_url`. - - See class docstring for the returned URL format. - """ - return (f'{_CHC_API_URL}/' - f'projects/{self.project_id}/' - f'locations/{self.location}/' - f'datasets/{self.dataset_id}/' - f'dicomStores/{self.dicom_store_id}/dicomWeb') - - @classmethod - def from_string(cls, base_url: str) -> 'GoogleCloudHealthcareURL': - """Creates an instance from ``base_url``. - - Parameters - ---------- - base_url: str - The URL for the DICOMweb API Service endpoint corresponding to a - `CHC API DICOM Store - `_. - See class docstring for supported formats. - - Raises - ------ - ValueError - If ``base_url`` does not match the specifications in the class - docstring. - """ - if not base_url.startswith(f'{_CHC_API_URL}/'): - raise ValueError('Invalid CHC API v1 URL: {base_url!r}') - resource_suffix = base_url[len(_CHC_API_URL) + 1:] - - store_match = _STORE_REGEX.match(resource_suffix) - if store_match is None: - raise ValueError( - 'Invalid CHC API v1 DICOM Store name: {resource_suffix!r}') - - return cls(store_match.group(1), store_match.group(2), - store_match.group(3), store_match.group(4)) - - def _validate_base_url(url: str) -> None: """Validates the Base (DICOMweb service) URL supplied to `URI`.""" parse_result = urlparse.urlparse(url) diff --git a/tests/test_ext_gcp_uri.py b/tests/test_ext_gcp_uri.py new file mode 100644 index 0000000..2d74838 --- /dev/null +++ b/tests/test_ext_gcp_uri.py @@ -0,0 +1,78 @@ +"""Unit tests for `dicomweb_client.ext.gcp.uri` module.""" +from dicomweb_client.ext.gcp.uri import GoogleCloudHealthcareURL + +import pytest + +_PROJECT_ID = 'my-project44' +_LOCATION = 'us-central1' +_DATASET_ID = 'my-44.dataset' +_DICOM_STORE_ID = 'my.d1com_store' +_CHC_API_URL = 'https://healthcare.googleapis.com/v1' +_CHC_BASE_URL = ( + f'{_CHC_API_URL}/' + f'projects/{_PROJECT_ID}/locations/{_LOCATION}/' + f'datasets/{_DATASET_ID}/dicomStores/{_DICOM_STORE_ID}/dicomWeb') + + +def test_chc_dicom_store_str(): + """Locks down `GoogleCloudHealthcareURL.__str__()`.""" + assert str( + GoogleCloudHealthcareURL( + _PROJECT_ID, + _LOCATION, + _DATASET_ID, + _DICOM_STORE_ID)) == _CHC_BASE_URL + + +@pytest.mark.parametrize('name', ['hmmm.1', '#95', '43/']) +def test_chc_invalid_project_or_location(name): + """Tests for bad `project_id`, `location`.""" + with pytest.raises(ValueError, match='project_id'): + GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) + with pytest.raises(ValueError, match='location'): + GoogleCloudHealthcareURL( + _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) + + +@pytest.mark.parametrize('name', ['hmmm.!', '#95', '43/']) +def test_chc_invalid_dataset_or_store(name): + """Tests for bad `dataset_id`, `dicom_store_id`.""" + with pytest.raises(ValueError, match='dataset_id'): + GoogleCloudHealthcareURL(_PROJECT_ID, _LOCATION, name, _DICOM_STORE_ID) + with pytest.raises(ValueError, match='dicom_store_id'): + GoogleCloudHealthcareURL( + _PROJECT_ID, _LOCATION, _DATASET_ID, name) + + +@pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) +def test_chc_from_string_invalid_api(url): + """Tests for bad API URL error`GoogleCloudHealthcareURL.from_string()`.""" + with pytest.raises(ValueError, match='v1 URL'): + GoogleCloudHealthcareURL.from_string(url) + + +@pytest.mark.parametrize('url', [ + f'{_CHC_BASE_URL}/', # Trailing slash disallowed. + f'{_CHC_API_URL}/project/p/locations/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/location/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/dataset/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomStore/ds/dicomWeb', + f'{_CHC_API_URL}/locations/l/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/datasets/d/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/dicomStores/ds/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomWeb', + f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' +]) +def test_chc_from_string_invalid_store_name(url): + """Tests for bad Store name `GoogleCloudHealthcareURL.from_string()`.""" + with pytest.raises(ValueError, match='v1 DICOM'): + GoogleCloudHealthcareURL.from_string(url) + + +def test_chc_from_string_success(): + """Locks down `GoogleCloudHealthcareURL.from_string()`.""" + store = GoogleCloudHealthcareURL.from_string(_CHC_BASE_URL) + assert store.project_id == _PROJECT_ID + assert store.location == _LOCATION + assert store.dataset_id == _DATASET_ID + assert store.dicom_store_id == _DICOM_STORE_ID diff --git a/tests/test_uri.py b/tests/test_uri.py index 61112e5..e86330e 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -1,8 +1,4 @@ -from dicomweb_client.uri import ( - GoogleCloudHealthcareURL, - URI, - URISuffix, - URIType) +from dicomweb_client.uri import URI, URISuffix, URIType import pytest @@ -484,67 +480,3 @@ def test_update_error(uri_args, update_args, error_msg): """Tests for failure if the `URI` returned by `update()` is invalid.""" with pytest.raises(ValueError, match=error_msg): URI(*uri_args).update(*update_args) - - -def test_chc_dicom_store_str(): - """Locks down `GoogleCloudHealthcareURL.__str__()`.""" - assert str( - GoogleCloudHealthcareURL( - _PROJECT_ID, - _LOCATION, - _DATASET_ID, - _DICOM_STORE_ID)) == _CHC_BASE_URL - - -@pytest.mark.parametrize('name', ['hmmm.1', '#95', '43/']) -def test_chc_invalid_project_or_location(name): - """Tests for bad `project_id`, `location`.""" - with pytest.raises(ValueError, match='project_id'): - GoogleCloudHealthcareURL(name, _LOCATION, _DATASET_ID, _DICOM_STORE_ID) - with pytest.raises(ValueError, match='location'): - GoogleCloudHealthcareURL( - _PROJECT_ID, name, _DATASET_ID, _DICOM_STORE_ID) - - -@pytest.mark.parametrize('name', ['hmmm.!', '#95', '43/']) -def test_chc_invalid_dataset_or_store(name): - """Tests for bad `dataset_id`, `dicom_store_id`.""" - with pytest.raises(ValueError, match='dataset_id'): - GoogleCloudHealthcareURL(_PROJECT_ID, _LOCATION, name, _DICOM_STORE_ID) - with pytest.raises(ValueError, match='dicom_store_id'): - GoogleCloudHealthcareURL( - _PROJECT_ID, _LOCATION, _DATASET_ID, name) - - -@pytest.mark.parametrize('url', [f'{_CHC_API_URL}beta', 'https://some.url']) -def test_chc_from_string_invalid_api(url): - """Tests for bad API URL error`GoogleCloudHealthcareURL.from_string()`.""" - with pytest.raises(ValueError, match='v1 URL'): - GoogleCloudHealthcareURL.from_string(url) - - -@pytest.mark.parametrize('url', [ - f'{_CHC_BASE_URL}/', # Trailing slash disallowed. - f'{_CHC_API_URL}/project/p/locations/l/datasets/d/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/location/l/datasets/d/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/locations/l/dataset/d/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomStore/ds/dicomWeb', - f'{_CHC_API_URL}/locations/l/datasets/d/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/datasets/d/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/locations/l/dicomStores/ds/dicomWeb', - f'{_CHC_API_URL}/projects/p/locations/l/datasets/d/dicomWeb', - f'{_CHC_API_URL}/projects/p/locations/l//datasets/d/dicomStores/ds/dicomWeb' -]) -def test_chc_from_string_invalid_store_name(url): - """Tests for bad Store name `GoogleCloudHealthcareURL.from_string()`.""" - with pytest.raises(ValueError, match='v1 DICOM'): - GoogleCloudHealthcareURL.from_string(url) - - -def test_chc_from_string_success(): - """Locks down `GoogleCloudHealthcareURL.from_string()`.""" - store = GoogleCloudHealthcareURL.from_string(_CHC_BASE_URL) - assert store.project_id == _PROJECT_ID - assert store.location == _LOCATION - assert store.dataset_id == _DATASET_ID - assert store.dicom_store_id == _DICOM_STORE_ID From 94eb5497a86f08c0b5bd5124cab9652f40e0ff11 Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 24 May 2021 04:54:05 +0000 Subject: [PATCH 15/18] Remove dead code from `tests/test_uri.py`. --- tests/test_uri.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/test_uri.py b/tests/test_uri.py index e86330e..c568c6d 100644 --- a/tests/test_uri.py +++ b/tests/test_uri.py @@ -14,17 +14,6 @@ _INSTANCE_URI = f'{_SERIES_URI}/instances/{_INSTANCE_UID}' _FRAME_URI = f'{_INSTANCE_URI}/frames/{",".join(str(f) for f in _FRAMES)}' -# CHC DICOMweb URI parameters. -_PROJECT_ID = 'my-project44' -_LOCATION = 'us-central1' -_DATASET_ID = 'my-44.dataset' -_DICOM_STORE_ID = 'my.d1com_store' -_CHC_API_URL = 'https://healthcare.googleapis.com/v1' -_CHC_BASE_URL = ( - f'{_CHC_API_URL}/' - f'projects/{_PROJECT_ID}/locations/{_LOCATION}/' - f'datasets/{_DATASET_ID}/dicomStores/{_DICOM_STORE_ID}/dicomWeb') - @pytest.mark.parametrize('illegal_char', ['/', '@', 'a', 'A']) def test_uid_illegal_character(illegal_char): From 0a3e9c6d139580030a7e3e4dac1801ebd28c308f Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 24 May 2021 04:58:39 +0000 Subject: [PATCH 16/18] Disable lint check for unused import. --- src/dicomweb_client/ext/gcp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dicomweb_client/ext/gcp/__init__.py b/src/dicomweb_client/ext/gcp/__init__.py index dd7869e..294364f 100644 --- a/src/dicomweb_client/ext/gcp/__init__.py +++ b/src/dicomweb_client/ext/gcp/__init__.py @@ -7,4 +7,4 @@ For further details about GCP, visit: https://cloud.google.com """ -from dicomweb_client.ext.gcp.uri import GoogleCloudHealthcareURL +from dicomweb_client.ext.gcp.uri import GoogleCloudHealthcareURL # noqa From 7fb78578f615867757232812b5f2d27654f34fef Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Mon, 24 May 2021 07:19:45 +0000 Subject: [PATCH 17/18] Copy `session_utils.create_session_from_gcp_credentials()` to `ext.gcp.session_utils` and add deprecation warning to the original. --- docs/package.rst | 12 ++++++-- src/dicomweb_client/ext/gcp/session_utils.py | 31 ++++++++++++++++++++ src/dicomweb_client/session_utils.py | 18 ++++++------ 3 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 src/dicomweb_client/ext/gcp/session_utils.py diff --git a/docs/package.rst b/docs/package.rst index adf3979..e34644a 100644 --- a/docs/package.rst +++ b/docs/package.rst @@ -45,15 +45,23 @@ dicomweb\_client.session_utils module :show-inheritance: dicomweb\_client.uri module -+++++++++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++ .. automodule:: dicomweb_client.uri :members: :undoc-members: :show-inheritance: +dicomweb\_client.ext.gcp.session_utils module ++++++++++++++++++++++++++++++++++++++++++++++ + +.. automodule:: dicomweb_client.ext.gcp.session_utils + :members: + :undoc-members: + :show-inheritance: + dicomweb\_client.ext.gcp.uri module -+++++++++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++++++++++ .. automodule:: dicomweb_client.ext.gcp.uri :members: diff --git a/src/dicomweb_client/ext/gcp/session_utils.py b/src/dicomweb_client/ext/gcp/session_utils.py new file mode 100644 index 0000000..322b8ec --- /dev/null +++ b/src/dicomweb_client/ext/gcp/session_utils.py @@ -0,0 +1,31 @@ +"""Session management utilities for Google Cloud Platform (GCP).""" +from typing import Optional, Any + +import google.auth +from google.auth.transport import requests as google_requests +import requests + + +def create_session_from_gcp_credentials( + google_credentials: Optional[Any] = None + ) -> requests.Session: + """Creates an authorized session for Google Cloud Platform. + + Parameters + ---------- + google_credentials: Any + Google Cloud credentials. + (see https://cloud.google.com/docs/authentication/production + for more information on Google Cloud authentication). + If not set, will be initialized to ``google.auth.default()``. + + Returns + ------- + requests.Session + Google Cloud authorized session. + """ + if google_credentials is None: + google_credentials, _ = google.auth.default( + scopes=['https://www.googleapis.com/auth/cloud-platform'] + ) + return google_requests.AuthorizedSession(google_credentials) diff --git a/src/dicomweb_client/session_utils.py b/src/dicomweb_client/session_utils.py index 6dfd583..fa468f2 100644 --- a/src/dicomweb_client/session_utils.py +++ b/src/dicomweb_client/session_utils.py @@ -1,6 +1,7 @@ import logging import os from typing import Optional, Any +import warnings import requests @@ -128,20 +129,19 @@ def create_session_from_gcp_credentials( ------- requests.Session Google cloud authorized session - """ + warnings.warn( + 'This method shall be deprecated in a future release. Prefer using the ' + 'underlying implementation directly, now moved to ' + '`dicomweb_client.ext.gcp.session_utils`.', + DeprecationWarning) try: - from google.auth.transport import requests as google_requests - if google_credentials is None: - import google.auth - google_credentials, _ = google.auth.default( - scopes=['https://www.googleapis.com/auth/cloud-platform'] - ) + import dicomweb_client.ext.gcp.session_utils as gcp_session_utils except ImportError: raise ImportError( 'The dicomweb-client package needs to be installed with the ' '"gcp" extra requirements to support interaction with the ' 'Google Cloud Healthcare API: pip install dicomweb-client[gcp]' ) - logger.debug('initialize, authenticate and authorize HTTP session') - return google_requests.AuthorizedSession(google_credentials) + return gcp_session_utils.create_session_from_gcp_credentials( + google_credentials) From e92771ac5f2b9a156f02fe2e2e568158c69faebb Mon Sep 17 00:00:00 2001 From: Arnav Agharwal Date: Tue, 25 May 2021 01:39:35 +0000 Subject: [PATCH 18/18] Move dependency check in `session_utils.create_session_from_gcp_credentials()` to `ext.gcp.session_utils`. --- src/dicomweb_client/ext/gcp/session_utils.py | 10 ++++++++-- src/dicomweb_client/session_utils.py | 9 +-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/dicomweb_client/ext/gcp/session_utils.py b/src/dicomweb_client/ext/gcp/session_utils.py index 322b8ec..a5271e0 100644 --- a/src/dicomweb_client/ext/gcp/session_utils.py +++ b/src/dicomweb_client/ext/gcp/session_utils.py @@ -1,8 +1,14 @@ """Session management utilities for Google Cloud Platform (GCP).""" from typing import Optional, Any -import google.auth -from google.auth.transport import requests as google_requests +try: + import google.auth + from google.auth.transport import requests as google_requests +except ImportError: + raise ImportError( + 'The `dicomweb-client` package needs to be installed with the ' + '"gcp" extra requirements to use this module, as follows: ' + '`pip install dicomweb-client[gcp]`') import requests diff --git a/src/dicomweb_client/session_utils.py b/src/dicomweb_client/session_utils.py index fa468f2..a6f004a 100644 --- a/src/dicomweb_client/session_utils.py +++ b/src/dicomweb_client/session_utils.py @@ -135,13 +135,6 @@ def create_session_from_gcp_credentials( 'underlying implementation directly, now moved to ' '`dicomweb_client.ext.gcp.session_utils`.', DeprecationWarning) - try: - import dicomweb_client.ext.gcp.session_utils as gcp_session_utils - except ImportError: - raise ImportError( - 'The dicomweb-client package needs to be installed with the ' - '"gcp" extra requirements to support interaction with the ' - 'Google Cloud Healthcare API: pip install dicomweb-client[gcp]' - ) + import dicomweb_client.ext.gcp.session_utils as gcp_session_utils return gcp_session_utils.create_session_from_gcp_credentials( google_credentials)