Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all 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
12 changes: 11 additions & 1 deletion graphs/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django.urls import re_path

from .views import BadgeHandler, GraphHandler
from .views import BadgeHandler, BundleBadgeHandler, GraphHandler

urlpatterns = [
re_path(
Expand All @@ -13,6 +13,16 @@
BadgeHandler.as_view(),
name="default-badge",
),
re_path(
"branch/(?P<branch>.+)/(graph|graphs)/bundle/(?P<bundle>.+)/badge.(?P<ext>[^/]+)",
BundleBadgeHandler.as_view(),
name="branch-bundle-badge",
),
re_path(
"(graph|graphs)/bundle/(?P<bundle>.+)/badge.(?P<ext>[^/]+)",
BundleBadgeHandler.as_view(),
name="default-bundle-badge",
Comment on lines +16 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

The URL patterns for bundle badges are similar to existing patterns. Consider using Django's path() instead of re_path() for improved readability and maintainability. Also, the regex patterns could be simplified and constants could be used for repeated patterns.

),
re_path(
"pull/(?P<pullid>[^/]+)/(graph|graphs)/(?P<graph>tree|icicle|sunburst|commits).(?P<ext>[^/]+)",
GraphHandler.as_view(),
Expand Down
88 changes: 87 additions & 1 deletion graphs/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,15 @@
from api.shared.mixins import RepoPropertyMixin
from core.models import Branch, Pull
from graphs.settings import settings
from services.bundle_analysis import load_report
from services.components import commit_components

from .helpers.badge import format_coverage_precision, get_badge
from .helpers.badge import (
format_bundle_bytes,
format_coverage_precision,
get_badge,
get_bundle_badge,
)
from .helpers.graphs import icicle, sunburst, tree
from .mixins import GraphBadgeAPIMixin

Expand Down Expand Up @@ -193,6 +199,86 @@
return filtered_report.totals.coverage


class BundleBadgeHandler(APIView, RepoPropertyMixin, GraphBadgeAPIMixin):
content_negotiation_class = IgnoreClientContentNegotiation

permission_classes = [AllowAny]

extensions = ["svg", "txt"]
precisions = ["0", "1", "2"]
filename = "bundle-badge"

def get_object(self, request, *args, **kwargs):
# Validate precision query param
precision = self.request.query_params.get("precision", "2")
if precision not in self.precisions:
raise NotFound("Bundle size precision should be one of [ 0 || 1 || 2 ]")
Comment on lines +209 to +215
Copy link
Contributor

Choose a reason for hiding this comment

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

The error handling for precision validation could be moved to a separate validation method, following the Single Responsibility Principle. Also, consider using Django's built-in validators or form validation for this.

precision = int(precision)

Check warning on line 216 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L213-L216

Added lines #L213 - L216 were not covered by tests

bundle_size_bytes = self.get_bundle_size()

Check warning on line 218 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L218

Added line #L218 was not covered by tests

if self.kwargs.get("ext") == "txt":
return (

Check warning on line 221 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L220-L221

Added lines #L220 - L221 were not covered by tests
"unknown"
if bundle_size_bytes is None
else format_bundle_bytes(bundle_size_bytes, precision)
)

return get_bundle_badge(bundle_size_bytes, precision)

Check warning on line 227 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L227

Added line #L227 was not covered by tests

def get_bundle_size(self) -> int | None:
try:
repo = self.repo
except Http404:
log.warning("Repo not found", extra=dict(repo=self.kwargs.get("repo_name")))
return None

Check warning on line 234 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L230-L234

Added lines #L230 - L234 were not covered by tests

if repo.private and repo.image_token != self.request.query_params.get("token"):
log.warning(

Check warning on line 237 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L236-L237

Added lines #L236 - L237 were not covered by tests
"Token provided does not match repo's image token",
extra=dict(repo=repo),
)
return None

Check warning on line 241 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L241

Added line #L241 was not covered by tests

branch_name = self.kwargs.get("branch") or repo.branch
branch = Branch.objects.filter(

Check warning on line 244 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L243-L244

Added lines #L243 - L244 were not covered by tests
name=branch_name, repository_id=repo.repoid
).first()

if branch is None:
log.warning(

Check warning on line 249 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L248-L249

Added lines #L248 - L249 were not covered by tests
"Branch not found", extra=dict(branch_name=branch_name, repo=repo)
)
return None

Check warning on line 252 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L252

Added line #L252 was not covered by tests

try:
commit: Commit = repo.commits.filter(commitid=branch.head).first()
except ObjectDoesNotExist:
log.warning("Commit not found", extra=dict(commit=branch.head))
return None

Check warning on line 258 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L254-L258

Added lines #L254 - L258 were not covered by tests

commit_bundles = load_report(commit)

Check warning on line 260 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L260

Added line #L260 was not covered by tests

if commit_bundles is None:
log.warning(

Check warning on line 263 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L262-L263

Added lines #L262 - L263 were not covered by tests
"Bundle analysis report not found for commit",
extra=dict(commit=branch.head),
)
return None

Check warning on line 267 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L267

Added line #L267 was not covered by tests

bundle_name = str(self.kwargs.get("bundle"))
bundle = commit_bundles.bundle_report(bundle_name)

Check warning on line 270 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L269-L270

Added lines #L269 - L270 were not covered by tests

if bundle is None:
log.warning(

Check warning on line 273 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L272-L273

Added lines #L272 - L273 were not covered by tests
"Bundle with provided name not found for commit",
extra=dict(commit=branch.head),
)
return None

Check warning on line 277 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L277

Added line #L277 was not covered by tests
Comment on lines +228 to +277
Copy link
Contributor

Choose a reason for hiding this comment

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

The get_bundle_size method is quite long and handles multiple responsibilities (permission checking, repo validation, branch lookup, commit lookup, bundle loading). Consider breaking this down into smaller, more focused methods to improve readability and maintainability. This would also make it easier to test individual pieces of functionality.

Comment on lines +228 to +277
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding caching for bundle size calculations to improve performance, especially for frequently accessed badges. This could be implemented using Django's cache framework.


return bundle.total_size()

Check warning on line 279 in graphs/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

graphs/views.py#L279

Added line #L279 was not covered by tests


class GraphHandler(APIView, RepoPropertyMixin, GraphBadgeAPIMixin):
permission_classes = [AllowAny]

Expand Down
Loading