file-browser: refactor how we load metadata for file trees.
Before we used to use JSON data to map the nodes to json and fill in metadata. Now we use rendered parts of html. This is nicer for caching as it would allow us to replace the view with cached tree and then after ajax load replace it again with cached with metadata. On the next request we'll get the cached with metadata and thus we can skip entirely second ajax call for metadata. This is part of #4083
This commit is contained in:
parent
e22bef75b4
commit
f48e94aa14
9 changed files with 98 additions and 82 deletions
|
|
@ -1103,9 +1103,9 @@ def make_map(config):
|
|||
conditions={'function': check_repo},
|
||||
requirements=URL_NAME_REQUIREMENTS, jsroute=True)
|
||||
|
||||
rmap.connect('files_metadata_list_home',
|
||||
'/{repo_name}/metadata_list/{revision}/{f_path}',
|
||||
controller='files', action='metadata_list',
|
||||
rmap.connect('files_nodetree_full',
|
||||
'/{repo_name}/nodetree_full/{commit_id}/{f_path}',
|
||||
controller='files', action='nodetree_full',
|
||||
conditions={'function': check_repo},
|
||||
requirements=URL_NAME_REQUIREMENTS, jsroute=True)
|
||||
|
||||
|
|
|
|||
|
|
@ -136,10 +136,12 @@ class FilesController(BaseRepoController):
|
|||
_namespace = caches.get_repo_namespace_key(namespace_type, repo_name)
|
||||
return caches.get_cache_manager('repo_cache_long', _namespace)
|
||||
|
||||
def _get_tree_at_commit(self, repo_name, commit_id, f_path):
|
||||
def _get_tree_at_commit(self, repo_name, commit_id, f_path,
|
||||
full_load=False, force=False):
|
||||
def _cached_tree():
|
||||
log.debug('Generating cached file tree for %s, %s, %s',
|
||||
repo_name, commit_id, f_path)
|
||||
c.full_load = full_load
|
||||
return render('files/files_browser_tree.html')
|
||||
|
||||
cache_manager = self.__get_tree_cache_manager(
|
||||
|
|
@ -148,6 +150,10 @@ class FilesController(BaseRepoController):
|
|||
cache_key = caches.compute_key_from_params(
|
||||
repo_name, commit_id, f_path)
|
||||
|
||||
if force:
|
||||
# we want to force recompute of caches
|
||||
cache_manager.remove_value(cache_key)
|
||||
|
||||
return cache_manager.get(cache_key, createfunc=_cached_tree)
|
||||
|
||||
def _get_nodelist_at_commit(self, repo_name, commit_id, f_path):
|
||||
|
|
@ -165,22 +171,6 @@ class FilesController(BaseRepoController):
|
|||
repo_name, commit_id, f_path)
|
||||
return cache_manager.get(cache_key, createfunc=_cached_nodes)
|
||||
|
||||
def _get_metadata_at_commit(self, repo_name, commit, dir_node):
|
||||
def _cached_metadata():
|
||||
log.debug('Generating cached metadata for %s, %s, %s',
|
||||
repo_name, commit.raw_id, safe_str(dir_node.path))
|
||||
|
||||
data = ScmModel().get_dirnode_metadata(commit, dir_node)
|
||||
return data
|
||||
|
||||
cache_manager = self.__get_tree_cache_manager(
|
||||
repo_name, caches.FILE_TREE_META)
|
||||
|
||||
cache_key = caches.compute_key_from_params(
|
||||
repo_name, commit.raw_id, safe_str(dir_node.path))
|
||||
|
||||
return cache_manager.get(cache_key, createfunc=_cached_metadata)
|
||||
|
||||
@LoginRequired()
|
||||
@HasRepoPermissionAnyDecorator(
|
||||
'repository.read', 'repository.write', 'repository.admin')
|
||||
|
|
@ -246,6 +236,7 @@ class FilesController(BaseRepoController):
|
|||
c.authors = []
|
||||
c.file_tree = self._get_tree_at_commit(
|
||||
repo_name, c.commit.raw_id, f_path)
|
||||
|
||||
except RepositoryError as e:
|
||||
h.flash(safe_str(e), category='error')
|
||||
raise HTTPNotFound()
|
||||
|
|
@ -1092,23 +1083,32 @@ class FilesController(BaseRepoController):
|
|||
@XHRRequired()
|
||||
@HasRepoPermissionAnyDecorator(
|
||||
'repository.read', 'repository.write', 'repository.admin')
|
||||
@jsonify
|
||||
def metadata_list(self, repo_name, revision, f_path):
|
||||
def nodetree_full(self, repo_name, commit_id, f_path):
|
||||
"""
|
||||
Returns a json dict that contains commit date, author, revision
|
||||
and id for the specified repo, revision and file path
|
||||
Returns rendered html of file tree that contains commit date,
|
||||
author, revision for the specified combination of
|
||||
repo, commit_id and file path
|
||||
|
||||
:param repo_name: name of the repository
|
||||
:param revision: revision of files
|
||||
:param commit_id: commit_id of file tree
|
||||
:param f_path: file path of the requested directory
|
||||
"""
|
||||
|
||||
commit = self.__get_commit_or_redirect(revision, repo_name)
|
||||
commit = self.__get_commit_or_redirect(commit_id, repo_name)
|
||||
try:
|
||||
file_node = commit.get_node(f_path)
|
||||
dir_node = commit.get_node(f_path)
|
||||
except RepositoryError as e:
|
||||
return {'error': safe_str(e)}
|
||||
return 'error {}'.format(safe_str(e))
|
||||
|
||||
metadata = self._get_metadata_at_commit(
|
||||
repo_name, commit, file_node)
|
||||
return {'metadata': metadata}
|
||||
if dir_node.is_file():
|
||||
return ''
|
||||
|
||||
c.file = dir_node
|
||||
c.commit = commit
|
||||
|
||||
# using force=True here, make a little trick. We flush the cache and
|
||||
# compute it using the same key as without full_load, so the fully
|
||||
# loaded cached tree is now returned instead of partial
|
||||
return self._get_tree_at_commit(
|
||||
repo_name, commit.raw_id, dir_node.path, full_load=True,
|
||||
force=True)
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ FILE_SEARCH_TREE_META = 'cache_file_search_metadata'
|
|||
SUMMARY_STATS = 'cache_summary_stats'
|
||||
|
||||
# This list of caches gets purged when invalidation happens
|
||||
USED_REPO_CACHES = (FILE_TREE, FILE_TREE_META, FILE_TREE_META)
|
||||
USED_REPO_CACHES = (FILE_TREE, FILE_SEARCH_TREE_META)
|
||||
|
||||
DEFAULT_CACHE_MANAGER_CONFIG = {
|
||||
'type': 'memorylru_base',
|
||||
|
|
|
|||
|
|
@ -4,7 +4,8 @@
|
|||
* DO NOT CHANGE THIS FILE MANUALLY *
|
||||
* *
|
||||
* *
|
||||
* This file is automatically generated when the app starts up. *
|
||||
* This file is automatically generated when the app starts up with *
|
||||
* generate_js_files = true *
|
||||
* *
|
||||
* To add a route here pass jsroute=True to the route definition in the app *
|
||||
* *
|
||||
|
|
@ -44,7 +45,7 @@ function registerRCRoutes() {
|
|||
pyroutes.register('files_authors_home', '/%(repo_name)s/authors/%(revision)s/%(f_path)s', ['repo_name', 'revision', 'f_path']);
|
||||
pyroutes.register('files_archive_home', '/%(repo_name)s/archive/%(fname)s', ['repo_name', 'fname']);
|
||||
pyroutes.register('files_nodelist_home', '/%(repo_name)s/nodelist/%(revision)s/%(f_path)s', ['repo_name', 'revision', 'f_path']);
|
||||
pyroutes.register('files_metadata_list_home', '/%(repo_name)s/metadata_list/%(revision)s/%(f_path)s', ['repo_name', 'revision', 'f_path']);
|
||||
pyroutes.register('files_nodetree_full', '/%(repo_name)s/nodetree_full/%(commit_id)s/%(f_path)s', ['repo_name', 'commit_id', 'f_path']);
|
||||
pyroutes.register('summary_home_slash', '/%(repo_name)s/', ['repo_name']);
|
||||
pyroutes.register('summary_home', '/%(repo_name)s', ['repo_name']);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -91,36 +91,26 @@
|
|||
if (source_page) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($('#file-tree-wrapper').hasClass('full-load')) {
|
||||
// in case our HTML wrapper has full-load class we don't
|
||||
// trigger the async load of metadata
|
||||
return false;
|
||||
}
|
||||
|
||||
var state = getState('metadata');
|
||||
var url_data = {
|
||||
'repo_name': templateContext.repo_name,
|
||||
'revision': state.commit_id,
|
||||
'commit_id': state.commit_id,
|
||||
'f_path': state.f_path
|
||||
};
|
||||
|
||||
var url = pyroutes.url('files_metadata_list_home', url_data);
|
||||
var url = pyroutes.url('files_nodetree_full', url_data);
|
||||
|
||||
metadataRequest = $.ajax({url: url});
|
||||
|
||||
metadataRequest.done(function(data) {
|
||||
var data = data.metadata;
|
||||
var dataLength = data.length;
|
||||
for (var i = 0; i < dataLength; i++) {
|
||||
var rowData = data[i];
|
||||
var name = rowData.name.replace('\\', '\\\\');
|
||||
|
||||
$('td[title="size-' + name + '"]').html(rowData.size);
|
||||
var timeComponent = AgeModule.createTimeComponent(
|
||||
rowData.modified_ts, rowData.modified_at);
|
||||
$('td[title="modified_at-' + name + '"]').html(timeComponent);
|
||||
|
||||
$('td[title="revision-' + name + '"]').html(
|
||||
'<div class="tooltip" title="{0}"><pre>r{1}:{2}</pre></div>'.format(
|
||||
data[i].message, data[i].revision, data[i].short_id));
|
||||
$('td[title="author-' + name + '"]').html(
|
||||
'<span title="{0}">{1}</span>'.format(
|
||||
data[i].author, data[i].user_profile));
|
||||
}
|
||||
$('#file-tree').html(data);
|
||||
timeagoActivate();
|
||||
});
|
||||
metadataRequest.fail(function (data, textStatus, errorThrown) {
|
||||
|
|
@ -138,7 +128,7 @@
|
|||
// used for history, and switch to
|
||||
var initialCommitData = {
|
||||
id: null,
|
||||
text: "${_("Switch To Commit")}",
|
||||
text: '${_("Switch To Commit")}',
|
||||
type: 'sha',
|
||||
raw_id: null,
|
||||
files_url: null
|
||||
|
|
@ -329,5 +319,4 @@
|
|||
|
||||
</script>
|
||||
|
||||
|
||||
</%def>
|
||||
|
|
|
|||
|
|
@ -42,7 +42,9 @@
|
|||
</div>
|
||||
</div>
|
||||
## file tree is computed from caches, and filled in
|
||||
<div id="file-tree">
|
||||
${c.file_tree}
|
||||
</div>
|
||||
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
<div class="browser-body">
|
||||
<div id="file-tree-wrapper" class="browser-body ${'full-load' if c.full_load else ''}">
|
||||
<table class="code-browser rctable">
|
||||
<thead>
|
||||
<tr>
|
||||
|
|
@ -31,7 +31,7 @@
|
|||
<span class="submodule-dir">
|
||||
${h.link_to_if(
|
||||
node.url.startswith('http://') or node.url.startswith('https://'),
|
||||
node.name,node.url)}
|
||||
node.name, node.url)}
|
||||
</span>
|
||||
%else:
|
||||
<a href="${h.url('files_home',repo_name=c.repo_name,revision=c.commit.raw_id,f_path=h.safe_unicode(node.path))}" class="pjax-link">
|
||||
|
|
@ -40,12 +40,30 @@
|
|||
%endif
|
||||
</td>
|
||||
%if node.is_file():
|
||||
<td class="td-size" title="${'size-%s' % node.name}"></td>
|
||||
<td class="td-time" title="${'modified_at-%s' % node.name}">
|
||||
<span class="browser-loading">${_('Loading...')}</span>
|
||||
<td class="td-size" data-attr-name="size">
|
||||
% if c.full_load:
|
||||
<span data-size="${node.size}">${h.format_byte_size_binary(node.size)}</span>
|
||||
% else:
|
||||
${_('Loading ...')}
|
||||
% endif
|
||||
</td>
|
||||
<td class="td-time" data-attr-name="modified_at">
|
||||
% if c.full_load:
|
||||
<span data-date="${node.last_commit.date}">${h.age_component(node.last_commit.date)}</span>
|
||||
% endif
|
||||
</td>
|
||||
<td class="td-hash" data-attr-name="commit_id">
|
||||
% if c.full_load:
|
||||
<div class="tooltip" title="${node.last_commit.message}">
|
||||
<pre data-commit-id="${node.last_commit.raw_id}">r${node.last_commit.revision}:${node.last_commit.short_id}</pre>
|
||||
</div>
|
||||
% endif
|
||||
</td>
|
||||
<td class="td-user" data-attr-name="author">
|
||||
% if c.full_load:
|
||||
<span data-author="${node.last_commit.author}" title="${node.last_commit.author}">${h.gravatar_with_user(node.last_commit.author)|n}</span>
|
||||
% endif
|
||||
</td>
|
||||
<td class="td-hash" title="${'revision-%s' % node.name}"></td>
|
||||
<td class="td-user" title="${'author-%s' % node.name}"></td>
|
||||
%else:
|
||||
<td></td>
|
||||
<td></td>
|
||||
|
|
@ -57,4 +75,4 @@
|
|||
</tbody>
|
||||
<tbody id="tbody_filtered"></tbody>
|
||||
</table>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ class TestFilesController:
|
|||
|
||||
params = {
|
||||
'repo_name': backend.repo_name,
|
||||
'revision': commit.raw_id,
|
||||
'commit_id': commit.raw_id,
|
||||
'date': commit.date
|
||||
}
|
||||
assert_dirs_in_response(response, ['docs', 'vcs'], params)
|
||||
|
|
@ -135,7 +135,7 @@ class TestFilesController:
|
|||
files = ['README.rst']
|
||||
params = {
|
||||
'repo_name': backend.repo_name,
|
||||
'revision': commit.raw_id,
|
||||
'commit_id': commit.raw_id,
|
||||
}
|
||||
assert_dirs_in_response(response, dirs, params)
|
||||
assert_files_in_response(response, files, params)
|
||||
|
|
@ -302,31 +302,34 @@ class TestFilesController:
|
|||
url('files_nodelist_home', repo_name=backend.repo_name,
|
||||
f_path='/', revision='tip'), status=400)
|
||||
|
||||
def test_tree_metadata_list_success(self, backend, xhr_header):
|
||||
def test_nodetree_full_success(self, backend, xhr_header):
|
||||
commit = backend.repo.get_commit(commit_idx=173)
|
||||
response = self.app.get(
|
||||
url('files_metadata_list_home', repo_name=backend.repo_name,
|
||||
f_path='/', revision=commit.raw_id),
|
||||
url('files_nodetree_full', repo_name=backend.repo_name,
|
||||
f_path='/', commit_id=commit.raw_id),
|
||||
extra_environ=xhr_header)
|
||||
|
||||
expected_keys = ['author', 'message', 'modified_at', 'modified_ts',
|
||||
'name', 'revision', 'short_id', 'size']
|
||||
for filename in response.json.get('metadata'):
|
||||
for key in expected_keys:
|
||||
assert key in filename
|
||||
assert_response = AssertResponse(response)
|
||||
|
||||
def test_tree_metadata_list_if_file(self, backend, xhr_header):
|
||||
for attr in ['data-commit-id', 'data-date', 'data-author']:
|
||||
elements = assert_response.get_elements('[{}]'.format(attr))
|
||||
assert len(elements) > 1
|
||||
|
||||
for element in elements:
|
||||
assert element.get(attr)
|
||||
|
||||
def test_nodetree_full_if_file(self, backend, xhr_header):
|
||||
commit = backend.repo.get_commit(commit_idx=173)
|
||||
response = self.app.get(
|
||||
url('files_metadata_list_home', repo_name=backend.repo_name,
|
||||
f_path='README.rst', revision=commit.raw_id),
|
||||
url('files_nodetree_full', repo_name=backend.repo_name,
|
||||
f_path='README.rst', commit_id=commit.raw_id),
|
||||
extra_environ=xhr_header)
|
||||
assert response.json == {'metadata': []}
|
||||
assert response.body == ''
|
||||
|
||||
def test_tree_metadata_list_missing_xhr(self, backend):
|
||||
self.app.get(
|
||||
url('files_metadata_list_home', repo_name=backend.repo_name,
|
||||
f_path='/', revision='tip'), status=400)
|
||||
url('files_nodetree_full', repo_name=backend.repo_name,
|
||||
f_path='/', commit_id='tip'), status=400)
|
||||
|
||||
def test_access_empty_repo_redirect_to_summary_with_alert_write_perms(
|
||||
self, app, backend_stub, autologin_regular_user, user_regular,
|
||||
|
|
@ -917,13 +920,13 @@ class TestChangingFiles:
|
|||
|
||||
def assert_files_in_response(response, files, params):
|
||||
template = (
|
||||
"href='/%(repo_name)s/files/%(revision)s/%(name)s'")
|
||||
'href="/%(repo_name)s/files/%(commit_id)s/%(name)s"')
|
||||
_assert_items_in_response(response, files, template, params)
|
||||
|
||||
|
||||
def assert_dirs_in_response(response, dirs, params):
|
||||
template = (
|
||||
"href='/%(repo_name)s/files/%(revision)s/%(name)s'")
|
||||
'href="/%(repo_name)s/files/%(commit_id)s/%(name)s"')
|
||||
_assert_items_in_response(response, dirs, template, params)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -171,6 +171,9 @@ class AssertResponse(object):
|
|||
assert len(elements) == 1
|
||||
return elements[0]
|
||||
|
||||
def get_elements(self, css_selector):
|
||||
return self._get_elements(css_selector)
|
||||
|
||||
def _get_elements(self, css_selector):
|
||||
doc = fromstring(self.response.body)
|
||||
sel = CSSSelector(css_selector)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue