From 8ff0e3e1dc9e97fdf98bf358d33f815f9d9286dd Mon Sep 17 00:00:00 2001 From: Jeffrey Baumes Date: Wed, 25 Sep 2024 07:35:27 -0400 Subject: [PATCH 1/4] Setting global config file based on settings, so table view is always active --- girder/girder_large_image/__init__.py | 77 ++++++++++++++++++---- girder/girder_large_image/rest/__init__.py | 5 +- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/girder/girder_large_image/__init__.py b/girder/girder_large_image/__init__.py index a3d163145..b3b8af17d 100644 --- a/girder/girder_large_image/__init__.py +++ b/girder/girder_large_image/__init__.py @@ -476,6 +476,12 @@ def adjustConfigForUser(config, user): """ if not isinstance(config, dict): return config + if isinstance(config.get('access'), dict): + accessList = config.pop('access') + if user and isinstance(accessList.get('user'), dict): + config = _mergeDictionaries(config, accessList['user']) + if user and user.get('admin') and isinstance(accessList.get('admin'), dict): + config = _mergeDictionaries(config, accessList['admin']) if isinstance(config.get('groups'), dict): groups = config.pop('groups') if user: @@ -483,14 +489,45 @@ def adjustConfigForUser(config, user): {'_id': {'$in': user['groups']}}, sort=[('name', SortDir.ASCENDING)]): if isinstance(groups.get(group['name']), dict): config = _mergeDictionaries(config, groups[group['name']]) - if isinstance(config.get('access'), dict): - accessList = config.pop('access') - if user and isinstance(accessList.get('user'), dict): - config = _mergeDictionaries(config, accessList['user']) - if user and user.get('admin') and isinstance(accessList.get('admin'), dict): - config = _mergeDictionaries(config, accessList['admin']) + if isinstance(config.get('users'), dict): + users = config.pop('users') + if user and user['login'] in users: + config = _mergeDictionaries(config, users[user['login']]) return config +def addSettingsToConfig(config, user): + columns = [] + + showThumbnails = Setting().get(constants.PluginSettings.LARGE_IMAGE_SHOW_THUMBNAILS) + if showThumbnails: + columns.append({'type': 'image', 'value': 'thumbnail', 'title': 'Thumbnail'}) + + extraSetting = constants.PluginSettings.LARGE_IMAGE_SHOW_EXTRA_PUBLIC + if user is not None: + if user['admin']: + extraSetting = constants.PluginSettings.LARGE_IMAGE_SHOW_EXTRA_ADMIN + else: + extraSetting = constants.PluginSettings.LARGE_IMAGE_SHOW_EXTRA + + showExtra = None + try: + showExtra = json.loads(Setting().get(extraSetting)) + except Exception: + pass + if isinstance(showExtra, dict) and 'images' in showExtra and isinstance(showExtra['images'], list): + for value in showExtra['images']: + if value != '*': + columns.append({'type': 'image', 'value': value, 'title': value.title()}) + + columns.append({'type': 'record', 'value': 'name', 'title': 'Name'}) + columns.append({'type': 'record', 'value': 'controls', 'title': 'Contols'}) + columns.append({'type': 'record', 'value': 'size', 'title': 'Size'}) + + if 'itemList' not in config: + config['itemList'] = {'columns': columns} + if 'itemListDialog' not in config: + config['itemListDialog'] = {'columns': columns} + def yamlConfigFile(folder, name, user): """ @@ -515,12 +552,14 @@ def yamlConfigFile(folder, name, user): if isinstance(config, list) and len(config) == 1: config = config[0] # combine and adjust config values based on current user - if isinstance(config, dict) and 'access' in config or 'group' in config: + if isinstance(config, dict) and ('access' in config or 'groups' in config or 'users' in config): config = adjustConfigForUser(config, user) if addConfig and isinstance(config, dict): config = _mergeDictionaries(config, addConfig) if not isinstance(config, dict) or config.get('__inherit__') is not True: - return config + addConfig = config + last = True + break config.pop('__inherit__') addConfig = config if last: @@ -541,10 +580,16 @@ def yamlConfigFile(folder, name, user): last = True else: folder = Folder().load(folder['parentId'], user=user, level=AccessType.READ) + + if addConfig is None: + addConfig = {} + + addSettingsToConfig(addConfig, user) + return addConfig -def yamlConfigFileWrite(folder, name, user, yaml_config): +def yamlConfigFileWrite(folder, name, user, yaml_config, user_context): """ If the user has appropriate permissions, create or modify an item in the specified folder with the specified name, storing the config value as a @@ -554,24 +599,32 @@ def yamlConfigFileWrite(folder, name, user, yaml_config): :param name: the name of the config file. :param user: the user that the response if adjusted for. :param yaml_config: a yaml config string. + :param user_context: whether these settings should only apply to the current user. """ - # Check that we have valid yaml - yaml.safe_load(yaml_config) + yaml_parsed = yaml.safe_load(yaml_config) item = Item().createItem(name, user, folder, reuseExisting=True) existingFiles = list(Item().childFiles(item)) if (len(existingFiles) == 1 and existingFiles[0]['mimeType'] == 'application/yaml' and existingFiles[0]['name'] == name): + if user_context: + file = yaml.safe_load(File().open(existingFiles[0]).read()) + file.setdefault('users', {}) + file['users'].setdefault(user['login'], {}) + file['users'][user['login']].update(yaml_parsed) + yaml_config = yaml.safe_dump(file) upload = Upload().createUploadToFile( existingFiles[0], user, size=len(yaml_config)) else: + if user_context: + yaml_config = yaml.safe_dump({'users': {user['login']: yaml_parsed}}) upload = Upload().createUpload( user, name, 'item', item, size=len(yaml_config), mimeType='application/yaml', save=True) newfile = Upload().handleChunk(upload, yaml_config) with _configWriteLock: for entry in list(Item().childFiles(item)): - if entry['_id'] != newfile['_id'] and len(Item().childFiles(item)) > 1: + if entry['_id'] != newfile['_id'] and len(list(Item().childFiles(item))) > 1: File().remove(entry) diff --git a/girder/girder_large_image/rest/__init__.py b/girder/girder_large_image/rest/__init__.py index 07c3c5654..225798dee 100644 --- a/girder/girder_large_image/rest/__init__.py +++ b/girder/girder_large_image/rest/__init__.py @@ -141,13 +141,14 @@ def getYAMLConfigFile(self, folder, name): 'file may be permanently deleted.') .modelParam('id', model=Folder, level=AccessType.WRITE) .param('name', 'The name of the file.', paramType='path') + .param('user_context', 'Whether these settings should only apply to the current user.', paramType='query', dataType='boolean', default=False) .param('config', 'The contents of yaml config file to validate.', paramType='body'), ) @boundHandler() -def putYAMLConfigFile(self, folder, name, config): +def putYAMLConfigFile(self, folder, name, config, user_context): from .. import yamlConfigFileWrite user = self.getCurrentUser() config = config.read().decode('utf8') - return yamlConfigFileWrite(folder, name, user, config) + return yamlConfigFileWrite(folder, name, user, config, user_context) From 3bac9b65832641134ed2d05755e8648bdd47c8b9 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 25 Sep 2024 11:54:47 -0400 Subject: [PATCH 2/4] Fix tests. Readjust how group access level parameters are applied and document why. --- CHANGELOG.md | 1 + docs/girder_config_options.rst | 4 +++ girder/girder_large_image/__init__.py | 35 ++++++++++++------- girder/test_girder/test_large_image.py | 2 +- .../web_client_specs/imageViewerSpec.js | 2 +- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d989cf1c..608c6b470 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Improvements - Better handle images without enough tile layers ([#1648](../../pull/1648)) +- Add users option to config files; have a default config file ([#1649](../../pull/1649)) ## 1.29.11 diff --git a/docs/girder_config_options.rst b/docs/girder_config_options.rst index 069f50505..601504624 100644 --- a/docs/girder_config_options.rst +++ b/docs/girder_config_options.rst @@ -38,6 +38,10 @@ The yaml file has the following structure: : # groups can specify access based on user or admin, too. access: ... + # The users key specifies that a specific user has distinct settings + users: + : + : # If __inherit__ is true, then merge this config file with the next config # file in the parent folder hierarchy. __inherit__: true diff --git a/girder/girder_large_image/__init__.py b/girder/girder_large_image/__init__.py index b3b8af17d..02e5e1acb 100644 --- a/girder/girder_large_image/__init__.py +++ b/girder/girder_large_image/__init__.py @@ -476,12 +476,6 @@ def adjustConfigForUser(config, user): """ if not isinstance(config, dict): return config - if isinstance(config.get('access'), dict): - accessList = config.pop('access') - if user and isinstance(accessList.get('user'), dict): - config = _mergeDictionaries(config, accessList['user']) - if user and user.get('admin') and isinstance(accessList.get('admin'), dict): - config = _mergeDictionaries(config, accessList['admin']) if isinstance(config.get('groups'), dict): groups = config.pop('groups') if user: @@ -489,13 +483,31 @@ def adjustConfigForUser(config, user): {'_id': {'$in': user['groups']}}, sort=[('name', SortDir.ASCENDING)]): if isinstance(groups.get(group['name']), dict): config = _mergeDictionaries(config, groups[group['name']]) + # Do this after merging groups, because the group access-level values can + # override the base access-level options. For instance, if the base has + # an admin option, and the group has a user option, then doing this before + # group application can end up with user options for an admin. + if isinstance(config.get('access'), dict): + accessList = config.pop('access') + if user and isinstance(accessList.get('user'), dict): + config = _mergeDictionaries(config, accessList['user']) + if user and user.get('admin') and isinstance(accessList.get('admin'), dict): + config = _mergeDictionaries(config, accessList['admin']) if isinstance(config.get('users'), dict): users = config.pop('users') if user and user['login'] in users: config = _mergeDictionaries(config, users[user['login']]) return config + def addSettingsToConfig(config, user): + """ + Add the settings for showing thumbnails and images in item lists to a + config file if the itemList or itemListDialog options are not set. + + :param config: the config dictionary to modify. + :param user: the current user. + """ columns = [] showThumbnails = Setting().get(constants.PluginSettings.LARGE_IMAGE_SHOW_THUMBNAILS) @@ -514,7 +526,8 @@ def addSettingsToConfig(config, user): showExtra = json.loads(Setting().get(extraSetting)) except Exception: pass - if isinstance(showExtra, dict) and 'images' in showExtra and isinstance(showExtra['images'], list): + if (isinstance(showExtra, dict) and 'images' in showExtra and + isinstance(showExtra['images'], list)): for value in showExtra['images']: if value != '*': columns.append({'type': 'image', 'value': value, 'title': value.title()}) @@ -552,7 +565,8 @@ def yamlConfigFile(folder, name, user): if isinstance(config, list) and len(config) == 1: config = config[0] # combine and adjust config values based on current user - if isinstance(config, dict) and ('access' in config or 'groups' in config or 'users' in config): + if isinstance(config, dict) and ( + 'access' in config or 'groups' in config or 'users' in config): config = adjustConfigForUser(config, user) if addConfig and isinstance(config, dict): config = _mergeDictionaries(config, addConfig) @@ -581,11 +595,8 @@ def yamlConfigFile(folder, name, user): else: folder = Folder().load(folder['parentId'], user=user, level=AccessType.READ) - if addConfig is None: - addConfig = {} - + addConfig = {} if addConfig is None else addConfig addSettingsToConfig(addConfig, user) - return addConfig diff --git a/girder/test_girder/test_large_image.py b/girder/test_girder/test_large_image.py index c916e38a2..0b00a095e 100644 --- a/girder/test_girder/test_large_image.py +++ b/girder/test_girder/test_large_image.py @@ -482,7 +482,7 @@ def testYAMLConfigFile(server, admin, user, fsAssetstore): path='/folder/%s/yaml_config/sample.json' % str(colFolderB['_id']), method='GET') assert utilities.respStatus(resp) == 200 - assert resp.json is None + assert resp.json['itemList'] is not None colFolderConfig = Folder().createFolder( collection, '.config', parentType='collection', diff --git a/girder/test_girder/web_client_specs/imageViewerSpec.js b/girder/test_girder/web_client_specs/imageViewerSpec.js index a6d1d1bb7..5d56bf3fb 100644 --- a/girder/test_girder/web_client_specs/imageViewerSpec.js +++ b/girder/test_girder/web_client_specs/imageViewerSpec.js @@ -207,7 +207,7 @@ $(function () { }); it('test the metadata columns are not shown', function () { runs(function () { - expect($('.large_image_container').length).toBeGreaterThan(0); + expect($('.large_image_container').length).toBe(0); expect($('.large_image_thumbnail').length).toBeGreaterThan(0); expect($('.li-column-metadata').length).toBe(0); }); From f2facdbac61c9a7133e8de7d7d7a37352a699a17 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 25 Sep 2024 12:09:05 -0400 Subject: [PATCH 3/4] Allow a non-admin user to create a settings file with a user context --- girder/girder_large_image/__init__.py | 2 +- girder/girder_large_image/rest/__init__.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/girder/girder_large_image/__init__.py b/girder/girder_large_image/__init__.py index 02e5e1acb..1c0f90b0f 100644 --- a/girder/girder_large_image/__init__.py +++ b/girder/girder_large_image/__init__.py @@ -613,7 +613,7 @@ def yamlConfigFileWrite(folder, name, user, yaml_config, user_context): :param user_context: whether these settings should only apply to the current user. """ yaml_parsed = yaml.safe_load(yaml_config) - item = Item().createItem(name, user, folder, reuseExisting=True) + item = Item().createItem(name, user, folder, reuseExisting=True, force=bool(user_context)) existingFiles = list(Item().childFiles(item)) if (len(existingFiles) == 1 and existingFiles[0]['mimeType'] == 'application/yaml' and diff --git a/girder/girder_large_image/rest/__init__.py b/girder/girder_large_image/rest/__init__.py index 225798dee..ecb33766f 100644 --- a/girder/girder_large_image/rest/__init__.py +++ b/girder/girder_large_image/rest/__init__.py @@ -141,7 +141,8 @@ def getYAMLConfigFile(self, folder, name): 'file may be permanently deleted.') .modelParam('id', model=Folder, level=AccessType.WRITE) .param('name', 'The name of the file.', paramType='path') - .param('user_context', 'Whether these settings should only apply to the current user.', paramType='query', dataType='boolean', default=False) + .param('user_context', 'Whether these settings should only apply to the ' + 'current user.', paramType='query', dataType='boolean', default=False) .param('config', 'The contents of yaml config file to validate.', paramType='body'), ) From 94322395c4675f893b404d4e5c7ad7da3c2c9a79 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 25 Sep 2024 12:44:50 -0400 Subject: [PATCH 4/4] Fix access for user context on writing config files. This has to be done when checking if we can read or write the folder. --- girder/girder_large_image/__init__.py | 2 +- girder/girder_large_image/rest/__init__.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/girder/girder_large_image/__init__.py b/girder/girder_large_image/__init__.py index 1c0f90b0f..02e5e1acb 100644 --- a/girder/girder_large_image/__init__.py +++ b/girder/girder_large_image/__init__.py @@ -613,7 +613,7 @@ def yamlConfigFileWrite(folder, name, user, yaml_config, user_context): :param user_context: whether these settings should only apply to the current user. """ yaml_parsed = yaml.safe_load(yaml_config) - item = Item().createItem(name, user, folder, reuseExisting=True, force=bool(user_context)) + item = Item().createItem(name, user, folder, reuseExisting=True) existingFiles = list(Item().childFiles(item)) if (len(existingFiles) == 1 and existingFiles[0]['mimeType'] == 'application/yaml' and diff --git a/girder/girder_large_image/rest/__init__.py b/girder/girder_large_image/rest/__init__.py index ecb33766f..f06174673 100644 --- a/girder/girder_large_image/rest/__init__.py +++ b/girder/girder_large_image/rest/__init__.py @@ -131,7 +131,7 @@ def getYAMLConfigFile(self, folder, name): return yamlConfigFile(folder, name, user) -@access.public(scope=TokenScope.DATA_WRITE) +@access.public(scope=TokenScope.DATA_READ) @autoDescribeRoute( Description('Get a config file.') .notes( @@ -139,7 +139,7 @@ def getYAMLConfigFile(self, folder, name): 'specified name containing a single file also of the specified ' 'name. The file is added to the default assetstore, and any existing ' 'file may be permanently deleted.') - .modelParam('id', model=Folder, level=AccessType.WRITE) + .modelParam('id', model=Folder, level=AccessType.READ) .param('name', 'The name of the file.', paramType='path') .param('user_context', 'Whether these settings should only apply to the ' 'current user.', paramType='query', dataType='boolean', default=False) @@ -151,5 +151,7 @@ def putYAMLConfigFile(self, folder, name, config, user_context): from .. import yamlConfigFileWrite user = self.getCurrentUser() + if not user_context: + Folder().requireAccess(folder, user, AccessType.WRITE) config = config.read().decode('utf8') return yamlConfigFileWrite(folder, name, user, config, user_context)