diff --git a/backend/app/database/face_clusters.py b/backend/app/database/face_clusters.py index ba9f4ba4a..b9dfbbfe6 100644 --- a/backend/app/database/face_clusters.py +++ b/backend/app/database/face_clusters.py @@ -328,7 +328,13 @@ def db_get_images_by_cluster_id( import json - metadata_dict = json.loads(metadata) if metadata else None + if metadata: + try: + metadata_dict = json.loads(metadata) + except json.JSONDecodeError: + metadata_dict = None + else: + metadata_dict = None # Parse bbox JSON if it exists bbox = None if bbox_json: diff --git a/backend/app/routes/face_clusters.py b/backend/app/routes/face_clusters.py index 4951ac960..95470bfc1 100644 --- a/backend/app/routes/face_clusters.py +++ b/backend/app/routes/face_clusters.py @@ -174,12 +174,25 @@ def get_cluster_images(cluster_id: str): images_data = db_get_images_by_cluster_id(cluster_id) # Step 3: Convert to response models + + # Step 3: Convert to response models + import json + + def parse_metadata(metadata): + """Parse metadata if stored as JSON string.""" + if isinstance(metadata, str): + try: + metadata = json.loads(metadata) + except json.JSONDecodeError: + return None + return metadata if isinstance(metadata, dict) else None + images = [ ImageInCluster( id=img["image_id"], path=img["image_path"], thumbnailPath=img["thumbnail_path"], - metadata=img["metadata"], + metadata=parse_metadata(img["metadata"]), face_id=img["face_id"], confidence=img["confidence"], bbox=img["bbox"], diff --git a/backend/test_db.sqlite3 b/backend/test_db.sqlite3 new file mode 100644 index 000000000..fd5f2041f Binary files /dev/null and b/backend/test_db.sqlite3 differ diff --git a/backend/tests/test_face_clusters.py b/backend/tests/test_face_clusters.py index 2c09795c9..a7a92fd5a 100644 --- a/backend/tests/test_face_clusters.py +++ b/backend/tests/test_face_clusters.py @@ -370,18 +370,75 @@ def test_get_cluster_images_empty(self, mock_get_cluster, mock_get_images): assert data["data"]["total_images"] == 0 assert data["data"]["images"] == [] + # ============================================================================ + # Metadata JSON Parsing Tests (Fix for Issue #705) + # ============================================================================ + + @patch("app.routes.face_clusters.db_get_images_by_cluster_id") @patch("app.routes.face_clusters.db_get_cluster_by_id") - def test_get_cluster_images_database_error(self, mock_get_cluster): - """Test handling database errors during image retrieval.""" - cluster_id = "cluster_123" - mock_get_cluster.side_effect = Exception("Database connection failed") + def test_metadata_string_is_parsed(self, mock_get_cluster, mock_get_images): + """Metadata stored as JSON string should be parsed into a dict.""" + mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"} + mock_get_images.return_value = [{ + "image_id": "img_1", + "image_path": "/path/to/image1.jpg", + "thumbnail_path": "/path/to/thumb1.jpg", + "metadata": '{"camera": "Canon EOS", "location": "NYC"}', + "face_id": 101, + "confidence": 0.95, + "bbox": {"x": 100, "y": 200, "width": 150, "height": 200}, + }] - response = client.get(f"/face_clusters/{cluster_id}/images") + response = client.get("/face_clusters/cluster_123/images") + data = response.json() + image = data["data"]["images"][0] - assert response.status_code == 500 + assert isinstance(image["metadata"], dict) + assert image["metadata"]["camera"] == "Canon EOS" + + @patch("app.routes.face_clusters.db_get_images_by_cluster_id") + @patch("app.routes.face_clusters.db_get_cluster_by_id") + def test_metadata_dict_passes_through(self, mock_get_cluster, mock_get_images): + """Metadata already as dict should stay unchanged.""" + mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"} + mock_get_images.return_value = [{ + "image_id": "img_1", + "image_path": "/path/to/image1.jpg", + "thumbnail_path": "/path/to/thumb1.jpg", + "metadata": {"camera": "Canon EOS"}, + "face_id": 101, + "confidence": 0.95, + "bbox": {"x": 100, "y": 200, "width": 150, "height": 200}, + }] + + response = client.get("/face_clusters/cluster_123/images") data = response.json() - assert data["detail"]["success"] is False - assert data["detail"]["error"] == "Internal server error" + image = data["data"]["images"][0] + + assert isinstance(image["metadata"], dict) + assert image["metadata"]["camera"] == "Canon EOS" + + @patch("app.routes.face_clusters.db_get_images_by_cluster_id") + @patch("app.routes.face_clusters.db_get_cluster_by_id") + def test_invalid_metadata_returns_none(self, mock_get_cluster, mock_get_images): + """Broken JSON metadata should return None without crashing.""" + mock_get_cluster.return_value = {"cluster_id": "cluster_123", "cluster_name": "John"} + mock_get_images.return_value = [{ + "image_id": "img_1", + "image_path": "/path/to/image1.jpg", + "thumbnail_path": "/path/to/thumb1.jpg", + "metadata": "{bad json!!!}", + "face_id": 101, + "confidence": 0.95, + "bbox": {"x": 100, "y": 200, "width": 150, "height": 200}, + }] + + response = client.get("/face_clusters/cluster_123/images") + + assert response.status_code == 200 + data = response.json() + assert data["data"]["images"][0]["metadata"] is None + # ============================================================================ # Additional Edge Case Tests