Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions upload/tests/views/test_combined_upload.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
from unittest.mock import patch

from django.urls import reverse
from rest_framework.test import APIClient
from shared.django_apps.core.tests.factories import (
RepositoryFactory,
)


class TestCombinedUpload:
def test_get_repo(self, db):
repository = RepositoryFactory(
name="the_repo", author__username="codecov", author__service="github"
)
repository.save()
repo_slug = f"{repository.author.username}::::{repository.name}"
url = reverse(
"new_upload.combined_upload",
args=[repository.author.service, repo_slug],
)
client = APIClient()
client.credentials(HTTP_AUTHORIZATION="token " + repository.upload_token)
response = client.post(url, {}, format="json")
assert response.status_code == 400 # Bad request due to missing required fields
assert "commitid" in response.json()

@patch("services.task.TaskService.upload")
def test_get_repo_not_found(self, upload, db):
repository = RepositoryFactory(
name="the_repo", author__username="codecov", author__service="github"
)
repo_slug = "codecov::::wrong-repo-name"
url = reverse(
"new_upload.combined_upload",
args=[repository.author.service, repo_slug],
)
client = APIClient()
response = client.post(url, {}, format="json")
assert response.status_code == 401
assert response.json() == {"detail": "Not valid tokenless upload"}
assert not upload.called

def test_deactivated_repo(self, db):
repository = RepositoryFactory(
name="the_repo",
author__username="codecov",
author__service="github",
active=True,
activated=False,
)
repository.save()
repo_slug = f"{repository.author.username}::::{repository.name}"
url = reverse(
"new_upload.combined_upload",
args=[repository.author.service, repo_slug],
)
client = APIClient()
client.credentials(HTTP_AUTHORIZATION="token " + repository.upload_token)
response = client.post(url, {"commit_sha": "abc123"}, format="json")
assert response.status_code == 400
assert "This repository is deactivated" in str(response.json())

def test_combined_upload_with_errors(self, db):
repository = RepositoryFactory()
repo_slug = f"{repository.author.username}::::{repository.name}"
url = reverse(
"new_upload.combined_upload",
args=[repository.author.service, repo_slug],
)

client = APIClient()
client.credentials(HTTP_AUTHORIZATION="token " + repository.upload_token)

# Missing required fields
response = client.post(url, {}, format="json")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the same as test_get_repo, isn’t it?

I’m fine with checking multiple edge cases in a single test, though the other opinion to only test a single thing in one test is also fine.
Either way, you shouldn’t have the worst of both worlds :-D

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the tests are reused from test_uploads. I have added 2 more tests for the successful case, one with and one without shelter headers in the API request.

assert response.status_code == 400
assert "commitid" in response.json()

# Invalid flag format
response = client.post(
url, {"commit_sha": "abc123", "flags": "not-a-list"}, format="json"
)
assert response.status_code == 400
assert "flags" in response.json()
5 changes: 3 additions & 2 deletions upload/throttles.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Q
from rest_framework.exceptions import ValidationError
from rest_framework.throttling import BaseThrottle
from shared.reports.enums import UploadType
from shared.upload.utils import query_monthly_coverage_measurements
Expand Down Expand Up @@ -38,7 +39,7 @@ def allow_request(self, request, view):
)
return False
return True
except ObjectDoesNotExist:
except (ObjectDoesNotExist, ValidationError):
return True


Expand Down Expand Up @@ -71,5 +72,5 @@ def allow_request(self, request, view):
)
return False
return True
except ObjectDoesNotExist:
except (ObjectDoesNotExist, ValidationError):
return True
6 changes: 6 additions & 0 deletions upload/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.urls import path, re_path

from upload.views.bundle_analysis import BundleAnalysisView
from upload.views.combined_upload import CombinedUploadView
from upload.views.commits import CommitViews
from upload.views.empty_upload import EmptyUploadView
from upload.views.legacy import UploadDownloadHandler, UploadHandler
Expand Down Expand Up @@ -57,6 +58,11 @@
CommitViews.as_view(),
name="new_upload.commits",
),
path(
"<str:service>/<str:repo>/combined-upload",
CombinedUploadView.as_view(),
name="new_upload.combined_upload",
),
# This was getting in the way of the new endpoints, so I moved to the end
re_path(r"(?P<version>\w+)/?", UploadHandler.as_view(), name="upload-handler"),
]
171 changes: 171 additions & 0 deletions upload/views/combined_upload.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import logging

from django.conf import settings
from django.http import HttpRequest
from rest_framework import status
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 (
GitHubOIDCTokenAuthentication,
GlobalTokenAuthentication,
OrgLevelTokenAuthentication,
RepositoryLegacyTokenAuthentication,
TokenlessAuthentication,
UploadTokenRequiredAuthenticationCheck,
repo_auth_custom_exception_handler,
)
from services.archive import ArchiveService
from upload.helpers import generate_upload_prometheus_metrics_labels
from upload.metrics import API_UPLOAD_COUNTER
from upload.serializers import (
CommitReportSerializer,
CommitSerializer,
UploadSerializer,
)
from upload.throttles import UploadsPerCommitThrottle, UploadsPerWindowThrottle
from upload.views.base import GetterMixin
from upload.views.commits import CommitLogicMixin
from upload.views.reports import ReportLogicMixin
from upload.views.uploads import CanDoCoverageUploadsPermission, UploadLogicMixin

log = logging.getLogger(__name__)


class CombinedUploadView(
APIView, GetterMixin, CommitLogicMixin, ReportLogicMixin, UploadLogicMixin
):
permission_classes = [CanDoCoverageUploadsPermission]
authentication_classes = [
UploadTokenRequiredAuthenticationCheck,
GlobalTokenAuthentication,
OrgLevelTokenAuthentication,
GitHubOIDCTokenAuthentication,
RepositoryLegacyTokenAuthentication,
TokenlessAuthentication,
]
throttle_classes = [UploadsPerCommitThrottle, UploadsPerWindowThrottle]

def get_exception_handler(self):
return repo_auth_custom_exception_handler

def post(self, request: HttpRequest, *args, **kwargs) -> Response:
inc_counter(
API_UPLOAD_COUNTER,
labels=generate_upload_prometheus_metrics_labels(
action="coverage",
endpoint="combined_upload",
request=self.request,
is_shelter_request=self.is_shelter_request(),
position="start",
),
)

# Create commit
create_commit_data = dict(
commitid=request.data.get("commit_sha"),
parent_commit_id=request.data.get("parent_sha"),
pullid=request.data.get("pull_request_number"),
branch=request.data.get("branch"),
)
commit_serializer = CommitSerializer(data=create_commit_data)
if not commit_serializer.is_valid():
return Response(
commit_serializer.errors, status=status.HTTP_400_BAD_REQUEST
)
log.info(f"Creating commit for {commit_serializer.validated_data}")
repository = self.get_repo()

inc_counter(
API_UPLOAD_COUNTER,
labels=generate_upload_prometheus_metrics_labels(
action="coverage",
endpoint="combined_upload",
request=self.request,
is_shelter_request=self.is_shelter_request(),
position="create_commit",
),
)
commit = self.create_commit(commit_serializer, repository)

# Create report
commit_report_data = dict(
code=request.data.get("code"),
)
commit_report_serializer = CommitReportSerializer(data=commit_report_data)
if not commit_report_serializer.is_valid():
return Response(

Check warning on line 98 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L98

Added line #L98 was not covered by tests
commit_report_serializer.errors, status=status.HTTP_400_BAD_REQUEST
)

inc_counter(
API_UPLOAD_COUNTER,
labels=generate_upload_prometheus_metrics_labels(
action="coverage",
endpoint="combined_upload",
request=self.request,
is_shelter_request=self.is_shelter_request(),
position="create_report",
),
)
report = self.create_report(commit_report_serializer, repository, commit)

# Do upload
upload_data = dict(
ci_url=request.data.get("build_url"),
env=request.data.get("env_vars"),
flags=request.data.get("flags"),
ci_service=request.data.get("ci_service"),
job_code=request.data.get("job_code"),
name=request.data.get("name"),
)

upload_serializer = UploadSerializer(data=upload_data)
if not upload_serializer.is_valid():
return Response(
upload_serializer.errors, status=status.HTTP_400_BAD_REQUEST
)

inc_counter(

Check warning on line 130 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L130

Added line #L130 was not covered by tests
API_UPLOAD_COUNTER,
labels=generate_upload_prometheus_metrics_labels(
action="coverage",
endpoint="combined_upload",
request=self.request,
is_shelter_request=self.is_shelter_request(),
position="create_upload",
),
)
upload = self.create_upload(upload_serializer, repository, commit, report)

Check warning on line 140 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L140

Added line #L140 was not covered by tests

inc_counter(

Check warning on line 142 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L142

Added line #L142 was not covered by tests
API_UPLOAD_COUNTER,
labels=generate_upload_prometheus_metrics_labels(
action="coverage",
endpoint="combined_upload",
request=self.request,
is_shelter_request=self.is_shelter_request(),
position="end",
),
)

if upload:
commitid = upload.report.commit.commitid
upload_repository = upload.report.commit.repository
url = f"{settings.CODECOV_DASHBOARD_URL}/{upload_repository.author.service}/{upload_repository.author.username}/{upload_repository.name}/commit/{commitid}"
archive_service = ArchiveService(upload_repository)
raw_upload_location = archive_service.create_presigned_put(

Check warning on line 158 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L153-L158

Added lines #L153 - L158 were not covered by tests
upload.storage_path
)
return Response(

Check warning on line 161 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L161

Added line #L161 was not covered by tests
{
"url": url,
"raw_upload_location": raw_upload_location,
},
status=status.HTTP_201_CREATED,
)
else:
return Response(

Check warning on line 169 in upload/views/combined_upload.py

View check run for this annotation

Codecov Notifications / codecov/patch

upload/views/combined_upload.py#L169

Added line #L169 was not covered by tests
upload_serializer.errors, status=status.HTTP_400_BAD_REQUEST
)
13 changes: 9 additions & 4 deletions upload/views/commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@
log = logging.getLogger(__name__)


class CommitViews(ListCreateAPIView, GetterMixin):
class CommitLogicMixin:
def create_commit(self, serializer, repository):
validate_activated_repo(repository)
commit = serializer.save(repository=repository)
return commit


class CommitViews(ListCreateAPIView, GetterMixin, CommitLogicMixin):
serializer_class = CommitSerializer
permission_classes = [CanDoCoverageUploadsPermission]
authentication_classes = [
Expand Down Expand Up @@ -68,9 +75,7 @@ def perform_create(self, serializer):
),
)
repository = self.get_repo()
validate_activated_repo(repository)

commit = serializer.save(repository=repository)
commit = self.create_commit(serializer, repository)

log.info(
"Request to create new commit",
Expand Down
30 changes: 18 additions & 12 deletions upload/views/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,23 @@
log = logging.getLogger(__name__)


class ReportViews(ListCreateAPIView, GetterMixin):
class ReportLogicMixin:
def create_report(self, serializer, repository, commit):
code = serializer.validated_data.get("code")
if code == "default":
serializer.validated_data["code"] = None
instance, was_created = serializer.save(
commit_id=commit.id,
report_type=CommitReport.ReportType.COVERAGE,
)
if was_created:
TaskService().preprocess_upload(
repository.repoid, commit.commitid, instance.code
)
return instance


class ReportViews(ListCreateAPIView, GetterMixin, ReportLogicMixin):
serializer_class = CommitReportSerializer
permission_classes = [CanDoCoverageUploadsPermission]
authentication_classes = [
Expand Down Expand Up @@ -61,17 +77,7 @@ def perform_create(self, serializer):
"Request to create new report",
extra=dict(repo=repository.name, commit=commit.commitid),
)
code = serializer.validated_data.get("code")
if code == "default":
serializer.validated_data["code"] = None
instance, was_created = serializer.save(
commit_id=commit.id,
report_type=CommitReport.ReportType.COVERAGE,
)
if was_created:
TaskService().preprocess_upload(
repository.repoid, commit.commitid, instance.code
)
instance = self.create_report(serializer, repository, commit)

inc_counter(
API_UPLOAD_COUNTER,
Expand Down
Loading
Loading