From 7239a1c7e1584eefcf331b67d6bf7cb65f1ed5f7 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 16 Oct 2023 15:11:53 -0400 Subject: [PATCH 1/8] Standardize DICOMweb assetstore parameter names/style --- .../assetstore/rest.py | 31 ++++++++++--------- .../dicomwebAssetstoreImportButton.pug | 11 ++++--- .../web_client/views/AssetstoreImportView.js | 12 +++---- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index 7994a212e..2f7fdecbd 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -2,9 +2,10 @@ from girder.api import access from girder.api.describe import Description, describeRoute -from girder.api.rest import Resource, loadmodel +from girder.api.rest import Resource, boundHandler 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 @@ -20,20 +21,20 @@ def _importData(self, assetstore, params): """ :param assetstore: the destination assetstore. - :param params: a dictionary of parameters including parentId, - parentType, progress, and filters. + :param params: a dictionary of parameters including destinationId, + destinationType, progress, and filters. """ - self.requireParams(('parentId'), params) + self.requireParams(('destinationId'), params) user = self.getCurrentUser() - parentType = params.get('parentType', 'folder') - if parentType not in ('folder', 'user', 'collection'): - msg = f'Invalid parentType: {parentType}' + destinationType = params.get('destinationType', 'folder') + if destinationType not in ('folder', 'user', 'collection'): + msg = f'Invalid destinationType: {destinationType}' raise RestException(msg) - parent = ModelImporter.model(parentType).load(params['parentId'], force=True, - exc=True) + parent = ModelImporter.model(destinationType).load(params['destinationId'], force=True, + exc=True) limit = params.get('limit') or None if limit is not None: @@ -61,7 +62,7 @@ def _importData(self, assetstore, params): ) as ctx: items = adapter.importData( parent, - parentType, + destinationType, { 'limit': limit, 'search_filters': search_filters, @@ -76,14 +77,14 @@ def _importData(self, assetstore, params): raise RestException(msg) @access.admin(scope=TokenScope.DATA_WRITE) - @loadmodel(model='assetstore') + @boundHandler @describeRoute( Description('Import references to DICOM objects from a DICOMweb server') - .param('id', 'The ID of the assetstore representing the DICOMweb server.', - paramType='path') - .param('parentId', 'The ID of the parent folder, collection, or user ' + .modelParam('id', 'The ID of the assetstore representing the DICOMweb server.', + model=Assetstore) + .param('destinationId', 'The ID of the parent folder, collection, or user ' 'in the Girder data hierarchy under which to import the files.') - .param('parentType', 'The type of the parent object to import into.', + .param('destinationType', 'The type of the parent object to import into.', enum=('folder', 'user', 'collection'), required=False) .param('limit', 'The maximum number of results to import.', 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 d508e1ceb..ca55a7747 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,5 +1,6 @@ -a.g-dwas-import-button.btn.btn-sm.btn-success( - href=`#dicomweb_assetstore/${assetstore.get('_id')}/import`, - title="Import references to DICOM objects from a DICOMweb server") - i.icon-link-ext - | Import +.g-assetstore-import-button-container + a.g-dwas-import-button.btn.btn-sm.btn-success( + href=`#dicomweb_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/AssetstoreImportView.js b/sources/dicom/large_image_source_dicom/web_client/views/AssetstoreImportView.js index 2d82ce35b..b49950fe6 100644 --- a/sources/dicom/large_image_source_dicom/web_client/views/AssetstoreImportView.js +++ b/sources/dicom/large_image_source_dicom/web_client/views/AssetstoreImportView.js @@ -13,25 +13,25 @@ const AssetstoreImportView = View.extend({ e.preventDefault(); this.$('.g-validation-failed-message').empty(); - const parentType = this.$('#g-dwas-import-dest-type').val(); - const parentId = this.$('#g-dwas-import-dest-id').val().trim().split(/\s/)[0]; + const destinationType = this.$('#g-dwas-import-dest-type').val(); + const destinationId = this.$('#g-dwas-import-dest-id').val().trim().split(/\s/)[0]; const filters = this.$('#g-dwas-import-filters').val().trim(); const limit = this.$('#g-dwas-import-limit').val().trim(); - if (!parentId) { + if (!destinationId) { this.$('.g-validation-failed-message').html('Invalid Destination ID'); return; } this.$('.g-submit-dwas-import').addClass('disabled'); this.model.off().on('g:imported', function () { - router.navigate(parentType + '/' + parentId, { trigger: true }); + 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({ - parentId, - parentType, + destinationId, + destinationType, limit, filters, progress: true From f3112d0d89b64b496579e9c12541a9c7fcf1aacf Mon Sep 17 00:00:00 2001 From: willdunklin Date: Tue, 17 Oct 2023 10:58:15 -0400 Subject: [PATCH 2/8] Change dwas import route to POST for consistency --- sources/dicom/large_image_source_dicom/assetstore/rest.py | 2 +- .../web_client/models/AssetstoreModel.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index 2f7fdecbd..1a4d4db5d 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -15,7 +15,7 @@ class DICOMwebAssetstoreResource(Resource): def __init__(self): super().__init__() self.resourceName = 'dicomweb_assetstore' - self.route('PUT', (':id', 'import'), self.importData) + self.route('POST', (':id', 'import'), self.importData) def _importData(self, assetstore, params): """ 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 index 016068ff3..c8313a140 100644 --- a/sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js +++ b/sources/dicom/large_image_source_dicom/web_client/models/AssetstoreModel.js @@ -7,7 +7,7 @@ import { restRequest } from '@girder/core/rest'; AssetstoreModel.prototype.dicomwebImport = function (params) { return restRequest({ url: 'dicomweb_assetstore/' + this.get('_id') + '/import', - type: 'PUT', + type: 'POST', data: params, error: null }).done(() => { From 2914dbbb63f2778ee44db3779da04d2c77243bd6 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Tue, 17 Oct 2023 16:16:09 -0400 Subject: [PATCH 3/8] Update importData param defaults --- sources/dicom/large_image_source_dicom/assetstore/rest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index 1a4d4db5d..1cc122195 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -86,13 +86,13 @@ def _importData(self, assetstore, params): 'in the Girder data hierarchy under which to import the files.') .param('destinationType', 'The type of the parent object to import into.', enum=('folder', 'user', 'collection'), - required=False) + required=True) .param('limit', 'The maximum number of results to import.', required=False, dataType='int') .param('filters', 'Any search parameters to filter DICOM objects.', - required=False) + required=False, default={}) .param('progress', 'Whether to record progress on this operation (' - 'default=False)', required=False, dataType='boolean') + 'default=False)', required=False, default=False, dataType='boolean') .errorResponse() .errorResponse('You are not an administrator.', 403), ) From ca950c3ec9d8a79028ae79d7c5d5eeaf4ad2a2e8 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 30 Oct 2023 10:41:46 -0400 Subject: [PATCH 4/8] Remove extraneous dwas requireParams check --- sources/dicom/large_image_source_dicom/assetstore/rest.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index 1cc122195..dc6d93b0b 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -24,8 +24,6 @@ def _importData(self, assetstore, params): :param params: a dictionary of parameters including destinationId, destinationType, progress, and filters. """ - self.requireParams(('destinationId'), params) - user = self.getCurrentUser() destinationType = params.get('destinationType', 'folder') From 1b11b3ccdc93e51218c06c282d21951a6837ae12 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 30 Oct 2023 10:43:08 -0400 Subject: [PATCH 5/8] Rename dicomWebSpec test parentId to destinationId for consistency --- .../web_client_specs/dicomWebSpec.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js index 3895c067d..73a9b44b2 100644 --- a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js +++ b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js @@ -11,8 +11,8 @@ describe('DICOMWeb assetstore', function () { 'Admin', 'adminpassword!')); it('Create an assetstore and import data', function () { - var parentId; - var parentType; + var destinationId; + var destinationType; // After importing, we will verify that this item exists const verifyItemName = '1.3.6.1.4.1.5962.99.1.3510881361.982628633.1635598486609.2.0'; @@ -54,7 +54,7 @@ describe('DICOMWeb assetstore', function () { }, 'DICOMweb assetstore to be created'); runs(function () { - // Select the parentId and parentType + // Select the destinationId and destinationType // Get the user ID const resp = girder.rest.restRequest({ url: 'user', @@ -70,15 +70,15 @@ describe('DICOMWeb assetstore', function () { type: 'GET', async: false, data: { - parentType: 'user', - parentId: userId, + destinationType: 'user', + destinationId: userId, name: 'Public', } }); // Use the user's public folder - parentType = 'folder'; - parentId = resp.responseJSON[0]._id; + destinationType = 'folder'; + destinationId = resp.responseJSON[0]._id; }); runs(function () { @@ -103,8 +103,8 @@ describe('DICOMWeb assetstore', function () { runs(function () { // Set dest type and dest id - $('#g-dwas-import-dest-type').val(parentType); - $('#g-dwas-import-dest-id').val(parentId); + $('#g-dwas-import-dest-type').val(destinationType); + $('#g-dwas-import-dest-id').val(destinationId); // Test error for an invalid limit $('#g-dwas-import-limit').val('1.3'); From af694114748aac80d03f1fb2a1491902e26ba471 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 30 Oct 2023 10:54:14 -0400 Subject: [PATCH 6/8] Update dwas importData route decorators --- sources/dicom/large_image_source_dicom/assetstore/rest.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/sources/dicom/large_image_source_dicom/assetstore/rest.py b/sources/dicom/large_image_source_dicom/assetstore/rest.py index dc6d93b0b..3a9a47f49 100644 --- a/sources/dicom/large_image_source_dicom/assetstore/rest.py +++ b/sources/dicom/large_image_source_dicom/assetstore/rest.py @@ -1,8 +1,8 @@ import json from girder.api import access -from girder.api.describe import Description, describeRoute -from girder.api.rest import Resource, boundHandler +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 @@ -75,8 +75,7 @@ def _importData(self, assetstore, params): raise RestException(msg) @access.admin(scope=TokenScope.DATA_WRITE) - @boundHandler - @describeRoute( + @autoDescribeRoute( Description('Import references to DICOM objects from a DICOMweb server') .modelParam('id', 'The ID of the assetstore representing the DICOMweb server.', model=Assetstore) From d203d84cdf1bf6ed2d79d655ab95917a9faf38c3 Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 30 Oct 2023 16:23:10 -0400 Subject: [PATCH 7/8] Correct dicomWebSpec.js rest request params --- 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 73a9b44b2..cfbd83f8c 100644 --- a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js +++ b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js @@ -70,8 +70,8 @@ describe('DICOMWeb assetstore', function () { type: 'GET', async: false, data: { - destinationType: 'user', - destinationId: userId, + parentType: 'user', + parentId: userId, name: 'Public', } }); From c92abbcbb9095c0fa6beaf099632611c7288597a Mon Sep 17 00:00:00 2001 From: willdunklin Date: Mon, 30 Oct 2023 18:33:07 -0400 Subject: [PATCH 8/8] Correct dicomWebSpec test case to handle autoDescribeRoute --- sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js index cfbd83f8c..d8ecab991 100644 --- a/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js +++ b/sources/dicom/test_dicom/web_client_specs/dicomWebSpec.js @@ -112,7 +112,7 @@ describe('DICOMWeb assetstore', function () { }); waitsFor(function () { - return $('.g-validation-failed-message').html() === 'Invalid limit'; + return $('.g-validation-failed-message').html().includes('Invalid value'); }, 'Invalid limit check (float)'); runs(function () {