Store base and head revision references (#1600)

Author: EvaBardou

backend/code_review_backend/issues/admin.py

@@ -22,8 +22,8 @@ class DiffInline(admin.TabularInline):
 
 
 class RevisionAdmin(admin.ModelAdmin):
-    list_display = ("id", "title", "bugzilla_id", "repository")
-    list_filter = ("repository",)
+    list_display = ("id", "title", "bugzilla_id", "base_repository", "head_repository")
+    list_filter = ("base_repository", "head_repository")
     inlines = (DiffInline,)
 
 

backend/code_review_backend/issues/api.py

@@ -109,7 +109,11 @@ def get_queryset(self):
             # Because of the perf. hit filter issues that are not older than today - 3 months.
             .filter(created__gte=date.today() - timedelta(days=90))
             .prefetch_related(
-                "issues", "revision", "revision__repository", "repository"
+                "issues",
+                "revision",
+                "revision__base_repository",
+                "revision__head_repository",
+                "repository",
             )
             .annotate(nb_issues=Count("issues"))
             .annotate(nb_errors=Count("issues", filter=Q(issues__level="error")))
@@ -127,7 +131,8 @@ def get_queryset(self):
         repository = self.request.query_params.get("repository")
         if repository is not None:
             diffs = diffs.filter(
-                Q(revision__repository__slug=repository)
+                Q(revision__base_repository__slug=repository)
+                | Q(revision__head_repository__slug=repository)
                 | Q(repository__slug=repository)
             )
 
@@ -211,14 +216,16 @@ def get_queryset(self):
         repo = self.kwargs["repository"]
 
         queryset = (
-            Issue.objects.filter(revisions__repository__slug=repo)
+            Issue.objects.filter(revisions__head_repository__slug=repo)
             .filter(analyzer=self.kwargs["analyzer"])
             .filter(analyzer_check=self.kwargs["check"])
             .prefetch_related(
                 "diffs__repository",
                 Prefetch(
                     "diffs__revision",
-                    queryset=Revision.objects.select_related("repository"),
+                    queryset=Revision.objects.select_related(
+                        "base_repository", "head_repository"
+                    ),
                 ),
             )
             .order_by("-created")
@@ -257,14 +264,14 @@ class IssueCheckStats(CachedView, generics.ListAPIView):
     def get_queryset(self):
         queryset = (
             Issue.objects.values(
-                "revisions__repository__slug", "analyzer", "analyzer_check"
+                "revisions__head_repository__slug", "analyzer", "analyzer_check"
             )
             # We want to count distinct issues because they can be referenced on multiple diffs
             .annotate(total=Count("id", distinct=True))
             .annotate(
                 publishable=Count("id", filter=Q(in_patch=True) | Q(level=LEVEL_ERROR))
             )
-            .distinct("revisions__repository__slug", "analyzer", "analyzer_check")
+            .distinct("revisions__head_repository__slug", "analyzer", "analyzer_check")
         )
 
         # Filter issues by date
@@ -281,7 +288,7 @@ def get_queryset(self):
         queryset = queryset.filter(revisions__created__gte=since).distinct()
 
         return queryset.order_by(
-            "-total", "revisions__repository__slug", "analyzer", "analyzer_check"
+            "-total", "revisions__head_repository__slug", "analyzer", "analyzer_check"
         )
 
 
@@ -311,7 +318,10 @@ def get_queryset(self):
         # Filter by repository
         repository = self.request.query_params.get("repository")
         if repository:
-            queryset = queryset.filter(diffs__revision__repository__slug=repository)
+            queryset = queryset.filter(
+                Q(diffs__revision__base_repository__slug=repository)
+                | Q(diffs__revision__head_repository__slug=repository)
+            )
 
         # Filter by analyzer
         analyzer = self.request.query_params.get("analyzer")
@@ -362,7 +372,7 @@ def get_queryset(self):
                 "invalid repo_slug path argument - No repository match this slug"
             )
         else:
-            qs = qs.filter(revisions__repository=repo)
+            qs = qs.filter(revisions__head_repository=repo)
 
         # Always filter by path when the parameter is set
         if path := self.request.query_params.get("path"):

backend/code_review_backend/issues/management/commands/cleanup_issues.py

@@ -31,7 +31,8 @@ def add_arguments(self, parser):
     def handle(self, *args, **options):
         clean_until = timezone.now() - timedelta(days=options["nb_days"])
         rev_to_delete = Revision.objects.filter(
-            repository__slug__in=["autoland", "mozilla-central"],
+            base_repository__slug__in=["autoland", "mozilla-central"],
+            head_repository__slug__in=["autoland", "mozilla-central"],
             created__lte=clean_until,
         )
 

backend/code_review_backend/issues/management/commands/load_issues.py

@@ -179,30 +179,39 @@ def load_local_reports(self):
     def build_revision_and_diff(self, data, task_id):
         """Build or retrieve a revision and diff in current repo from report's data"""
         try:
-            repository = Repository.objects.get(url=data["repository"])
+            head_repository = Repository.objects.get(url=data["repository"])
         except Repository.DoesNotExist:
             logger.warning(
                 f"No repository found with URL {data['repository']}, skipping."
             )
             return None, None
-        revision, _ = repository.revisions.get_or_create(
+
+        try:
+            base_repository = Repository.objects.get(url=data["target_repository"])
+        except Repository.DoesNotExist:
+            logger.warning(
+                f"No repository found with URL {data['target_repository']}, skipping."
+            )
+            return None, None
+
+        revision, _ = head_repository.head_revisions.get_or_create(
             id=data["id"],
             defaults={
                 "phid": data["phid"],
                 "title": data["title"],
                 "bugzilla_id": int(data["bugzilla_id"])
                 if data["bugzilla_id"]
                 else None,
+                "base_repository": base_repository,
             },
         )
         diff, _ = revision.diffs.get_or_create(
             id=data["diff_id"],
             defaults={
-                "repository": repository,
+                "repository": head_repository,
                 "phid": data["diff_phid"],
                 "review_task_id": task_id,
                 "mercurial_hash": data["mercurial_revision"],
-                "repository": repository,
             },
         )
         return revision, diff

backend/code_review_backend/issues/migrations/0008_revision_base_head_references.py

@@ -0,0 +1,132 @@
+# -*- coding: utf-8 -*-
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import django.db.models.deletion
+from django.db import migrations
+from django.db import models
+from django.db.models import Count
+from django.db.models import F
+from django.db.models import OuterRef
+from django.db.models import Q
+from django.db.models import Subquery
+
+
+def update_revisions(apps, schema_editor):
+    """
+    Detect the head repository depending on the current data:
+     - all revisions without any diff use their current head repo
+     - all revisions with a only diffs on autoland use it as their head repo
+    """
+    Repository = apps.get_model("issues", "Repository")
+    Revision = apps.get_model("issues", "Revision")
+    Diff = apps.get_model("issues", "Diff")
+
+    # No need to run the following lines if the db is empty (e.g. during tests)
+    if not Revision.objects.exists():
+        return
+
+    # all revisions without any diff get their head repo as revision.repository_id
+    Revision.objects.filter(diffs__isnull=True).update(
+        head_repository_id=F("base_repository_id")
+    )
+
+    # all revisions with only diff on autoland use it as their head repo
+    autoland = Repository.objects.get(slug="autoland")
+    others = Repository.objects.exclude(id=autoland.id)
+    Revision.objects.annotate(
+        other_diffs=Count("diffs", filter=Q(diffs__repository__in=others))
+    ).filter(other_diffs=0, diffs__repository_id=autoland.id).update(
+        head_repository_id=autoland.id
+    )
+
+    # all revisions without any diff excluding diffs with autoland repo
+    # get their head repo as diff.repository_id
+    rev_repos = (
+        Diff.objects.filter(revision_id=OuterRef("id"))
+        .exclude(repository_id=autoland.id)
+        .values("repository_id")[:1]
+    )
+    Revision.objects.filter(head_repository_id__isnull=True).update(
+        head_repository_id=Subquery(rev_repos)
+    )
+
+    # Check no revisions miss their head repos
+    assert not Revision.objects.filter(
+        head_repository__isnull=True
+    ).exists(), "Some revisions miss their head repos"
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("issues", "0007_issuelink_swap"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="revision",
+            old_name="repository",
+            new_name="base_repository",
+        ),
+        migrations.AlterField(
+            model_name="revision",
+            name="base_repository",
+            field=models.ForeignKey(
+                help_text="Target repository where the revision has been produced and will land in the end",
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="base_revisions",
+                to="issues.repository",
+            ),
+        ),
+        migrations.AddField(
+            model_name="revision",
+            name="head_repository",
+            # Make the field temporarily nullable so it can be set on existing revisions
+            field=models.ForeignKey(
+                to="issues.repository",
+                on_delete=django.db.models.deletion.CASCADE,
+                null=True,
+            ),
+        ),
+        # Revision mercurial changeset reference is set to null on existing revisions
+        migrations.AddField(
+            model_name="revision",
+            name="base_changeset",
+            field=models.CharField(
+                max_length=40,
+                help_text="Mercurial hash identifier on the base repository",
+                null=True,
+                blank=True,
+            ),
+            preserve_default=False,
+        ),
+        # Head revision is set to null on existing revisions
+        migrations.AddField(
+            model_name="revision",
+            name="head_changeset",
+            field=models.CharField(
+                help_text="Mercurial hash identifier on the analyze repository (only set for try pushes)",
+                max_length=40,
+                null=True,
+                blank=True,
+            ),
+        ),
+        migrations.RunPython(
+            update_revisions,
+            reverse_code=migrations.RunPython.noop,
+            elidable=True,
+        ),
+        # Make head_repository a required field
+        migrations.AlterField(
+            model_name="revision",
+            name="head_repository",
+            field=models.ForeignKey(
+                help_text="Repository where the revision is actually analyzed (e.g. Try for patches analysis)",
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="head_revisions",
+                to="issues.repository",
+                null=False,
+            ),
+        ),
+    ]

backend/code_review_backend/issues/models.py

@@ -42,9 +42,30 @@ def __str__(self):
 
 
 class Revision(PhabricatorModel):
-    # The target repository where the revision will land in the end
-    repository = models.ForeignKey(
-        Repository, related_name="revisions", on_delete=models.CASCADE
+    base_repository = models.ForeignKey(
+        Repository,
+        related_name="base_revisions",
+        on_delete=models.CASCADE,
+        help_text="Target repository where the revision has been produced and will land in the end",
+    )
+    head_repository = models.ForeignKey(
+        Repository,
+        related_name="head_revisions",
+        on_delete=models.CASCADE,
+        help_text="Repository where the revision is actually analyzed (e.g. Try for patches analysis)",
+    )
+
+    base_changeset = models.CharField(
+        max_length=40,
+        null=True,
+        blank=True,
+        help_text="Mercurial hash identifier on the base repository",
+    )
+    head_changeset = models.CharField(
+        max_length=40,
+        null=True,
+        blank=True,
+        help_text="Mercurial hash identifier on the analyze repository (only set for try pushes)",
     )
 
     title = models.CharField(max_length=250)

backend/code_review_backend/issues/serializers.py

@@ -28,7 +28,10 @@ class RevisionSerializer(serializers.ModelSerializer):
     Serialize a Revision in a Repository
     """
 
-    repository = serializers.SlugRelatedField(
+    base_repository = serializers.SlugRelatedField(
+        queryset=Repository.objects.all(), slug_field="url"
+    )
+    head_repository = serializers.SlugRelatedField(
         queryset=Repository.objects.all(), slug_field="url"
     )
     diffs_url = serializers.HyperlinkedIdentityField(
@@ -43,7 +46,10 @@ class Meta:
         model = Revision
         fields = (
             "id",
-            "repository",
+            "base_repository",
+            "head_repository",
+            "base_changeset",
+            "head_changeset",
             "phid",
             "title",
             "bugzilla_id",
@@ -58,14 +64,26 @@ class RevisionLightSerializer(serializers.ModelSerializer):
     Serialize a Revision in a Diff light serializer
     """
 
-    repository = serializers.SlugRelatedField(
+    base_repository = serializers.SlugRelatedField(
+        queryset=Repository.objects.all(), slug_field="url"
+    )
+    head_repository = serializers.SlugRelatedField(
         queryset=Repository.objects.all(), slug_field="url"
     )
     phabricator_url = serializers.URLField(read_only=True)
 
     class Meta:
         model = Revision
-        fields = ("id", "repository", "title", "bugzilla_id", "phabricator_url")
+        fields = (
+            "id",
+            "base_repository",
+            "head_repository",
+            "base_changeset",
+            "head_changeset",
+            "title",
+            "bugzilla_id",
+            "phabricator_url",
+        )
 
 
 class DiffSerializer(serializers.ModelSerializer):
@@ -224,7 +242,7 @@ class IssueCheckStatsSerializer(serializers.Serializer):
     """
 
     # The view aggregates issues depending on their reference to a repository (via IssueLink M2M)
-    repository = serializers.SlugField(source="revisions__repository__slug")
+    repository = serializers.SlugField(source="revisions__head_repository__slug")
     analyzer = serializers.CharField()
     check = serializers.CharField(source="analyzer_check")
     total = serializers.IntegerField()