From de8f160c418582c661ec42bb06f84c378bf00bb5 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Thu, 17 Apr 2025 09:44:23 +0200 Subject: [PATCH] Deprecate the "local upload" `ReportResults` endpoints This will now return an empty response with a deprecation message, and will not call out to the worker task anymore. --- reports/tests/factories.py | 15 +--- services/task/task.py | 11 --- upload/serializers.py | 17 +---- upload/tests/test_serializers.py | 22 ------ upload/tests/views/test_reports.py | 116 ++--------------------------- upload/views/reports.py | 84 +++++---------------- 6 files changed, 26 insertions(+), 239 deletions(-) diff --git a/reports/tests/factories.py b/reports/tests/factories.py index 3c77e59d71..6da55100b8 100644 --- a/reports/tests/factories.py +++ b/reports/tests/factories.py @@ -6,7 +6,7 @@ from graphql_api.types.enums import UploadErrorEnum from reports import models -from reports.models import ReportResults, TestInstance +from reports.models import TestInstance class CommitReportFactory(DjangoModelFactory): @@ -77,19 +77,6 @@ class Meta: ) -class ReportResultsFactory(DjangoModelFactory): - class Meta: - model = ReportResults - - report = factory.SubFactory(CommitReportFactory) - state = factory.Iterator( - [ - ReportResults.ReportResultsStates.PENDING, - ReportResults.ReportResultsStates.COMPLETED, - ] - ) - - class TestFactory(factory.django.DjangoModelFactory): class Meta: model = models.Test diff --git a/services/task/task.py b/services/task/task.py index 50abc7fef8..926d9a3d85 100644 --- a/services/task/task.py +++ b/services/task/task.py @@ -311,17 +311,6 @@ def update_commit(self, commitid, repoid): kwargs=dict(commitid=commitid, repoid=repoid), ).apply_async() - def create_report_results(self, commitid, repoid, report_code, current_yaml=None): - self._create_signature( - "app.tasks.reports.save_report_results", - kwargs=dict( - commitid=commitid, - repoid=repoid, - report_code=report_code, - current_yaml=current_yaml, - ), - ).apply_async() - def http_request(self, url, method="POST", headers=None, data=None, timeout=None): self._create_signature( "app.tasks.http_request.HTTPRequest", diff --git a/upload/serializers.py b/upload/serializers.py index b46e047863..ac6ba5696c 100644 --- a/upload/serializers.py +++ b/upload/serializers.py @@ -7,7 +7,7 @@ from codecov_auth.models import Owner from core.models import Commit, Repository -from reports.models import CommitReport, ReportResults, ReportSession, RepositoryFlag +from reports.models import CommitReport, ReportSession, RepositoryFlag from services.task import TaskService @@ -178,18 +178,3 @@ def create(self, validated_data: Dict[str, Any]) -> tuple[CommitReport, bool]: report.save() return report, False return super().create(validated_data), True - - -class ReportResultsSerializer(serializers.ModelSerializer): - report = CommitReportSerializer(read_only=True) - - class Meta: - model = ReportResults - read_only_fields = ( - "external_id", - "report", - "state", - "result", - "completed_at", - ) - fields = read_only_fields diff --git a/upload/tests/test_serializers.py b/upload/tests/test_serializers.py index 8d58606153..e2d2ff8fad 100644 --- a/upload/tests/test_serializers.py +++ b/upload/tests/test_serializers.py @@ -9,14 +9,12 @@ from billing.helpers import mock_all_plans_and_tiers from reports.tests.factories import ( CommitReportFactory, - ReportResultsFactory, RepositoryFlagFactory, UploadFactory, ) from upload.serializers import ( CommitReportSerializer, CommitSerializer, - ReportResultsSerializer, UploadSerializer, ) @@ -237,23 +235,3 @@ def test_commit_report_serializer(transactional_db, mocker): "code": report.code, } assert serializer.data == expected_data - - -def test_report_results_serializer(transactional_db, mocker): - report_result = ReportResultsFactory.create() - serializer = ReportResultsSerializer(report_result) - expected_data = { - "external_id": str(report_result.external_id), - "report": { - "external_id": str(report_result.report.external_id), - "created_at": report_result.report.created_at.strftime( - "%Y-%m-%dT%H:%M:%S.%fZ" - ), - "commit_sha": report_result.report.commit.commitid, - "code": report_result.report.code, - }, - "state": report_result.state, - "result": report_result.result, - "completed_at": report_result.completed_at, - } - assert serializer.data == expected_data diff --git a/upload/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 22f1f25c34..fefc5332d4 100644 --- a/upload/tests/views/test_reports.py +++ b/upload/tests/views/test_reports.py @@ -10,9 +10,9 @@ RepositoryFactory, ) -from reports.models import CommitReport, ReportResults -from reports.tests.factories import ReportResultsFactory +from reports.models import CommitReport from services.task.task import TaskService +from upload.views.reports import EMPTY_RESPONSE from upload.views.uploads import CanDoCoverageUploadsPermission @@ -336,7 +336,6 @@ def test_reports_post_code_as_default(client, db, mocker): def test_reports_results_post_successful(client, db, mocker): - mocked_task = mocker.patch("services.task.TaskService.create_report_results") mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True ) @@ -362,10 +361,6 @@ def test_reports_results_post_successful(client, db, mocker): == f"/upload/github/codecov::::the_repo/commits/{commit.commitid}/reports/code/results" ) assert response.status_code == 201 - assert ReportResults.objects.filter( - report_id=commit_report.id, - ).exists() - mocked_task.assert_called_once() @patch("upload.helpers.jwt.decode") @@ -373,8 +368,6 @@ def test_reports_results_post_successful(client, db, mocker): def test_reports_results_post_successful_github_oidc_auth( mock_jwks_client, mock_jwt_decode, client, db, mocker ): - mocked_task = mocker.patch("services.task.TaskService.create_report_results") - mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True ) @@ -410,22 +403,6 @@ def test_reports_results_post_successful_github_oidc_auth( == f"/upload/github/codecov::::the_repo/commits/{commit.commitid}/reports/code/results" ) assert response.status_code == 201 - assert ReportResults.objects.filter( - report_id=commit_report.id, - ).exists() - mocked_task.assert_called_once() - mock_prometheus_metrics.assert_called_with( - **{ - "agent": "cli", - "version": "0.4.7", - "action": "coverage", - "endpoint": "create_report_results", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - "upload_version": None, - }, - ) @pytest.mark.parametrize("private", [False, True]) @@ -449,7 +426,6 @@ def test_reports_results_post_upload_token_required_auth_check( branch_sent, upload_token_required_for_public_repos, ): - mocked_task = mocker.patch("services.task.TaskService.create_report_results") repository = RepositoryFactory( name="the_repo", author__username="codecov", @@ -462,6 +438,7 @@ def test_reports_results_post_upload_token_required_auth_check( commit.branch = branch repository.save() commit.save() + commit_report.save() client = APIClient() url = reverse( @@ -492,55 +469,11 @@ def test_reports_results_post_upload_token_required_auth_check( or authorized_by_tokenless_auth_class ): assert response.status_code == 201 - assert ReportResults.objects.filter( - report_id=commit_report.id, - ).exists() - mocked_task.assert_called_once() else: assert response.status_code == 401 - assert not ReportResults.objects.filter( - report_id=commit_report.id, - ).exists() assert response.json().get("detail") == "Not valid tokenless upload" -def test_reports_results_already_exists_post_successful(client, db, mocker): - mocked_task = mocker.patch("services.task.TaskService.create_report_results") - mocker.patch.object( - CanDoCoverageUploadsPermission, "has_permission", return_value=True - ) - repository = RepositoryFactory( - name="the_repo", author__username="codecov", author__service="github" - ) - commit = CommitFactory(repository=repository) - commit_report = CommitReport.objects.create(commit=commit, code="code") - report_results = ReportResults.objects.create( - report=commit_report, state=ReportResults.ReportResultsStates.COMPLETED - ) - repository.save() - commit_report.save() - report_results.save() - - owner = repository.author - client = APIClient() - client.force_authenticate(user=owner) - url = reverse( - "new_upload.reports_results", - args=["github", "codecov::::the_repo", commit.commitid, "code"], - ) - response = client.post(url, content_type="application/json", data={}) - - assert ( - url - == f"/upload/github/codecov::::the_repo/commits/{commit.commitid}/reports/code/results" - ) - assert response.status_code == 201 - assert ReportResults.objects.filter( - report_id=commit_report.id, state=ReportResults.ReportResultsStates.PENDING - ).exists() - mocked_task.assert_called_once() - - def test_report_results_get_successful(client, db, mocker): mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True @@ -550,9 +483,8 @@ def test_report_results_get_successful(client, db, mocker): ) commit = CommitFactory(repository=repository) commit_report = CommitReport.objects.create(commit=commit, code="code") - commit_report_results = ReportResultsFactory(report=commit_report) repository.save() - commit_report_results.save() + commit_report.save() owner = repository.author client = APIClient() @@ -568,42 +500,4 @@ def test_report_results_get_successful(client, db, mocker): == f"/upload/github/codecov::::the_repo/commits/{commit.commitid}/reports/code/results" ) assert response.status_code == 200 - assert response.json() == { - "external_id": str(commit_report_results.external_id), - "report": { - "external_id": str(commit_report.external_id), - "created_at": commit_report.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), - "commit_sha": commit_report.commit.commitid, - "code": commit_report.code, - }, - "state": commit_report_results.state, - "result": {}, - "completed_at": None, - } - - -def test_report_results_get_unsuccessful(client, db, mocker): - mocker.patch.object( - CanDoCoverageUploadsPermission, "has_permission", return_value=True - ) - repository = RepositoryFactory( - name="the_repo", author__username="codecov", author__service="github" - ) - commit = CommitFactory(repository=repository) - CommitReport.objects.create(commit=commit, code="code") - repository.save() - - client = APIClient() - client.force_authenticate(user=OwnerFactory()) - url = reverse( - "new_upload.reports_results", - args=["github", "codecov::::the_repo", commit.commitid, "code"], - ) - response = client.get(url, content_type="application/json", data={}) - - assert ( - url - == f"/upload/github/codecov::::the_repo/commits/{commit.commitid}/reports/code/results" - ) - assert response.status_code == 400 - assert response.json() == ["Report Results not found"] + assert response.json() == EMPTY_RESPONSE diff --git a/upload/views/reports.py b/upload/views/reports.py index a6140d5a91..fc6d8abf4d 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -2,9 +2,10 @@ from typing import Any, Callable from django.http import HttpRequest, HttpResponseNotAllowed -from rest_framework.exceptions import ValidationError -from rest_framework.generics import CreateAPIView, ListCreateAPIView, RetrieveAPIView +from rest_framework import status +from rest_framework.generics import ListCreateAPIView from rest_framework.response import Response +from rest_framework.views import APIView from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( @@ -17,14 +18,14 @@ repo_auth_custom_exception_handler, ) from core.models import Commit, Repository -from reports.models import CommitReport, ReportResults +from reports.models import CommitReport from services.task import TaskService from upload.helpers import ( generate_upload_prometheus_metrics_labels, validate_activated_repo, ) from upload.metrics import API_UPLOAD_COUNTER -from upload.serializers import CommitReportSerializer, ReportResultsSerializer +from upload.serializers import CommitReportSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -102,12 +103,16 @@ def list( return HttpResponseNotAllowed(permitted_methods=["POST"]) -class ReportResultsView( - CreateAPIView, - RetrieveAPIView, - GetterMixin, -): - serializer_class = ReportResultsSerializer +EMPTY_RESPONSE = { + "state": "completed", + "result": { + "state": "deprecated", + "message": 'The "local upload" functionality has been deprecated.', + }, +} + + +class ReportResultsView(APIView, GetterMixin): permission_classes = [CanDoCoverageUploadsPermission] authentication_classes = [ UploadTokenRequiredAuthenticationCheck, @@ -121,59 +126,8 @@ class ReportResultsView( def get_exception_handler(self) -> Callable[[Exception, dict[str, Any]], Response]: return repo_auth_custom_exception_handler - def perform_create(self, serializer: ReportResultsSerializer) -> ReportResults: - inc_counter( - API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_labels( - action="coverage", - endpoint="create_report_results", - request=self.request, - is_shelter_request=self.is_shelter_request(), - position="start", - ), - ) - repository = self.get_repo() - commit = self.get_commit(repository) - report = self.get_report(commit) - instance = ReportResults.objects.filter(report=report).first() - if not instance: - instance = serializer.save( - report=report, state=ReportResults.ReportResultsStates.PENDING - ) - else: - instance.state = ReportResults.ReportResultsStates.PENDING - instance.save() - TaskService().create_report_results( - commitid=commit.commitid, - repoid=repository.repoid, - report_code=report.code, - ) - inc_counter( - API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_labels( - action="coverage", - endpoint="create_report_results", - request=self.request, - repository=repository, - is_shelter_request=self.is_shelter_request(), - position="end", - ), - ) - return instance + def get(self, request, *args, **kwargs): + return Response(EMPTY_RESPONSE) - def get_object(self) -> ReportResults: - repository = self.get_repo() - commit = self.get_commit(repository) - report = self.get_report(commit) - try: - report_results = ReportResults.objects.get(report=report) - except ReportResults.DoesNotExist: - log.info( - "Report Results not found", - extra=dict( - commit_sha=commit.commitid, - report_code=self.kwargs.get("report_code"), - ), - ) - raise ValidationError("Report Results not found") - return report_results + def post(self, request, *args, **kwargs): + return Response(EMPTY_RESPONSE, status=status.HTTP_201_CREATED)