(superset) 01/01: fix(subjects): truncate synced values to Subject column widths

[el...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a commit to branch subject-sync-secondary-label-truncate
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 05722f9e8ce09b50647f15b2e180368c6caf344f
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Wed Oct 7 20:57:30 2026 +0000

    fix(subjects): truncate synced values to Subject column widths
    
    Flask-AppBuilder allows user emails up to 320 characters and group
    descriptions up to 512, but Subject.secondary_label and
    Subject.extra_search are String(255). Syncing a user or group with a
    longer value failed with a "value too long" error on strict databases.
    Clip synced values to the Subject column lengths before writing.
    
    Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
 superset/subjects/sync.py              |  45 ++++++++++--
 tests/unit_tests/subjects/test_sync.py | 124 +++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 7 deletions(-)

diff --git a/superset/subjects/sync.py b/superset/subjects/sync.py
index 558a2ff3573..0cb9c11f52e 100644
--- a/superset/subjects/sync.py
+++ b/superset/subjects/sync.py
@@ -21,29 +21,56 @@ from __future__ import annotations
 import logging
 from typing import Any
 
+from sqlalchemy.orm import InstrumentedAttribute
+
 from superset.daos.subject import SubjectDAO
+from superset.subjects.models import Subject
 from superset.subjects.types import SubjectType
 from superset.subjects.utils import get_user_label
 
 logger = logging.getLogger(__name__)
 
 
+def _fit(value: str | None, column: InstrumentedAttribute[Any]) -> str | None:
+    """Truncate ``value`` to the length of a ``Subject`` string column.
+
+    Source fields can be wider than the Subject columns (e.g. FAB allows
+    320-char emails and 512-char group descriptions), so values are clipped
+    to avoid "value too long" errors on strict databases.
+    """
+    max_length = getattr(column.type, "length", None)
+    if value is None or max_length is None or len(value) <= max_length:
+        return value
+    return value[:max_length]
+
+
+def _user_subject_fields(user: Any) -> tuple[str, str | None, str | None]:
+    """Compute label, secondary_label, and extra_search for a user Subject."""
+    email = getattr(user, "email", None)
+    return (
+        _fit(get_user_label(user), Subject.label) or "",
+        _fit(email, Subject.secondary_label),
+        _fit(email, Subject.extra_search),
+    )
+
+
 def sync_user_subject(user: Any) -> None:
     """Create or update the Subject row for a User."""
     if not user or not getattr(user, "id", None):
         return
     subject = SubjectDAO.find_one_or_none(user_id=user.id)
+    label, secondary_label, extra_search = _user_subject_fields(user)
     if subject:
-        subject.label = get_user_label(user)
-        subject.secondary_label = getattr(user, "email", None)
-        subject.extra_search = getattr(user, "email", None)
+        subject.label = label
+        subject.secondary_label = secondary_label
+        subject.extra_search = extra_search
         subject.active = getattr(user, "active", True)
     else:
         SubjectDAO.create(
             attributes={
-                "label": get_user_label(user),
-                "secondary_label": getattr(user, "email", None),
-                "extra_search": getattr(user, "email", None),
+                "label": label,
+                "secondary_label": secondary_label,
+                "extra_search": extra_search,
                 "active": getattr(user, "active", True),
                 "type": SubjectType.USER,
                 "user_id": user.id,
@@ -95,7 +122,11 @@ def _group_subject_fields(group: Any) -> tuple[str, str | None, str | None]:
         secondary_label = description if description else None
 
     extra_search = name if group_label and group_label != name else None
-    return label, secondary_label, extra_search
+    return (
+        _fit(label, Subject.label) or "",
+        _fit(secondary_label, Subject.secondary_label),
+        _fit(extra_search, Subject.extra_search),
+    )
 
 
 def sync_group_subject(group: Any) -> None:
diff --git a/tests/unit_tests/subjects/test_sync.py b/tests/unit_tests/subjects/test_sync.py
new file mode 100644
index 00000000000..c388f6dc1f3
--- /dev/null
+++ b/tests/unit_tests/subjects/test_sync.py
@@ -0,0 +1,124 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for Subject sync helpers."""
+
+from sqlalchemy.orm.session import Session
+
+from superset.subjects.models import Subject
+from superset.subjects.types import SubjectType
+
+SECONDARY_LABEL_LENGTH = Subject.secondary_label.type.length
+EXTRA_SEARCH_LENGTH = Subject.extra_search.type.length
+
+
+def _setup(session: Session) -> None:
+    from flask_appbuilder.security.sqla.models import Group, User
+
+    engine = session.get_bind()
+    for model in (User, Group, Subject):
+        model.metadata.create_all(engine)  # pylint: disable=no-member
+
+
+def test_sync_user_subject_truncates_long_email(session: Session) -> None:
+    """Emails longer than the Subject columns are truncated, not rejected."""
+    from flask_appbuilder.security.sqla.models import User
+
+    from superset.subjects.sync import sync_user_subject
+
+    _setup(session)
+    email = f"{'a' * 300}@example.com"
+    assert len(email) > SECONDARY_LABEL_LENGTH
+    user = User(
+        first_name="Long",
+        last_name="Email",
+        username="long_email",
+        email=email,
+    )
+    session.add(user)
+    session.flush()
+
+    sync_user_subject(user)
+    session.flush()
+
+    subject = session.query(Subject).filter_by(user_id=user.id).one()
+    assert subject.type == SubjectType.USER
+    assert subject.label == "Long Email"
+    assert subject.secondary_label == email[:SECONDARY_LABEL_LENGTH]
+    assert subject.extra_search == email[:EXTRA_SEARCH_LENGTH]
+
+    # Updating an existing Subject truncates as well.
+    user.email = f"{'b' * 300}@example.com"
+    sync_user_subject(user)
+    session.flush()
+
+    session.refresh(subject)
+    assert subject.secondary_label == user.email[:SECONDARY_LABEL_LENGTH]
+    assert subject.extra_search == user.email[:EXTRA_SEARCH_LENGTH]
+
+
+def test_sync_user_subject_keeps_short_email(session: Session) -> None:
+    """Values that fit the column are stored unchanged."""
+    from flask_appbuilder.security.sqla.models import User
+
+    from superset.subjects.sync import sync_user_subject
+
+    _setup(session)
+    user = User(
+        first_name="Short",
+        last_name="Email",
+        username="short_email",
+        email="short@example.com",
+    )
+    session.add(user)
+    session.flush()
+
+    sync_user_subject(user)
+    session.flush()
+
+    subject = session.query(Subject).filter_by(user_id=user.id).one()
+    assert subject.secondary_label == "short@example.com"
+    assert subject.extra_search == "short@example.com"
+
+
+def test_sync_group_subject_truncates_long_description(session: Session) -> None:
+    """Group descriptions longer than the Subject column are truncated."""
+    from flask_appbuilder.security.sqla.models import Group
+
+    from superset.subjects.sync import sync_group_subject
+
+    _setup(session)
+    description = "d" * 512
+    group = Group(name="long_desc", label="Long Description", description=description)
+    session.add(group)
+    session.flush()
+
+    sync_group_subject(group)
+    session.flush()
+
+    subject = session.query(Subject).filter_by(group_id=group.id).one()
+    assert subject.type == SubjectType.GROUP
+    assert subject.label == "Long Description"
+    assert subject.secondary_label == description[:SECONDARY_LABEL_LENGTH]
+    assert subject.extra_search == "long_desc"
+
+    # Updating an existing Subject truncates as well.
+    group.description = "e" * 400
+    sync_group_subject(group)
+    session.flush()
+
+    session.refresh(subject)
+    assert subject.secondary_label == "e" * SECONDARY_LABEL_LENGTH