From 133e9dabb7c70cdd9186adf0d8c8e5e633eaf025 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 11:17:10 +0100 Subject: [PATCH 01/24] Add test for update_book_by_id happy path --- tests/test_mongo_helper.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index f546117..40472d2 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -5,7 +5,7 @@ from bson import ObjectId from app.datastore.mongo_helper import (delete_book_by_id, find_books, - insert_book_to_mongo) + insert_book_to_mongo, update_book_by_id) def test_insert_book_to_mongo_calls_insert_one(): @@ -131,3 +131,33 @@ def test_soft_delete_already_deleted_book_returns_none(): mock_collection.find_one_and_update.assert_called_with( expected_filter, expected_update ) + + +def test_update_book_by_id_happy_path(): + """Given a valid book_id, update_book_by_id returns a matched_count of 1.""" + # Arrange + valid_id_str = str(ObjectId()) + new_book_data = { + "title": "A Mocked Book: After", + "author": "The Mockist: After", + "synopsis": "A tale of fakes and stubs.", + } + + # Create mock to represent Pymongo result inc. the matched_count attribute + mock_update_result = MagicMock() + mock_update_result.matched_count = 1 + + # Create mock collection + mock_collection = MagicMock() + mock_collection.update_one.return_value = mock_update_result + + # Act + result = update_book_by_id(mock_collection, valid_id_str, new_book_data) + + # Assert + assert result == 1 + mock_collection.update_one.assert_called_once() + mock_collection.update_one.assert_called_once_with( + {'_id': ObjectId(valid_id_str)}, + {'$set': new_book_data} + ) From a1b8e39c2d16869348296a9a47d18239d6639e2a Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 12:06:32 +0100 Subject: [PATCH 02/24] Add test for testing invalid_id input --- tests/test_mongo_helper.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index 40472d2..edc1b9e 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -161,3 +161,24 @@ def test_update_book_by_id_happy_path(): {'_id': ObjectId(valid_id_str)}, {'$set': new_book_data} ) + +def test_update_book_by_id_invalid_id_returns_none(): + """ + Given an invalidly formatted book_id string, + update_book_by_id should return None. + """ + # Arrange + invalid_id = "1234-this-is-not-a-valid-id" + new_book_data = { + "title": "A Mocked Book: After", + "author": "The Mockist: After", + "synopsis": "A tale of fakes and stubs.", + } + mock_collection = MagicMock() + + # Act + result = update_book_by_id(mock_collection, invalid_id, new_book_data) + + # Assert + assert result is None + mock_collection.update_one.assert_not_called() From 85143fe5a8e27e4a991694b5725c474afe7c6da6 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 12:09:41 +0100 Subject: [PATCH 03/24] Add update_book_by_id for full MongoDB doc updates --- app/datastore/mongo_helper.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 3fc57ec..2acdd80 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -1,6 +1,6 @@ """Module containing pymongo helper functions.""" -from bson.objectid import ObjectId +from bson.objectid import ObjectId, InvalidId from pymongo.cursor import Cursor @@ -65,7 +65,7 @@ def find_books(collection, query_filter=None, projection=None, limit=None) -> Cu return cursor -def delete_book_by_id(book_collection, book_id): +def delete_book_by_id(book_collection: dict, book_id: str): """ Soft delete book with given id Returns: The original document if found and updated, otherwise None. @@ -81,3 +81,23 @@ def delete_book_by_id(book_collection, book_id): result = book_collection.find_one_and_update(filter_query, update_operation) return result + + +def update_book_by_id(book_collection, book_id, data): + + """Updates an ENTIRE book document in the database""" + try: + # convert string ID to ObjectId + object_id_to_update = ObjectId(book_id) + + except InvalidId: + return None + + # use $set operator to update the fields OR + # create them if they don't exist + result = book_collection.update_one( + {'_id': object_id_to_update}, + {'$set': data} + ) + + return result.matched_count From 6e47b36c16f7fa6277803820b50f03dc0d7d9954 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 14:15:29 +0100 Subject: [PATCH 04/24] Update failing tests to reflect mongodb integration --- tests/test_api_security.py | 77 +++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/tests/test_api_security.py b/tests/test_api_security.py index cceeca1..0ce24ac 100644 --- a/tests/test_api_security.py +++ b/tests/test_api_security.py @@ -125,42 +125,57 @@ def test_add_book_fails_if_api_key_not_configured_on_the_server(client, test_app # -------------- PUT -------------------------- def test_update_book_succeeds_with_valid_api_key(monkeypatch, client): """Test successful book update with valid API key.""" + # Arrange + test_book_id = "65a9a4b3f3a2c4a8c2b3d4e5" - # 1. Patch the books list - test_books = [ - { - "id": "abc123", - "title": "Old Title", - "synopsis": "Old Synopsis", - "author": "Old Author", - } - ] - monkeypatch.setattr("app.routes.books", test_books) - - # 2. Patch append_hostname to just return the book - monkeypatch.setattr("app.routes.append_hostname", lambda book, host: book) + mock_collection = MagicMock() + mock_collection.replace_one.return_value.matched_count = 1 - # 3. Call the endpoint with request details - response = client.put("/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"]) + expected_book_from_db = { + "_id": ObjectId(test_book_id), + # Use dictionary unpacking to merge our payload + **DUMMY_PAYLOAD + } + mock_collection.find_one.return_value = expected_book_from_db + + # monkeypatcj to replace the real get_book_collection with a function + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + + # ACT + response = client.put( + f"/books/{test_book_id}", + json=DUMMY_PAYLOAD, + headers=HEADERS["VALID"] + ) - # 4. Assert response + # ASSERT 1 assert response.status_code == 200 - assert response.json["title"] == "A Test Book" + response_data = response.json + assert response_data["id"] == test_book_id + assert response_data["title"] == DUMMY_PAYLOAD["title"] + assert "links" in response_data + assert response_data["links"]["self"].endswith(f"/books/{test_book_id}") + + # Assert 2 + mock_collection.replace_one.assert_called_once() + mock_collection.replace_one.assert_called_with( + {'_id': ObjectId(test_book_id)}, + DUMMY_PAYLOAD + ) + mock_collection.find_one.assert_called_once() + mock_collection.find_one.assert_called_with({'_id': ObjectId(test_book_id)}) -def test_update_book_fails_with_missing_api_key(monkeypatch, client): +def test_update_book_fails_with_missing_api_key(client): """Should return 401 if no API key is provided.""" - monkeypatch.setattr("app.routes.books", []) - response = client.put("/books/abc123", json=DUMMY_PAYLOAD) assert response.status_code == 401 assert "API key is missing." in response.json["error"]["message"] -def test_update_book_fails_with_invalid_api_key(client, monkeypatch): - monkeypatch.setattr("app.routes.books", []) +def test_update_book_fails_with_invalid_api_key(client): response = client.put( "/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["INVALID"] @@ -170,17 +185,17 @@ def test_update_book_fails_with_invalid_api_key(client, monkeypatch): assert "Invalid API key." in response.json["error"]["message"] -def test_update_book_fails_if_api_key_not_configured_on_the_server( - client, test_app, monkeypatch -): - # ARRANGE: Remove API_KEY from the test_app config - monkeypatch.setattr("app.routes.books", []) - test_app.config.pop("API_KEY", None) +# def test_update_book_fails_if_api_key_not_configured_on_the_server( +# client, test_app, monkeypatch +# ): +# # ARRANGE: Remove API_KEY from the test_app config +# monkeypatch.setattr("app.routes.books", []) +# test_app.config.pop("API_KEY", None) - response = client.put("/books/abc123", json=DUMMY_PAYLOAD) +# response = client.put("/books/abc123", json=DUMMY_PAYLOAD) - assert response.status_code == 500 - assert "API key not configured on the server." in response.json["error"]["message"] +# assert response.status_code == 500 +# assert "API key not configured on the server." in response.json["error"]["message"] # -------------- DELETE -------------------------- From a9d21cfa9b747eceea55981a2230f3d919dbd06c Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 14:19:52 +0100 Subject: [PATCH 05/24] Switch to replace_one for full document replacment --- app/datastore/mongo_helper.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 2acdd80..082fd67 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -83,21 +83,23 @@ def delete_book_by_id(book_collection: dict, book_id: str): return result -def update_book_by_id(book_collection, book_id, data): +def replace_book_by_id(book_collection, book_id, new_data): - """Updates an ENTIRE book document in the database""" + """ + Updates an ENTIRE book document in the database. + Returns True on success, False if book not found. + """ try: - # convert string ID to ObjectId object_id_to_update = ObjectId(book_id) except InvalidId: - return None + return False # use $set operator to update the fields OR # create them if they don't exist - result = book_collection.update_one( + result = book_collection.replace_one( {'_id': object_id_to_update}, - {'$set': data} + new_data ) - return result.matched_count + return result.matched_count > 0 From d6b72adfbedbe904db02b59548aaf4e5f4b6a48d Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 14:36:18 +0100 Subject: [PATCH 06/24] Use replace_one in tests; clarify variable names --- app/datastore/mongo_helper.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 082fd67..8a77f6e 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -83,6 +83,40 @@ def delete_book_by_id(book_collection: dict, book_id: str): return result +def validate_book_put_payload(payload: dict): + """ + Validates the payload for a PUT request. + A PUT must contain all required fields and no extra fields + + Returns: + A tuple: (is_valid, error_dictionary) + If valid, error_dictionary is None. + """ + if not isinstance(payload, dict): + return False, {"error": "JSON payload must be a dictionary"} + + required_fields = {"title", "synopsis", "author"} + payload_keys = set(payload.keys()) + print(payload_keys) + + # Check 1: any missing fields? + missing_fields = required_fields - payload_keys + if missing_fields: + return False, { + "error": f"Missing required fields: {', '.join(sorted(list(missing_fields)))}" + } + + # Check 2: Any extra, unexpected fields? + extra_fields = payload_keys - required_fields + if extra_fields: + return False, { + "error": f"Unexpected fields provided: {', '.join(sorted(list(extra_fields)))}" + } + + # If all checks pass: + return True, None + + def replace_book_by_id(book_collection, book_id, new_data): """ From 05d7f6e36d6d4c75bdb0cbecbc0f8265a33ea2be Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 14:56:40 +0100 Subject: [PATCH 07/24] Update test with updated mocks/dummy data --- tests/test_app.py | 95 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 30 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index ae78dc7..16f87e2 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -49,6 +49,20 @@ }, ] +# A dictionary for headers to keep things clean +HEADERS = { + "VALID": {"X-API-KEY": "test-key-123"}, + "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, + "MISSING": {}, +} + +# A sample payload for POST/PUT requests +DUMMY_PAYLOAD = { + "title": "A Test Book", + "synopsis": "A test synopsis.", + "author": "Tester McTestFace", +} + # ------------------- Tests for POST --------------------------------------------- @@ -601,44 +615,65 @@ def test_returns_404_if_helper_function_result_is_none(client): # ------------------------ Tests for PUT -------------------------------------------- -def test_update_book_request_returns_correct_status_and_content_type(client): - with patch("app.routes.books", books_database): +# THIS MAY ALREADY BE COVERED BY THE INTEGRATION TEST- DOUBLE check +# def test_update_book_request_returns_correct_status_and_content_type(client): +# with patch("app.routes.books", books_database): - test_book = { - "title": "Test Book", - "author": "AN Other", - "synopsis": "Test Synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} +# test_book = { +# "title": "Test Book", +# "author": "AN Other", +# "synopsis": "Test Synopsis", +# } +# # Define the valid headers, including the API key that matches conftest.py +# valid_headers = {"X-API-KEY": "test-key-123"} - # send PUT request - response = client.put("/books/1", json=test_book, headers=valid_headers) +# # send PUT request +# response = client.put("/books/1", json=test_book, headers=valid_headers) - # Check response status code and content type - assert response.status_code == 200 - assert response.content_type == "application/json" +# # Check response status code and content type +# assert response.status_code == 200 +# assert response.content_type == "application/json" -def test_update_book_request_returns_required_fields(client): - with patch("app.routes.books", books_database): - test_book = { - "title": "Test Book", - "author": "AN Other", - "synopsis": "Test Synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} +def test_update_book_response_contains_all_required_fields(monkeypatch, client): + """ + GIVEN a successful PUT request + WHEN the response is received + THEN it should be a 200 OK and the JSON body must contain all required fields. + """ + test_book_id = str(ObjectId()) - # Send PUT request - response = client.put("/books/1", json=test_book, headers=valid_headers) - response_data = response.get_json() + book_doc_from_db = { + "_id": ObjectId(test_book_id), + **DUMMY_PAYLOAD + } + + # Create and configure our mock collection + mock_collection = MagicMock() + mock_collection.replace_one.return_value.matched_count = 1 + mock_collection.find_one.return_value = book_doc_from_db - # Check that required fields are in the response data - required_fields = ["title", "synopsis", "author", "links"] - for field in required_fields: - assert field in response_data, f"{field} not in response_data" + # Patch the function that provides the database collection + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + + # ACT + # Send the PUT request to the endpoint + response = client.put( + f"/books/{test_book_id}", + json=DUMMY_PAYLOAD, + headers=HEADERS["VALID"] + ) + + # Assert + assert response.status_code == 200 + response_data = response.get_json() + # Check that ALL required fields are in the response data + required_fields = ["id", "title", "synopsis", "author", "links"] + for field in required_fields: + assert field in response_data, f"Required field '{field}' is missing from the response" + assert response_data["id"] == test_book_id + assert isinstance(response_data["links"], dict) def test_update_book_replaces_whole_object(client): book_to_be_changed = { From b88ae096241aab28548318e7685400f1dfcec322 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 15:47:49 +0100 Subject: [PATCH 08/24] Update tests to reflect mongodb integration --- tests/test_app.py | 215 ++++++++++++++++++++++++---------------------- 1 file changed, 113 insertions(+), 102 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index 16f87e2..f0360b7 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -675,110 +675,120 @@ def test_update_book_response_contains_all_required_fields(monkeypatch, client): assert response_data["id"] == test_book_id assert isinstance(response_data["links"], dict) -def test_update_book_replaces_whole_object(client): - book_to_be_changed = { - "id": "1", - "title": "Original Title", - "author": "Original Author", - "synopsis": "Original Synopsis", - "links": { - "self": "link to be changed", - "reservations": "link to be changed", - "reviews": "link to be changed", - }, +def test_update_book_replaces_whole_object(monkeypatch, client): + + test_book_id = str(ObjectId()) + updated_payload = { + "title": "Updated Title", + "author": "Updated Author", + "synopsis": "Updated Synopsis", + } + # This is what the document should look like in the database *after* + # the `replace_one` call. + book_doc_after_put = { + "_id": ObjectId(test_book_id), + **updated_payload } - # Patch the books list with just this book (no links) - with patch("app.routes.books", [book_to_be_changed]): - updated_data = { - "title": "Updated Title", - "author": "Updated Author", - "synopsis": "Updated Synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} - response = client.put("/books/1", json=updated_data, headers=valid_headers) - assert response.status_code == 200 - data = response.get_json() - assert "links" in data - assert "/books/1" in data["links"]["self"] - assert "/books/1/reservations" in data["links"]["reservations"] - assert "/books/1/reviews" in data["links"]["reviews"] + # Set up our mock database collection. + mock_collection = MagicMock() + mock_collection.replace_one.return_value.matched_count = 1 + # Simulate fetching the new document after it has been replaced. + mock_collection.find_one.return_value = book_doc_after_put - # Verify other fields were updated - assert data["title"] == "Updated Title" - assert data["author"] == "Updated Author" - assert data["synopsis"] == "Updated Synopsis" + # Inject our mock into the application. + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + # ACT + response = client.put( + f"/books/{test_book_id}", + json=updated_payload, + headers=HEADERS["VALID"] + ) -def test_update_book_sent_with_invalid_book_id(client): - with patch("app.routes.books", books_database): - test_book = { - "title": "Some title", - "author": "Some author", - "synopsis": "Some synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} - response = client.put("/books/999", json=test_book, headers=valid_headers) - assert response.status_code == 404 - assert "Book not found" in response.get_json()["error"] + # Assert + assert response.status_code == 200 + response_data = response.get_json() + assert response_data["title"] == "Updated Title" + assert response_data["author"] == "Updated Author" + assert response_data["synopsis"] == "Updated Synopsis" + # Verify the server-side formatting is correct. + assert response_data["id"] == test_book_id + assert "links" in response_data + assert response_data["links"]["self"].endswith(f"/books/{test_book_id}") + assert response_data["links"]["reservations"].endswith(f"/books/{test_book_id}/reservations") + assert response_data["links"]["reviews"].endswith(f"/books/{test_book_id}/reviews") -def test_book_database_is_initialized_for_update_book_route(client): - with patch("app.routes.books", None): - test_book = { - "title": "Test Book", - "author": "AN Other", - "synopsis": "Test Synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} - # Send PUT request - response = client.put("/books/1", json=test_book, headers=valid_headers) - assert response.status_code == 500 - assert "Book collection not initialized" in response.get_json()["error"] +def test_update_book_sent_with_invalid_book_id(monkeypatch, client): -def test_update_book_check_request_header_is_json(client): + non_existent_id = str(ObjectId()) + mock_collection = MagicMock() + + # Simulate a failed replacement by setting matched_count to 0. + mock_collection.replace_one.return_value.matched_count = 0 + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) response = client.put( - "/books/1", - data="This is not a JSON object", - headers={"content-type": "text/plain", "X-API-KEY": "test-key-123"}, + f"/books/{non_existent_id}", + json=DUMMY_PAYLOAD, + headers=HEADERS["VALID"] ) - assert response.status_code == 415 - assert "Request must be JSON" in response.get_json()["error"] + assert response.status_code == 404 + # Check for the specific error message. + response_data = response.get_json() + assert "not found" in response_data["error"] + assert non_existent_id in response_data["error"] + mock_collection.find_one.assert_not_called() -def test_update_book_with_invalid_json_content(client): - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} +def test_book_database_is_initialized_for_update_book_route(monkeypatch, client): + monkeypatch.setattr("app.routes.get_book_collection", lambda: None) + response = client.put( + "/books/123", + json=DUMMY_PAYLOAD, + headers=HEADERS["VALID"] + ) + assert response.status_code == 500 + response_data = response.get_json() + assert "Book collection not initialized" in response_data["error"] - # This should trigger a TypeError + +def test_update_book_check_request_header_is_json(client): + """ + GIVEN a request with a non-JSON content-type and body + WHEN a PUT request is made + THEN the API should return a 400 Bad Request error. + """ + valid_id = str(ObjectId()) response = client.put( - "/books/1", json="This is not a JSON object", headers=valid_headers + f"/books/{valid_id}", + json="This is not a JSON object", + headers=HEADERS["VALID"], ) assert response.status_code == 400 assert "JSON payload must be a dictionary" in response.get_json()["error"] + def test_update_book_sent_with_missing_required_fields(client): - test_book = { + incomplete_payload = { "author": "AN Other" # missing 'title' and 'synopsis' } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} + valid_id = str(ObjectId()) - response = client.put("/books/1", json=test_book, headers=valid_headers) + # ACT + response = client.put(f"/books/{valid_id}", json=incomplete_payload, headers=HEADERS["VALID"]) assert response.status_code == 400 response_data = response.get_json() assert "error" in response_data - assert "Missing required fields: title, synopsis" in response.get_json()["error"] + expected_error = "Missing required fields: synopsis, title" + assert response_data["error"] == expected_error # ------------------------ Tests for HELPER FUNCTIONS ------------------------------------- @@ -948,41 +958,42 @@ def test_append_host_to_links_in_get_book(client, monkeypatch): mock_appender.assert_called_once_with(book_from_db, ANY) -def test_append_host_to_links_in_put(client): - - test_book = { - "title": "Test Book", - "author": "AN Other", - "synopsis": "Test Synopsis", - } - # Define the valid headers, including the API key that matches conftest.py - valid_headers = {"X-API-KEY": "test-key-123"} - - response = client.put("/books/1", json=test_book, headers=valid_headers) - - assert response.status_code == 200 - assert response.headers["content-type"] == "application/json" +def test_append_host_to_links_in_put(monkeypatch, client): + """ + GIVEN a successful PUT operation + WHEN the response is being formatted + THEN the `update_book` route must call the `append_hostname` helper. + """ - # Get the response data, the ID and links - response_data = response.get_json() - book_id = response_data.get("id") - links = response_data.get("links") + test_book_id = str(ObjectId()) + test_payload = DUMMY_PAYLOAD - assert book_id is not None, "Response JSON must contain an 'id'" - assert links is not None, "Response JSON must contain a 'links' object" + # a) Set up the mock database flow + book_doc_from_db = {"_id": ObjectId(test_book_id), **test_payload} + mock_collection = MagicMock() + mock_collection.replace_one.return_value.matched_count = 1 + mock_collection.find_one.return_value = book_doc_from_db + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) - self_link = links.get("self") - assert self_link is not None, "'links' object must contain a 'self' link" + with patch("app.routes.append_hostname") as mock_append_hostname: + mock_append_hostname.side_effect = lambda book, host: book - expected_link_start = "http://localhost" - assert self_link.startswith( - expected_link_start - ), f"Link should start with the test server's hostname '{expected_link_start}'" + # --- 2. ACT --- + response = client.put( + f"/books/{test_book_id}", + json=test_payload, + headers=HEADERS["VALID"] + ) - expected_path = f"/books/{book_id}" - assert self_link.endswith( - expected_path - ), f"Link should end with the resource path '{expected_path}'" + assert response.status_code == 200 + mock_append_hostname.assert_called_once() + call_args, call_kwargs = mock_append_hostname.call_args # pylint: disable=unused-variable + book_arg = call_args[0] + host_arg = call_args[1] + + assert "links" in book_arg + assert book_arg["links"]["self"] == f"/books/{test_book_id}" + assert host_arg == "http://localhost" # Flask test client default def test_get_book_collection_handles_connection_failure(): From 6c2006e14379a9147480a0e7417d98259ec2e997 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 15:50:16 +0100 Subject: [PATCH 09/24] Refactor to append_hostname over direct,dynamic injection --- app/routes.py | 84 +++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/app/routes.py b/app/routes.py index d9d24a8..9b3bab6 100644 --- a/app/routes.py +++ b/app/routes.py @@ -1,18 +1,20 @@ """Flask application module for managing a collection of books.""" -import copy - from bson.objectid import InvalidId, ObjectId from flask import jsonify, request from pymongo.errors import ConnectionFailure from werkzeug.exceptions import HTTPException, NotFound from app.datastore.mongo_db import get_book_collection -from app.datastore.mongo_helper import delete_book_by_id, insert_book_to_mongo +from app.datastore.mongo_helper import ( + delete_book_by_id, + insert_book_to_mongo, + replace_book_by_id, + validate_book_put_payload +) from app.services.book_service import fetch_active_books, format_books_for_api from app.utils.api_security import require_api_key from app.utils.helper import append_hostname -from data import books def register_routes(app): # pylint: disable=too-many-statements @@ -200,52 +202,48 @@ def delete_book(book_id): def update_book(book_id): """ Update a book by its unique ID using JSON from the request body. - Returns a single dictionary with the updated book's details. + Finds, replaces, and returns the single updated document. """ - if not books: + # VALIDATION + request_body = request.get_json(silent=True) + if request_body is None: + return jsonify({"error": "Request must be valid JSON"}), 400 + + is_valid, error = validate_book_put_payload(request_body) + if not is_valid: + return jsonify(error), 400 + + # DATABASE INTERACTION + book_collection = get_book_collection() + if not book_collection: return jsonify({"error": "Book collection not initialized"}), 500 - # check if request is json - if not request.is_json: - return jsonify({"error": "Request must be JSON"}), 415 + was_replaced = replace_book_by_id(book_collection, book_id, request_body) + if not was_replaced: + return jsonify({"error": f"Book with ID '{book_id}' not found"}), 404 + + # RESPONSE HANDLING: Format API response + # replace_one doesn't return the document, so we must fetch it to return it. + updated_book = book_collection.find_one({'_id': ObjectId(book_id)}) + # Convert ObjectId to string for the JSON response + updated_book['id'] = str(updated_book['_id']) + + # Add HATEOAS links + host = request.host_url.strip("/") # Use rstrip to avoid double slashes + book_obj_id = updated_book['id'] + updated_book["links"] = { + "self": f"/books/{book_obj_id}", + "reservations": f"/books/{book_obj_id}/reservations", + "reviews": f"/books/{book_obj_id}/reviews", + } - # check request body is valid - request_body = request.get_json() - if not isinstance(request_body, dict): - return jsonify({"error": "JSON payload must be a dictionary"}), 400 + updated_book_with_hostname = append_hostname(updated_book, host) - # check request body contains required fields - required_fields = ["title", "synopsis", "author"] - missing_fields = [ - field for field in required_fields if field not in request_body - ] - if missing_fields: - return { - "error": f"Missing required fields: {', '.join(missing_fields)}" - }, 400 + # Remove internal '_id' field + del updated_book_with_hostname['_id'] - host = request.host_url - - # now that we have a book object that is valid, loop through books - for book in books: - if book.get("id") == book_id: - # update the book values to what is in the request - book["title"] = request.json.get("title") - book["synopsis"] = request.json.get("synopsis") - book["author"] = request.json.get("author") - - # Add links exists as paths only - book["links"] = { - "self": f"/books/{book_id}", - "reservations": f"/books/{book_id}/reservations", - "reviews": f"/books/{book_id}/reviews", - } - # make a deepcopy of the modified book - book_copy = copy.deepcopy(book) - book_with_hostname = append_hostname(book_copy, host) - return jsonify(book_with_hostname), 200 + return jsonify(updated_book_with_hostname), 200 - return jsonify({"error": "Book not found"}), 404 # ----------- CUSTOM ERROR HANDLERS ------------------ From afc138fc80691048519b3bbebaa369ca833a6ea1 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 15:51:46 +0100 Subject: [PATCH 10/24] Sort missing fields so response is predictable --- app/datastore/mongo_helper.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 8a77f6e..7f6d78d 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -97,13 +97,14 @@ def validate_book_put_payload(payload: dict): required_fields = {"title", "synopsis", "author"} payload_keys = set(payload.keys()) - print(payload_keys) # Check 1: any missing fields? missing_fields = required_fields - payload_keys if missing_fields: + # Convert the set to a list and sort it. + sorted_missing = sorted(list(missing_fields)) return False, { - "error": f"Missing required fields: {', '.join(sorted(list(missing_fields)))}" + "error": f"Missing required fields: {', '.join(sorted_missing)}" } # Check 2: Any extra, unexpected fields? From 4e908eec21a8335479644fbdc3c83b1777d276d3 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 15:53:27 +0100 Subject: [PATCH 11/24] Update tests to conform to boolean response --- tests/test_mongo_helper.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index edc1b9e..75e89b1 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -4,8 +4,11 @@ from bson import ObjectId -from app.datastore.mongo_helper import (delete_book_by_id, find_books, - insert_book_to_mongo, update_book_by_id) +from app.datastore.mongo_helper import ( + delete_book_by_id, find_books, + insert_book_to_mongo, + replace_book_by_id +) def test_insert_book_to_mongo_calls_insert_one(): @@ -133,8 +136,8 @@ def test_soft_delete_already_deleted_book_returns_none(): ) -def test_update_book_by_id_happy_path(): - """Given a valid book_id, update_book_by_id returns a matched_count of 1.""" +def test_replace_book_by_id_happy_path(): + """Given a valid book_id, replace_book_by_id returns a matched_count of 1.""" # Arrange valid_id_str = str(ObjectId()) new_book_data = { @@ -144,28 +147,28 @@ def test_update_book_by_id_happy_path(): } # Create mock to represent Pymongo result inc. the matched_count attribute - mock_update_result = MagicMock() - mock_update_result.matched_count = 1 + mock_pymongo_result = MagicMock() + mock_pymongo_result.matched_count = 1 # Create mock collection mock_collection = MagicMock() - mock_collection.update_one.return_value = mock_update_result + mock_collection.replace_one.return_value = mock_pymongo_result # Act - result = update_book_by_id(mock_collection, valid_id_str, new_book_data) + result = replace_book_by_id(mock_collection, valid_id_str, new_book_data) # Assert - assert result == 1 - mock_collection.update_one.assert_called_once() - mock_collection.update_one.assert_called_once_with( + assert result is True + mock_collection.replace_one.assert_called_once() + mock_collection.replace_one.assert_called_once_with( {'_id': ObjectId(valid_id_str)}, - {'$set': new_book_data} + new_book_data ) -def test_update_book_by_id_invalid_id_returns_none(): +def test_replace_book_by_id_invalid_id_returns_false(): """ Given an invalidly formatted book_id string, - update_book_by_id should return None. + update_book_by_id should return False. """ # Arrange invalid_id = "1234-this-is-not-a-valid-id" @@ -177,8 +180,8 @@ def test_update_book_by_id_invalid_id_returns_none(): mock_collection = MagicMock() # Act - result = update_book_by_id(mock_collection, invalid_id, new_book_data) + result = replace_book_by_id(mock_collection, invalid_id, new_book_data) # Assert - assert result is None + assert result is False mock_collection.update_one.assert_not_called() From 8a403793cc4904a2c333c02daf359b4467d4fb07 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 15:54:00 +0100 Subject: [PATCH 12/24] Run formatting --- app/datastore/mongo_helper.py | 12 ++------ app/routes.py | 21 ++++++-------- tests/test_api_security.py | 11 +++----- tests/test_app.py | 52 ++++++++++++++--------------------- tests/test_mongo_helper.py | 12 ++++---- 5 files changed, 42 insertions(+), 66 deletions(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 7f6d78d..542cdd7 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -1,6 +1,6 @@ """Module containing pymongo helper functions.""" -from bson.objectid import ObjectId, InvalidId +from bson.objectid import InvalidId, ObjectId from pymongo.cursor import Cursor @@ -103,9 +103,7 @@ def validate_book_put_payload(payload: dict): if missing_fields: # Convert the set to a list and sort it. sorted_missing = sorted(list(missing_fields)) - return False, { - "error": f"Missing required fields: {', '.join(sorted_missing)}" - } + return False, {"error": f"Missing required fields: {', '.join(sorted_missing)}"} # Check 2: Any extra, unexpected fields? extra_fields = payload_keys - required_fields @@ -119,7 +117,6 @@ def validate_book_put_payload(payload: dict): def replace_book_by_id(book_collection, book_id, new_data): - """ Updates an ENTIRE book document in the database. Returns True on success, False if book not found. @@ -132,9 +129,6 @@ def replace_book_by_id(book_collection, book_id, new_data): # use $set operator to update the fields OR # create them if they don't exist - result = book_collection.replace_one( - {'_id': object_id_to_update}, - new_data - ) + result = book_collection.replace_one({"_id": object_id_to_update}, new_data) return result.matched_count > 0 diff --git a/app/routes.py b/app/routes.py index 9b3bab6..6a80d8c 100644 --- a/app/routes.py +++ b/app/routes.py @@ -6,12 +6,10 @@ from werkzeug.exceptions import HTTPException, NotFound from app.datastore.mongo_db import get_book_collection -from app.datastore.mongo_helper import ( - delete_book_by_id, - insert_book_to_mongo, - replace_book_by_id, - validate_book_put_payload -) +from app.datastore.mongo_helper import (delete_book_by_id, + insert_book_to_mongo, + replace_book_by_id, + validate_book_put_payload) from app.services.book_service import fetch_active_books, format_books_for_api from app.utils.api_security import require_api_key from app.utils.helper import append_hostname @@ -224,13 +222,13 @@ def update_book(book_id): # RESPONSE HANDLING: Format API response # replace_one doesn't return the document, so we must fetch it to return it. - updated_book = book_collection.find_one({'_id': ObjectId(book_id)}) + updated_book = book_collection.find_one({"_id": ObjectId(book_id)}) # Convert ObjectId to string for the JSON response - updated_book['id'] = str(updated_book['_id']) + updated_book["id"] = str(updated_book["_id"]) # Add HATEOAS links - host = request.host_url.strip("/") # Use rstrip to avoid double slashes - book_obj_id = updated_book['id'] + host = request.host_url.strip("/") # Use rstrip to avoid double slashes + book_obj_id = updated_book["id"] updated_book["links"] = { "self": f"/books/{book_obj_id}", "reservations": f"/books/{book_obj_id}/reservations", @@ -240,11 +238,10 @@ def update_book(book_id): updated_book_with_hostname = append_hostname(updated_book, host) # Remove internal '_id' field - del updated_book_with_hostname['_id'] + del updated_book_with_hostname["_id"] return jsonify(updated_book_with_hostname), 200 - # ----------- CUSTOM ERROR HANDLERS ------------------ @app.errorhandler(NotFound) diff --git a/tests/test_api_security.py b/tests/test_api_security.py index 0ce24ac..85005a1 100644 --- a/tests/test_api_security.py +++ b/tests/test_api_security.py @@ -134,7 +134,7 @@ def test_update_book_succeeds_with_valid_api_key(monkeypatch, client): expected_book_from_db = { "_id": ObjectId(test_book_id), # Use dictionary unpacking to merge our payload - **DUMMY_PAYLOAD + **DUMMY_PAYLOAD, } mock_collection.find_one.return_value = expected_book_from_db @@ -143,9 +143,7 @@ def test_update_book_succeeds_with_valid_api_key(monkeypatch, client): # ACT response = client.put( - f"/books/{test_book_id}", - json=DUMMY_PAYLOAD, - headers=HEADERS["VALID"] + f"/books/{test_book_id}", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"] ) # ASSERT 1 @@ -159,11 +157,10 @@ def test_update_book_succeeds_with_valid_api_key(monkeypatch, client): # Assert 2 mock_collection.replace_one.assert_called_once() mock_collection.replace_one.assert_called_with( - {'_id': ObjectId(test_book_id)}, - DUMMY_PAYLOAD + {"_id": ObjectId(test_book_id)}, DUMMY_PAYLOAD ) mock_collection.find_one.assert_called_once() - mock_collection.find_one.assert_called_with({'_id': ObjectId(test_book_id)}) + mock_collection.find_one.assert_called_with({"_id": ObjectId(test_book_id)}) def test_update_book_fails_with_missing_api_key(client): diff --git a/tests/test_app.py b/tests/test_app.py index f0360b7..c06c6be 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -643,10 +643,7 @@ def test_update_book_response_contains_all_required_fields(monkeypatch, client): """ test_book_id = str(ObjectId()) - book_doc_from_db = { - "_id": ObjectId(test_book_id), - **DUMMY_PAYLOAD - } + book_doc_from_db = {"_id": ObjectId(test_book_id), **DUMMY_PAYLOAD} # Create and configure our mock collection mock_collection = MagicMock() @@ -659,9 +656,7 @@ def test_update_book_response_contains_all_required_fields(monkeypatch, client): # ACT # Send the PUT request to the endpoint response = client.put( - f"/books/{test_book_id}", - json=DUMMY_PAYLOAD, - headers=HEADERS["VALID"] + f"/books/{test_book_id}", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"] ) # Assert @@ -670,11 +665,14 @@ def test_update_book_response_contains_all_required_fields(monkeypatch, client): # Check that ALL required fields are in the response data required_fields = ["id", "title", "synopsis", "author", "links"] for field in required_fields: - assert field in response_data, f"Required field '{field}' is missing from the response" + assert ( + field in response_data + ), f"Required field '{field}' is missing from the response" assert response_data["id"] == test_book_id assert isinstance(response_data["links"], dict) + def test_update_book_replaces_whole_object(monkeypatch, client): test_book_id = str(ObjectId()) @@ -685,10 +683,7 @@ def test_update_book_replaces_whole_object(monkeypatch, client): } # This is what the document should look like in the database *after* # the `replace_one` call. - book_doc_after_put = { - "_id": ObjectId(test_book_id), - **updated_payload - } + book_doc_after_put = {"_id": ObjectId(test_book_id), **updated_payload} # Set up our mock database collection. mock_collection = MagicMock() @@ -701,9 +696,7 @@ def test_update_book_replaces_whole_object(monkeypatch, client): # ACT response = client.put( - f"/books/{test_book_id}", - json=updated_payload, - headers=HEADERS["VALID"] + f"/books/{test_book_id}", json=updated_payload, headers=HEADERS["VALID"] ) # Assert @@ -717,7 +710,9 @@ def test_update_book_replaces_whole_object(monkeypatch, client): assert response_data["id"] == test_book_id assert "links" in response_data assert response_data["links"]["self"].endswith(f"/books/{test_book_id}") - assert response_data["links"]["reservations"].endswith(f"/books/{test_book_id}/reservations") + assert response_data["links"]["reservations"].endswith( + f"/books/{test_book_id}/reservations" + ) assert response_data["links"]["reviews"].endswith(f"/books/{test_book_id}/reviews") @@ -731,9 +726,7 @@ def test_update_book_sent_with_invalid_book_id(monkeypatch, client): monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) response = client.put( - f"/books/{non_existent_id}", - json=DUMMY_PAYLOAD, - headers=HEADERS["VALID"] + f"/books/{non_existent_id}", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"] ) assert response.status_code == 404 @@ -746,11 +739,7 @@ def test_update_book_sent_with_invalid_book_id(monkeypatch, client): def test_book_database_is_initialized_for_update_book_route(monkeypatch, client): monkeypatch.setattr("app.routes.get_book_collection", lambda: None) - response = client.put( - "/books/123", - json=DUMMY_PAYLOAD, - headers=HEADERS["VALID"] - ) + response = client.put("/books/123", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"]) assert response.status_code == 500 response_data = response.get_json() assert "Book collection not initialized" in response_data["error"] @@ -773,7 +762,6 @@ def test_update_book_check_request_header_is_json(client): assert "JSON payload must be a dictionary" in response.get_json()["error"] - def test_update_book_sent_with_missing_required_fields(client): incomplete_payload = { "author": "AN Other" @@ -782,7 +770,9 @@ def test_update_book_sent_with_missing_required_fields(client): valid_id = str(ObjectId()) # ACT - response = client.put(f"/books/{valid_id}", json=incomplete_payload, headers=HEADERS["VALID"]) + response = client.put( + f"/books/{valid_id}", json=incomplete_payload, headers=HEADERS["VALID"] + ) assert response.status_code == 400 response_data = response.get_json() @@ -980,20 +970,20 @@ def test_append_host_to_links_in_put(monkeypatch, client): # --- 2. ACT --- response = client.put( - f"/books/{test_book_id}", - json=test_payload, - headers=HEADERS["VALID"] + f"/books/{test_book_id}", json=test_payload, headers=HEADERS["VALID"] ) assert response.status_code == 200 mock_append_hostname.assert_called_once() - call_args, call_kwargs = mock_append_hostname.call_args # pylint: disable=unused-variable + call_args, call_kwargs = ( + mock_append_hostname.call_args + ) # pylint: disable=unused-variable book_arg = call_args[0] host_arg = call_args[1] assert "links" in book_arg assert book_arg["links"]["self"] == f"/books/{test_book_id}" - assert host_arg == "http://localhost" # Flask test client default + assert host_arg == "http://localhost" # Flask test client default def test_get_book_collection_handles_connection_failure(): diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index 75e89b1..25e94a1 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -4,11 +4,9 @@ from bson import ObjectId -from app.datastore.mongo_helper import ( - delete_book_by_id, find_books, - insert_book_to_mongo, - replace_book_by_id -) +from app.datastore.mongo_helper import (delete_book_by_id, find_books, + insert_book_to_mongo, + replace_book_by_id) def test_insert_book_to_mongo_calls_insert_one(): @@ -161,10 +159,10 @@ def test_replace_book_by_id_happy_path(): assert result is True mock_collection.replace_one.assert_called_once() mock_collection.replace_one.assert_called_once_with( - {'_id': ObjectId(valid_id_str)}, - new_book_data + {"_id": ObjectId(valid_id_str)}, new_book_data ) + def test_replace_book_by_id_invalid_id_returns_false(): """ Given an invalidly formatted book_id string, From 13eb88a05dc478a6639c3787cac045d779471381 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 16:18:11 +0100 Subject: [PATCH 13/24] Add tests for malformed JSON request bodies; 100% coverage --- tests/test_app.py | 51 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index c06c6be..463f0fe 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -780,6 +780,53 @@ def test_update_book_sent_with_missing_required_fields(client): expected_error = "Missing required fields: synopsis, title" assert response_data["error"] == expected_error +def test_update_book_fails_with_malformed_json_body(client): + # --- ARRANGE --- + malformed_json_string = '{"title": "A Test Book", }' + + headers_with_bad_body = { + "Content-Type": "application/json", + "X-API-KEY": "test-key-123" + } + # --- ACT --- + # Use the `data` argument to send the raw, broken string. + # If we used `json=`, the test client would fix it for us! + response = client.put( + "/books/some_id", + data=malformed_json_string, + headers=headers_with_bad_body + ) + # --- ASSERT --- + assert response.status_code == 400 + response_data = response.get_json() + assert response_data["error"] == "Request must be valid JSON" + + +def test_update_book_fails_with_wrong_content_type(client): + """ + GIVEN a request with a non-JSON content-type (e.g., 'text/plain') + WHEN a PUT request is made + THEN the API should return a 400 Bad Request error. + """ + # --- ARRANGE --- + headers_with_wrong_type = { + "Content-Type": "text/plain", # The wrong type + "X-API-KEY": "test-key-123" + } + + # --- ACT --- + response = client.put( + "/books/some_id", + data="This is just plain text", + headers=headers_with_wrong_type + ) + + # --- ASSERT --- + assert response.status_code == 400 + response_data = response.get_json() + assert response_data["error"] == "Request must be valid JSON" + + # ------------------------ Tests for HELPER FUNCTIONS ------------------------------------- @@ -975,9 +1022,9 @@ def test_append_host_to_links_in_put(monkeypatch, client): assert response.status_code == 200 mock_append_hostname.assert_called_once() - call_args, call_kwargs = ( + call_args, call_kwargs = ( # pylint: disable=unused-variable mock_append_hostname.call_args - ) # pylint: disable=unused-variable + ) book_arg = call_args[0] host_arg = call_args[1] From 58a148652338f2dca65116d0e3fffeeb57a6a864 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 16:19:08 +0100 Subject: [PATCH 14/24] Add test to validate fields in PUT payload --- tests/test_mongo_helper.py | 41 +++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index 25e94a1..5fdfb9a 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -6,7 +6,8 @@ from app.datastore.mongo_helper import (delete_book_by_id, find_books, insert_book_to_mongo, - replace_book_by_id) + replace_book_by_id, + validate_book_put_payload) def test_insert_book_to_mongo_calls_insert_one(): @@ -183,3 +184,41 @@ def test_replace_book_by_id_invalid_id_returns_false(): # Assert assert result is False mock_collection.update_one.assert_not_called() + + +def test_validate_payload_fails_with_extra_fields(): + payload_with_extra_field = { + "title": "Valid Title", + "author": "Valid Author", + "synopsis": "A valid synopsis.", + "rating": 5 # This is the unexpected, extra field + } + + is_valid, error_dict = validate_book_put_payload(payload_with_extra_field) + + assert is_valid is False + assert isinstance(error_dict, dict) + assert "error" in error_dict + + expected_error_message = "Unexpected fields provided: rating" + assert error_dict["error"] == expected_error_message + +def test_validate_payload_fails_with_multiple_extra_fields(): + """ + GIVEN a payload with multiple extra fields + WHEN validate_book_put_payload is called + THEN the error message should list all extra fields in alphabetical order. + """ + payload_with_extra_fields = { + "title": "Valid Title", + "author": "Valid Author", + "synopsis": "A valid synopsis.", + "year": 2024, # Extra field + "isbn": "123-456" # Extra field + } + + is_valid, error_dict = validate_book_put_payload(payload_with_extra_fields) + + assert is_valid is False + expected_error_message = "Unexpected fields provided: isbn, year" + assert error_dict["error"] == expected_error_message From 9ca8396f74a212da28d5e52bd14b49e317dc4ee4 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 16:27:05 +0100 Subject: [PATCH 15/24] Move shared constants to conftest.py to reduce duplication (R0801) --- tests/conftest.py | 15 +++++++++++++++ tests/test_api_security.py | 15 ++------------- tests/test_app.py | 17 +++-------------- 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4982b70..96f1bbf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,21 @@ from app.datastore.mongo_db import get_book_collection +# A dictionary for headers to keep things clean +HEADERS = { + "VALID": {"X-API-KEY": "test-key-123"}, + "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, + "MISSING": {}, +} + +# A sample payload for POST/PUT requests +DUMMY_PAYLOAD = { + "title": "A Test Book", + "synopsis": "A test synopsis.", + "author": "Tester McTestFace", +} + + @pytest.fixture(name="_insert_book_to_db") def stub_insert_book(): """Fixture that mocks insert_book_to_mongo() to prevent real DB writes during tests. Returns a mock with a fixed inserted_id.""" diff --git a/tests/test_api_security.py b/tests/test_api_security.py index 85005a1..759c984 100644 --- a/tests/test_api_security.py +++ b/tests/test_api_security.py @@ -8,19 +8,8 @@ import pytest from bson.objectid import ObjectId -# A dictionary for headers to keep things clean -HEADERS = { - "VALID": {"X-API-KEY": "test-key-123"}, - "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, - "MISSING": {}, -} - -# A sample payload for POST/PUT requests -DUMMY_PAYLOAD = { - "title": "A Test Book", - "synopsis": "A test synopsis.", - "author": "Tester McTestFace", -} +from conftest import HEADERS, DUMMY_PAYLOAD + # -------------- LOGGING -------------------------- diff --git a/tests/test_app.py b/tests/test_app.py index 463f0fe..6b93a8a 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -6,9 +6,12 @@ from bson.objectid import ObjectId from pymongo.errors import ConnectionFailure, ServerSelectionTimeoutError +from conftest import HEADERS, DUMMY_PAYLOAD + from app import create_app, routes from app.datastore.mongo_db import get_book_collection + # Mock book database object books_database = [ { @@ -49,20 +52,6 @@ }, ] -# A dictionary for headers to keep things clean -HEADERS = { - "VALID": {"X-API-KEY": "test-key-123"}, - "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, - "MISSING": {}, -} - -# A sample payload for POST/PUT requests -DUMMY_PAYLOAD = { - "title": "A Test Book", - "synopsis": "A test synopsis.", - "author": "Tester McTestFace", -} - # ------------------- Tests for POST --------------------------------------------- From 57ec7d8156ba403cef150b9c66ebb7acbe165cc3 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Fri, 1 Aug 2025 17:00:36 +0100 Subject: [PATCH 16/24] Move append_hostname test to separate file to fix pylint length --- tests/test_app.py | 226 +------------------------------- tests/test_app_utils_helper.py | 232 +++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 224 deletions(-) create mode 100644 tests/test_app_utils_helper.py diff --git a/tests/test_app.py b/tests/test_app.py index 6b93a8a..b753ab0 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -2,13 +2,12 @@ from unittest.mock import ANY, MagicMock, patch -import pytest from bson.objectid import ObjectId -from pymongo.errors import ConnectionFailure, ServerSelectionTimeoutError +from pymongo.errors import ConnectionFailure from conftest import HEADERS, DUMMY_PAYLOAD -from app import create_app, routes +from app import routes from app.datastore.mongo_db import get_book_collection @@ -814,224 +813,3 @@ def test_update_book_fails_with_wrong_content_type(client): assert response.status_code == 400 response_data = response.get_json() assert response_data["error"] == "Request must be valid JSON" - - - -# ------------------------ Tests for HELPER FUNCTIONS ------------------------------------- - - -def test_add_book_response_contains_absolute_urls(client, monkeypatch): - # Arrange - test_book_payload = { - "title": "Append Test Book", - "author": "AN Other II", - "synopsis": "Test Synopsis", - } - valid_headers = {"X-API-KEY": "test-key-123"} - - # A. Mock the result of the insert operation - mock_insert_result = MagicMock() - mock_insert_result.inserted_id = ObjectId() - book_id_str = str(mock_insert_result.inserted_id) - - # B. Mock the book as it would look coming from the database, *before* formatting. - # It has an `_id` and RELATIVE links. - mock_book_from_db = test_book_payload.copy() - mock_book_from_db["_id"] = mock_insert_result.inserted_id - mock_book_from_db["links"] = {"self": f"/books/{book_id_str}"} - - # C. Mock the collection object - mock_collection = MagicMock() - mock_collection.find_one.return_value = mock_book_from_db - - # D. Mock the helper function that gets called - mock_insert_helper = MagicMock(return_value=mock_insert_result) - - # E. Apply all patches to isolate the route from the database - monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) - monkeypatch.setattr("app.routes.insert_book_to_mongo", mock_insert_helper) - - # Act - response = client.post("/books", json=test_book_payload, headers=valid_headers) - - # Assert - assert response.status_code == 201, f"Expected 201 but got {response.status_code}" - - response_data = response.get_json() - - assert "links" in response_data - assert "self" in response_data["links"] - - expected_link_start = f"http://localhost/books/{book_id_str}" - actual_link = response_data["links"]["self"] - - assert ( - actual_link == expected_link_start - ), f"Link did not have the correct absolute URL. Expected '{expected_link_start}', got '{actual_link}'" # pylint: disable=line-too-long - assert actual_link is not None, "Response JSON must contain a 'links' object" - - -@patch("app.services.book_service.find_books") -def test_append_host_to_links_in_get(mock_find_books, client): - - # ARRANGE: Provide sample data for the mock to return - mock_find_books.return_value = [ - { - "_id": "123", - "title": "A Test Book", - "author": "The Tester", - "synopsis": "A book for testing.", - "links": { - "self": "/books/123", - "reservations": "/books/123", - "reviews": "/books/123", - }, - } - ] - - # ACT - response = client.get("/books") - response_data = response.get_json() - - # ASSERT - assert response.status_code == 200 - - book = response_data["items"][0] - assert response.headers["content-type"] == "application/json" - assert isinstance(response_data, dict) - assert "total_count" in response_data - assert "items" in response_data - # response_data["items"]["links"]["self"] - for book in response_data["items"]: - new_book_id = book.get("id") - assert book["links"]["self"].startswith("http://localhost") - assert book["links"]["self"] == "http://localhost/books/123" - assert book["links"]["reservations"].startswith("http://localhost") - assert book["links"]["reviews"].startswith("http://localhost") - assert book["links"]["self"].endswith(f"books/{new_book_id}") - - -def test_append_host_to_links_in_get_book(client, monkeypatch): - """ - GIVEN a request for a book ID - WHEN the database and helper functions are mocked - THEN the route handler correctly calls its dependencies and formats the final JSON response. - """ - # Arrange: define constants and mock return values - book_id = ObjectId() - book_id_str = str(book_id) - book_from_db = { - "_id": book_id, - "title": "Some Title", - "author": "Some Author", - "synopsis": "Foo bar.", - "state": "active", - "links": {}, - } - # Mock the external dependencies - fake_collection = MagicMock() - fake_collection.find_one.return_value = book_from_db - # Patch the function that the route uses to get the collection - monkeypatch.setattr(routes, "get_book_collection", lambda: fake_collection) - - # mock append_hostname helper, define its output - mock_appender = MagicMock( - return_value={ - **book_from_db, - "links": { - "self": f"http://localhost/books/{book_id_str}", - "reservations": f"http://localhost/books/{book_id_str}", - "reviews": f"http://localhost/books/{book_id_str}", - }, - } - ) - monkeypatch.setattr(routes, "append_hostname", mock_appender) - - # ACT - response = client.get(f"/books/{book_id_str}") - - # Assert - assert response.status_code == 200 - response_data = response.get_json() - assert response_data["id"] == book_id_str - assert "links" in response_data - # Assert the final JSON is correct (based on the mock_appender's return value) - assert ( - response_data.get("links", {}).get("self") - == f"http://localhost/books/{book_id_str}" - ) - - # Now specifically check the 'self' link - self_link = response_data["links"]["self"] - reservations_link = response_data["links"]["reservations"] - reviews_link = response_data["links"]["reviews"] - assert self_link, "Expected a 'self' link in the response" - assert reservations_link - assert reviews_link - - # By default Flask's test_client serves on http://localhost/ - assert self_link.startswith( - "http://localhost" - ), f"Expected the link to start with 'http://localhost', got {self_link!r}" - - # And it must end with the resource path - assert self_link.endswith( - f"/books/{book_id_str}" - ), f"Expected link to end with '/books/{book_id_str}', got {self_link!r}" - # Assert the internal behavior was correct - expected_query = {"_id": book_id, "state": {"$ne": "deleted"}} - fake_collection.find_one.assert_called_once_with(expected_query) - mock_appender.assert_called_once_with(book_from_db, ANY) - - -def test_append_host_to_links_in_put(monkeypatch, client): - """ - GIVEN a successful PUT operation - WHEN the response is being formatted - THEN the `update_book` route must call the `append_hostname` helper. - """ - - test_book_id = str(ObjectId()) - test_payload = DUMMY_PAYLOAD - - # a) Set up the mock database flow - book_doc_from_db = {"_id": ObjectId(test_book_id), **test_payload} - mock_collection = MagicMock() - mock_collection.replace_one.return_value.matched_count = 1 - mock_collection.find_one.return_value = book_doc_from_db - monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) - - with patch("app.routes.append_hostname") as mock_append_hostname: - mock_append_hostname.side_effect = lambda book, host: book - - # --- 2. ACT --- - response = client.put( - f"/books/{test_book_id}", json=test_payload, headers=HEADERS["VALID"] - ) - - assert response.status_code == 200 - mock_append_hostname.assert_called_once() - call_args, call_kwargs = ( # pylint: disable=unused-variable - mock_append_hostname.call_args - ) - book_arg = call_args[0] - host_arg = call_args[1] - - assert "links" in book_arg - assert book_arg["links"]["self"] == f"/books/{test_book_id}" - assert host_arg == "http://localhost" # Flask test client default - - -def test_get_book_collection_handles_connection_failure(): - with patch("app.datastore.mongo_db.MongoClient") as mock_client: - # Set the side effect to raise a ServerSelectionTimeoutError - mock_client.side_effect = ServerSelectionTimeoutError("Mock Connection Timeout") - - app = create_app() - with app.app_context(): # <-- Push the app context here - with pytest.raises(Exception) as exc_info: - get_book_collection() - - assert "Could not connect to MongoDB: Mock Connection Timeout" in str( - exc_info.value - ) diff --git a/tests/test_app_utils_helper.py b/tests/test_app_utils_helper.py new file mode 100644 index 0000000..7383f65 --- /dev/null +++ b/tests/test_app_utils_helper.py @@ -0,0 +1,232 @@ +# pylint: disable=missing-docstring + +from unittest.mock import ANY, MagicMock, patch + +import pytest +from bson.objectid import ObjectId +from pymongo.errors import ServerSelectionTimeoutError + +from conftest import HEADERS, DUMMY_PAYLOAD + +from app import create_app, routes +from app.datastore.mongo_db import get_book_collection + + +# ------------------------ Tests for HELPER FUNCTIONS ------------------------------------- + + +def test_add_book_response_contains_absolute_urls(client, monkeypatch): + # Arrange + test_book_payload = { + "title": "Append Test Book", + "author": "AN Other II", + "synopsis": "Test Synopsis", + } + valid_headers = {"X-API-KEY": "test-key-123"} + + # A. Mock the result of the insert operation + mock_insert_result = MagicMock() + mock_insert_result.inserted_id = ObjectId() + book_id_str = str(mock_insert_result.inserted_id) + + # B. Mock the book as it would look coming from the database, *before* formatting. + # It has an `_id` and RELATIVE links. + mock_book_from_db = test_book_payload.copy() + mock_book_from_db["_id"] = mock_insert_result.inserted_id + mock_book_from_db["links"] = {"self": f"/books/{book_id_str}"} + + # C. Mock the collection object + mock_collection = MagicMock() + mock_collection.find_one.return_value = mock_book_from_db + + # D. Mock the helper function that gets called + mock_insert_helper = MagicMock(return_value=mock_insert_result) + + # E. Apply all patches to isolate the route from the database + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + monkeypatch.setattr("app.routes.insert_book_to_mongo", mock_insert_helper) + + # Act + response = client.post("/books", json=test_book_payload, headers=valid_headers) + + # Assert + assert response.status_code == 201, f"Expected 201 but got {response.status_code}" + + response_data = response.get_json() + + assert "links" in response_data + assert "self" in response_data["links"] + + expected_link_start = f"http://localhost/books/{book_id_str}" + actual_link = response_data["links"]["self"] + + assert ( + actual_link == expected_link_start + ), f"Link did not have the correct absolute URL. Expected '{expected_link_start}', got '{actual_link}'" # pylint: disable=line-too-long + assert actual_link is not None, "Response JSON must contain a 'links' object" + + +@patch("app.services.book_service.find_books") +def test_append_host_to_links_in_get(mock_find_books, client): + + # ARRANGE: Provide sample data for the mock to return + mock_find_books.return_value = [ + { + "_id": "123", + "title": "A Test Book", + "author": "The Tester", + "synopsis": "A book for testing.", + "links": { + "self": "/books/123", + "reservations": "/books/123", + "reviews": "/books/123", + }, + } + ] + + # ACT + response = client.get("/books") + response_data = response.get_json() + + # ASSERT + assert response.status_code == 200 + + book = response_data["items"][0] + assert response.headers["content-type"] == "application/json" + assert isinstance(response_data, dict) + assert "total_count" in response_data + assert "items" in response_data + # response_data["items"]["links"]["self"] + for book in response_data["items"]: + new_book_id = book.get("id") + assert book["links"]["self"].startswith("http://localhost") + assert book["links"]["self"] == "http://localhost/books/123" + assert book["links"]["reservations"].startswith("http://localhost") + assert book["links"]["reviews"].startswith("http://localhost") + assert book["links"]["self"].endswith(f"books/{new_book_id}") + + +def test_append_host_to_links_in_get_book(client, monkeypatch): + """ + GIVEN a request for a book ID + WHEN the database and helper functions are mocked + THEN the route handler correctly calls its dependencies and formats the final JSON response. + """ + # Arrange: define constants and mock return values + book_id = ObjectId() + book_id_str = str(book_id) + book_from_db = { + "_id": book_id, + "title": "Some Title", + "author": "Some Author", + "synopsis": "Foo bar.", + "state": "active", + "links": {}, + } + # Mock the external dependencies + fake_collection = MagicMock() + fake_collection.find_one.return_value = book_from_db + # Patch the function that the route uses to get the collection + monkeypatch.setattr(routes, "get_book_collection", lambda: fake_collection) + + # mock append_hostname helper, define its output + mock_appender = MagicMock( + return_value={ + **book_from_db, + "links": { + "self": f"http://localhost/books/{book_id_str}", + "reservations": f"http://localhost/books/{book_id_str}", + "reviews": f"http://localhost/books/{book_id_str}", + }, + } + ) + monkeypatch.setattr(routes, "append_hostname", mock_appender) + + # ACT + response = client.get(f"/books/{book_id_str}") + + # Assert + assert response.status_code == 200 + response_data = response.get_json() + assert response_data["id"] == book_id_str + assert "links" in response_data + # Assert the final JSON is correct (based on the mock_appender's return value) + assert ( + response_data.get("links", {}).get("self") + == f"http://localhost/books/{book_id_str}" + ) + + # Now specifically check the 'self' link + self_link = response_data["links"]["self"] + reservations_link = response_data["links"]["reservations"] + reviews_link = response_data["links"]["reviews"] + assert self_link, "Expected a 'self' link in the response" + assert reservations_link + assert reviews_link + + # By default Flask's test_client serves on http://localhost/ + assert self_link.startswith( + "http://localhost" + ), f"Expected the link to start with 'http://localhost', got {self_link!r}" + + # And it must end with the resource path + assert self_link.endswith( + f"/books/{book_id_str}" + ), f"Expected link to end with '/books/{book_id_str}', got {self_link!r}" + # Assert the internal behavior was correct + expected_query = {"_id": book_id, "state": {"$ne": "deleted"}} + fake_collection.find_one.assert_called_once_with(expected_query) + mock_appender.assert_called_once_with(book_from_db, ANY) + + +def test_append_host_to_links_in_put(monkeypatch, client): + """ + GIVEN a successful PUT operation + WHEN the response is being formatted + THEN the `update_book` route must call the `append_hostname` helper. + """ + + test_book_id = str(ObjectId()) + test_payload = DUMMY_PAYLOAD + + # a) Set up the mock database flow + book_doc_from_db = {"_id": ObjectId(test_book_id), **test_payload} + mock_collection = MagicMock() + mock_collection.replace_one.return_value.matched_count = 1 + mock_collection.find_one.return_value = book_doc_from_db + monkeypatch.setattr("app.routes.get_book_collection", lambda: mock_collection) + + with patch("app.routes.append_hostname") as mock_append_hostname: + mock_append_hostname.side_effect = lambda book, host: book + + # --- 2. ACT --- + response = client.put( + f"/books/{test_book_id}", json=test_payload, headers=HEADERS["VALID"] + ) + + assert response.status_code == 200 + mock_append_hostname.assert_called_once() + call_args, call_kwargs = ( # pylint: disable=unused-variable + mock_append_hostname.call_args + ) + book_arg = call_args[0] + host_arg = call_args[1] + + assert "links" in book_arg + assert book_arg["links"]["self"] == f"/books/{test_book_id}" + assert host_arg == "http://localhost" # Flask test client default + + +def test_get_book_collection_handles_connection_failure(): + with patch("app.datastore.mongo_db.MongoClient") as mock_client: + # Set the side effect to raise a ServerSelectionTimeoutError + mock_client.side_effect = ServerSelectionTimeoutError("Mock Connection Timeout") + + app = create_app() + with app.app_context(): # <-- Push the app context here + with pytest.raises(Exception) as exc_info: + get_book_collection() + + assert "Could not connect to MongoDB: Mock Connection Timeout" in str( + exc_info.value + ) From 6a16467426fb7b209e96bf5759e0535a702a91bf Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 09:27:04 +0100 Subject: [PATCH 17/24] Add tests_data.py to fix pylint constant import errors --- tests/conftest.py | 15 --------------- tests/test_api_security.py | 2 +- tests/test_app.py | 2 +- tests/test_app_utils_helper.py | 2 +- tests/test_data.py | 16 ++++++++++++++++ 5 files changed, 19 insertions(+), 18 deletions(-) create mode 100644 tests/test_data.py diff --git a/tests/conftest.py b/tests/conftest.py index 96f1bbf..4982b70 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,21 +13,6 @@ from app.datastore.mongo_db import get_book_collection -# A dictionary for headers to keep things clean -HEADERS = { - "VALID": {"X-API-KEY": "test-key-123"}, - "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, - "MISSING": {}, -} - -# A sample payload for POST/PUT requests -DUMMY_PAYLOAD = { - "title": "A Test Book", - "synopsis": "A test synopsis.", - "author": "Tester McTestFace", -} - - @pytest.fixture(name="_insert_book_to_db") def stub_insert_book(): """Fixture that mocks insert_book_to_mongo() to prevent real DB writes during tests. Returns a mock with a fixed inserted_id.""" diff --git a/tests/test_api_security.py b/tests/test_api_security.py index 759c984..5377885 100644 --- a/tests/test_api_security.py +++ b/tests/test_api_security.py @@ -8,7 +8,7 @@ import pytest from bson.objectid import ObjectId -from conftest import HEADERS, DUMMY_PAYLOAD +from tests.test_data import HEADERS, DUMMY_PAYLOAD # -------------- LOGGING -------------------------- diff --git a/tests/test_app.py b/tests/test_app.py index b753ab0..a1fbab0 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -5,7 +5,7 @@ from bson.objectid import ObjectId from pymongo.errors import ConnectionFailure -from conftest import HEADERS, DUMMY_PAYLOAD +from tests.test_data import HEADERS, DUMMY_PAYLOAD from app import routes from app.datastore.mongo_db import get_book_collection diff --git a/tests/test_app_utils_helper.py b/tests/test_app_utils_helper.py index 7383f65..1a465d4 100644 --- a/tests/test_app_utils_helper.py +++ b/tests/test_app_utils_helper.py @@ -6,7 +6,7 @@ from bson.objectid import ObjectId from pymongo.errors import ServerSelectionTimeoutError -from conftest import HEADERS, DUMMY_PAYLOAD +from tests.test_data import HEADERS, DUMMY_PAYLOAD from app import create_app, routes from app.datastore.mongo_db import get_book_collection diff --git a/tests/test_data.py b/tests/test_data.py new file mode 100644 index 0000000..9c699a0 --- /dev/null +++ b/tests/test_data.py @@ -0,0 +1,16 @@ +"""Constants which couldnt be added to conftest""" + + +# A dictionary for headers to keep things clean +HEADERS = { + "VALID": {"X-API-KEY": "test-key-123"}, + "INVALID": {"X-API-KEY": "This-is-the-wrong-key-12345"}, + "MISSING": {}, +} + +# A sample payload for POST/PUT requests +DUMMY_PAYLOAD = { + "title": "A Test Book", + "synopsis": "A test synopsis.", + "author": "Tester McTestFace", +} From c98597de4321b23131d8c4d4d62f3e708bd8301f Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 09:55:52 +0100 Subject: [PATCH 18/24] Update openapi t reflect new PUT --- openapi.yml | 54 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/openapi.yml b/openapi.yml index a889cc8..f285cb3 100644 --- a/openapi.yml +++ b/openapi.yml @@ -303,10 +303,10 @@ paths: put: tags: - Books - summary: Update a book + summary: Update a book by replacement security: - ApiKeyAuth: [] - description: Updates an existing book by its unique ID. The entire book object must be provided in the request body. + description: Updates an existing book by its unique ID. This is a full replacement operation (HTTP PUT). The request body must contain all required fields (`title`, `synopsis`, `author`) and no extra fields. operationId: updateBook parameters: - name: book_id @@ -318,7 +318,7 @@ paths: pattern: '^[a-f\d]{24}$' example: "635f3a7e3a8e3bcfc8e6a1e0" requestBody: - description: Book object that needs to be updated. + description: A complete Book object to replace the existing one. required: true content: application/json: @@ -332,15 +332,55 @@ paths: schema: $ref: '#/components/schemas/BookOutput' '400': - $ref: '#/components/responses/BadRequest' + description: |- + Bad Request. The request is invalid for one of the following reasons: + - The request body is not valid JSON. + - The JSON payload is missing required fields. + - The JSON payload contains unexpected fields. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + examples: + missingFields: + summary: Missing Fields Error + value: + error: "Missing required fields: author, synopsis" + extraFields: + summary: Extra Fields Error + value: + error: "Unexpected fields provided: publication_year" + invalidJsonBody: + summary: Invalid JSON + value: + error: "Request must be valid JSON" '401': $ref: '#/components/responses/Unauthorized' '404': - $ref: '#/components/responses/NotFound' + description: The book with the specified ID was not found. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Book with ID '635f3a7e3a8e3bcfc8e6a1e1' not found" '415': - $ref: '#/components/responses/UnsupportedMediaType' + description: |- + Unsupported Media Type. The client sent a request with a Content-Type header other than `application/json`. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Request must have Content-Type: application/json" '500': - $ref: '#/components/responses/InternalServerError' + description: An internal server error occurred, likely related to the database connection. + content: + application/json: + schema: + $ref: '#/components/schemas/SimpleError' + example: + error: "Book collection not initialized" # -------------------------------------------- From 505f4a6ec3415149d2bca84e72d35b450807e9da Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 10:14:24 +0100 Subject: [PATCH 19/24] Add comments, disable pylint error --- app/datastore/mongo_helper.py | 1 + tests/test_app_utils_helper.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 542cdd7..0d2e043 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -82,6 +82,7 @@ def delete_book_by_id(book_collection: dict, book_id: str): return result +# ------ PUT helpers ------------ def validate_book_put_payload(payload: dict): """ diff --git a/tests/test_app_utils_helper.py b/tests/test_app_utils_helper.py index 1a465d4..c159464 100644 --- a/tests/test_app_utils_helper.py +++ b/tests/test_app_utils_helper.py @@ -1,4 +1,4 @@ -# pylint: disable=missing-docstring +# pylint: disable=missing-docstring, duplicate-code from unittest.mock import ANY, MagicMock, patch From cbf5d75eb34d34845f3d106928458a9cceeb90a4 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 10:54:42 +0100 Subject: [PATCH 20/24] Update tests/test_mongo_helper.py Fix test with correct assertion (replace_one()) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/test_mongo_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index 5fdfb9a..3bc68c0 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -183,7 +183,7 @@ def test_replace_book_by_id_invalid_id_returns_false(): # Assert assert result is False - mock_collection.update_one.assert_not_called() + mock_collection.replace_one.assert_not_called() def test_validate_payload_fails_with_extra_fields(): From 0cc3dcc52c5874ade7b1785bb56b6c87df1dd373 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 11:07:44 +0100 Subject: [PATCH 21/24] Update tests/test_app_utils_helper.py unpack with '_' vs kwargs, indicate intentional throwaway Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/test_app_utils_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_app_utils_helper.py b/tests/test_app_utils_helper.py index c159464..2f325bd 100644 --- a/tests/test_app_utils_helper.py +++ b/tests/test_app_utils_helper.py @@ -206,7 +206,7 @@ def test_append_host_to_links_in_put(monkeypatch, client): assert response.status_code == 200 mock_append_hostname.assert_called_once() - call_args, call_kwargs = ( # pylint: disable=unused-variable + call_args, _ = ( # pylint: disable=unused-variable mock_append_hostname.call_args ) book_arg = call_args[0] From 7edf44d05237f2c29893297b7b77ccb52c7a1287 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 11:10:34 +0100 Subject: [PATCH 22/24] Update app/datastore/mongo_helper.py Use pymongo Collection type for book_collection annotation Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/datastore/mongo_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 0d2e043..329d2c3 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -65,7 +65,7 @@ def find_books(collection, query_filter=None, projection=None, limit=None) -> Cu return cursor -def delete_book_by_id(book_collection: dict, book_id: str): +def delete_book_by_id(book_collection: Collection, book_id: str): """ Soft delete book with given id Returns: The original document if found and updated, otherwise None. From 35c31caef9a6f4d1ff1c909e1d421ae4d33879cc Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 11:17:18 +0100 Subject: [PATCH 23/24] Import pymongo.collection to use Collection --- app/datastore/mongo_helper.py | 1 + tests/test_app.py | 1 + 2 files changed, 2 insertions(+) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 329d2c3..c2a889a 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -2,6 +2,7 @@ from bson.objectid import InvalidId, ObjectId from pymongo.cursor import Cursor +from pymongo.collection import Collection def insert_book_to_mongo(book_data, collection): diff --git a/tests/test_app.py b/tests/test_app.py index a1fbab0..72c7c54 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,6 +4,7 @@ from bson.objectid import ObjectId from pymongo.errors import ConnectionFailure +from pymongo.collection import Collection from tests.test_data import HEADERS, DUMMY_PAYLOAD From 1880204fa4f19b252fff66c2f5a5fe4e8141f8ae Mon Sep 17 00:00:00 2001 From: codesungrape Date: Mon, 4 Aug 2025 11:18:40 +0100 Subject: [PATCH 24/24] Remove unuse import --- tests/test_app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_app.py b/tests/test_app.py index 72c7c54..a1fbab0 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,7 +4,6 @@ from bson.objectid import ObjectId from pymongo.errors import ConnectionFailure -from pymongo.collection import Collection from tests.test_data import HEADERS, DUMMY_PAYLOAD