Migrate from get_request_list() to get_request_collection()

The new function uses a new, fast API call.

osc/commandline.py

@@ -1673,7 +1673,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                       % (devloc, dst_package))
                 sys.exit(1)
 
-        reqs = get_request_list(apiurl, dst_project, dst_package, req_type='submit', req_state=['new', 'review'])
+        reqs = get_request_collection(apiurl, project=dst_project, package=dst_package, types=['submit'], states=['new', 'review'])
         user = conf.get_apiurl_usr(apiurl)
         myreqs = [i for i in reqs if i.state.who == user and i.reqid != opts.supersede]
         myreq_ids = [r.reqid for r in myreqs]
@@ -2409,7 +2409,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
             who = ''
             if cmd == 'approvenew':
                 states = ('new')
-                results = get_request_list(apiurl, project, package, '', ['new'])
+                results = get_request_collection(apiurl, project=project, package=package, states=['new'])
             else:
                 state_list = opts.state.split(',')
                 if state_list == ['']:
@@ -2439,8 +2439,9 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                         results = get_user_projpkgs_request_list(apiurl, who, req_state=state_list,
                                                                  req_type=opts.type, exclude_projects=opts.exclude_target_project or [])
                     else:
-                        results = get_request_list(apiurl, project, package, who,
+                        results = get_request_collection(
-                                                   state_list, opts.type, opts.exclude_target_project or [])
+                            apiurl, project=project, package=package, user=who,
+                            states=state_list, types=opts.type, roles=roles)
 
             # Check if project actually exists if result list is empty
             if not results:
@@ -3717,7 +3718,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                 raise oscerr.WrongArgs('Package argument is empty')
 
             # FIXME: core.py:commitDelPackage() should have something similar
-            rlist = get_request_list(apiurl, prj, pkg)
+            rlist = get_request_collection(apiurl, project=prj, package=pkg)
             for rq in rlist:
                 print(rq)
             if len(rlist) >= 1 and not opts.force:
@@ -4275,8 +4276,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
         new_packages = meta_get_packagelist(apiurl, newprj)
 
         if opts.requests:
-            requests = get_request_list(apiurl, project=oldprj,
+            requests = get_request_collection(apiurl, project=oldprj, states=('new', 'review'))
-                                        req_state=('new', 'review'))
 
         for pkg in old_packages:
             if self._prdiff_skip_package(opts, pkg):
@@ -4299,8 +4299,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
                 self._prdiff_output_diff(opts, rdiff)
 
                 if opts.requests:
-                    self._prdiff_output_matching_requests(opts, requests,
+                    self._prdiff_output_matching_requests(opts, requests, newprj, pkg)
-                                                          newprj, pkg)
             else:
                 print("identical: %s" % pkg)
 
@@ -7402,7 +7401,7 @@ Please submit there instead, or use --nodevelproject to force direct submission.
         elif type in args_prj:
             what = {'project': ''}
         elif type in args_sr:
-            requests = get_request_collection(apiurl, 'creator', req_who=user)
+            requests = get_request_collection(apiurl, roles=['creator'], user=user)
             for r in sorted(requests, key=lambda x: x.reqid):
                 print(r.list_view(), '\n')
             return

osc/core.py

@@ -4430,18 +4430,53 @@ def get_review_list(apiurl, project='', package='', byuser='', bygroup='', bypro
         requests.append(r)
     return requests
 
+
 # this function uses the logic in the api which is faster and more exact then the xpath search
+def get_request_collection(
+        apiurl,
+        user=None, group=None, roles=None,
+        project=None, package=None,
+        states=None, review_states=None,
+        types=None, ids=None):
 
-
+    # We don't want to overload server by requesting everything.
-def get_request_collection(apiurl, role=None, req_who=None, req_states=('new', 'review')):
+    # Let's enforce specifying at least some search criteria.
+    if not any([user, group, project, package, ids]):
+        raise ValueError("Please specify search criteria")
 
     query = {"view": "collection"}
-    if role:
-        query["roles"] = role
-    if req_who:
-        query["user"] = req_who
 
-    query["states"] = ",".join(req_states)
+    if user:
+        query["user"] = user
+
+    if group:
+        query["group"] = group
+
+    if roles:
+        query["roles"] = ",".join(roles)
+
+    if project:
+        query["project"] = project
+
+    if package:
+        if not project:
+            raise ValueError("Project must be set to query a package; see https://github.com/openSUSE/open-build-service/issues/13075")
+        query["package"] = package
+
+    states = states or ("new", "review")
+    if states:
+        if "all" not in states:
+            query["states"] = ",".join(states)
+
+    if review_states:
+        if "all" not in review_states:
+            query["review_states"] = ",".join(review_states)
+
+    if types:
+        query["types"] = ",".join(types)
+
+    if ids:
+        query["ids"] = ",".join(ids)
 
     u = makeurl(apiurl, ['request'], query)
     f = http_GET(u)
@@ -4487,51 +4522,33 @@ def get_exact_request_list(apiurl, src_project, dst_project, src_package=None, d
     return requests
 
 
-def get_request_list(apiurl, project='', package='', req_who='', req_state=('new', 'review', 'declined'), req_type=None, exclude_target_projects=None,
+def get_request_list(apiurl, project='', package='', req_who='',
-                     withfullhistory=False):
+                     req_state=('new', 'review', 'declined'), req_type=None,
-    exclude_target_projects = exclude_target_projects or []
+                     exclude_target_projects=None, withfullhistory=False, roles=None):
-    xpath = ''
-    if 'all' not in req_state:
-        for state in req_state:
-            xpath = xpath_join(xpath, 'state/@name=\'%s\'' % state, inner=True)
-    if req_who:
-        xpath = xpath_join(xpath, '(state/@who=\'%(who)s\' or history/@who=\'%(who)s\')' % {'who': req_who}, op='and')
 
-    # XXX: we cannot use the '|' in the xpath expression because it is not supported
+    import warnings
-    #      in the backend
+    warnings.warn(
-    todo = {}
+        "osc.core.get_request_list() is deprecated. "
-    if project:
+        "Use osc.core.get_request_collection() instead.",
-        todo['project'] = project
+        DeprecationWarning
-    if package:
+    )
-        todo['package'] = package
-    for kind, val in todo.items():
-        xpath_base = 'action/target/@%(kind)s=\'%(val)s\''
-        if conf.config['include_request_from_project']:
-            xpath_base = xpath_join(xpath_base, 'action/source/@%(kind)s=\'%(val)s\'', op='or', inner=True)
-        xpath = xpath_join(xpath, xpath_base % {'kind': kind, 'val': val}, op='and', nexpr_parentheses=True)
 
-    if req_type:
+    kwargs = {
-        xpath = xpath_join(xpath, 'action/@type=\'%s\'' % req_type, op='and')
+        "apiurl": apiurl,
-    for i in exclude_target_projects:
+        "user": req_who,
-        xpath = xpath_join(xpath, '(not(action/target/@project=\'%(prj)s\'))' % {'prj': i}, op='and')
+        "roles": roles,
+        "project": project,
+        "package": package,
+        "states": req_state,
+    }
+
+    assert not exclude_target_projects, "unsupported"
+    assert not withfullhistory, "unsupported"
+
+    return get_request_collection(**kwargs)
 
-    if conf.config['debug']:
-        print('[ %s ]' % xpath)
-    queries = {}
-    if withfullhistory:
-        queries['request'] = {'withfullhistory': '1'}
-    res = search(apiurl, queries=queries, request=xpath)
-    collection = res['request']
-    requests = []
-    for root in collection.findall('request'):
-        r = Request()
-        r.read(root)
-        requests.append(r)
-    return requests
 
 # old style search, this is to be removed
-
-
 def get_user_projpkgs_request_list(apiurl, user, req_state=('new', 'review', ), req_type=None, exclude_projects=None, projpkgs=None):
     """OBSOLETE: user involved request search is supported by OBS 2.2 server side in a better way
        Return all running requests for all projects/packages where is user is involved"""
@@ -7541,7 +7558,7 @@ def print_request_list(apiurl, project, package=None, states=('new', 'review'),
     """
     if not conf.config['check_for_request_on_action'] and not force:
         return
-    requests = get_request_list(apiurl, project, package, req_state=states)
+    requests = get_request_collection(apiurl, project=project, package=package, states=states)
     msg = '\nPending requests for %s: %s (%s)'
     if sys.stdout.isatty():
         msg = f'\033[1m{msg}\033[0m'

tests/test_commit.py

@@ -33,7 +33,7 @@ class TestCommit(OscTestCase):
          exp='This file didn\'t change but\nis modified.\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/simple?comment=&cmd=commitfilelist&user=Admin',
           file='testSimple_cfilesremote', expfile='testSimple_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27simple%27+or+action%2Fsource%2F%40package%3D%27simple%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=simple&states=new%2Creview', file='testOpenRequests')
     def test_simple(self):
         """a simple commit (only one modified file)"""
         self._change_to_pkg('simple')
@@ -57,7 +57,7 @@ class TestCommit(OscTestCase):
          exp='added file\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/add?comment=&cmd=commitfilelist&user=Admin',
           file='testAddfile_cfilesremote', expfile='testAddfile_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27add%27+or+action%2Fsource%2F%40package%3D%27add%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=add&states=new%2Creview', file='testOpenRequests')
     def test_addfile(self):
         """commit a new file"""
         self._change_to_pkg('add')
@@ -79,7 +79,7 @@ class TestCommit(OscTestCase):
           exp='', text='<services />')
     @POST('http://localhost/source/osctest/delete?comment=&cmd=commitfilelist&user=Admin&withvalidate=1',
           file='testDeletefile_cfilesremote', expfile='testDeletefile_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27delete%27+or+action%2Fsource%2F%40package%3D%27delete%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=delete&states=new%2Creview', file='testOpenRequests')
     def test_deletefile(self):
         """delete a file"""
         self._change_to_pkg('delete')
@@ -132,7 +132,7 @@ class TestCommit(OscTestCase):
     @PUT('http://localhost/source/osctest/multiple/add2?rev=repository', exp='add2\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/multiple?comment=&cmd=commitfilelist&user=Admin',
           file='testMultiple_cfilesremote', expfile='testMultiple_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27multiple%27+or+action%2Fsource%2F%40package%3D%27multiple%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=multiple&states=new%2Creview', file='testOpenRequests')
     def test_multiple(self):
         """a simple commit (only one modified file)"""
         self._change_to_pkg('multiple')
@@ -161,7 +161,7 @@ class TestCommit(OscTestCase):
     @PUT('http://localhost/source/osctest/multiple/nochange?rev=repository', exp='This file did change.\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/multiple?comment=&cmd=commitfilelist&user=Admin',
           file='testPartial_cfilesremote', expfile='testPartial_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27multiple%27+or+action%2Fsource%2F%40package%3D%27multiple%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=multiple&states=new%2Creview', file='testOpenRequests')
     def test_partial(self):
         """commit only some files"""
         self._change_to_pkg('multiple')
@@ -208,7 +208,7 @@ class TestCommit(OscTestCase):
     @PUT('http://localhost/source/osctest/allstates/nochange?rev=repository', exp='This file did change.\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/allstates?comment=&cmd=commitfilelist&user=Admin',
           file='testAllStates_cfilesremote', expfile='testAllStates_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27allstates%27+or+action%2Fsource%2F%40package%3D%27allstates%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=allstates&states=new%2Creview', file='testOpenRequests')
     def test_allstates(self):
         """commit all files (all states are available except 'C')"""
         self._change_to_pkg('allstates')
@@ -234,7 +234,7 @@ class TestCommit(OscTestCase):
           exp='', text='<services />')
     @POST('http://localhost/source/osctest/add?comment=&cmd=commitfilelist&user=Admin&withvalidate=1',
           file='testAddfile_cfilesremote', expfile='testAddfile_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27add%27+or+action%2Fsource%2F%40package%3D%27add%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=add&states=new%2Creview', file='testOpenRequests')
     def test_remoteexists(self):
         """file 'add' should be committed but already exists on the server"""
         self._change_to_pkg('add')
@@ -260,7 +260,7 @@ class TestCommit(OscTestCase):
     @POST('http://localhost/source/osctest/branch?comment=&cmd=commitfilelist&user=Admin&keeplink=1',
           file='testExpand_cfilesremote', expfile='testExpand_lfilelist')
     @GET('http://localhost/source/osctest/branch?rev=87ea02aede261b0267aabaa97c756e7a', file='testExpand_expandedfilesremote')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27branch%27+or+action%2Fsource%2F%40package%3D%27branch%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=branch&states=new%2Creview', file='testOpenRequests')
     def test_expand(self):
         """commit an expanded package"""
         self._change_to_pkg('branch')
@@ -292,7 +292,7 @@ class TestCommit(OscTestCase):
     @PUT('http://localhost/source/osctest/added_missing/bar?rev=repository', exp='foobar\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/added_missing?comment=&cmd=commitfilelist&user=Admin',
           file='testAddedMissing_cfilesremote', expfile='testAddedMissing_lfilelist')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27added_missing%27+or+action%2Fsource%2F%40package%3D%27added_missing%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=added_missing&states=new%2Creview', file='testOpenRequests')
     def test_added_missing2(self):
         """commit an added file, another added file missing (but it's not part of the commit)"""
         self._change_to_pkg('added_missing')
@@ -334,7 +334,7 @@ class TestCommit(OscTestCase):
          exp='This file didn\'t change but\nis modified.\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/simple?comment=&cmd=commitfilelist&user=Admin',
           file='testSimple_cfilesremote', expfile='testSimple_lfilelistwithSHA')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27simple%27+or+action%2Fsource%2F%40package%3D%27simple%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=simple&states=new%2Creview', file='testOpenRequests')
     def test_simple_sha256(self):
         """a simple commit (only one modified file)"""
         self._change_to_pkg('simple')
@@ -359,7 +359,7 @@ class TestCommit(OscTestCase):
     @PUT('http://localhost/source/osctest/added_missing/bar?rev=repository', exp='foobar\n', text=rev_dummy)
     @POST('http://localhost/source/osctest/added_missing?comment=&cmd=commitfilelist&user=Admin',
           file='testAddedMissing_cfilesremote', expfile='testAddedMissing_lfilelistwithSHA')
-    @GET('http://localhost/search/request?match=%28state%2F%40name%3D%27new%27+or+state%2F%40name%3D%27review%27%29+and+%28action%2Ftarget%2F%40project%3D%27osctest%27+or+action%2Fsource%2F%40project%3D%27osctest%27%29+and+%28action%2Ftarget%2F%40package%3D%27added_missing%27+or+action%2Fsource%2F%40package%3D%27added_missing%27%29', file='testOpenRequests')
+    @GET('http://localhost/request?view=collection&project=osctest&package=added_missing&states=new%2Creview', file='testOpenRequests')
     def test_added_missing2_sha256(self):
         """commit an added file, another added file missing (but it's not part of the commit)"""
         self._change_to_pkg('added_missing')

tests/test_prdiff.py

@@ -21,8 +21,7 @@ def rdiff_url(pkg, oldprj, newprj):
 
 
 def request_url(prj):
-    return 'http://localhost/search/request?match=%%28state%%2F%%40name%%3D%%27new%%27+or+state%%2F%%40name%%3D%%27review%%27%%29+and+%%28action%%2Ftarget%%2F%%40project%%3D%%27%s%%27+or+action%%2Fsource%%2F%%40project%%3D%%27%s%%27%%29' % \
+    return "http://localhost/request" + f"?view=collection&project={prj}&states=new,review".replace(":", "%3A").replace(",", "%2C")
-        tuple([prj.replace(':', '%3A')] * 2)
 
 
 def GET_PROJECT_PACKAGES(*projects):