diff --git a/swh/storage/api/client.py b/swh/storage/api/client.py --- a/swh/storage/api/client.py +++ b/swh/storage/api/client.py @@ -93,24 +93,8 @@ def object_find_by_sha1_git(self, ids): return self.post('object/find_by_sha1_git', {'ids': ids}) - def snapshot_add(self, snapshots, origin=None, visit=None): - if origin: - assert visit - (origin, visit, snapshots) = (snapshots, origin, visit) - warnings.warn("arguments 'origin' and 'visit' of snapshot_add " - "are deprecated since v0.0.131, please use " - "snapshot_add([snapshot]) + " - "origin_visit_update(origin, visit, " - "snapshot=snapshot['id']) instead.", - DeprecationWarning) - return self.post('snapshot/add', { - 'origin': origin, 'visit': visit, 'snapshots': snapshots, - }) - else: - assert not visit - return self.post('snapshot/add', { - 'snapshots': snapshots, - }) + def snapshot_add(self, snapshots): + return self.post('snapshot/add', {'snapshots': snapshots}) def snapshot_get(self, snapshot_id): return self.post('snapshot', { diff --git a/swh/storage/api/server.py b/swh/storage/api/server.py --- a/swh/storage/api/server.py +++ b/swh/storage/api/server.py @@ -307,8 +307,6 @@ @process_metrics def snapshot_add(): req_data = decode_request(request) - if 'snapshot' in req_data: - req_data['snapshots'] = req_data.pop('snapshot') return get_storage().snapshot_add(**req_data) diff --git a/swh/storage/in_memory.py b/swh/storage/in_memory.py --- a/swh/storage/in_memory.py +++ b/swh/storage/in_memory.py @@ -670,7 +670,7 @@ for rel_id in releases: yield copy.deepcopy(self._releases.get(rel_id)) - def snapshot_add(self, snapshots, origin=None, visit=None): + def snapshot_add(self, snapshots): """Add a snapshot to the storage Args: @@ -699,19 +699,6 @@ snapshot_added: Count of object actually stored in db """ - if origin: - if not visit: - raise TypeError( - 'snapshot_add expects one argument (or, as a legacy ' - 'behavior, three arguments), not two') - if isinstance(snapshots, (int, str)): - # Called by legacy code that uses the new api/client.py - (origin, visit, snapshots) = \ - (snapshots, origin, [visit]) - else: - # Called by legacy code that uses the old api/client.py - snapshots = [snapshots] - count = 0 for snapshot in snapshots: snapshot_id = snapshot['id'] @@ -727,11 +714,6 @@ self._objects[snapshot_id].append(('snapshot', snapshot_id)) count += 1 - if visit: - # Legacy API, there can be only one snapshot - self.origin_visit_update( - origin, visit, snapshot=snapshots[0]['id']) - return {'snapshot:add': count} def snapshot_get(self, snapshot_id): diff --git a/swh/storage/storage.py b/swh/storage/storage.py --- a/swh/storage/storage.py +++ b/swh/storage/storage.py @@ -885,8 +885,7 @@ yield data if data['target_type'] else None @db_transaction() - def snapshot_add(self, snapshots, origin=None, visit=None, - db=None, cur=None): + def snapshot_add(self, snapshots, db=None, cur=None): """Add snapshots to the storage. Args: @@ -905,8 +904,6 @@ - **target** (:class:`bytes`): identifier of the target (currently a ``sha1_git`` for all object kinds, or the name of the target branch for aliases) - origin (int): legacy argument for backward compatibility - visit (int): legacy argument for backward compatibility Raises: ValueError: if the origin or visit id does not exist. @@ -918,24 +915,6 @@ snapshot:add: Count of object actually stored in db """ - if origin: - if not visit: - raise TypeError( - 'snapshot_add expects one argument (or, as a legacy ' - 'behavior, three arguments), not two') - if isinstance(snapshots, (int, str)): - # Called by legacy code that uses the new api/client.py - (origin_id, visit_id, snapshots) = \ - (snapshots, origin, [visit]) - else: - # Called by legacy code that uses the old api/client.py - origin_id = origin - visit_id = visit - snapshots = [snapshots] - else: - # Called by new code that uses the new api/client.py - origin_id = visit_id = None - created_temp_table = False count = 0 @@ -966,12 +945,6 @@ db.snapshot_add(snapshot['id'], cur) count += 1 - if visit_id: - # Legacy API, there can be only one snapshot - self.origin_visit_update( - origin_id, visit_id, snapshot=snapshots[0]['id'], - db=db, cur=cur) - return {'snapshot:add': count} @db_transaction(statement_timeout=2000) diff --git a/swh/storage/tests/algos/test_snapshot.py b/swh/storage/tests/algos/test_snapshot.py --- a/swh/storage/tests/algos/test_snapshot.py +++ b/swh/storage/tests/algos/test_snapshot.py @@ -26,9 +26,7 @@ snapshots(min_size=0, max_size=10, only_objects=False)) def test_snapshot_small(self, origin, ts, snapshot): snapshot = snapshot.to_dict() - origin_id = self.storage.origin_add_one(origin) - visit = self.storage.origin_visit_add(origin_id, ts) - self.storage.snapshot_add(origin_id, visit['visit'], snapshot) + self.storage.snapshot_add([snapshot]) returned_snapshot = snapshot_get_all_branches(self.storage, snapshot['id']) @@ -39,8 +37,6 @@ branch_names(), branch_targets(only_objects=True)) def test_snapshot_large(self, origin, ts, branch_name, branch_target): branch_target = branch_target.to_dict() - origin_id = self.storage.origin_add_one(origin) - visit = self.storage.origin_visit_add(origin_id, ts) snapshot = { 'branches': { @@ -50,7 +46,7 @@ } snapshot['id'] = identifier_to_bytes(snapshot_identifier(snapshot)) - self.storage.snapshot_add(origin_id, visit['visit'], snapshot) + self.storage.snapshot_add([snapshot]) returned_snapshot = snapshot_get_all_branches(self.storage, snapshot['id']) diff --git a/swh/storage/tests/test_storage.py b/swh/storage/tests/test_storage.py --- a/swh/storage/tests/test_storage.py +++ b/swh/storage/tests/test_storage.py @@ -1911,8 +1911,11 @@ origin_id_or_url, date=self.date_visit2) - self.storage.snapshot_add(origin_id, origin_visit1['visit'], - self.snapshot) + self.storage.snapshot_add([self.snapshot]) + self.storage.origin_visit_update( + origin_id_or_url, + origin_visit1['visit'], + snapshot=self.snapshot['id']) # Add some other {origin, visit} entries self.storage.origin_visit_add( @@ -2189,8 +2192,8 @@ # visit1 and require_snapshot=False still returns visit2 self.storage.snapshot_add([self.complete_snapshot]) self.storage.origin_visit_update( - origin_id_or_url, - visit1_id, snapshot=self.complete_snapshot['id']) + origin_id_or_url, visit1_id, + snapshot=self.complete_snapshot['id']) self.assertEqual( {**origin_visit1, 'snapshot': self.complete_snapshot['id']}, self.storage.origin_visit_get_latest( @@ -2228,8 +2231,8 @@ # Add snapshot to visit2 and check that the new snapshot is returned self.storage.snapshot_add([self.empty_snapshot]) self.storage.origin_visit_update( - origin_id_or_url, - visit2_id, snapshot=self.empty_snapshot['id']) + origin_id_or_url, visit2_id, + snapshot=self.empty_snapshot['id']) self.assertEqual( {**origin_visit2, 'snapshot': self.empty_snapshot['id']}, self.storage.origin_visit_get_latest( @@ -2253,8 +2256,7 @@ # Add snapshot to visit3 (same date as visit2) self.storage.snapshot_add([self.complete_snapshot]) self.storage.origin_visit_update( - origin_id_or_url, - visit3_id, snapshot=self.complete_snapshot['id']) + origin_id_or_url, visit3_id, snapshot=self.complete_snapshot['id']) self.assertEqual( { **origin_visit1, @@ -2356,53 +2358,15 @@ ('snapshot', self.empty_snapshot), ('origin_visit', data2)]) - def test_snapshot_add_get_empty__legacy_add(self): - origin_id = self.storage.origin_add_one(self.origin) - origin_visit1 = self.storage.origin_visit_add(origin_id, - self.date_visit1) - visit_id = origin_visit1['visit'] - - self.storage.snapshot_add(origin_id, visit_id, self.empty_snapshot) - - by_id = self.storage.snapshot_get(self.empty_snapshot['id']) - self.assertEqual(by_id, self.empty_snapshot) - - by_ov = self.storage.snapshot_get_by_origin_visit(origin_id, visit_id) - self.assertEqual(by_ov, self.empty_snapshot) - - expected_origin = self.origin.copy() - data1 = { - 'origin': expected_origin, - 'date': self.date_visit1, - 'visit': origin_visit1['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': None, - } - data2 = { - 'origin': expected_origin, - 'date': self.date_visit1, - 'visit': origin_visit1['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': self.empty_snapshot['id'], - } - self.assertEqual(list(self.journal_writer.objects), - [('origin', expected_origin), - ('origin_visit', data1), - ('snapshot', self.empty_snapshot), - ('origin_visit', data2)]) - def test_snapshot_add_get_complete(self): origin_id = self.storage.origin_add_one(self.origin) origin_visit1 = self.storage.origin_visit_add(origin_id, self.date_visit1) visit_id = origin_visit1['visit'] - actual_result = self.storage.snapshot_add( - origin_id, visit_id, self.complete_snapshot) + actual_result = self.storage.snapshot_add([self.complete_snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, snapshot=self.complete_snapshot['id']) self.assertEqual(actual_result, {'snapshot:add': 1}) by_id = self.storage.snapshot_get(self.complete_snapshot['id']) @@ -2446,8 +2410,9 @@ self.date_visit1) visit_id = origin_visit1['visit'] - actual_result = self.storage.snapshot_add( - origin_id, visit_id, self.complete_snapshot) + actual_result = self.storage.snapshot_add([self.complete_snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, snapshot=self.complete_snapshot['id']) self.assertEqual(actual_result, {'snapshot:add': 1}) snp_id = self.complete_snapshot['id'] @@ -2471,7 +2436,10 @@ self.date_visit1) visit_id = origin_visit1['visit'] - self.storage.snapshot_add(origin_id, visit_id, self.complete_snapshot) + self.storage.snapshot_add([self.complete_snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, + snapshot=self.complete_snapshot['id']) snp_id = self.complete_snapshot['id'] branches = self.complete_snapshot['branches'] @@ -2531,7 +2499,9 @@ self.date_visit1) visit_id = origin_visit1['visit'] - self.storage.snapshot_add(origin_id, visit_id, self.complete_snapshot) + self.storage.snapshot_add([self.complete_snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, snapshot=self.complete_snapshot['id']) snp_id = self.complete_snapshot['id'] branches = self.complete_snapshot['branches'] @@ -2572,7 +2542,9 @@ self.date_visit1) visit_id = origin_visit1['visit'] - self.storage.snapshot_add(origin_id, visit_id, self.complete_snapshot) + self.storage.snapshot_add([self.complete_snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, snapshot=self.complete_snapshot['id']) snp_id = self.complete_snapshot['id'] branches = self.complete_snapshot['branches'] @@ -2649,7 +2621,9 @@ self.date_visit1) visit_id = origin_visit1['visit'] - self.storage.snapshot_add(origin_id, visit_id, self.snapshot) + self.storage.snapshot_add([self.snapshot]) + self.storage.origin_visit_update( + origin_id, visit_id, snapshot=self.snapshot['id']) by_id = self.storage.snapshot_get(self.snapshot['id']) self.assertEqual(by_id, self.snapshot) @@ -2676,21 +2650,6 @@ self.assertEqual(list(self.journal_writer.objects), [ ('snapshot', self.snapshot)]) - def test_snapshot_add_nonexistent_visit__legacy_add(self): - origin_id = self.storage.origin_add_one(self.origin) - visit_id = 54164461156 - - self.journal_writer.objects[:] = [] - - with self.assertRaises(ValueError): - self.storage.snapshot_add(origin_id, visit_id, self.snapshot) - - # Note: the actual legacy behavior was to abort before adding - # the snapshot; but delaying non-existence checks makes the - # compatibility code simpler - self.assertEqual(list(self.journal_writer.objects), [ - ('snapshot', self.snapshot)]) - def test_snapshot_add_twice(self): origin_id = self.storage.origin_add_one(self.origin) origin_visit1 = self.storage.origin_visit_add(origin_id, @@ -2761,84 +2720,6 @@ ('origin_visit', data3), ('origin_visit', data4)]) - def test_snapshot_add_twice__legacy_add(self): - origin_id = self.storage.origin_add_one(self.origin) - origin_visit1 = self.storage.origin_visit_add(origin_id, - self.date_visit1) - visit1_id = origin_visit1['visit'] - self.storage.snapshot_add(origin_id, visit1_id, self.snapshot) - - by_ov1 = self.storage.snapshot_get_by_origin_visit(origin_id, - visit1_id) - self.assertEqual(by_ov1, self.snapshot) - - origin_visit2 = self.storage.origin_visit_add(origin_id, - self.date_visit2) - visit2_id = origin_visit2['visit'] - - self.storage.snapshot_add(origin_id, visit2_id, self.snapshot) - - by_ov2 = self.storage.snapshot_get_by_origin_visit(origin_id, - visit2_id) - self.assertEqual(by_ov2, self.snapshot) - - expected_origin = self.origin.copy() - data1 = { - 'origin': expected_origin, - 'date': self.date_visit1, - 'visit': origin_visit1['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': None, - } - data2 = { - 'origin': expected_origin, - 'date': self.date_visit1, - 'visit': origin_visit1['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': self.snapshot['id'], - } - data3 = { - 'origin': expected_origin, - 'date': self.date_visit2, - 'visit': origin_visit2['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': None, - } - data4 = { - 'origin': expected_origin, - 'date': self.date_visit2, - 'visit': origin_visit2['visit'], - 'type': self.origin['type'], - 'status': 'ongoing', - 'metadata': None, - 'snapshot': self.snapshot['id'], - } - self.assertEqual(list(self.journal_writer.objects), - [('origin', expected_origin), - ('origin_visit', data1), - ('snapshot', self.snapshot), - ('origin_visit', data2), - ('origin_visit', data3), - ('origin_visit', data4)]) - - def test_snapshot_get_nonexistent(self): - bogus_snapshot_id = b'bogus snapshot id 00' - bogus_origin_id = 1 - bogus_visit_id = 1 - - by_id = self.storage.snapshot_get(bogus_snapshot_id) - self.assertIsNone(by_id) - - by_ov = self.storage.snapshot_get_by_origin_visit(bogus_origin_id, - bogus_visit_id) - self.assertIsNone(by_ov) - @given(strategies.booleans()) def test_snapshot_get_latest(self, use_url): self.reset_storage() @@ -2984,83 +2865,6 @@ origin_id_or_url) ) - @given(strategies.booleans()) - def test_snapshot_get_latest__legacy_add(self, use_url): - self.reset_storage() - - origin_id = self.storage.origin_add_one(self.origin) - origin_id_or_url = self.origin['url'] if use_url else origin_id - origin_visit1 = self.storage.origin_visit_add( - origin_id_or_url, - self.date_visit1) - visit1_id = origin_visit1['visit'] - origin_visit2 = self.storage.origin_visit_add( - origin_id_or_url, - self.date_visit2) - visit2_id = origin_visit2['visit'] - - # Add a visit with the same date as the previous one - origin_visit3 = self.storage.origin_visit_add( - origin_id_or_url, - self.date_visit2) - visit3_id = origin_visit3['visit'] - - # Two visits, both with no snapshot: latest snapshot is None - self.assertIsNone(self.storage.snapshot_get_latest( - origin_id_or_url)) - - # Add snapshot to visit1, latest snapshot = visit 1 snapshot - self.storage.snapshot_add( - origin_id_or_url, - visit1_id, self.complete_snapshot) - self.assertEqual(self.complete_snapshot, - self.storage.snapshot_get_latest( - origin_id_or_url)) - - # Status filter: both visits are status=ongoing, so no snapshot - # returned - self.assertIsNone( - self.storage.snapshot_get_latest( - origin_id_or_url, - allowed_statuses=['full']) - ) - - # Mark the first visit as completed and check status filter again - self.storage.origin_visit_update( - origin_id_or_url, - visit1_id, status='full') - self.assertEqual( - self.complete_snapshot, - self.storage.snapshot_get_latest( - origin_id_or_url, - allowed_statuses=['full']), - ) - - # Add snapshot to visit2 and check that the new snapshot is returned - self.storage.snapshot_add( - origin_id_or_url, - visit2_id, self.empty_snapshot) - self.assertEqual(self.empty_snapshot, - self.storage.snapshot_get_latest( - origin_id_or_url)) - - # Check that the status filter is still working - self.assertEqual( - self.complete_snapshot, - self.storage.snapshot_get_latest( - origin_id_or_url, - allowed_statuses=['full']), - ) - - # Add snapshot to visit3 (same date as visit2) and check that - # the new snapshot is returned - self.storage.snapshot_add( - origin_id_or_url, - visit3_id, self.complete_snapshot) - self.assertEqual(self.complete_snapshot, - self.storage.snapshot_get_latest( - origin_id_or_url)) - def test_stat_counters(self): expected_keys = ['content', 'directory', 'origin', 'person', 'revision'] @@ -3091,8 +2895,10 @@ self.storage.origin_add_one(self.origin2) origin_visit1 = self.storage.origin_visit_add( self.origin2['url'], date=self.date_visit2) - self.storage.snapshot_add( - self.origin2['url'], origin_visit1['visit'], self.snapshot) + self.storage.snapshot_add([self.snapshot]) + self.storage.origin_visit_update( + self.origin2['url'], origin_visit1['visit'], + snapshot=self.snapshot['id']) self.storage.directory_add([self.dir]) self.storage.revision_add([self.revision]) self.storage.release_add([self.release])