diff --git a/swh/web/api/views/origin.py b/swh/web/api/views/origin.py --- a/swh/web/api/views/origin.py +++ b/swh/web/api/views/origin.py @@ -54,8 +54,9 @@ Origins are sorted by ids before returning them. - :query int origin_from: The first origin id that will be included - in returned results (default to 1) + :query string origin_from: The first origin id that will be included + in returned results (default to + 0000000000000000000000000000000000000000) :query int origin_count: The maximum number of origins to return (default to 100, can not exceed 10000) @@ -82,7 +83,7 @@ :swh_web_api:`origins?origin_from=50000&origin_count=500` """ # noqa - origin_from = int(request.query_params.get('origin_from', '1')) + origin_from = request.query_params.get('origin_from', '00'*20) origin_count = int(request.query_params.get('origin_count', '100')) origin_count = min(origin_count, 10000) results = api_lookup( @@ -92,12 +93,13 @@ if len(results) > origin_count: origin_from = results.pop()['id'] response['headers']['link-next'] = reverse( - 'api-origins', query_params={'origin_from': origin_from, - 'origin_count': origin_count}) + 'api-origins', + query_params={'origin_from': origin_from, + 'origin_count': origin_count}) return response -@api_route(r'/origin/(?P[0-9]+)/', 'api-origin') +@api_route(r'/origin/(?P[0-9a-f]+)/', 'api-origin') @api_route(r'/origin/(?P[a-z]+)/url/(?P.+)/', 'api-origin') @api_doc('/origin/') @@ -161,7 +163,7 @@ :swh_web_api:`origin/git/url/https://github.com/python/cpython/` """ # noqa ori_dict = { - 'id': int(origin_id) if origin_id else None, + 'id': origin_id, 'type': origin_type, 'url': origin_url } @@ -297,7 +299,7 @@ } -@api_route(r'/origin/(?P[0-9]+)/visits/', 'api-origin-visits') +@api_route(r'/origin/(?P[0-9a-f]+)/visits/', 'api-origin-visits') @api_doc('/origin/visits/') def api_origin_visits(request, origin_id): """ @@ -339,7 +341,7 @@ :swh_web_api:`origin/1/visits/` """ # noqa result = {} - origin_id = int(origin_id) + origin_id = origin_id per_page = int(request.query_params.get('per_page', '10')) last_visit = request.query_params.get('last_visit') if last_visit: @@ -377,9 +379,10 @@ query_params['per_page'] = per_page result['headers'] = { - 'link-next': reverse('api-origin-visits', - url_args={'origin_id': origin_id}, - query_params=query_params) + 'link-next': reverse( + 'api-origin-visits', + url_args={'origin_id': origin_id}, + query_params=query_params) } result.update({ @@ -389,7 +392,7 @@ return result -@api_route(r'/origin/(?P[0-9]+)/visit/(?P[0-9]+)/', +@api_route(r'/origin/(?P[0-9a-f]+)/visit/(?P[0-9]+)/', 'api-origin-visit') @api_doc('/origin/visit/') def api_origin_visit(request, origin_id, visit_id): @@ -406,7 +409,8 @@ :resheader Content-Type: this depends on :http:header:`Accept` header of request :>json string date: ISO representation of the visit date (in UTC) - :>json number origin: the origin unique identifier + :>json dict metadata: extra information on the visit + :>json string origin: the origin unique identifier :>json string origin_url: link to get information about the origin :>jsonarr string snapshot: the snapshot identifier of the visit :>jsonarr string snapshot_url: link to :http:get:`/api/1/snapshot/(snapshot_id)/` @@ -426,7 +430,7 @@ :swh_web_api:`origin/1500/visit/1/` """ # noqa return api_lookup( - service.lookup_origin_visit, int(origin_id), int(visit_id), + service.lookup_origin_visit, origin_id, int(visit_id), notfound_msg=('No visit {} for origin {} found' .format(visit_id, origin_id)), enrich_fn=partial(_enrich_origin_visit, diff --git a/swh/web/api/views/revision.py b/swh/web/api/views/revision.py --- a/swh/web/api/views/revision.py +++ b/swh/web/api/views/revision.py @@ -47,15 +47,15 @@ return result -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/log/', 'api-revision-origin-log') -@api_route(r'/revision/origin/(?P[0-9]+)/log/', +@api_route(r'/revision/origin/(?P[0-9a-z]+)/log/', 'api-revision-origin-log') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/ts/(?P.+)/log/', 'api-revision-origin-log') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)' r'/ts/(?P.+)/log/', 'api-revision-origin-log') @@ -126,7 +126,8 @@ error_msg += (' and time stamp %s.' % ts) if ts else '.' rev_get = api_lookup( - lookup_revision_log_by_with_limit, int(origin_id), branch_name, ts, + lookup_revision_log_by_with_limit, origin_id, + branch_name, ts, notfound_msg=error_msg, enrich_fn=utils.enrich_revision) @@ -159,20 +160,21 @@ return result -@api_route(r'/revision/origin/(?P[0-9]+)/directory/', +@api_route(r'/revision/origin/(?P[0-9a-z]+)/directory/', 'api-revision-origin-directory') -@api_route(r'/revision/origin/(?P[0-9]+)/directory/(?P.+)/', +@api_route(r'/revision/origin/(?P[0-9a-z]+)' + r'/directory/(?P.+)/', 'api-revision-origin-directory') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/directory/', 'api-revision-origin-directory') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/ts/(?P.+)/directory/', 'api-revision-origin-directory') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/directory/(?P.+)/', 'api-revision-origin-directory') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/ts/(?P.+)' r'/directory/(?P.+)/', 'api-revision-origin-directory') @@ -189,7 +191,7 @@ if ts: ts = parse_timestamp(ts) - return _revision_directory_by({'origin_id': int(origin_id), + return _revision_directory_by({'origin_id': origin_id, 'branch_name': branch_name, 'ts': ts }, @@ -197,15 +199,15 @@ with_data=with_data) -@api_route(r'/revision/origin/(?P[0-9]+)/', +@api_route(r'/revision/origin/(?P[0-9a-z]+)/', 'api-revision-origin') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/', 'api-revision-origin') -@api_route(r'/revision/origin/(?P[0-9]+)' +@api_route(r'/revision/origin/(?P[0-9a-z]+)' r'/branch/(?P.+)/ts/(?P.+)/', 'api-revision-origin') -@api_route(r'/revision/origin/(?P[0-9]+)/ts/(?P.+)/', +@api_route(r'/revision/origin/(?P[0-9a-z]+)/ts/(?P.+)/', 'api-revision-origin') @api_doc('/revision/origin/') def api_revision_with_origin(request, origin_id, @@ -264,7 +266,7 @@ :swh_web_api:`revision/origin/13706355/branch/refs/heads/2.7/` """ # noqa return api_lookup( - service.lookup_revision_by, int(origin_id), branch_name, ts, + service.lookup_revision_by, origin_id, branch_name, ts, notfound_msg=('Revision with (origin_id: {}, branch_name: {}' ', ts: {}) not found.'.format(origin_id, branch_name, ts)), diff --git a/swh/web/browse/views/origin.py b/swh/web/browse/views/origin.py --- a/swh/web/browse/views/origin.py +++ b/swh/web/browse/views/origin.py @@ -261,7 +261,7 @@ return HttpResponse(results, content_type='application/json') -@browse_route(r'origin/(?P[0-9]+)/latest_snapshot/', +@browse_route(r'origin/(?P[0-9a-f]+)/latest_snapshot/', view_name='browse-origin-latest-snapshot') def _origin_latest_snapshot(request, origin_id): """ diff --git a/swh/web/common/converters.py b/swh/web/common/converters.py --- a/swh/web/common/converters.py +++ b/swh/web/common/converters.py @@ -183,7 +183,7 @@ """Convert from a swh origin to an origin dictionary. """ - return from_swh(origin) + return from_swh(origin, hashess=['id']) def from_release(release): @@ -321,7 +321,7 @@ """ ov = from_swh(visit, - hashess={'target', 'snapshot'}, + hashess={'origin', 'target', 'snapshot'}, bytess={'branch'}, dates={'date'}, empty_dict={'metadata'}) 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 @@ -8,6 +8,7 @@ from collections import defaultdict from swh.model import hashutil +from swh.model.hashutil import hash_to_bytes, hash_to_hex from swh.storage.algos import revisions_walker @@ -42,9 +43,9 @@ the hash is present in storage, elem['found'] = false if not. """ - hashlist = [hashutil.hash_to_bytes(elem['sha1']) for elem in hashes] + hashlist = [hash_to_bytes(elem['sha1']) for elem in hashes] content_missing = storage.content_missing_per_sha1(hashlist) - missing = [hashutil.hash_to_hex(x) for x in content_missing] + missing = [hash_to_hex(x) for x in content_missing] for x in hashes: x.update({'found': True}) for h in hashes: @@ -219,10 +220,10 @@ origin information as dict. """ - origin_info = storage.origin_get(origin) + origin_info = storage.origin_get(_to_origin_sha1_bin_dict(origin)) if not origin_info: if 'id' in origin and origin['id']: - msg = 'Origin with id %s not found!' % origin['id'] + msg = 'Origin with id %s not found!' % hash_to_hex(origin['id']) else: msg = 'Origin with type %s and url %s not found!' % \ (origin['type'], origin['url']) @@ -230,7 +231,7 @@ return converters.from_origin(origin_info) -def lookup_origins(origin_from=1, origin_count=100): +def lookup_origins(origin_from=b'\x00'*20, origin_count=100): """Get list of archived software origins in a paginated way. Origins are sorted by id before returning them @@ -242,7 +243,8 @@ Yields: origins information as dicts """ - origins = storage.origin_get_range(origin_from, origin_count) + origins = storage.origin_get_range( + hash_to_bytes(origin_from), origin_count) return map(converters.from_origin, origins) @@ -285,6 +287,7 @@ result = converters.from_origin( storage.origin_get({'id': match.pop('id')})) result['metadata'] = match + result['id'] = hashutil.hash_to_hex(result['id']) results.append(result) return results @@ -316,6 +319,13 @@ return sha1_git_bin +def _to_origin_sha1_bin_dict(origin): + origin = origin.copy() + if 'id' in origin: + origin['id'] = hashutil.hash_to_bytes(origin['id']) + return origin + + def _check_directory_exists(sha1_git, sha1_git_bin): if len(list(storage.directory_missing([sha1_git_bin]))): raise NotFoundExc('Directory with sha1_git %s not found' % sha1_git) @@ -502,7 +512,7 @@ if not rev_id: raise NotFoundExc('Revision for origin %s and branch %s not found.' - % (origin_id, branch_name)) + % (hash_to_hex(origin_id), branch_name)) return rev_id @@ -802,7 +812,7 @@ """ limit = min(limit, MAX_LIMIT) yield from storage.origin_visit_get( - origin_id, last_visit=last_visit, limit=limit) + hash_to_bytes(origin_id), last_visit=last_visit, limit=limit) def lookup_origin_visits(origin_id, last_visit=None, per_page=10): @@ -833,10 +843,11 @@ The dict origin_visit concerned """ - visit = storage.origin_visit_get_by(origin_id, visit_id) + visit = storage.origin_visit_get_by(hash_to_bytes(origin_id), visit_id) if not visit: raise NotFoundExc('Origin with id %s or its visit ' - 'with id %s not found!' % (origin_id, visit_id)) + 'with id %s not found!' % + (origin_id, visit_id)) return converters.from_origin_visit(visit) @@ -903,7 +914,8 @@ Returns: A dict filled with the snapshot content. """ - snapshot = storage.snapshot_get_latest(origin_id, allowed_statuses) + snapshot = storage.snapshot_get_latest( + hash_to_bytes(origin_id), allowed_statuses) return converters.from_snapshot(snapshot) diff --git a/swh/web/tests/api/views/test_origin.py b/swh/web/tests/api/views/test_origin.py --- a/swh/web/tests/api/views/test_origin.py +++ b/swh/web/tests/api/views/test_origin.py @@ -6,11 +6,14 @@ import random from hypothesis import given +from hypothesis.strategies import integers from rest_framework.test import APITestCase from unittest.mock import patch from swh.storage.exc import StorageDBError, StorageAPIError +from swh.model.hashutil import hash_to_hex + from swh.web.common.utils import reverse from swh.web.common.origin_visits import get_origin_visits from swh.web.tests.strategies import ( @@ -30,7 +33,7 @@ mock_get_origin_visits.side_effect = ValueError(err_msg) - url = reverse('api-origin-visits', url_args={'origin_id': 2}) + url = reverse('api-origin-visits', url_args={'origin_id': '42'*20}) rv = self.client.get(url) self.assertEqual(rv.status_code, 400) @@ -47,10 +50,10 @@ mock_get_origin_visits.side_effect = StorageDBError(err_msg) - url = reverse('api-origin-visits', url_args={'origin_id': 2}) + url = reverse('api-origin-visits', url_args={'origin_id': '42'*20}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 503) + self.assertEqual(rv.status_code, 503, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') self.assertEqual(rv.data, { 'exception': 'StorageDBError', @@ -65,7 +68,7 @@ mock_get_origin_visits.side_effect = StorageAPIError(err_msg) - url = reverse('api-origin-visits', url_args={'origin_id': 2}) + url = reverse('api-origin-visits', url_args={'origin_id': '42'*20}) rv = self.client.get(url) self.assertEqual(rv.status_code, 503) @@ -94,25 +97,27 @@ (all_visits[1]['visit'], all_visits[2:4])): url = reverse('api-origin-visits', - url_args={'origin_id': origin_id}, + url_args={'origin_id': hash_to_hex(origin_id)}, query_params={'per_page': 2, 'last_visit': last_visit}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.status_code, 200, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') for expected_visit in expected_visits: origin_visit_url = reverse( 'api-origin-visit', - url_args={'origin_id': origin_id, + url_args={'origin_id': hash_to_hex(origin_id), 'visit_id': expected_visit['visit']}) snapshot_url = reverse( 'api-snapshot', url_args={'snapshot_id': expected_visit['snapshot']}) expected_visit['origin_visit_url'] = origin_visit_url expected_visit['snapshot_url'] = snapshot_url + expected_visit['origin'] = \ + hash_to_hex(expected_visit['origin']) self.assertEqual(rv.data, expected_visits) @@ -128,24 +133,27 @@ self.storage.snapshot_add(origin_id, origin_visit['visit'], new_snapshots[i]) url = reverse('api-origin-visit', - url_args={'origin_id': origin_id, + url_args={'origin_id': hash_to_hex(origin_id), 'visit_id': visit_id}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.status_code, 200, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') expected_visit = self.origin_visit_get_by(origin_id, visit_id) - origin_url = reverse('api-origin', - url_args={'origin_id': origin_id}) + origin_url = reverse( + 'api-origin', + url_args={'origin_id': hash_to_hex(origin_id)}) snapshot_url = reverse( 'api-snapshot', url_args={'snapshot_id': expected_visit['snapshot']}) + expected_visit['origin'] = hash_to_hex(expected_visit['origin']) expected_visit['origin_url'] = origin_url expected_visit['snapshot_url'] = snapshot_url + self.maxDiff = None self.assertEqual(rv.data, expected_visit) @given(origin()) @@ -156,34 +164,37 @@ max_visit_id = max([v['visit'] for v in all_visits]) url = reverse('api-origin-visit', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'visit_id': max_visit_id + 1}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 404) + self.assertEqual(rv.status_code, 404, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') self.assertEqual(rv.data, { 'exception': 'NotFoundExc', 'reason': 'Origin with id %s or its visit with id %s not found!' % - (origin['id'], max_visit_id+1) + (hash_to_hex(origin['id']), max_visit_id+1) }) @given(origin()) def test_api_origin_by_id(self, origin): - url = reverse('api-origin', url_args={'origin_id': origin['id']}) + url = reverse('api-origin', + url_args={'origin_id': hash_to_hex(origin['id'])}) rv = self.client.get(url) expected_origin = self.origin_get(origin) - origin_visits_url = reverse('api-origin-visits', - url_args={'origin_id': origin['id']}) + origin_visits_url = reverse( + 'api-origin-visits', + url_args={'origin_id': hash_to_hex(origin['id'])}) expected_origin['origin_visits_url'] = origin_visits_url + expected_origin['id'] = hash_to_hex(expected_origin['id']) - self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.status_code, 200, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') self.assertEqual(rv.data, expected_origin) @@ -193,16 +204,21 @@ url = reverse('api-origin', url_args={'origin_type': origin['type'], 'origin_url': origin['url']}) + from swh.web.config import get_config + get_config()['debug'] = True rv = self.client.get(url) + get_config()['debug'] = False expected_origin = self.origin_get(origin) - origin_visits_url = reverse('api-origin-visits', - url_args={'origin_id': origin['id']}) + origin_visits_url = reverse( + 'api-origin-visits', + url_args={'origin_id': hash_to_hex(origin['id'])}) expected_origin['origin_visits_url'] = origin_visits_url + expected_origin['id'] = hash_to_hex(expected_origin['id']) - self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.status_code, 200, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') self.assertEqual(rv.data, expected_origin) @@ -250,7 +266,7 @@ self.assertEqual(rv.status_code, 200, rv.content) self.assertEqual(rv['Content-Type'], 'application/json') expected_data = [{ - 'id': origin['id'], + 'id': hash_to_hex(origin['id']), 'type': origin['type'], 'url': origin['url'], 'metadata': { @@ -335,41 +351,48 @@ self.assertEqual(rv.status_code, 400, rv.content) mock_idx_storage.assert_not_called() - @given(new_origins(20)) - def test_api_lookup_origins(self, new_origins): + @given(new_origins(10), integers(min_value=1, max_value=9)) + def test_api_lookup_origins(self, new_origins, origin_count): + self.maxDiff = None + + self.storage.origin_add(new_origins) - nb_origins = len(new_origins) + origins = list(self.storage.origin_get_range(origin_count=10000)) + origins.sort(key=lambda origin: origin['id']) - expected_origins = self.storage.origin_add(new_origins) + nb_origins = len(origins) origin_from_idx = random.randint(1, nb_origins-1) - 1 - origin_from = expected_origins[origin_from_idx]['id'] - max_origin_id = expected_origins[-1]['id'] - origin_count = random.randint(1, max_origin_id - origin_from) + origin_from = origins[origin_from_idx]['id'] + + if origin_from_idx + origin_count >= len(origins): + origin_count = len(origins) - origin_from_idx - 1 url = reverse('api-origins', - query_params={'origin_from': origin_from, + query_params={'origin_from': hash_to_hex(origin_from), 'origin_count': origin_count}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.status_code, 200, rv.data) start = origin_from_idx end = origin_from_idx + origin_count - expected_origins = expected_origins[start:end] + expected_origins = origins[start:end] for expected_origin in expected_origins: + expected_origin['id'] = hash_to_hex(expected_origin['id']) expected_origin['origin_visits_url'] = reverse( 'api-origin-visits', url_args={'origin_id': expected_origin['id']}) self.assertEqual(rv.data, expected_origins) - next_origin_id = expected_origins[-1]['id']+1 + next_origin_id = origins[end]['id'] if self.storage.origin_get({'id': next_origin_id}): self.assertIn('Link', rv) - next_url = reverse('api-origins', - query_params={'origin_from': next_origin_id, - 'origin_count': origin_count}) + next_url = reverse( + 'api-origins', + query_params={'origin_from': hash_to_hex(next_origin_id), + 'origin_count': origin_count}) self.assertIn(next_url, rv['Link']) diff --git a/swh/web/tests/api/views/test_revision.py b/swh/web/tests/api/views/test_revision.py --- a/swh/web/tests/api/views/test_revision.py +++ b/swh/web/tests/api/views/test_revision.py @@ -101,7 +101,7 @@ def test_api_revision_with_origin_not_found(self, unknown_origin_id): url = reverse('api-revision-origin', - url_args={'origin_id': unknown_origin_id}) + url_args={'origin_id': hash_to_hex(unknown_origin_id)}) rv = self.client.get(url) self.assertEqual(rv.status_code, 404) @@ -109,13 +109,13 @@ self.assertEqual(rv.data, { 'exception': 'NotFoundExc', 'reason': 'Origin with id %s not found!' % - unknown_origin_id}) + hash_to_hex(unknown_origin_id)}) @given(origin()) def test_api_revision_with_origin(self, origin): url = reverse('api-revision-origin', - url_args={'origin_id': origin['id']}) + url_args={'origin_id': hash_to_hex(origin['id'])}) rv = self.client.get(url) snapshot = self.snapshot_get_latest(origin['id']) @@ -138,7 +138,7 @@ if snapshot['branches'][b]['target_type'] == 'revision')) url = reverse('api-revision-origin', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'branch_name': branch_name}) rv = self.client.get(url) @@ -164,7 +164,7 @@ if snapshot['branches'][b]['target_type'] == 'revision')) url = reverse('api-revision-origin', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'branch_name': branch_name, 'ts': visit['date']}) @@ -195,7 +195,7 @@ formatted_date = date.strftime('Today is %B %d, %Y at %X') url = reverse('api-revision-origin', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'branch_name': branch_name, 'ts': formatted_date}) @@ -215,22 +215,22 @@ unknown_origin_id): url = reverse('api-revision-origin-directory', - url_args={'origin_id': unknown_origin_id}) + url_args={'origin_id': hash_to_hex(unknown_origin_id)}) rv = self.client.get(url) - self.assertEqual(rv.status_code, 404) + self.assertEqual(rv.status_code, 404, rv.data) self.assertEqual(rv['Content-Type'], 'application/json') self.assertEqual(rv.data, { 'exception': 'NotFoundExc', 'reason': 'Origin with id %s not found!' % - unknown_origin_id + hash_to_hex(unknown_origin_id) }) @given(origin()) def test_api_directory_through_revision_origin(self, origin): url = reverse('api-revision-origin-directory', - url_args={'origin_id': origin['id']}) + url_args={'origin_id': hash_to_hex(origin['id'])}) rv = self.client.get(url) snapshot = self.snapshot_get_latest(origin['id']) @@ -246,7 +246,7 @@ ) entry['dir_url'] = reverse( 'api-revision-origin-directory', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'path': entry['name']}) elif entry['type'] == 'file': entry['target_url'] = reverse( @@ -255,7 +255,7 @@ ) entry['file_url'] = reverse( 'api-revision-origin-directory', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'path': entry['name']}) elif entry['type'] == 'rev': entry['target_url'] = reverse( @@ -264,7 +264,7 @@ ) entry['rev_url'] = reverse( 'api-revision-origin-directory', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'path': entry['name']}) expected_result = { @@ -354,7 +354,7 @@ per_page = 10 url = reverse('api-revision-origin-log', - url_args={'origin_id': origin['id']}, + url_args={'origin_id': hash_to_hex(origin['id'])}, query_params={'per_page': per_page}) rv = self.client.get(url) @@ -376,7 +376,7 @@ self.assertIn('Link', rv) next_log_url = reverse( 'api-revision-origin-log', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'branch_name': 'HEAD'}, query_params={'per_page': per_page, 'sha1_git': expected_log[-1]['id']}) @@ -388,7 +388,7 @@ invalid_branch_name = 'foobar' url = reverse('api-revision-origin-log', - url_args={'origin_id': origin['id'], + url_args={'origin_id': hash_to_hex(origin['id']), 'branch_name': invalid_branch_name}) rv = self.client.get(url) @@ -400,7 +400,7 @@ rv.data, {'exception': 'NotFoundExc', 'reason': 'Revision for origin %s and branch %s not found.' % - (origin['id'], invalid_branch_name)}) + (hash_to_hex(origin['id']), invalid_branch_name)}) @patch('swh.web.api.views.revision._revision_directory_by') def test_api_revision_directory_ko_not_found(self, mock_rev_dir): diff --git a/swh/web/tests/common/test_origin_visits.py b/swh/web/tests/common/test_origin_visits.py --- a/swh/web/tests/common/test_origin_visits.py +++ b/swh/web/tests/common/test_origin_visits.py @@ -34,11 +34,10 @@ mock_service.lookup_origin_visits.side_effect = _lookup_origin_visits - origin_info = { - 'id': 1, + origin_info = self.storage.origin_add([{ 'type': 'git', 'url': 'https://github.com/foo/bar', - } + }])[0] origin_visits = get_origin_visits(origin_info) diff --git a/swh/web/tests/common/test_service.py b/swh/web/tests/common/test_service.py --- a/swh/web/tests/common/test_service.py +++ b/swh/web/tests/common/test_service.py @@ -9,6 +9,7 @@ from collections import defaultdict from hypothesis import given +from hypothesis.strategies import integers from swh.model.hashutil import hash_to_bytes, hash_to_hex from swh.model.from_disk import DentryPerms @@ -192,7 +193,8 @@ self.storage.origin_visit_add(origin_id, ts) actual_origin_visits = list( - service.lookup_origin_visits(origin_id, per_page=100)) + service.lookup_origin_visits( + hash_to_hex(origin_id), per_page=100)) expected_visits = self.origin_visit_get(origin_id) @@ -206,27 +208,32 @@ visits.append(self.storage.origin_visit_add(origin_id, ts)) visit = random.choice(visits)['visit'] - actual_origin_visit = service.lookup_origin_visit(origin_id, visit) + actual_origin_visit = service.lookup_origin_visit( + hash_to_hex(origin_id), visit) - expected_visit = dict(self.storage.origin_visit_get_by(origin_id, - visit)) + expected_visit = dict(self.storage.origin_visit_get_by( + origin_id, visit)) expected_visit['date'] = expected_visit['date'].isoformat() expected_visit['metadata'] = {} + expected_visit['origin'] = hash_to_hex(expected_visit['origin']) self.assertEqual(actual_origin_visit, expected_visit) @given(new_origin()) def test_lookup_origin(self, new_origin): origin_id = self.storage.origin_add_one(new_origin) - actual_origin = service.lookup_origin({'id': origin_id}) + actual_origin = service.lookup_origin( + {'id': hash_to_hex(origin_id)}) expected_origin = self.storage.origin_get({'id': origin_id}) + expected_origin['id'] = hash_to_hex(expected_origin['id']) self.assertEqual(actual_origin, expected_origin) actual_origin = service.lookup_origin({'type': new_origin['type'], 'url': new_origin['url']}) expected_origin = self.storage.origin_get({'type': new_origin['type'], 'url': new_origin['url']}) + expected_origin['id'] = hash_to_hex(expected_origin['id']) self.assertEqual(actual_origin, expected_origin) @given(invalid_sha1()) @@ -768,7 +775,7 @@ rev = root_rev_log[-1]['id'] self.assertEqual(service.lookup_revision_through({ - 'origin_id': origin['id'], + 'origin_id': hash_to_hex(origin['id']), 'branch_name': branch_name, 'ts': None, 'sha1_git': rev @@ -784,7 +791,7 @@ branch_name = random.choice(list(branches.keys())) self.assertEqual(service.lookup_revision_through({ - 'origin_id': origin['id'], + 'origin_id': hash_to_hex(origin['id']), 'branch_name': branch_name, 'ts': None, }), @@ -856,20 +863,25 @@ revision, dir_entry['name'], with_data=True)) ) - @given(new_origins(20)) - def test_lookup_origins(self, new_origins): + @given(new_origins(10), integers(min_value=1, max_value=9)) + def test_lookup_origins(self, new_origins, origin_count): nb_origins = len(new_origins) - expected_origins = self.storage.origin_add(new_origins) + self.storage.origin_add(new_origins) + + origins = list(self.storage.origin_get_range(origin_count=10000)) + origins.sort(key=lambda orig: orig['id']) origin_from_idx = random.randint(1, nb_origins-1) - 1 - origin_from = expected_origins[origin_from_idx]['id'] - max_origin_idx = expected_origins[-1]['id'] - origin_count = random.randint(1, max_origin_idx - origin_from) + origin_from = origins[origin_from_idx]['id'] + if origin_from_idx + origin_count >= len(origins): + origin_count = len(origins) - origin_from_idx - 1 actual_origins = list(service.lookup_origins(origin_from, origin_count)) expected_origins = list(self.storage.origin_get_range(origin_from, origin_count)) + for orig in expected_origins: + orig['id'] = hash_to_hex(orig['id']) self.assertEqual(actual_origins, expected_origins) 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 @@ -114,20 +114,17 @@ # input data for tests _TEST_ORIGINS = [ { - 'id': 1, 'type': 'git', 'url': 'https://github.com/wcoder/highlightjs-line-numbers.js', 'archives': ['highlightjs-line-numbers.js.zip', 'highlightjs-line-numbers.js_visit2.zip'] }, { - 'id': 2, 'type': 'git', 'url': 'https://github.com/memononen/libtess2', 'archives': ['libtess2.zip'] }, { - 'id': 3, 'type': 'git', 'url': 'repo_with_submodules', 'archives': ['repo_with_submodules.tgz'] @@ -154,6 +151,7 @@ loader.load(origin['url'], origin_repo_archive, None) if nb_visits > 1 and i != nb_visits - 1: time.sleep(1) + origin['id'] = storage.origin_get(origin)['id'] contents = set() directories = set() 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 @@ -238,7 +238,7 @@ Hypothesis strategy returning a random origin id not ingested into the test archive. """ - return integers(min_value=1000000) + return binary(min_size=20, max_size=20) def new_origin():