From d8754165cc584ee81141cbafea4635161c80d606 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Tue, 22 Jul 2025 15:18:53 +0100 Subject: [PATCH 01/21] Clean up comments --- app/datastore/mongo_db.py | 4 ++-- app/datastore/mongo_helper.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/datastore/mongo_db.py b/app/datastore/mongo_db.py index 8d400eb..0fcb3ca 100644 --- a/app/datastore/mongo_db.py +++ b/app/datastore/mongo_db.py @@ -14,10 +14,10 @@ def get_book_collection(): client = MongoClient( current_app.config["MONGO_URI"], serverSelectionTimeoutMS=5000 ) - # Check the status of the server, will fail if server is down + db = client[current_app.config["DB_NAME"]] books_collection = db[current_app.config["COLLECTION_NAME"]] return books_collection except ConnectionFailure as e: - # Handle the connection error and return error information + raise ConnectionFailure(f"Could not connect to MongoDB: {str(e)}") from e diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 72956d9..9538ce9 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -24,7 +24,7 @@ def insert_book_to_mongo(book_data, collection): # - "If you DON'T find a document, INSERT the new data as a new document." result = collection.replace_one(query_filter, book_data, upsert=True) - # 3. Check the result for logging/feedback. + # Check the result for logging/feedback. if result.upserted_id: print(f"✅ INSERTED new book with id: {result.upserted_id}") elif result.modified_count > 0: From 13c783c9ddc05560c65f2c42a94c38121b6985c6 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 13:47:45 +0100 Subject: [PATCH 02/21] Format file with make format --- tests/test_api_security.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_api_security.py b/tests/test_api_security.py index 694eeb9..eb34c29 100644 --- a/tests/test_api_security.py +++ b/tests/test_api_security.py @@ -55,7 +55,6 @@ def test_add_book_fails_with_missing_key(client, monkeypatch): ) monkeypatch.setattr("app.routes.append_hostname", lambda book, host: book) - # Hit the endpoint without Authorization header response = client.post("/books", json=DUMMY_PAYLOAD) @@ -114,7 +113,6 @@ def test_update_book_succeeds_with_valid_api_key(monkeypatch, client): # 2. Patch append_hostname to just return the book monkeypatch.setattr("app.routes.append_hostname", lambda book, host: book) - # 3. Call the endpoint with request details response = client.put("/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["VALID"]) @@ -137,7 +135,9 @@ def test_update_book_fails_with_missing_api_key(monkeypatch, client): def test_update_book_fails_with_invalid_api_key(client, monkeypatch): monkeypatch.setattr("app.routes.books", []) - response = client.put("/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["INVALID"]) + response = client.put( + "/books/abc123", json=DUMMY_PAYLOAD, headers=HEADERS["INVALID"] + ) # ASSERT: Verify the server rejected the request as expected. assert response.status_code == 401 assert "Invalid API key." in response.json["error"]["message"] From ecbcc76aabf2161c6a32375d56ca5b444763ab18 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 14:11:17 +0100 Subject: [PATCH 03/21] Create book_service.py with fetch_active_books_cursor function --- app/services/book_service.py | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 app/services/book_service.py diff --git a/app/services/book_service.py b/app/services/book_service.py new file mode 100644 index 0000000..7ffc59a --- /dev/null +++ b/app/services/book_service.py @@ -0,0 +1,10 @@ +from app.datastore.mongo_db import get_book_collection +from app.datastore.mongo_helper import find_books + +def fetch_active_books_cursor(): + """Fetches a cursor for all non-deleted books from the database.""" + + collection = get_book_collection() + query_filter = {"state": {"ne": "deleted"}} # Only non-deleted books + + return find_books(collection, query_filter=query_filter) From f8e3c3c5375fd2f29c3c9b959c256f4e67e93420 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 14:20:58 +0100 Subject: [PATCH 04/21] Add find_books() to wrap collection.find and return a cursor --- app/datastore/mongo_helper.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 9538ce9..140ddb0 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -1,5 +1,7 @@ """Module containing pymongo helper functions.""" +from pymongo.cursor import Cursor + def insert_book_to_mongo(book_data, collection): """ @@ -31,3 +33,22 @@ def insert_book_to_mongo(book_data, collection): print(f"✅ REPLACED existing book with id: {book_data['id']}") return result + + +def find_books(collection, query_filter=None, projection=None, limit=None) -> Cursor: + """This acts as a wrapper around pymongo's collection.find() method. + + Args: + collection: The pymongo collection object. + filter: The MongoDB query filter. Defaults to None (find all). + projection: The fields to include/exclude. Defaults to None (all fields). + limit: The maximum number of results to return. Defaults to None. + + Returns: + A pymongo Cursor for the query results. + """ + query_filter = query_filter or {} + cursor = collection.find(query_filter, projection) + if limit is not None and limit > 0: + cursor = cursor.limit(limit) + return cursor From 71fb70acf8ce864fde6a19803a647e781d99ac1f Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 15:09:41 +0100 Subject: [PATCH 05/21] Add API formatter for books fetched from MongoDB --- app/services/book_service.py | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/app/services/book_service.py b/app/services/book_service.py index 7ffc59a..d27af90 100644 --- a/app/services/book_service.py +++ b/app/services/book_service.py @@ -1,3 +1,6 @@ +"""Service layer for handling book-related operations.""" + +from app.utils.helper import append_hostname from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import find_books @@ -8,3 +11,54 @@ def fetch_active_books_cursor(): query_filter = {"state": {"ne": "deleted"}} # Only non-deleted books return find_books(collection, query_filter=query_filter) + + +def format_books_for_api(books_cursor, host_url): + """ + Validate and format a list or cursor of raw book dicts for API response. + + Returns: + tuple: (formatted_list, None) on success, + (None, msg_lines) on validation failure. + """ + + required_fields = ["id", "title", "synopsis", "author", "links"] + missing_fields_info = [] + + # 1) Collect all validation errors + for raw in books_cursor: + missing = [f for f in required_fields if f not in raw] + if missing: + missing_fields_info.append( + {"book": raw, "missing_fields": missing} + ) + + # 2) If any errors, build error message and return + if missing_fields_info: + msg_lines_list = ["Missing required fields:"] + + # In a loop, add new strings to the list + for info in missing_fields_info: + fields = ", ".join(info["missing_fields"]) + msg_lines_list.append(f"- {fields} in book: {info['book']}") + + # Join all the strings in the list ONCE at the end + error_message = "\n".join(msg_lines_list) + + # 4. Return the final string + return None, error_message + + + + # FORMAT: Remove fields not meant for the public API + formatted_list = [] + for raw in books_cursor: + book = raw.copy() + book.pop("_id", None) + book.pop("state", None) + + # Call the helper with the host_url it needs + book_with_hostname = append_hostname(book, host_url) + formatted_list.append(book_with_hostname) + + return formatted_list From daa48bd820e9ff9e48146fbe0a7b83d3066eb3ee Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 15:17:11 +0100 Subject: [PATCH 06/21] Refactor GET /books to use service layer helpers; format code --- app/routes.py | 60 ++++++++++++++++----------------------------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/app/routes.py b/app/routes.py index 112bfa0..df64f77 100644 --- a/app/routes.py +++ b/app/routes.py @@ -8,6 +8,8 @@ from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import insert_book_to_mongo +from app.services.book_service import (fetch_active_books_cursor, + format_books_for_api) from app.utils.api_security import require_api_key from app.utils.helper import append_hostname from data import books @@ -26,7 +28,7 @@ def register_routes(app): # pylint: disable=too-many-statements @require_api_key def add_book(): """Function to add a new book to the collection.""" - # check if request is json + if not request.is_json: return jsonify({"error": "Request must be JSON"}), 415 @@ -65,9 +67,9 @@ def add_book(): if not isinstance(new_book[field], expected_type): return {"error": f"Field {field} is not of type {expected_type}"}, 400 - # use helper function + # establish connection to mongoDB books_collection = get_book_collection() - # check if mongoDB connected?? + # use mongoDB helper to insert/replace new book insert_book_to_mongo(new_book, books_collection) # Get the host from the request headers @@ -89,45 +91,28 @@ def get_all_books(): return them in a JSON response including the total count. """ - if not books: + raw_books = fetch_active_books_cursor() # pylint: disable=redefined-outer-name + + if not raw_books: return jsonify({"error": "No books found"}), 404 - all_books = [] # extract host from the request host = request.host_url - for book in books: - # check if the book has the "deleted" state - if book.get("state") != "deleted": - # Remove state unless it's "deleted", then append - book_copy = copy.deepcopy(book) - book_copy.pop("state", None) - book_with_hostname = append_hostname(book_copy, host) - all_books.append(book_with_hostname) - - # validation - required_fields = ["id", "title", "synopsis", "author", "links"] - missing_fields_info = [] - - for book in all_books: - missing_fields = [field for field in required_fields if field not in book] - if missing_fields: - missing_fields_info.append( - {"book": book, "missing_fields": missing_fields} - ) + all_formatted_books, error = format_books_for_api(raw_books, host) - if missing_fields_info: - error_message = "Missing required fields:\n" - for info in missing_fields_info: - error_message += f"Missing fields: {', '.join(info['missing_fields'])} in {info['book']}. \n" # pylint: disable=line-too-long + if error: + # Return HTTP error in controller layer + return jsonify({"error": error}), 500 - print(error_message) - return jsonify({"error": error_message}), 500 + if not all_formatted_books: + return jsonify({"error": "No books found"}), 404 - count_books = len(all_books) - response_data = {"total_count": count_books, "items": all_books} + return jsonify({ + "total_count": len(all_formatted_books), + "items": all_formatted_books + }), 200 - return jsonify(response_data), 200 @app.route("/books/", methods=["GET"]) def get_book(book_id): @@ -219,7 +204,6 @@ def update_book(book_id): return jsonify({"error": "Book not found"}), 404 - # ----------- CUSTOM ERROR HANDLERS ------------------ @app.errorhandler(NotFound) @@ -234,13 +218,7 @@ def handle_http_exception(e): """ # e.code is the HTTP status code (e.g. 401) # e.description is the text you passed to abort() - response = { - "error": { - "code": e.code, - "name": e.name, - "message": e.description - } - } + response = {"error": {"code": e.code, "name": e.name, "message": e.description}} return jsonify(response), e.code @app.errorhandler(Exception) From ce9c2b1ae4bf2483aa2ca156f025a70807d01815 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 16:17:13 +0100 Subject: [PATCH 07/21] Refactor fetch_active_books to return list; update callers --- app/routes.py | 9 +++------ app/services/book_service.py | 16 +++++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/routes.py b/app/routes.py index df64f77..953e703 100644 --- a/app/routes.py +++ b/app/routes.py @@ -8,7 +8,7 @@ from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import insert_book_to_mongo -from app.services.book_service import (fetch_active_books_cursor, +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 @@ -91,13 +91,13 @@ def get_all_books(): return them in a JSON response including the total count. """ - raw_books = fetch_active_books_cursor() # pylint: disable=redefined-outer-name + raw_books = fetch_active_books() if not raw_books: return jsonify({"error": "No books found"}), 404 # extract host from the request - host = request.host_url + host = request.host_url.rstrip("/") all_formatted_books, error = format_books_for_api(raw_books, host) @@ -105,9 +105,6 @@ def get_all_books(): # Return HTTP error in controller layer return jsonify({"error": error}), 500 - if not all_formatted_books: - return jsonify({"error": "No books found"}), 404 - return jsonify({ "total_count": len(all_formatted_books), "items": all_formatted_books diff --git a/app/services/book_service.py b/app/services/book_service.py index d27af90..45b13df 100644 --- a/app/services/book_service.py +++ b/app/services/book_service.py @@ -4,16 +4,19 @@ from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import find_books -def fetch_active_books_cursor(): - """Fetches a cursor for all non-deleted books from the database.""" +def fetch_active_books(): + """ + Fetches a cursor for all non-deleted books from the database. + Returns a Python list. + """ collection = get_book_collection() query_filter = {"state": {"ne": "deleted"}} # Only non-deleted books - return find_books(collection, query_filter=query_filter) + return list(find_books(collection, query_filter=query_filter)) -def format_books_for_api(books_cursor, host_url): +def format_books_for_api(books, host_url): """ Validate and format a list or cursor of raw book dicts for API response. @@ -26,7 +29,7 @@ def format_books_for_api(books_cursor, host_url): missing_fields_info = [] # 1) Collect all validation errors - for raw in books_cursor: + for raw in books: missing = [f for f in required_fields if f not in raw] if missing: missing_fields_info.append( @@ -49,10 +52,9 @@ def format_books_for_api(books_cursor, host_url): return None, error_message - # FORMAT: Remove fields not meant for the public API formatted_list = [] - for raw in books_cursor: + for raw in books: book = raw.copy() book.pop("_id", None) book.pop("state", None) From f2a41818170b54af457998e1f904dd1eb119d633 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 16:58:49 +0100 Subject: [PATCH 08/21] Update test to mock new service layer functions --- tests/test_app.py | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index e9c1d07..8b29eae 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,6 +1,6 @@ # pylint: disable=missing-docstring -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest from pymongo.errors import ServerSelectionTimeoutError @@ -152,25 +152,46 @@ def test_500_response_is_json(client): # ------------------------ Tests for GET -------------------------------------------- -def test_get_all_books_returns_all_books(client): + +@patch("app.routes.format_books_for_api") +@patch("app.routes.fetch_active_books") +def test_get_all_books_returns_all_books(mock_fetch, mock_format, client): + + mock_books_list = MagicMock() + mock_fetch.return_value = mock_books_list + + mock_formatted_data = [ + {"id": "1", "title": "A", "synopsis": "x", "author": "y", "links": {}}, + {"id": "2", "title": "B", "synopsis": "z", "author": "w", "links": {}}, + ] + + mock_format.return_value = (mock_formatted_data, None) + response = client.get("/books") + # Assert HTTP properties assert response.status_code == 200 assert response.headers["content-type"] == "application/json" + + # Assert the repsonse body response_data = response.get_json() assert isinstance(response_data, dict) - assert "total_count" in response_data - assert "items" in response_data + assert response_data["total_count"] == 2 + assert response_data["items"] == mock_formatted_data + + # Assert that mocks were called correctly + mock_fetch.assert_called_once() + mock_format.assert_called_once_with(mock_books_list, "http://localhost") -def test_return_error_404_when_list_is_empty(client): +def test_get_all_books_returns_error_404_when_list_is_empty(client): with patch("app.routes.books", []): response = client.get("/books") assert response.status_code == 404 assert "No books found" in response.get_json()["error"] -def test_get_books_returns_404_when_books_is_none(client): +def test_get_book_returns_404_when_books_is_none(client): with patch("app.routes.books", None): response = client.get("/books") assert response.status_code == 404 From 993eace080a66ad76490a6070971f575ba27a4ef Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 17:13:37 +0100 Subject: [PATCH 09/21] Fix indent to correctly aggregate validation errors --- app/services/book_service.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/services/book_service.py b/app/services/book_service.py index 45b13df..f985a60 100644 --- a/app/services/book_service.py +++ b/app/services/book_service.py @@ -1,8 +1,9 @@ """Service layer for handling book-related operations.""" -from app.utils.helper import append_hostname from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import find_books +from app.utils.helper import append_hostname + def fetch_active_books(): """ @@ -32,9 +33,7 @@ def format_books_for_api(books, host_url): for raw in books: missing = [f for f in required_fields if f not in raw] if missing: - missing_fields_info.append( - {"book": raw, "missing_fields": missing} - ) + missing_fields_info.append({"book": raw, "missing_fields": missing}) # 2) If any errors, build error message and return if missing_fields_info: @@ -45,12 +44,11 @@ def format_books_for_api(books, host_url): fields = ", ".join(info["missing_fields"]) msg_lines_list.append(f"- {fields} in book: {info['book']}") - # Join all the strings in the list ONCE at the end - error_message = "\n".join(msg_lines_list) - - # 4. Return the final string - return None, error_message + # Join all the strings in the list ONCE at the end + error_message = "\n".join(msg_lines_list) + # 4. Return the final string + return None, error_message # FORMAT: Remove fields not meant for the public API formatted_list = [] From 2ee43fa4e234afd9d95a74756bbbccebdf156348 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Wed, 23 Jul 2025 17:17:53 +0100 Subject: [PATCH 10/21] Align book endpoint tests with service layer --- tests/test_app.py | 51 ++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index 8b29eae..af7b2f9 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -153,6 +153,7 @@ def test_500_response_is_json(client): # ------------------------ Tests for GET -------------------------------------------- + @patch("app.routes.format_books_for_api") @patch("app.routes.fetch_active_books") def test_get_all_books_returns_all_books(mock_fetch, mock_format, client): @@ -184,6 +185,33 @@ def test_get_all_books_returns_all_books(mock_fetch, mock_format, client): mock_format.assert_called_once_with(mock_books_list, "http://localhost") +@patch("app.routes.fetch_active_books") +def test_missing_fields_in_book_object_returned_by_database(mock_fetch, client): + + bad_raw_data = [ + {"id": "1", "synopsis": "x", "author": "y", "links": {}}, # Missing 'title' + {"id": "2", "title": "B", "author": "w", "links": {}}, # Missing 'synopsis' + ] + mock_fetch.return_value = bad_raw_data + + expected_error_message = ( + "Missing required fields:\n" + f"- title in book: {bad_raw_data[0]}\n" + f"- synopsis in book: {bad_raw_data[1]}" + ) + + # --- ACT --- + response = client.get("/books") + + # --- ASSERT --- + assert response.status_code == 500 + + response_data = response.get_json() + assert "error" in response_data + assert response_data["error"] == expected_error_message + mock_fetch.assert_called_once() + + def test_get_all_books_returns_error_404_when_list_is_empty(client): with patch("app.routes.books", []): response = client.get("/books") @@ -198,29 +226,6 @@ def test_get_book_returns_404_when_books_is_none(client): assert "No books found" in response.get_json()["error"] -def test_missing_fields_in_book_object_returned_by_database(client): - with patch( - "app.routes.books", - [ - { - "id": "1", - "title": "The Great Adventure", - "synopsis": "A thrilling adventure through the jungles of South America.", - "author": "Jane Doe", - }, - {"id": "2", "title": "Mystery of the Old Manor"}, - { - "id": "3", - "title": "The Science of Everything", - "synopsis": "An in-depth look at the scientific principles that govern our world.", - }, - ], - ): - response = client.get("/books") - assert response.status_code == 500 - assert "Missing fields" in response.get_json()["error"] - - # -------- Tests for filter GET /books by delete ---------------- From 3ee22686ad916366b5788dcff46ffc4cd8a70559 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 10:00:58 +0100 Subject: [PATCH 11/21] Fix $ MongoDB syntax; return None for consistency --- app/services/book_service.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/services/book_service.py b/app/services/book_service.py index f985a60..e55ff9e 100644 --- a/app/services/book_service.py +++ b/app/services/book_service.py @@ -12,9 +12,10 @@ def fetch_active_books(): """ collection = get_book_collection() - query_filter = {"state": {"ne": "deleted"}} # Only non-deleted books + query_filter = {"state": {"$ne": "deleted"}} # Only non-deleted books - return list(find_books(collection, query_filter=query_filter)) + cursor = find_books(collection, query_filter=query_filter) + return list(cursor) def format_books_for_api(books, host_url): @@ -61,4 +62,4 @@ def format_books_for_api(books, host_url): book_with_hostname = append_hostname(book, host_url) formatted_list.append(book_with_hostname) - return formatted_list + return formatted_list, None From 41f92a425889c820cd7533f17332fcd7d124e626 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 13:07:31 +0100 Subject: [PATCH 12/21] Fix format_books_for_api order: create id before validation --- app/services/book_service.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/app/services/book_service.py b/app/services/book_service.py index e55ff9e..0d6e473 100644 --- a/app/services/book_service.py +++ b/app/services/book_service.py @@ -20,23 +20,27 @@ def fetch_active_books(): def format_books_for_api(books, host_url): """ - Validate and format a list or cursor of raw book dicts for API response. - - Returns: - tuple: (formatted_list, None) on success, - (None, msg_lines) on validation failure. + Process, validate, and format a list of raw book dicts for API response. """ + # 1) PRE-PROCESS: Create a new list with the fields we will validate. + processed_books = [] + for raw in books: + book = raw.copy() + # Create the 'id' field from '_id' for consistent validation. + if "_id" in book: + book["id"] = str(book["_id"]) + processed_books.append(book) required_fields = ["id", "title", "synopsis", "author", "links"] missing_fields_info = [] - # 1) Collect all validation errors - for raw in books: + # 2) Collect all validation errors + for raw in processed_books: missing = [f for f in required_fields if f not in raw] if missing: missing_fields_info.append({"book": raw, "missing_fields": missing}) - # 2) If any errors, build error message and return + # 3) If any errors, build error message and return if missing_fields_info: msg_lines_list = ["Missing required fields:"] @@ -53,8 +57,7 @@ def format_books_for_api(books, host_url): # FORMAT: Remove fields not meant for the public API formatted_list = [] - for raw in books: - book = raw.copy() + for book in processed_books: book.pop("_id", None) book.pop("state", None) From 47cdfc7cd26202c734846fd05598f701b61fcae7 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 13:13:46 +0100 Subject: [PATCH 13/21] Replace in-memory GET books test with find_books mock and format check --- tests/test_app.py | 154 ++++++++++++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 67 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index af7b2f9..2bc8cda 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,6 +1,6 @@ # pylint: disable=missing-docstring -from unittest.mock import MagicMock, patch +from unittest.mock import ANY, MagicMock, patch import pytest from pymongo.errors import ServerSelectionTimeoutError @@ -212,79 +212,99 @@ def test_missing_fields_in_book_object_returned_by_database(mock_fetch, client): mock_fetch.assert_called_once() -def test_get_all_books_returns_error_404_when_list_is_empty(client): - with patch("app.routes.books", []): - response = client.get("/books") - assert response.status_code == 404 - assert "No books found" in response.get_json()["error"] +@patch("app.routes.fetch_active_books") +def test_get_all_books_returns_error_404_when_list_is_empty(mock_fetch, client): + empty_data = [] + mock_fetch.return_value = empty_data + response = client.get("/books") + assert response.status_code == 404 + assert "No books found" in response.get_json()["error"] -def test_get_book_returns_404_when_books_is_none(client): - with patch("app.routes.books", None): - response = client.get("/books") - assert response.status_code == 404 - assert "No books found" in response.get_json()["error"] - - -# -------- Tests for filter GET /books by delete ---------------- - - -def test_get_books_excludes_deleted_books_and_omits_state_field(client): - # Add a book so we have a known ID - with patch( - "app.routes.books", - [ - { - "id": "1", - "title": "The Great Adventure", - "synopsis": "A thrilling adventure through the jungles of South America.", - "author": "Jane Doe", - "links": { - "self": "/books/1", - "reservations": "/books/1/reservations", - "reviews": "/books/1/reviews", - }, - "state": "deleted", +@patch("app.routes.fetch_active_books") +def test_get_book_returns_404_when_books_is_none(mock_fetch, client): + none_data = None + mock_fetch.return_value = none_data + response = client.get("/books") + assert response.status_code == 404 + assert "No books found" in response.get_json()["error"] + + +@patch("app.services.book_service.find_books") +def test_get_books_retrieves_and_formats_books_correctly(mock_find_books, client): + """ + GIVEN a mocked database service + WHEN the /books endpoint is called + THEN the service layer correctly queries the database for non-deleted books + AND the API response is correctly formatted with absolute URLs + """ + # ARRANGE + filtered_db_result = [ + { + "_id": "2", + "title": "Mystery of the Old Manor", + "author": "John Smith", + "synopsis": "A detective story set in an old manor with many secrets.", + "links": { + "self": "/books/2", + "reservations": "/books/2/reservations", + "reviews": "/books/2/reviews", + }, + "state": "active", + }, + { + "_id": "3", + "title": "The Science of Everything", + "author": "Alice Johnson", + "synopsis": "An in-depth look at the scientific principles that govern our world.", + "links": { + "self": "/books/3", + "reservations": "/books/3/reservations", + "reviews": "/books/3/reviews", }, - { - "id": "2", - "title": "Mystery of the Old Manor", - "synopsis": "A detective story set in an old manor with many secrets.", - "author": "John Smith", - "links": { - "self": "/books/2", - "reservations": "/books/2/reservations", - "reviews": "/books/2/reviews", - }, - "state": "active", + # No 'state' field, correctly simulating a record that is implicitly active. + }, + ] + mock_find_books.return_value = filtered_db_result + + base_url = "http://localhost" + expected_response_items = [ + { + "id": "2", # Renamed from _id + "title": "Mystery of the Old Manor", + "author": "John Smith", + "synopsis": "A detective story set in an old manor with many secrets.", + "links": { + "self": f"{base_url}/books/2", + "reservations": f"{base_url}/books/2/reservations", + "reviews": f"{base_url}/books/2/reviews", }, - { - "id": "3", - "title": "The Science of Everything", - "synopsis": "An in-depth look at the scientific principles that govern our world.", - "author": "Alice Johnson", - "links": { - "self": "/books/3", - "reservations": "/books/3/reservations", - "reviews": "/books/3/reviews", - }, + }, + { + "id": "3", # Renamed from _id + "title": "The Science of Everything", + "author": "Alice Johnson", + "synopsis": "An in-depth look at the scientific principles that govern our world.", + "links": { + "self": f"{base_url}/books/3", + "reservations": f"{base_url}/books/3/reservations", + "reviews": f"{base_url}/books/3/reviews", }, - ], - ): - response = client.get("/books") - assert response.status_code == 200 - - data = response.get_json() - assert "items" in data - books = data["items"] + }, + ] - # Check right object is returned - assert len(books) == 2 - for book in books: - assert "state" not in book - assert books[0].get("id") == "2" - assert books[1].get("title") == "The Science of Everything" + # ACT + response = client.get("/books") + # ASSERT + assert response.status_code == 200 + # 1) Service layer called with correct filter + expected_db_filter = {"state": {"$ne": "deleted"}} + mock_find_books.assert_called_once_with(ANY, query_filter=expected_db_filter) + # 2) Response formatting is exactly as expected + response_data = response.get_json() + assert response_data["items"] == expected_response_items + assert len(response_data["items"]) == 2 # -------- Tests for GET a single resource ---------------- From 5162fb5012c2932a371f39f368d0f770be149ac1 Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 13:16:45 +0100 Subject: [PATCH 14/21] Apply formatting to mongo_helper and routes --- app/datastore/mongo_helper.py | 2 +- app/routes.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/datastore/mongo_helper.py b/app/datastore/mongo_helper.py index 140ddb0..a9d6f5b 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -45,7 +45,7 @@ def find_books(collection, query_filter=None, projection=None, limit=None) -> Cu limit: The maximum number of results to return. Defaults to None. Returns: - A pymongo Cursor for the query results. + A pymongo Cursor for the query results. """ query_filter = query_filter or {} cursor = collection.find(query_filter, projection) diff --git a/app/routes.py b/app/routes.py index 953e703..51b7559 100644 --- a/app/routes.py +++ b/app/routes.py @@ -8,8 +8,7 @@ from app.datastore.mongo_db import get_book_collection from app.datastore.mongo_helper import insert_book_to_mongo -from app.services.book_service import (fetch_active_books, - format_books_for_api) +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 @@ -105,11 +104,12 @@ def get_all_books(): # Return HTTP error in controller layer return jsonify({"error": error}), 500 - return jsonify({ - "total_count": len(all_formatted_books), - "items": all_formatted_books - }), 200 - + return ( + jsonify( + {"total_count": len(all_formatted_books), "items": all_formatted_books} + ), + 200, + ) @app.route("/books/", methods=["GET"]) def get_book(book_id): From e6cf2894a4d810c335639faa426a77abb7c75d4d Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 13:27:32 +0100 Subject: [PATCH 15/21] Update failin test with mock database patch --- tests/test_app.py | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index 2bc8cda..9b68efa 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -306,6 +306,7 @@ def test_get_books_retrieves_and_formats_books_correctly(mock_find_books, client assert response_data["items"] == expected_response_items assert len(response_data["items"]) == 2 + # -------- Tests for GET a single resource ---------------- @@ -580,22 +581,41 @@ def test_append_host_to_links_in_post(client, _insert_book_to_db): ), f"Link should end with the resource path '{expected_path}'" -def test_append_host_to_links_in_get(client): +@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 - assert response.headers["content-type"] == "application/json" - # Get the response data - response_data = response.get_json() + 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}") From be7c671b19893b11645e2218f56c07bb90765c3c Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 14:09:07 +0100 Subject: [PATCH 16/21] Handle database connection failures gracefully on GET /books --- app/routes.py | 10 +++++++++- tests/test_app.py | 23 ++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/routes.py b/app/routes.py index 51b7559..ac55c69 100644 --- a/app/routes.py +++ b/app/routes.py @@ -4,6 +4,7 @@ import uuid 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 @@ -90,7 +91,14 @@ def get_all_books(): return them in a JSON response including the total count. """ - raw_books = fetch_active_books() + try: + raw_books = fetch_active_books() + except ConnectionFailure: + error_payload = { + "error": "The database service is temporarily unavailable." + } + + return jsonify(error_payload), 503 if not raw_books: return jsonify({"error": "No books found"}), 404 diff --git a/tests/test_app.py b/tests/test_app.py index 9b68efa..8db5d55 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -3,7 +3,7 @@ from unittest.mock import ANY, MagicMock, patch import pytest -from pymongo.errors import ServerSelectionTimeoutError +from pymongo.errors import ConnectionFailure, ServerSelectionTimeoutError from app import create_app from app.datastore.mongo_db import get_book_collection @@ -307,6 +307,27 @@ def test_get_books_retrieves_and_formats_books_correctly(mock_find_books, client assert len(response_data["items"]) == 2 +@patch("app.services.book_service.get_book_collection") +def test_get_books_handles_database_connection_error(mock_get_collection, client): + """ + GIVEN the database connection fails + WHEN the /books endpoint is called + THEN a 503 Service Unavailable error should be returned with a friendly message. + """ + # ARRANGE: Configure the mock to raise the exception when called + mock_get_collection.side_effect = ConnectionFailure("Could not connect to DB") + + # ACT + response = client.get("/books") + + # ASSERT + assert response.status_code == 503 # Now asserting the correct code + + # This assertion will now pass because your controller is returning the correct message + expected_error = "The database service is temporarily unavailable." + assert expected_error == response.get_json()["error"] + + # -------- Tests for GET a single resource ---------------- From 0ec345d25c90acd73819464accd01ab3eadb13bc Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 14:43:08 +0100 Subject: [PATCH 17/21] Add tests for find_books db wrapper for 100% coverage --- tests/test_mongo_helper.py | 56 +++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/test_mongo_helper.py b/tests/test_mongo_helper.py index cbae6d4..094476d 100644 --- a/tests/test_mongo_helper.py +++ b/tests/test_mongo_helper.py @@ -2,7 +2,7 @@ from unittest.mock import MagicMock -from app.datastore.mongo_helper import insert_book_to_mongo +from app.datastore.mongo_helper import find_books, insert_book_to_mongo def test_insert_book_to_mongo_calls_replace_one_with_upsert(): @@ -29,3 +29,57 @@ def test_insert_book_to_mongo_calls_replace_one_with_upsert(): new_book, # Argument 2: The replacement document upsert=True, # Argument 3: The upsert keyword argument ) + + +def test_find_books_calls_find_with_filter_and_projection(): + # Arrange + mock_collection = MagicMock() + mock_cursor = MagicMock() + + mock_collection.find.return_value = mock_cursor + + # Define the inputs for our function. + test_filter = {"author": "Jane Doe"} + test_projection = {"_id": 0, "title": 1} + + # Act + result_cursor = find_books( + collection=mock_collection, query_filter=test_filter, projection=test_projection + ) + + # Assert + mock_collection.find.assert_called_once_with(test_filter, test_projection) + mock_cursor.limit.assert_not_called() + assert result_cursor == mock_cursor + + +def test_find_books_applies_limit_when_provided(): + """ + GIVEN a mock collection and a limit + WHEN find_books is called with a positive limit + THEN it should call the limit method on the cursor with the correct value. + """ + # Arrange + mock_collection = MagicMock() + mock_cursor = MagicMock() + # Create a NEW mock for the final cursor after .limit() is called. + mock_limited_cursor = MagicMock() + + mock_collection.find.return_value = mock_cursor + # Teach the first cursor what to do when .limit() is called. + mock_cursor.limit.return_value = mock_limited_cursor + + test_limit = 10 + + # Act + result_cursor = find_books(collection=mock_collection, limit=test_limit) + + # Assert + # 1. Check that find() was called (this time with a default filter). + mock_collection.find.assert_called_once() + + # 2. CRUCIAL: Check that the .limit() method was called exactly once + mock_cursor.limit.assert_called_once_with(test_limit) + + # 3. Check that the function returned the FINAL cursor from the chain. + assert result_cursor == mock_limited_cursor From 7c01565da75bc5007934ab581f0807da81a28eef Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 15:07:56 +0100 Subject: [PATCH 18/21] Update ConnectionFailure payload to reflect error schema --- app/routes.py | 6 +++++- tests/test_app.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/routes.py b/app/routes.py index ac55c69..b14d50b 100644 --- a/app/routes.py +++ b/app/routes.py @@ -95,7 +95,11 @@ def get_all_books(): raw_books = fetch_active_books() except ConnectionFailure: error_payload = { - "error": "The database service is temporarily unavailable." + "error": { + "code": 503, + "name": "Service Unavailable", + "message": "The database service is temporarily unavailable.", + } } return jsonify(error_payload), 503 diff --git a/tests/test_app.py b/tests/test_app.py index 8db5d55..46f5e0f 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -325,7 +325,7 @@ def test_get_books_handles_database_connection_error(mock_get_collection, client # This assertion will now pass because your controller is returning the correct message expected_error = "The database service is temporarily unavailable." - assert expected_error == response.get_json()["error"] + assert expected_error in response.json["error"]["message"] # -------- Tests for GET a single resource ---------------- From c8f9cf75196aa7933b76c4c4f7a4a27df09e372d Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 15:09:17 +0100 Subject: [PATCH 19/21] Update openapi.yml GET to reflect 503 response --- openapi.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/openapi.yml b/openapi.yml index 7be90bf..8386217 100644 --- a/openapi.yml +++ b/openapi.yml @@ -162,6 +162,12 @@ components: application/json: schema: $ref: '#/components/schemas/StandardError' + ServiceUnavailable: + description: The service is temporarily unavailable, likely due to a database connection issue or server maintenance. + content: + application/json: + schema: + $ref: '#/components/schemas/StandardError' InternalServerError: description: An unexpected error occurred on the server. content: @@ -230,7 +236,9 @@ paths: '404': $ref: '#/components/responses/NotFound' '500': - $ref: '#/components/responses/InternalServerError' + $ref: '#/components/responses/InternalServerError' + '503': + $ref: '#/components/responses/ServiceUnavailable' # -------------------------------------------- /books/{book_id}: # -------------------------------------------- From cb9e4b6c4dbb61427cc0b1da83dc329b0449ebba Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 15:24:12 +0100 Subject: [PATCH 20/21] Update app/datastore/mongo_helper.py Fix docstring to reflect parameter name, copilot review 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 a9d6f5b..ed8bee9 100644 --- a/app/datastore/mongo_helper.py +++ b/app/datastore/mongo_helper.py @@ -40,7 +40,7 @@ def find_books(collection, query_filter=None, projection=None, limit=None) -> Cu Args: collection: The pymongo collection object. - filter: The MongoDB query filter. Defaults to None (find all). + query_filter: The MongoDB query filter. Defaults to None (find all). projection: The fields to include/exclude. Defaults to None (all fields). limit: The maximum number of results to return. Defaults to None. From bf4114958e00637287459e3ac61bdaa4ff35daeb Mon Sep 17 00:00:00 2001 From: codesungrape Date: Thu, 24 Jul 2025 15:24:51 +0100 Subject: [PATCH 21/21] Update tests/test_app.py Copilot review- fix spelling error Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/test_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_app.py b/tests/test_app.py index 46f5e0f..8a7ebc6 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -174,7 +174,7 @@ def test_get_all_books_returns_all_books(mock_fetch, mock_format, client): assert response.status_code == 200 assert response.headers["content-type"] == "application/json" - # Assert the repsonse body + # Assert the response body response_data = response.get_json() assert isinstance(response_data, dict) assert response_data["total_count"] == 2