Repository: dmwm/wmcore
Issue: RequestPriority validation only works during the workflow creation

**Impact of the bug**
ReqMgr2

**Describe the bug**
`RequestPriority` spec parameter is supposed to be integer, according to its specification here:
https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/WMSpec/StdSpecs/StdBase.py#L1033-L1034

however, I just spotted a few requests in the system with real values, example:
https://cmsweb.cern.ch/reqmgr2/fetch?rid=cmsunified_ACDC2_task_TOP-RunIISummer19UL17wmLHEGEN-00071__v1_T_201010_132628_5975
```
    244840.32: 2020-10-10 11:26:28 UTC DN: /DC=ch/DC=cern/OU=computers/CN=unified/vocms0268.cern.ch
```

**How to reproduce it**
I guess one can try even from the Gui for a workflow in active status.

**Expected behavior**
Such values should not be allowed and the HTTP request - to update the workflow priority - should fail. In other words, the spec parameter validation function needs to be properly applied, including the data type.

**Additional context and error message**
I'm still not sure whether this can cause other problems, perhaps at job submission or something like that...

Changed files:
src/python/WMCore/ReqMgr/Utils/Validation.py, src/python/WMCore/ReqMgr/Service/Request.py, test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py

diff --git a/src/python/WMCore/ReqMgr/Service/Request.py b/src/python/WMCore/ReqMgr/Service/Request.py
index 93877676a..acd350d67 100644
--- a/src/python/WMCore/ReqMgr/Service/Request.py
+++ b/src/python/WMCore/ReqMgr/Service/Request.py
@@ -22,7 +22,8 @@ from WMCore.ReqMgr.DataStructs.RequestStatus import (REQUEST_STATE_LIST,
                                                      REQUEST_STATE_TRANSITION, ACTIVE_STATUS)
 from WMCore.ReqMgr.DataStructs.RequestType import REQUEST_TYPES
 from WMCore.ReqMgr.Utils.Validation import (validate_request_create_args, validate_request_update_args,
-                                            validate_clone_create_args, validateOutputDatasets, workqueue_stat_validation)
+                                            validate_clone_create_args, validateOutputDatasets,
+                                            validate_request_priority, workqueue_stat_validation)
 from WMCore.Services.RequestDB.RequestDBWriter import RequestDBWriter
 from WMCore.Services.WorkQueue.WorkQueue import WorkQueue
 
@@ -409,14 +410,16 @@ class Request(RESTEntity):
         if 'RequestPriority' in request_args:
             # Yes, we completely ignore any other arguments posted by the user (web UI case)
             request_args = {'RequestPriority': request_args['RequestPriority']}
+            validate_request_priority(request_args)
             # must update three places: GQ elements, workload_cache and workload spec
             self.gq_service.updatePriority(workload.name(), request_args['RequestPriority'])
             report = self.reqmgr_db_service.updateRequestProperty(workload.name(), request_args, dn)
             workload.setPriority(request_args['RequestPriority'])
             workload.saveCouchUrl(workload.specUrl())
-            cherrypy.log('Updated priority of "%s" to %s' % (workload.name(), request_args['RequestPriority']))
+            cherrypy.log('Updated priority of "{}" to: {}'.format(workload.name(), request_args['RequestPriority']))
         elif workqueue_stat_validation(request_args):
             report = self.reqmgr_db_service.updateRequestStats(workload.name(), request_args)
+            cherrypy.log('Updated workqueue statistics of "{}", with:  {}'.format(workload.name(), request_args))
         else:
             msg = "There are invalid arguments for no-status update: %s" % request_args
             raise InvalidSpecParameterValue(msg)
@@ -431,6 +434,7 @@ class Request(RESTEntity):
             msg = "There are invalid arguments for assignment-approved transition: %s" % request_args
             raise InvalidSpecParameterValue(msg)
 
+        validate_request_priority(request_args)
         report = self.reqmgr_db_service.updateRequestProperty(workload.name(), request_args, dn)
         return report
 
