From f48e94aa143ea0d1580831df8903fbd60697dedd Mon Sep 17 00:00:00 2001 From: Marcin Kuzminski Date: Tue, 12 Jul 2016 11:45:51 +0200 Subject: [PATCH] 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 --- rhodecode/config/routing.py | 6 +- rhodecode/controllers/files.py | 56 +++++++++---------- rhodecode/lib/caches.py | 2 +- rhodecode/public/js/rhodecode/routes.js | 5 +- rhodecode/templates/files/files.html | 33 ++++------- rhodecode/templates/files/files_browser.html | 2 + .../templates/files/files_browser_tree.html | 34 ++++++++--- rhodecode/tests/functional/test_files.py | 39 +++++++------ rhodecode/tests/utils.py | 3 + 9 files changed, 98 insertions(+), 82 deletions(-) diff --git a/rhodecode/config/routing.py b/rhodecode/config/routing.py index b0420375..c246a3d7 100644 --- 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 index 25723a2c..b4a59a9e 100644 --- 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)) - 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) diff --git a/rhodecode/lib/caches.py b/rhodecode/lib/caches.py index 924d3632..05de576f 100644 --- a/rhodecode/lib/caches.py +++ b/rhodecode/lib/caches.py @@ -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', diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js index 12deb8d1..f2567cef 100644 --- 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 index f8a01e7e..2f99da2c 100644 --- 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 index c486519b..ee539255 100644 --- 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 index 43d8d8a4..220a6852 100644 --- 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 index 092017de..dc4c3ff4 100644 --- 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 index 49329dc3..1ceb41d9 100644 --- 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)