fix: fetch accurate branch head (#352)

* fix: fetch accurate branch head

This commit is a fix for the recent issue where
branch heads were all set to the same value each
time they were updated due to a bug in the Branch
django model.

This fix creates a helper function that checks for the 2
remaining innacurate commit shas that a large number of
branches were set to. If the current branch head is set to
one of those shas, we update the branch head by fetching
the latest commit on that branch from the db.

Note that we must use a raw sql query here to update the
branch object because using the django ORM to update it
would lead to the same issue we are trying to fix.

* fix: fix get or update branch head commits query

* fix: add to test and refresh branches from db

Signed-off-by: joseph-sentry <[email protected]>

* fix: fix get_or_update_branch_head to not return None

Signed-off-by: joseph-sentry <[email protected]>

api/internal/coverage/views.py

@@ -8,6 +8,7 @@
 from api.shared.permissions import RepositoryArtifactPermissions
 from api.shared.report.serializers import TreeSerializer
 from services.path import ReportPaths
+from utils.temp_branch_fix import get_or_update_branch_head
 
 
 class CoverageViewSet(viewsets.ViewSet, RepoPropertyMixin):
@@ -23,7 +24,9 @@ def get_object(self):
                     f"The branch '{branch_name}' in not in our records. Please provide a valid branch name.",
                     404,
                 )
-            commit_sha = branch.head
+            commit_sha = get_or_update_branch_head(
+                self.repo.commits, branch, self.repo.repoid
+            )
 
         commit = self.repo.commits.filter(commitid=commit_sha).first()
         if commit is None:

api/public/v2/branch/serializers.py

@@ -2,6 +2,7 @@
 
 from api.public.v2.commit.serializers import CommitDetailSerializer
 from core.models import Branch, Commit
+from utils.temp_branch_fix import get_or_update_branch_head
 
 
 class BranchSerializer(serializers.ModelSerializer):
@@ -19,6 +20,8 @@ class BranchDetailSerializer(BranchSerializer):
     )
 
     def get_head_commit(self, branch: Branch) -> CommitDetailSerializer:
+        _ = get_or_update_branch_head(Commit.objects, branch, branch.repository_id)
+        branch.refresh_from_db()
         commit = (
             Commit.objects.filter(
                 repository_id=branch.repository_id, commitid=branch.head

api/shared/mixins.py

@@ -1,6 +1,7 @@
 from typing import Optional
 
 from django.conf import settings
+from django.db import connection
 from django.http import Http404
 from django.shortcuts import get_object_or_404
 from django.utils.functional import cached_property
@@ -19,6 +20,7 @@
 from codecov_auth.models import Owner, Service
 from core.models import Commit, Repository
 from utils.services import get_long_service_name
+from utils.temp_branch_fix import get_or_update_branch_head
 
 
 class OwnerPropertyMixin:
@@ -50,7 +52,12 @@ def get_commit(self, commit_sha: Optional[str] = None) -> Commit:
                     f"The branch '{branch_name}' in not in our records. Please provide a valid branch name.",
                     404,
                 )
-            commit_sha = branch.head
+
+            commit_sha = get_or_update_branch_head(
+                self.repo.commits,
+                branch,
+                self.repo.repoid,
+            )
 
         commit = self.repo.commits.filter(commitid=commit_sha).first()
         if commit is None:

graphql_api/types/branch/branch.py

@@ -4,17 +4,20 @@
 
 from core.models import Branch, Commit
 from graphql_api.dataloader.commit import CommitLoader
+from utils.temp_branch_fix import get_or_update_branch_head
 
 branch_bindable = ObjectType("Branch")
 
 
 @branch_bindable.field("headSha")
 def resolve_head_sha(branch: Branch, info) -> str:
-    return branch.head
+    head = get_or_update_branch_head(Commit.objects, branch, branch.repository_id)
+    return head
 
 
 @branch_bindable.field("head")
 def resolve_head_commit(branch: Branch, info) -> Optional[Commit]:
-    if branch.head:
+    head = get_or_update_branch_head(Commit.objects, branch, branch.repository_id)
+    if head:
         loader = CommitLoader.loader(info, branch.repository_id)
-        return loader.load(branch.head)
+        return loader.load(head)

graphs/tests/test_badge_handler.py

@@ -683,12 +683,37 @@ def test_branch_badge(self):
             "p": 0,
             "s": 1,
         }
