Validate preference values set via the CLI (#7346)

docs/en_US/release_notes_9_16.rst

@@ -44,6 +44,7 @@ Bug fixes
 *********
 
   | `Issue #6308 <https://github.com/pgadmin-org/pgadmin4/issues/6308>`_ -  Fix the infinite loading spinner after an idle database connection is silently dropped, by detecting stale connections and offering a reconnect dialog.
+  | `Issue #7346 <https://github.com/pgadmin-org/pgadmin4/issues/7346>`_ -  Fixed an issue where preferences set via the CLI (setup.py set-prefs) were not validated, so invalid values were stored silently; CLI preference values are now validated against the preference type and rejected (and reported) if invalid.
   | `Issue #7596 <https://github.com/pgadmin-org/pgadmin4/issues/7596>`_ -  Fix the Query Tool turning into a blank white screen when the runtime has a malformed default locale, by guarding the Query History date/time formatting against the resulting RangeError.
   | `Issue #8318 <https://github.com/pgadmin-org/pgadmin4/issues/8318>`_ -  Fixed an error ("i.default.find(...) is undefined") that prevented deleting a table or relationship link in the ERD tool when a foreign key referenced a column that had been renamed.
   | `Issue #9060 <https://github.com/pgadmin-org/pgadmin4/issues/9060>`_ -  Fixed an issue in the Create Table dialog where renaming a column did not update the column references in foreign key and unique constraint definitions for the new table.

web/pgadmin/utils/preferences.py

@@ -165,12 +165,15 @@ def _get_format_data(self, res):
 
         return False, None
 
-    def set(self, value):
+    def set(self, value, user_id=None):
         """
         set
-        Set the value into the configuration table for this current user.
+        Set the value into the configuration table for this current user
+        (or the given user_id, when called outside a request context).
 
         :param value: Value to be set
+        :param user_id: User to set the preference for; defaults to the
+                        current user.
 
         :returns: nothing.
         """
@@ -217,14 +220,15 @@ def set(self, value):
                 "Invalid value for {0} option.".format(
                     error_map.get(self._type, self._type)))
 
+        uid = user_id if user_id is not None else current_user.id
         pref = UserPrefTable.query.filter_by(
             pid=self.pid
-        ).filter_by(uid=current_user.id).first()
+        ).filter_by(uid=uid).first()
 
         value = "{}".format(value)
         if pref is None:
             pref = UserPrefTable(
-                uid=current_user.id, pid=self.pid, value=value
+                uid=uid, pid=self.pid, value=value
             )
             db.session.add(pref)
         else:
@@ -597,30 +601,53 @@ def module(cls, name, create=True):
     @classmethod
     def save_cli(cls, mid, cid, pid, user_id, value):
         """
-        save
-        Update the value for the preference in the configuration database.
+        save_cli
+        Validate and update the value for the preference in the
+        configuration database for the given user (used by the CLI).
 
         :param mid: Module ID
         :param cid: Category ID
         :param pid: Preference ID
+        :param user_id: User to set the preference for
         :param value: Value for the options
         """
+        # Find the entry for this module in the configuration database.
+        module = ModulePrefTable.query.filter_by(id=mid).first()
 
-        pref = UserPrefTable.query.filter_by(
-            pid=pid
-        ).filter_by(uid=user_id).first()
+        if module is None:
+            return False, gettext("Could not find the specified module.")
 
-        value = "{}".format(value)
-        if pref is None:
-            pref = UserPrefTable(
-                uid=user_id, pid=pid, value=value
-            )
-            db.session.add(pref)
-        else:
-            pref.value = value
-        db.session.commit()
+        m = cls.modules.get(module.name)
+        if m is None:
+            return False, gettext(
+                "Module '{0}' is no longer in use."
+            ).format(module.name)
 
-        return True, None
+        category = None
+        for c in m.categories:
+            cat = m.categories[c]
+            if cid == cat['id']:
+                category = cat
+                break
+
+        if category is None:
+            return False, gettext(
+                "Module '{0}' does not have category with id '{1}'"
+            ).format(module.name, cid)
+
+        preference = None
+        for p in category['preferences']:
+            pref = (category['preferences'])[p]
+            if pref.pid == pid:
+                preference = pref
+                break
+
+        if preference is None:
+            return False, gettext("Could not find the specified preference.")
+
+        # Delegate to set() so the value is validated against the
+        # preference type, just like the GUI path.
+        return preference.set(value, user_id=user_id)
 
     @classmethod
     def save(cls, mid, cid, pid, value):

web/setup.py

@@ -728,6 +728,7 @@ def set_prefs(username,
         prefs = ManagePreferences.fetch_prefs(True)
         app = create_app(config.APP_NAME + '-cli')
         invalid_prefs = []
+        invalid_value_prefs = []
         valid_prefs = []
         with app.app_context():
             from pgadmin.preferences import save_pref
@@ -749,11 +750,13 @@ def set_prefs(username,
                         'name': final_opt[2],
                         'user_id': user_id,
                         'value': val}
-                    save_pref(_row)
-                    valid_prefs.append(_row)
+                    if save_pref(_row):
+                        valid_prefs.append(_row)
 
-                    if not json:
-                        table.add_row(jsonlib.dumps(_row))
+                        if not json:
+                            table.add_row(jsonlib.dumps(_row))
+                    else:
+                        invalid_value_prefs.append(f)
                 else:
                     invalid_prefs.append(f)
 
@@ -762,6 +765,11 @@ def set_prefs(username,
                     (', ').join(
                         invalid_prefs)))
 
+            if len(invalid_value_prefs) >= 1:
+                print("Invalid value provided for preference(s) "
+                      "[red]{0}[/red].".format(
+                          (', ').join(invalid_value_prefs)))
+
             if not json and console:
                 print(table)
             elif json and console: