From be3e1ad5564b0c6ad22bf3d32e6b357ea328f81c Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Jun 07 2021 12:32:06 +0000 Subject: [PATCH 1/4] frontend: by default we show builds from all dirs This caused that the UI didn't respect the (default ON) button "all builds". Fixes bug caused by ff8d25820f75d1b4033afedad582460742000827 --- diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py index 8f9d275..ae89008 100644 --- a/frontend/coprs_frontend/coprs/logic/builds_logic.py +++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py @@ -394,9 +394,7 @@ class BuildsLogic(object): query = models.Build.query.filter(models.Build.copr_id==copr.id) if dirname: copr_dir = coprs_logic.CoprDirsLogic.get_by_copr(copr, dirname).one() - else: - copr_dir = copr.main_dir - query = query.filter(models.Build.copr_dir_id==copr_dir.id) + query = query.filter(models.Build.copr_dir_id==copr_dir.id) query = query.options(selectinload('build_chroots'), selectinload('package')) return query From c95381b32e73246ab2352cfa56eb63f4691bcf3a Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Jun 07 2021 12:32:06 +0000 Subject: [PATCH 2/4] frontend: colorize CoprDir-buttons on builds page This introduces a new KEEP_PR_DIRS_DAYS option, by default set to 40. When the lastest build in the pull-request CoprDir was submitted more than 40 days ago, the "filter" button in web-UI will be colored "red" (as outdated). When it is older than 1/4*40 (==20) days, the button will be "orange". All the buttons have a pop-up title saying how long the directory will be kept. --- diff --git a/frontend/coprs_frontend/coprs/config.py b/frontend/coprs_frontend/coprs/config.py index 78cc6f4..f4ec7a4 100644 --- a/frontend/coprs_frontend/coprs/config.py +++ b/frontend/coprs_frontend/coprs/config.py @@ -125,6 +125,9 @@ class Config(object): # Setting this to True requires special installation PROFILER = False + # We remove pull-request directories after some time. + KEEP_PR_DIRS_DAYS = 40 + class ProductionConfig(Config): DEBUG = False # SECRET_KEY = "put_some_secret_here" diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py b/frontend/coprs_frontend/coprs/logic/coprs_logic.py index b707ce2..76f49d1 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -1,6 +1,10 @@ +import locale import os import time import datetime +from functools import cmp_to_key +from itertools import zip_longest + import flask from sqlalchemy import and_, not_ @@ -12,7 +16,7 @@ from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.attributes import get_history from copr_common.enums import ActionTypeEnum, BackendResultEnum, ActionPriorityEnum -from coprs import db +from coprs import app, db from coprs import exceptions from coprs import helpers from coprs import models @@ -563,6 +567,104 @@ class CoprDirsLogic(object): for copr_dir in copr.dirs: db.session.delete(copr_dir) + @staticmethod + def _compare_dir_pairs(pair1, pair2): + """ + Compare list of pairs from get_all_with_latest_submitted_build_query + """ + + dirname1 = pair1.CoprDir.name + dirname2 = pair2.CoprDir.name + + if ':' not in dirname1: + return -1 + if ':' not in dirname2: + return 1 + + pr = False + for left, right in zip_longest(dirname1.split(':'), dirname2.split(':')): + if None in [left, right]: + return 1 if left is None else -1 + if pr: + try: + # _try_ to compare as numbers now + return int(right) - int(left) + except ValueError: + # let's fallback to string comparison + pr = False + rc = locale.strcoll(left, right) + if rc != 0: + return rc + + # go down to another field + if left == 'pr': + pr = True + return 0 + + @classmethod + def get_all_with_latest_submitted_build_query(cls, copr_id): + """ + Get query returning list of pairs (CoprDir, latest_build_submitted_on) + """ + subquery = ( + db.session.query( + func.max(models.Build.submitted_on) + ) + .filter(models.Build.copr_dir_id==models.CoprDir.id) + .group_by(models.Build.copr_dir_id) + .label("latest_build_submitted_on") + ) + return ( + db.session.query(models.CoprDir, subquery) + .filter(models.CoprDir.copr_id==copr_id) + ) + + @classmethod + def get_all_with_latest_submitted_build(cls, copr_id): + """ + Return a list of pairs like [(CoprDir, latest_build_submitted_on), ...] + ordered by name. + """ + + keep_days = app.config["KEEP_PR_DIRS_DAYS"] + now = time.time() + + pairs = list(cls.get_all_with_latest_submitted_build_query(copr_id)) + + results = [] + for pair in sorted(pairs, key=cmp_to_key(cls._compare_dir_pairs)): + last = pair.latest_build_submitted_on + copr_dir = pair.CoprDir + + removal_candidate = True + if ':pr:' not in copr_dir.name: + removal_candidate = False + delete = False # never ever remove this! + remaining_days = 'infinity' + days_since_last = 0 + + elif last is None: + delete = True + remaining_days = 0 + days_since_last = float('inf') + + else: + seconds_since_last = now - last + days_since_last = seconds_since_last/3600/24 + delete = days_since_last > keep_days + remaining_days = int(keep_days - days_since_last) + if remaining_days < 0: + remaining_days = 0 + + results += [{ + 'copr_dir': copr_dir, + 'warn': days_since_last > keep_days / 2, + 'delete': delete, + 'remaining_days': remaining_days, + 'removal_candidate': removal_candidate, + }] + + return results @listens_for(models.Copr.auto_createrepo, 'set') def on_auto_createrepo_change(target_copr, value_acr, old_value_acr, initiator): diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/builds.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/builds.html index f72deee..700707c 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/detail/builds.html +++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/builds.html @@ -35,10 +35,20 @@ all builds - {% for copr_dir in copr.dirs|sort(attribute='name') %} + {% for copr_dir_info in copr_dirs %} + {% set copr_dir = copr_dir_info.copr_dir %} {% if copr_dir.main or (copr_dir.packages | count) > 0 %} - - {{ copr_dir.name }} + + {{ copr_dir.name }} {% endif %} {% endfor %} diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index 056cfac..9e9392d 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -10,6 +10,7 @@ from coprs import helpers from coprs.logic import builds_logic from coprs.logic.builds_logic import BuildsLogic from coprs.logic.complex_logic import ComplexLogic +from coprs.logic.coprs_logic import CoprDirsLogic from coprs.views.misc import (login_required, req_with_copr, send_build_icon) from coprs.views.coprs_ns import coprs_ns @@ -59,10 +60,13 @@ def copr_builds(copr): dirname = flask.request.args.get('dirname') builds_query = builds_logic.BuildsLogic.get_copr_builds_list(copr, dirname) builds = builds_query.yield_per(1000) + dirs = CoprDirsLogic.get_all_with_latest_submitted_build(copr.id) + response = flask.Response(stream_with_context(helpers.stream_template("coprs/detail/builds.html", copr=copr, builds=builds, current_dirname=dirname, + copr_dirs=dirs, flashes=flashes))) flask.session.pop('_flashes', []) From 4669725152a4c4c47180c1942be9ffa2a0f3d370 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Jun 07 2021 12:33:58 +0000 Subject: [PATCH 3/4] backend, frontend, common: automatically delete PR CoprDirs Using another daily Cron job, delete all outdated pull-request directories from DB, and also generate a new action that will remove the corresponding data on the backend side. Fixes: #820 --- diff --git a/backend/copr-backend.spec b/backend/copr-backend.spec index 0f346cb..8b21feb 100644 --- a/backend/copr-backend.spec +++ b/backend/copr-backend.spec @@ -6,6 +6,8 @@ %global tests_version 2 %global tests_tar test-data-copr-backend +%global copr_common_version 0.11.1.dev + Name: copr-backend Version: 1.148 Release: 1%{?dist} @@ -35,6 +37,7 @@ BuildRequires: python3-devel BuildRequires: python3-setuptools BuildRequires: python3-copr +BuildRequires: python3-copr-common >= %copr_common_version BuildRequires: python3-copr-messaging BuildRequires: python3-daemon BuildRequires: python3-dateutil @@ -73,7 +76,7 @@ Requires: obs-signd Requires: openssh-clients Requires: prunerepo >= %prunerepo_version Requires: python3-copr -Requires: python3-copr-common >= 0.10.1.dev +Requires: python3-copr-common >= %copr_common_version Requires: python3-copr-messaging Requires: python3-daemon Requires: python3-dateutil diff --git a/backend/copr_backend/actions.py b/backend/copr_backend/actions.py index 7f9b617..8c22aa5 100644 --- a/backend/copr_backend/actions.py +++ b/backend/copr_backend/actions.py @@ -63,6 +63,7 @@ class Action(object): ActionType.FORK: Fork, ActionType.BUILD_MODULE: BuildModule, ActionType.DELETE: Delete, + ActionType.REMOVE_DIRS: RemoveDirs, }.get(action_type, None) if action_type == ActionType.DELETE: @@ -506,6 +507,33 @@ class BuildModule(Action): return result +class RemoveDirs(Action): + """ + Delete outdated CoprDir instances. Frontend gives us only a list of + sub-directories to remove. + """ + def _run_internal(self): + copr_dirs = json.loads(self.data["data"]) + for copr_dir in copr_dirs: + assert len(copr_dir.split('/')) == 2 + assert ':pr:' in copr_dir + directory = os.path.join(self.destdir, copr_dir) + self.log.info("RemoveDirs: removing %s", directory) + try: + shutil.rmtree(directory) + except FileNotFoundError: + self.log.error("RemoveDirs: %s not found", directory) + + def run(self): + result = ActionResult.FAILURE + try: + self._run_internal() + result = ActionResult.SUCCESS + except OSError: + self.log.exception("RemoveDirs OSError") + return result + + # TODO: sync with ActionTypeEnum from common class ActionType(object): DELETE = 0 @@ -519,6 +547,7 @@ class ActionType(object): UPDATE_MODULE_MD = 8 # Deprecated BUILD_MODULE = 9 CANCEL_BUILD = 10 + REMOVE_DIRS = 11 class ActionQueueTask(QueueTask): diff --git a/backend/copr_backend/frontend.py b/backend/copr_backend/frontend.py index f19ed71..86a3297 100644 --- a/backend/copr_backend/frontend.py +++ b/backend/copr_backend/frontend.py @@ -8,7 +8,7 @@ import logging from copr_common.request import SafeRequest, RequestError from copr_backend.exceptions import FrontendClientException -MIN_FE_BE_API = 1 +MIN_FE_BE_API = 2 class FrontendClient: """ diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py index 037af16..e9248b3 100644 --- a/backend/tests/test_action.py +++ b/backend/tests/test_action.py @@ -973,3 +973,22 @@ class TestAction(object): with open(file, "r") as fd: lines = fd.readlines() assert lines == [text] + + @mock.patch("copr_backend.actions.shutil.rmtree") + def test_remove_dirs(self, mock_rmtree, mc_time): + _unused = mc_time + test_action = Action.create_from( + opts=self.opts, + action={ + "action_type": ActionType.REMOVE_DIRS, + "data": json.dumps([ + "@python/python3.8:pr:11", + "jdoe/some:pr:123", + ]), + }, + ) + assert test_action.run() == ActionResult.SUCCESS + assert mock_rmtree.call_args_list == [ + mock.call('/var/lib/copr/public_html/results/@python/python3.8:pr:11'), + mock.call('/var/lib/copr/public_html/results/jdoe/some:pr:123'), + ] diff --git a/backend/tests/test_frontend.py b/backend/tests/test_frontend.py index 2b56527..73b1c16 100644 --- a/backend/tests/test_frontend.py +++ b/backend/tests/test_frontend.py @@ -20,6 +20,9 @@ def post_req(): def f_request_method(request): 'mock the requests.{get,post,put} method' with mock.patch("copr_common.request.{}".format(request.param)) as ctx: + ctx.return_value.headers = { + "Copr-FE-BE-API-Version": "666", + } yield (request.param, ctx) diff --git a/common/copr_common/enums.py b/common/copr_common/enums.py index 0edf13b..6e3840e 100644 --- a/common/copr_common/enums.py +++ b/common/copr_common/enums.py @@ -35,6 +35,7 @@ class ActionTypeEnum(with_metaclass(EnumType, object)): "update_module_md": 8, "build_module": 9, "cancel_build": 10, + "remove_dirs": 11, } diff --git a/common/python-copr-common.spec b/common/python-copr-common.spec index 994cfec..880d434 100644 --- a/common/python-copr-common.spec +++ b/common/python-copr-common.spec @@ -16,7 +16,7 @@ %endif Name: python-copr-common -Version: 0.11 +Version: 0.11.1.dev Release: 1%{?dist} Summary: Python code used by Copr diff --git a/frontend/conf/cron.daily/copr-frontend b/frontend/conf/cron.daily/copr-frontend index afa2ff8..492c0bd 100644 --- a/frontend/conf/cron.daily/copr-frontend +++ b/frontend/conf/cron.daily/copr-frontend @@ -7,3 +7,4 @@ runuser -c '/usr/share/copr/coprs_frontend/manage.py vacuum-graphs' - copr-fe runuser -c '/usr/share/copr/coprs_frontend/manage.py clean-expired-projects' - copr-fe runuser -c '/usr/share/copr/coprs_frontend/manage.py clean-old-builds' - copr-fe +runuser -c '/usr/share/copr/coprs_frontend/manage.py delete-dirs' - copr-fe diff --git a/frontend/copr-frontend.spec b/frontend/copr-frontend.spec index 74b87ab..6f343bd 100644 --- a/frontend/copr-frontend.spec +++ b/frontend/copr-frontend.spec @@ -6,6 +6,8 @@ # https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_of_Additional_RPM_Macros %global macrosdir %(d=%{_rpmconfigdir}/macros.d; [ -d $d ] || d=%{_sysconfdir}/rpm; echo $d) +%global copr_common_version 0.11.1.dev + # Please bump the %%flavor_guard version every-time some incompatible change # happens (since the last release) in %%flavor_files set of files. Those files # are basically replaced by third-party flavor providers, and any file removal, @@ -75,7 +77,7 @@ BuildRequires: python3-click BuildRequires: python3-CommonMark BuildRequires: python3-blinker BuildRequires: python3-beautifulsoup4 -BuildRequires: python3-copr-common >= 0.7 +BuildRequires: python3-copr-common >= %copr_common_version BuildRequires: python3-email-validator BuildRequires: python3-dateutil BuildRequires: python3-decorator @@ -134,7 +136,7 @@ Requires: python3-click Requires: python3-CommonMark Requires: python3-alembic Requires: python3-blinker -Requires: python3-copr-common >= 0.7 +Requires: python3-copr-common >= %copr_common_version Requires: python3-dateutil Requires: python3-email-validator Requires: python3-flask diff --git a/frontend/coprs_frontend/commands/delete_dirs.py b/frontend/coprs_frontend/commands/delete_dirs.py new file mode 100644 index 0000000..b1e2786 --- /dev/null +++ b/frontend/coprs_frontend/commands/delete_dirs.py @@ -0,0 +1,21 @@ +""" +Admin/Cron logic for removing outdated CoprDir objects +""" + +import click + +from coprs import db +from coprs.logic.coprs_logic import CoprDirsLogic + +def _delete_dirs_function(): + CoprDirsLogic.send_delete_dirs_action() + db.session.commit() + +@click.command() +def delete_dirs(): + """ + Delete outdated pull request directories (e.g. like copr-dev:pr:123) + from the database and generate an action so the data are removed from + backend, too. + """ + _delete_dirs_function() diff --git a/frontend/coprs_frontend/config/copr_unit_test.conf b/frontend/coprs_frontend/config/copr_unit_test.conf index c32b5e5..49c7376 100644 --- a/frontend/coprs_frontend/config/copr_unit_test.conf +++ b/frontend/coprs_frontend/config/copr_unit_test.conf @@ -71,5 +71,9 @@ PAGURE_EVENTS = { 'io.pagure.prod.pagure.git.receive' : 'https://pagure.io/', 'io.pagure.prod.pagure.pull-request.new' : 'https://pagure.io/', 'io.pagure.prod.pagure.pull-request.comment.added' : 'https://pagure.io/', - 'io.pagure.prod.pagure.pull-request.updated' : 'https://pagure.io/' + 'io.pagure.prod.pagure.pull-request.updated' : 'https://pagure.io/', + 'org.fedoraproject.prod.pagure.git.receive': "https://src.fedoraproject.org/", + 'org.fedoraproject.prod.pagure.pull-request.new': "https://src.fedoraproject.org/", + 'org.fedoraproject.prod.pagure.pull-request.comment.added': "https://src.fedoraproject.org/", + 'org.fedoraproject.prod.pagure.pull-request.updated': "https://src.fedoraproject.org/", } diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py b/frontend/coprs_frontend/coprs/logic/coprs_logic.py index 76f49d1..4cb5f6b 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -1,4 +1,5 @@ import locale +import json import os import time import datetime @@ -563,6 +564,16 @@ class CoprDirsLogic(object): db.session.delete(copr_dir) @classmethod + def delete_with_builds(cls, copr_dir): + """ + Delete CoprDir istance from database, and transitively delete all + assigned Builds and BuildChroots. No Backend action is generated. + """ + models.Build.query.filter(models.Build.copr_dir_id==copr_dir.id)\ + .delete() + cls.delete(copr_dir) + + @classmethod def delete_all_by_copr(cls, copr): for copr_dir in copr.dirs: db.session.delete(copr_dir) @@ -616,7 +627,7 @@ class CoprDirsLogic(object): ) return ( db.session.query(models.CoprDir, subquery) - .filter(models.CoprDir.copr_id==copr_id) + .filter(models.CoprDir.copr_id==int(copr_id)) ) @classmethod @@ -666,6 +677,50 @@ class CoprDirsLogic(object): return results + @classmethod + def get_copr_ids_with_pr_dirs(cls): + """ + Get query returning all Copr instances that have some PR directories + """ + return ( + db.session.query(models.CoprDir.copr_id) + .filter(models.CoprDir.name.like("%:pr:%")) + .group_by(models.CoprDir.copr_id) + ) + + @classmethod + def send_delete_dirs_action(cls): + """ + Go through all projects, and check if there are some PR directories to + be removed. Generate delete action for them, and drop them. + """ + copr_ids = cls.get_copr_ids_with_pr_dirs().all() + + remove_dirs = [] + for copr_id in copr_ids: + copr_id = copr_id[0] + all_dirs = cls.get_all_with_latest_submitted_build(copr_id) + for copr_dir in all_dirs: + dir_object = copr_dir["copr_dir"] + if not copr_dir['delete']: + continue + + dirname = "{}/{}".format( + dir_object.copr.owner_name, + dir_object.name, + ) + print("{} is going to be deleted".format(dirname)) + cls.delete_with_builds(dir_object) + remove_dirs.append(dirname) + + action = models.Action( + action_type=ActionTypeEnum("remove_dirs"), + object_type="copr", + data=json.dumps(remove_dirs), + created_on=int(time.time())) + + db.session.add(action) + @listens_for(models.Copr.auto_createrepo, 'set') def on_auto_createrepo_change(target_copr, value_acr, old_value_acr, initiator): """ Emit createrepo action when auto_createrepo re-enabled""" diff --git a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py index 446b133..8291bfb 100644 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -27,7 +27,7 @@ def send_frontend_version(response): do something new with the protocol. On the Backend/builder side we can setup the version according to our needs. """ - response.headers['Copr-FE-BE-API-Version'] = '1' + response.headers['Copr-FE-BE-API-Version'] = '2' return response diff --git a/frontend/coprs_frontend/manage.py b/frontend/coprs_frontend/manage.py index b91b181..176ea2a 100755 --- a/frontend/coprs_frontend/manage.py +++ b/frontend/coprs_frontend/manage.py @@ -16,6 +16,7 @@ import commands.drop_db import commands.create_chroot import commands.alter_chroot import commands.display_chroots +import commands.delete_dirs import commands.drop_chroot import commands.branch_fedora import commands.comment_chroot @@ -82,6 +83,7 @@ commands_list = [ "clean_expired_projects", "clean_old_builds", "delete_orphans", + "delete_dirs", ] diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index 1c52e1f..52423ee 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -22,6 +22,7 @@ from coprs.logic.coprs_logic import BranchesLogic, CoprChrootsLogic from coprs.logic.dist_git_logic import DistGitLogic from tests.request_test_api import WebUIRequests, API3Requests, BackendRequests +from tests.lib.pagure_pull_requests import PullRequestTrigger class CoprsTestCase(object): @@ -75,6 +76,7 @@ class CoprsTestCase(object): self.web_ui = WebUIRequests(self) self.api3 = API3Requests(self) self.backend = BackendRequests(self) + self.pr_trigger = PullRequestTrigger(self) def teardown_method(self, method): # delete just data, not the tables diff --git a/frontend/coprs_frontend/tests/lib/__init__.py b/frontend/coprs_frontend/tests/lib/__init__.py new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/frontend/coprs_frontend/tests/lib/__init__.py diff --git a/frontend/coprs_frontend/tests/lib/pagure_pull_requests.py b/frontend/coprs_frontend/tests/lib/pagure_pull_requests.py new file mode 100644 index 0000000..b38ea62 --- /dev/null +++ b/frontend/coprs_frontend/tests/lib/pagure_pull_requests.py @@ -0,0 +1,121 @@ +""" +Helper methods for generating PR builds +""" + +import copy + +from unittest import mock + +from coprs import db, models +from copr_common.enums import StatusEnum + +from pagure_events import build_on_fedmsg_loop + + +class _Message: + def __init__(self, topic, body): + self.topic = topic + self.body = body + + +class PullRequestTrigger: + """ Trigger builds using "Pagure-like" message """ + test_object = None + + data = { + "msg": { + "agent": "test", + "pullrequest": { + "branch": "master", + "branch_from": "test_PR", + "id": 1, + "commit_start": "78a74b02771506daf8927b3391a669cbc32ccf10", + "commit_stop": "da3d120f2ff24fa730067735c19a0b52c8bc1a44", + "repo_from": { + "fullname": "test/copr/copr", + "url_path": "test/copr/copr", + }, + "project": { + "fullname": "test/copr/copr", + "url_path": "test/copr/copr", + }, + 'status': 'Open', + "comments": [], + 'user': { + "fullname": "John Doe", + "url_path": "user/jdoe", + "full_url": "https://src.fedoraproject.org/user/jdoe", + "name": "jdoe" + }, + } + } + } + + def __init__(self, test_object): + self.test_object = test_object + self.build = build_on_fedmsg_loop() + + @staticmethod + def _get_the_package(project, pkgname): + return models.Package.query.join(models.CoprDir).filter( + models.Package.copr_id==project.id, + models.Package.name==pkgname, + models.CoprDir.main).one_or_none() + + @mock.patch('pagure_events.get_repeatedly', mock.Mock()) + def build_package(self, project_name, pkgname, pr_id): + """ Trigger a build in Copr with name """ + project = models.Copr.query.filter( + models.Copr.name == project_name).one() + + package = self._get_the_package(project, pkgname) + + build_count = models.Build.query.count() + + if not package: + self.test_object.api3.create_distgit_package( + project.name, + pkgname, + {"webhook_rebuild": True}, + ) + db.session.commit() + + package = self._get_the_package(project, pkgname) + + data = copy.deepcopy(self.data['msg']) + data['pullrequest']['project'] = { + "fullname": "rpms/" + pkgname, + "url_path": "rpms/" + pkgname, + } + data['pullrequest']['id'] = int(pr_id) + message = _Message( + 'org.fedoraproject.prod.pagure.pull-request.updated', + data, + ) + with mock.patch('pagure_events.helpers.raw_commit_changes') as patch: + patch.return_value = { + 'tests/integration/conftest.py @@ -28,6 +28,16 @@ def test_env(): return env', + 'tests/integration/conftest.py b/tests/integration/conftest.py index ' + '1747874..a2b81f6 100644 --- a/tests/integration/conftest.py +++'} + self.build(message) + + builds = models.Build.query.order_by("id").all() + + build_count_new = models.Build.query.count() + assert build_count_new == build_count + 1 + + build = builds[-1] # last build + + build.source_status = StatusEnum("succeeded") + for bch in build.build_chroots: + bch.status = StatusEnum("succeeded") + + return build + + def build_package_with_args(self, project_name, pkgname, pr_id, + submitted_on=None): + """ Trigger a build in Copr with name and additional args """ + build = self.build_package(project_name, pkgname, pr_id) + if submitted_on is not None: + build.submitted_on = submitted_on + db.session.commit() diff --git a/frontend/coprs_frontend/tests/request_test_api.py b/frontend/coprs_frontend/tests/request_test_api.py index 162a848..15027ce 100644 --- a/frontend/coprs_frontend/tests/request_test_api.py +++ b/frontend/coprs_frontend/tests/request_test_api.py @@ -54,7 +54,7 @@ class _RequestsInterface: """ Modify CoprChroot """ raise NotImplementedError - def create_distgit_package(self, project, pkgname): + def create_distgit_package(self, project, pkgname, options=None): """ Modify CoprChroot """ raise NotImplementedError @@ -117,7 +117,9 @@ class WebUIRequests(_RequestsInterface): assert resp.status_code == 302 return resp - def create_distgit_package(self, project, pkgname): + def create_distgit_package(self, project, pkgname, options=None): + if options: + raise NotImplementedError data = { "package_name": pkgname, } @@ -270,10 +272,13 @@ class API3Requests(_RequestsInterface): resp = self.post(route, data) return resp - def create_distgit_package(self, project, pkgname): + def create_distgit_package(self, project, pkgname, options=None): route = "/api_3/package/add/{}/{}/{}/distgit".format( self.transaction_username, project, pkgname) - resp = self.post(route, {"package_name": pkgname}) + data = {"package_name": pkgname} + if options is not None: + data.update(options) + resp = self.post(route, data) return resp def rebuild_package(self, project, pkgname, build_options=None): @@ -300,7 +305,7 @@ class BackendRequests: def update(self, data): """ Post to the "/backend/update/" using a dict """ - self.client.post( + return self.client.post( "/backend/update/", content_type="application/json", headers=self.test_class_object.auth_header, diff --git a/frontend/coprs_frontend/tests/test_logic/test_copr_dirs_logic.py b/frontend/coprs_frontend/tests/test_logic/test_copr_dirs_logic.py new file mode 100644 index 0000000..0578199 --- /dev/null +++ b/frontend/coprs_frontend/tests/test_logic/test_copr_dirs_logic.py @@ -0,0 +1,98 @@ +""" +Tests for CoprDirsLogic +""" + +from datetime import datetime, timedelta +import json +import pytest +from coprs import db, models +from tests.coprs_test_case import ( + CoprsTestCase, + TransactionDecorator, + new_app_context +) +from commands.delete_dirs import _delete_dirs_function + + +class TestCoprDirsLogic(CoprsTestCase): + @TransactionDecorator("u1") + @new_app_context + @pytest.mark.usefixtures("f_users", "f_mock_chroots", "f_users_api", "f_db") + def test_coprdir_cleanup_no_removal(self): + self.api3.new_project("test-pr-dirs", ["fedora-17-i386"]) + self.pr_trigger.build_package("test-pr-dirs", "testpkg", 1) + self.pr_trigger.build_package("test-pr-dirs", "testpkg", 2) + self.pr_trigger.build_package("test-pr-dirs", "other", 1) + self.pr_trigger.build_package("test-pr-dirs", "testpkg", 1) + self.pr_trigger.build_package("test-pr-dirs", "testpkg", 3) + _delete_dirs_function() + # nothing got removed + assert models.Build.query.count() == 5 + assert models.CoprDir.query.count() == 4 # main + 3 PRs + + @TransactionDecorator("u1") + @new_app_context + @pytest.mark.usefixtures("f_users", "f_mock_chroots", "f_users_api", "f_db") + def test_coprdir_cleanup_no_build(self): + self.api3.new_project("test-pr-dirs", ["fedora-17-i386"]) + self.pr_trigger.build_package("test-pr-dirs", "testpkg", 1) + build = models.Build.query.get(1) + db.session.delete(build) + models.Action.query.delete() + db.session.commit() + assert models.Build.query.count() == 0 + assert models.CoprDir.query.count() == 2 + _delete_dirs_function() + # nothing got removed + assert models.Build.query.count() == 0 + assert models.CoprDir.query.count() == 1 + action = models.Action.query.one() + assert action.action_type == 11 + assert json.loads(action.data) == ["user1/test-pr-dirs:pr:1"] + + @TransactionDecorator("u1") + @new_app_context + @pytest.mark.usefixtures("f_users", "f_mock_chroots", "f_users_api", "f_db") + def test_coprdir_cleanup_old_pr(self): + self.api3.new_project("test-pr-dirs", ["fedora-17-i386"]) + old_build_on = datetime.timestamp(datetime.now() - timedelta(days=100)) + self.pr_trigger.build_package_with_args("test-pr-dirs", "testpkg", 1, + old_build_on) + models.Action.query.delete() + db.session.commit() + assert models.Build.query.count() == 1 + assert models.CoprDir.query.count() == 2 + _delete_dirs_function() + # nothing got removed + assert models.Build.query.count() == 0 + assert models.CoprDir.query.count() == 1 + action = models.Action.query.one() + assert action.action_type == 11 + assert json.loads(action.data) == ["user1/test-pr-dirs:pr:1"] + + @TransactionDecorator("u1") + @new_app_context + @pytest.mark.usefixtures("f_users", "f_mock_chroots", "f_users_api", "f_db") + def test_coprdir_cleanup_one_prs(self): + self.api3.new_project("test-pr-dirs", ["fedora-17-i386"]) + old_build_on = datetime.timestamp(datetime.now() - timedelta(days=100)) + self.pr_trigger.build_package_with_args("test-pr-dirs", "testpkg", 1, + old_build_on) + # this assures the pr:1 is kept + self.pr_trigger.build_package_with_args("test-pr-dirs", "another", 1) + self.pr_trigger.build_package_with_args("test-pr-dirs", "pr-2-package", 2, + old_build_on) + self.pr_trigger.build_package_with_args("test-pr-dirs", "pr-3-package", 3, + old_build_on) + models.Action.query.delete() + db.session.commit() + assert models.Build.query.count() == 4 + assert models.CoprDir.query.count() == 4 + _delete_dirs_function() + # nothing got removed + assert models.Build.query.count() == 2 + assert models.CoprDir.query.count() == 2 + action = models.Action.query.one() + assert action.action_type == 11 + assert set(json.loads(action.data)) == set(["user1/test-pr-dirs:pr:2", + "user1/test-pr-dirs:pr:3"]) From 564aa2fce48f132d625fef14c3036ac19fe75f41 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Jun 07 2021 12:34:01 +0000 Subject: [PATCH 4/4] frontend: pg-only index => pg|sqlite-only index We don't need to labor on this, as we anyway use PostgreSQL only + SQLite for unit-tests. Also avoid using the main==True construct which confuses PyLint. --- diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py index 17f02dd..224bcfd 100644 --- a/frontend/coprs_frontend/coprs/models.py +++ b/frontend/coprs_frontend/coprs/models.py @@ -743,11 +743,9 @@ class CoprDir(db.Model): copr = db.relationship("Copr", backref=db.backref("dirs")) __table_args__ = ( - # TODO: Drop the PG only index. Add (a) normal unique index, (b) add a - # check index for 'main == true' (NULLs are not calculated), and (c) - # flip all the false values to NULL. db.Index('only_one_main_copr_dir', copr_id, main, - unique=True, postgresql_where=(main==True)), + unique=True, postgresql_where=(main), + sqlite_where=(main)), db.UniqueConstraint('ownername', 'name', name='ownername_copr_dir_uniq'),