EnxDev commented on code in PR #42472:
URL: https://github.com/apache/superset/pull/42472#discussion_r3658300042
##########
superset/subjects/utils.py:
##########
@@ -176,6 +288,65 @@ def get_or_create_role_subject(role_id: int) -> Subject |
None:
return get_role_subject(role_id)
+def get_group_subject(group_id: int) -> Subject | None:
+ """Get the GROUP-type Subject for a given group ID."""
+ return (
+ db.session.query(Subject)
+ .filter_by(
+ group_id=group_id,
+ type=SubjectType.GROUP,
+ )
+ .first()
+ )
+
+
+def get_or_create_group_subject(group_id: int) -> Subject | None:
+ """Get or create the GROUP-type Subject for a given group ID.
+
+ Mirrors ``get_or_create_role_subject``: a group that exists but has no
+ Subject row yet (e.g. created before the sync hooks were installed and not
+ yet backfilled) is synced on demand rather than skipped. Returns ``None``
+ only when no such group exists.
+ """
+ if subject := get_group_subject(group_id):
+ return subject
+
+ from superset import security_manager
+ from superset.subjects.sync import sync_group_subject
+
+ group = db.session.get(security_manager.group_model, group_id)
+ if not group:
+ return None
+
+ sync_group_subject(group)
+ db.session.flush()
+ return get_group_subject(group_id)
Review Comment:
Good catch — fixed. Subject.group_id is unique, so a concurrent backfill of
the same group could fail the flush() with an IntegrityError and poison the
transaction. The insert now runs inside a SAVEPOINT
(db.session.begin_nested()); on conflict the savepoint rolls back and the row
the other request wrote is reloaded, leaving the outer transaction usable.
Added a test that drives the IntegrityError/reload path. (Note the sibling
get_or_create_role_subject/get_or_create_user_subject have the same
check-then-insert shape against their own unique columns; I scoped the fix to
the group helper this PR introduces, since the on-demand backfill makes it the
reachable one — happy to harden the other two in a follow-up if you'd prefer.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
