Repository: rucio/rucio
Issue: bug in resolving site attribute to only one RSE

Motivation
----------
in the new request listing API from #2878 the use of sites does not work properly as the sitename gets resolved to only one RSE.


Modification
------------
resolve sitename to all RSEs with the site attribute



Changed files:
lib/rucio/web/rest/webpy/v1/request.py, lib/rucio/api/request.py, lib/rucio/tests/test_request.py, lib/rucio/core/request.py, lib/rucio/web/rest/flaskapi/v1/request.py

diff --git a/lib/rucio/api/request.py b/lib/rucio/api/request.py
index a7151706c..e5ad6f1c3 100644
--- a/lib/rucio/api/request.py
+++ b/lib/rucio/api/request.py
@@ -137,21 +137,21 @@ def get_request_by_did(scope, name, rse, issuer):
     return api_update_return_dict(req)
 
 
-def list_requests(src_rse, dst_rse, states, issuer):
+def list_requests(src_rses, dst_rses, states, issuer):
     """
     List all requests in a specific state from a source RSE to a destination RSE.
 
-    :param src_rse: source RSE.
-    :param dst_rse: destination RSE.
+    :param src_rses: source RSEs.
+    :param dst_rses: destination RSEs.
     :param states: list of request states.
     :param issuer: Issuing account as a string.
     """
-    src_rse_id = get_rse_id(rse=src_rse)
-    dst_rse_id = get_rse_id(rse=dst_rse)
+    src_rse_ids = [get_rse_id(rse=rse) for rse in src_rses]
+    dst_rse_ids = [get_rse_id(rse=rse) for rse in dst_rses]
 
-    kwargs = {'src_rse_id': src_rse_id, 'dst_rse_id': dst_rse_id, 'issuer': issuer}
+    kwargs = {'src_rse_id': src_rse_ids, 'dst_rse_id': dst_rse_ids, 'issuer': issuer}
     if not permission.has_permission(issuer=issuer, action='list_requests', kwargs=kwargs):
         raise exception.AccessDenied('%(issuer)s cannot list requests from RSE %(src_rse)s to RSE %(dst_rse)s' % locals())
 
-    for req in request.list_requests(src_rse_id, dst_rse_id, states):
+    for req in request.list_requests(src_rse_ids, dst_rse_ids, states):
         yield api_update_return_dict(req)
diff --git a/lib/rucio/core/request.py b/lib/rucio/core/request.py
index 523dac33c..b2c129631 100644
--- a/lib/rucio/core/request.py
+++ b/lib/rucio/core/request.py
@@ -1405,17 +1405,17 @@ def __get_source_rse(request_id, src_url, session=None):
 
 
 @stream_session
-def list_requests(src_rse_id, dst_rse_id, states=[RequestState.WAITING], session=None):
+def list_requests(src_rse_ids, dst_rse_ids, states=[RequestState.WAITING], session=None):
     """
     List all requests in a specific state from a source RSE to a destination RSE.
 
-    :param src_rse_id: source RSE id.
-    :param dst_rse_id: destination RSE id.
+    :param src_rse_ids: source RSE ids.
+    :param dst_rse_ids: destination RSE ids.
     :param states: list of request states.
     :param session: The database session in use.
     """
     query = session.query(models.Request).filter(models.Request.state.in_(states),
-                                                 models.Request.source_rse_id == src_rse_id,
-                                                 models.Request.dest_rse_id == dst_rse_id)
+                                                 models.Request.source_rse_id.in_(src_rse_ids),
+                                                 models.Request.dest_rse_id.in_(dst_rse_ids))
     for request in query.yield_per(500):
         yield request
diff --git a/lib/rucio/tests/test_request.py b/lib/rucio/tests/test_request.py
index 0321799b2..6c62db34b 100644
--- a/lib/rucio/tests/test_request.py
+++ b/lib/rucio/tests/test_request.py
@@ -835,10 +835,13 @@ class TestRequestCore(object):
         models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id, dest_rse_id=self.dest_rse_id).save(session=self.db_session)
         models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id, dest_rse_id=self.dest_rse_id).save(session=self.db_session)
 
