commits: re-implemented fetching a single commit for git case.

- faster using always cache if available
- fixed problems after changes of __bool__ of repo object which always rebuilt cache and this is not the case now
- make few error messages better

rhodecode/apps/repository/views/repo_commits.py

@@ -105,10 +105,9 b' class RepoCommitsView(RepoAppView):'
 
             c.commit_ranges = commits
             if not c.commit_ranges:
-                raise RepositoryError(
-                    'The commit range returned an empty result')
-        except CommitDoesNotExistError:
-            msg = _('No such commit exists for this repository')
+                raise RepositoryError('The commit range returned an empty result')
+        except CommitDoesNotExistError as e:
+            msg = _('No such commit exists. Org exception: `{}`').format(e)
             h.flash(msg, category='error')
             raise HTTPNotFound()
         except Exception:

rhodecode/lib/vcs/backends/git/inmemory.py

@@ -94,8 +94,9 b' class GitInMemoryCommit(base.BaseInMemor'
             commit_data, branch, commit_tree, updated, removed)
 
         # Update vcs repository object
-        self.repository.commit_ids.append(commit_id)
-        self.repository._rebuild_cache(self.repository.commit_ids)
+        if commit_id not in self.repository.commit_ids:
+            self.repository.commit_ids.append(commit_id)
+            self.repository._rebuild_cache(self.repository.commit_ids)
 
         # invalidate parsed refs after commit
         self.repository._refs = self.repository._get_refs()

rhodecode/lib/vcs/backends/git/repository.py

@@ -222,13 +222,10 b' class GitRepository(BaseRepository):'
             return []
         return output.splitlines()
 
-    def _get_commit_id(self, commit_id_or_idx):
+    def _lookup_commit(self, commit_id_or_idx, translate_tag=True):
         def is_null(value):
             return len(value) == commit_id_or_idx.count('0')
 
-        if self.is_empty():
-            raise EmptyRepositoryError("There are no commits yet")
-
         if commit_id_or_idx in (None, '', 'tip', 'HEAD', 'head', -1):
             return self.commit_ids[-1]
 
@@ -238,8 +235,7 b' class GitRepository(BaseRepository):'
             try:
                 commit_id_or_idx = self.commit_ids[int(commit_id_or_idx)]
             except Exception:
-                msg = "Commit %s does not exist for %s" % (
-                    commit_id_or_idx, self)
+                msg = "Commit %s does not exist for %s" % (commit_id_or_idx, self.name)
                 raise CommitDoesNotExistError(msg)
 
         elif is_bstr:
@@ -261,8 +257,7 b' class GitRepository(BaseRepository):'
 
             if (not SHA_PATTERN.match(commit_id_or_idx) or
                     commit_id_or_idx not in self.commit_ids):
-                msg = "Commit %s does not exist for %s" % (
-                    commit_id_or_idx, self)
+                msg = "Commit %s does not exist for %s" % (commit_id_or_idx, self.name)
                 raise CommitDoesNotExistError(msg)
 
         # Ensure we return full id
@@ -431,19 +426,42 b' class GitRepository(BaseRepository):'
         Returns `GitCommit` object representing commit from git repository
         at the given `commit_id` or head (most recent commit) if None given.
         """
+        if self.is_empty():
+            raise EmptyRepositoryError("There are no commits yet")
+
         if commit_id is not None:
             self._validate_commit_id(commit_id)
+            try:
+                # we have cached idx, use it without contacting the remote
+                idx = self._commit_ids[commit_id]
+                return GitCommit(self, commit_id, idx, pre_load=pre_load)
+            except KeyError:
+                pass
+
         elif commit_idx is not None:
             self._validate_commit_idx(commit_idx)
-            commit_id = commit_idx
-        commit_id = self._get_commit_id(commit_id)
+            try:
+                _commit_id = self.commit_ids[commit_idx]
+                if commit_idx < 0:
+                    commit_idx = self.commit_ids.index(_commit_id)
+                return GitCommit(self, _commit_id, commit_idx, pre_load=pre_load)
+            except IndexError:
+                commit_id = commit_idx
+        else:
+            commit_id = "tip"
+
+        commit_id = self._lookup_commit(commit_id)
+        remote_idx = None
+        if translate_tag:
+            # Need to call remote to translate id for tagging scenario
+            remote_data = self._remote.get_object(commit_id)
+            commit_id = remote_data["commit_id"]
+            remote_idx = remote_data["idx"]
+
         try:
-            if translate_tag:
-                # Need to call remote to translate id for tagging scenario
-                commit_id = self._remote.get_object(commit_id)["commit_id"]
             idx = self._commit_ids[commit_id]
         except KeyError:
-            raise RepositoryError("Cannot get object with id %s" % commit_id)
+            idx = remote_idx or 0
 
         return GitCommit(self, commit_id, idx, pre_load=pre_load)
 
@@ -472,6 +490,7 b' class GitRepository(BaseRepository):'
         """
         if self.is_empty():
             raise EmptyRepositoryError("There are no commits yet")
