Fixed cognitive complexity issues and added some more RESQL test cases for roles module.

web/pgadmin/browser/server_groups/servers/roles/__init__.py

@@ -111,35 +111,75 @@ class RoleView(PGChildNodeView):
         'variables': [{'get': 'variables'}],
     })
 
-    @staticmethod
+    def _validate_input_dict_for_new(self, data, req_keys):
-    def _get_request_data():
         """
-        Get data from client request.
+        This functions validates the input dict and check for required
+        keys in the dict when creating a new object
+        :param data: input dict
+        :param req_keys: required keys
+        :return: Valid or Invalid
         """
-        if request.data:
+        if type(data) != list:
-            data = json.loads(request.data, encoding='utf-8')
+            return False
-        else:
-            data = dict()
-            req = request.args or request.form
 
-            for key in req:
+        for item in data:
+            if type(item) != dict:
+                return False
 
-                val = req[key]
+            for a_key in req_keys:
-                if key in [
+                if a_key not in item:
-                    u'rolcanlogin', u'rolsuper', u'rolcreatedb',
+                    return False
-                    u'rolcreaterole', u'rolinherit', u'rolreplication',
-                    u'rolcatupdate', u'variables', u'rolmembership',
-                    u'seclabels'
-                ]:
-                    data[key] = json.loads(val, encoding='utf-8')
-                else:
-                    data[key] = val
-        return data
 
-    @staticmethod
+        return True
-    def _check_roleconnlimit(data):
+
+    def _validate_input_dict_for_update(
+            self, data, req_add_keys, req_delete_keys):
         """
-        Check connection limit for role.
+        This functions validates the input dict and check for required
+        keys in the dict when updating an existing object
+        :param data: input dict
+        :param req_add_keys: required keys when adding, updating
+        :param req_delete_keys: required keys when deleting
+        :return: Valid or Invalid
+        """
+        if type(data) != dict:
+            return False
+
+        for op in [u'added', u'deleted', u'changed']:
+            op_data = data.get(op, [])
+            check_keys = req_add_keys \
+                if op in [u'added', u'changed'] else req_delete_keys
+            if not self._validate_input_dict_for_new(op_data, check_keys):
+                return False
+
+        return True
+
+    def _validate_rolvaliduntil(self, data):
+        """
+        Validate the rolvaliduntil in input data dict
+        :param data: role data
+        :return: valid or invalid message
+        """
+        if u'rolvaliduntil' in data:
+            # Make date explicit so that it works with every
+            # postgres database datestyle format
+            try:
+                if data[u'rolvaliduntil'] is not None and \
+                    data[u'rolvaliduntil'] != '' and \
+                        len(data[u'rolvaliduntil']) > 0:
+                    data[u'rolvaliduntil'] = dateutil_parser.parse(
+                        data[u'rolvaliduntil']
+                    ).isoformat()
+            except Exception:
+                return _("Date format is invalid.")
+
+        return None
+
+    def _validate_rolconnlimit(self, data):
+        """
+        Validate the rolconnlimit data dict
+        :param data: role data
+        :return: valid or invalid message
         """
         if u'rolconnlimit' in data:
             # If roleconnlimit is empty string then set it to -1
@@ -150,165 +190,62 @@ class RoleView(PGChildNodeView):
                 data[u'rolconnlimit'] = int(data[u'rolconnlimit'])
                 if type(data[u'rolconnlimit']) != int or \
                         data[u'rolconnlimit'] < -1:
-                    return True, "Connection limit must be an integer value " \
+                    return _("Connection limit must be an integer value "
-                                 "or equal to -1."
+                             "or equal to -1.")
+        return None
 
-        return False, ''
+    def _process_rolemembership(self, id, data):
-
-    @staticmethod
-    def _check_role(data):
         """
-        Check user role
+        Process the input rolemembership list to appropriate keys
+        :param id: id of role
+        :param data: input role data
         """
-        msg = _("""
+        def _part_dict_list(dict_list, condition, list_key=None):
-        Role membership information must be passed as an array of JSON objects
+            ret_val = []
-        in the following format:
+            for d in dict_list:
+                if condition(d):
+                    ret_val.append(d[list_key])
 
-        rolmembership:[{
+            return ret_val
-            role: [rolename],
-            admin: True/False
-            },
-            ...
-        ]""")
-        if type(data[u'rolmembership']) != list:
-            return True, msg
 
