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
This commit is contained in:
Marcin Kuzminski 2019-06-10 12:28:47 +02:00
parent cc0ae374fd
commit 6a2e2f0b94
11 changed files with 72 additions and 55 deletions

View file

@ -105,10 +105,9 @@ 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:

View file

@ -94,8 +94,9 @@ class GitInMemoryCommit(base.BaseInMemoryCommit):
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()

View file

@ -222,13 +222,10 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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:

View file

@ -83,14 +83,15 @@ class MercurialInMemoryCommit(BaseInMemoryCommit):
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()

View file

@ -425,18 +425,20 @@ 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 @@ 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)

View file

@ -2437,6 +2437,7 @@ 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 @@ 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 @@ 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)

View file

@ -683,7 +683,6 @@ 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 @@ 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 @@ 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)

View file

@ -129,20 +129,19 @@ def test_strip_with_single_heads(backend_hg):
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])

View file

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

View file

@ -159,8 +159,7 @@ class TestCommitsInNonEmptyRepo(BackendTestMixin):
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)

View file

@ -1091,23 +1091,23 @@ class TestGitSpecificWithRepo(BackendTestMixin):
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")