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..ed8bee9 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): """ @@ -24,10 +26,29 @@ 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: 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. + 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. + + 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 diff --git a/app/routes.py b/app/routes.py index 112bfa0..b14d50b 100644 --- a/app/routes.py +++ b/app/routes.py @@ -4,10 +4,12 @@ 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 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.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,37 @@ def get_all_books(): return them in a JSON response including the total count. """ - if not 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) + try: + raw_books = fetch_active_books() + except ConnectionFailure: + error_payload = { + "error": { + "code": 503, + "name": "Service Unavailable", + "message": "The database service is temporarily unavailable.", + } + } - # validation - required_fields = ["id", "title", "synopsis", "author", "links"] - missing_fields_info = [] + return jsonify(error_payload), 503 - 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} - ) + if not raw_books: + return jsonify({"error": "No books found"}), 404 - 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 + # extract host from the request + host = request.host_url.rstrip("/") - print(error_message) - return jsonify({"error": error_message}), 500 + all_formatted_books, error = format_books_for_api(raw_books, host) - count_books = len(all_books) - response_data = {"total_count": count_books, "items": all_books} + if error: + # Return HTTP error in controller layer + return jsonify({"error": error}), 500 - return jsonify(response_data), 200 + return ( + jsonify( + {"total_count": len(all_formatted_books), "items": all_formatted_books} + ), + 200, + ) @app.route("/books/", methods=["GET"]) def get_book(book_id): @@ -219,7 +213,6 @@ def update_book(book_id): return jsonify({"error": "Book not found"}), 404 - # ----------- CUSTOM ERROR HANDLERS ------------------ @app.errorhandler(NotFound) @@ -234,13 +227,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) diff --git a/app/services/book_service.py b/app/services/book_service.py new file mode 100644 index 0000000..0d6e473 --- /dev/null +++ b/app/services/book_service.py @@ -0,0 +1,68 @@ +"""Service layer for handling book-related operations.""" + +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(): + """ + 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 + + cursor = find_books(collection, query_filter=query_filter) + return list(cursor) + + +def format_books_for_api(books, host_url): + """ + 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 = [] + + # 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}) + + # 3) 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 book in processed_books: + 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, None 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}: # -------------------------------------------- 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"] diff --git a/tests/test_app.py b/tests/test_app.py index e9c1d07..8a7ebc6 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,9 +1,9 @@ # pylint: disable=missing-docstring -from unittest.mock import patch +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 @@ -152,112 +152,180 @@ 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 response 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): - 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_missing_fields_in_book_object_returned_by_database(mock_fetch, client): -def test_get_books_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"] - - -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.", + 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() + + +@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"] + + +@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", }, - ], - ): - response = client.get("/books") - assert response.status_code == 500 - assert "Missing fields" 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", + "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"] + # 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 - # 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" + +@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 in response.json["error"]["message"] # -------- Tests for GET a single resource ---------------- @@ -534,22 +602,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}") 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