From 7709fc33bee132759527a749fd202a3cba5dd39d Mon Sep 17 00:00:00 2001 From: Scott Barnes Date: Wed, 14 Aug 2024 21:54:57 -0700 Subject: [PATCH] Add more tests for import validator --- .../plugins/importapi/import_validator.py | 14 +-- .../importapi/tests/test_import_validator.py | 88 +++++++++++++------ 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/openlibrary/plugins/importapi/import_validator.py b/openlibrary/plugins/importapi/import_validator.py index 48f93eea8a86..b704c7486a1a 100644 --- a/openlibrary/plugins/importapi/import_validator.py +++ b/openlibrary/plugins/importapi/import_validator.py @@ -15,21 +15,21 @@ class Author(BaseModel): name: NonEmptyStr -class CompleteBookPlus(BaseModel): +class CompleteBook(BaseModel): """ - The model for a complete book, plus source_records and publishers. + The model for a complete book, plus source_records. - A complete book has title, authors, and publish_date. See #9440. + A complete book has title, authors, and publish_date, as well as + source_records. See #9440. """ title: NonEmptyStr source_records: NonEmptyList[NonEmptyStr] authors: NonEmptyList[Author] - publishers: NonEmptyList[NonEmptyStr] publish_date: NonEmptyStr -class StrongIdentifierBookPlus(BaseModel): +class StrongIdentifierBook(BaseModel): """ The model for a book with a title, strong identifier, plus source_records. @@ -68,13 +68,13 @@ def validate(self, data: dict[str, Any]) -> bool: errors = [] try: - CompleteBookPlus.model_validate(data) + CompleteBook.model_validate(data) return True except ValidationError as e: errors.append(e) try: - StrongIdentifierBookPlus.model_validate(data) + StrongIdentifierBook.model_validate(data) return True except ValidationError as e: errors.append(e) diff --git a/openlibrary/plugins/importapi/tests/test_import_validator.py b/openlibrary/plugins/importapi/tests/test_import_validator.py index 97b60a2ad94a..0a19a318be86 100644 --- a/openlibrary/plugins/importapi/tests/test_import_validator.py +++ b/openlibrary/plugins/importapi/tests/test_import_validator.py @@ -2,24 +2,18 @@ from pydantic import ValidationError -from openlibrary.plugins.importapi.import_validator import import_validator, Author +from openlibrary.plugins.importapi.import_validator import import_validator -def test_create_an_author_with_no_name(): - Author(name="Valid Name") - with pytest.raises(ValidationError): - Author(name="") - - -valid_values = { +# The required fields for a import with a complete record. +complete_values = { "title": "Beowulf", "source_records": ["key:value"], - "author": {"name": "Tom Robbins"}, "authors": [{"name": "Tom Robbins"}, {"name": "Dean Koontz"}], - "publishers": ["Harper Collins", "OpenStax"], "publish_date": "December 2018", } +# The required fields for an import with a title and strong identifier. valid_values_strong_identifier = { "title": "Beowulf", "source_records": ["key:value"], @@ -29,44 +23,58 @@ def test_create_an_author_with_no_name(): validator = import_validator() -def test_validate(): - assert validator.validate(valid_values) is True +def test_validate_complete_record(): + """A complete records should validate.""" + assert validator.validate(complete_values) is True -def test_validate_strong_identifier_minimal(): - """The least amount of data for a strong identifier record to validate.""" +def test_validate_strong_identifier(): + """A record with a title + strong identifier should validate.""" assert validator.validate(valid_values_strong_identifier) is True +def test_validate_both_complete_and_strong(): + """ + A record that is both complete and that has a strong identifier should + validate. + """ + valid_record = complete_values.copy() | valid_values_strong_identifier.copy() + assert validator.validate(valid_record) is True + + @pytest.mark.parametrize( - 'field', ["title", "source_records", "authors", "publishers", "publish_date"] + 'field', ["title", "source_records", "authors", "publish_date"] ) def test_validate_record_with_missing_required_fields(field): - invalid_values = valid_values.copy() + """Ensure a record will not validate as complete without each required field.""" + invalid_values = complete_values.copy() del invalid_values[field] with pytest.raises(ValidationError): validator.validate(invalid_values) @pytest.mark.parametrize('field', ['title', 'publish_date']) -def test_validate_empty_string(field): - invalid_values = valid_values.copy() +def test_cannot_validate_with_empty_string_values(field): + """Ensure the title and publish_date are not mere empty strings.""" + invalid_values = complete_values.copy() invalid_values[field] = "" with pytest.raises(ValidationError): validator.validate(invalid_values) -@pytest.mark.parametrize('field', ['source_records', 'authors', 'publishers']) -def test_validate_empty_list(field): - invalid_values = valid_values.copy() +@pytest.mark.parametrize('field', ['source_records', 'authors']) +def test_cannot_validate_with_with_empty_lists(field): + """Ensure list values will not validate if they are empty.""" + invalid_values = complete_values.copy() invalid_values[field] = [] with pytest.raises(ValidationError): validator.validate(invalid_values) -@pytest.mark.parametrize('field', ['source_records', 'publishers']) -def test_validate_list_with_an_empty_string(field): - invalid_values = valid_values.copy() +@pytest.mark.parametrize('field', ['source_records']) +def test_cannot_validate_list_with_an_empty_string(field): + """Ensure lists will not validate with empty string values.""" + invalid_values = complete_values.copy() invalid_values[field] = [""] with pytest.raises(ValidationError): validator.validate(invalid_values) @@ -74,7 +82,7 @@ def test_validate_list_with_an_empty_string(field): @pytest.mark.parametrize('field', ['isbn_10', 'lccn']) def test_validate_multiple_strong_identifiers(field): - """More than one strong identifier should still validate.""" + """Records with more than one strong identifier should still validate.""" multiple_valid_values = valid_values_strong_identifier.copy() multiple_valid_values[field] = ["non-empty"] assert validator.validate(multiple_valid_values) is True @@ -82,8 +90,36 @@ def test_validate_multiple_strong_identifiers(field): @pytest.mark.parametrize('field', ['isbn_13']) def test_validate_not_complete_no_strong_identifier(field): - """An incomplete record without a strong identifier won't validate.""" + """ + Ensure a record cannot validate if it lacks both (1) complete and (2) a title + and strong identifier, in addition to a source_records field. + """ invalid_values = valid_values_strong_identifier.copy() invalid_values[field] = [""] with pytest.raises(ValidationError): validator.validate(invalid_values) + + +def test_can_import_a_valid_author() -> None: + """ + Valid authors, e.g. [{"name": "Hilary Putnam"}, {"name": "Willard V. O. Quine"}], + will validate. + """ + record_with_valid_author = complete_values.copy() + assert validator.validate(record_with_valid_author) is True + + +@pytest.mark.parametrize( + "authors", + [ + ([{"not_name": "Hilary Putnam"}]), # No `name` key. + ({"name": "Hilary Putnam"}), # Not a list + ([{"name": 1}]), # `name` value isn't a string. + ], +) +def test_cannot_import_an_invalid_author(authors) -> None: + """Authors of the shape [{"name": "Hilary Putnam"}] will validate.""" + record_with_invalid_author = complete_values.copy() + record_with_invalid_author["authors"] = authors + with pytest.raises(ValidationError): + validator.validate(record_with_invalid_author)