Skip to content

Commit d166d51

Browse files
simplify: inline namespace extraction, remove extra methods and tests
Made-with: Cursor
1 parent 2f448a4 commit d166d51

File tree

2 files changed

+19
-97
lines changed

2 files changed

+19
-97
lines changed

apps/codecov-api/webhook_handlers/tests/test_gitlab.py

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,7 @@ def test_handle_system_hook_when_not_enterprise(self):
276276
response = self._post_event_data(**event_data)
277277
assert response.status_code == status.HTTP_403_FORBIDDEN
278278

279-
def test_duplicate_repos_disambiguated_by_job_project_name(self):
280-
"""Job Hook payloads include project_name ('namespace/repo') which
281-
can disambiguate when two owners share the same service_id."""
279+
def test_duplicate_repos_disambiguated_by_project_name(self):
282280
owner2 = OwnerFactory(service="gitlab")
283281
RepositoryFactory(author=owner2, service_id=self.repo.service_id, active=True)
284282

@@ -288,15 +286,13 @@ def test_duplicate_repos_disambiguated_by_job_project_name(self):
288286
"object_kind": "build",
289287
"project_id": self.repo.service_id,
290288
"build_status": "pending",
291-
"project_name": f"{self.repo.author.username}/my-repo",
289+
"project_name": f"{self.repo.author.username} / my-repo",
292290
},
293291
)
294292
assert response.status_code == status.HTTP_200_OK
295293
assert response.data == WebhookHandlerErrorMessages.SKIP_PENDING_STATUSES
296294

297-
def test_duplicate_repos_disambiguated_by_push_path(self):
298-
"""Push events include project.path_with_namespace which can
299-
disambiguate duplicate repos."""
295+
def test_duplicate_repos_disambiguated_by_path_with_namespace(self):
300296
owner2 = OwnerFactory(service="gitlab")
301297
RepositoryFactory(author=owner2, service_id=self.repo.service_id, active=True)
302298

@@ -313,38 +309,6 @@ def test_duplicate_repos_disambiguated_by_push_path(self):
313309
assert response.status_code == status.HTTP_200_OK
314310
assert response.data == "No yaml cached yet."
315311

316-
def test_duplicate_repos_disambiguated_by_job_project_name_spaced(self):
317-
"""Some GitLab versions use 'namespace / repo' format with spaces."""
318-
owner2 = OwnerFactory(service="gitlab")
319-
RepositoryFactory(author=owner2, service_id=self.repo.service_id, active=True)
320-
321-
response = self._post_event_data(
322-
event=GitLabWebhookEvents.JOB,
323-
data={
324-
"object_kind": "build",
325-
"project_id": self.repo.service_id,
326-
"build_status": "pending",
327-
"project_name": f"{self.repo.author.username} / my-repo",
328-
},
329-
)
330-
assert response.status_code == status.HTTP_200_OK
331-
assert response.data == WebhookHandlerErrorMessages.SKIP_PENDING_STATUSES
332-
333-
def test_duplicate_repos_without_namespace_falls_through(self):
334-
"""Without namespace info, duplicate repos still raise an error."""
335-
owner2 = OwnerFactory(service="gitlab")
336-
RepositoryFactory(author=owner2, service_id=self.repo.service_id, active=True)
337-
338-
response = self._post_event_data(
339-
event=GitLabWebhookEvents.JOB,
340-
data={
341-
"object_kind": "build",
342-
"project_id": self.repo.service_id,
343-
"build_status": "pending",
344-
},
345-
)
346-
assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
347-
348312
def test_secret_validation(self):
349313
owner = OwnerFactory(service="gitlab")
350314
repo = RepositoryFactory(

apps/codecov-api/webhook_handlers/views/gitlab.py

Lines changed: 16 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -51,60 +51,6 @@ def _inc_err(self, reason: str):
5151
error_reason=reason,
5252
).inc()
5353

54-
def _get_owner_namespace(self, request) -> str | None:
55-
"""Extract the owner namespace from the webhook payload.
56-
57-
Different GitLab event types place this in different fields:
58-
- Push/MR/Pipeline events: project.path_with_namespace ("namespace/repo")
59-
- System hooks: path_with_namespace ("namespace/repo")
60-
- Job hooks: project_name ("namespace/repo" or "namespace / repo")
61-
62-
See: https://docs.gitlab.com/ee/user/project/integrations/webhook_events.html
63-
"""
64-
path_with_namespace = request.data.get("project", {}).get(
65-
"path_with_namespace"
66-
) or request.data.get("path_with_namespace")
67-
68-
if path_with_namespace:
69-
parts = path_with_namespace.split("/")
70-
if len(parts) >= 2:
71-
return parts[0].strip()
72-
73-
project_name = request.data.get("project_name")
74-
if project_name:
75-
parts = project_name.split("/")
76-
if len(parts) >= 2:
77-
return parts[0].strip()
78-
79-
return None
80-
81-
def _get_repo_from_webhook(self, request, project_id) -> Repository:
82-
"""Look up the Repository for the given webhook, disambiguating by owner
83-
namespace when multiple repos share the same service_id."""
84-
base_filter = {
85-
"author__service": self.service_name,
86-
"service_id": project_id,
87-
}
88-
owner_namespace = self._get_owner_namespace(request)
89-
90-
if owner_namespace:
91-
try:
92-
return Repository.objects.get(
93-
**base_filter, author__username=owner_namespace
94-
)
95-
except Repository.DoesNotExist:
96-
pass
97-
except Repository.MultipleObjectsReturned:
98-
log.warning(
99-
"Multiple repos found even with namespace filter",
100-
extra={
101-
"service_id": project_id,
102-
"namespace": owner_namespace,
103-
},
104-
)
105-
106-
return get_object_or_404(Repository, **base_filter)
107-
10854
def post(self, request, *args, **kwargs):
10955
"""
11056
Helpful docs for working with GitLab webhooks
@@ -135,11 +81,23 @@ def post(self, request, *args, **kwargs):
13581
self._inc_err("permission_denied")
13682
raise PermissionDenied()
13783

84+
# Use the owner namespace from the payload to disambiguate when
85+
# multiple owners have repos with the same GitLab project_id.
86+
# See: https://docs.gitlab.com/ee/user/project/integrations/webhook_events.html
87+
path_with_ns = (
88+
request.data.get("project", {}).get("path_with_namespace")
89+
or request.data.get("path_with_namespace")
90+
or request.data.get("project_name")
91+
)
92+
repo_filter = {
93+
"author__service": self.service_name,
94+
"service_id": project_id,
95+
}
96+
if path_with_ns and "/" in path_with_ns:
97+
repo_filter["author__username"] = path_with_ns.split("/")[0].strip()
98+
13899
try:
139-
repo = self._get_repo_from_webhook(request, project_id)
140-
except Repository.MultipleObjectsReturned as e:
141-
self._inc_err("repo_multiple_found")
142-
raise e
100+
repo = get_object_or_404(Repository, **repo_filter)
143101
except Exception as e:
144102
self._inc_err("repo_not_found")
145103
raise e

0 commit comments

Comments
 (0)