-        requests = [request for request in list_requests(self.source_rse_id, self.dest_rse_id, [constants.RequestState.SUBMITTED], session=self.db_session)]
+        requests = [request for request in list_requests([self.source_rse_id], [self.dest_rse_id], [constants.RequestState.SUBMITTED], session=self.db_session)]
         assert_equal(len(requests), 2)
 
-        requests = [request for request in list_requests(self.source_rse_id, self.dest_rse_id, [constants.RequestState.QUEUED], session=self.db_session)]
+        requests = [request for request in list_requests([self.source_rse_id, self.source_rse_id2], [self.dest_rse_id], [constants.RequestState.SUBMITTED], session=self.db_session)]
+        assert_equal(len(requests), 3)
+
+        requests = [request for request in list_requests([self.source_rse_id], [self.dest_rse_id], [constants.RequestState.QUEUED], session=self.db_session)]
         assert_equal(len(requests), 0)
 
 
@@ -854,6 +857,8 @@ class TestRequestREST():
         cls.source_rse_id = get_rse_id(cls.source_rse)
         cls.source_rse2 = 'MOCK2'
         cls.source_rse_id2 = get_rse_id(cls.source_rse2)
+        cls.source_rse3 = 'MOCK5'
+        cls.source_rse_id3 = get_rse_id(cls.source_rse3)
         cls.dest_rse = 'MOCK3'
         cls.dest_rse_id = get_rse_id(cls.dest_rse)
         cls.dest_rse2 = 'MOCK4'
@@ -865,6 +870,7 @@ class TestRequestREST():
         cls.dst_site2 = 'SITE4'
         add_rse_attribute(cls.source_rse_id, 'site', cls.source_site)
         add_rse_attribute(cls.source_rse_id2, 'site', cls.source_site2)
+        add_rse_attribute(cls.source_rse_id3, 'site', cls.source_site)
         add_rse_attribute(cls.dest_rse_id, 'site', cls.dst_site)
         add_rse_attribute(cls.dest_rse_id2, 'site', cls.dst_site2)
 
@@ -901,6 +907,7 @@ class TestRequestREST():
         models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id, dest_rse_id=self.dest_rse_id2, name=name1).save(session=self.db_session)
         models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id, dest_rse_id=self.dest_rse_id, name=name1).save(session=self.db_session)
         models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id, dest_rse_id=self.dest_rse_id, name=name2).save(session=self.db_session)
+        models.Request(state=constants.RequestState.SUBMITTED, source_rse_id=self.source_rse_id3, dest_rse_id=self.dest_rse_id, name=name2).save(session=self.db_session)
         self.db_session.commit()
 
         params = {'src_rse': self.source_rse, 'dst_rse': self.dest_rse, 'request_states': 'S'}
@@ -927,6 +934,8 @@ class TestRequestREST():
         expected_requests = set()
         expected_requests.add(('SUBMITTED', self.source_rse_id, self.dest_rse_id, name1))
         expected_requests.add(('SUBMITTED', self.source_rse_id, self.dest_rse_id, name2))
+        # check correct resolution of site attribute to multiple RSE
+        expected_requests.add(('SUBMITTED', self.source_rse_id3, self.dest_rse_id, name2))
         self.check_correct_api(params, expected_requests)
 
         params = {'src_site': self.source_site, 'dst_site': self.dst_site, 'request_states': 'S,W,Q'}
@@ -934,6 +943,7 @@ class TestRequestREST():
         expected_requests.add(('SUBMITTED', self.source_rse_id, self.dest_rse_id, name1))
         expected_requests.add(('SUBMITTED', self.source_rse_id, self.dest_rse_id, name2))
         expected_requests.add(('WAITING', self.source_rse_id, self.dest_rse_id, name3))
+        expected_requests.add(('SUBMITTED', self.source_rse_id3, self.dest_rse_id, name2))
         self.check_correct_api(params, expected_requests)
 
         params = {'src_site': self.source_site2, 'dst_site': self.dst_site, 'request_states': 'S'}
