Merge branch 'master' into rasterio

girder · May 24, 2023 · 4c14c57 · 4c14c57
2 parents a43f09f + 16c2f91
commit 4c14c57
Show file tree

Hide file tree

Showing 23 changed files with 412 additions and 209 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,27 @@
 # Change Log
 
+## 1.21.1
+
+### Improvements
+- Parse qptiff and imagej vendor information ([#1168](../../pull/1168))
+
+## 1.21.0
+
+### Improvements
+- getPixel method and endpoint now returns rawer values ([#1155](../../pull/1155))
+- Allow more sophisticated filtering on item lists from the Girder client ([#1157](../../pull/1157), [#1159](../../pull/1159), [#1161](../../pull/1161))
+
+### Changes
+- Rename tiff exceptions to be better follow python standards ([#1162](../../pull/1162))
+- Require a newer version of girder ([#1163](../../pull/1163))
+- Add manual mongo index names where index names might get too long ([#1166](../../pull/1166))
+- Avoid using $unionWith for annotation searches as it isn't supported everywhere ([#1167](../../pull/1167))
+
+### Bug Fixes
+- The deepzoom tile source misreported the format of its tile output ([#1158](../../pull/1158))
+- Guard against errors in a log message ([#1164](../../pull/1164))
+- Fix thumbnail query typo ([#1165](../../pull/1165))
+
 ## 1.20.6
 
 ### Improvements

diff --git a/girder/girder_large_image/models/image_item.py b/girder/girder_large_image/models/image_item.py
@@ -499,7 +499,7 @@ def removeThumbnailFiles(self, item, keep=0, sort=None, imageKey=None,
             }
             if imageKey and key == 'isLargeImageThumbnail':
                 if imageKey == 'none':
-                    query['thumbnailKey'] = {'not': {'$regex': '"imageKey":'}}
+                    query['thumbnailKey'] = {'$not': {'$regex': '"imageKey":'}}
                 else:
                     query['thumbnailKey'] = {'$regex': '"imageKey":"%s"' % imageKey}
             query.update(kwargs)

diff --git a/girder/girder_large_image/rest/__init__.py b/girder/girder_large_image/rest/__init__.py
@@ -35,6 +35,11 @@ def altItemFind(self, folderId, text, name, limit, offset, sort, filters=None):
                 text = None
             except Exception as exc:
                 logger.warning('Failed to parse _filter_ from text field: %r', exc)
+        if filters:
+            try:
+                logger.debug('Item find filters: %s', json.dumps(filters))
+            except Exception:
+                pass
         if recurse:
             return _itemFindRecursive(
                 self, origItemFind, folderId, text, name, limit, offset, sort, filters)

diff --git a/girder/girder_large_image/web_client/package.json b/girder/girder_large_image/web_client/package.json
@@ -27,7 +27,7 @@
         "vue-color": "^2.8.1",
         "vue-loader": "~15.9.8",
         "vue-template-compiler": "~2.6.14",
-        "webpack": "^2.7.0",
+        "webpack": "^3",
         "yaml": "^2.1.1"
     },
     "main": "./index.js",

diff --git a/girder/girder_large_image/web_client/views/itemList.js b/girder/girder_large_image/web_client/views/itemList.js
@@ -219,50 +219,97 @@ wrap(ItemListWidget, 'render', function (render) {
         addToRoute({filter: this._generalFilter});
     };
 
+    this._unescapePhrase = (val) => {
+        if (val !== undefined) {
+            val = val.replace('\\\'', '\'').replace('\\"', '"').replace('\\\\', '\\');
+        }
+        return val;
+    };
+
     this._setFilter = () => {
         const val = this._generalFilter;
         let filter;
         const usedPhrases = {};
         const columns = (this._confList() || {}).columns || [];
         if (val !== undefined && val !== '' && columns.length) {
+            // a value can be surrounded by single or double quotes, which will
+            // be removed.
+            const quotedValue = /((?:"((?:[^\\"]|\\\\|\\")*)(?:"|$)|'((?:[^\\']|\\\\|\\')*)(?:'|$)|([^:,\s]+)))/g;
+            const phraseRE = new RegExp(
+                new RegExp('((?:' + quotedValue.source + ':|))').source +
+                /(-?)/.source +
+                quotedValue.source +
+                new RegExp('((?:,' + quotedValue.source + ')*)').source, 'g');
             filter = [];
-            val.match(/"[^"]*"|'[^']*'|\S+/g).forEach((phrase) => {
-                if (!phrase.length || usedPhrases[phrase]) {
-                    return;
+            [...val.matchAll(phraseRE)].forEach((match) => {
+                const coltag = this._unescapePhrase(match[5] || match[4] || match[3]);
+                const phrase = this._unescapePhrase(match[10] || match[9] || match[8]);
+                const negation = match[6] === '-';
+                var phrases = [{phrase: phrase, exact: match[8] !== undefined}];
+                if (match[11]) {
+                    [...match[11].matchAll(quotedValue)].forEach((submatch) => {
+                        const subphrase = this._unescapePhrase(submatch[4] || submatch[3] || submatch[2]);
+                        // remove dupes?
+                        if (subphrase && subphrase.length) {
+                            phrases.push({phrase: subphrase, exact: submatch[2] !== undefined});
+                        }
+                    });
                 }
-                usedPhrases[phrase] = true;
-                if (phrase[0] === phrase.substr(phrase.length - 1) && ['"', "'"].includes(phrase[0])) {
-                    phrase = phrase.substr(1, phrase.length - 2);
+                const key = `${coltag}:` + phrases.join('|||');
+                if (!phrases.length || usedPhrases[key]) {
+                    return;
                 }
-                const numval = +phrase;
-                /* If numval is a non-zero number not in exponential notation.
-                 * delta is the value of one for the least significant digit.
-                 * This will be NaN if phrase is not a number. */
-                const delta = Math.abs(+numval.toString().replace(/\d(?=.*[1-9](0*\.|)0*$)/g, '0').replace(/[1-9]/, '1'));
-                // escape for regex
-                phrase = phrase.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+                usedPhrases[key] = true;
                 const clause = [];
-                columns.forEach((col) => {
-                    let key;
+                phrases.forEach(({phrase, exact}) => {
+                    const numval = +phrase;
+                    /* If numval is a non-zero number not in exponential
+                     * notation, delta is the value of one for the least
+                     * significant digit.  This will be NaN if phrase is not a
+                     * number. */
+                    const delta = Math.abs(+numval.toString().replace(/\d(?=.*[1-9](0*\.|)0*$)/g, '0').replace(/[1-9]/, '1'));
+                    // escape for regex
+                    phrase = phrase.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 
-                    if (col.type === 'record' && col.value !== 'controls') {
-                        key = col.value;
-                    } else if (col.type === 'metadata') {
-                        key = 'meta.' + col.value;
-                    }
-                    if (key) {
-                        clause.push({[key]: {$regex: phrase, $options: 'i'}});
-                        if (!_.isNaN(numval)) {
-                            clause.push({[key]: {$eq: numval}});
-                            if (numval > 0 && delta) {
-                                clause.push({[key]: {$gte: numval, $lt: numval + delta}});
-                            } else if (numval < 0 && delta) {
-                                clause.push({[key]: {$lte: numval, $gt: numval + delta}});
+                    columns.forEach((col) => {
+                        let key;
+                        if (coltag &&
+                            coltag.localeCompare(col.title || col.value, undefined, {sensitivity: 'accent'}) &&
+                            coltag.localeCompare(col.value, undefined, {sensitivity: 'accent'})
+                        ) {
+                            return;
+                        }
+                        if (col.type === 'record' && col.value !== 'controls') {
+                            key = col.value;
+                        } else if (col.type === 'metadata') {
+                            key = 'meta.' + col.value;
+                        }
+                        if (!coltag && !exact) {
+                            const r = new RegExp('^' + (phrase.substr(phrase.length - 1) === ':' ? phrase.substr(0, phrase.length - 1) : phrase), 'i');
+                            if (r.exec(col.value) || r.exec(col.title || col.value)) {
+                                clause.push({[key]: {$exists: true}});
                             }
                         }
-                    }
+                        if (key && exact) {
+                            clause.push({[key]: {$regex: '^' + phrase + '$', $options: 'i'}});
+                        } else if (key) {
+                            clause.push({[key]: {$regex: phrase, $options: 'i'}});
+                            if (!_.isNaN(numval)) {
+                                clause.push({[key]: {$eq: numval}});
+                                if (numval > 0 && delta) {
+                                    clause.push({[key]: {$gte: numval, $lt: numval + delta}});
+                                } else if (numval < 0 && delta) {
+                                    clause.push({[key]: {$lte: numval, $gt: numval + delta}});
+                                }
+                            }
+                        }
+                    });
                 });
-                filter.push({$or: clause});
+                if (clause.length > 0) {
+                    filter.push(!negation ? {$or: clause} : {$nor: clause});
+                } else if (!negation) {
+                    filter.push({$or: [{_no_such_value_: '_no_such_value_'}]});
+                }
             });
             if (filter.length === 0) {
                 filter = undefined;
@@ -296,7 +343,16 @@ wrap(ItemListWidget, 'render', function (render) {
                 func = 'before';
             }
             if (base.length) {
-                base[func]('<span class="li-item-list-filter">Filter: <input class="li-item-list-filter-input""></input></span>');
+                base[func]('<span class="li-item-list-filter">Filter: <input class="li-item-list-filter-input" title="' +
+                    'All specified terms must be included.  ' +
+                    'Surround with single quotes to include spaces, double quotes for exact value match.  ' +
+                    'Prefix with - to exclude that value.  ' +
+                    'By default, all columns are searched.  ' +
+                    'Use <column>:<value1>[,<value2>...] to require that a column matches a specified value or any of a list of specified values.  ' +
+                    'Column and value names can be quoted to include spaces (single quotes for substring match, double quotes for exact value match).  ' +
+                    'If <column>:-<value1>[,<value2>...] is specified, matches will exclude the list of values.  ' +
+                    'Non-exact matches without a column specifier will also match columns that start with the specified value.  ' +
+                    '"></input></span>');
                 if (this._generalFilter) {
                     root.find('.li-item-list-filter-input').val(this._generalFilter);
                 }

diff --git a/girder/setup.py b/girder/setup.py
@@ -53,7 +53,7 @@ def prerelease_local_scheme(version):
         'Programming Language :: Python :: 3.11',
     ],
     install_requires=[
-        'girder>=3.0.4',
+        'girder>=3.1.18',
         'girder-jobs>=3.0.3',
         f'large_image{limit_version}',
         'importlib-metadata<5 ; python_version < "3.8"',

diff --git a/girder_annotation/girder_large_image_annotation/handlers.py b/girder_annotation/girder_large_image_annotation/handlers.py
@@ -111,13 +111,10 @@ def process_annotations(event):  # noqa: C901
     item = results['item']
     user = results['user']
 
-    startTime = time.time()
     file = File().load(
         event.info.get('file', {}).get('_id'),
         level=AccessType.READ, user=user
     )
-    if time.time() - startTime > 10:
-        logger.info('Loaded annotation file in %5.3fs', time.time() - startTime)
     startTime = time.time()
 
     if not file:

diff --git a/girder_annotation/girder_large_image_annotation/models/annotationelement.py b/girder_annotation/girder_large_image_annotation/models/annotationelement.py
@@ -63,16 +63,22 @@ def initialize(self):
                 ('bbox.lowx', SortDir.DESCENDING),
                 ('bbox.highx', SortDir.ASCENDING),
                 ('bbox.size', SortDir.DESCENDING),
-            ], {}),
+            ], {
+                'name': 'annotationBboxIdx'
+            }),
             ([
                 ('annotationId', SortDir.ASCENDING),
                 ('bbox.size', SortDir.DESCENDING),
-            ], {}),
+            ], {
+                'name': 'annotationBboxSizeIdx'
+            }),
             ([
                 ('annotationId', SortDir.ASCENDING),
                 ('_version', SortDir.DESCENDING),
                 ('element.group', SortDir.ASCENDING),
-            ], {}),
+            ], {
+                'name': 'annotationGroupIdx'
+            }),
             ([
                 ('created', SortDir.ASCENDING),
                 ('_version', SortDir.ASCENDING),

diff --git a/girder_annotation/girder_large_image_annotation/rest/annotation.py b/girder_annotation/girder_large_image_annotation/rest/annotation.py
@@ -582,20 +582,7 @@ def deleteItemAnnotations(self, item):
 
     def getFolderAnnotations(self, id, recurse, user, limit=False, offset=False, sort=False,
                              sortDir=False, count=False):
-        recursivePipeline = [
-            {'$graphLookup': {
-                'from': 'folder',
-                'startWith': '$_id',
-                'connectFromField': '_id',
-                'connectToField': 'parentId',
-                'as': '__children'
-            }},
-            {'$unwind': {'path': '$__children'}},
-            {'$replaceRoot': {'newRoot': '$__children'}},
-            {'$unionWith': {
-                'coll': 'folder',
-                'pipeline': [{'$match': {'_id': ObjectId(id)}}]
-            }}] if recurse else []
+
         accessPipeline = [
             {'$match': {
                 '$or': [
@@ -612,36 +599,59 @@ def getFolderAnnotations(self, id, recurse, user, limit=False, offset=False, sor
                 ]
             }}
         ] if not user['admin'] else []
-        pipeline = [
-            {'$match': {'_id': 'none'}},
-            {'$unionWith': {
-                'coll': 'folder',
-                'pipeline': [{'$match': {'_id': ObjectId(id)}}] +
-                recursivePipeline +
-                [{'$lookup': {
-                    'from': 'item',
-                    'localField': '_id',
-                    'foreignField': 'folderId',
-                    'as': '__items'
-                }}, {'$lookup': {
-                    'from': 'annotation',
-                    'localField': '__items._id',
-                    'foreignField': 'itemId',
-                    'as': '__annotations'
-                }}, {'$unwind': '$__annotations'},
-                    {'$replaceRoot': {'newRoot': '$__annotations'}},
-                    {'$match': {'_active': {'$ne': False}}}
-                ] + accessPipeline
+        recursivePipeline = [
+            {'$match': {'_id': ObjectId(id)}},
+            {'$facet': {
+                'documents1': [{'$match': {'_id': ObjectId(id)}}],
+                'documents2': [
+                    {'$graphLookup': {
+                        'from': 'folder',
+                        'startWith': '$_id',
+                        'connectFromField': '_id',
+                        'connectToField': 'parentId',
+                        'as': '__children'
+                    }},
+                    {'$unwind': {'path': '$__children'}},
+                    {'$replaceRoot': {'newRoot': '$__children'}}
+                ]
             }},
-        ]
+            {'$project': {'__children': {'$concatArrays': [
+                '$documents1', '$documents2'
+            ]}}},
+            {'$unwind': {'path': '$__children'}},
+            {'$replaceRoot': {'newRoot': '$__children'}}
+        ] if recurse else [{'$match': {'_id': ObjectId(id)}}]
+
+        # We are only finding anntoations that we can change the permissions
+        # on.  If we wanted to expose annotations based on a permissions level,
+        # we need to add a folder access pipeline immediately after the
+        # recursivePipleine that for write and above would include the
+        # ANNOTATION_ACCSESS_FLAG
+        pipeline = recursivePipeline + [
+            {'$lookup': {
+                'from': 'item',
+                'localField': '_id',
+                'foreignField': 'folderId',
+                'as': '__items'
+            }},
+            {'$lookup': {
+                'from': 'annotation',
+                'localField': '__items._id',
+                'foreignField': 'itemId',
+                'as': '__annotations'
+            }},
+            {'$unwind': '$__annotations'},
+            {'$replaceRoot': {'newRoot': '$__annotations'}},
+            {'$match': {'_active': {'$ne': False}}}
+        ] + accessPipeline
+
         if count:
             pipeline += [{'$count': 'count'}]
         else:
             pipeline = pipeline + [{'$sort': {sort: sortDir}}] if sort else pipeline
             pipeline = pipeline + [{'$skip': offset}] if offset else pipeline
             pipeline = pipeline + [{'$limit': limit}] if limit else pipeline
-
-        return Annotation().collection.aggregate(pipeline)
+        return Folder().collection.aggregate(pipeline)
 
     @autoDescribeRoute(
         Description('Check if the user owns any annotations for the items in a folder')