+
         self._validate_branch_name(branch_name)
 
         if start_id is not None:
@@ -479,9 +498,9 b' class GitRepository(BaseRepository):'
         if end_id is not None:
             self._validate_commit_id(end_id)
 
-        start_raw_id = self._get_commit_id(start_id)
+        start_raw_id = self._lookup_commit(start_id)
         start_pos = self._commit_ids[start_raw_id] if start_id else None
-        end_raw_id = self._get_commit_id(end_id)
+        end_raw_id = self._lookup_commit(end_id)
         end_pos = max(0, self._commit_ids[end_raw_id]) if end_id else None
 
         if None not in [start_id, end_id] and start_pos > end_pos:

rhodecode/lib/vcs/backends/hg/inmemory.py

@@ -83,14 +83,15 b' class MercurialInMemoryCommit(BaseInMemo'
 
         date, tz = date_to_timestamp_plus_offset(date)
 
-        new_id = self.repository._remote.commitctx(
+        commit_id = self.repository._remote.commitctx(
             message=message, parents=parent_ids,
             commit_time=date, commit_timezone=tz, user=author,
             files=self.get_paths(), extra=kwargs, removed=removed,
             updated=updated)
+        if commit_id not in self.repository.commit_ids:
+            self.repository.commit_ids.append(commit_id)
+            self.repository._rebuild_cache(self.repository.commit_ids)
 
-        self.repository.commit_ids.append(new_id)
-        self.repository._rebuild_cache(self.repository.commit_ids)
         self.repository.branches = self.repository._get_branches()
         tip = self.repository.get_commit()
         self.reset()

rhodecode/lib/vcs/backends/hg/repository.py

@@ -425,18 +425,20 b' class MercurialRepository(BaseRepository'
         if commit_id is not None:
             self._validate_commit_id(commit_id)
             try:
+                # we have cached idx, use it without contacting the remote
                 idx = self._commit_ids[commit_id]
                 return MercurialCommit(self, commit_id, idx, pre_load=pre_load)
             except KeyError:
                 pass
+
         elif commit_idx is not None:
             self._validate_commit_idx(commit_idx)
             try:
-                id_ = self.commit_ids[commit_idx]
+                _commit_id = self.commit_ids[commit_idx]
                 if commit_idx < 0:
-                    commit_idx += len(self.commit_ids)
-                return MercurialCommit(
-                    self, id_, commit_idx, pre_load=pre_load)
+                    commit_idx = self.commit_ids.index(_commit_id)
+
+                return MercurialCommit(self, _commit_id, commit_idx, pre_load=pre_load)
             except IndexError:
                 commit_id = commit_idx
         else:
@@ -448,8 +450,7 b' class MercurialRepository(BaseRepository'
         try:
             raw_id, idx = self._remote.lookup(commit_id, both=True)
         except CommitDoesNotExistError:
-            msg = "Commit %s does not exist for %s" % (
-                commit_id, self)
+            msg = "Commit %s does not exist for %s" % (commit_id, self.name)
             raise CommitDoesNotExistError(msg)
 
         return MercurialCommit(self, raw_id, idx, pre_load=pre_load)

rhodecode/model/db.py

@@ -2437,6 +2437,7 b' class Repository(Base, BaseModel):'
         # control over cache behaviour
         if cache is None and full_cache and not config:
             return self._get_instance_cached()
+        # cache here is sent to the "vcs server"
         return self._get_instance(cache=bool(cache), config=config)
 
     def _get_instance_cached(self):
@@ -2479,7 +2480,8 b' class Repository(Base, BaseModel):'
             with_wire=custom_wire,
             create=False,
             _vcs_alias=self.repo_type)
-
+        if repo is not None:
+            repo.count()  # cache rebuild
         return repo
 
     def __json__(self):
@@ -4433,9 +4435,9 b' class Gist(Base, BaseModel):'
 
     def scm_instance(self, **kwargs):
         """
-        Get explicit Mercurial repository used
+        Get an instance of VCS Repository
+
         :param kwargs:
-        :return:
         """
         from rhodecode.model.gist import GistModel
         full_repo_path = os.path.join(self.base_path(), self.gist_access_id)

