From 867d0d5c8a296549bbddf599374c7aadfc996be4 Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Tue, 19 Sep 2023 15:42:38 +0200 Subject: [PATCH] frontend: unify naming convention for safe methods Originally, safe methods were considered as methods raising ObjectNotFound for API (mainly with session.one() method). Now methods which returns None or ignores the error completely are also considered as "safe". For the sake of consistency: - *_noop_safe -> methods that ignores error completely (they basically logs) - *_none_safe -> returns None if object was not found in the database - *_safe -> raises an exception (ObjectNotFound) if no object found --- .../coprs/logic/complex_logic.py | 55 ++++++++++++------- .../coprs_frontend/coprs/logic/coprs_logic.py | 12 ++-- .../coprs/views/apiv3_ns/__init__.py | 2 +- .../views/apiv3_ns/apiv3_build_chroots.py | 2 +- .../coprs/views/apiv3_ns/apiv3_builds.py | 12 ++-- .../views/apiv3_ns/apiv3_project_chroots.py | 6 +- .../coprs/views/apiv3_ns/apiv3_projects.py | 2 +- .../coprs/views/backend_ns/backend_general.py | 10 ++-- .../coprs/views/coprs_ns/coprs_builds.py | 12 ++-- .../coprs/views/coprs_ns/coprs_chroots.py | 6 +- .../coprs/views/coprs_ns/coprs_general.py | 12 ++-- .../coprs/views/coprs_ns/coprs_packages.py | 12 ++-- .../coprs/views/groups_ns/groups_general.py | 2 +- frontend/coprs_frontend/coprs/views/misc.py | 6 +- .../coprs/views/user_ns/user_general.py | 4 +- .../views/webhooks_ns/webhooks_general.py | 12 ++-- frontend/coprs_frontend/pagure_events.py | 2 +- .../tests/test_logic/test_complex_logic.py | 12 ++-- 18 files changed, 97 insertions(+), 84 deletions(-) diff --git a/frontend/coprs_frontend/coprs/logic/complex_logic.py b/frontend/coprs_frontend/coprs/logic/complex_logic.py index 2bdb4bd45..8242e4612 100644 --- a/frontend/coprs_frontend/coprs/logic/complex_logic.py +++ b/frontend/coprs_frontend/coprs/logic/complex_logic.py @@ -65,7 +65,7 @@ def get_transitive_runtime_dependencies(cls, copr): for dep in analyzed_copr.runtime_deps: try: - copr_dep = cls.get_copr_by_repo_safe(dep) + copr_dep = cls.get_copr_by_repo(dep) except exceptions.ObjectNotFound: non_existing.add(dep) continue @@ -170,8 +170,8 @@ def fork_copr(cls, copr, user, dstname, dstgroup=None): return fcopr, created @staticmethod - def get_group_copr_safe(group_name, copr_name, **kwargs): - group = ComplexLogic.get_group_by_name_safe(group_name) + def get_group_copr(group_name, copr_name, **kwargs): + group = ComplexLogic.get_group_by_name(group_name) try: return CoprsLogic.get_by_group_id( group.id, copr_name, **kwargs).one() @@ -181,10 +181,10 @@ def get_group_copr_safe(group_name, copr_name, **kwargs): .format(group_name, copr_name)) @staticmethod - def get_copr_safe(user_name, copr_name, **kwargs): + def get_copr(user_name, copr_name, **kwargs): """ Get one project. - This always return personal project. For group projects see get_group_copr_safe(). + This always return personal project. For group projects see get_group_copr(). """ try: return CoprsLogic.get(user_name, copr_name, **kwargs).filter(Copr.group_id.is_(None)).one() @@ -194,13 +194,26 @@ def get_copr_safe(user_name, copr_name, **kwargs): .format(user_name, copr_name)) @staticmethod - def get_copr_by_owner_safe(owner_name, copr_name, **kwargs): + def get_copr_by_owner(owner_name, copr_name, **kwargs): if owner_name[0] == "@": - return ComplexLogic.get_group_copr_safe(owner_name[1:], copr_name, **kwargs) - return ComplexLogic.get_copr_safe(owner_name, copr_name, **kwargs) + return ComplexLogic.get_group_copr(owner_name[1:], copr_name, **kwargs) + return ComplexLogic.get_copr(owner_name, copr_name, **kwargs) @staticmethod - def get_copr_by_repo_safe(repo_url): + def get_copr_by_repo(repo_url): + """ + Safely get copr repo by repo url. + + Args: + repo_url: str + + Returns: + Copr repo or None in case of invalid url format or different url + scheme than copr. + + Raises: + ObjectNotFound if no such Copr (group) repo was found. + """ copr_repo = helpers.copr_repo_fullname(repo_url) if not copr_repo: return None @@ -209,10 +222,10 @@ def get_copr_by_repo_safe(repo_url): except: # invalid format, e.g. multiple slashes in copr_repo return None - return ComplexLogic.get_copr_by_owner_safe(owner, copr) + return ComplexLogic.get_copr_by_owner(owner, copr) @staticmethod - def get_copr_dir_safe(ownername, copr_dirname, **kwargs): + def get_copr_dir(ownername, copr_dirname): try: return CoprDirsLogic.get_by_ownername(ownername, copr_dirname).one() except sqlalchemy.orm.exc.NoResultFound: @@ -220,7 +233,7 @@ def get_copr_dir_safe(ownername, copr_dirname, **kwargs): .format(ownername, copr_dirname)) @staticmethod - def get_copr_by_id_safe(copr_id): + def get_copr_by_id(copr_id): try: return CoprsLogic.get_by_id(copr_id).one() except sqlalchemy.orm.exc.NoResultFound: @@ -229,7 +242,7 @@ def get_copr_by_id_safe(copr_id): .format(copr_id)) @staticmethod - def get_build_safe(build_id): + def get_build(build_id): try: return BuildsLogic.get_by_id(build_id).one() except (sqlalchemy.orm.exc.NoResultFound, @@ -243,7 +256,7 @@ def get_build_chroot(build_id, chrootname): Return a `models.BuildChroot` instance based on build ID and name of the chroot. If there is no such chroot, `ObjectNotFound` execption is raised. """ - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) try: return build.chroots_dict_by_name[chrootname] except KeyError: @@ -253,7 +266,7 @@ def get_build_chroot(build_id, chrootname): raise ObjectNotFound(message=msg) @staticmethod - def get_package_by_id_safe(package_id): + def get_package_by_id(package_id): try: return PackagesLogic.get_by_id(package_id).one() except sqlalchemy.orm.exc.NoResultFound: @@ -261,7 +274,7 @@ def get_package_by_id_safe(package_id): message="Package {} does not exist.".format(package_id)) @staticmethod - def get_package_safe(copr, package_name): + def get_package(copr, package_name): try: return PackagesLogic.get(copr.id, package_name).one() except sqlalchemy.orm.exc.NoResultFound: @@ -270,7 +283,7 @@ def get_package_safe(copr, package_name): .format(package_name, copr)) @staticmethod - def get_group_by_name_safe(group_name): + def get_group_by_name(group_name): try: group = UsersLogic.get_group_by_alias(group_name).one() except sqlalchemy.orm.exc.NoResultFound: @@ -279,9 +292,9 @@ def get_group_by_name_safe(group_name): return group @staticmethod - def get_copr_chroot_safe(copr, chroot_name): + def get_copr_chroot(copr, chroot_name): try: - chroot = CoprChrootsLogic.get_by_name_safe(copr, chroot_name) + chroot = CoprChrootsLogic.get_by_name_or_none(copr, chroot_name) except (ValueError, KeyError, RuntimeError) as e: raise ObjectNotFound(message=str(e)) @@ -552,14 +565,14 @@ def get_additional_repo_views(cls, repos_list, chroot_id): "name": "Additional repo " + helpers.generate_repo_name(repo), } - # We ask get_copr_by_repo_safe() here only to resolve the + # We ask get_copr_by_repo() here only to resolve the # module_hotfixes attribute. If the asked project doesn't exist, we # still adjust the 'repos' variable -- the build will eventually # fail on repo downloading, but at least the copr maintainer will be # notified about the misconfiguration. Better than just skip the # repo. try: - copr = ComplexLogic.get_copr_by_repo_safe(repo) + copr = ComplexLogic.get_copr_by_repo(repo) except ObjectNotFound: copr = None if copr and copr.module_hotfixes: diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py b/frontend/coprs_frontend/coprs/logic/coprs_logic.py index d3a41746c..f75fc0238 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -185,7 +185,7 @@ def get_by_ownername(cls, ownername): return cls.filter_by_ownername(query, ownername) @classmethod - def get_by_ownername_coprname(cls, ownername, coprname): + def get_by_ownername_coprname_safe(cls, ownername, coprname): """ Return a Copr object owned by OWNERNAME (either '@groupname' or 'username') with a name COPRNAME. @@ -206,7 +206,7 @@ def get_by_ownername_and_dirname(cls, ownername, dirname): # Some of the calling methods (routes) strictly require us not to check # if the dirname exists. coprname = CoprDirsLogic.copr_name_from_dirname(dirname) - return cls.get_by_ownername_coprname(ownername, coprname) + return cls.get_by_ownername_coprname_safe(ownername, coprname) @classmethod def filter_without_ids(cls, query, ids): @@ -622,7 +622,7 @@ def request_permission(cls, copr, user, permission, req_bool): class CoprDirsLogic(object): @classmethod - def get_by_copr_safe(cls, copr, dirname): + def get_by_copr_or_none(cls, copr, dirname): """ Return _query_ for getting CoprDir by Copr and dirname """ @@ -637,7 +637,7 @@ def get_by_copr(cls, copr, dirname): Return CoprDir instance per given Copr instance and dirname. Raise ObjectNotFound if it doesn't exist. """ - coprdir = cls.get_by_copr_safe(copr, dirname) + coprdir = cls.get_by_copr_or_none(copr, dirname) if not coprdir: raise exceptions.ObjectNotFound( "Dirname '{}' doesn't exist in '{}' copr".format( @@ -669,7 +669,7 @@ def get_or_create(cls, copr, dirname, trusted_caller=False): submitted. We don't create the "main" CoprDirs here (those are created when a new project is created. """ - copr_dir = cls.get_by_copr_safe(copr, dirname) + copr_dir = cls.get_by_copr_or_none(copr, dirname) if copr_dir: return copr_dir @@ -948,7 +948,7 @@ def get_by_name(cls, copr, chroot_name, active_only=True): return cls.get_by_mock_chroot_id(copr, mc.id) @classmethod - def get_by_name_safe(cls, copr, chroot_name): + def get_by_name_or_none(cls, copr, chroot_name): """ :rtype: models.CoprChroot """ diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py index d15e9f307..cbb6953fe 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py @@ -123,7 +123,7 @@ def get_copr(ownername=None, projectname=None): request = flask.request ownername = ownername or request.form.get("ownername") or request.json["ownername"] projectname = projectname or request.form.get("projectname") or request.json["projectname"] - return ComplexLogic.get_copr_by_owner_safe(ownername, projectname) + return ComplexLogic.get_copr_by_owner(ownername, projectname) class Paginator(object): diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_build_chroots.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_build_chroots.py index dc5ec446c..868dd9f7d 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_build_chroots.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_build_chroots.py @@ -19,7 +19,7 @@ def to_dict(build_chroot): def build_config(build_chroot): config = BuildConfigLogic.generate_build_config(build_chroot.build.copr, build_chroot.name) - copr_chroot = CoprChrootsLogic.get_by_name_safe(build_chroot.build.copr, build_chroot.name) + copr_chroot = CoprChrootsLogic.get_by_name_or_none(build_chroot.build.copr, build_chroot.name) dict_data = { "repos": config.get("repos"), "additional_repos": BuildConfigLogic.generate_additional_repos(copr_chroot), diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py index 83a352ece..bca1c4601 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py @@ -102,7 +102,7 @@ def get(self, build_id): Get a build Get details for a single Copr build. """ - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) result = to_dict(build) # Workaround - `marshal_with` needs the input `build_id` to be present @@ -154,13 +154,13 @@ def get_build_list(ownername, projectname, packagename=None, status=None, **kwar @apiv3_ns.route("/build/source-chroot//", methods=GET) def get_source_chroot(build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) return flask.jsonify(to_source_chroot(build)) @apiv3_ns.route("/build/source-build-config//", methods=GET) def get_source_build_config(build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) return flask.jsonify(to_source_build_config(build)) @@ -169,14 +169,14 @@ def get_build_built_packages(build_id): """ Return built packages (NEVRA dicts) for a given build """ - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) return flask.jsonify(build.results_dict) @apiv3_ns.route("/build/cancel/", methods=PUT) @api_login_required def cancel_build(build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) BuildsLogic.cancel_build(flask.g.user, build) db.session.commit() return render_build(build) @@ -396,7 +396,7 @@ def process_creating_new_build(copr, form, create_new_build): @apiv3_ns.route("/build/delete/", methods=DELETE) @api_login_required def delete_build(build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) build_dict = to_dict(build) BuildsLogic.delete_build(flask.g.user, build) db.session.commit() diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_project_chroots.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_project_chroots.py index 0fe3f9ab1..a62043f04 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_project_chroots.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_project_chroots.py @@ -86,7 +86,7 @@ def get(self): """ args = self.parser.parse_args() copr = get_copr(args.ownername, args.projectname) - chroot = ComplexLogic.get_copr_chroot_safe(copr, args.chrootname) + chroot = ComplexLogic.get_copr_chroot(copr, args.chrootname) return to_dict(chroot) @@ -103,7 +103,7 @@ def get(self): """ args = self.parser.parse_args() copr = get_copr(args.ownername, args.projectname) - chroot = ComplexLogic.get_copr_chroot_safe(copr, args.chrootname) + chroot = ComplexLogic.get_copr_chroot(copr, args.chrootname) if not chroot: raise ObjectNotFound('Chroot not found.') return to_build_config_dict(chroot) @@ -116,7 +116,7 @@ def edit_project_chroot(ownername, projectname, chrootname): copr = get_copr(ownername, projectname) data = rename_fields(get_form_compatible_data(preserve=["additional_modules"])) form = forms.ModifyChrootForm(data, meta={'csrf': False}) - chroot = ComplexLogic.get_copr_chroot_safe(copr, chrootname) + chroot = ComplexLogic.get_copr_chroot(copr, chrootname) if not form.validate_on_submit(): raise InvalidForm(form) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py index dd5e99133..80fd1f919 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py @@ -70,7 +70,7 @@ def owner2tuple(ownername): user = flask.g.user group = None if ownername[0] == "@": - group = ComplexLogic.get_group_by_name_safe(ownername[1:]) + group = ComplexLogic.get_group_by_name(ownername[1:]) elif ownername != flask.g.user.name: user = UsersLogic.get(ownername).first() if not user: 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 18c916f8a..018ea57d6 100755 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -5,7 +5,7 @@ from coprs.logic import actions_logic from coprs.logic.builds_logic import BuildsLogic from coprs.logic.complex_logic import ComplexLogic, BuildConfigLogic -from coprs.logic.coprs_logic import MockChrootsLogic +from coprs.logic.coprs_logic import CoprChrootsLogic, MockChrootsLogic from coprs.exceptions import CoprHttpException, ObjectNotFound from coprs.helpers import streamed_json @@ -81,7 +81,7 @@ def dist_git_upload_completed(): build_id = flask.request.json.get("build_id") try: - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) except ObjectNotFound: return flask.jsonify({"updated": False}) @@ -168,7 +168,7 @@ def get_build_record(task, for_backend=False): "repo_priority": task.build.copr.repo_priority }) - copr_chroot = ComplexLogic.get_copr_chroot_safe(task.build.copr, task.mock_chroot.name) + copr_chroot = CoprChrootsLogic.get_by_name_or_none(task.build.copr, task.mock_chroot.name) modules = copr_chroot.module_setup_commands if modules: build_record["modules"] = {'toggle': modules} @@ -419,7 +419,7 @@ def starting_build(): data = flask.request.json try: - build = ComplexLogic.get_build_safe(data.get('build_id')) + build = ComplexLogic.get_build(data.get('build_id')) except ObjectNotFound: return flask.jsonify({"can_start": False}) @@ -440,7 +440,7 @@ def reschedule_build_chroot(): chroot = flask.request.json.get("chroot") try: - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) except ObjectNotFound: response["result"] = "noop" response["msg"] = "Build {} wasn't found".format(build_id) 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 eb298ead7..51eb97965 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -33,7 +33,7 @@ @coprs_ns.route("/build//") def copr_build_redirect(build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) copr = build.copr return flask.redirect(helpers.copr_url("coprs_ns.copr_build", copr, build_id=build_id)) @@ -53,7 +53,7 @@ def copr_build(copr, build_id): def render_copr_build(build_id, copr): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) return render_template("coprs/detail/build.html", build=build, copr=copr) @@ -440,7 +440,7 @@ def copr_new_build_rebuild(copr, build_id): view='coprs_ns.copr_new_build' url_on_success = helpers.copr_url("coprs_ns.copr_builds", copr) def factory(**build_options): - source_build = ComplexLogic.get_build_safe(build_id) + source_build = ComplexLogic.get_build(build_id) BuildsLogic.create_new_from_other_build( flask.g.user, copr, source_build, chroot_names=form.selected_chroots, @@ -458,7 +458,7 @@ def factory(**build_options): @login_required @req_with_copr def copr_repeat_build(copr, build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) if not flask.g.user.can_build_in(build.copr): flask.flash("You are not allowed to repeat this build.") @@ -520,7 +520,7 @@ def process_cancel_build(build): @req_with_copr def copr_cancel_build(copr, build_id): # only the user who ran the build can cancel it - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) return process_cancel_build(build) @@ -530,7 +530,7 @@ def copr_cancel_build(copr, build_id): methods=["POST"]) @login_required def copr_delete_build(username, coprname, build_id): - build = ComplexLogic.get_build_safe(build_id) + build = ComplexLogic.get_build(build_id) try: builds_logic.BuildsLogic.delete_build(flask.g.user, build) diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py index 0465601f7..6326ed85b 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_chroots.py @@ -20,7 +20,7 @@ @req_with_copr def chroot_edit(copr, chrootname): """ Route for editing CoprChroot's """ - chroot = ComplexLogic.get_copr_chroot_safe(copr, chrootname) + chroot = ComplexLogic.get_copr_chroot(copr, chrootname) form = forms.ChrootForm(buildroot_pkgs=chroot.buildroot_pkgs, repos=chroot.repos, module_toggle=chroot.module_toggle, with_opts=chroot.with_opts, without_opts=chroot.without_opts, @@ -51,7 +51,7 @@ def render_chroot_edit(form, copr, chroot): def chroot_update(copr, chrootname): chroot_name = chrootname form = forms.ChrootForm() - chroot = ComplexLogic.get_copr_chroot_safe(copr, chroot_name) + chroot = ComplexLogic.get_copr_chroot(copr, chroot_name) if not flask.g.user.can_edit(copr): raise AccessRestricted( @@ -102,5 +102,5 @@ def chroot_view_comps(copr, chrootname): def render_chroot_view_comps(copr, chroot_name): - chroot = ComplexLogic.get_copr_chroot_safe(copr, chroot_name) + chroot = ComplexLogic.get_copr_chroot(copr, chroot_name) return Response(chroot.comps or "", mimetype="text/plain; charset=utf-8") diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index 615731140..a493e26d2 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -241,7 +241,7 @@ def copr_add(username=None, group_name=None): for chroot in MockChrootsLogic.get_multiple(active_only=True): comments[chroot.name] = chroot.comment if group_name: - group = ComplexLogic.get_group_by_name_safe(group_name) + group = ComplexLogic.get_group_by_name(group_name) return flask.render_template("coprs/group_add.html", form=form, group=group, comments=comments) return flask.render_template("coprs/add.html", form=form, comments=comments) @@ -257,7 +257,7 @@ def copr_new(username=None, group_name=None): group = None redirect = "coprs/add.html" if group_name: - group = ComplexLogic.get_group_by_name_safe(group_name) + group = ComplexLogic.get_group_by_name(group_name) redirect = "coprs/group_add.html" form = forms.CoprFormFactory.create_form_cls(group=group)() @@ -737,8 +737,8 @@ def process_copr_repositories(copr, on_success): raise ValidationError("Ambiguous to what project the chroot belongs") if not copr: - copr = ComplexLogic.get_copr_by_owner_safe(form.ownername.data, - form.projectname.data) + copr = ComplexLogic.get_copr_by_owner(form.ownername.data, + form.projectname.data) if not flask.g.user.can_edit(copr): flask.flash("You don't have access to this page.", "error") return flask.redirect(url_for_copr_details(copr)) @@ -766,7 +766,7 @@ def process_copr_repositories(copr, on_success): @coprs_ns.route("/id//createrepo/", methods=["POST"]) @login_required def copr_createrepo(copr_id): - copr = ComplexLogic.get_copr_by_id_safe(copr_id) + copr = ComplexLogic.get_copr_by_id(copr_id) if not flask.g.user.can_edit(copr): flask.flash( "You are not allowed to recreate repository metadata of copr with id {}.".format(copr_id), "error") @@ -939,7 +939,7 @@ def render_generate_repo_file(copr_dir, name_release, arch=None): owner_name = runtime_dep.owner.name if isinstance(runtime_dep.owner, models.Group): owner_name = "@{0}".format(owner_name) - copr_dep_dir = ComplexLogic.get_copr_dir_safe(owner_name, runtime_dep.name) + copr_dep_dir = ComplexLogic.get_copr_dir(owner_name, runtime_dep.name) response_content += "\n" + render_repo_template(copr_dep_dir, mock_chroot, runtime_dep=dep_idx, dependent=copr_dir.copr) diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_packages.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_packages.py index 527c73d1d..ac6089247 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_packages.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_packages.py @@ -63,7 +63,7 @@ def copr_packages(copr, page=1): @coprs_ns.route("/g///package//") @req_with_copr def copr_package(copr, package_name): - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) return flask.render_template("coprs/detail/package.html", package=package, copr=copr) @coprs_ns.route("///package//status_image/last_build.png") @@ -71,7 +71,7 @@ def copr_package(copr, package_name): @req_with_copr def copr_package_icon(copr, package_name): try: - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) except ObjectNotFound: return send_file("static/status_images/bad_url.png", mimetype='image/png') @@ -96,7 +96,7 @@ def copr_rebuild_all_packages(copr): try: packages = [] for package_name in form.packages.data: - packages.append(ComplexLogic.get_package_safe(copr, package_name)) + packages.append(ComplexLogic.get_package(copr, package_name)) PackagesLogic.batch_build( flask.g.user, @@ -126,7 +126,7 @@ def copr_rebuild_all_packages(copr): @coprs_ns.route("/g///package//rebuild") @req_with_copr def copr_rebuild_package(copr, package_name): - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) data = package.source_json_dict if package.source_type_text == "scm": @@ -204,7 +204,7 @@ def copr_new_package(copr, source_type_text): @coprs_ns.route("/g///package//edit/") @req_with_copr def copr_edit_package(copr, package_name, source_type_text=None, **kwargs): - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) data = package.source_json_dict data["webhook_rebuild"] = package.webhook_rebuild data["chroot_denylist"] = package.chroot_denylist_raw @@ -311,7 +311,7 @@ def process_save_package(copr, source_type_text, package_name, view, view_method @login_required @req_with_copr def copr_delete_package(copr, package_id): - package = ComplexLogic.get_package_by_id_safe(package_id) + package = ComplexLogic.get_package_by_id(package_id) try: PackagesLogic.delete_package(flask.g.user, package) diff --git a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py index 92be07286..00bd432c2 100644 --- a/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py +++ b/frontend/coprs_frontend/coprs/views/groups_ns/groups_general.py @@ -65,7 +65,7 @@ def activate_group(fas_group): @groups_ns.route("/g//coprs/", defaults={"page": 1}) @groups_ns.route("/g//coprs/") def list_projects_by_group(group_name, page=1): - group = ComplexLogic.get_group_by_name_safe(group_name) + group = ComplexLogic.get_group_by_name(group_name) pinned = [pin.copr for pin in PinnedCoprsLogic.get_by_group_id(group.id)] if page == 1 else [] query = CoprsLogic.get_multiple_by_group_id(group.id) diff --git a/frontend/coprs_frontend/coprs/views/misc.py b/frontend/coprs_frontend/coprs/views/misc.py index 0f0c4f76a..8f6db88f7 100644 --- a/frontend/coprs_frontend/coprs/views/misc.py +++ b/frontend/coprs_frontend/coprs/views/misc.py @@ -238,10 +238,10 @@ def wrapper(**kwargs): coprname = kwargs.pop("coprname") if "group_name" in kwargs: group_name = kwargs.pop("group_name") - copr = ComplexLogic.get_group_copr_safe(group_name, coprname, with_mock_chroots=True) + copr = ComplexLogic.get_group_copr(group_name, coprname, with_mock_chroots=True) else: username = kwargs.pop("username") - copr = ComplexLogic.get_copr_safe(username, coprname, with_mock_chroots=True) + copr = ComplexLogic.get_copr(username, coprname, with_mock_chroots=True) return f(copr, **kwargs) return wrapper @@ -254,7 +254,7 @@ def wrapper(**kwargs): else: ownername = kwargs.pop("username") copr_dirname = kwargs.pop("copr_dirname") - copr_dir = ComplexLogic.get_copr_dir_safe(ownername, copr_dirname) + copr_dir = ComplexLogic.get_copr_dir(ownername, copr_dirname) return f(copr_dir, **kwargs) return wrapper diff --git a/frontend/coprs_frontend/coprs/views/user_ns/user_general.py b/frontend/coprs_frontend/coprs/views/user_ns/user_general.py index 9bb8a3e3d..fa5b637bf 100644 --- a/frontend/coprs_frontend/coprs/views/user_ns/user_general.py +++ b/frontend/coprs_frontend/coprs/views/user_ns/user_general.py @@ -49,7 +49,7 @@ def delete_data(): @user_ns.route("/customize-pinned/") @login_required def pinned_projects(group_name=None): - owner = flask.g.user if not group_name else ComplexLogic.get_group_by_name_safe(group_name) + owner = flask.g.user if not group_name else ComplexLogic.get_group_by_name(group_name) return render_pinned_projects(owner) @@ -76,7 +76,7 @@ def render_pinned_projects(owner, form=None): @user_ns.route("/customize-pinned/", methods=["POST"]) @login_required def pinned_projects_post(group_name=None): - owner = flask.g.user if not group_name else ComplexLogic.get_group_by_name_safe(group_name) + owner = flask.g.user if not group_name else ComplexLogic.get_group_by_name(group_name) url_on_success = helpers.owner_url(owner) return process_pinned_projects_post(owner, url_on_success) diff --git a/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py b/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py index c3d1fa417..a60f03c90 100644 --- a/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py +++ b/frontend/coprs_frontend/coprs/views/webhooks_ns/webhooks_general.py @@ -51,7 +51,7 @@ def decorated_function(**kwargs): copr_id = kwargs.pop('copr_id') try: - copr = ComplexLogic.get_copr_by_id_safe(copr_id) + copr = ComplexLogic.get_copr_by_id(copr_id) except ObjectNotFound: return "PROJECT_NOT_FOUND\n", 404 @@ -71,7 +71,7 @@ def decorated_function(copr, **kwargs): package_name = kwargs.pop('package_name') try: - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) except ObjectNotFound: return "PACKAGE_NOT_FOUND\n", 404 @@ -85,7 +85,7 @@ def decorated_function(copr, **kwargs): def webhooks_bitbucket_push(copr_id, uuid, pkg_name: Optional[str] = None): # For the documentation of the data we receive see: # https://confluence.atlassian.com/bitbucket/event-payloads-740262817.html - copr = ComplexLogic.get_copr_by_id_safe(copr_id) + copr = ComplexLogic.get_copr_by_id(copr_id) if copr.webhook_secret != uuid: raise AccessRestricted("This webhook is not valid") @@ -128,7 +128,7 @@ def webhooks_git_push(copr_id: int, uuid, pkg_name: Optional[str] = None): return "OK", 200 # For the documentation of the data we receive see: # https://developer.github.com/v3/activity/events/types/#pushevent - copr = ComplexLogic.get_copr_by_id_safe(copr_id) + copr = ComplexLogic.get_copr_by_id(copr_id) if copr.webhook_secret != uuid: raise AccessRestricted("This webhook is not valid") @@ -175,7 +175,7 @@ def webhooks_git_push(copr_id: int, uuid, pkg_name: Optional[str] = None): def webhooks_gitlab_push(copr_id: int, uuid, pkg_name: Optional[str] = None): # For the documentation of the data we receive see: # https://gitlab.com/help/user/project/integrations/webhooks#events - copr = ComplexLogic.get_copr_by_id_safe(copr_id) + copr = ComplexLogic.get_copr_by_id(copr_id) if copr.webhook_secret != uuid: raise AccessRestricted("This webhook is not valid") @@ -267,7 +267,7 @@ def webhooks_coprdir_custom(ownername, dirname, uuid, package_name): return "PROJECT_NOT_FOUND\n", 404 try: - package = ComplexLogic.get_package_safe(copr, package_name) + package = ComplexLogic.get_package(copr, package_name) except ObjectNotFound: return "PACKAGE_NOT_FOUND\n", 404 diff --git a/frontend/coprs_frontend/pagure_events.py b/frontend/coprs_frontend/pagure_events.py index ae651b190..a46dab0dd 100755 --- a/frontend/coprs_frontend/pagure_events.py +++ b/frontend/coprs_frontend/pagure_events.py @@ -51,7 +51,7 @@ def __init__(self, db_row): self.committish = self.source_json_dict.get('committish') or '' self.subdirectory = self.source_json_dict.get('subdirectory') or '' - self.package = ComplexLogic.get_package_by_id_safe(db_row.id) + self.package = ComplexLogic.get_package_by_id(db_row.id) self.copr = self.package.copr def build(self, source_dict_update, copr_dir, update_callback, diff --git a/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py b/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py index 5b60c0c93..46bd210d4 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_complex_logic.py @@ -233,15 +233,15 @@ def test_forking_into_existing_project(self): def test_copr_by_repo_safe(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db): - assert ComplexLogic.get_copr_by_repo_safe("xxx") == None - assert ComplexLogic.get_copr_by_repo_safe("copr://") == None - assert ComplexLogic.get_copr_by_repo_safe("copr://a/b/c") == None + assert ComplexLogic.get_copr_by_repo("xxx") == None + assert ComplexLogic.get_copr_by_repo("copr://") == None + assert ComplexLogic.get_copr_by_repo("copr://a/b/c") == None - assert ComplexLogic.get_copr_by_repo_safe("copr://user1/foocopr") != None + assert ComplexLogic.get_copr_by_repo("copr://user1/foocopr") != None # we could fix these in future - assert ComplexLogic.get_copr_by_repo_safe("copr:///user1/foocopr") == None - assert ComplexLogic.get_copr_by_repo_safe("copr://user1//foocopr") == None + assert ComplexLogic.get_copr_by_repo("copr:///user1/foocopr") == None + assert ComplexLogic.get_copr_by_repo("copr://user1//foocopr") == None @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds", "f_db")