From eddac2a3c8cfb9819dff0bc87aa6defe0f97c4ec Mon Sep 17 00:00:00 2001 From: willdunklin Date: Wed, 10 Apr 2024 16:21:39 -0400 Subject: [PATCH 1/7] Switch DICOMweb assetstore imports to use girder generics --- .../assetstore/dicomweb_assetstore_adapter.py | 84 ++++++++++++++----- .../assetstore/rest.py | 34 +------- .../web_client/main.js | 3 +- .../web_client/models/AssetstoreModel.js | 20 ----- .../web_client/routes.js | 17 ---- .../dicomwebAssetstoreImportButton.pug | 2 +- .../web_client/views/DICOMwebImportView.js | 14 +++- 7 files changed, 77 insertions(+), 97 deletions(-) delete mode 100644 sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js delete mode 100644 sources/dicom/large_image_source_dicom/web_client/routes.js diff --git a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py index 6a3e25261..da2fba6c6 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py +++ b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py @@ -1,3 +1,5 @@ +import json + import cherrypy import requests from large_image_source_dicom.dicom_tags import dicom_key_to_tag @@ -394,31 +396,10 @@ def _infer_file_size_singlepart_content_length(self, file): except ValueError: return - def importData(self, parent, parentType, params, progress, user, **kwargs): - """ - Import DICOMweb WSI instances from a DICOMweb server. - - :param parent: The parent object to import into. - :param parentType: The model type of the parent object. - :type parentType: str - :param params: Additional parameters required for the import process. - This dictionary may include the following keys: - - :limit: (optional) limit the number of studies imported. - :search_filters: (optional) a dictionary of additional search - filters to use with dicomweb_client's `search_for_series()` - function. - - :type params: dict - :param progress: Object on which to record progress if possible. - :type progress: :py:class:`girder.utility.progress.ProgressContext` - :param user: The Girder user performing the import. - :type user: dict or None - :return: a list of items that were created - """ + def _importData(self, parent, parentType, params, progress, user): if parentType not in ('folder', 'user', 'collection'): msg = f'Invalid parent type: {parentType}' - raise RuntimeError(msg) + raise ValidationException(msg) limit = params.get('limit') search_filters = params.get('search_filters', {}) @@ -512,6 +493,63 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): return items + def importData(self, parent, parentType, params, progress, user, **kwargs): + """ + Import DICOMweb WSI instances from a DICOMweb server. + + :param parent: The parent object to import into. + :param parentType: The model type of the parent object. + :type parentType: str + :param params: Additional parameters required for the import process. + This dictionary may include the following keys: + + :limit: (optional) limit the number of studies imported. + :search_filters: (optional) a dictionary of additional search + filters to use with dicomweb_client's `search_for_series()` + function. + + :type params: dict + :param progress: Object on which to record progress if possible. + :type progress: :py:class:`girder.utility.progress.ProgressContext` + :param user: The Girder user performing the import. + :type user: dict or None + :return: a list of items that were created + """ + # Validate the parameters + limit = params.get('limit') or None + if limit is not None: + error_msg = f'Invalid limit: {limit}' + try: + limit = int(limit) + except ValueError: + raise ValidationException(error_msg) + + if limit < 1: + raise ValidationException(error_msg) + + try: + search_filters = json.loads(params.get('filters') or '{}') + except json.JSONDecodeError as e: + msg = f'Invalid filters: "{params.get("filters")}". {e}' + raise ValidationException(msg) + + items = self._importData( + parent, + parentType, + { + 'limit': limit, + 'search_filters': search_filters, + }, + progress, + user, + ) + + if not items or len(items) == 0: + msg = 'No studies matching the search filters were found' + raise ValidationException(msg) + + return items + @property def auth_session(self): return _create_auth_session(self.assetstore_meta) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index a53f49987..e0e329902 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -1,15 +1,14 @@ -import json - from girder.api import access from girder.api.describe import Description, autoDescribeRoute from girder.api.rest import Resource from girder.constants import TokenScope from girder.exceptions import RestException from girder.models.assetstore import Assetstore -from girder.utility import assetstore_utilities from girder.utility.model_importer import ModelImporter from girder.utility.progress import ProgressContext +from .dicomweb_assetstore_adapter import DICOMwebAssetstoreAdapter + class DICOMwebAssetstoreResource(Resource): def __init__(self): @@ -34,39 +33,14 @@ def _importData(self, assetstore, params, progress): parent = ModelImporter.model(destinationType).load(params['destinationId'], force=True, exc=True) - limit = params.get('limit') or None - if limit is not None: - error_msg = 'Invalid limit' - try: - limit = int(limit) - except ValueError: - raise RestException(error_msg) - - if limit < 1: - raise RestException(error_msg) - - try: - search_filters = json.loads(params.get('filters') or '{}') - except json.JSONDecodeError as e: - msg = f'Invalid filters: {e}' - raise RestException(msg) - - adapter = assetstore_utilities.getAssetstoreAdapter(assetstore) - items = adapter.importData( + return DICOMwebAssetstoreAdapter(assetstore).importData( parent, destinationType, - { - 'limit': limit, - 'search_filters': search_filters, - }, + params, progress, user, ) - if not items: - msg = 'No studies matching the search filters were found' - raise RestException(msg) - @access.admin(scope=TokenScope.DATA_WRITE) @autoDescribeRoute( Description('Import references to DICOM objects from a DICOMweb server') diff --git a/sources/dicom/large_image_source_dicom/web_client/main.js b/sources/dicom/large_image_source_dicom/web_client/main.js index d697a6181..3b64ee4aa 100644 --- a/sources/dicom/large_image_source_dicom/web_client/main.js +++ b/sources/dicom/large_image_source_dicom/web_client/main.js @@ -1,7 +1,6 @@ -import './routes'; - // Extends and overrides API import './constants'; +import './views/DICOMwebImportView'; import './views/AssetstoresView'; import './views/EditAssetstoreWidget'; import './views/NewAssetstoreWidget'; diff --git a/sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js b/sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js deleted file mode 100644 index c8313a140..000000000 --- a/sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js +++ /dev/null @@ -1,20 +0,0 @@ -import AssetstoreModel from '@girder/core/models/AssetstoreModel'; -import { restRequest } from '@girder/core/rest'; - -/** - * Extends the core assetstore model to add DICOMweb-specific functionality. - */ -AssetstoreModel.prototype.dicomwebImport = function (params) { - return restRequest({ - url: 'dicomweb_assetstore/' + this.get('_id') + '/import', - type: 'POST', - data: params, - error: null - }).done(() => { - this.trigger('g:imported'); - }).fail((err) => { - this.trigger('g:error', err); - }); -}; - -export default AssetstoreModel; diff --git a/sources/dicom/large_image_source_dicom/web_client/routes.js b/sources/dicom/large_image_source_dicom/web_client/routes.js deleted file mode 100644 index 22e84ad77..000000000 --- a/sources/dicom/large_image_source_dicom/web_client/routes.js +++ /dev/null @@ -1,17 +0,0 @@ -import router from '@girder/core/router'; -import events from '@girder/core/events'; - -import AssetstoreModel from './models/AssetstoreModel'; -import DICOMwebImportView from './views/DICOMwebImportView'; - -router.route('dicomweb_assetstore/:id/import', 'dwasImport', function (id) { - // Fetch the assetstore by id, then render the view. - const assetstore = new AssetstoreModel({ _id: id }); - assetstore.once('g:fetched', function () { - events.trigger('g:navigateTo', DICOMwebImportView, { - model: assetstore - }); - }).once('g:error', function () { - router.navigate('assetstores', { trigger: true }); - }).fetch(); -}); diff --git a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreImportButton.pug b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreImportButton.pug index ca55a7747..a5a909c33 100644 --- a/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreImportButton.pug +++ b/sources/dicom/large_image_source_dicom/web_client/templates/dicomwebAssetstoreImportButton.pug @@ -1,6 +1,6 @@ .g-assetstore-import-button-container a.g-dwas-import-button.btn.btn-sm.btn-success( - href=`#dicomweb_assetstore/${assetstore.get('_id')}/import`, + href=`#assetstore/${assetstore.get('_id')}/import`, title="Import references to DICOM objects from a DICOMweb server") i.icon-link-ext | Import data diff --git a/sources/dicom/large_image_source_dicom/web_client/views/DICOMwebImportView.js b/sources/dicom/large_image_source_dicom/web_client/views/DICOMwebImportView.js index eceef92d8..b77fb3aad 100644 --- a/sources/dicom/large_image_source_dicom/web_client/views/DICOMwebImportView.js +++ b/sources/dicom/large_image_source_dicom/web_client/views/DICOMwebImportView.js @@ -5,6 +5,9 @@ import router from '@girder/core/router'; import View from '@girder/core/views/View'; import { restRequest } from '@girder/core/rest'; +import { assetstoreImportViewMap } from '@girder/core/views/body/AssetstoresView'; +import { AssetstoreType } from '@girder/core/constants'; + import DWASImportTemplate from '../templates/assetstoreImport.pug'; const DICOMwebImportView = View.extend({ @@ -24,12 +27,12 @@ const DICOMwebImportView = View.extend({ } this.$('.g-submit-dwas-import').addClass('disabled'); - this.model.off().on('g:imported', function () { + this.assetstore.off().on('g:imported', function () { router.navigate(destinationType + '/' + destinationId, { trigger: true }); }, this).on('g:error', function (err) { this.$('.g-submit-dwas-import').removeClass('disabled'); this.$('.g-validation-failed-message').html(err.responseJSON.message); - }, this).dicomwebImport({ + }, this).import({ destinationId, destinationType, limit, @@ -40,7 +43,7 @@ const DICOMwebImportView = View.extend({ 'click .g-open-browser': '_openBrowser' }, - initialize: function () { + initialize: function (settings) { this._browserWidgetView = new BrowserWidget({ parentView: this, titleText: 'Destination', @@ -75,12 +78,13 @@ const DICOMwebImportView = View.extend({ } }); }); + this.assetstore = settings.assetstore; this.render(); }, render: function () { this.$el.html(DWASImportTemplate({ - assetstore: this.model + assetstore: this.assetstore })); return this; @@ -91,4 +95,6 @@ const DICOMwebImportView = View.extend({ } }); +assetstoreImportViewMap[AssetstoreType.DICOMWEB] = DICOMwebImportView; + export default DICOMwebImportView; From 0409e9cd7f193b6bc401ef9d2a6934c14ca6a692 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Wed, 10 Apr 2024 16:23:01 -0400 Subject: [PATCH 2/7] Bump dicomweb's girder version requirement --- sources/dicom/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sources/dicom/setup.py b/sources/dicom/setup.py index 5ca0fb120..b8ecddeea 100644 --- a/sources/dicom/setup.py +++ b/sources/dicom/setup.py @@ -69,6 +69,7 @@ def prerelease_local_scheme(version): install_requires=[ f'large-image{limit_version}', 'wsidicom>=0.9.0', + 'girder>=3.2.3.dev23', ], extras_require={ 'girder': f'girder-large-image{limit_version}', From 0febaa9900a26dfe3867609c6b16ef8370496953 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Thu, 11 Apr 2024 16:13:39 -0400 Subject: [PATCH 3/7] Update dicomweb assetstore version requirement --- sources/dicom/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/dicom/setup.py b/sources/dicom/setup.py index b8ecddeea..700ce450a 100644 --- a/sources/dicom/setup.py +++ b/sources/dicom/setup.py @@ -69,7 +69,7 @@ def prerelease_local_scheme(version): install_requires=[ f'large-image{limit_version}', 'wsidicom>=0.9.0', - 'girder>=3.2.3.dev23', + 'girder>=3.2.3', ], extras_require={ 'girder': f'girder-large-image{limit_version}', From 8d0d6ec303189696c4c76bda79d6808eb82df0f6 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Thu, 11 Apr 2024 16:13:58 -0400 Subject: [PATCH 4/7] Update dicomweb assetstore web tests with expected values --- sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js index a13f25a0b..d200ea5fb 100644 --- a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js +++ b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js @@ -143,7 +143,7 @@ describe('DICOMWeb assetstore', function () { }); waitsFor(function () { - return $('.g-validation-failed-message').html() === 'Invalid limit'; + return $('.g-validation-failed-message').html() === 'Invalid limit: 1.3'; }, 'Invalid limit check (float)'); runs(function () { @@ -156,7 +156,7 @@ describe('DICOMWeb assetstore', function () { }); waitsFor(function () { - return $('.g-validation-failed-message').html() === 'Invalid limit'; + return $('.g-validation-failed-message').html() === 'Invalid limit: -1'; }, 'Invalid limit check (negative)'); runs(function () { From e6801ef2df75bb5e24b5ce1ad33abd5458831496 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Fri, 12 Apr 2024 14:37:02 -0400 Subject: [PATCH 5/7] Change dicom source's girder dependency to be extras_require --- sources/dicom/setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sources/dicom/setup.py b/sources/dicom/setup.py index 700ce450a..e91120f4c 100644 --- a/sources/dicom/setup.py +++ b/sources/dicom/setup.py @@ -69,10 +69,9 @@ def prerelease_local_scheme(version): install_requires=[ f'large-image{limit_version}', 'wsidicom>=0.9.0', - 'girder>=3.2.3', ], extras_require={ - 'girder': f'girder-large-image{limit_version}', + 'girder': [f'girder-large-image{limit_version}', 'girder>=3.2.3'], }, include_package_data=True, keywords='large_image, tile source', From b14f3a415e4c89c4e3f1f0e13b238e7e27508a42 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Fri, 12 Apr 2024 14:46:47 -0400 Subject: [PATCH 6/7] Improve dwas adapter importData param handling --- .../assetstore/dicomweb_assetstore_adapter.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py index da2fba6c6..158f4cbbd 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py +++ b/sources/dicom/large_image_source_dicom/assetstore/dicomweb_assetstore_adapter.py @@ -504,7 +504,7 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): This dictionary may include the following keys: :limit: (optional) limit the number of studies imported. - :search_filters: (optional) a dictionary of additional search + :filters: (optional) a dictionary/JSON string of additional search filters to use with dicomweb_client's `search_for_series()` function. @@ -527,11 +527,13 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): if limit < 1: raise ValidationException(error_msg) - try: - search_filters = json.loads(params.get('filters') or '{}') - except json.JSONDecodeError as e: - msg = f'Invalid filters: "{params.get("filters")}". {e}' - raise ValidationException(msg) + search_filters = params.get('filters', {}) + if isinstance(search_filters, str): + try: + search_filters = json.loads(search_filters) + except json.JSONDecodeError as e: + msg = f'Invalid filters: "{params.get("filters")}". {e}' + raise ValidationException(msg) items = self._importData( parent, @@ -544,7 +546,7 @@ def importData(self, parent, parentType, params, progress, user, **kwargs): user, ) - if not items or len(items) == 0: + if not items: msg = 'No studies matching the search filters were found' raise ValidationException(msg) From a1355b88d5b7891c257ef0f789bb6ff5eb650e2d Mon Sep 17 00:00:00 2001 From: willdunklin Date: Fri, 12 Apr 2024 14:51:01 -0400 Subject: [PATCH 7/7] Make dwas girder extras_require conditional >=3.9 --- sources/dicom/setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sources/dicom/setup.py b/sources/dicom/setup.py index e91120f4c..dac379536 100644 --- a/sources/dicom/setup.py +++ b/sources/dicom/setup.py @@ -41,11 +41,14 @@ def prerelease_local_scheme(version): ], } +girder_extras = [f'girder-large-image{limit_version}'] + if sys.version_info >= (3, 9): # For Python >= 3.9, include the DICOMweb plugin entry_points['girder.plugin'] = [ 'dicomweb = large_image_source_dicom.girder_plugin:DICOMwebPlugin', ] + girder_extras.append('girder>=3.2.3') setup( name='large-image-source-dicom', @@ -71,7 +74,7 @@ def prerelease_local_scheme(version): 'wsidicom>=0.9.0', ], extras_require={ - 'girder': [f'girder-large-image{limit_version}', 'girder>=3.2.3'], + 'girder': girder_extras, }, include_package_data=True, keywords='large_image, tile source',