-        data[u'members'] = []
+        if id == -1:
-        data[u'admins'] = []
+            data[u'members'] = []
+            data[u'admins'] = []
 
-        for r in data[u'rolmembership']:
+            data[u'admins'] = _part_dict_list(
-            if type(r) != dict or u'role' not in r or \
+                data[u'rolmembership'], lambda d: d[u'admin'], u'role')
-                    u'admin' not in r:
+            data[u'members'] = _part_dict_list(
-                return True, msg
+                data[u'rolmembership'], lambda d: not d[u'admin'], u'role')
-            else:
+        else:
-                if r[u'admin']:
+            data[u'admins'] = _part_dict_list(
-                    data[u'admins'].append(r[u'role'])
+                data[u'rolmembership'].get(u'added', []),
-                else:
+                lambda d: d[u'admin'], u'role')
-                    data[u'members'].append(r[u'role'])
+            data[u'members'] = _part_dict_list(
-        return False, ''
+                data[u'rolmembership'].get(u'added', []),
+                lambda d: not d[u'admin'], u'role')
 
-    @staticmethod
+            data[u'admins'].extend(_part_dict_list(
-    def _check_precondition_added(data):
+                data[u'rolmembership'].get(u'changed', []),
+                lambda d: d[u'admin'], u'role'))
+            data[u'revoked_admins'] = _part_dict_list(
+                data[u'rolmembership'].get(u'changed', []),
+                lambda d: not d[u'admin'], u'role')
+
+            data[u'revoked'] = _part_dict_list(
+                data[u'rolmembership'].get(u'deleted', []),
+                lambda _: True, u'role')
+
+    def _validate_rolemembership(self, id, data):
         """
-        Check for pre condition for added
+        Validate the rolmembership data dict
+        :param data: role data
+        :return: valid or invalid message
         """
-        if u'added' in data[u'rolmembership']:
+        if u'rolmembership' not in data:
-            roles = (data[u'rolmembership'])[u'added']
+            return None
 
