Skip to content

Commit

Permalink
Make sure errors and warning columns save with SQL NULL
Browse files Browse the repository at this point in the history
#306

This commit just makes sure no new bad data is written.
  • Loading branch information
odscjames committed Sep 28, 2020
1 parent 15334e4 commit ae1c8c0
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 4 deletions.
8 changes: 4 additions & 4 deletions ocdskingfisherprocess/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ def __init__(self, config):
nullable=False),
sa.Column('filename', sa.Text, nullable=False),
sa.Column('url', sa.Text, nullable=False),
sa.Column('warnings', JSONB, nullable=True),
sa.Column('errors', JSONB, nullable=True),
sa.Column('warnings', JSONB(none_as_null=True), nullable=True),
sa.Column('errors', JSONB(none_as_null=True), nullable=True),
sa.UniqueConstraint('collection_id', 'filename',
name='unique_collection_file_identifiers'),
sa.Index('collection_file_collection_id_idx', 'collection_id'),
Expand All @@ -92,8 +92,8 @@ def __init__(self, config):
nullable=False
),
sa.Column('number', sa.Integer, nullable=False),
sa.Column('warnings', JSONB, nullable=True),
sa.Column('errors', JSONB, nullable=True),
sa.Column('warnings', JSONB(none_as_null=True), nullable=True),
sa.Column('errors', JSONB(none_as_null=True), nullable=True),
sa.UniqueConstraint('collection_file_id', 'number',
name='unique_collection_file_item_identifiers'),
sa.Index('collection_file_item_collection_file_id_idx',
Expand Down
91 changes: 91 additions & 0 deletions tests/test_web_api_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,72 @@ def test_api_v1_submit_file(self):
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 1
assert file_items[0].errors == None # noqa
assert file_items[0].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0

# Test warnings and errors are empty and actually NULL's in database, for easier finding later
# https://github.com/open-contracting/kingfisher-process/issues/306
with self.database.get_engine().begin() as connection:

collection_file_results = connection.execute(
'SELECT (warnings is null) as w, (errors is null) as e from collection_file'
)
for collection_file_result in collection_file_results:
assert collection_file_result[0] == True # noqa
assert collection_file_result[1] == True # noqa

collection_file_item_results = connection.execute(
'SELECT (warnings is null) as w, (errors is null) as e from collection_file_item'
)
for collection_file_item_result in collection_file_item_results:
assert collection_file_item_result[0] == True # noqa
assert collection_file_item_result[1] == True # noqa

def test_api_v1_submit_file_with_control_code(self):
# Call
data = {
'collection_source': 'test',
'collection_data_version': '2018-10-10 00:12:23',
'collection_sample': 'true',
'file_name': 'test.json',
'url': 'http://example.com',
'data_type': 'record',
'file': (io.BytesIO(b' {"control_code": "\\u0000"}'), "data.json")
}

result = self.flaskclient.post('/api/v1/submit/file/',
data=data,
content_type='multipart/form-data',
headers={'Authorization': 'ApiKey ' + self.config.web_api_keys[0]})

assert result.status_code == 200

# Check
collection_id = self.database.get_collection_id('test', '2018-10-10 00:12:23', True)
assert collection_id

collection = self.database.get_collection(collection_id)
assert collection.store_start_at != None # noqa
assert collection.store_end_at == None # noqa

files = self.database.get_all_files_in_collection(collection_id)
assert len(files) == 1
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == ['We had to replace control codes: \\u0000'] # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 1
assert file_items[0].errors == None # noqa
assert file_items[0].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -120,6 +186,10 @@ def test_api_v1_submit_file_that_is_not_even_json(self):
assert files[0].url == 'http://example.com'
assert len(files[0].errors) == 1
assert files[0].errors[0] == "JSONDecodeError('Expecting value: line 1 column 1 (char 0)',)"
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 0

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -160,6 +230,12 @@ def test_api_v1_submit_local_file(self):
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 1
assert file_items[0].errors == None # noqa
assert file_items[0].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -208,6 +284,12 @@ def test_api_v1_submit_item(self):
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 1
assert file_items[0].errors == None # noqa
assert file_items[0].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -287,6 +369,10 @@ def test_api_v1_submit_file_errors(self):
assert files[0].url == 'http://example.com'
assert len(files[0].errors) == 1
assert files[0].errors[0] == 'The error was ... because reasons.'
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 0

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -334,10 +420,12 @@ def test_api_v1_submit_item_no_release_key(self):
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 1
assert file_items[0].errors == ['Release list not found']
assert file_items[0].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0
Expand Down Expand Up @@ -398,11 +486,14 @@ def test_api_v1_submit_item_no_release_key_succesfull_call_first(self, ):
assert files[0].filename == 'test.json'
assert files[0].url == 'http://example.com'
assert files[0].errors == None # noqa
assert files[0].warnings == None # noqa

file_items = self.database.get_all_files_items_in_file(files[0])
assert len(file_items) == 2
assert file_items[0].errors == None # noqa
assert file_items[0].warnings == None # noqa
assert file_items[1].errors == ['Release list not found']
assert file_items[1].warnings == None # noqa

notes = self.database.get_all_notes_in_collection(collection_id)
assert len(notes) == 0

0 comments on commit ae1c8c0

Please sign in to comment.