rhodecode/model/pull_request.py

@@ -683,7 +683,6 b' class PullRequestModel(BaseModel):'
 
         # source repo
         source_repo = pull_request.source_repo.scm_instance()
-        source_repo.count()  # cache rebuild
 
         try:
             source_commit = source_repo.get_commit(commit_id=source_ref_name)
@@ -698,7 +697,6 b' class PullRequestModel(BaseModel):'
 
         # target repo
         target_repo = pull_request.target_repo.scm_instance()
-        target_repo.count()  # cache rebuild
 
         try:
             target_commit = target_repo.get_commit(commit_id=target_ref_name)
@@ -1342,7 +1340,6 b' class PullRequestModel(BaseModel):'
         else:
             name_or_id = reference.commit_id
 
-        vcs_repository.count() # cache rebuild
         refreshed_commit = vcs_repository.get_commit(name_or_id)
         refreshed_reference = Reference(
             reference.type, reference.name, refreshed_commit.raw_id)

rhodecode/tests/models/test_scm.py

@@ -129,20 +129,19 b' def test_strip_with_single_heads(backend'
     assert commit_ids['b'] not in rest_commit_ids
 
 
-def test_get_nodes_returns_unicode_flat(backend_random):
-    repo = backend_random.repo
-    directories, files = scm.ScmModel().get_nodes(
-        repo.repo_name, repo.get_commit(commit_idx=0).raw_id,
-        flat=True)
+def test_get_nodes_returns_unicode_flat(backend):
+    repo = backend.repo
+    commit_id = repo.get_commit(commit_idx=0).raw_id
+    directories, files = scm.ScmModel().get_nodes(repo.repo_name, commit_id, flat=True)
     assert_contains_only_unicode(directories)
     assert_contains_only_unicode(files)
 
 
-def test_get_nodes_returns_unicode_non_flat(backend_random):
-    repo = backend_random.repo
-    directories, files = scm.ScmModel().get_nodes(
-        repo.repo_name, repo.get_commit(commit_idx=0).raw_id,
-        flat=False)
+def test_get_nodes_returns_unicode_non_flat(backend):
+    repo = backend.repo
+    commit_id = repo.get_commit(commit_idx=0).raw_id
+
+    directories, files = scm.ScmModel().get_nodes(repo.repo_name, commit_id, flat=False)
     # johbo: Checking only the names for now, since that is the critical
     # part.
     assert_contains_only_unicode([d['name'] for d in directories])

rhodecode/tests/vcs/conftest.py

@@ -148,7 +148,6 b' def _add_commits_to_repo(repo, commits):'
             author=unicode(commit['author']),
             date=commit['date'],
             branch=commit.get('branch'))
-
     return tip
 
 

rhodecode/tests/vcs/test_commits.py

@@ -159,8 +159,7 b' class TestCommitsInNonEmptyRepo(BackendT'
             parents=[initial],
             branch=DEFAULT_BRANCH,)
 
-        default_branch_commits = self.repo.get_commits(
-            branch_name=DEFAULT_BRANCH)
+        default_branch_commits = self.repo.get_commits(branch_name=DEFAULT_BRANCH)
         assert docs_branch_commit1 not in list(default_branch_commits)
         assert docs_branch_commit2 not in list(default_branch_commits)
 

rhodecode/tests/vcs/test_git.py

@@ -1091,23 +1091,23 b' class TestGitSpecificWithRepo(BackendTes'
         self.repo.get_diff(self.repo[0], self.repo[1])
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', self.repo._get_commit_id(0),
-             self.repo._get_commit_id(1)])
+             '--abbrev=40', self.repo._lookup_commit(0),
+             self.repo._lookup_commit(1)])
 
     def test_get_diff_runs_git_command_with_str_hashes(self):
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(self.repo.EMPTY_COMMIT, self.repo[1])
         self.repo.run_git_command.assert_called_once_with(
             ['show', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', self.repo._get_commit_id(1)])
+             '--abbrev=40', self.repo._lookup_commit(1)])
 
     def test_get_diff_runs_git_command_with_path_if_its_given(self):
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(self.repo[0], self.repo[1], 'foo')
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U3', '--full-index', '--binary', '-p', '-M',
-             '--abbrev=40', self.repo._get_commit_id(0),
-             self.repo._get_commit_id(1), '--', 'foo'])
+             '--abbrev=40', self.repo._lookup_commit(0),
+             self.repo._lookup_commit(1), '--', 'foo'])
 
 
 @pytest.mark.usefixtures("vcs_repository_support")