# HG changeset patch # User Marcin Kuzminski # Date 2016-07-12 09:45:51 # Node ID 9930e2c89ff31974c762cb9710deac8a468c657d # Parent a5bc4f35d66a20ff34b5d1822648cae0d85fe985 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 diff --git a/rhodecode/config/routing.py b/rhodecode/config/routing.py --- a/rhodecode/config/routing.py +++ b/rhodecode/config/routing.py @@ -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) diff --git a/rhodecode/controllers/files.py b/rhodecode/controllers/files.py --- a/rhodecode/controllers/files.py +++ b/rhodecode/controllers/files.py @@ -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)) + + if dir_node.is_file(): + return '' - metadata = self._get_metadata_at_commit( - repo_name, commit, file_node) - return {'metadata': metadata} + 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) diff --git a/rhodecode/lib/caches.py b/rhodecode/lib/caches.py --- a/rhodecode/lib/caches.py +++ b/rhodecode/lib/caches.py @@ -35,7 +35,7 @@ FILE_SEARCH_TREE_META = 'cache_file_sear 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', diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js --- a/rhodecode/public/js/rhodecode/routes.js +++ b/rhodecode/public/js/rhodecode/routes.js @@ -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']); } diff --git a/rhodecode/templates/files/files.html b/rhodecode/templates/files/files.html --- a/rhodecode/templates/files/files.html +++ b/rhodecode/templates/files/files.html @@ -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( - '
r{1}:{2}
'.format( - data[i].message, data[i].revision, data[i].short_id)); - $('td[title="author-' + name + '"]').html( - '{1}'.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 @@ - diff --git a/rhodecode/templates/files/files_browser.html b/rhodecode/templates/files/files_browser.html --- a/rhodecode/templates/files/files_browser.html +++ b/rhodecode/templates/files/files_browser.html @@ -42,7 +42,9 @@ ## file tree is computed from caches, and filled in +
${c.file_tree} +
diff --git a/rhodecode/templates/files/files_browser_tree.html b/rhodecode/templates/files/files_browser_tree.html --- a/rhodecode/templates/files/files_browser_tree.html +++ b/rhodecode/templates/files/files_browser_tree.html @@ -1,4 +1,4 @@ -
+
@@ -31,7 +31,7 @@ ${h.link_to_if( node.url.startswith('http://') or node.url.startswith('https://'), - node.name,node.url)} + node.name, node.url)} %else: @@ -40,12 +40,30 @@ %endif %if node.is_file(): - - + - - + + %else: @@ -57,4 +75,4 @@
- ${_('Loading...')} + + % if c.full_load: + ${h.format_byte_size_binary(node.size)} + % else: + ${_('Loading ...')} + % endif + + % if c.full_load: + ${h.age_component(node.last_commit.date)} + % endif + % if c.full_load: +
+ 
r${node.last_commit.revision}:${node.last_commit.short_id}
+
+ % endif + 		+ % if c.full_load: + ${h.gravatar_with_user(node.last_commit.author)|n} + % endif +
-
\ No newline at end of file +
diff --git a/rhodecode/tests/functional/test_files.py b/rhodecode/tests/functional/test_files.py --- a/rhodecode/tests/functional/test_files.py +++ b/rhodecode/tests/functional/test_files.py @@ -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) diff --git a/rhodecode/tests/utils.py b/rhodecode/tests/utils.py --- a/rhodecode/tests/utils.py +++ b/rhodecode/tests/utils.py @@ -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)