From 5b8c198b3ec82ad08c2eecdec93f3edbac4b4819 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Fri, 12 Jul 2024 14:13:43 -0700 Subject: [PATCH 1/4] updated endpoint & tests --- .../organization_artifactbundle_assemble.py | 36 ++++++-- ...st_organization_artifactbundle_assemble.py | 82 +++++++++++++++++++ 2 files changed, 111 insertions(+), 7 deletions(-) diff --git a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py index 33dabcee3ec8ac..e5a80361f9ef3a 100644 --- a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py +++ b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py @@ -56,19 +56,41 @@ def post(self, request: Request, organization) -> Response: except Exception: return Response({"error": "Invalid json body"}, status=400) - projects = set(data.get("projects", [])) - if len(projects) == 0: + input_projects = set(data.get("projects", [])) + if len(input_projects) == 0: return Response({"error": "You need to specify at least one project"}, status=400) - project_ids = list( + input_project_slug = set() + input_project_id = set() + for project in input_projects: + # IDs are always numeric, slugs cannot be numeric + if str(project).isnumeric(): + input_project_id.add(project) + else: + input_project_slug.add(project) + + matched_projects_by_slug = set( Project.objects.filter( - organization=organization, status=ObjectStatus.ACTIVE, slug__in=projects + organization=organization, + status=ObjectStatus.ACTIVE, + slug__in=input_project_slug, ).values_list("id", flat=True) ) - if len(project_ids) != len(projects): + + matched_projects_by_id = set( + Project.objects.filter( + organization=organization, + status=ObjectStatus.ACTIVE, + id__in=input_project_id, + ).values_list("id", flat=True) + ) + + project_ids = matched_projects_by_slug.union(matched_projects_by_id) + + if len(project_ids) != len(input_projects): return Response({"error": "One or more projects are invalid"}, status=400) - if not self.has_release_permission(request, organization, project_ids=set(project_ids)): + if not self.has_release_permission(request, organization, project_ids=project_ids): raise ResourceDoesNotExist checksum = data.get("checksum") @@ -131,6 +153,6 @@ def post(self, request: Request, organization) -> Response: ) if is_org_auth_token_auth(request.auth): - update_org_auth_token_last_used(request.auth, project_ids) + update_org_auth_token_last_used(request.auth, list(project_ids)) return Response({"state": ChunkFileState.CREATED, "missingChunks": []}, status=200) diff --git a/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py b/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py index ba0761e18e7cde..46f97899a95ef0 100644 --- a/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py +++ b/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py @@ -141,6 +141,88 @@ def test_assemble_with_invalid_projects(self): assert response.status_code == 400, response.content assert response.data["error"] == "One or more projects are invalid" + def test_assemble_with_valid_project_slugs(self): + # Test with all valid project slugs + valid_project = self.create_project() + another_valid_project = self.create_project() + + bundle_file = self.create_artifact_bundle_zip( + org=self.organization.slug, release=self.release.version + ) + total_checksum = sha1(bundle_file).hexdigest() + + blob = FileBlob.from_file(ContentFile(bundle_file)) + FileBlobOwner.objects.get_or_create(organization_id=self.organization.id, blob=blob) + + response = self.client.post( + self.url, + data={ + "checksum": total_checksum, + "chunks": [blob.checksum], + "projects": [valid_project.slug, another_valid_project.slug], + }, + HTTP_AUTHORIZATION=f"Bearer {self.token.token}", + ) + + self.assertEqual(response.status_code, 200) + + def test_assemble_with_valid_project_ids(self): + # Test with all valid project IDs + valid_project = self.create_project() + another_valid_project = self.create_project() + + bundle_file = self.create_artifact_bundle_zip( + org=self.organization.slug, release=self.release.version + ) + total_checksum = sha1(bundle_file).hexdigest() + + blob = FileBlob.from_file(ContentFile(bundle_file)) + FileBlobOwner.objects.get_or_create(organization_id=self.organization.id, blob=blob) + + response = self.client.post( + self.url, + data={ + "checksum": total_checksum, + "chunks": [blob.checksum], + "projects": [str(valid_project.id), str(another_valid_project.id)], + }, + HTTP_AUTHORIZATION=f"Bearer {self.token.token}", + ) + + self.assertEqual(response.status_code, 200) + + def test_assemble_with_mix_of_slugs_and_ids(self): + # Test with a mix of valid project slugs and IDs + valid_project = self.create_project() + another_valid_project = self.create_project() + third_valid_project = self.create_project() + + bundle_file = self.create_artifact_bundle_zip( + org=self.organization.slug, release=self.release.version + ) + total_checksum = sha1(bundle_file).hexdigest() + + blob = FileBlob.from_file(ContentFile(bundle_file)) + FileBlobOwner.objects.get_or_create(organization_id=self.organization.id, blob=blob) + + response = self.client.post( + self.url, + data={ + "checksum": total_checksum, + "chunks": [blob.checksum], + "projects": [ + valid_project.slug, + str(another_valid_project.id), + str(third_valid_project.id), + ], + }, + HTTP_AUTHORIZATION=f"Bearer {self.token.token}", + ) + + self.assertEqual(response.status_code, 200) + + # You can add more tests here following the same pattern to cover additional scenarios + @patch("sentry.tasks.assemble.assemble_artifacts") def test_assemble_without_version_and_dist(self, mock_assemble_artifacts): bundle_file = self.create_artifact_bundle_zip( From 8c503aeb8ecc28ea5284f012757a959cde44e012 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Fri, 12 Jul 2024 14:21:49 -0700 Subject: [PATCH 2/4] nit --- .../api/endpoints/test_organization_artifactbundle_assemble.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py b/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py index 46f97899a95ef0..4fe8b0285820c0 100644 --- a/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py +++ b/tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py @@ -221,8 +221,6 @@ def test_assemble_with_mix_of_slugs_and_ids(self): self.assertEqual(response.status_code, 200) - # You can add more tests here following the same pattern to cover additional scenarios - @patch("sentry.tasks.assemble.assemble_artifacts") def test_assemble_without_version_and_dist(self, mock_assemble_artifacts): bundle_file = self.create_artifact_bundle_zip( From 1e2755b9700d3ca26e994ed8cf0c94e32751368c Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Mon, 15 Jul 2024 08:56:14 -0700 Subject: [PATCH 3/4] pr feedback --- .../organization_artifactbundle_assemble.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py index e5a80361f9ef3a..e5b6be4616d3ca 100644 --- a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py +++ b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py @@ -1,5 +1,6 @@ import jsonschema import orjson +from django.db.models import Q from rest_framework.request import Request from rest_framework.response import Response @@ -64,29 +65,19 @@ def post(self, request: Request, organization) -> Response: input_project_id = set() for project in input_projects: # IDs are always numeric, slugs cannot be numeric - if str(project).isnumeric(): + if str(project).isdecimal(): input_project_id.add(project) else: input_project_slug.add(project) - matched_projects_by_slug = set( + project_ids = set( Project.objects.filter( + (Q(id__in=input_project_id) | Q(slug__in=input_project_slug)), organization=organization, status=ObjectStatus.ACTIVE, - slug__in=input_project_slug, ).values_list("id", flat=True) ) - matched_projects_by_id = set( - Project.objects.filter( - organization=organization, - status=ObjectStatus.ACTIVE, - id__in=input_project_id, - ).values_list("id", flat=True) - ) - - project_ids = matched_projects_by_slug.union(matched_projects_by_id) - if len(project_ids) != len(input_projects): return Response({"error": "One or more projects are invalid"}, status=400) From b1da53bcd84a1089b168480d81ac0de955586078 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Mon, 15 Jul 2024 12:52:53 -0700 Subject: [PATCH 4/4] pr feedback --- .../organization_artifactbundle_assemble.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py index e5b6be4616d3ca..669b22f7828d74 100644 --- a/src/sentry/api/endpoints/organization_artifactbundle_assemble.py +++ b/src/sentry/api/endpoints/organization_artifactbundle_assemble.py @@ -57,7 +57,7 @@ def post(self, request: Request, organization) -> Response: except Exception: return Response({"error": "Invalid json body"}, status=400) - input_projects = set(data.get("projects", [])) + input_projects = data.get("projects", []) if len(input_projects) == 0: return Response({"error": "You need to specify at least one project"}, status=400) @@ -70,18 +70,16 @@ def post(self, request: Request, organization) -> Response: else: input_project_slug.add(project) - project_ids = set( - Project.objects.filter( - (Q(id__in=input_project_id) | Q(slug__in=input_project_slug)), - organization=organization, - status=ObjectStatus.ACTIVE, - ).values_list("id", flat=True) - ) + project_ids = Project.objects.filter( + (Q(id__in=input_project_id) | Q(slug__in=input_project_slug)), + organization=organization, + status=ObjectStatus.ACTIVE, + ).values_list("id", flat=True) if len(project_ids) != len(input_projects): return Response({"error": "One or more projects are invalid"}, status=400) - if not self.has_release_permission(request, organization, project_ids=project_ids): + if not self.has_release_permission(request, organization, project_ids=set(project_ids)): raise ResourceDoesNotExist checksum = data.get("checksum")