diff --git a/sql/upgrades/140.sql b/sql/upgrades/140.sql new file mode 100644 --- /dev/null +++ b/sql/upgrades/140.sql @@ -0,0 +1,11 @@ +-- SWH DB schema upgrade +-- from_version: 139 +-- to_version: 140 +-- description: Add constraint checking that release.author is null implies that release.date is null. + +insert into dbversion(version, release, description) + values(140, now(), 'Work In Progress'); + +alter table release add constraint release_author_date_check check ((date is null) or (author is not null)) not valid; +alter table release validate constraint release_author_date_check; + 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 @@ -17,9 +17,8 @@ import attr -from swh.model.model import Content, Directory +from swh.model.model import Content, Directory, Revision, Release from swh.model.hashutil import DEFAULT_ALGORITHMS -from swh.model.identifiers import normalize_timestamp from swh.objstorage import get_objstorage from swh.objstorage.exc import ObjNotFoundError @@ -587,17 +586,16 @@ if self.journal_writer: self.journal_writer.write_additions('revision', revisions) + revisions = [Revision.from_dict(rev) for rev in revisions] + count = 0 for revision in revisions: - if revision['id'] not in self._revisions: - self._revisions[revision['id']] = rev = copy.deepcopy(revision) - self._person_add(rev['committer']) - self._person_add(rev['author']) - rev['date'] = normalize_timestamp(rev.get('date')) - rev['committer_date'] = normalize_timestamp( - rev.get('committer_date')) - self._objects[revision['id']].append( - ('revision', revision['id'])) + if revision.id not in self._revisions: + revision.committer = self._person_add(revision.committer) + revision.author = self._person_add(revision.author) + self._revisions[revision.id] = revision + self._objects[revision.id].append( + ('revision', revision.id)) count += 1 return {'revision:add': count} @@ -618,7 +616,10 @@ def revision_get(self, revisions): for id in revisions: - yield copy.deepcopy(self._revisions.get(id)) + if id in self._revisions: + yield self._revisions.get(id).to_dict() + else: + yield None def _get_parent_revs(self, rev_id, seen, limit): if limit and len(seen) >= limit: @@ -626,8 +627,8 @@ if rev_id in seen or rev_id not in self._revisions: return seen.add(rev_id) - yield self._revisions[rev_id] - for parent in self._revisions[rev_id]['parents']: + yield self._revisions[rev_id].to_dict() + for parent in self._revisions[rev_id].parents: yield from self._get_parent_revs(parent, seen, limit) def revision_log(self, revisions, limit=None): @@ -688,16 +689,16 @@ if self.journal_writer: self.journal_writer.write_additions('release', releases) + releases = [Release.from_dict(rel) for rel in releases] + count = 0 for rel in releases: - if rel['id'] not in self._releases: - rel = copy.deepcopy(rel) - rel['date'] = normalize_timestamp(rel['date']) - if rel['author']: - self._person_add(rel['author']) - self._objects[rel['id']].append( - ('release', rel['id'])) - self._releases[rel['id']] = rel + if rel.id not in self._releases: + if rel.author: + self._person_add(rel.author) + self._objects[rel.id].append( + ('release', rel.id)) + self._releases[rel.id] = rel count += 1 return {'release:add': count} @@ -726,7 +727,10 @@ """ for rel_id in releases: - yield copy.deepcopy(self._releases.get(rel_id)) + if rel_id in self._releases: + yield self._releases[rel_id].to_dict() + else: + yield None def snapshot_add(self, snapshots): """Add a snapshot to the storage @@ -1686,15 +1690,15 @@ person: dictionary with keys fullname, name and email. """ - key = ('person', person['fullname']) + key = ('person', person.fullname) if key not in self._objects: person_id = len(self._persons) + 1 - self._persons.append(dict(person)) + self._persons.append(person) self._objects[key].append(('person', person_id)) else: person_id = self._objects[key][0][1] - p = self._persons[person_id-1] - person.update(p.items()) + person = self._persons[person_id-1] + return person @staticmethod def _content_key(content): diff --git a/swh/storage/sql/30-swh-schema.sql b/swh/storage/sql/30-swh-schema.sql --- a/swh/storage/sql/30-swh-schema.sql +++ b/swh/storage/sql/30-swh-schema.sql @@ -17,7 +17,7 @@ -- latest schema version insert into dbversion(version, release, description) - values(139, now(), 'Work In Progress'); + values(140, now(), 'Work In Progress'); -- a SHA1 checksum create domain sha1 as bytea check (length(value) = 20); diff --git a/swh/storage/sql/60-swh-indexes.sql b/swh/storage/sql/60-swh-indexes.sql --- a/swh/storage/sql/60-swh-indexes.sql +++ b/swh/storage/sql/60-swh-indexes.sql @@ -146,6 +146,10 @@ alter table release add constraint release_author_fkey foreign key (author) references person(id) not valid; alter table release validate constraint release_author_fkey; +-- if the author is null, then the date must be null +alter table release add constraint release_author_date_check check ((date is null) or (author is not null)) not valid; +alter table release validate constraint release_author_date_check; + -- tool create unique index tool_pkey on tool(id); alter table tool add primary key using index tool_pkey; 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 @@ -1173,6 +1173,31 @@ actual_result = self.storage.revision_add([self.revision]) self.assertEqual(actual_result, {'revision:add': 0}) + def test_revision_add_validation(self): + rev = copy.deepcopy(self.revision) + rev['date']['offset'] = 2**16 + + with self.assertRaisesRegex( + (ValueError, psycopg2.errors.NumericValueOutOfRange), + 'offset'): + self.storage.revision_add([rev]) + + rev = copy.deepcopy(self.revision) + rev['committer_date']['offset'] = 2**16 + + with self.assertRaisesRegex( + (ValueError, psycopg2.errors.NumericValueOutOfRange), + 'offset'): + self.storage.revision_add([rev]) + + rev = copy.deepcopy(self.revision) + rev['type'] = 'foobar' + + with self.assertRaisesRegex( + (ValueError, psycopg2.errors.InvalidTextRepresentation), + '(?i)type'): + self.storage.revision_add([rev]) + def test_revision_add_name_clash(self): revision1 = self.revision.copy() revision2 = self.revision2.copy() @@ -1330,6 +1355,23 @@ self.assertEqual(list(self.journal_writer.objects), [('release', release)]) + def test_release_add_validation(self): + rel = copy.deepcopy(self.release) + rel['date']['offset'] = 2**16 + + with self.assertRaisesRegex( + (ValueError, psycopg2.errors.NumericValueOutOfRange), + 'offset'): + self.storage.release_add([rel]) + + rel = copy.deepcopy(self.release) + rel['author'] = None + + with self.assertRaisesRegex( + (ValueError, psycopg2.errors.CheckViolation), + 'date'): + self.storage.release_add([rel]) + def test_release_add_name_clash(self): release1 = self.release.copy() release2 = self.release2.copy()