Skip to content

Commit 2e1a0fc

Browse files
authored
ref(db): Add test to prevent further usage of legacy create_or_update (#97564)
There is an ongoing effort to get rid of custom legacy `create_or_update` method. In order to prevent any further usage of that method, this test is added. All of the current usages are allowlisted, but any future usage should be prevented with a clear message to use alternative method in Django ORM.
1 parent b806079 commit 2e1a0fc

File tree

1 file changed

+160
-0
lines changed

1 file changed

+160
-0
lines changed
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
from __future__ import annotations
2+
3+
import ast
4+
from collections.abc import Iterable
5+
from dataclasses import dataclass
6+
from pathlib import Path
7+
from typing import Any
8+
9+
# Allowlist of files (relative to repo root) that may still use legacy method
10+
# create_or_update. Instead, use Django's update_or_create.
11+
# Reduce over time as code is refactored. DO NOT add new files here.
12+
ALLOWLIST_FILES: set[str] = {
13+
"src/sentry/buffer/base.py",
14+
"src/sentry/db/models/manager/base.py",
15+
"src/sentry/notifications/services/impl.py",
16+
"src/sentry/nodestore/django/backend.py",
17+
"src/sentry/issues/ignored.py",
18+
"src/sentry/issues/endpoints/organization_group_search_view_visit.py",
19+
"src/sentry/api/helpers/group_index/update.py",
20+
"src/sentry/api/endpoints/organization_pinned_searches.py",
21+
"src/sentry/api/endpoints/project_template_detail.py",
22+
"src/sentry/api/endpoints/release_deploys.py",
23+
"src/sentry/api/endpoints/organization_unsubscribe.py",
24+
"src/sentry/api/endpoints/organization_recent_searches.py",
25+
"src/sentry/api/endpoints/prompts_activity.py",
26+
"src/sentry/dashboards/endpoints/organization_dashboard_details.py",
27+
"src/sentry/onboarding_tasks/backends/organization_onboarding_task.py",
28+
"src/sentry/explore/endpoints/explore_saved_query_detail.py",
29+
"src/sentry/models/featureadoption.py",
30+
"src/sentry/models/groupmeta.py",
31+
"src/sentry/models/release.py",
32+
"src/sentry/models/releases/set_commits.py",
33+
"src/sentry/models/options/organization_option.py",
34+
"src/sentry/models/options/project_template_option.py",
35+
"src/sentry/audit_log/services/log/impl.py",
36+
"src/sentry/utils/mockdata/core.py",
37+
"src/sentry/core/endpoints/organization_details.py",
38+
"src/sentry/flags/endpoints/secrets.py",
39+
"src/sentry/tasks/assemble.py",
40+
"src/sentry/tasks/commits.py",
41+
"src/sentry/options/store.py",
42+
"src/sentry/services/nodestore/django/backend.py",
43+
}
44+
45+
46+
@dataclass
47+
class Usage:
48+
file_path: str
49+
line: int
50+
col: int
51+
qualified_context: str
52+
53+
54+
class CreateOrUpdateVisitor(ast.NodeVisitor):
55+
def __init__(self, module_qualname: str) -> None:
56+
self.module_qualname = module_qualname
57+
self.context_stack: list[str] = []
58+
self.usages: list[Usage] = []
59+
60+
def visit_ClassDef(self, node: ast.ClassDef) -> Any:
61+
self.context_stack.append(node.name)
62+
self.generic_visit(node)
63+
self.context_stack.pop()
64+
65+
def visit_FunctionDef(self, node: ast.FunctionDef) -> Any:
66+
self.context_stack.append(node.name)
67+
self.generic_visit(node)
68+
self.context_stack.pop()
69+
70+
def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> Any:
71+
self.context_stack.append(node.name)
72+
self.generic_visit(node)
73+
self.context_stack.pop()
74+
75+
def visit_Call(self, node: ast.Call) -> Any:
76+
func = node.func
77+
is_create_or_update = False
78+
if isinstance(func, ast.Name):
79+
is_create_or_update = func.id == "create_or_update"
80+
elif isinstance(func, ast.Attribute):
81+
is_create_or_update = func.attr == "create_or_update"
82+
83+
if is_create_or_update:
84+
context = ".".join(self.context_stack) if self.context_stack else "<module>"
85+
qualified = (
86+
f"{self.module_qualname}.{context}"
87+
if context != "<module>"
88+
else self.module_qualname
89+
)
90+
# file_path is filled in by the scanner
91+
self.usages.append(
92+
Usage(
93+
file_path="",
94+
line=node.lineno,
95+
col=getattr(node, "col_offset", 0),
96+
qualified_context=qualified,
97+
)
98+
)
99+
100+
self.generic_visit(node)
101+
102+
103+
def _iter_python_files(root: Path) -> Iterable[Path]:
104+
yield from root.rglob("*.py")
105+
106+
107+
def _module_qualname_from_path(repo_root: Path, file_path: Path) -> str:
108+
rel = file_path.relative_to(repo_root)
109+
# strip .py and convert / to .
110+
parts = list(rel.parts)
111+
# remove trailing .py
112+
parts[-1] = parts[-1][:-3]
113+
return ".".join(parts)
114+
115+
116+
def _scan_create_or_update(repo_root: Path, src_root: Path) -> list[Usage]:
117+
results: list[Usage] = []
118+
for file_path in _iter_python_files(src_root):
119+
text = file_path.read_text(encoding="utf-8")
120+
try:
121+
tree = ast.parse(text)
122+
except SyntaxError:
123+
# Ignore unparsable files (should not happen under src/)
124+
continue
125+
126+
module_qualname = _module_qualname_from_path(repo_root, file_path)
127+
visitor = CreateOrUpdateVisitor(module_qualname)
128+
visitor.visit(tree)
129+
rel_path = str(file_path.relative_to(repo_root))
130+
for u in visitor.usages:
131+
# fill file path for each usage
132+
u.file_path = rel_path
133+
results.extend(visitor.usages)
134+
return results
135+
136+
137+
def test_no_new_create_or_update_usage() -> None:
138+
repo_root = Path(__file__).resolve().parents[2]
139+
src_root = repo_root / "src"
140+
141+
usages = _scan_create_or_update(repo_root=repo_root, src_root=src_root)
142+
143+
violations: list[str] = []
144+
for u in usages:
145+
file_allowed = u.file_path in ALLOWLIST_FILES
146+
if not file_allowed:
147+
violations.append(
148+
f"{u.file_path}:{u.line}:{u.col}: create_or_update used in {u.qualified_context}. "
149+
f"Use Django's update_or_create instead."
150+
)
151+
152+
if violations:
153+
header = (
154+
"Found disallowed uses of create_or_update. New code must use Django's update_or_create.\n"
155+
"See Django docs: https://docs.djangoproject.com/en/5.2/ref/models/querysets/#update-or-create\n"
156+
"If this is legacy code, add the specific function or file to the allowlist in "
157+
"tests/sentry/test_no_create_or_update_usage.py and plan its refactor.\n\n"
158+
)
159+
detail = "\n".join(sorted(violations))
160+
raise AssertionError(header + detail)

0 commit comments

Comments
 (0)