-            if type(roles) != list:
+        if id == -1:
-                return True
+            msg = _("""
-
-            for r in roles:
-                if type(r) != dict or \
-                    u'role' not in r or \
-                        u'admin' not in r:
-                    return True
-
-                if r[u'admin']:
-                    data[u'admins'].append(r[u'role'])
-                else:
-                    data[u'members'].append(r[u'role'])
-        return False
-
-    @staticmethod
-    def _check_precondition_deleted(data):
-        if u'deleted' in data[u'rolmembership']:
-            roles = (data[u'rolmembership'])[u'deleted']
-
-            if type(roles) != list:
-                return True
-
-            for r in roles:
-                if type(r) != dict or u'role' not in r:
-                    return True
-
-                data[u'revoked'].append(r[u'role'])
-
-        return False
-
-    @staticmethod
-    def _check_precondition_change(data):
-        if u'changed' in data[u'rolmembership']:
-            roles = (data[u'rolmembership'])[u'changed']
-
-            if type(roles) != list:
-                return True
-
-            for r in roles:
-                if type(r) != dict or \
-                    u'role' not in r or \
-                        u'admin' not in r:
-                    return True
-
-                if not r[u'admin']:
-                    data[u'revoked_admins'].append(r[u'role'])
-                else:
-                    data[u'admins'].append(r[u'role'])
-
-        return False
-
-    def validate_request(f):
-        @wraps(f)
-        def wrap(self, **kwargs):
-
-            data = None
-            if request.data:
-                data = json.loads(request.data, encoding='utf-8')
-            else:
-                data = dict()
-                req = request.args or request.form
-
-                for key in req:
-
-                    val = req[key]
-                    if key in [
-                        u'rolcanlogin', u'rolsuper', u'rolcreatedb',
-                        u'rolcreaterole', u'rolinherit', u'rolreplication',
-                        u'rolcatupdate', u'variables', u'rolmembership',
-                        u'seclabels'
-                    ]:
-                        data[key] = json.loads(val, encoding='utf-8')
-                    else:
-                        data[key] = val
-
-            if (u'rid' not in kwargs or kwargs['rid'] == -1) and \
-                    u'rolname' not in data:
-                return precondition_required(
-                    _("Name must be specified.")
-                )
-
-            if u'rolvaliduntil' in data:
-                # Make date explicit so that it works with every
-                # postgres database datestyle format
-                try:
-                    if data[u'rolvaliduntil'] is not None and \
-                            data[u'rolvaliduntil'] != '' and \
-                            len(data[u'rolvaliduntil']) > 0:
-                        data[u'rolvaliduntil'] = dateutil_parser.parse(
-                            data[u'rolvaliduntil']
-                        ).isoformat()
-                except Exception:
-                    return precondition_required(
-                        _("Date format is invalid.")
-                    )
-
-            if u'rolconnlimit' in data:
-                # If roleconnlimit is empty string then set it to -1
-                if data[u'rolconnlimit'] == '':
-                    data[u'rolconnlimit'] = -1
-
-                if data[u'rolconnlimit'] is not None:
-                    data[u'rolconnlimit'] = int(data[u'rolconnlimit'])
-                    if type(data[u'rolconnlimit']) != int or \
-                            data[u'rolconnlimit'] < -1:
-                        return precondition_required(
-                            _("Connection limit must be an integer value "
-                              "or equal to -1.")
-                        )
-
-            if u'rolmembership' in data:
-                if u'rid' not in kwargs or kwargs['rid'] == -1:
-                    msg = _("""
 Role membership information must be passed as an array of JSON objects in the
 following format:
 
@@ -318,23 +255,15 @@ rolmembership:[{
     },
     ...
 ]""")
-                    if type(data[u'rolmembership']) != list:
-                        return precondition_required(msg)
 
-                    data[u'members'] = []
+            if not self._validate_input_dict_for_new(
-                    data[u'admins'] = []
+                    data[u'rolmembership'], [u'role', u'admin']):
+                return msg
 
-                    for r in data[u'rolmembership']:
+            self._process_rolemembership(id, data)
-                        if type(r) != dict or u'role' not in r or \
+            return None
-                                u'admin' not in r:
+
-                            return precondition_required(msg)
+        msg = _("""
-                        else:
-                            if r[u'admin']:
-                                data[u'admins'].append(r[u'role'])
-                            else:
-                                data[u'members'].append(r[u'role'])
-                else:
-                    msg = _("""
 Role membership information must be passed as a string representing an array of
 JSON objects in the following format:
 rolmembership:{
@@ -357,63 +286,24 @@ rolmembership:{
         ...
         ]
 """)
-                    if type(data[u'rolmembership']) != dict:
+        if not self._validate_input_dict_for_update(
-                        return precondition_required(msg)
+                data[u'rolmembership'], [u'role', u'admin'], [u'role']):
+            return msg
 
-                    data[u'members'] = []
+        self._process_rolemembership(id, data)
-                    data[u'admins'] = []
+        return None
-                    data[u'revoked_admins'] = []
-                    data[u'revoked'] = []
 
-                    if u'added' in data[u'rolmembership']:
+    def _validate_seclabels(self, id, data):
-                        roles = (data[u'rolmembership'])[u'added']
+        """
+        Validate the seclabels data dict
+        :param data: role data
+        :return: valid or invalid message
+        """
+        if u'seclabels' not in data or self.manager.version < 90200:
+            return None
 
-                        if type(roles) != list:
+        if id == -1:
-                            return precondition_required(msg)
+            msg = _("""
-
-                        for r in roles:
-                            if type(r) != dict or \
-                                    u'role' not in r or \
-                                    u'admin' not in r:
-                                return precondition_required(msg)
-
-                            if r[u'admin']:
-                                data[u'admins'].append(r[u'role'])
-                            else:
-                                data[u'members'].append(r[u'role'])
-
-                    if u'deleted' in data[u'rolmembership']:
-                        roles = (data[u'rolmembership'])[u'deleted']
-
-                        if type(roles) != list:
-                            return precondition_required(msg)
-
-                        for r in roles:
-                            if type(r) != dict or u'role' not in r:
-                                return precondition_required(msg)
-
-                            data[u'revoked'].append(r[u'role'])
-
-                    if u'changed' in data[u'rolmembership']:
-                        roles = (data[u'rolmembership'])[u'changed']
-
-                        if type(roles) != list:
-                            return precondition_required(msg)
-
-                        for r in roles:
-                            if type(r) != dict or \
-                                    u'role' not in r or \
-                                    u'admin' not in r:
-                                return precondition_required(msg)
-
-                            if not r[u'admin']:
-                                data[u'revoked_admins'].append(r[u'role'])
-                            else:
-                                data[u'admins'].append(r[u'role'])
-
-            if self.manager.version >= 90200 and u'seclabels' in data:
-                if u'rid' not in kwargs or kwargs['rid'] == -1:
-                    msg = _("""
 Security Label must be passed as an array of JSON objects in the following
 format:
 seclabels:[{
@@ -422,16 +312,13 @@ seclabels:[{
     },
     ...
 ]""")
-                    if type(data[u'seclabels']) != list:
+            if not self._validate_input_dict_for_new(
-                        return precondition_required(msg)
+                    data[u'seclabels'], [u'provider', u'label']):
+                return msg
 
-                    for s in data[u'seclabels']:
+            return None
-                        if type(s) != dict or \
+
-                                u'provider' not in s or \
+        msg = _("""
-                                u'label' not in s:
-                            return precondition_required(msg)
-                else:
-                    msg = _("""
 Security Label must be passed as an array of JSON objects in the following
 format:
 seclabels:{
@@ -454,125 +341,129 @@ seclabels:{
         ...
         ]
 """)
-                    seclabels = data[u'seclabels']
+        if not self._validate_input_dict_for_update(
-                    if type(seclabels) != dict:
+                data[u'seclabels'], [u'provider', u'label'], [u'provider']):
-                        return precondition_required(msg)
+            return msg
 
-                    if u'added' in seclabels:
+        return None
-                        new_seclabels = seclabels[u'added']
 
-                        if type(new_seclabels) != list:
+    def _validate_variables(self, id, data):
-                            return precondition_required(msg)
+        """
+        Validate the variables data dict
+        :param data: role data
+        :return: valid or invalid message
+        """
+        if u'variables' not in data:
+            return None
 
-                        for s in new_seclabels:
+        if id == -1:
-                            if type(s) != dict or \
+            msg = _("""
-                                    u'provider' not in s or \
-                                    u'label' not in s:
-                                return precondition_required(msg)
-
-                    if u'deleted' in seclabels:
-                        removed_seclabels = seclabels[u'deleted']
-
-                        if type(removed_seclabels) != list:
-                            return precondition_required(msg)
-
-                        for s in removed_seclabels:
-                            if (type(s) != dict or u'provider' not in s):
-                                return precondition_required(msg)
-
-                    if u'changed' in seclabels:
-                        changed_seclabels = seclabels[u'deleted']
-
-                        if type(changed_seclabels) != list:
-                            return precondition_required(msg)
-
-                        for s in changed_seclabels:
-                            if type(s) != dict or \
-                                    u'provider' not in s and \
-                                    u'label' not in s:
-                                return precondition_required(msg)
-
-            if u'variables' in data:
-                if u'rid' not in kwargs or kwargs['rid'] == -1:
-                    msg = _("""
 Configuration parameters/variables must be passed as an array of JSON objects
 in the following format in create mode:
 variables:[{
+database: <database> or null,
+name: <configuration>,
+value: <value>
+},
+...
+]""")
+            if not self._validate_input_dict_for_new(
+                    data[u'variables'], [u'name', u'value']):
+                return msg
+
+            return None
+
+        msg = _("""
+Configuration parameters/variables must be passed as an array of JSON objects
+in the following format in update mode:
+rolmembership:{
+'added': [{
     database: <database> or null,
     name: <configuration>,
     value: <value>
     },
     ...
-]""")
+    ],
-                    if type(data[u'variables']) != list:
+'deleted': [{
-                        return precondition_required(msg)
+    database: <database> or null,
-
+    name: <configuration>,
-                    for r in data[u'variables']:
+    value: <value>
-                        if type(r) != dict or u'name' not in r or \
+    },
-                                u'value' not in r:
+    ...
-                            return precondition_required(msg)
+    ],
-                else:
+'updated': [{
-                    msg = _("""
+    database: <database> or null,
-Configuration parameters/variables must be passed as an array of JSON objects
+    name: <configuration>,
-in the following format in update mode:
+    value: <value>
-rolmembership:{
+    },
-    'added': [{
+    ...
-        database: <database> or null,
+    ]
-        name: <configuration>,
-        value: <value>
-        },
-        ...
-        ],
-    'deleted': [{
-        database: <database> or null,
-        name: <configuration>,
-        value: <value>
-        },
-        ...
-        ],
-    'updated': [{
-        database: <database> or null,
-        name: <configuration>,
-        value: <value>
-        },
-        ...
-        ]
 """)
-                    variables = data[u'variables']
+        if not self._validate_input_dict_for_update(
-                    if type(variables) != dict:
+                data[u'variables'], [u'name', u'value'], [u'name']):
-                        return precondition_required(msg)
+            return msg
+        return None
 
-                    if u'added' in variables:
+    def _validate_rolname(self, id, data):
-                        new_vars = variables[u'added']
+        """
+        Validate the rolname data dict
+        :param data: role data
+        :return: valid or invalid message
+        """
+        if (id == -1) and u'rolname' not in data:
+            return precondition_required(
+                _("Name must be specified.")
+            )
+        return None
 
-                        if type(new_vars) != list:
+    def validate_request(f):
-                            return precondition_required(msg)
+        @wraps(f)
+        def wrap(self, **kwargs):
+            if request.data:
+                data = json.loads(request.data, encoding='utf-8')
+            else:
+                data = dict()
+                req = request.args or request.form
 
-                        for v in new_vars:
+                for key in req:
-                            if type(v) != dict or u'name' not in v or \
-                                    u'value' not in v:
-                                return precondition_required(msg)
 
-                    if u'deleted' in variables:
+                    val = req[key]
-                        delete_vars = variables[u'deleted']
+                    if key in [
+                        u'rolcanlogin', u'rolsuper', u'rolcreatedb',
+                        u'rolcreaterole', u'rolinherit', u'rolreplication',
+                        u'rolcatupdate', u'variables', u'rolmembership',
+                        u'seclabels'
+                    ]:
+                        data[key] = json.loads(val, encoding='utf-8')
+                    else:
+                        data[key] = val
 
-                        if type(delete_vars) != list:
+            invalid_msg = self._validate_rolname(kwargs.get('rid', -1), data)
-                            return precondition_required(msg)
+            if invalid_msg is not None:
+                return precondition_required(invalid_msg)
 
-                        for v in delete_vars:
+            invalid_msg = self._validate_rolvaliduntil(data)
-                            if type(v) != dict or u'name' not in v:
+            if invalid_msg is not None:
-                                return precondition_required(msg)
+                return precondition_required(invalid_msg)
 
-                    if u'changed' in variables:
+            invalid_msg = self._validate_rolconnlimit(data)
-                        new_vars = variables[u'changed']
+            if invalid_msg is not None:
+                return precondition_required(invalid_msg)
 
-                        if type(new_vars) != list:
+            invalid_msg = self._validate_rolemembership(
-                            return precondition_required(msg)
+                kwargs.get(u'rid', -1), data)
+            if invalid_msg is not None:
+                return precondition_required(invalid_msg)
 
-                        for v in new_vars:
+            invalid_msg = self._validate_seclabels(
-                            if type(v) != dict or u'name' not in v or \
+                kwargs.get(u'rid', -1), data)
-                                    u'value' not in v:
+            if invalid_msg is not None:
-                                return precondition_required(msg)
+                return precondition_required(invalid_msg)
+
+            invalid_msg = self._validate_variables(
+                kwargs.get(u'rid', -1), data)
+            if invalid_msg is not None:
+                return precondition_required(invalid_msg)
 
             self.request = data
 
@@ -1090,7 +981,7 @@ rolmembership:{
         )
 
     @staticmethod
-    def _handel_dependents_type(types, type_str, type_name, rel_name, row):
+    def _handle_dependents_type(types, type_str, type_name, rel_name, row):
         if types[type_str[0]] is None:
             if type_str[0] == 'i':
                 type_name = 'index'
@@ -1104,7 +995,7 @@ rolmembership:{
         return type_name, rel_name
 
     @staticmethod
-    def _handel_dependents_data(result, types, dependents, db_row):
+    def _handle_dependents_data(result, types, dependents, db_row):
         for row in result['rows']:
             rel_name = row['nspname']
             if rel_name is not None:
@@ -1123,7 +1014,7 @@ rolmembership:{
             if type_str[0] in types:
                 # if type is present in the types dictionary, but it's
                 # value is None then it requires special handling.
-                type_name, rel_name = RoleView._handel_dependents_type(
+                type_name, rel_name = RoleView._handle_dependents_type(
                     types, type_str, type_name, rel_name, row)
             else:
                 continue
@@ -1149,7 +1040,7 @@ rolmembership:{
             if not status:
                 current_app.logger.error(result)
 
-            RoleView._handel_dependents_data(result, types, dependents, db_row)
+            RoleView._handle_dependents_data(result, types, dependents, db_row)
 
     @staticmethod
     def _release_connection(is_connected, manager, db_row):

web/pgadmin/browser/server_groups/servers/roles/templates/roles/sql/9.4_plus/properties.sql

@@ -7,6 +7,7 @@ SELECT
 		FROM
 			(SELECT * FROM pg_auth_members WHERE member = r.oid) am
 			LEFT JOIN pg_catalog.pg_roles rm ON (rm.oid = am.roleid)
+		ORDER BY rm.rolname
 	) AS rolmembership,
 	(SELECT array_agg(provider || '=' || label) FROM pg_shseclabel sl1 WHERE sl1.objoid=r.oid) AS seclabels
 FROM

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options1.msql

@@ -7,5 +7,7 @@ ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#"
 	VALID UNTIL '2050-01-01T00:00:00+05:30'
 	PASSWORD 'xxxxxx';
 
+GRANT pg_signal_backend TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;
+GRANT pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#";
 ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#" IN DATABASE postgres
-    SET application_name TO 'pg4';
+    SET application_name TO 'pg4';

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options1.sql

@@ -0,0 +1,20 @@
+-- Role: "Role2_$%{}[]()&*^!@""'`\/#"
+-- DROP ROLE "Role2_$%{}[]()&*^!@""'`\/#";
+
+CREATE ROLE "Role2_$%{}[]()&*^!@""'`\/#" WITH
+  NOLOGIN
+  SUPERUSER
+  INHERIT
+  CREATEDB
+  NOCREATEROLE
+  NOREPLICATION
+  CONNECTION LIMIT 100
+  ENCRYPTED PASSWORD '<PASSWORD>'
+  VALID UNTIL '<TIMESTAMPTZ_1>';
+
+GRANT pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#";
+GRANT pg_signal_backend TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;
+
+ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#" IN DATABASE postgres SET application_name TO 'pg4';
+
+COMMENT ON ROLE "Role2_$%{}[]()&*^!@""'`\/#" IS 'This is detailed description';

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options2.msql

@@ -0,0 +1,2 @@
+REVOKE ADMIN OPTION FOR pg_signal_backend FROM "Role2_$%{}[]()&*^!@""'`\/#";
+GRANT pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options2.sql

@@ -0,0 +1,20 @@
+-- Role: "Role2_$%{}[]()&*^!@""'`\/#"
+-- DROP ROLE "Role2_$%{}[]()&*^!@""'`\/#";
+
+CREATE ROLE "Role2_$%{}[]()&*^!@""'`\/#" WITH
+  NOLOGIN
+  SUPERUSER
+  INHERIT
+  CREATEDB
+  NOCREATEROLE
+  NOREPLICATION
+  CONNECTION LIMIT 100
+  ENCRYPTED PASSWORD '<PASSWORD>'
+  VALID UNTIL '2050-01-01 00:00:00+05:30';
+
+GRANT pg_signal_backend TO "Role2_$%{}[]()&*^!@""'`\/#";
+GRANT pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;
+
+ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#" IN DATABASE postgres SET application_name TO 'pg4';
+
+COMMENT ON ROLE "Role2_$%{}[]()&*^!@""'`\/#" IS 'This is detailed description';

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options3.msql

@@ -0,0 +1 @@
+REVOKE pg_signal_backend FROM "Role2_$%{}[]()&*^!@""'`\/#";

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options3.sql

@@ -10,7 +10,9 @@ CREATE ROLE "Role2_$%{}[]()&*^!@""'`\/#" WITH
   NOREPLICATION
   CONNECTION LIMIT 100
   ENCRYPTED PASSWORD '<PASSWORD>'
-  VALID UNTIL '<TIMESTAMPTZ_1>';
+  VALID UNTIL '2050-01-01 00:00:00+05:30';
+
+GRANT pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;
 
 ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#" IN DATABASE postgres SET application_name TO 'pg4';
 

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options4.msql

@@ -0,0 +1 @@
+GRANT pg_signal_backend, pg_monitor TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/alter_role_options4.sql

@@ -0,0 +1,19 @@
+-- Role: "Role2_$%{}[]()&*^!@""'`\/#"
+-- DROP ROLE "Role2_$%{}[]()&*^!@""'`\/#";
+
+CREATE ROLE "Role2_$%{}[]()&*^!@""'`\/#" WITH
+  NOLOGIN
+  SUPERUSER
+  INHERIT
+  CREATEDB
+  NOCREATEROLE
+  NOREPLICATION
+  CONNECTION LIMIT 100
+  ENCRYPTED PASSWORD '<PASSWORD>'
+  VALID UNTIL '2050-01-01 00:00:00+05:30';
+
+GRANT pg_monitor, pg_signal_backend TO "Role2_$%{}[]()&*^!@""'`\/#" WITH ADMIN OPTION;
+
+ALTER ROLE "Role2_$%{}[]()&*^!@""'`\/#" IN DATABASE postgres SET application_name TO 'pg4';
+
+COMMENT ON ROLE "Role2_$%{}[]()&*^!@""'`\/#" IS 'This is detailed description';

web/pgadmin/browser/server_groups/servers/roles/tests/9.4_plus/test.json

@@ -50,7 +50,7 @@
     },
     {
       "type": "alter",
-      "name": "Alter Role options",
+      "name": "Alter Role options 1",
       "endpoint": "NODE-role.obj_id",
       "sql_endpoint": "NODE-role.sql_id",
       "msql_endpoint": "NODE-role.msql_id",
@@ -61,10 +61,56 @@
         "rolpassword": "abc123",
         "rolconnlimit": 100,
         "rolvaliduntil": "2050-01-01 00:00:00 +05:30",
-        "variables": { "added": [{"name":"application_name","value":"pg4","database":"postgres"}] }
+        "variables": { "added": [{"name":"application_name","value":"pg4","database":"postgres"}] },
+        "rolmembership": { "added": [{"role": "pg_signal_backend", "admin": true}, {"role": "pg_monitor", "admin": false}] }
       },
-      "expected_sql_file": "alter_role_options.sql",
+      "expected_sql_file": "alter_role_options1.sql",
-      "expected_msql_file": "alter_role_options.msql",
+      "expected_msql_file": "alter_role_options1.msql",
+      "convert_timestamp_columns": ["rolvaliduntil"],
+      "replace_password": true
+    },
+    {
+      "type": "alter",
+      "name": "Alter Role options 2",
+      "endpoint": "NODE-role.obj_id",
+      "sql_endpoint": "NODE-role.sql_id",
+      "msql_endpoint": "NODE-role.msql_id",
+      "data": {
+        "rolmembership": { "changed": [{"role": "pg_signal_backend", "admin": false}, {"role": "pg_monitor", "admin": true}] }
+      },
+      "expected_sql_file": "alter_role_options2.sql",
+      "expected_msql_file": "alter_role_options2.msql",
+      "convert_timestamp_columns": ["rolvaliduntil"],
+      "replace_password": true
+    },
+    {
+      "type": "alter",
+      "name": "Alter Role options 3",
+      "endpoint": "NODE-role.obj_id",
+      "sql_endpoint": "NODE-role.sql_id",
+      "msql_endpoint": "NODE-role.msql_id",
+      "data": {
+        "rolmembership": { "deleted": [{"role": "pg_signal_backend"}] }
+      },
+      "expected_sql_file": "alter_role_options3.sql",
+      "expected_msql_file": "alter_role_options3.msql",
+      "convert_timestamp_columns": ["rolvaliduntil"],
+      "replace_password": true
+    },
+    {
+      "type": "alter",
+      "name": "Alter Role options 4",
+      "endpoint": "NODE-role.obj_id",
+      "sql_endpoint": "NODE-role.sql_id",
+      "msql_endpoint": "NODE-role.msql_id",
+      "data": {
+        "rolmembership": {
+          "added": [{"role": "pg_signal_backend", "admin": true}],
+          "changed": [{"role": "pg_monitor", "admin": true}]
+        }
+      },
+      "expected_sql_file": "alter_role_options4.sql",
+      "expected_msql_file": "alter_role_options4.msql",
       "convert_timestamp_columns": ["rolvaliduntil"],
       "replace_password": true
     },