kallithea Changeset - 91b38dc6d891

Changeset - 91b38dc6d891

Parent rev.

Child rev.

[Not reviewed]

default

0 10 0

Søren Løvborg - 9 years ago 2016-08-03 16:07:39
sorenl@unity3d.com

model: refactor and simplify _get_instance

_get_instance is a BaseModel method, but never uses "self". Instead
it takes a class argument, which indicates that it's better suited as
a classmethod on said classes.

Also rename to something more descriptive, remove leading underscore
since it's not a private API, and refactor for readability.

10 files changed with 47 insertions and 44 deletions:

kallithea/model/__init__.py

kallithea/model/changeset_status.py

kallithea/model/comment.py

kallithea/model/db.py

kallithea/model/gist.py

kallithea/model/notification.py

kallithea/model/pull_request.py

kallithea/model/repo.py

kallithea/model/repo_group.py

kallithea/model/user_group.py

0 comments (0 inline, 0 general)

kallithea/model/__init__.py

➞

Show inline comments

@@ @@ -43,13 +43,13 @@ Original author and date, and relevant c @@
 """
 import logging
 from kallithea.model import meta
-from kallithea.lib.utils2 import safe_str, obfuscate_url_pw
 from kallithea.lib.utils2 import obfuscate_url_pw
 log = logging.getLogger(__name__)
 def init_model(engine):
     """
@@ @@ -75,58 +75,35 @@ class BaseModel(object): @@
     def __init__(self, sa=None):
         if sa is not None:
             self.sa = sa
         else:
             self.sa = meta.Session()
     def _get_instance(self, cls, instance, callback=None):
         """
         Gets instance of given cls using some simple lookup mechanism.
         :param cls: class to fetch
         :param instance: int or Instance
         :param callback: callback to call if all lookups failed
         """
         if isinstance(instance, cls):
             return instance
         elif isinstance(instance, (int, long)) or safe_str(instance).isdigit():
             return cls.get(instance)
         else:
             if instance is not None:
                 if callback is None:
                     raise Exception(
                         'given object must be int, long or Instance of %s '
                         'got %s, no callback provided' % (cls, type(instance))
+                    )
                 else:
                     return callback(instance)
     def _get_user(self, user):
         """
         Helper method to get user by ID, or username fallback
         :param user: UserID, username, or User instance
         """
         from kallithea.model.db import User
