diff --git a/swh/web/browse/utils.py b/swh/web/browse/utils.py --- a/swh/web/browse/utils.py +++ b/swh/web/browse/utils.py @@ -958,9 +958,9 @@ releases = list(reversed(releases)) - snapshot_size = service.lookup_snapshot_size(snapshot_id) + snapshot_sizes = service.lookup_snapshot_sizes(snapshot_id) - is_empty = sum(snapshot_size.values()) == 0 + is_empty = sum(snapshot_sizes.values()) == 0 swh_snp_id = persistent_identifier('snapshot', snapshot_id) @@ -968,7 +968,7 @@ 'swh_type': swh_type, 'swh_object_id': swh_snp_id, 'snapshot_id': snapshot_id, - 'snapshot_size': snapshot_size, + 'snapshot_sizes': snapshot_sizes, 'is_empty': is_empty, 'origin_info': origin_info, 'visit_info': visit_info, diff --git a/swh/web/browse/views/utils/snapshot_context.py b/swh/web/browse/views/utils/snapshot_context.py --- a/swh/web/browse/views/utils/snapshot_context.py +++ b/swh/web/browse/views/utils/snapshot_context.py @@ -63,7 +63,7 @@ return snp_branch[0] -def _get_release(releases, release_name): +def _get_release(releases, release_name, snapshot_id): """ Utility function to get a specific release from a releases list. Returns None if the release can not be found in the list. @@ -71,10 +71,21 @@ filtered_releases = [r for r in releases if r['name'] == release_name] if filtered_releases: return filtered_releases[0] - - -def _branch_not_found(branch_type, branch, branches, snapshot_id=None, - origin_info=None, timestamp=None, visit_id=None): + else: + # case where a large branches list has been truncated + for branch_name in (release_name, f'refs/tags/{release_name}'): + snp = service.lookup_snapshot(snapshot_id, + branches_from=branch_name, + branches_count=1, + target_types=['release']) + _, snp_release = process_snapshot_branches(snp) + if snp_release and snp_release[0]['name'] == release_name: + releases.append(snp_release[0]) + return snp_release[0] + + +def _branch_not_found(branch_type, branch, snapshot_id, snapshot_sizes, + origin_info, timestamp, visit_id): """ Utility function to raise an exception when a specified branch/release can not be found. @@ -82,17 +93,19 @@ if branch_type == 'branch': branch_type = 'Branch' branch_type_plural = 'branches' + target_type = 'revision' else: branch_type = 'Release' branch_type_plural = 'releases' + target_type = 'release' - if snapshot_id and not branches: + if snapshot_id and snapshot_sizes[target_type] == 0: msg = ('Snapshot with id %s has an empty list' ' of %s!' % (snapshot_id, branch_type_plural)) elif snapshot_id: msg = ('%s %s for snapshot with id %s' ' not found!' % (branch_type, branch, snapshot_id)) - elif visit_id and not branches: + elif visit_id and snapshot_sizes[target_type] == 0: msg = ('Origin with url %s' ' for visit with id %s has an empty list' ' of %s!' % (origin_info['url'], visit_id, @@ -102,7 +115,7 @@ ' id %s for origin with url %s' ' not found!' % (branch_type, branch, visit_id, origin_info['url'])) - elif not branches: + elif snapshot_sizes[target_type] == 0: msg = ('Origin with url %s' ' for visit with timestamp %s has an empty list' ' of %s!' % (origin_info['url'], @@ -149,7 +162,8 @@ release_id = None branch_name = None - snapshot_total_size = sum(snapshot_context['snapshot_size'].values()) + snapshot_sizes = snapshot_context['snapshot_sizes'] + snapshot_total_size = sum(snapshot_sizes.values()) if snapshot_total_size and revision_id: revision = service.lookup_revision(revision_id) @@ -161,15 +175,16 @@ branch_name = revision_id query_params['revision'] = revision_id elif snapshot_total_size and release_name: - release = _get_release(releases, release_name) + release = _get_release(releases, release_name, + snapshot_context['snapshot_id']) try: root_sha1_git = release['directory'] revision_id = release['target'] release_id = release['id'] query_params['release'] = release_name except Exception: - _branch_not_found("release", release_name, releases, snapshot_id, - origin_info, timestamp, visit_id) + _branch_not_found('release', release_name, snapshot_id, + snapshot_sizes, origin_info, timestamp, visit_id) elif snapshot_total_size: branch_name = request.GET.get('branch', None) if branch_name: @@ -181,8 +196,8 @@ revision_id = branch['revision'] root_sha1_git = branch['directory'] except Exception: - _branch_not_found("branch", branch_name, branches, snapshot_id, - origin_info, timestamp, visit_id) + _branch_not_found('branch', branch_name, snapshot_id, + snapshot_sizes, origin_info, timestamp, visit_id) for b in branches: branch_url_args = dict(url_args) @@ -412,6 +427,7 @@ sha1_git = None query_string = None content_data = None + directory_id = None split_path = path.split('/') filename = split_path[-1] filepath = path[:-len(filename)] diff --git a/swh/web/common/service.py b/swh/web/common/service.py --- a/swh/web/common/service.py +++ b/swh/web/common/service.py @@ -876,7 +876,7 @@ return converters.from_origin_visit(visit) -def lookup_snapshot_size(snapshot_id): +def lookup_snapshot_sizes(snapshot_id): """Count the number of branches in the snapshot with the given id Args: @@ -887,26 +887,26 @@ values their corresponding amount """ snapshot_id_bin = _to_sha1_bin(snapshot_id) - snapshot_size = storage.snapshot_count_branches(snapshot_id_bin) - if 'revision' not in snapshot_size: - snapshot_size['revision'] = 0 - if 'release' not in snapshot_size: - snapshot_size['release'] = 0 + snapshot_sizes = storage.snapshot_count_branches(snapshot_id_bin) + if 'revision' not in snapshot_sizes: + snapshot_sizes['revision'] = 0 + if 'release' not in snapshot_sizes: + snapshot_sizes['release'] = 0 # adjust revision / release count for display if aliases are defined - if 'alias' in snapshot_size: + if 'alias' in snapshot_sizes: aliases = lookup_snapshot(snapshot_id, - branches_count=snapshot_size['alias'], + branches_count=snapshot_sizes['alias'], target_types=['alias']) for alias in aliases['branches'].values(): if lookup_snapshot(snapshot_id, branches_from=alias['target'], branches_count=1, target_types=['revision']): - snapshot_size['revision'] += 1 + snapshot_sizes['revision'] += 1 else: - snapshot_size['release'] += 1 - del snapshot_size['alias'] - return snapshot_size + snapshot_sizes['release'] += 1 + del snapshot_sizes['alias'] + return snapshot_sizes def lookup_snapshot(snapshot_id, branches_from='', branches_count=1000, diff --git a/swh/web/templates/includes/snapshot-context.html b/swh/web/templates/includes/snapshot-context.html --- a/swh/web/templates/includes/snapshot-context.html +++ b/swh/web/templates/includes/snapshot-context.html @@ -16,16 +16,16 @@ {% if snapshot_context.visit_info %}
  • Snapshot date: {{ snapshot_context.visit_info.fmt_date }}
    • {% endif %} - {% if not snapshot_context.snapshot_size.revision %} + {% if not snapshot_context.snapshot_sizes.revision %}
  • Branches (0)
    • {% else %} -
  • Branches ({{ snapshot_context.snapshot_size.revision}})
    • +
  • Branches ({{ snapshot_context.snapshot_sizes.revision}})
    • {% endif %} - {% if not snapshot_context.snapshot_size.release %} + {% if not snapshot_context.snapshot_sizes.release %}
  • Releases (0)
    • {% else %} -
  • Releases ({{ snapshot_context.snapshot_size.release }})
    • +
  • Releases ({{ snapshot_context.snapshot_sizes.release }})
    • {% endif %}
    \ No newline at end of file diff --git a/swh/web/templates/includes/top-navigation.html b/swh/web/templates/includes/top-navigation.html --- a/swh/web/templates/includes/top-navigation.html +++ b/swh/web/templates/includes/top-navigation.html @@ -41,11 +41,11 @@ {% endfor %} - {% if snapshot_context.branches|length < snapshot_context.snapshot_size.revision %} + {% if snapshot_context.branches|length < snapshot_context.snapshot_sizes.revision %}
  • Branches list truncated to {{ snapshot_context.branches|length }} entries, - {{ snapshot_context.branches|length|mul:-1|add:snapshot_context.snapshot_size.revision }} + {{ snapshot_context.branches|length|mul:-1|add:snapshot_context.snapshot_sizes.revision }} were omitted.
    • {% endif %} @@ -67,11 +67,11 @@ {% endif %} {% endfor %} - {% if snapshot_context.releases|length < snapshot_context.snapshot_size.release %} + {% if snapshot_context.releases|length < snapshot_context.snapshot_sizes.release %}
  • Releases list truncated to {{ snapshot_context.releases|length }} entries, - {{ snapshot_context.releases|length|mul:-1|add:snapshot_context.snapshot_size.release }} + {{ snapshot_context.releases|length|mul:-1|add:snapshot_context.snapshot_sizes.release }} were omitted.
    • {% endif %} diff --git a/swh/web/tests/browse/views/test_origin.py b/swh/web/tests/browse/views/test_origin.py --- a/swh/web/tests/browse/views/test_origin.py +++ b/swh/web/tests/browse/views/test_origin.py @@ -6,6 +6,8 @@ import random import re +import swh.web.browse.utils + from django.utils.html import escape from hypothesis import given @@ -21,7 +23,7 @@ from swh.web.tests.django_asserts import assert_contains, assert_template_used from swh.web.tests.strategies import ( origin, origin_with_multiple_visits, new_origin, - new_snapshot, visit_dates, revisions + new_snapshot, visit_dates, revisions, origin_with_releases ) @@ -364,7 +366,7 @@ 'message': ''}], [] ) - mock_utils_service.lookup_snapshot_size.return_value = { + mock_utils_service.lookup_snapshot_sizes.return_value = { 'revision': 1, 'release': 0 } @@ -407,18 +409,25 @@ 'origin': 457, 'snapshot': 'bdaf9ac436488a8c6cda927a0f44e172934d3f65', 'status': 'full', + 'type': 'git', 'visit': 1 }] mock_get_origin_visit_snapshot.side_effect = None mock_get_origin_visit_snapshot.return_value = ([], []) + mock_utils_service.lookup_snapshot_sizes.return_value = { + 'revision': 0, + 'release': 0 + } + mock_utils_service.lookup_origin.return_value = {'type': 'foo', + 'url': 'bar', + 'id': 457} url = reverse('browse-origin-content', url_args={'origin_url': 'bar', 'path': 'baz'}) resp = client.get(url) - assert resp.status_code == 404 + assert resp.status_code == 200 assert_template_used('error.html') - assert re.search('Origin.*has an empty list of branches', - resp.content.decode('utf-8')) + assert re.search('snapshot.*is empty', resp.content.decode('utf-8')) mock_get_origin_visit_snapshot.return_value = ( [{'directory': 'ae59ceecf46367e8e4ad800e231fc76adc3afffb', @@ -428,6 +437,10 @@ 'message': ''}], [] ) + mock_utils_service.lookup_snapshot_sizes.return_value = { + 'revision': 1, + 'release': 0 + } mock_snapshot_service.lookup_directory_with_path.return_value = { 'target': '5ecd9f37b7a2d2e9980d201acd6286116f2ba1f1' } @@ -468,7 +481,7 @@ 'visit': 1 }] mock_get_origin_visit_snapshot.return_value = ([], []) - mock_utils_service.lookup_snapshot_size.return_value = { + mock_utils_service.lookup_snapshot_sizes.return_value = { 'revision': 0, 'release': 0 } @@ -484,6 +497,42 @@ assert re.search('snapshot.*is empty', resp.content.decode('utf-8')) +@given(origin_with_releases()) +def test_origin_release_browse(client, archive_data, origin): + # for swh.web.browse.utils.get_snapshot_content to only return one branch + snapshot_max_size = swh.web.browse.utils.snapshot_content_max_size + swh.web.browse.utils.snapshot_content_max_size = 1 + try: + snapshot = archive_data.snapshot_get_latest(origin['url']) + release = [b for b in snapshot['branches'].values() + if b['target_type'] == 'release'][-1] + release_data = archive_data.release_get(release['target']) + url = reverse('browse-origin-directory', + url_args={'origin_url': origin['url']}, + query_params={'release': release_data['name']}) + + resp = client.get(url) + assert resp.status_code == 200 + assert_contains(resp, release_data['name']) + assert_contains(resp, release['target']) + finally: + swh.web.browse.utils.snapshot_content_max_size = snapshot_max_size + + +@given(origin_with_releases()) +def test_origin_release_browse_not_found(client, archive_data, origin): + + invalid_release_name = 'swh-foo-bar' + url = reverse('browse-origin-directory', + url_args={'origin_url': origin['url']}, + query_params={'release': invalid_release_name}) + + resp = client.get(url) + assert resp.status_code == 404 + assert re.search(f'Release {invalid_release_name}.*not found', + resp.content.decode('utf-8')) + + def _origin_content_view_test_helper(client, origin_info, origin_visits, origin_branches, origin_releases, root_dir_sha1, content, diff --git a/swh/web/tests/browse/views/test_release.py b/swh/web/tests/browse/views/test_release.py --- a/swh/web/tests/browse/views/test_release.py +++ b/swh/web/tests/browse/views/test_release.py @@ -12,7 +12,7 @@ ) from swh.web.tests.django_asserts import assert_contains, assert_template_used from swh.web.tests.strategies import ( - release, origin_with_release, unknown_release + release, origin_with_releases, unknown_release ) @@ -28,7 +28,7 @@ _release_browse_checks(resp, release_data) -@given(origin_with_release()) +@given(origin_with_releases()) def test_release_browse_with_origin(client, archive_data, origin): snapshot = archive_data.snapshot_get_latest(origin['url']) release = random.choice([b for b in snapshot['branches'].values() diff --git a/swh/web/tests/strategies.py b/swh/web/tests/strategies.py --- a/swh/web/tests/strategies.py +++ b/swh/web/tests/strategies.py @@ -235,7 +235,7 @@ return sampled_from(ret) -def origin_with_release(): +def origin_with_releases(): """ Hypothesis strategy returning a random origin ingested into the test archive.