From cfb130ffbd617f70d1d7f80fece922e8d350abb9 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Tue, 8 Oct 2019 15:36:53 +0200 Subject: [PATCH 01/25] Use Django as backend --- .gitignore | 1 + .isort.cfg | 2 +- backend/code_review_backend/__init__.py | 0 backend/code_review_backend/app/__init__.py | 0 backend/code_review_backend/app/settings.py | 119 ++++++++++++++++++++ backend/code_review_backend/app/urls.py | 24 ++++ backend/code_review_backend/app/wsgi.py | 21 ++++ backend/manage.py | 26 +++++ 8 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 backend/code_review_backend/__init__.py create mode 100644 backend/code_review_backend/app/__init__.py create mode 100644 backend/code_review_backend/app/settings.py create mode 100644 backend/code_review_backend/app/urls.py create mode 100644 backend/code_review_backend/app/wsgi.py create mode 100755 backend/manage.py diff --git a/.gitignore b/.gitignore index 2e7fa2ee3..eec28d86b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .vscode/ *.pyc *.egg-info +*.sqlite3 diff --git a/.isort.cfg b/.isort.cfg index 41aa15cec..4bdca7e65 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -1,6 +1,6 @@ [settings] known_first_party = code_review_bot,code_review_tools,code_review_events,conftest -known_third_party = influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml +known_third_party = django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml force_single_line = True default_section=FIRSTPARTY line_length=159 diff --git a/backend/code_review_backend/__init__.py b/backend/code_review_backend/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/app/__init__.py b/backend/code_review_backend/app/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py new file mode 100644 index 000000000..afc988b05 --- /dev/null +++ b/backend/code_review_backend/app/settings.py @@ -0,0 +1,119 @@ +# -*- 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/. + +""" +Django settings for backend project. + +Generated by 'django-admin startproject' using Django 2.2.6. + +For more information on this file, see +https://docs.djangoproject.com/en/2.2/topics/settings/ + +For the full list of settings and their values, see +https://docs.djangoproject.com/en/2.2/ref/settings/ +""" + +import os + +# Build paths inside the project like this: os.path.join(BASE_DIR, ...) +BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +# Quick-start development settings - unsuitable for production +# See https://docs.djangoproject.com/en/2.2/howto/deployment/checklist/ + +# SECURITY WARNING: keep the secret key used in production secret! +SECRET_KEY = "t!+s!@x5p!85x19q83jufr#95_z0fv7$!u5z*c&gi!%hr3^w+r" + +# SECURITY WARNING: don't run with debug turned on in production! +DEBUG = True + +ALLOWED_HOSTS = [] + + +# Application definition + +INSTALLED_APPS = [ + "django.contrib.admin", + "django.contrib.auth", + "django.contrib.contenttypes", + "django.contrib.sessions", + "django.contrib.messages", + "django.contrib.staticfiles", +] + +MIDDLEWARE = [ + "django.middleware.security.SecurityMiddleware", + "django.contrib.sessions.middleware.SessionMiddleware", + "django.middleware.common.CommonMiddleware", + "django.middleware.csrf.CsrfViewMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + "django.contrib.messages.middleware.MessageMiddleware", + "django.middleware.clickjacking.XFrameOptionsMiddleware", +] + +ROOT_URLCONF = "code_review_backend.app.urls" + +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "DIRS": [], + "APP_DIRS": True, + "OPTIONS": { + "context_processors": [ + "django.template.context_processors.debug", + "django.template.context_processors.request", + "django.contrib.auth.context_processors.auth", + "django.contrib.messages.context_processors.messages", + ] + }, + } +] + +WSGI_APPLICATION = "code_review_backend.app.wsgi.application" + + +# Database +# https://docs.djangoproject.com/en/2.2/ref/settings/#databases + +DATABASES = { + "default": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": os.path.normpath(os.path.join(BASE_DIR, "../db.sqlite3")), + } +} + + +# Password validation +# https://docs.djangoproject.com/en/2.2/ref/settings/#auth-password-validators + +AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator" + }, + {"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator"}, + {"NAME": "django.contrib.auth.password_validation.CommonPasswordValidator"}, + {"NAME": "django.contrib.auth.password_validation.NumericPasswordValidator"}, +] + + +# Internationalization +# https://docs.djangoproject.com/en/2.2/topics/i18n/ + +LANGUAGE_CODE = "en-us" + +TIME_ZONE = "UTC" + +USE_I18N = True + +USE_L10N = True + +USE_TZ = True + + +# Static files (CSS, JavaScript, Images) +# https://docs.djangoproject.com/en/2.2/howto/static-files/ + +STATIC_URL = "/static/" diff --git a/backend/code_review_backend/app/urls.py b/backend/code_review_backend/app/urls.py new file mode 100644 index 000000000..ef2b68c6e --- /dev/null +++ b/backend/code_review_backend/app/urls.py @@ -0,0 +1,24 @@ +# -*- 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/. + +"""backend URL Configuration + +The `urlpatterns` list routes URLs to views. For more information please see: + https://docs.djangoproject.com/en/2.2/topics/http/urls/ +Examples: +Function views + 1. Add an import: from my_app import views + 2. Add a URL to urlpatterns: path('', views.home, name='home') +Class-based views + 1. Add an import: from other_app.views import Home + 2. Add a URL to urlpatterns: path('', Home.as_view(), name='home') +Including another URLconf + 1. Import the include() function: from django.urls import include, path + 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) +""" +from django.contrib import admin +from django.urls import path + +urlpatterns = [path("admin/", admin.site.urls)] diff --git a/backend/code_review_backend/app/wsgi.py b/backend/code_review_backend/app/wsgi.py new file mode 100644 index 000000000..ca3a145ea --- /dev/null +++ b/backend/code_review_backend/app/wsgi.py @@ -0,0 +1,21 @@ +# -*- 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/. + +""" +WSGI config for backend project. + +It exposes the WSGI callable as a module-level variable named ``application``. + +For more information on this file, see +https://docs.djangoproject.com/en/2.2/howto/deployment/wsgi/ +""" + +import os + +from django.core.wsgi import get_wsgi_application + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "code_review_backend.app.settings") + +application = get_wsgi_application() diff --git a/backend/manage.py b/backend/manage.py new file mode 100755 index 000000000..fac678717 --- /dev/null +++ b/backend/manage.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python +# -*- 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/. + +"""Django's command-line utility for administrative tasks.""" +import os +import sys + + +def main(): + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "code_review_backend.app.settings") + try: + from django.core.management import execute_from_command_line + except ImportError as exc: + raise ImportError( + "Couldn't import Django. Are you sure it's installed and " + "available on your PYTHONPATH environment variable? Did you " + "forget to activate a virtual environment?" + ) from exc + execute_from_command_line(sys.argv) + + +if __name__ == "__main__": + main() From 3225948dfc4c5596c7a2e5aee69708ad723c309e Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Tue, 8 Oct 2019 15:37:24 +0200 Subject: [PATCH 02/25] Reqs --- backend/requirements.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 backend/requirements.txt diff --git a/backend/requirements.txt b/backend/requirements.txt new file mode 100644 index 000000000..bb9455509 --- /dev/null +++ b/backend/requirements.txt @@ -0,0 +1,3 @@ +Django==2.2.6 +pytz==2019.3 +sqlparse==0.3.0 From 2a5f30c81dfd8c31582b6bb4a434605d543153c3 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Tue, 8 Oct 2019 16:25:13 +0200 Subject: [PATCH 03/25] Base mode hierarchy --- .isort.cfg | 2 +- backend/code_review_backend/app/settings.py | 1 + backend/code_review_backend/app/urls.py | 15 ------- .../code_review_backend/issues/__init__.py | 0 backend/code_review_backend/issues/admin.py | 17 +++++++ backend/code_review_backend/issues/apps.py | 10 +++++ .../issues/migrations/__init__.py | 0 backend/code_review_backend/issues/models.py | 44 +++++++++++++++++++ 8 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 backend/code_review_backend/issues/__init__.py create mode 100644 backend/code_review_backend/issues/admin.py create mode 100644 backend/code_review_backend/issues/apps.py create mode 100644 backend/code_review_backend/issues/migrations/__init__.py create mode 100644 backend/code_review_backend/issues/models.py diff --git a/.isort.cfg b/.isort.cfg index 4bdca7e65..3ff76f92e 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -1,5 +1,5 @@ [settings] -known_first_party = code_review_bot,code_review_tools,code_review_events,conftest +known_first_party = code_review_backend,code_review_bot,code_review_tools,code_review_events,conftest known_third_party = django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml force_single_line = True default_section=FIRSTPARTY diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index afc988b05..a166ea323 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -42,6 +42,7 @@ "django.contrib.sessions", "django.contrib.messages", "django.contrib.staticfiles", + "code_review_backend.issues", ] MIDDLEWARE = [ diff --git a/backend/code_review_backend/app/urls.py b/backend/code_review_backend/app/urls.py index ef2b68c6e..e6b0a988d 100644 --- a/backend/code_review_backend/app/urls.py +++ b/backend/code_review_backend/app/urls.py @@ -3,21 +3,6 @@ # 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/. -"""backend URL Configuration - -The `urlpatterns` list routes URLs to views. For more information please see: - https://docs.djangoproject.com/en/2.2/topics/http/urls/ -Examples: -Function views - 1. Add an import: from my_app import views - 2. Add a URL to urlpatterns: path('', views.home, name='home') -Class-based views - 1. Add an import: from other_app.views import Home - 2. Add a URL to urlpatterns: path('', Home.as_view(), name='home') -Including another URLconf - 1. Import the include() function: from django.urls import include, path - 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) -""" from django.contrib import admin from django.urls import path diff --git a/backend/code_review_backend/issues/__init__.py b/backend/code_review_backend/issues/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/issues/admin.py b/backend/code_review_backend/issues/admin.py new file mode 100644 index 000000000..eb967649b --- /dev/null +++ b/backend/code_review_backend/issues/admin.py @@ -0,0 +1,17 @@ +# -*- 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/. + +from django.contrib import admin + +from code_review_backend.issues.models import Issue + + +class IssueAdmin(admin.ModelAdmin): + """ + Manage stored issues + """ + + +admin.site.register(Issue, IssueAdmin) diff --git a/backend/code_review_backend/issues/apps.py b/backend/code_review_backend/issues/apps.py new file mode 100644 index 000000000..fe6b5d320 --- /dev/null +++ b/backend/code_review_backend/issues/apps.py @@ -0,0 +1,10 @@ +# -*- 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/. + +from django.apps import AppConfig + + +class IssuesConfig(AppConfig): + name = "issues" diff --git a/backend/code_review_backend/issues/migrations/__init__.py b/backend/code_review_backend/issues/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py new file mode 100644 index 000000000..1c4dc78a0 --- /dev/null +++ b/backend/code_review_backend/issues/models.py @@ -0,0 +1,44 @@ +# -*- 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 uuid + +from django.db import models + + +class PhabricatorModel(models.Model): + id = models.PositiveIntegerField(primary_key=True) + phid = models.CharField(max_length=40, unique=True) + + class Meta: + abstract = True + + +class Repository(PhabricatorModel): + + url = models.URLField(unique=True) + slug = models.SlugField(unique=True) + + +class Revision(PhabricatorModel): + repository = models.ForeignKey( + Repository, related_name="revisions", on_delete=models.CASCADE + ) + + title = models.CharField(max_length=250) + + +class Diff(PhabricatorModel): + revision = models.ForeignKey( + Revision, related_name="diffs", on_delete=models.CASCADE + ) + + +class Issue(models.Model): + """An issue detected on a Phabricator patch""" + + id = models.UUIDField(primary_key=True, default=uuid.uuid4) + + diff = models.ForeignKey(Diff, related_name="issues", on_delete=models.CASCADE) From 519b65458651dc11fd7cae1e174055b83fb87944 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Wed, 9 Oct 2019 12:32:56 +0200 Subject: [PATCH 04/25] Add models --- .../issues/migrations/0001_initial.py | 108 ++++++++++++++++++ backend/code_review_backend/issues/models.py | 23 +++- 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 backend/code_review_backend/issues/migrations/0001_initial.py diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py new file mode 100644 index 000000000..38e4ac9bc --- /dev/null +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -0,0 +1,108 @@ +# -*- 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/. + +# Generated by Django 2.2.6 on 2019-10-09 10:27 + +import uuid + +import django.db.models.deletion +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="Diff", + fields=[ + ("id", models.PositiveIntegerField(primary_key=True, serialize=False)), + ("phid", models.CharField(max_length=40, unique=True)), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ("review_task_id", models.CharField(max_length=20, unique=True)), + ], + options={"abstract": False}, + ), + migrations.CreateModel( + name="Repository", + fields=[ + ("id", models.PositiveIntegerField(primary_key=True, serialize=False)), + ("phid", models.CharField(max_length=40, unique=True)), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ("url", models.URLField(unique=True)), + ("slug", models.SlugField(unique=True)), + ], + options={"abstract": False}, + ), + migrations.CreateModel( + name="Revision", + fields=[ + ("id", models.PositiveIntegerField(primary_key=True, serialize=False)), + ("phid", models.CharField(max_length=40, unique=True)), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ("title", models.CharField(max_length=250)), + ("bugzilla_id", models.PositiveIntegerField()), + ( + "repository", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="revisions", + to="issues.Repository", + ), + ), + ], + options={"abstract": False}, + ), + migrations.CreateModel( + name="Issue", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, primary_key=True, serialize=False + ), + ), + ("path", models.CharField(max_length=250)), + ("line", models.PositiveIntegerField()), + ("nb_lines", models.PositiveIntegerField()), + ("char", models.PositiveIntegerField(null=True)), + ( + "level", + models.CharField( + choices=[("warning", "Warning"), ("error", "Error")], + max_length=20, + ), + ), + ("check", models.CharField(max_length=250)), + ("message", models.TextField()), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ( + "diff", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="issues", + to="issues.Diff", + ), + ), + ], + ), + migrations.AddField( + model_name="diff", + name="revision", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="diffs", + to="issues.Revision", + ), + ), + ] diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 1c4dc78a0..251b48e0e 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -7,11 +7,18 @@ from django.db import models +LEVEL_WARNING = "warning" +LEVEL_ERROR = "error" +ISSUE_LEVELS = ((LEVEL_WARNING, "Warning"), (LEVEL_ERROR, "Error")) + class PhabricatorModel(models.Model): id = models.PositiveIntegerField(primary_key=True) phid = models.CharField(max_length=40, unique=True) + created = models.DateTimeField(auto_now_add=True) + updated = models.DateTimeField(auto_now=True) + class Meta: abstract = True @@ -28,6 +35,7 @@ class Revision(PhabricatorModel): ) title = models.CharField(max_length=250) + bugzilla_id = models.PositiveIntegerField() class Diff(PhabricatorModel): @@ -35,10 +43,23 @@ class Diff(PhabricatorModel): Revision, related_name="diffs", on_delete=models.CASCADE ) + review_task_id = models.CharField(max_length=20, unique=True) + class Issue(models.Model): """An issue detected on a Phabricator patch""" id = models.UUIDField(primary_key=True, default=uuid.uuid4) - diff = models.ForeignKey(Diff, related_name="issues", on_delete=models.CASCADE) + + # Raw issue data + path = models.CharField(max_length=250) + line = models.PositiveIntegerField() + nb_lines = models.PositiveIntegerField() + char = models.PositiveIntegerField(null=True) + level = models.CharField(max_length=20, choices=ISSUE_LEVELS) + check = models.CharField(max_length=250) + message = models.TextField() + + created = models.DateTimeField(auto_now_add=True) + updated = models.DateTimeField(auto_now=True) From 472613ebc33130782dd585b56b5606a880549809 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Wed, 9 Oct 2019 15:19:30 +0200 Subject: [PATCH 05/25] Load all issues from Taskcluster reports --- backend/code_review_backend/issues/admin.py | 25 ++- .../issues/management/__init__.py | 0 .../issues/management/commands/__init__.py | 0 .../issues/management/commands/load_issues.py | 148 ++++++++++++++++++ .../issues/migrations/0001_initial.py | 13 +- backend/code_review_backend/issues/models.py | 21 ++- backend/requirements.txt | 2 + 7 files changed, 196 insertions(+), 13 deletions(-) create mode 100644 backend/code_review_backend/issues/management/__init__.py create mode 100644 backend/code_review_backend/issues/management/commands/__init__.py create mode 100644 backend/code_review_backend/issues/management/commands/load_issues.py diff --git a/backend/code_review_backend/issues/admin.py b/backend/code_review_backend/issues/admin.py index eb967649b..7907a00e5 100644 --- a/backend/code_review_backend/issues/admin.py +++ b/backend/code_review_backend/issues/admin.py @@ -5,13 +5,32 @@ from django.contrib import admin +from code_review_backend.issues.models import Diff from code_review_backend.issues.models import Issue +from code_review_backend.issues.models import Repository +from code_review_backend.issues.models import Revision + + +class RepositoryAdmin(admin.ModelAdmin): + list_display = ("slug", "url") + + +class DiffInline(admin.TabularInline): + # Read only inline + model = Diff + readonly_fields = ("id", "phid", "review_task_id") + + +class RevisionAdmin(admin.ModelAdmin): + list_display = ("id", "title", "bugzilla_id", "repository") + list_filter = ("repository",) + inlines = (DiffInline,) class IssueAdmin(admin.ModelAdmin): - """ - Manage stored issues - """ + list_display = ("id", "path", "line", "level", "analyzer", "check", "diff") +admin.site.register(Repository, RepositoryAdmin) +admin.site.register(Revision, RevisionAdmin) admin.site.register(Issue, IssueAdmin) diff --git a/backend/code_review_backend/issues/management/__init__.py b/backend/code_review_backend/issues/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/issues/management/commands/__init__.py b/backend/code_review_backend/issues/management/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py new file mode 100644 index 000000000..f1159a560 --- /dev/null +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -0,0 +1,148 @@ +# -*- 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 json +import logging +import os +import tempfile + +import taskcluster +from django.core.management.base import BaseCommand +from django.core.management.base import CommandError +from django.db import transaction + +from code_review_backend.issues.models import Issue +from code_review_backend.issues.models import Repository + +logger = logging.getLogger(__name__) + +INDEX_PATH = "project.relman.production.code-review.phabricator.diff" + + +class Command(BaseCommand): + help = "Load issues from remote taskcluster reports" + + def add_arguments(self, parser): + parser.add_argument( + "--repository", + default="mozilla-central", + help="Repository slug to use for new revisions", + ) + parser.add_argument( + "--offline", + action="store_true", + default=False, + help="Use only previously downloaded reports", + ) + + def handle(self, *args, **options): + # Load repository + try: + self.repository = Repository.objects.get(slug=options["repository"]) + except Repository.DoesNotExist: + raise CommandError(f"Missing repository {options['repository']}") + + # Setup cache dir + self.cache_dir = os.path.join(tempfile.gettempdir(), "code-review-reports") + os.makedirs(self.cache_dir, exist_ok=True) + + # Load available tasks from Taskcluster or already downloaded + tasks = self.load_local_reports() if options["offline"] else self.load_tasks() + + for task_id, report in tasks: + + # Build revision & diff + revision, diff = self.build_hierarchy(report["revision"], task_id) + + with transaction.atomic(): + # Remove all issues from diff + diff.issues.all().delete() + + # Build all issues for that diff, in a single DB call + out = Issue.objects.bulk_create( + Issue( + diff=diff, + path=i["path"], + line=i["line"], + nb_lines=i.get("nb_lines", 1), + char=i.get("char"), + level=i.get("level", "warning"), + check=i.get("kind") or i.get("check"), + message=i.get("message"), + analyzer=i["analyzer"], + ) + for i in report["issues"] + ) + print(task_id, len(out)) + + def load_tasks(self, chunk=200): + # Direct unauthenticated usage + index = taskcluster.Index({"rootUrl": "https://taskcluster.net"}) + queue = taskcluster.Queue({"rootUrl": "https://taskcluster.net"}) + + token = None + while True: + + query = {"limit": chunk} + if token is not None: + query["continuationToken"] = token + data = index.listTasks(INDEX_PATH, query=query) + + for task in data["tasks"]: + + if not task["data"].get("issues"): + continue + + # Lookup artifact in cache + path = os.path.join(self.cache_dir, task["taskId"]) + if os.path.exists(path): + artifact = json.load(open(path)) + + else: + + # Download the task report + logging.info(f"Download task {task['taskId']}") + try: + artifact = queue.getLatestArtifact( + task["taskId"], "public/results/report.json" + ) + except taskcluster.exceptions.TaskclusterRestFailure as e: + if e.status_code == 404: + logging.info(f"Missing artifact") + continue + raise + + # Store artifact in cache + with open(path, "w") as f: + json.dump(artifact, f, sort_keys=True, indent=4) + + yield task["taskId"], artifact + + token = data.get("continuationToken") + if token is None: + break + + def load_local_reports(self): + for task_id in os.listdir(self.cache_dir): + report = json.load(open(os.path.join(self.cache_dir, task_id))) + yield task_id, report + + def build_hierarchy(self, data, task_id): + """Build or retrieve a revision and diff in current repo from report's data""" + revision, _ = self.repository.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, + }, + ) + diff, _ = revision.diffs.get_or_create( + id=data["diff_id"], + defaults={"phid": data["diff_phid"], "review_task_id": task_id}, + ) + return revision, diff diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index 38e4ac9bc..72eb2a634 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-09 10:27 +# Generated by Django 2.2.6 on 2019-10-09 13:18 import uuid @@ -26,7 +26,7 @@ class Migration(migrations.Migration): ("phid", models.CharField(max_length=40, unique=True)), ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), - ("review_task_id", models.CharField(max_length=20, unique=True)), + ("review_task_id", models.CharField(max_length=30, unique=True)), ], options={"abstract": False}, ), @@ -40,7 +40,7 @@ class Migration(migrations.Migration): ("url", models.URLField(unique=True)), ("slug", models.SlugField(unique=True)), ], - options={"abstract": False}, + options={"verbose_name_plural": "repositories"}, ), migrations.CreateModel( name="Revision", @@ -50,7 +50,7 @@ class Migration(migrations.Migration): ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), ("title", models.CharField(max_length=250)), - ("bugzilla_id", models.PositiveIntegerField()), + ("bugzilla_id", models.PositiveIntegerField(null=True)), ( "repository", models.ForeignKey( @@ -82,8 +82,9 @@ class Migration(migrations.Migration): max_length=20, ), ), - ("check", models.CharField(max_length=250)), - ("message", models.TextField()), + ("check", models.CharField(max_length=250, null=True)), + ("message", models.TextField(null=True)), + ("analyzer", models.CharField(max_length=50)), ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), ( diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 251b48e0e..8a46b8976 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -28,6 +28,12 @@ class Repository(PhabricatorModel): url = models.URLField(unique=True) slug = models.SlugField(unique=True) + class Meta: + verbose_name_plural = "repositories" + + def __str__(self): + return self.slug + class Revision(PhabricatorModel): repository = models.ForeignKey( @@ -35,7 +41,10 @@ class Revision(PhabricatorModel): ) title = models.CharField(max_length=250) - bugzilla_id = models.PositiveIntegerField() + bugzilla_id = models.PositiveIntegerField(null=True) + + def __str__(self): + return f"D{self.id} - {self.title}" class Diff(PhabricatorModel): @@ -43,7 +52,10 @@ class Diff(PhabricatorModel): Revision, related_name="diffs", on_delete=models.CASCADE ) - review_task_id = models.CharField(max_length=20, unique=True) + review_task_id = models.CharField(max_length=30, unique=True) + + def __str__(self): + return f"Diff {self.id}" class Issue(models.Model): @@ -58,8 +70,9 @@ class Issue(models.Model): nb_lines = models.PositiveIntegerField() char = models.PositiveIntegerField(null=True) level = models.CharField(max_length=20, choices=ISSUE_LEVELS) - check = models.CharField(max_length=250) - message = models.TextField() + check = models.CharField(max_length=250, null=True) + message = models.TextField(null=True) + analyzer = models.CharField(max_length=50) created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) diff --git a/backend/requirements.txt b/backend/requirements.txt index bb9455509..965be50ae 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,3 +1,5 @@ Django==2.2.6 pytz==2019.3 sqlparse==0.3.0 +taskcluster==19.0.0 +taskcluster-urls==11.0.0 From 37e1fb98aa8300269c3970c63c1d920850c42f36 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Wed, 9 Oct 2019 15:25:20 +0200 Subject: [PATCH 06/25] Naming --- backend/code_review_backend/issues/admin.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/code_review_backend/issues/admin.py b/backend/code_review_backend/issues/admin.py index 7907a00e5..ef10e520e 100644 --- a/backend/code_review_backend/issues/admin.py +++ b/backend/code_review_backend/issues/admin.py @@ -34,3 +34,6 @@ class IssueAdmin(admin.ModelAdmin): admin.site.register(Repository, RepositoryAdmin) admin.site.register(Revision, RevisionAdmin) admin.site.register(Issue, IssueAdmin) + +# Naming +admin.site.site_header = "Mozilla Code Review Backend" From e369965449b09ba5b8499eaced73f2cbe9f9c81d Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 10 Oct 2019 10:07:30 +0200 Subject: [PATCH 07/25] Load mercurial rev --- backend/code_review_backend/issues/admin.py | 2 +- .../issues/management/commands/load_issues.py | 48 ++++++++++++++----- .../issues/migrations/0001_initial.py | 3 +- backend/code_review_backend/issues/models.py | 6 +++ 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/backend/code_review_backend/issues/admin.py b/backend/code_review_backend/issues/admin.py index ef10e520e..b91f6c1e6 100644 --- a/backend/code_review_backend/issues/admin.py +++ b/backend/code_review_backend/issues/admin.py @@ -18,7 +18,7 @@ class RepositoryAdmin(admin.ModelAdmin): class DiffInline(admin.TabularInline): # Read only inline model = Diff - readonly_fields = ("id", "phid", "review_task_id") + readonly_fields = ("id", "mercurial", "phid", "review_task_id") class RevisionAdmin(admin.ModelAdmin): diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index f1159a560..0c9b13134 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -25,11 +25,6 @@ class Command(BaseCommand): help = "Load issues from remote taskcluster reports" def add_arguments(self, parser): - parser.add_argument( - "--repository", - default="mozilla-central", - help="Repository slug to use for new revisions", - ) parser.add_argument( "--offline", action="store_true", @@ -38,11 +33,12 @@ def add_arguments(self, parser): ) def handle(self, *args, **options): - # Load repository - try: - self.repository = Repository.objects.get(slug=options["repository"]) - except Repository.DoesNotExist: - raise CommandError(f"Missing repository {options['repository']}") + # Check repositories + for repo in ("mozilla-central", "nss"): + try: + Repository.objects.get(slug=repo) + except Repository.DoesNotExist: + raise CommandError(f"Missing repository {repo}") # Setup cache dir self.cache_dir = os.path.join(tempfile.gettempdir(), "code-review-reports") @@ -114,6 +110,29 @@ def load_tasks(self, chunk=200): continue raise + # Load the task definition of the main task in group + # to get the mercurial revision of the patch + # It should always be the decision task that has mercurial refs + decision_task = queue.task(task["data"]["try_group_id"]) + decision_env = decision_task["payload"].get("env", {}) + if "GECKO_HEAD_REV" in decision_env: + # mozilla-central rev + repo_slug = "mozilla-central" + repo_rev = decision_env["GECKO_HEAD_REV"] + + elif "NSS_HEAD_REVISION" in decision_env: + # nss rev + repo_slug = "nss" + repo_rev = decision_env["NSS_HEAD_REVISION"] + else: + raise Exception( + f"Missing gecko rev in task {task['data']['try_group_id']}" + ) + + # Add missing data in artifact + artifact["revision"]["mercurial"] = repo_rev + artifact["revision"]["repository"] = repo_slug + # Store artifact in cache with open(path, "w") as f: json.dump(artifact, f, sort_keys=True, indent=4) @@ -131,7 +150,8 @@ def load_local_reports(self): def build_hierarchy(self, data, task_id): """Build or retrieve a revision and diff in current repo from report's data""" - revision, _ = self.repository.revisions.get_or_create( + repository = Repository.objects.get(slug=data["repository"]) + revision, _ = repository.revisions.get_or_create( id=data["id"], defaults={ "phid": data["phid"], @@ -143,6 +163,10 @@ def build_hierarchy(self, data, task_id): ) diff, _ = revision.diffs.get_or_create( id=data["diff_id"], - defaults={"phid": data["diff_phid"], "review_task_id": task_id}, + defaults={ + "phid": data["diff_phid"], + "review_task_id": task_id, + "mercurial": data["mercurial"], + }, ) return revision, diff diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index 72eb2a634..9d8b68732 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-09 13:18 +# Generated by Django 2.2.6 on 2019-10-10 07:12 import uuid @@ -27,6 +27,7 @@ class Migration(migrations.Migration): ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), ("review_task_id", models.CharField(max_length=30, unique=True)), + ("mercurial", models.CharField(max_length=40)), ], options={"abstract": False}, ), diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 8a46b8976..23c93ee16 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -53,6 +53,7 @@ class Diff(PhabricatorModel): ) review_task_id = models.CharField(max_length=30, unique=True) + mercurial = models.CharField(max_length=40) def __str__(self): return f"Diff {self.id}" @@ -76,3 +77,8 @@ class Issue(models.Model): created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) + + def build_hash(self): + + # First we need the affected lines + pass From dcf5c1425e4d827812604e82d431c27921dd90e0 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 10 Oct 2019 11:28:52 +0200 Subject: [PATCH 08/25] Build hashes --- .gitignore | 3 +- backend/code_review_backend/app/settings.py | 6 +- .../issues/management/commands/load_issues.py | 8 ++- .../issues/migrations/0001_initial.py | 3 +- backend/code_review_backend/issues/models.py | 63 ++++++++++++++++++- 5 files changed, 76 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index eec28d86b..fa99d4b36 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .vscode/ *.pyc *.egg-info -*.sqlite3 +*.sqlite* +backend/hgmo diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index a166ea323..e594c6bb3 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -19,6 +19,7 @@ # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +ROOT_DIR = os.path.dirname(BASE_DIR) # Quick-start development settings - unsuitable for production @@ -82,7 +83,7 @@ DATABASES = { "default": { "ENGINE": "django.db.backends.sqlite3", - "NAME": os.path.normpath(os.path.join(BASE_DIR, "../db.sqlite3")), + "NAME": os.path.join(ROOT_DIR, "db.sqlite3"), } } @@ -118,3 +119,6 @@ # https://docs.djangoproject.com/en/2.2/howto/static-files/ STATIC_URL = "/static/" + +# Remote HGMO local cache +HGMO_CACHE = os.environ.get("HGMO_CACHE", os.path.join(ROOT_DIR, "hgmo")) diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index 0c9b13134..cf979a069 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -57,7 +57,7 @@ def handle(self, *args, **options): diff.issues.all().delete() # Build all issues for that diff, in a single DB call - out = Issue.objects.bulk_create( + issues = Issue.objects.bulk_create( Issue( diff=diff, path=i["path"], @@ -71,7 +71,11 @@ def handle(self, *args, **options): ) for i in report["issues"] ) - print(task_id, len(out)) + print(task_id, len(issues)) + + for issue in issues: + issue.hash = issue.build_hash() + issue.save() def load_tasks(self, chunk=200): # Direct unauthenticated usage diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index 9d8b68732..822f02685 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-10 07:12 +# Generated by Django 2.2.6 on 2019-10-10 09:25 import uuid @@ -86,6 +86,7 @@ class Migration(migrations.Migration): ("check", models.CharField(max_length=250, null=True)), ("message", models.TextField(null=True)), ("analyzer", models.CharField(max_length=50)), + ("hash", models.CharField(max_length=32, null=True)), ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), ( diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 23c93ee16..e6d79b2c4 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -3,8 +3,13 @@ # 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 hashlib +import os +import urllib.parse import uuid +import requests +from django.conf import settings from django.db import models LEVEL_WARNING = "warning" @@ -58,6 +63,34 @@ class Diff(PhabricatorModel): def __str__(self): return f"Diff {self.id}" + def load_file(self, path): + """ + Load a file content at patch's revision + Using remote HGMO + """ + # Check in hgmo cache first + local_path = os.path.join( + settings.HGMO_CACHE, self.revision.repository.slug, path, self.mercurial + ) + os.makedirs(os.path.dirname(local_path), exist_ok=True) + if os.path.exists(local_path): + return open(local_path).read() + + # Retrieve remote file + url = urllib.parse.urljoin( + self.revision.repository.url, f"raw-file/{self.mercurial}/{path}" + ) + print("Downloading", url) + response = requests.get(url) + response.raise_for_status() + + # Store in cache + content = response.content.decode("utf-8") + with open(local_path, "w") as f: + f.write(content) + + return content + class Issue(models.Model): """An issue detected on a Phabricator patch""" @@ -75,10 +108,36 @@ class Issue(models.Model): message = models.TextField(null=True) analyzer = models.CharField(max_length=50) + # Calculated hash identifying issue + hash = models.CharField(max_length=32, null=True) + created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) + def get_lines(self): + """Load all the lines affected by the issue""" + file_content = self.diff.load_file(self.path) + file_lines = file_content.splitlines() + start = self.line - 1 # file_lines start at 0, not 1 + return file_lines[start : start + self.nb_lines] # noqa E203 + def build_hash(self): - # First we need the affected lines - pass + # Build raw content: + # 1. lines affected by patch + # 2. without any spaces around each line + try: + raw_content = "".join([line.strip() for line in self.get_lines()]) + except requests.exceptions.HTTPError as e: + if e.response.status_code == 404: + return + raise + + # Build hash payload using issue data + # excluding file position information (lines & char) + payload = ":".join( + [self.path, self.analyzer, self.check or "", raw_content] + ).encode("utf-8") + + # Finally build the MD5 hash + return hashlib.md5(payload).hexdigest() From f6db19b7440d2bcbe5c8d320acf9497214c55bd2 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 09:23:35 +0200 Subject: [PATCH 09/25] Readme + envs --- backend/README.md | 35 +++++++++++++++++++ .../issues/management/commands/load_issues.py | 27 ++++++++++---- backend/fixtures/repositories.json | 24 +++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 backend/README.md create mode 100644 backend/fixtures/repositories.json diff --git a/backend/README.md b/backend/README.md new file mode 100644 index 000000000..df9ede4dd --- /dev/null +++ b/backend/README.md @@ -0,0 +1,35 @@ +# Code Review Backend + +## Developer setup + +``` +mkvirtualenv -p /usr/bin/python3 code-review-backend +cd backend +pip install -r requirements.txt +./manage migrate +./manage createsuperuser +./manage runserver +./manage loaddata fixtures/repositories.json +``` + +At this point, you can log into http://127.0.0.1:8000/admin/ with the credentials you mentioned during the `createsuperuser` step. + +## Load existing issues + +To load remote issues from production (default configuration): + +``` +./manage load_issues +``` + +To load already retrieved issues + +``` +./manage load_issues --offline +``` + +To load from testing + +``` +./manage load_issues --environment=testing +``` diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index cf979a069..45d48f310 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -18,7 +18,7 @@ logger = logging.getLogger(__name__) -INDEX_PATH = "project.relman.production.code-review.phabricator.diff" +INDEX_PATH = "project.relman.{environment}.code-review.phabricator.diff" class Command(BaseCommand): @@ -31,6 +31,13 @@ def add_arguments(self, parser): default=False, help="Use only previously downloaded reports", ) + parser.add_argument( + "-e, --environment", + dest="environment", + default="production", + choices=("production", "testing"), + help="Specify the environment to load issues from", + ) def handle(self, *args, **options): # Check repositories @@ -41,11 +48,17 @@ def handle(self, *args, **options): raise CommandError(f"Missing repository {repo}") # Setup cache dir - self.cache_dir = os.path.join(tempfile.gettempdir(), "code-review-reports") + self.cache_dir = os.path.join( + tempfile.gettempdir(), "code-review-reports", options["environment"] + ) os.makedirs(self.cache_dir, exist_ok=True) # Load available tasks from Taskcluster or already downloaded - tasks = self.load_local_reports() if options["offline"] else self.load_tasks() + tasks = ( + self.load_local_reports() + if options["offline"] + else self.load_tasks(options["environment"]) + ) for task_id, report in tasks: @@ -77,7 +90,7 @@ def handle(self, *args, **options): issue.hash = issue.build_hash() issue.save() - def load_tasks(self, chunk=200): + def load_tasks(self, environment, chunk=200): # Direct unauthenticated usage index = taskcluster.Index({"rootUrl": "https://taskcluster.net"}) queue = taskcluster.Queue({"rootUrl": "https://taskcluster.net"}) @@ -88,7 +101,9 @@ def load_tasks(self, chunk=200): query = {"limit": chunk} if token is not None: query["continuationToken"] = token - data = index.listTasks(INDEX_PATH, query=query) + data = index.listTasks( + INDEX_PATH.format(environment=environment), query=query + ) for task in data["tasks"]: @@ -147,7 +162,7 @@ def load_tasks(self, chunk=200): if token is None: break - def load_local_reports(self): + def load_local_reports(self, environment): for task_id in os.listdir(self.cache_dir): report = json.load(open(os.path.join(self.cache_dir, task_id))) yield task_id, report diff --git a/backend/fixtures/repositories.json b/backend/fixtures/repositories.json new file mode 100644 index 000000000..6e337137a --- /dev/null +++ b/backend/fixtures/repositories.json @@ -0,0 +1,24 @@ +[ +{ + "model": "issues.repository", + "pk": 1, + "fields": { + "phid": "PHID-REPO-saax4qdxlbbhahhp2kg5", + "created": "2019-10-17T07:17:06.396Z", + "updated": "2019-10-17T07:17:06.396Z", + "url": "https://hg.mozilla.org/mozilla-central/", + "slug": "mozilla-central" + } +}, +{ + "model": "issues.repository", + "pk": 8, + "fields": { + "phid": "PHID-REPO-3lrloqw4qf6fluy2a5ni", + "created": "2019-10-17T07:17:32.970Z", + "updated": "2019-10-17T07:17:32.970Z", + "url": "https://hg.mozilla.org/nss", + "slug": "nss" + } +} +] From 804e86579591ff143d257afe0de954c70386b9c3 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 09:41:57 +0200 Subject: [PATCH 10/25] Remove build hash, rely on remote data --- backend/code_review_backend/app/settings.py | 3 - backend/code_review_backend/issues/admin.py | 1 + .../issues/management/commands/load_issues.py | 38 +++-------- backend/code_review_backend/issues/models.py | 63 +------------------ 4 files changed, 12 insertions(+), 93 deletions(-) diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index e594c6bb3..be293154b 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -119,6 +119,3 @@ # https://docs.djangoproject.com/en/2.2/howto/static-files/ STATIC_URL = "/static/" - -# Remote HGMO local cache -HGMO_CACHE = os.environ.get("HGMO_CACHE", os.path.join(ROOT_DIR, "hgmo")) diff --git a/backend/code_review_backend/issues/admin.py b/backend/code_review_backend/issues/admin.py index b91f6c1e6..7ab73d94a 100644 --- a/backend/code_review_backend/issues/admin.py +++ b/backend/code_review_backend/issues/admin.py @@ -28,6 +28,7 @@ class RevisionAdmin(admin.ModelAdmin): class IssueAdmin(admin.ModelAdmin): + list_filter = ("analyzer",) list_display = ("id", "path", "line", "level", "analyzer", "check", "diff") diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index 45d48f310..4176dfb97 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -81,15 +81,12 @@ def handle(self, *args, **options): check=i.get("kind") or i.get("check"), message=i.get("message"), analyzer=i["analyzer"], + hash=i["hash"], ) for i in report["issues"] ) print(task_id, len(issues)) - for issue in issues: - issue.hash = issue.build_hash() - issue.save() - def load_tasks(self, environment, chunk=200): # Direct unauthenticated usage index = taskcluster.Index({"rootUrl": "https://taskcluster.net"}) @@ -129,28 +126,13 @@ def load_tasks(self, environment, chunk=200): continue raise - # Load the task definition of the main task in group - # to get the mercurial revision of the patch - # It should always be the decision task that has mercurial refs - decision_task = queue.task(task["data"]["try_group_id"]) - decision_env = decision_task["payload"].get("env", {}) - if "GECKO_HEAD_REV" in decision_env: - # mozilla-central rev - repo_slug = "mozilla-central" - repo_rev = decision_env["GECKO_HEAD_REV"] - - elif "NSS_HEAD_REVISION" in decision_env: - # nss rev - repo_slug = "nss" - repo_rev = decision_env["NSS_HEAD_REVISION"] - else: - raise Exception( - f"Missing gecko rev in task {task['data']['try_group_id']}" - ) - - # Add missing data in artifact - artifact["revision"]["mercurial"] = repo_rev - artifact["revision"]["repository"] = repo_slug + # Check the artifact has repositories & revision + revision = artifact["revision"] + assert "repository" in revision, "Missing repository" + assert "target_repository" in revision, "Missing target_repository" + assert ( + "mercurial_revision" in revision + ), "Missing mercurial_revision" # Store artifact in cache with open(path, "w") as f: @@ -169,7 +151,7 @@ def load_local_reports(self, environment): def build_hierarchy(self, data, task_id): """Build or retrieve a revision and diff in current repo from report's data""" - repository = Repository.objects.get(slug=data["repository"]) + repository = Repository.objects.get(slug=data["target_repository"]) revision, _ = repository.revisions.get_or_create( id=data["id"], defaults={ @@ -185,7 +167,7 @@ def build_hierarchy(self, data, task_id): defaults={ "phid": data["diff_phid"], "review_task_id": task_id, - "mercurial": data["mercurial"], + "mercurial": data["mercurial_revision"], }, ) return revision, diff diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index e6d79b2c4..6367695bc 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -3,13 +3,8 @@ # 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 hashlib -import os -import urllib.parse import uuid -import requests -from django.conf import settings from django.db import models LEVEL_WARNING = "warning" @@ -63,34 +58,6 @@ class Diff(PhabricatorModel): def __str__(self): return f"Diff {self.id}" - def load_file(self, path): - """ - Load a file content at patch's revision - Using remote HGMO - """ - # Check in hgmo cache first - local_path = os.path.join( - settings.HGMO_CACHE, self.revision.repository.slug, path, self.mercurial - ) - os.makedirs(os.path.dirname(local_path), exist_ok=True) - if os.path.exists(local_path): - return open(local_path).read() - - # Retrieve remote file - url = urllib.parse.urljoin( - self.revision.repository.url, f"raw-file/{self.mercurial}/{path}" - ) - print("Downloading", url) - response = requests.get(url) - response.raise_for_status() - - # Store in cache - content = response.content.decode("utf-8") - with open(local_path, "w") as f: - f.write(content) - - return content - class Issue(models.Model): """An issue detected on a Phabricator patch""" @@ -109,35 +76,7 @@ class Issue(models.Model): analyzer = models.CharField(max_length=50) # Calculated hash identifying issue - hash = models.CharField(max_length=32, null=True) + hash = models.CharField(max_length=32) created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) - - def get_lines(self): - """Load all the lines affected by the issue""" - file_content = self.diff.load_file(self.path) - file_lines = file_content.splitlines() - start = self.line - 1 # file_lines start at 0, not 1 - return file_lines[start : start + self.nb_lines] # noqa E203 - - def build_hash(self): - - # Build raw content: - # 1. lines affected by patch - # 2. without any spaces around each line - try: - raw_content = "".join([line.strip() for line in self.get_lines()]) - except requests.exceptions.HTTPError as e: - if e.response.status_code == 404: - return - raise - - # Build hash payload using issue data - # excluding file position information (lines & char) - payload = ":".join( - [self.path, self.analyzer, self.check or "", raw_content] - ).encode("utf-8") - - # Finally build the MD5 hash - return hashlib.md5(payload).hexdigest() From 5cb24281787f444976e5a3119073701f64a395b4 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 09:49:30 +0200 Subject: [PATCH 11/25] Add try_url --- .../code_review_backend/issues/migrations/0001_initial.py | 7 ++++--- backend/code_review_backend/issues/models.py | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index 822f02685..ebbccb9f1 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-10 09:25 +# Generated by Django 2.2.6 on 2019-10-17 07:48 import uuid @@ -38,8 +38,9 @@ class Migration(migrations.Migration): ("phid", models.CharField(max_length=40, unique=True)), ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), - ("url", models.URLField(unique=True)), ("slug", models.SlugField(unique=True)), + ("url", models.URLField(unique=True)), + ("try_url", models.URLField(blank=True, null=True)), ], options={"verbose_name_plural": "repositories"}, ), @@ -86,7 +87,7 @@ class Migration(migrations.Migration): ("check", models.CharField(max_length=250, null=True)), ("message", models.TextField(null=True)), ("analyzer", models.CharField(max_length=50)), - ("hash", models.CharField(max_length=32, null=True)), + ("hash", models.CharField(max_length=32)), ("created", models.DateTimeField(auto_now_add=True)), ("updated", models.DateTimeField(auto_now=True)), ( diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 6367695bc..359d69fa1 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -25,8 +25,9 @@ class Meta: class Repository(PhabricatorModel): - url = models.URLField(unique=True) slug = models.SlugField(unique=True) + url = models.URLField(unique=True) + try_url = models.URLField(null=True, blank=True) class Meta: verbose_name_plural = "repositories" From fc3c60c636f0080c97dc35faf62599a737abfa3f Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 09:54:41 +0200 Subject: [PATCH 12/25] Fixes for repo try --- .../issues/management/commands/load_issues.py | 6 +++--- backend/fixtures/repositories.json | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index 4176dfb97..96c279316 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -32,8 +32,8 @@ def add_arguments(self, parser): help="Use only previously downloaded reports", ) parser.add_argument( - "-e, --environment", - dest="environment", + "-e", + "--environment", default="production", choices=("production", "testing"), help="Specify the environment to load issues from", @@ -144,7 +144,7 @@ def load_tasks(self, environment, chunk=200): if token is None: break - def load_local_reports(self, environment): + def load_local_reports(self): for task_id in os.listdir(self.cache_dir): report = json.load(open(os.path.join(self.cache_dir, task_id))) yield task_id, report diff --git a/backend/fixtures/repositories.json b/backend/fixtures/repositories.json index 6e337137a..5525b9a6a 100644 --- a/backend/fixtures/repositories.json +++ b/backend/fixtures/repositories.json @@ -6,7 +6,8 @@ "phid": "PHID-REPO-saax4qdxlbbhahhp2kg5", "created": "2019-10-17T07:17:06.396Z", "updated": "2019-10-17T07:17:06.396Z", - "url": "https://hg.mozilla.org/mozilla-central/", + "url": "https://hg.mozilla.org/mozilla-central", + "try_url": "https://hg.mozilla.org/try", "slug": "mozilla-central" } }, @@ -17,7 +18,8 @@ "phid": "PHID-REPO-3lrloqw4qf6fluy2a5ni", "created": "2019-10-17T07:17:32.970Z", "updated": "2019-10-17T07:17:32.970Z", - "url": "https://hg.mozilla.org/nss", + "url": "https://hg.mozilla.org/projects/nss", + "try_url": "https://hg.mozilla.org/projects/nss-try", "slug": "nss" } } From 6ca5d07a992032d049de31aac2c74f016affc3c7 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 10:14:48 +0200 Subject: [PATCH 13/25] Add dockerfile --- .dockerignore | 2 ++ backend/Dockerfile | 8 ++++++ backend/VERSION | 1 + backend/requirements-dev.txt | 1 + backend/setup.py | 47 ++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+) create mode 100644 .dockerignore create mode 100644 backend/Dockerfile create mode 100644 backend/VERSION create mode 100644 backend/requirements-dev.txt create mode 100644 backend/setup.py diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 000000000..57d9cd579 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,2 @@ +*/node_modules +*.sqlite* diff --git a/backend/Dockerfile b/backend/Dockerfile new file mode 100644 index 000000000..390e61496 --- /dev/null +++ b/backend/Dockerfile @@ -0,0 +1,8 @@ +FROM python:3.7-slim + +ADD backend /src/backend + +RUN cd /src/backend && pip install --no-cache-dir . +RUN pip install --no-cache-dir gunicorn + +CMD gunicorn code_review_backend.app.wsgi diff --git a/backend/VERSION b/backend/VERSION new file mode 100644 index 000000000..ee90284c2 --- /dev/null +++ b/backend/VERSION @@ -0,0 +1 @@ +1.0.4 diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt new file mode 100644 index 000000000..f555665e4 --- /dev/null +++ b/backend/requirements-dev.txt @@ -0,0 +1 @@ +pre-commit==1.18.3 diff --git a/backend/setup.py b/backend/setup.py new file mode 100644 index 000000000..1dc11cae2 --- /dev/null +++ b/backend/setup.py @@ -0,0 +1,47 @@ +# -*- 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 setuptools + + +def read_requirements(file_): + lines = [] + with open(file_) as f: + for line in f.readlines(): + line = line.strip() + if ( + line.startswith("-e ") + or line.startswith("http://") + or line.startswith("https://") + ): + extras = "" + if "[" in line: + extras = "[" + line.split("[")[1].split("]")[0] + "]" + line = line.split("#")[1].split("egg=")[1] + extras + elif line == "" or line.startswith("#") or line.startswith("-"): + continue + line = line.split("#")[0].strip() + lines.append(line) + return sorted(list(set(lines))) + + +with open("VERSION") as f: + VERSION = f.read().strip() + + +setuptools.setup( + name="code_review_backend", + version=VERSION, + description="Store and compare issues found in Mozilla code review tasks", + author="Mozilla Release Management", + author_email="release-mgmt-analysis@mozilla.com", + url="https://github.com/mozilla/code-review", + tests_require=read_requirements("requirements-dev.txt"), + install_requires=read_requirements("requirements.txt"), + packages=setuptools.find_packages(), + include_package_data=True, + zip_safe=False, + license="MPL2", +) From ca740a00b74c78dfa2704e80d30d7ea307f2984b Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 11:53:43 +0200 Subject: [PATCH 14/25] Add taskcluster build & ship --- .taskcluster.yml | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/.taskcluster.yml b/.taskcluster.yml index a4eb5529f..582253688 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -98,6 +98,26 @@ tasks: owner: bastien@mozilla.com source: https://github.com/mozilla/code-review + - taskId: {$eval: as_slugid("backend_check_tests")} + provisionerId: aws-provisioner-v1 + workerType: github-worker + created: {$fromNow: ''} + deadline: {$fromNow: '1 hour'} + payload: + maxRunTime: 3600 + image: python:3-slim + command: + - sh + - -lxce + - "git clone --quiet ${repository} /src && cd /src && git checkout ${head_rev} -b checks && + cd /src/backend && pip install . && pip install -r requirements-dev.txt && + ./manage.py test" + metadata: + name: "Code Review Backend checks: unit tests" + description: Check python code with Django tests + owner: bastien@mozilla.com + source: https://github.com/mozilla/code-review + - taskId: {$eval: as_slugid("frontend_build")} provisionerId: aws-provisioner-v1 workerType: github-worker @@ -205,6 +225,47 @@ tasks: owner: bastien@mozilla.com source: https://github.com/mozilla/code-review + - taskId: {$eval: as_slugid("backend_build")} + created: {$fromNow: ''} + deadline: {$fromNow: '1 hour'} + provisionerId: aws-provisioner-v1 + workerType: releng-svc + dependencies: + - {$eval: as_slugid("check_lint")} + - {$eval: as_slugid("backend_check_tests")} + payload: + capabilities: + privileged: true + maxRunTime: 3600 + image: "${taskboot_image}" + env: + GIT_REPOSITORY: ${repository} + GIT_REVISION: ${head_rev} + command: + - taskboot + - build + - --image + - mozilla/code-review + - --tag + - "${channel}" + - --tag + - "${head_rev}" + - --write + - /backend.tar + - backend/Dockerfile + artifacts: + public/code-review-backend.tar: + expires: {$fromNow: '2 weeks'} + path: /backend.tar + type: file + scopes: + - docker-worker:capability:privileged + metadata: + name: Code Review Backend docker build + description: Build docker image of code review backend + owner: bastien@mozilla.com + source: https://github.com/mozilla/code-review + - $if: 'channel in ["testing", "production"]' then: taskId: {$eval: as_slugid("frontend_deploy")} @@ -326,3 +387,33 @@ tasks: description: Deploy docker image on Heroku owner: bastien@mozilla.com source: https://github.com/mozilla/code-review + + - $if: 'channel in ["testing", "production"]' + then: + taskId: {$eval: as_slugid("backend_deploy")} + created: {$fromNow: ''} + deadline: {$fromNow: '1 hour'} + provisionerId: aws-provisioner-v1 + workerType: github-worker + dependencies: + - {$eval: as_slugid("backend_build")} + payload: + features: + taskclusterProxy: true + maxRunTime: 3600 + image: "${taskboot_image}" + command: + - taskboot + - deploy-heroku + - --heroku-app + - "code-review-backend-${channel}" + - web:public/code-review-backend.tar + env: + TASKCLUSTER_SECRET: "project/relman/code-review/deploy-${channel}" + scopes: + - "secrets:get:project/relman/code-review/deploy-${channel}" + metadata: + name: "Code Review Backend deployment (${channel})" + description: Deploy docker image on Heroku + owner: bastien@mozilla.com + source: https://github.com/mozilla/code-review From b6376a58361e1f1781bf07f46a6693375029fedf Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 12:17:04 +0200 Subject: [PATCH 15/25] Heroku setup --- .isort.cfg | 2 +- .taskcluster.yml | 3 +- backend/code_review_backend/app/settings.py | 32 +++++++++++++++++++++ backend/requirements.txt | 2 ++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/.isort.cfg b/.isort.cfg index 3ff76f92e..948a75fc9 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -1,6 +1,6 @@ [settings] known_first_party = code_review_backend,code_review_bot,code_review_tools,code_review_events,conftest -known_third_party = django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml +known_third_party = dj_database_url,django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml force_single_line = True default_section=FIRSTPARTY line_length=159 diff --git a/.taskcluster.yml b/.taskcluster.yml index 582253688..4b80bb399 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -388,7 +388,8 @@ tasks: owner: bastien@mozilla.com source: https://github.com/mozilla/code-review - - $if: 'channel in ["testing", "production"]' + # TODO: remove backend branch after tests are OK + - $if: 'channel in ["testing", "production"] || head_branch == "refs/heads/backend"' then: taskId: {$eval: as_slugid("backend_deploy")} created: {$fromNow: ''} diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index be293154b..c688c2d6d 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -15,8 +15,13 @@ https://docs.djangoproject.com/en/2.2/ref/settings/ """ +import logging import os +import dj_database_url + +logger = logging.getLogger(__name__) + # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) ROOT_DIR = os.path.dirname(BASE_DIR) @@ -119,3 +124,30 @@ # https://docs.djangoproject.com/en/2.2/howto/static-files/ STATIC_URL = "/static/" + + +# Heroku settings override to run the web app in production mode +if "DYNO" in os.environ: + logger.info("Setting up Heroku environment") + ALLOWED_HOSTS = ["*"] + DEBUG = False + + # Database setup + if "DATABASE_URL" in os.environ: + logger.info("Using remote database from $DATABASE_URL") + DATABASES["default"] = dj_database_url.config( + os.environ["DATABASE_URL"], ssl_require=True + ) + else: + logger.info("DATABASE_URL not found, will use sqlite. Data may be lost.") + + # Staticfiles configuration + STATIC_ROOT = os.path.join(BASE_DIR, "staticfiles") + STATIC_URL = "/static/" + os.makedirs(STATIC_ROOT, exist_ok=True) + + # Insert Whitenoise Middleware in top position + MIDDLEWARE.insert(0, "whitenoise.middleware.WhiteNoiseMiddleware") + + # Enable GZip + STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" diff --git a/backend/requirements.txt b/backend/requirements.txt index 965be50ae..3dda70f81 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,5 +1,7 @@ Django==2.2.6 +dj-database-url==0.5.0 pytz==2019.3 sqlparse==0.3.0 taskcluster==19.0.0 taskcluster-urls==11.0.0 +whitenoise==4.1.4 From e2439ee5af39fa7b57e25bce44d601d648f12a62 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 12:24:51 +0200 Subject: [PATCH 16/25] Small fixes on CI --- .taskcluster.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.taskcluster.yml b/.taskcluster.yml index 4b80bb399..62472ec82 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -105,12 +105,12 @@ tasks: deadline: {$fromNow: '1 hour'} payload: maxRunTime: 3600 - image: python:3-slim + image: python:3 command: - sh - -lxce - "git clone --quiet ${repository} /src && cd /src && git checkout ${head_rev} -b checks && - cd /src/backend && pip install . && pip install -r requirements-dev.txt && + cd /src/backend && pip install -q . && pip install -q -r requirements-dev.txt && ./manage.py test" metadata: name: "Code Review Backend checks: unit tests" @@ -389,7 +389,7 @@ tasks: source: https://github.com/mozilla/code-review # TODO: remove backend branch after tests are OK - - $if: 'channel in ["testing", "production"] || head_branch == "refs/heads/backend"' + - $if: 'channel in ["testing", "production"] || head_branch == "backend"' then: taskId: {$eval: as_slugid("backend_deploy")} created: {$fromNow: ''} From af755b7202d67aa71afb8d083aa7590dcd01fab3 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 12:46:49 +0200 Subject: [PATCH 17/25] Heroku fixes --- backend/code_review_backend/app/settings.py | 2 +- backend/requirements.txt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index c688c2d6d..aa9b6794c 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -135,7 +135,7 @@ # Database setup if "DATABASE_URL" in os.environ: logger.info("Using remote database from $DATABASE_URL") - DATABASES["default"] = dj_database_url.config( + DATABASES["default"] = dj_database_url.parse( os.environ["DATABASE_URL"], ssl_require=True ) else: diff --git a/backend/requirements.txt b/backend/requirements.txt index 3dda70f81..7ac0e986b 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,5 +1,6 @@ Django==2.2.6 dj-database-url==0.5.0 +psycopg2-binary==2.8.3 pytz==2019.3 sqlparse==0.3.0 taskcluster==19.0.0 From cd811fef4fbcff9a8092c02ed27a080785094378 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 12:59:50 +0200 Subject: [PATCH 18/25] Allow running debug env on heroku --- backend/code_review_backend/app/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index aa9b6794c..8b2b74455 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -130,7 +130,7 @@ if "DYNO" in os.environ: logger.info("Setting up Heroku environment") ALLOWED_HOSTS = ["*"] - DEBUG = False + DEBUG = os.environ.get("DEBUG", "false").lower() == "true" # Database setup if "DATABASE_URL" in os.environ: From 32ea57e16e0336b627dfd69c0e22272123929fb0 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 13:13:20 +0200 Subject: [PATCH 19/25] Embed static files in image --- backend/Dockerfile | 10 +++++++++- backend/code_review_backend/app/settings.py | 22 +++++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/backend/Dockerfile b/backend/Dockerfile index 390e61496..d28a01703 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -2,7 +2,15 @@ FROM python:3.7-slim ADD backend /src/backend -RUN cd /src/backend && pip install --no-cache-dir . +WORKDIR /src/backend + +# Activate Django settings for in docker image +ENV DJANGO_DOCKER=true + +RUN pip install --no-cache-dir . RUN pip install --no-cache-dir gunicorn +# Collect all static files +RUN ./manage.py collectstatic --no-input + CMD gunicorn code_review_backend.app.wsgi diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index 8b2b74455..acf585b7b 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -125,6 +125,17 @@ STATIC_URL = "/static/" +# Internal logging setup +LOGGING = { + "version": 1, + "disable_existing_loggers": False, + "handlers": {"console": {"class": "logging.StreamHandler"}}, + "loggers": {"django": {"handlers": ["console"], "level": "INFO"}}, +} + +# Static files are set in a dedicated path in Docker image +if "DJANGO_DOCKER" in os.environ: + STATIC_ROOT = "/static" # Heroku settings override to run the web app in production mode if "DYNO" in os.environ: @@ -141,13 +152,8 @@ else: logger.info("DATABASE_URL not found, will use sqlite. Data may be lost.") - # Staticfiles configuration - STATIC_ROOT = os.path.join(BASE_DIR, "staticfiles") - STATIC_URL = "/static/" - os.makedirs(STATIC_ROOT, exist_ok=True) - - # Insert Whitenoise Middleware in top position - MIDDLEWARE.insert(0, "whitenoise.middleware.WhiteNoiseMiddleware") + # Insert Whitenoise Middleware after the security one + MIDDLEWARE.insert(1, "whitenoise.middleware.WhiteNoiseMiddleware") - # Enable GZip + # Enable GZip and cache STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" From 24c0d90fdd756dc50c5984eef372bede1d222324 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 13:54:43 +0200 Subject: [PATCH 20/25] Fix static file storage setting --- backend/code_review_backend/app/settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index acf585b7b..99473bd68 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -137,6 +137,9 @@ if "DJANGO_DOCKER" in os.environ: STATIC_ROOT = "/static" + # Enable GZip and cache, and build a manifest during collectstatic + STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" + # Heroku settings override to run the web app in production mode if "DYNO" in os.environ: logger.info("Setting up Heroku environment") @@ -154,6 +157,3 @@ # Insert Whitenoise Middleware after the security one MIDDLEWARE.insert(1, "whitenoise.middleware.WhiteNoiseMiddleware") - - # Enable GZip and cache - STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" From 1e599a31bf0b76f5af300f52a3b9b4f9383998aa Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 14:09:26 +0200 Subject: [PATCH 21/25] Move gunicorn in base reqs --- backend/Dockerfile | 1 - backend/requirements.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/Dockerfile b/backend/Dockerfile index d28a01703..963b48045 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -8,7 +8,6 @@ WORKDIR /src/backend ENV DJANGO_DOCKER=true RUN pip install --no-cache-dir . -RUN pip install --no-cache-dir gunicorn # Collect all static files RUN ./manage.py collectstatic --no-input diff --git a/backend/requirements.txt b/backend/requirements.txt index 7ac0e986b..6583614a8 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,5 +1,6 @@ Django==2.2.6 dj-database-url==0.5.0 +gunicorn==19.9.0 psycopg2-binary==2.8.3 pytz==2019.3 sqlparse==0.3.0 From b3563cffc9c5fbee4956a32b13ec6294b1b09e97 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 14:27:47 +0200 Subject: [PATCH 22/25] Support full file issues --- backend/code_review_backend/app/settings.py | 5 ++++- .../issues/management/commands/load_issues.py | 14 +++++++++++--- .../issues/migrations/0001_initial.py | 6 +++--- backend/code_review_backend/issues/models.py | 4 ++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index 99473bd68..a101189f5 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -130,7 +130,10 @@ "version": 1, "disable_existing_loggers": False, "handlers": {"console": {"class": "logging.StreamHandler"}}, - "loggers": {"django": {"handlers": ["console"], "level": "INFO"}}, + "loggers": { + "django": {"handlers": ["console"], "level": "INFO"}, + "code_review_backend": {"handlers": ["console"], "level": "INFO"}, + }, } # Static files are set in a dedicated path in Docker image diff --git a/backend/code_review_backend/issues/management/commands/load_issues.py b/backend/code_review_backend/issues/management/commands/load_issues.py index 96c279316..aae1943fb 100644 --- a/backend/code_review_backend/issues/management/commands/load_issues.py +++ b/backend/code_review_backend/issues/management/commands/load_issues.py @@ -21,6 +21,14 @@ INDEX_PATH = "project.relman.{environment}.code-review.phabricator.diff" +def positive_int(x): + """ + Some analyzer may give line & char positions below 0 to indicate a full file + We store it as null values + """ + return x if isinstance(x, int) and x >= 0 else None + + class Command(BaseCommand): help = "Load issues from remote taskcluster reports" @@ -74,9 +82,9 @@ def handle(self, *args, **options): Issue( diff=diff, path=i["path"], - line=i["line"], + line=positive_int(i["line"]), nb_lines=i.get("nb_lines", 1), - char=i.get("char"), + char=positive_int(i.get("char")), level=i.get("level", "warning"), check=i.get("kind") or i.get("check"), message=i.get("message"), @@ -85,7 +93,7 @@ def handle(self, *args, **options): ) for i in report["issues"] ) - print(task_id, len(issues)) + logger.info(f"Imported task {task_id} - {len(issues)}") def load_tasks(self, environment, chunk=200): # Direct unauthenticated usage diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index ebbccb9f1..06b7e9c09 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-17 07:48 +# Generated by Django 2.2.6 on 2019-10-17 12:25 import uuid @@ -74,8 +74,8 @@ class Migration(migrations.Migration): ), ), ("path", models.CharField(max_length=250)), - ("line", models.PositiveIntegerField()), - ("nb_lines", models.PositiveIntegerField()), + ("line", models.PositiveIntegerField(null=True)), + ("nb_lines", models.PositiveIntegerField(null=True)), ("char", models.PositiveIntegerField(null=True)), ( "level", diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 359d69fa1..a43f2fcc6 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -68,8 +68,8 @@ class Issue(models.Model): # Raw issue data path = models.CharField(max_length=250) - line = models.PositiveIntegerField() - nb_lines = models.PositiveIntegerField() + line = models.PositiveIntegerField(null=True) + nb_lines = models.PositiveIntegerField(null=True) char = models.PositiveIntegerField(null=True) level = models.CharField(max_length=20, choices=ISSUE_LEVELS) check = models.CharField(max_length=250, null=True) From 98a831b5e56d944178ffe1425bdd7c86973ed2ab Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 15:45:49 +0200 Subject: [PATCH 23/25] Add base API for revision & diff --- .isort.cfg | 2 +- backend/code_review_backend/app/settings.py | 28 +++++++++---- backend/code_review_backend/app/urls.py | 10 ++++- backend/code_review_backend/issues/api.py | 40 +++++++++++++++++++ .../code_review_backend/issues/serializers.py | 36 +++++++++++++++++ backend/requirements.txt | 1 + 6 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 backend/code_review_backend/issues/api.py create mode 100644 backend/code_review_backend/issues/serializers.py diff --git a/.isort.cfg b/.isort.cfg index 948a75fc9..ef244a0fe 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -1,6 +1,6 @@ [settings] known_first_party = code_review_backend,code_review_bot,code_review_tools,code_review_events,conftest -known_third_party = dj_database_url,django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,setuptools,structlog,taskcluster,toml +known_third_party = dj_database_url,django,influxdb,libmozdata,libmozevent,logbook,parsepatch,pytest,raven,requests,responses,rest_framework,setuptools,structlog,taskcluster,toml force_single_line = True default_section=FIRSTPARTY line_length=159 diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index a101189f5..3167c00fd 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -48,6 +48,7 @@ "django.contrib.sessions", "django.contrib.messages", "django.contrib.staticfiles", + "rest_framework", "code_review_backend.issues", ] @@ -119,12 +120,32 @@ USE_TZ = True +# API configuration +REST_FRAMEWORK = { + # Use Django's standard `django.contrib.auth` permissions, + # or allow read-only access for unauthenticated users. + "DEFAULT_PERMISSION_CLASSES": [ + "rest_framework.permissions.DjangoModelPermissionsOrAnonReadOnly" + ], + # Setup pagination + "PAGE_SIZE": 50, + "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.LimitOffsetPagination", +} + # Static files (CSS, JavaScript, Images) # https://docs.djangoproject.com/en/2.2/howto/static-files/ STATIC_URL = "/static/" +# Static files are set in a dedicated path in Docker image +if "DJANGO_DOCKER" in os.environ: + STATIC_ROOT = "/static" + + # Enable GZip and cache, and build a manifest during collectstatic + STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" + + # Internal logging setup LOGGING = { "version": 1, @@ -136,13 +157,6 @@ }, } -# Static files are set in a dedicated path in Docker image -if "DJANGO_DOCKER" in os.environ: - STATIC_ROOT = "/static" - - # Enable GZip and cache, and build a manifest during collectstatic - STATICFILES_STORAGE = "whitenoise.storage.CompressedManifestStaticFilesStorage" - # Heroku settings override to run the web app in production mode if "DYNO" in os.environ: logger.info("Setting up Heroku environment") diff --git a/backend/code_review_backend/app/urls.py b/backend/code_review_backend/app/urls.py index e6b0a988d..0e18f219f 100644 --- a/backend/code_review_backend/app/urls.py +++ b/backend/code_review_backend/app/urls.py @@ -4,6 +4,14 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. from django.contrib import admin +from django.shortcuts import redirect +from django.urls import include from django.urls import path -urlpatterns = [path("admin/", admin.site.urls)] +from code_review_backend.issues import api + +urlpatterns = [ + path("", lambda request: redirect("v1/", permanent=False)), + path("v1/", include(api.router.urls)), + path("admin/", admin.site.urls), +] diff --git a/backend/code_review_backend/issues/api.py b/backend/code_review_backend/issues/api.py new file mode 100644 index 000000000..cd48f71f1 --- /dev/null +++ b/backend/code_review_backend/issues/api.py @@ -0,0 +1,40 @@ +# -*- 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/. + +from rest_framework import mixins +from rest_framework import routers +from rest_framework import viewsets + +from code_review_backend.issues.models import Diff +from code_review_backend.issues.models import Revision +from code_review_backend.issues.serializers import DiffSerializer +from code_review_backend.issues.serializers import RevisionSerializer + + +class CreateListRetrieveViewSet( + mixins.CreateModelMixin, + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + viewsets.GenericViewSet, +): + """ + A viewset that allows creation, listing and retrieval of Model instances + From https://www.django-rest-framework.org/api-guide/viewsets/#custom-viewset-base-classes + """ + + +class RevisionViewSet(CreateListRetrieveViewSet): + queryset = Revision.objects.all() + serializer_class = RevisionSerializer + + +class DiffViewSet(CreateListRetrieveViewSet): + queryset = Diff.objects.all() + serializer_class = DiffSerializer + + +router = routers.DefaultRouter() +router.register(r"revision", RevisionViewSet) +router.register(r"diff", DiffViewSet) diff --git a/backend/code_review_backend/issues/serializers.py b/backend/code_review_backend/issues/serializers.py new file mode 100644 index 000000000..c67d27426 --- /dev/null +++ b/backend/code_review_backend/issues/serializers.py @@ -0,0 +1,36 @@ +# -*- 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/. + +from rest_framework import serializers + +from code_review_backend.issues.models import Diff +from code_review_backend.issues.models import Repository +from code_review_backend.issues.models import Revision + + +class RevisionSerializer(serializers.HyperlinkedModelSerializer): + """ + Serialize a Revision in a repository + """ + + repository = serializers.SlugRelatedField( + queryset=Repository.objects.all(), slug_field="slug" + ) + + class Meta: + model = Revision + fields = ("id", "repository", "phid", "title", "bugzilla_id") + + +class DiffSerializer(serializers.HyperlinkedModelSerializer): + """ + Serialize a Diff in a revision + """ + + revision = serializers.PrimaryKeyRelatedField(queryset=Revision.objects.all()) + + class Meta: + model = Diff + fields = ("id", "revision", "phid", "review_task_id", "mercurial") diff --git a/backend/requirements.txt b/backend/requirements.txt index 6583614a8..0222ff9f1 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,5 +1,6 @@ Django==2.2.6 dj-database-url==0.5.0 +djangorestframework==3.10.3 gunicorn==19.9.0 psycopg2-binary==2.8.3 pytz==2019.3 From 52508219b913d657d5e5f513fe69a3973467ddf1 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 17:24:02 +0200 Subject: [PATCH 24/25] Issue api --- backend/code_review_backend/issues/api.py | 10 ++++++ .../issues/migrations/0001_initial.py | 7 +++-- backend/code_review_backend/issues/models.py | 4 +++ .../code_review_backend/issues/serializers.py | 31 +++++++++++++++++-- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/backend/code_review_backend/issues/api.py b/backend/code_review_backend/issues/api.py index cd48f71f1..b8b737fd6 100644 --- a/backend/code_review_backend/issues/api.py +++ b/backend/code_review_backend/issues/api.py @@ -8,8 +8,10 @@ from rest_framework import viewsets from code_review_backend.issues.models import Diff +from code_review_backend.issues.models import Issue from code_review_backend.issues.models import Revision from code_review_backend.issues.serializers import DiffSerializer +from code_review_backend.issues.serializers import IssueSerializer from code_review_backend.issues.serializers import RevisionSerializer @@ -35,6 +37,14 @@ class DiffViewSet(CreateListRetrieveViewSet): serializer_class = DiffSerializer +class IssueViewSet(CreateListRetrieveViewSet): + serializer_class = IssueSerializer + + def get_queryset(self): + return Issue.objects.filter(diff_id=self.kwargs["diff_id"]) + + router = routers.DefaultRouter() router.register(r"revision", RevisionViewSet) router.register(r"diff", DiffViewSet) +router.register(r"diff/(?P\d+)/issues", IssueViewSet, basename="issues") diff --git a/backend/code_review_backend/issues/migrations/0001_initial.py b/backend/code_review_backend/issues/migrations/0001_initial.py index 06b7e9c09..985c76cb4 100644 --- a/backend/code_review_backend/issues/migrations/0001_initial.py +++ b/backend/code_review_backend/issues/migrations/0001_initial.py @@ -3,7 +3,7 @@ # 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/. -# Generated by Django 2.2.6 on 2019-10-17 12:25 +# Generated by Django 2.2.6 on 2019-10-17 15:23 import uuid @@ -29,7 +29,7 @@ class Migration(migrations.Migration): ("review_task_id", models.CharField(max_length=30, unique=True)), ("mercurial", models.CharField(max_length=40)), ], - options={"abstract": False}, + options={"ordering": ("id",), "abstract": False}, ), migrations.CreateModel( name="Repository", @@ -62,7 +62,7 @@ class Migration(migrations.Migration): ), ), ], - options={"abstract": False}, + options={"ordering": ("id",), "abstract": False}, ), migrations.CreateModel( name="Issue", @@ -99,6 +99,7 @@ class Migration(migrations.Migration): ), ), ], + options={"ordering": ("diff", "path", "line", "analyzer")}, ), migrations.AddField( model_name="diff", diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index a43f2fcc6..c88d7482c 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -21,6 +21,7 @@ class PhabricatorModel(models.Model): class Meta: abstract = True + ordering = ("id",) class Repository(PhabricatorModel): @@ -81,3 +82,6 @@ class Issue(models.Model): created = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) + + class Meta: + ordering = ("diff", "path", "line", "analyzer") diff --git a/backend/code_review_backend/issues/serializers.py b/backend/code_review_backend/issues/serializers.py index c67d27426..b2c3a5554 100644 --- a/backend/code_review_backend/issues/serializers.py +++ b/backend/code_review_backend/issues/serializers.py @@ -6,13 +6,14 @@ from rest_framework import serializers from code_review_backend.issues.models import Diff +from code_review_backend.issues.models import Issue from code_review_backend.issues.models import Repository from code_review_backend.issues.models import Revision class RevisionSerializer(serializers.HyperlinkedModelSerializer): """ - Serialize a Revision in a repository + Serialize a Revision in a Repository """ repository = serializers.SlugRelatedField( @@ -26,11 +27,35 @@ class Meta: class DiffSerializer(serializers.HyperlinkedModelSerializer): """ - Serialize a Diff in a revision + Serialize a Diff in a Revision """ revision = serializers.PrimaryKeyRelatedField(queryset=Revision.objects.all()) + issues_url = serializers.HyperlinkedIdentityField( + view_name="issues-list", lookup_url_kwarg="diff_id" + ) class Meta: model = Diff - fields = ("id", "revision", "phid", "review_task_id", "mercurial") + fields = ("id", "revision", "phid", "review_task_id", "mercurial", "issues_url") + + +class IssueSerializer(serializers.HyperlinkedModelSerializer): + """ + Serialize an Issue in a Diff + """ + + class Meta: + model = Issue + fields = ( + "id", + "hash", + "analyzer", + "path", + "line", + "nb_lines", + "char", + "level", + "check", + "message", + ) From 2198bf6e1061aee65fbdd2396ac21a367665b9ba Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Thu, 17 Oct 2019 18:25:37 +0200 Subject: [PATCH 25/25] Unit tests + fixes --- backend/code_review_backend/app/settings.py | 2 +- backend/code_review_backend/issues/api.py | 4 + .../issues/tests/__init__.py | 0 .../issues/tests/test_api.py | 147 ++++++++++++++++++ 4 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 backend/code_review_backend/issues/tests/__init__.py create mode 100644 backend/code_review_backend/issues/tests/test_api.py diff --git a/backend/code_review_backend/app/settings.py b/backend/code_review_backend/app/settings.py index 3167c00fd..460f2f199 100644 --- a/backend/code_review_backend/app/settings.py +++ b/backend/code_review_backend/app/settings.py @@ -125,7 +125,7 @@ # Use Django's standard `django.contrib.auth` permissions, # or allow read-only access for unauthenticated users. "DEFAULT_PERMISSION_CLASSES": [ - "rest_framework.permissions.DjangoModelPermissionsOrAnonReadOnly" + "rest_framework.permissions.IsAuthenticatedOrReadOnly" ], # Setup pagination "PAGE_SIZE": 50, diff --git a/backend/code_review_backend/issues/api.py b/backend/code_review_backend/issues/api.py index b8b737fd6..c514e40a6 100644 --- a/backend/code_review_backend/issues/api.py +++ b/backend/code_review_backend/issues/api.py @@ -43,6 +43,10 @@ class IssueViewSet(CreateListRetrieveViewSet): def get_queryset(self): return Issue.objects.filter(diff_id=self.kwargs["diff_id"]) + def perform_create(self, serializer): + # Attach diff to issue created + serializer.save(diff=Diff.objects.get(pk=self.kwargs["diff_id"])) + router = routers.DefaultRouter() router.register(r"revision", RevisionViewSet) diff --git a/backend/code_review_backend/issues/tests/__init__.py b/backend/code_review_backend/issues/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/code_review_backend/issues/tests/test_api.py b/backend/code_review_backend/issues/tests/test_api.py new file mode 100644 index 000000000..776a3df49 --- /dev/null +++ b/backend/code_review_backend/issues/tests/test_api.py @@ -0,0 +1,147 @@ +# -*- 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/. + +from django.contrib.auth.models import User +from rest_framework import status +from rest_framework.test import APITestCase + +from code_review_backend.issues.models import Diff +from code_review_backend.issues.models import Issue +from code_review_backend.issues.models import Repository +from code_review_backend.issues.models import Revision + + +class CreationAPITestCase(APITestCase): + def setUp(self): + # Create a user + self.user = User.objects.create(username="crash_user") + + # Create a repo + self.repo = Repository.objects.create( + id=1, phid="PHID-REPO-xxx", slug="myrepo", url="http://repo.test/myrepo" + ) + + def test_create_revision(self): + """ + Check we can create a revision through the API + """ + data = { + "id": 123, + "phid": "PHID-REV-xxx", + "title": "Bug XXX - Some bug", + "bugzilla_id": 123456, + "repository": "myrepo", + } + + # No auth will give a permission denied + response = self.client.post("/v1/revision/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Once authenticated, creation will work + self.assertEqual(Revision.objects.count(), 0) + self.client.force_authenticate(user=self.user) + response = self.client.post("/v1/revision/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # Check a revision has been created + self.assertEqual(Revision.objects.count(), 1) + revision = Revision.objects.get(pk=123) + self.assertEqual(revision.title, "Bug XXX - Some bug") + self.assertEqual(revision.bugzilla_id, 123456) + + def test_create_diff(self): + """ + Check we can create a diff through the API + """ + data = { + "id": 1234, + "revision": 123, + "phid": "PHID-DIFF-xxx", + "review_task_id": "deadbeef123", + "mercurial": "coffee12345", + } + + # No auth will give a permission denied + response = self.client.post("/v1/revision/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Once authenticated, creation will require the revision to exist + self.assertEqual(Diff.objects.count(), 0) + self.client.force_authenticate(user=self.user) + response = self.client.post("/v1/diff/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual( + response.json(), {"revision": ['Invalid pk "123" - object does not exist.']} + ) + + # Create the requested revision + revision = Revision.objects.create( + id=123, + phid="PHID-REV-XXX", + repository=self.repo, + title="Bug XXX - Another bug", + bugzilla_id=123456, + ) + + # Now creation will work + self.assertEqual(Diff.objects.count(), 0) + self.client.force_authenticate(user=self.user) + response = self.client.post("/v1/diff/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # Response should have url to create issues + self.assertEqual( + response.json()["issues_url"], "http://testserver/v1/diff/1234/issues/" + ) + + # Check a diff has been created + self.assertEqual(Diff.objects.count(), 1) + diff = Diff.objects.get(pk=1234) + self.assertEqual(diff.mercurial, "coffee12345") + self.assertEqual(diff.revision, revision) + + def test_create_issue(self): + """ + Check we can create a issue through the API + """ + # Create revision and diff + revision = self.repo.revisions.create( + id=456, + phid="PHID-REV-XXX", + title="Bug XXX - Yet Another bug", + bugzilla_id=78901, + ) + diff = revision.diffs.create( + id=1234, + phid="PHID-DIFF-xxx", + review_task_id="deadbeef123", + mercurial="coffee12345", + ) + + data = { + "hash": "somemd5hash", + "line": 1, + "analyzer": "remote-flake8", + "level": "error", + "path": "path/to/file.py", + } + + # No auth will give a permission denied + response = self.client.post("/v1/diff/1234/issues/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # Once authenticated, creation will work + self.assertEqual(Issue.objects.count(), 0) + self.client.force_authenticate(user=self.user) + response = self.client.post("/v1/diff/1234/issues/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # Check a revision has been created + self.assertEqual(Diff.objects.count(), 1) + issue = Issue.objects.first() + self.assertEqual(issue.path, "path/to/file.py") + self.assertEqual(issue.line, 1) + self.assertEqual(issue.diff, diff) + self.assertEqual(issue.diff.revision, revision)