-        return self._get_instance(User, user,
+        return User.guess_instance(user,
                                   callback=User.get_by_username)
     def _get_repo(self, repository):
         """
         Helper method to get repository by ID, or repository name
         :param repository: RepoID, repository name or Repository Instance
         """
         from kallithea.model.db import Repository
-        return self._get_instance(Repository, repository,
+        return Repository.guess_instance(repository,
                                   callback=Repository.get_by_repo_name)
     def _get_perm(self, permission):
         """
         Helper method to get permission by ID, or permission name
         :param permission: PermissionID, permission_name or Permission instance
         """
         from kallithea.model.db import Permission
-        return self._get_instance(Permission, permission,
+        return Permission.guess_instance(permission,
                                   callback=Permission.get_by_key)

kallithea/model/changeset_status.py

➞

Show inline comments

@@ @@ -47,13 +47,13 @@ class ChangesetStatusModel(BaseModel): @@
             # only report the latest vote across all users! TODO: be smarter!
             q = q.filter(ChangesetStatus.version == 0)
         if revision:
             q = q.filter(ChangesetStatus.revision == revision)
         elif pull_request:
-            pull_request = self._get_instance(PullRequest, pull_request)
+            pull_request = PullRequest.guess_instance(pull_request)
             q = q.filter(ChangesetStatus.pull_request == pull_request)
         else:
             raise Exception('Please specify revision or pull_request')
         q = q.order_by(ChangesetStatus.version.asc())
         return q
@@ @@ -156,13 +156,13 @@ class ChangesetStatusModel(BaseModel): @@
             assert pull_request is None
             q = q.filter(ChangesetStatus.repo == repo)
             q = q.filter(ChangesetStatus.revision == revision)
             revisions = [revision]
         else:
             assert pull_request is not None
-            pull_request = self._get_instance(PullRequest, pull_request)
+            pull_request = PullRequest.guess_instance(pull_request)
             repo = pull_request.org_repo
             q = q.filter(ChangesetStatus.repo == repo)
             q = q.filter(ChangesetStatus.revision.in_(pull_request.revisions))
             revisions = pull_request.revisions
         cur_statuses = q.all()

kallithea/model/comment.py

➞

Show inline comments

@@ @@ -175,13 +175,13 @@ class ChangesetCommentsModel(BaseModel): @@
         comment.f_path = f_path
         comment.line_no = line_no
         if revision is not None:
             comment.revision = revision
         elif pull_request is not None:
-            pull_request = self._get_instance(PullRequest, pull_request)
+            pull_request = PullRequest.guess_instance(pull_request)
             comment.pull_request = pull_request
         else:
             raise Exception('Please specify revision or pull_request_id')
         Session().add(comment)
         Session().flush()
@@ @@ -216,13 +216,13 @@ class ChangesetCommentsModel(BaseModel): @@
                     email_kwargs=email_kwargs
+                )
         return comment
     def delete(self, comment):
-        comment = self._get_instance(ChangesetComment, comment)
+        comment = ChangesetComment.guess_instance(comment)
         Session().delete(comment)
         return comment
     def get_comments(self, repo_id, revision=None, pull_request=None):
         """
@@ @@ -261,12 +261,12 @@ class ChangesetCommentsModel(BaseModel): @@
                 .filter(ChangesetComment.f_path == None)
         if revision is not None:
             q = q.filter(ChangesetComment.revision == revision) \
                 .filter(ChangesetComment.repo_id == repo_id)
         elif pull_request is not None:
-            pull_request = self._get_instance(PullRequest, pull_request)
+            pull_request = PullRequest.guess_instance(pull_request)
             q = q.filter(ChangesetComment.pull_request == pull_request)
         else:
             raise Exception('Please specify either revision or pull_request')
         return q.order_by(ChangesetComment.created_on).all()

kallithea/model/db.py

➞

Show inline comments

@@ @@ -119,12 +119,38 @@ class BaseModel(object): @@
     @classmethod
     def get(cls, id_):
         if id_:
             return cls.query().get(id_)
     @classmethod
     def guess_instance(cls, value, callback=None):
         """Haphazardly attempt to convert `value` to a `cls` instance.
         If `value` is None or already a `cls` instance, return it. If `value`
         is a number (or looks like one if you squint just right), assume it's
         a database primary key and let SQLAlchemy sort things out. Otherwise,
         fall back to resolving it using `callback` (if specified); this could
         e.g. be a function that looks up instances by name (though that won't
         work if the name begins with a digit). Otherwise, raise Exception.
         """
         if value is None:
             return None
         if isinstance(value, cls):
             return value
         if isinstance(value, (int, long)) or safe_str(value).isdigit():
             return cls.get(value)
         if callback is not None:
             return callback(value)
         raise Exception(
             'given object must be int, long or Instance of %s '
             'got %s, no callback provided' % (cls, type(value))
+        )
     @classmethod
     def get_or_404(cls, id_):
         try:
             id_ = int(id_)
         except (TypeError, ValueError):
             raise HTTPNotFound

kallithea/model/gist.py

➞

Show inline comments

@@ @@ -50,13 +50,13 @@ class GistModel(BaseModel): @@
     def _get_gist(self, gist):
         """
         Helper method to get gist by ID, or gist_access_id as a fallback
         :param gist: GistID, gist_access_id, or Gist instance
         """
-        return self._get_instance(Gist, gist, callback=Gist.get_by_access_id)
+        return Gist.guess_instance(gist, callback=Gist.get_by_access_id)
     def __delete_gist(self, gist):
         """
         removes gist from filesystem
         :param gist: gist object

kallithea/model/notification.py

➞

Show inline comments

@@ @@ -137,13 +137,13 @@ class NotificationModel(BaseModel): @@
         return notif
     def delete(self, user, notification):
         # we don't want to remove actual notification just the assignment
         try:
-            notification = self._get_instance(Notification, notification)
+            notification = Notification.guess_instance(notification)
             user = self._get_user(user)
             if notification and user:
                 obj = UserNotification.query() \
                         .filter(UserNotification.user == user) \
                         .filter(UserNotification.notification
                                 == notification) \
@@ @@ -175,13 +175,13 @@ class NotificationModel(BaseModel): @@
             q = q.filter(Notification.type_.in_(filter_))
         return q
     def mark_read(self, user, notification):
         try:
-            notification = self._get_instance(Notification, notification)
+            notification = Notification.guess_instance(notification)
             user = self._get_user(user)
             if notification and user:
                 obj = UserNotification.query() \
                         .filter(UserNotification.user == user) \
                         .filter(UserNotification.notification
                                 == notification) \
@@ @@ -220,13 +220,13 @@ class NotificationModel(BaseModel): @@
         return [x.notification for x in UserNotification.query() \
                 .filter(UserNotification.read == False) \
                 .filter(UserNotification.user == user).all()]
     def get_user_notification(self, user, notification):
         user = self._get_user(user)
-        notification = self._get_instance(Notification, notification)
+        notification = Notification.guess_instance(notification)
         return UserNotification.query() \
             .filter(UserNotification.notification == notification) \
             .filter(UserNotification.user == user).scalar()
     def make_description(self, notification, show_age=True):

kallithea/model/pull_request.py

➞

Show inline comments

@@ @@ -194,13 +194,13 @@ class PullRequestModel(BaseModel): @@
         log.debug("Mentioning %s", mention_recipients)
         self.__add_reviewers(user, pr, set(), mention_recipients)
     def update_reviewers(self, user, pull_request, reviewers_ids):
         reviewers_ids = set(reviewers_ids)
-        pull_request = self._get_instance(PullRequest, pull_request)
+        pull_request = PullRequest.guess_instance(pull_request)
         current_reviewers = PullRequestReviewers.query() \
             .options(joinedload('user')) \
             .filter_by(pull_request=pull_request) \
             .all()
         current_reviewer_users = set(x.user for x in current_reviewers)
         new_reviewer_users = set(self._get_valid_reviewers(reviewers_ids))
@@ @@ -217,14 +217,14 @@ class PullRequestModel(BaseModel): @@
         log.debug("Removing %s reviewers", to_remove)
         for prr in current_reviewers:
             if prr.user in to_remove:
                 Session().delete(prr)
     def delete(self, pull_request):
-        pull_request = self._get_instance(PullRequest, pull_request)
+        pull_request = PullRequest.guess_instance(pull_request)
         Session().delete(pull_request)
     def close_pull_request(self, pull_request):
-        pull_request = self._get_instance(PullRequest, pull_request)
+        pull_request = PullRequest.guess_instance(pull_request)
         pull_request.status = PullRequest.STATUS_CLOSED
         pull_request.updated_on = datetime.datetime.now()
         Session().add(pull_request)

kallithea/model/repo.py

➞

Show inline comments

@@ @@ -56,17 +56,17 @@ log = logging.getLogger(__name__) @@
 class RepoModel(BaseModel):
     URL_SEPARATOR = Repository.url_sep()
     def _get_user_group(self, users_group):
-        return self._get_instance(UserGroup, users_group,
+        return UserGroup.guess_instance(users_group,
                                   callback=UserGroup.get_by_group_name)
     def _get_repo_group(self, repo_group):
-        return self._get_instance(RepoGroup, repo_group,
+        return RepoGroup.guess_instance(repo_group,
                                   callback=RepoGroup.get_by_group_name)
     def _create_default_perms(self, repository, private):
         # create default permission
         default = 'repository.read'
         def_user = User.get_default_user()

kallithea/model/repo_group.py

➞

Show inline comments

@@ @@ -41,17 +41,17 @@ from kallithea.model.db import RepoGroup @@
 log = logging.getLogger(__name__)
 class RepoGroupModel(BaseModel):
     def _get_user_group(self, users_group):
-        return self._get_instance(UserGroup, users_group,
+        return UserGroup.guess_instance(users_group,
                                   callback=UserGroup.get_by_group_name)
     def _get_repo_group(self, repo_group):
-        return self._get_instance(RepoGroup, repo_group,
+        return RepoGroup.guess_instance(repo_group,
                                   callback=RepoGroup.get_by_group_name)
     @LazyProperty
     def repos_path(self):
         """
         Gets the repositories root path from database

kallithea/model/user_group.py

➞

Show inline comments

@@ @@ -37,13 +37,13 @@ from kallithea.lib.exceptions import Use @@
 log = logging.getLogger(__name__)
 class UserGroupModel(BaseModel):
     def _get_user_group(self, user_group):
-        return self._get_instance(UserGroup, user_group,
+        return UserGroup.guess_instance(user_group,
                                   callback=UserGroup.get_by_group_name)
     def _create_default_perms(self, user_group):
         # create default permission
         default_perm = 'usergroup.read'
         def_user = User.get_default_user()

0 comments (0 inline, 0 general)