+        commit_3_totals = {
+            "C": 0,
+            "M": 0,
+            "N": 0,
+            "b": 0,
+            "c": "80.00000",
+            "d": 0,
+            "diff": [1, 2, 1, 1, 0, "50.00000", 0, 0, 0, 0, 0, 0, 0],
+            "f": 3,
+            "h": 17,
+            "m": 3,
+            "n": 20,
+            "p": 0,
+            "s": 1,
+        }
         commit_2 = CommitFactory(
-            repository=repo, author=gh_owner, totals=commit_2_totals
+            commitid="81c2b4fa3ae9ef615c8f740c5cba95d9851f9ae8s",
+            repository=repo,
+            author=gh_owner,
+            totals=commit_2_totals,
         )
         branch_2 = BranchFactory(
             repository=repo, name="branch1", head=commit_2.commitid
         )
+        commit_3 = CommitFactory(
+            commitid="a1c2b4fa3ae9ef615c8f740c5cba95d9851f9ae8s",
+            repository=repo,
+            author=gh_owner,
+            totals=commit_3_totals,
+            branch="branch1",
+        )
         # test default precision
         response = self._get_branch(
             kwargs={
@@ -710,14 +735,14 @@ def test_branch_badge(self):
                 </mask>
                 <g mask="url(#a)">
                     <path fill="#555" d="M0 0h73v20H0z" />
-                    <path fill="#8aca02" d="M73 0h39v20H73z" />
+                    <path fill="#efa41b" d="M73 0h39v20H73z" />
                     <path fill="url(#b)" d="M0 0h112v20H0z" />
                 </g>
                 <g fill="#fff" text-anchor="middle" font-family="DejaVu Sans,Verdana,Geneva,sans-serif" font-size="11">
                     <text x="46" y="15" fill="#010101" fill-opacity=".3">codecov</text>
                     <text x="46" y="14">codecov</text>
-                    <text x="93" y="15" fill="#010101" fill-opacity=".3">95%</text>
-                    <text x="93" y="14">95%</text>
+                    <text x="93" y="15" fill="#010101" fill-opacity=".3">80%</text>
+                    <text x="93" y="14">80%</text>
                 </g>
                 <svg viewBox="120 -8 60 60">
                     <path d="M23.013 0C10.333.009.01 10.22 0 22.762v.058l3.914 2.275.053-.036a11.291 11.291 0 0 1 8.352-1.767 10.911 10.911 0 0 1 5.5 2.726l.673.624.38-.828c.368-.802.793-1.556 1.264-2.24.19-.276.398-.554.637-.851l.393-.49-.484-.404a16.08 16.08 0 0 0-7.453-3.466 16.482 16.482 0 0 0-7.705.449C7.386 10.683 14.56 5.016 23.03 5.01c4.779 0 9.272 1.84 12.651 5.18 2.41 2.382 4.069 5.35 4.807 8.591a16.53 16.53 0 0 0-4.792-.723l-.292-.002a16.707 16.707 0 0 0-1.902.14l-.08.012c-.28.037-.524.074-.748.115-.11.019-.218.041-.327.063-.257.052-.51.108-.75.169l-.265.067a16.39 16.39 0 0 0-.926.276l-.056.018c-.682.23-1.36.511-2.016.838l-.052.026c-.29.145-.584.305-.899.49l-.069.04a15.596 15.596 0 0 0-4.061 3.466l-.145.175c-.29.36-.521.666-.723.96-.17.247-.34.513-.552.864l-.116.199c-.17.292-.32.57-.449.824l-.03.057a16.116 16.116 0 0 0-.843 2.029l-.034.102a15.65 15.65 0 0 0-.786 5.174l.003.214a21.523 21.523 0 0 0 .04.754c.009.119.02.237.032.355.014.145.032.29.049.432l.01.08c.01.067.017.133.026.197.034.242.074.48.119.72.463 2.419 1.62 4.836 3.345 6.99l.078.098.08-.095c.688-.81 2.395-3.38 2.539-4.922l.003-.029-.014-.025a10.727 10.727 0 0 1-1.226-4.956c0-5.76 4.545-10.544 10.343-10.89l.381-.014a11.403 11.403 0 0 1 6.651 1.957l.054.036 3.862-2.237.05-.03v-.056c.006-6.08-2.384-11.793-6.729-16.089C34.932 2.361 29.16 0 23.013 0" fill="#F01F7A" fill-rule="evenodd"/>
@@ -728,6 +753,8 @@ def test_branch_badge(self):
         expected_badge = [line.strip() for line in expected_badge.split("\n")]
         assert expected_badge == badge
         assert response.status_code == status.HTTP_200_OK
+        branch_2.refresh_from_db()
+        assert branch_2.head == "a1c2b4fa3ae9ef615c8f740c5cba95d9851f9ae8s"
 
     def test_badge_with_100_coverage(self):
         gh_owner = OwnerFactory(service="github")

graphs/views.py

@@ -1,6 +1,7 @@
 import logging
 
 from django.core.exceptions import ObjectDoesNotExist
+from django.db import connection
 from django.http import Http404
 from rest_framework import exceptions
 from rest_framework.exceptions import NotFound
@@ -13,6 +14,7 @@
 from api.shared.mixins import RepoPropertyMixin
 from core.models import Branch, Pull
 from graphs.settings import settings
+from utils.temp_branch_fix import get_or_update_branch_head
 
 from .helpers.badge import format_coverage_precision, get_badge
 from .helpers.graphs import icicle, sunburst, tree
@@ -42,7 +44,6 @@ def select_renderer(self, request, renderers, format_suffix):
 
 
 class BadgeHandler(APIView, RepoPropertyMixin, GraphBadgeAPIMixin):
-
     content_negotiation_class = IgnoreClientContentNegotiation
 
     permission_classes = [AllowAny]
@@ -100,7 +101,9 @@ def get_coverage(self):
             )
             return None, coverage_range
         try:
-            commit = repo.commits.get(commitid=branch.head)
+            _ = get_or_update_branch_head(repo.commits, branch, repo.repoid)
+            branch.refresh_from_db()
+            commit = repo.commits.filter(commitid=branch.head).first()
         except ObjectDoesNotExist:
             # if commit does not exist return None coverage
             log.warning("Commit not found", extra=dict(commit=branch.head))
@@ -246,6 +249,8 @@ def get_commit(self):
             if branch is None:
                 return None
 
+            _ = get_or_update_branch_head(repo.commits, branch, repo.repoid)
+            branch.refresh_from_db()
             commit = repo.commits.filter(commitid=branch.head).first()
 
         return commit

utils/temp_branch_fix.py

@@ -0,0 +1,38 @@
+from django.db import connection
+
+
+def get_or_update_branch_head(
+    commits,
+    branch,
+    repoid,
+):
+    # there was a bug that set a large number of branch heads to these two shas, so we are rectifying that issue here
+    if (
+        branch.head == "81c2b4fa3ae9ef615c8f740c5cba95d9851f9ae8s"
+        or branch.head == "9587100eacc554aa9c03422e28b269c551dc1a72"
+    ):
+        commit = (
+            commits.filter(branch=branch.name, repository_id=repoid)
+            .defer("_report")
+            .order_by("-updatestamp")
+            .first()
+        )
+
+        if commit is None:
+            return branch.head
+
+        # using this raw sql because the current branches table does not allow for updating based on repoid
+        # it only updates based on branch name which means if we were to use the django orm to update this
+        # branch.head, all branches with the same name as the one we are trying to update would have their head
+        # updated to the value of this ones head
+        with connection.cursor() as cursor:
+            cursor.execute(
+                "UPDATE branches SET head = %s WHERE branches.repoid = %s AND branches.branch = %s",
+                [commit.commitid, repoid, branch.name],
+            )
+
+        commit_sha = commit.commitid
+        return commit_sha
+
+    else:
+        return branch.head