From 44aa094c5389eb8e9caf106f37ff85a4c9111a8d 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: - *_or_none -> returns None if object was not found in the database - *_safe -> methods that ignores error completely (they basically log and/or are safe from exceptions) and renaming some of the current safe methods to normal ones that aren't safe by the new rules --- CONTRIBUTE.md | 9 + .../coprs/logic/complex_logic.py | 210 ++++++++++++++---- .../coprs_frontend/coprs/logic/coprs_logic.py | 18 +- .../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 +- 19 files changed, 240 insertions(+), 111 deletions(-) diff --git a/CONTRIBUTE.md b/CONTRIBUTE.md index 62f2ec232..2f1265129 100644 --- a/CONTRIBUTE.md +++ b/CONTRIBUTE.md @@ -40,5 +40,14 @@ normal form. Having numeric primary keys is a convention agreed among team members. +## Safe, or_none methods + +In sake of consistency we have two conventions when naming a method/function + +- `*_safe`: Some methods are considered as "safe" in the sense of "safe from exceptions". They +deal with the exceptions in proper way and/or logs the errors. +- `*_or_none`: returns the desired output or if object was not found in the database they +return None. + [pep8]: https://www.python.org/dev/peps/pep-0008/ diff --git a/frontend/coprs_frontend/coprs/logic/complex_logic.py b/frontend/coprs_frontend/coprs/logic/complex_logic.py index 2bdb4bd45..9fbe0b22e 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,120 +170,232 @@ 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): + """ + Get group Copr by group and copr name. + + Returns: + Copr model + + Raises: + ObjectNotFound to API if nothing is found in database + """ + group = ComplexLogic.get_group_by_name(group_name) try: return CoprsLogic.get_by_group_id( group.id, copr_name, **kwargs).one() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Project @{}/{} does not exist." - .format(group_name, copr_name)) + message="Project @{}/{} does not exist.".format( + group_name, copr_name + ) + ) from exc @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() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Project {}/{} does not exist." - .format(user_name, copr_name)) + message="Project {}/{} does not exist.".format( + user_name, copr_name + ) + ) from exc @staticmethod - def get_copr_by_owner_safe(owner_name, copr_name, **kwargs): + def get_copr_by_owner(owner_name, copr_name, **kwargs): + """ + Get Copr by owner name and copr name. + + Returns: + Copr model + + Raises: + ObjectNotFound to API if nothing is found in database + """ 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 to the API if no such Copr (group) result was found + in database. + """ copr_repo = helpers.copr_repo_fullname(repo_url) if not copr_repo: return None try: owner, copr = copr_repo.split("/") - except: + except ValueError: # 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): + """ + Get CoprDir by owner name and dir name. + + Returns: + CoprDir model + + Raises: + ObjectNotFound to the API if no result was found in database. + """ try: return CoprDirsLogic.get_by_ownername(ownername, copr_dirname).one() - except sqlalchemy.orm.exc.NoResultFound: - raise ObjectNotFound(message="copr dir {}/{} does not exist." - .format(ownername, copr_dirname)) + except sqlalchemy.orm.exc.NoResultFound as exc: + raise ObjectNotFound( + message="copr dir {}/{} does not exist.".format( + ownername, copr_dirname + ) + ) from exc @staticmethod - def get_copr_by_id_safe(copr_id): + def get_copr_by_id(copr_id): + """ + Get Copr by its ID. + + Returns: + Copr model + + Raises: + ObjectNotFound to the API if no such project with ID exists. + """ try: return CoprsLogic.get_by_id(copr_id).one() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Project with id {} does not exist." - .format(copr_id)) + message="Project with id {} does not exist.".format(copr_id) + ) from exc @staticmethod - def get_build_safe(build_id): + def get_build(build_id): + """ + Get Build by its ID. + + Returns: + Build model + + Raises: + ObjectNotFound to the API if no such build with ID exists. + """ try: return BuildsLogic.get_by_id(build_id).one() - except (sqlalchemy.orm.exc.NoResultFound, - sqlalchemy.exc.DataError): + except (sqlalchemy.orm.exc.NoResultFound, sqlalchemy.exc.DataError) as exc: raise ObjectNotFound( - message="Build {} does not exist.".format(build_id)) + message="Build {} does not exist.".format(build_id) + ) from exc @staticmethod 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. + Get a BuildChroot instance based on build ID and name of the chroot. + + Returns: + BuildChroot model + + Raises: + 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: + except KeyError as exc: msg = "Build {} was not submitted to {} chroot.".format(build_id, chrootname) if not MockChrootsLogic.get_from_name(chrootname).one_or_none(): msg = "Chroot {} does not exist".format(chrootname) - raise ObjectNotFound(message=msg) + raise ObjectNotFound(message=msg) from exc @staticmethod - def get_package_by_id_safe(package_id): + def get_package_by_id(package_id): + """ + Get Package instance based on its ID. + + Returns: + Package model + + Raises: + ObjectNotFound to the API if no such package with ID exists. + """ try: return PackagesLogic.get_by_id(package_id).one() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Package {} does not exist.".format(package_id)) + message="Package {} does not exist.".format(package_id) + ) from exc @staticmethod - def get_package_safe(copr, package_name): + def get_package(copr, package_name): + """ + Get Package instance based on Copr instance and package name. + + Returns: + Package model + + Raises: + ObjectNotFound to the API if no such package with given name + exists. + """ try: return PackagesLogic.get(copr.id, package_name).one() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Package {} in the copr {} does not exist." - .format(package_name, copr)) + message="Package {} in the copr {} does not exist.".format( + package_name, copr + ) + ) from exc @staticmethod - def get_group_by_name_safe(group_name): + def get_group_by_name(group_name): + """ + Get Group instance based on a given name. + + Returns: + Group model + + Raises: + ObjectNotFound for the API if no such group name exists. + """ try: group = UsersLogic.get_group_by_alias(group_name).one() - except sqlalchemy.orm.exc.NoResultFound: + except sqlalchemy.orm.exc.NoResultFound as exc: raise ObjectNotFound( - message="Group {} does not exist.".format(group_name)) + message="Group {} does not exist.".format(group_name) + ) from exc return group @staticmethod - def get_copr_chroot_safe(copr, chroot_name): + def get_copr_chroot(copr, chroot_name): + """ + Get CoprChroot by Copr model and chroot name. + + Returns: + CoprChroot model + + Raises: + ObjectNotFound for the API if no such chroot name exists in Copr. + """ 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)) + raise ObjectNotFound(message=str(e)) from e if not chroot: raise ObjectNotFound( @@ -552,14 +664,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..36133d352 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -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,9 +948,17 @@ 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 + Get CoprChroot in Copr model by chroot name. + + Args: + copr: Copr model + chroot_name: str + + Returns: + CoprChroot model or None if no such chroot name in given Copr + model exists. """ try: return cls.get_by_name(copr, chroot_name).one() 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..7999227ce 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") is None + assert ComplexLogic.get_copr_by_repo("copr://") is None + assert ComplexLogic.get_copr_by_repo("copr://a/b/c") is None - assert ComplexLogic.get_copr_by_repo_safe("copr://user1/foocopr") != None + assert ComplexLogic.get_copr_by_repo("copr://user1/foocopr") is not 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") is None + assert ComplexLogic.get_copr_by_repo("copr://user1//foocopr") is None @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds", "f_db")