diff --git a/src/python/WMCore/ReqMgr/Utils/Validation.py b/src/python/WMCore/ReqMgr/Utils/Validation.py
index 75873ec3a..641a21652 100644
--- a/src/python/WMCore/ReqMgr/Utils/Validation.py
+++ b/src/python/WMCore/ReqMgr/Utils/Validation.py
@@ -13,6 +13,7 @@ from WMCore.ReqMgr.DataStructs.RequestStatus import check_allowed_transition, ST
 from WMCore.ReqMgr.Tools.cms import releases, architectures, dashboardActivities
 from WMCore.Services.DBS.DBS3Reader import getDataTiers
 from WMCore.WMFactory import WMFactory
+from WMCore.WMSpec.StdSpecs.StdBase import StdBase
 from WMCore.WMSpec.WMWorkload import WMWorkloadHelper
 from WMCore.WMSpec.WMWorkloadTools import loadSpecClassByType, setArgumentsWithDefault
 from WMCore.Cache.GenericDataCache import GenericDataCache, MemoryCacheStruct
@@ -63,15 +64,27 @@ def validate_request_update_args(request_args, config, reqmgr_db_service, param)
         elif request_args["RequestStatus"] == 'assigned':
             workload.validateArgumentForAssignment(request_args)
 
-    # TODO: fetch it from the assignment arg definition
-    if 'RequestPriority' in request_args:
-        request_args['RequestPriority'] = int(request_args['RequestPriority'])
-        if (lambda x: (x >= 0 and x < 1e6))(request_args['RequestPriority']) is False:
-            raise InvalidSpecParameterValue("RequestPriority must be an integer between 0 and 1e6")
+    validate_request_priority(request_args)
 
     return workload, request_args
 
 
+def validate_request_priority(reqArgs):
+    """
+    Validate the RequestPriority argument against its definition
+    in StdBase
+    :param reqArgs: dictionary of user request arguments
+    :return: nothing, but raises an exception in case of an invalid value
+    """
+    if 'RequestPriority' in reqArgs:
+        reqPrioDefin = StdBase.getWorkloadCreateArgs()['RequestPriority']
+        if not isinstance(reqArgs['RequestPriority'], reqPrioDefin['type']):
+            msg = "RequestPriority must be of integer type, not: {}".format(type(reqArgs['RequestPriority']))
+            raise InvalidSpecParameterValue(msg)
+        if reqPrioDefin['validate'](reqArgs['RequestPriority']) is False:
+            raise InvalidSpecParameterValue("RequestPriority must be an integer between 0 and 999999")
+
+
 def validate_request_create_args(request_args, config, reqmgr_db_service, *args, **kwargs):
     """
     *arg and **kwargs are only for the interface
diff --git a/test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py b/test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py
index 22723f270..99ec8e549 100644
--- a/test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py
+++ b/test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py
@@ -7,8 +7,8 @@ from __future__ import division, print_function
 
 import unittest
 
-from WMCore.ReqMgr.Utils.Validation import validateOutputDatasets
 from WMCore.ReqMgr.DataStructs.RequestError import InvalidSpecParameterValue
+from WMCore.ReqMgr.Utils.Validation import validateOutputDatasets, validate_request_priority
 
 
 class ValidationTests(unittest.TestCase):
@@ -48,6 +48,30 @@ class ValidationTests(unittest.TestCase):
         with self.assertRaises(InvalidSpecParameterValue):
             validateOutputDatasets(outputDsets, dbsUrl)
 
+    def testRequestPriorityValidation(self):
+        """
+        Test the `validate_request_priority` function, which validates the
+        RequestPriority parameter
+        :return: nothing, raises an exception if there are problems
+        """
+        # test valid cases, integer in the range of [0, 1e6]
+        for goodPrio in [0, 100, int(1e6 - 1)]:
+            reqArgs = {'RequestPriority': goodPrio}
+            print(reqArgs)
+            validate_request_priority(reqArgs)
+
+        # test invalid ranges
+        for badPrio in [-10, 1e6, 1e7]:
+            reqArgs = {'RequestPriority': badPrio}
+            with self.assertRaises(InvalidSpecParameterValue):
+                validate_request_priority(reqArgs)
+
+        # test invalid data types
+        for badPrio in ["1234", 1234.35, 1e6, [123]]:
+            reqArgs = {'RequestPriority': badPrio}
+            with self.assertRaises(InvalidSpecParameterValue):
+                validate_request_priority(reqArgs)
+
 
 if __name__ == '__main__':
     unittest.main()