diff --git a/swh/web/api/views/snapshot.py b/swh/web/api/views/snapshot.py --- a/swh/web/api/views/snapshot.py +++ b/swh/web/api/views/snapshot.py @@ -81,6 +81,7 @@ branches_from, branches_count, target_types, + branch_name_exclude_prefix=None, notfound_msg="Snapshot with id {} not found.".format(snapshot_id), enrich_fn=enrich_snapshot, request=request, diff --git a/swh/web/api/views/utils.py b/swh/web/api/views/utils.py --- a/swh/web/api/views/utils.py +++ b/swh/web/api/views/utils.py @@ -29,6 +29,7 @@ notfound_msg: Optional[str] = "Object not found", enrich_fn: Optional[EnrichFunction] = None, request: Optional[HttpRequest] = None, + **kwargs: Any, ): r""" Capture a redundant behavior of: @@ -64,7 +65,7 @@ if enrich_fn is None: enrich_fn = _enrich_fn_noop - res = lookup_fn(*args) + res = lookup_fn(*args, **kwargs) if res is None: raise NotFoundExc(notfound_msg) if isinstance(res, (list, GeneratorType)) or type(res) == map: diff --git a/swh/web/common/archive.py b/swh/web/common/archive.py --- a/swh/web/common/archive.py +++ b/swh/web/common/archive.py @@ -595,7 +595,10 @@ def _lookup_revision_id_by(origin, branch_name, timestamp): def _get_snapshot_branch(snapshot, branch_name): snapshot = lookup_snapshot( - visit["snapshot"], branches_from=branch_name, branches_count=10 + visit["snapshot"], + branches_from=branch_name, + branches_count=10, + branch_name_exclude_prefix=None, ) branch = None if branch_name in snapshot["branches"]: @@ -1024,7 +1027,7 @@ def lookup_snapshot_sizes( - snapshot_id: str, branch_name_exclude_prefix: Optional[str] = None + snapshot_id: str, branch_name_exclude_prefix: Optional[str] = "refs/pull/" ) -> Dict[str, int]: """Count the number of branches in the snapshot with the given id @@ -1054,7 +1057,7 @@ branches_count: int = 1000, target_types: Optional[List[str]] = None, branch_name_include_substring: Optional[str] = None, - branch_name_exclude_prefix: Optional[str] = None, + branch_name_exclude_prefix: Optional[str] = "refs/pull/", ) -> Dict[str, Any]: """Return information about a snapshot, aka the list of named branches found during a specific visit of an origin. @@ -1141,6 +1144,7 @@ target_types=[REVISION], branches_from=branches_from, branches_count=per_page + 1, + branch_name_exclude_prefix=None, ) branches += [ diff --git a/swh/web/tests/api/views/test_snapshot.py b/swh/web/tests/api/views/test_snapshot.py --- a/swh/web/tests/api/views/test_snapshot.py +++ b/swh/web/tests/api/views/test_snapshot.py @@ -12,7 +12,11 @@ from swh.web.api.utils import enrich_snapshot from swh.web.common.utils import reverse from swh.web.tests.data import random_sha1 -from swh.web.tests.strategies import new_snapshot, snapshot +from swh.web.tests.strategies import ( + new_snapshot, + origin_with_pull_request_branches, + snapshot, +) from swh.web.tests.utils import check_api_get_responses, check_http_get_response @@ -150,3 +154,15 @@ archive_data.snapshot_add([Snapshot.from_dict(snp_dict)]) url = reverse("api-1-snapshot", url_args={"snapshot_id": snp_id}) check_api_get_responses(api_client, url, status_code=200) + + +@given(origin_with_pull_request_branches()) +def test_api_snapshot_no_pull_request_branches_filtering( + api_client, archive_data, origin +): + """Pull request branches should not be filtered out when querying + a snapshot with the Web API.""" + snapshot = archive_data.snapshot_get_latest(origin.url) + url = reverse("api-1-snapshot", url_args={"snapshot_id": snapshot["id"]}) + resp = check_api_get_responses(api_client, url, status_code=200) + assert any([b.startswith("refs/pull/") for b in resp.data["branches"]]) 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 @@ -37,6 +37,7 @@ new_snapshot, origin, origin_with_multiple_visits, + origin_with_pull_request_branches, origin_with_releases, ) from swh.web.tests.strategies import release as existing_release @@ -1280,3 +1281,20 @@ "browse-origin", query_params={"origin_url": origin_url} ) assert_contains(resp, f'href="{browse_origin_url}"') + + +@given(origin_with_pull_request_branches()) +def test_pull_request_branches_filtering(client, origin): + # check no pull request branches are displayed in the Branches / Releases dropdown + url = reverse("browse-origin-directory", query_params={"origin_url": origin.url}) + resp = check_html_get_response( + client, url, status_code=200, template_used="browse/directory.html" + ) + assert_not_contains(resp, "refs/pull/") + + # check no pull request branches are displayed in the branches view + url = reverse("browse-origin-branches", query_params={"origin_url": origin.url}) + resp = check_html_get_response( + client, url, status_code=200, template_used="browse/branches.html" + ) + assert_not_contains(resp, "refs/pull/") diff --git a/swh/web/tests/data.py b/swh/web/tests/data.py --- a/swh/web/tests/data.py +++ b/swh/web/tests/data.py @@ -17,8 +17,15 @@ from swh.indexer.storage import get_indexer_storage from swh.indexer.storage.model import OriginIntrinsicMetadataRow from swh.loader.git.from_disk import GitLoaderFromArchive -from swh.model.hashutil import DEFAULT_ALGORITHMS, hash_to_bytes, hash_to_hex -from swh.model.model import Content, Directory, Origin, OriginVisit, OriginVisitStatus +from swh.model.hashutil import DEFAULT_ALGORITHMS, hash_to_hex +from swh.model.model import ( + Content, + Directory, + Origin, + OriginVisit, + OriginVisitStatus, + Snapshot, +) from swh.search import get_search from swh.storage import get_storage from swh.storage.algos.dir_iterators import dir_iterator @@ -166,6 +173,27 @@ ORIGIN_MASTER_REVISION = {} +def _add_origin(storage, search, origin_url, visit_type="git", snapshot_branches={}): + storage.origin_add([Origin(url=origin_url)]) + search.origin_update( + [{"url": origin_url, "has_visits": True, "visit_types": [visit_type]}] + ) + date = now() + visit = OriginVisit(origin=origin_url, date=date, type=visit_type) + visit = storage.origin_visit_add([visit])[0] + snapshot = Snapshot.from_dict({"branches": snapshot_branches}) + storage.snapshot_add([snapshot]) + visit_status = OriginVisitStatus( + origin=origin_url, + visit=visit.visit, + date=date + timedelta(minutes=1), + type=visit.type, + status="full", + snapshot=snapshot.id, + ) + storage.origin_visit_status_add([visit_status]) + + # Tests data initialization def _init_tests_data(): # To hold reference to the memory storage @@ -207,22 +235,9 @@ ) for i in range(250): - url = "https://many.origins/%d" % (i + 1) - # storage.origin_add([{'url': url}]) - storage.origin_add([Origin(url=url)]) - search.origin_update([{"url": url, "has_visits": True, "visit_types": ["tar"]}]) - date = now() - visit = OriginVisit(origin=url, date=date, type="tar") - visit = storage.origin_visit_add([visit])[0] - visit_status = OriginVisitStatus( - origin=url, - visit=visit.visit, - date=date + timedelta(minutes=1), - type=visit.type, - status="full", - snapshot=hash_to_bytes("1a8893e6a86f444e8be8e7bda6cb34fb1735a00e"), + _add_origin( + storage, search, origin_url=f"https://many.origins/{i+1}", visit_type="tar" ) - storage.origin_visit_status_add([visit_status]) sha1s: Set[Sha1] = set() directories = set() @@ -324,6 +339,26 @@ # Add empty content to the test archive storage.content_add([Content.from_data(data=b"")]) + # Add fake git origin with pull request branches + _add_origin( + storage, + search, + origin_url="https://git.example.org/project", + snapshot_branches={ + b"refs/heads/master": { + "target_type": "revision", + "target": next(iter(revisions)), + }, + **{ + f"refs/pull/{i}".encode(): { + "target_type": "revision", + "target": next(iter(revisions)), + } + for i in range(300) + }, + }, + ) + # Return tests data return { "search": search, 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 @@ -298,6 +298,22 @@ return sampled_from(ret) +def origin_with_pull_request_branches(): + """ + Hypothesis strategy returning a random origin with pull request branches + ingested into the test archive. + """ + ret = [] + tests_data = get_tests_data() + storage = tests_data["storage"] + origins = storage.origin_list(limit=1000) + for origin in origins.results: + snapshot = snapshot_get_latest(storage, origin.url) + if any([b"refs/pull/" in b for b in snapshot.branches]): + ret.append(origin) + return sampled_from(ret) + + def new_origin(): """ Hypothesis strategy returning a random origin not ingested