From 16ee4f5a9486d77cd993c9e7d4a825f75a745d40 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Oct 09 2020 12:12:47 +0000 Subject: frontend: don't re-set Build.package value When the package is already assigned to build, we don't want to re-assign it to a different package - even if the source RPM had a different name (pkg_name in /update/ route payload). The `Package` object basically only holds a concrete build configuration, and so picking a different configuration means that the build may suddenly start behaving against the user's expectation (e.g. the chroot selected chroot list we build into may be different). Spotted in PR#1530 by @schlupov. --- diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py index 8f6b530..4f18a2a 100644 --- a/frontend/coprs_frontend/coprs/logic/builds_logic.py +++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py @@ -797,10 +797,11 @@ class BuildsLogic(object): """ log.info("Updating build {} by: {}".format(build.id, upd_dict)) - # create the package if it doesn't exist pkg_name = upd_dict.get('pkg_name', None) - if pkg_name: + if not build.package and pkg_name: + # assign the package if it isn't already if not PackagesLogic.get(build.copr_dir.id, pkg_name).first(): + # create the package if it doesn't exist try: package = PackagesLogic.add( build.copr.user, build.copr_dir, diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index 7f51304..c1ed81d 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -20,7 +20,7 @@ from coprs import cache from coprs.logic.coprs_logic import BranchesLogic, CoprChrootsLogic from coprs.logic.dist_git_logic import DistGitLogic -from tests.request_test_api import WebUIRequests, API3Requests +from tests.request_test_api import WebUIRequests, API3Requests, BackendRequests class CoprsTestCase(object): @@ -74,6 +74,7 @@ class CoprsTestCase(object): self.web_ui = WebUIRequests(self) self.api3 = API3Requests(self) + self.backend = BackendRequests(self) def teardown_method(self, method): # delete just data, not the tables diff --git a/frontend/coprs_frontend/tests/request_test_api.py b/frontend/coprs_frontend/tests/request_test_api.py index 483114b..d14baaf 100644 --- a/frontend/coprs_frontend/tests/request_test_api.py +++ b/frontend/coprs_frontend/tests/request_test_api.py @@ -2,6 +2,8 @@ Library that simplifies interacting with Frontend routes. """ +import json + from bs4 import BeautifulSoup from coprs import models @@ -114,6 +116,8 @@ class WebUIRequests(_RequestsInterface): @staticmethod def _form_data_from_build_options(build_options): + if build_options is None: + build_options = {} form_data = {} chroots = build_options.get("chroots") if chroots: @@ -207,6 +211,8 @@ class API3Requests(_RequestsInterface): @staticmethod def _form_data_from_build_options(build_options): + if not build_options: + build_options = {} form_data = {} for arg in ["chroots", "bootstrap"]: if arg not in build_options: @@ -242,3 +248,33 @@ class API3Requests(_RequestsInterface): "package_name": pkgname, } return self.post(route, rebuild_data) + + +class BackendRequests: + """ Requests on /backend/ namespace """ + def __init__(self, test_class_object): + self.test_class_object = test_class_object + + @property + def client(self): + """ Initialized flask http client """ + return self.test_class_object.test_client + + def update(self, data): + """ Post to the "/backend/update/" using a dict """ + self.client.post( + "/backend/update/", + content_type="application/json", + headers=self.test_class_object.auth_header, + data=json.dumps(data), + ) + + def importing_queue(self): + """ return the dict with importing tasks """ + resp = self.client.get( + "/backend/importing/", + content_type="application/json", + headers=self.test_class_object.auth_header, + ) + assert resp.status_code == 200 + return json.loads(resp.data) diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py index f75e6cd..ebd5c65 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py @@ -1,8 +1,9 @@ # -*- encoding: utf-8 -*- + import json +import time import pytest -import time from sqlalchemy.orm.exc import NoResultFound from coprs import models @@ -21,6 +22,8 @@ from tests.coprs_test_case import CoprsTestCase, TransactionDecorator class TestBuildsLogic(CoprsTestCase): + # pylint: disable=too-many-public-methods + data = """ { "builds":[ @@ -417,3 +420,64 @@ class TestBuildsLogic(CoprsTestCase): builds = models.Build.query.all() assert len(builds) == 2 assert {b.package.name for b in builds} == {"tar", "cpio"} + + @TransactionDecorator("u1") + @pytest.mark.usefixtures("f_users", "f_users_api", "f_mock_chroots", "f_db") + def test_package_not_updated_after_source_ready(self): + # create a package and submit a build + self.web_ui.new_project("test", ["fedora-rawhide-i386"]) + self.web_ui.create_distgit_package("test", "copr-cli") + self.api3.rebuild_package("test", "copr-cli") + + build = models.Build.query.get(1) + assert build.status == StatusEnum("pending") + + form_data = { + "builds": [{ + "id": 1, + "task_id": "1", + "srpm_url": "http://foo", + "status": 1, + "pkg_name": "foo", # not a 'copr-cli'! + "pkg_version": 1 + }], + } + + self.backend.update(form_data) + + importing = self.backend.importing_queue() + assert len(importing) == 1 + assert importing[0]['pkg_name'] == "copr-cli" + + build = models.Build.query.get(1) + assert build.status == StatusEnum("importing") + assert build.package.name == "copr-cli" + + @TransactionDecorator("u1") + @pytest.mark.usefixtures("f_users", "f_users_api", "f_mock_chroots", "f_db") + def test_package_set_when_source_ready(self): + self.web_ui.new_project("test", ["fedora-rawhide-i386"]) + self.web_ui.submit_url_build("test") + + build = models.Build.query.get(1) + assert len(build.build_chroots) == 0 + assert build.source_status == StatusEnum("pending") + assert build.package is None + + # define the package name to foo + form_data = { + "builds": [{ + "id": 1, + "task_id": "1", + "srpm_url": "http://foo", + "status": 1, + "pkg_name": "foo", # not a 'copr-cli'! + "pkg_version": 1 + }], + } + + self.backend.update(form_data) + build = models.Build.query.get(1) + assert len(build.build_chroots) == 1 + assert build.source_status == StatusEnum("importing") + assert build.package.name == "foo"