This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch db/8607 in repository https://gitbox.apache.org/repos/asf/allura.git
commit 0059dc966ecd2399a59932a2c84c4ca84ca01a47 Author: Dave Brondsema <[email protected]> AuthorDate: Fri May 22 16:55:29 2026 -0400 [#8607] solr: change ticket searches from **kw passthru to explicit --- Allura/allura/lib/search.py | 14 +++++++-- ForgeTracker/forgetracker/model/ticket.py | 34 +++++++++++----------- .../forgetracker/tests/functional/test_root.py | 1 + .../forgetracker/tests/unit/test_ticket_model.py | 16 ++++++---- ForgeTracker/forgetracker/tracker_main.py | 33 +++++++++++---------- 5 files changed, 58 insertions(+), 40 deletions(-) diff --git a/Allura/allura/lib/search.py b/Allura/allura/lib/search.py index 69531f5e9..147c30c47 100644 --- a/Allura/allura/lib/search.py +++ b/Allura/allura/lib/search.py @@ -161,11 +161,20 @@ def search(q, short_timeout=False, ignore_errors=True, **kw): (match.group(1) if match else e)) -def search_artifact(atype, q, history=False, rows=10, short_timeout=False, filter=None, **kw): +def search_artifact(atype, q, history=False, rows=10, short_timeout=False, filter=None, + sort: str | None = None, start: int | None = None, fl: str | None = None, + **kw): """Performs SOLR search. Raises SearchError if SOLR returns an error. + + :param kwargs: `fq` or various `facet.*` params can be passed through """ + + for k in kw: + if k not in ['fq', 'facet', 'facet.field', 'facet.limit', 'facet.sort', 'facet.mincount']: + raise ValueError(f'Unexpected kwarg {k} passed to search_artifact') + # first, grab an artifact and get the fields that it indexes a = atype.query.find().first() if a is None: @@ -199,7 +208,8 @@ def search_artifact(atype, q, history=False, rows=10, short_timeout=False, filte fq.append(' OR '.join(parts)) if not history: fq.append('is_history_b:False') - return search(q, fq=fq, rows=rows, short_timeout=short_timeout, ignore_errors=False, **kw) + return search(q, fq=fq, rows=rows, short_timeout=short_timeout, ignore_errors=False, sort=sort, start=start, + fl=fl, **kw) def site_admin_search(model, q, field, **kw): diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py index a2e51a334..9dac8202a 100644 --- a/ForgeTracker/forgetracker/model/ticket.py +++ b/ForgeTracker/forgetracker/model/ticket.py @@ -1223,7 +1223,7 @@ def __json__(self, posts_limit=None, is_export=False): custom_fields=dict(self.custom_fields)) @classmethod - def paged_query(cls, app_config, user, query, limit=None, page=0, sort=None, deleted=False, **kw): + def paged_query(cls, app_config, user, query, limit=None, page=0, sort=None, deleted=False): """ Query tickets, filtering for 'read' permission, sorting and paginating the result. @@ -1253,21 +1253,18 @@ def paged_query(cls, app_config, user, query, limit=None, page=0, sort=None, del return dict( tickets=tickets, - count=count, q=json.dumps(query), limit=limit, page=page, sort=sort, - **kw) + count=count, q=json.dumps(query), limit=limit, page=page, sort=sort) @classmethod def paged_search(cls, app_config, user, q, limit=None, page=0, sort=None, show_deleted=False, - filter=None, **kw): + filter=None): """Query tickets from Solr, filtering for 'read' permission, sorting and paginating the result. See also paged_query which does a mongo search. We do the sorting and skipping right in SOLR, before we ever ask - Mongo for the actual tickets. Other keywords for - search_artifact (e.g., history) or for SOLR are accepted through - kw. The output is intended to be used directly in templates, - e.g., exposed controller methods can just: + Mongo for the actual tickets. The output is intended to be used + directly in templates, e.g., exposed controller methods can just: return paged_query(q, ...) @@ -1290,8 +1287,7 @@ def paged_search(cls, app_config, user, q, limit=None, page=0, sort=None, show_d try: if q: # also query for choices for filter options right away - params = kw.copy() - params.update(tsearch.FACET_PARAMS) + params = dict(tsearch.FACET_PARAMS) if not show_deleted: params['fq'] = ['deleted_b:False'] @@ -1331,15 +1327,17 @@ def paged_search(cls, app_config, user, q, limit=None, page=0, sort=None, show_d count=count, q=q, limit=limit, page=page, sort=sort, filter=filter, filter_choices=tsearch.get_facets(matches), - solr_error=solr_error, **kw) + solr_error=solr_error) @classmethod def paged_query_or_search(cls, app_config, user, query, search_query, filter, - limit=None, page=0, sort=None, **kw): + limit=None, page=0, sort=None, deleted=False, show_deleted=False): """Switch between paged_query and paged_search based on filter. - query - query in mongo syntax - search_query - query in solr syntax + query - query in mongo syntax (used by paged_query path) + search_query - query in solr syntax (used by paged_search path) + deleted - value for mongo 'deleted' field (used by paged_query path) + show_deleted - whether deleted records should be included (used by paged_search path) """ solr_sort = None if sort and ' ' in sort: @@ -1348,15 +1346,17 @@ def paged_query_or_search(cls, app_config, user, query, search_query, filter, solr_col = _mongo_col_to_solr_col(sort_split[0]) solr_sort = f'{solr_col} {sort_split[1]}' if not filter: - result = cls.paged_query(app_config, user, query, sort=sort, limit=limit, page=page, **kw) + result = cls.paged_query(app_config, user, query, sort=sort, limit=limit, page=page, + deleted=deleted) t = cls.query.find().first() if t: search_query = cls.translate_query(search_query, t.index()) result['filter_choices'] = tsearch.query_filter_choices( - search_query, fq=[] if kw.get('show_deleted', False) else ['deleted_b:False']) + search_query, fq=[] if show_deleted else ['deleted_b:False']) else: result = cls.paged_search(app_config, user, search_query, filter=filter, - sort=solr_sort, limit=limit, page=page, **kw) + sort=solr_sort, limit=limit, page=page, + show_deleted=show_deleted) result['sort'] = sort result['url_sort'] = solr_sort if solr_sort else '' diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py index 92259e040..acd307dc1 100644 --- a/ForgeTracker/forgetracker/tests/functional/test_root.py +++ b/ForgeTracker/forgetracker/tests/functional/test_root.py @@ -1378,6 +1378,7 @@ def test_search(self): # 'filter' is special kwarg, don't let it cause problems r = self.app.get('/p/test/bugs/search/?q=test&filter=blah') + r = self.app.get('/p/test/bugs/search/?q=test&defType=asdf') def test_search_canonical(self): self.new_ticket(summary='test first ticket') diff --git a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py index dcd1f32e1..2afd810cc 100644 --- a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py +++ b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py @@ -374,18 +374,24 @@ def test_paged_query_or_search(self, query, search, tsearch): app_cfg, user = mock.Mock(), mock.Mock() mongo_query = 'mongo query' solr_query = 'solr query' - kw = {'kw1': 'test1', 'kw2': 'test2'} + deleted_val = {'$in': [False]} filter = None - Ticket.paged_query_or_search(app_cfg, user, mongo_query, solr_query, filter, **kw) - query.assert_called_once_with(app_cfg, user, mongo_query, sort=None, limit=None, page=0, **kw) + Ticket.paged_query_or_search(app_cfg, user, mongo_query, solr_query, filter, + deleted=deleted_val, show_deleted=False) + # mongo path: 'deleted' is forwarded, 'show_deleted' is not + query.assert_called_once_with(app_cfg, user, mongo_query, sort=None, limit=None, page=0, + deleted=deleted_val) assert tsearch.query_filter_choices.call_count == 1 assert tsearch.query_filter_choices.call_args[0][0] == 'solr query' assert search.call_count == 0 query.reset_mock(), search.reset_mock(), tsearch.reset_mock() filter = {'status': 'unread'} - Ticket.paged_query_or_search(app_cfg, user, mongo_query, solr_query, filter, **kw) - search.assert_called_once_with(app_cfg, user, solr_query, filter=filter, sort=None, limit=None, page=0, **kw) + Ticket.paged_query_or_search(app_cfg, user, mongo_query, solr_query, filter, + deleted=deleted_val, show_deleted=False) + # solr path: 'show_deleted' is forwarded, 'deleted' (mongo-only) is not + search.assert_called_once_with(app_cfg, user, solr_query, filter=filter, + sort=None, limit=None, page=0, show_deleted=False) assert query.call_count == 0 assert tsearch.query_filter_choices.call_count == 0 diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py index b7ab8a64a..a3482cf70 100644 --- a/ForgeTracker/forgetracker/tracker_main.py +++ b/ForgeTracker/forgetracker/tracker_main.py @@ -731,22 +731,21 @@ def tags(self, term=None, **kw): @expose('jinja:forgetracker:templates/tracker/index.html') @validate(dict(deleted=validators.StringBool(if_empty=False), filter=V.JsonConverter(if_empty={}))) - def index(self, limit=None, columns=None, page=0, sort='ticket_num desc', deleted=False, filter=None, **kw): + def index(self, q=None, limit=None, columns=None, page=0, sort='ticket_num desc', + deleted=False, filter=None): show_deleted = [False] if deleted and has_access(c.app, 'delete'): show_deleted = [False, True] elif deleted and not has_access(c.app, 'delete'): deleted = False - # it's just our original query mangled and sent back to us - kw.pop('q', None) result = TM.Ticket.paged_query_or_search(c.app.config, c.user, c.app.globals.not_closed_mongo_query, c.app.globals.not_closed_query, filter, sort=sort, limit=limit, page=page, deleted={'$in': show_deleted}, - show_deleted=deleted, **kw) + show_deleted=deleted) result['columns'] = columns or mongo_columns() result[ @@ -838,7 +837,7 @@ def update_milestones(self, field_name=None, milestones=None, **kw): @expose('jinja:forgetracker:templates/tracker/search.html') @validate(validators=search_validators) def search(self, q=None, query=None, project=None, columns=None, page=0, sort=None, - deleted=False, filter=None, **kw): + deleted=False, filter=None, history=None, limit=None): require_access(c.app, 'read') if deleted and not has_access(c.app, 'delete'): @@ -851,9 +850,9 @@ def search(self, q=None, query=None, project=None, columns=None, page=0, sort=No bin = TM.Bin.query.find( dict(app_config_id=c.app.config._id, terms=q)).first() if project: - redirect(c.project.url() + 'search?' + urlencode(dict(q=q, history=kw.get('history')))) + redirect(c.project.url() + 'search?' + urlencode(dict(q=q, history=history))) result = TM.Ticket.paged_search(c.app.config, c.user, q, page=page, sort=sort, - show_deleted=deleted, filter=filter, **kw) + show_deleted=deleted, filter=filter, limit=limit) result['columns'] = columns or solr_columns() result[ 'sortable_custom_fields'] = c.app.globals.sortable_custom_fields_shown_in_search() @@ -870,11 +869,13 @@ def search(self, q=None, query=None, project=None, columns=None, page=0, sort=No @h.vardec @expose() @validate(validators=search_validators) - def search_feed(self, q=None, query=None, project=None, page=0, sort=None, deleted=False, **kw): + def search_feed(self, q=None, query=None, project=None, page=0, sort=None, + deleted=False, limit=None, filter=None): if query and not q: q = query result = TM.Ticket.paged_search( - c.app.config, c.user, q, page=page, sort=sort, show_deleted=deleted, **kw) + c.app.config, c.user, q, page=page, sort=sort, show_deleted=deleted, + limit=limit, filter=filter) response.headers['Content-Type'] = '' response.content_type = 'application/xml' d = dict(title='Ticket search results', link=h.absurl(c.app.url), @@ -971,11 +972,11 @@ def save_ticket(self, ticket_form=None, **post_data): limit=validators.Int(if_empty=10, if_invalid=10), page=validators.Int(if_empty=0, if_invalid=0), sort=v.UnicodeString(if_empty='ticket_num_i asc'))) - def edit(self, q=None, limit=None, page=None, sort=None, filter=None, **kw): + def edit(self, q=None, limit=None, page=None, sort=None, filter=None): require_access(c.app, 'update') result = TM.Ticket.paged_search(c.app.config, c.user, q, filter=filter, sort=sort, limit=limit, page=page, - show_deleted=False, **kw) + show_deleted=False) # if c.app.globals.milestone_names is None: # c.app.globals.milestone_names = '' @@ -999,11 +1000,11 @@ def edit(self, q=None, limit=None, page=None, sort=None, filter=None, **kw): limit=validators.Int(if_empty=10, if_invalid=10), page=validators.Int(if_empty=0, if_invalid=0), sort=v.UnicodeString(if_empty='ticket_num_i asc'))) - def move(self, q=None, limit=None, page=None, sort=None, filter=None, **kw): + def move(self, q=None, limit=None, page=None, sort=None, filter=None): require_access(c.app, 'admin') result = TM.Ticket.paged_search(c.app.config, c.user, q, filter=filter, sort=sort, limit=limit, page=page, - show_deleted=False, **kw) + show_deleted=False) result['columns'] = solr_columns() result[ @@ -1965,7 +1966,7 @@ def __init__(self, root, field, milestone): filter=V.JsonConverter(if_empty={}), deleted=validators.StringBool(if_empty=False))) def index(self, q=None, columns=None, page=0, query=None, sort=None, - deleted=False, filter=None, **kw): + deleted=False, filter=None, limit=None): require_access(c.app, 'read') show_deleted = [False] if deleted and has_access(c.app, 'delete'): @@ -1976,9 +1977,9 @@ def index(self, q=None, columns=None, page=0, query=None, sort=None, result = TM.Ticket.paged_query_or_search(c.app.config, c.user, self.mongo_query, self.solr_query, - filter, sort=sort, page=page, + filter, sort=sort, page=page, limit=limit, deleted={'$in': show_deleted}, - show_deleted=deleted, **kw) + show_deleted=deleted) result['columns'] = columns or mongo_columns() result[
