verify proper value type

Author: davidgamez

functions-python/operations_api/src/feeds_operations/impl/user_feature_flags_impl.py

@@ -217,17 +217,21 @@ def put_user_feature_flags(
         # Validate all provided flag IDs exist before making any changes
         if flag_ids:
             existing_flags = (
-                db_session.query(FeatureFlagORM.id)
+                db_session.query(FeatureFlagORM.id, FeatureFlagORM.value_type)
                 .filter(FeatureFlagORM.id.in_(flag_ids))
                 .all()
             )
-            found_ids = {row.id for row in existing_flags}
-            missing = set(flag_ids) - found_ids
+            value_type_by_id = {row.id: row.value_type for row in existing_flags}
+            missing = set(flag_ids) - value_type_by_id.keys()
             if missing:
                 raise HTTPException(
                     status_code=404,
                     detail=f"Feature flag(s) not found: {', '.join(sorted(missing))}",
                 )
+            # Verify each provided value matches the flag's declared value_type.
+            for a in assignments:
+                if a.value is not None:
+                    _validate_value_type(value_type_by_id[a.feature_flag_id], a.value)
 
         # Replace: delete all existing assignments then insert the new set
         db_session.query(UserFeatureFlag).filter_by(user_id=user_id).delete()

functions-python/operations_api/tests/feeds_operations/impl/test_user_feature_flags.py

@@ -417,17 +417,6 @@ def setUp(self):
         self.api = UserFeatureFlagsApiImpl()
         self.session = MagicMock()
 
-    def _setup_flags_query(self, flag_ids):
-        """Make session.query(FeatureFlagORM.id).filter(...).all() return rows for given IDs."""
-        mock_rows = [MagicMock(id=fid) for fid in flag_ids]
-        mock_q = MagicMock()
-        mock_q.filter.return_value = mock_q
-        mock_q.all.return_value = mock_rows
-        self.session.query.side_effect = lambda model: (
-            mock_q if model is FeatureFlagORM.id else self.session.query(model)
-        )
-        return mock_q
-
     def test_raises_404_when_user_not_found(self):
         _mock_query(self.session, None)
 
@@ -440,7 +429,7 @@ def test_raises_404_when_user_not_found(self):
     def test_raises_404_when_flag_not_found(self):
         user = _make_user(user_feature_flags=[])
 
-        def query_side_effect(model):
+        def query_side_effect(*args):
             mock_q = MagicMock()
             mock_q.options.return_value = mock_q
             mock_q.filter_by.return_value = mock_q
@@ -460,6 +449,91 @@ def query_side_effect(model):
         self.assertEqual(ctx.exception.status_code, 404)
         self.assertIn("nonexistent", ctx.exception.detail)
 
+    def _dispatch_put_queries(self, user, flag_meta_rows):
+        """Route session.query() calls for the put flow.
+
+        `flag_meta_rows` are the (id, value_type) rows returned for the
+        flag-validation query.
+        """
+        user_q = MagicMock()
+        user_q.options.return_value = user_q
+        user_q.filter_by.return_value = user_q
+        user_q.first.return_value = user
+
+        meta_q = MagicMock()
+        meta_q.filter.return_value = meta_q
+        meta_q.all.return_value = flag_meta_rows
+
+        delete_q = MagicMock()
+        delete_q.filter_by.return_value = delete_q
+
+        flags_q = MagicMock()
+        flags_q.order_by.return_value = flags_q
+        flags_q.all.return_value = []
+
+        def side_effect(*args):
+            first = args[0]
+            if first is AppUser:
+                return user_q
+            if first is FeatureFlagORM.id:
+                return meta_q
+            if first is UserFeatureFlag:
+                return delete_q
+            return flags_q
+
+        self.session.query.side_effect = side_effect
+        return delete_q
+
+    def test_assigns_value_matching_flag_type(self):
+        user = _make_user(user_feature_flags=[])
+        rows = [MagicMock(id="beta_editor", value_type="boolean")]
+        delete_q = self._dispatch_put_queries(user, rows)
+
+        req = PutUserFeatureFlagsRequest(
+            assignments=[
+                FeatureFlagAssignment(feature_flag_id="beta_editor", value=True)
+            ]
+        )
+        self.api.put_user_feature_flags("uid-1", req, db_session=self.session)
+
+        delete_q.delete.assert_called_once()
+        added = self.session.add_all.call_args[0][0]
+        self.assertEqual(len(added), 1)
+        self.assertEqual(added[0].feature_flag_id, "beta_editor")
+        self.assertEqual(added[0].value, True)
+
+    def test_raises_422_when_value_type_mismatch(self):
+        user = _make_user(user_feature_flags=[])
+        rows = [MagicMock(id="beta_editor", value_type="boolean")]
+        self._dispatch_put_queries(user, rows)
+
+        req = PutUserFeatureFlagsRequest(
+            assignments=[
+                FeatureFlagAssignment(feature_flag_id="beta_editor", value="not-a-bool")
+            ]
+        )
+        with self.assertRaises(HTTPException) as ctx:
+            self.api.put_user_feature_flags("uid-1", req, db_session=self.session)
+
+        self.assertEqual(ctx.exception.status_code, 422)
+        self.session.add_all.assert_not_called()
+
+    def test_skips_type_validation_when_value_is_null(self):
+        user = _make_user(user_feature_flags=[])
+        rows = [MagicMock(id="beta_editor", value_type="boolean")]
+        self._dispatch_put_queries(user, rows)
+
+        req = PutUserFeatureFlagsRequest(
+            assignments=[
+                FeatureFlagAssignment(feature_flag_id="beta_editor", value=None)
+            ]
+        )
+        self.api.put_user_feature_flags("uid-1", req, db_session=self.session)
+
+        added = self.session.add_all.call_args[0][0]
+        self.assertEqual(len(added), 1)
+        self.assertIsNone(added[0].value)
+
     def test_clears_flags_when_empty_list_provided(self):
         flag = _make_flag()
         user = _make_user(user_feature_flags=[_make_uff(flag)])