diff --git a/lib/rucio/web/rest/flaskapi/v1/request.py b/lib/rucio/web/rest/flaskapi/v1/request.py
index d39f51b91..54c4479de 100755
--- a/lib/rucio/web/rest/flaskapi/v1/request.py
+++ b/lib/rucio/web/rest/flaskapi/v1/request.py
@@ -111,18 +111,23 @@ class RequestsGet(MethodView):
         except ValueError:
             return generate_http_error_flask(400, 'Invalid', 'Request state value is invalid')
 
+        src_rses = []
+        dst_rses = []
         if src_site:
-            src_rse = get_rses_with_attribute_value(key='site', value=src_site, lookup_key='site')
-            if not src_rse:
+            src_rses = get_rses_with_attribute_value(key='site', value=src_site, lookup_key='site')
+            if not src_rses:
                 return generate_http_error_flask(404, 'NotFound', 'Could not resolve site name %s to RSE' % src_site)
-            src_rse = get_rse_name(src_rse[0]['rse_id'])
-            dst_rse = get_rses_with_attribute_value(key='site', value=dst_site, lookup_key='site')
-            if not dst_rse:
+            src_rses = [get_rse_name(rse['rse_id']) for rse in src_rses]
+            dst_rses = get_rses_with_attribute_value(key='site', value=dst_site, lookup_key='site')
+            if not dst_rses:
                 return generate_http_error_flask(404, 'NotFound', 'Could not resolve site name %s to RSE' % dst_site)
-            dst_rse = get_rse_name(dst_rse[0]['rse_id'])
+            dst_rses = [get_rse_name(rse['rse_id']) for rse in dst_rses]
+        else:
+            dst_rses = [dst_rse]
+            src_rses = [src_rse]
 
         results = []
-        for result in request.list_requests(src_rse, dst_rse, states, issuer=f_request.environ.get('issuer')):
+        for result in request.list_requests(src_rses, dst_rses, states, issuer=f_request.environ.get('issuer')):
             result = result.to_dict()
             del result['_sa_instance_state']
             results.append(result)
diff --git a/lib/rucio/web/rest/webpy/v1/request.py b/lib/rucio/web/rest/webpy/v1/request.py
index 8c6c068b2..2fcca4877 100755
--- a/lib/rucio/web/rest/webpy/v1/request.py
+++ b/lib/rucio/web/rest/webpy/v1/request.py
@@ -119,17 +119,22 @@ class RequestsGet(RucioController):
         except ValueError:
             raise generate_http_error(400, 'Invalid', 'Request state value is invalid')
 
+        src_rses = []
+        dst_rses = []
         if src_site:
-            src_rse = get_rses_with_attribute_value(key='site', value=src_site, lookup_key='site')
-            if not src_rse:
+            src_rses = get_rses_with_attribute_value(key='site', value=src_site, lookup_key='site')
+            if not src_rses:
                 raise generate_http_error(404, 'NotFound', 'Could not resolve site name %s to RSE' % src_site)
-            src_rse = get_rse_name(src_rse[0]['rse_id'])
-            dst_rse = get_rses_with_attribute_value(key='site', value=dst_site, lookup_key='site')
-            if not dst_rse:
+            src_rses = [get_rse_name(rse['rse_id']) for rse in src_rses]
+            dst_rses = get_rses_with_attribute_value(key='site', value=dst_site, lookup_key='site')
+            if not dst_rses:
                 raise generate_http_error(404, 'NotFound', 'Could not resolve site name %s to RSE' % dst_site)
-            dst_rse = get_rse_name(dst_rse[0]['rse_id'])
+            dst_rses = [get_rse_name(rse['rse_id']) for rse in dst_rses]
+        else:
+            dst_rses = [dst_rse]
+            src_rses = [src_rse]
 
-        for result in request.list_requests(src_rse, dst_rse, states, issuer=ctx.env.get('issuer')):
+        for result in request.list_requests(src_rses, dst_rses, states, issuer=ctx.env.get('issuer')):
             result = result.to_dict()
             del result['_sa_instance_state']
             yield render_json(**result) + '\\n'