Properly validate RequestPriority parameter

src/python/WMCore/ReqMgr/Service/Request.py

@@ -22,7 +22,8 @@
                                                      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 @@ def _handleNoStatusUpdate(self, workload, request_args, dn):
         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 @@ def _handleAssignmentApprovedTransition(self, workload, request_args, dn):
             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
 

src/python/WMCore/ReqMgr/Utils/Validation.py

@@ -13,6 +13,7 @@
 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

test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py

@@ -7,8 +7,8 @@
 
 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 @@ def testValidateOutputDatasets(self):
         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()