Browse Source

chore(api): Created Test for Path Params (#70909)

Here, I introduce a test to make sure endpoint in `sentry` always have
`_id_or_slug` in their path params.

The test is based of the convert_args tests I have written before.

I find all the urls and if the url class isn't allowlisted (if its web
or auth), I iterate through all the params to make sure it doesn't end
with _slug.

If the check fails, we throw an error like:

![image](https://github.com/getsentry/sentry/assets/33237075/169b56bb-d5d8-4d90-8036-344a4de008cc)
Raj Joshi 10 months ago
parent
commit
50075465f9
2 changed files with 54 additions and 0 deletions
  1. 1 0
      .github/CODEOWNERS
  2. 53 0
      tests/sentry/api/test_path_params.py

+ 1 - 0
.github/CODEOWNERS

@@ -419,6 +419,7 @@ static/app/components/events/eventStatisticalDetector/                    @getse
 /.github/workflows/openapi.yml        @getsentry/owners-api
 /.github/workflows/openapi-diff.yml   @getsentry/owners-api
 /src/sentry/conf/api_pagination_allowlist_do_not_modify.py  @getsentry/owners-api
+/tests/sentry/api/test_path_params.py                      @getsentry/owners-api
 ## End of APIs
 
 

+ 53 - 0
tests/sentry/api/test_path_params.py

@@ -0,0 +1,53 @@
+import re
+from collections.abc import Generator
+from typing import Any
+
+from django.urls import URLPattern, URLResolver
+from django.urls.resolvers import get_resolver
+
+from sentry.testutils.cases import TestCase
+from sentry.testutils.silo import no_silo_test
+
+
+def extract_slug_path_params(path: str) -> list[str]:
+    return re.findall(r"<(\w+_slug)>", path)
+
+
+def extract_all_url_patterns(
+    urlpatterns, base: str = ""
+) -> Generator[Any | tuple[str, Any], Any, None]:
+    for pattern in urlpatterns:
+        if isinstance(pattern, URLResolver):
+            yield from extract_all_url_patterns(pattern.url_patterns, base + str(pattern.pattern))
+        elif isinstance(pattern, URLPattern):
+            url_pattern = base + str(pattern.pattern).replace("^", "").replace("$", "")
+            callback = pattern.callback
+            if hasattr(callback, "view_class"):
+                yield (url_pattern, callback.view_class)
+            elif hasattr(callback, "dispatch_mapping"):
+                for view_func in callback.dispatch_mapping.keys():
+                    if callable(view_func):
+                        yield (url_pattern, view_func.cls)
+
+
+@no_silo_test
+class TestPathParams(TestCase):
+
+    IGNORE_CLASS_PREFIXES = ("sentry.web", "sentry.auth")
+
+    def test_if_sentry_endpoints_have_id_or_slug_path_params(self):
+        """
+        Extract all path parameters, if the url is for an endpoint, check if all path params have _id_or_slug suffix
+        """
+        root_resolver = get_resolver()
+        all_url_patterns = extract_all_url_patterns(root_resolver.url_patterns)
+        for pattern, callback in all_url_patterns:
+            if hasattr(callback, "convert_args"):
+                if slug_path_params := extract_slug_path_params(pattern):
+                    if not callback.__module__.startswith(self.IGNORE_CLASS_PREFIXES):
+                        # if any of the path params are just *_slug and not *_id_or_slug, error
+                        for param in slug_path_params:
+                            if not param.endswith("_id_or_slug"):
+                                self.fail(
+                                    f"Path param {param} in {callback} is missing '_id_or_slug' suffix. Our endpoints support ids and slugs, please only use *_id_or_slug path params."
+                                )