fix(sqla): coerce ARRAY element type in conv_ARRAY (closes #1724) (#2902)

* fix(sqla): coerce ARRAY element type in `conv_ARRAY` (closes #1724)

`AdminModelConverter.conv_ARRAY` returned a `Select2TagsField` with no
`coerce` callable, so every submitted value was cast via the default
`text_type` and the resulting Python list was always `list[str]`. For a
Postgres `ARRAY(Integer)` column that round-trip fails on save with

    column "x" is of type integer[] but expression is of type text[]

(reported in #1724 in 2018 and still reproducible on `master` as of
v2.2.0; the long-standing workaround in that thread is to subclass
`Select2TagsField` and override `process_formdata` to call `int()` on
each entry).

---------

Co-authored-by: ElLorans <lorenzo.cerreta@gmail.com>

Author: Jean-Baptiste Braun

doc/changelog.rst

@@ -6,6 +6,7 @@ Changelog
 
 Bugfixes:
 * Fix encoding for editing file in FileAdmin. Now it uses UTF-8 and accepts non-ASCII characters.
+* SQLAlchemy backend: ``conv_ARRAY`` now infers the array element's ``python_type`` and passes it through as the ``Select2TagsField`` ``coerce`` callable. Saving a Postgres ``ARRAY(Integer)`` / ``ARRAY(Float)`` column no longer fails with ``column "x" is of type integer[] but expression is of type text[]`` (closes #1724).
 
 Type hints:
 * Type hints added to all functions and methods (some using `typing.Any` where full typing not yet available)

flask_admin/_types.py

@@ -280,3 +280,7 @@ class _T_MONGOENGINE_FIELD_PROTOCOL(t.Protocol):
     id: t.Any
     data: t.Any
     name: str
+
+
+class T_FIELD_ARGS_VALIDATORS_COERCE(T_FIELD_ARGS_VALIDATORS, total=False):
+    coerce: t.Callable[[t.Any], t.Any]

flask_admin/contrib/sqla/form.py

@@ -35,6 +35,7 @@
 from ..._types import T_FIELD_ARGS_PLACES
 from ..._types import T_FIELD_ARGS_VALIDATORS
 from ..._types import T_FIELD_ARGS_VALIDATORS_ALLOW_BLANK
+from ..._types import T_FIELD_ARGS_VALIDATORS_COERCE
 from ..._types import T_FIELD_ARGS_VALIDATORS_FILES
 from ..._types import T_INSTRUMENTED_ATTRIBUTE
 from ..._types import T_MODEL_VIEW
@@ -621,8 +622,19 @@ def conv_PGUuid(
         "sqlalchemy.dialects.postgresql.base.ARRAY", "sqlalchemy.sql.sqltypes.ARRAY"
     )
     def conv_ARRAY(
-        self, field_args: T_FIELD_ARGS_VALIDATORS, **extra: t.Any
+        self, field_args: T_FIELD_ARGS_VALIDATORS_COERCE, **extra: t.Any
     ) -> form.Select2TagsField:
+        # Ensure Select2TagsField uses the correct Python type for ARRAY element values.
+        # Otherwise, WTForms defaults to text_type, causing Postgres ARRAY type errors.
+        column = extra.get("column")
+        item_type = getattr(getattr(column, "type", None), "item_type", None)
+        if item_type is not None:
+            try:
+                python_type = item_type.python_type
+            except (AttributeError, NotImplementedError):
+                python_type = None
+            if python_type is not None and python_type is not str:
+                field_args.setdefault("coerce", python_type)
         return form.Select2TagsField(save_as_list=True, **field_args)
 
     @converts("HSTORE")

flask_admin/tests/sqla/test_form.py

@@ -4,6 +4,11 @@
 
 import pytest
 import wtforms
+from sqlalchemy import ARRAY
+from sqlalchemy import Column
+from sqlalchemy import Float
+from sqlalchemy import Integer
+from sqlalchemy import String
 from wtforms.fields.simple import StringField
 
 from flask_admin.contrib.sqla.form import AdminModelConverter
@@ -53,3 +58,87 @@ class TestForm(wtforms.Form):
             pass
 
         assert field() == "<p>widget overridden</p>"
+
+
+class TestArrayConverter:
+    """Regression tests for `AdminModelConverter.conv_ARRAY` -- see issue #1724.
+
+    Without an inner-type-aware coerce callable, every submitted value for a
+    Postgres `ARRAY(Integer)` column is sent back to the DB as a Python
+    `str`, so the resulting `text[]` value is rejected with
+        column "x" is of type integer[] but expression is of type text[]
+    The converter now passes a `coerce` derived from the array element's
+    `python_type` so the round-trip lines up with the column type.
+    """
+
+    def _bind(self, unbound_field: t.Any) -> t.Any:
+        """The converter returns a wtforms UnboundField. Bind it onto a real
+        form so we can drive `process_formdata` and inspect `.data`.
+        """
+
+        class _F(wtforms.Form):
+            x = unbound_field
+
+        return _F().x
+
+    def test_conv_ARRAY_integer_coerces_each_item_to_int(self) -> None:
+        converter = AdminModelConverter(None, None)  # type: ignore[arg-type]
+        column: Column[t.Any] = Column("x", ARRAY(Integer))
+        bound = self._bind(
+            converter.conv_ARRAY(field_args={"validators": []}, column=column)
+        )
+
+        bound.process_formdata(["1,2,3"])
+
+        assert bound.data == [1, 2, 3]
+        # Hard-pin element types so a future change of the inner coerce can't
+        # silently regress to strings.
+        assert all(isinstance(v, int) for v in bound.data), bound.data
+
+    def test_conv_ARRAY_float_coerces_each_item_to_float(self) -> None:
+        converter = AdminModelConverter(None, None)  # type: ignore[arg-type]
+        column: Column[t.Any] = Column("x", ARRAY(Float))
+        bound = self._bind(
+            converter.conv_ARRAY(field_args={"validators": []}, column=column)
+        )
+
+        bound.process_formdata(["1.5, 2.0"])
+
+        assert bound.data == [1.5, 2.0]
+        assert all(isinstance(v, float) for v in bound.data), bound.data
+
+    def test_conv_ARRAY_string_keeps_string_default(self) -> None:
+        """String arrays must continue to work exactly as before -- no
+        spurious coercion that would round-trip values through `int()`.
+        """
+        converter = AdminModelConverter(None, None)  # type: ignore[arg-type]
+        column: Column[t.Any] = Column("x", ARRAY(String))
+        bound = self._bind(
+            converter.conv_ARRAY(field_args={"validators": []}, column=column)
+        )
+
+        bound.process_formdata(["alpha,beta,gamma"])
+
+        assert bound.data == ["alpha", "beta", "gamma"]
+
+    def test_conv_ARRAY_missing_item_type_falls_back_to_text(self) -> None:
+        """If the column object can't be introspected (e.g. legacy callers
+        passing a MagicMock), the converter must not raise. The previous
+        default (string coerce) is preserved.
+        """
+        converter = AdminModelConverter(None, None)  # type: ignore[arg-type]
+        # MagicMock().type.item_type silently returns another MagicMock, whose
+        # python_type access would itself succeed and return a MagicMock --
+        # which is precisely the kind of pathological case we need to handle
+        # without exploding.
+        column = MagicMock()
+        # Force item_type to be absent so the fallback branch is exercised.
+        column.type.spec_set = ["item_type"]
+        del column.type.item_type
+
+        bound = self._bind(
+            converter.conv_ARRAY(field_args={"validators": []}, column=column)
+        )
+
+        bound.process_formdata(["x,y"])
+        assert bound.data == ["x", "y"]