frontend: unify naming convention for safe methods

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/

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:

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()

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):

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),