Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 72 additions & 3 deletions backend/app/database/images.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Standard library imports
import sqlite3
from typing import Any, List, Mapping, Tuple, TypedDict, Union
import math

# App-specific imports
from app.config.settings import (
Expand Down Expand Up @@ -39,6 +40,24 @@ class UntaggedImageRecord(TypedDict):
metadata: Mapping[str, Any]


class PaginationInfo(TypedDict):
"""Represents pagination metadata"""

page: int
limit: int
total_count: int
total_pages: int
has_next: bool
has_previous: bool


class PaginatedImagesResult(TypedDict):
"""Represents paginated images result"""

images: List[dict]
pagination: PaginationInfo


ImageClassPair = Tuple[ImageId, ClassId]


Expand Down Expand Up @@ -120,22 +139,39 @@ def db_bulk_insert_images(image_records: List[ImageRecord]) -> bool:
conn.close()


def db_get_all_images(tagged: Union[bool, None] = None) -> List[dict]:
def db_get_all_images(
tagged: Union[bool, None] = None,
page: Union[int, None] = None,
limit: Union[int, None] = None,
) -> Union[List[dict], PaginatedImagesResult]:
"""
Get all images from the database with their tags.

Args:
tagged: Optional filter for tagged status. If None, returns all images.
If True, returns only tagged images. If False, returns only untagged images.
page: Optional page number (1-indexed). If None, returns all images without pagination.
limit: Optional number of images per page. If None, returns all images without pagination.

Returns:
List of dictionaries containing all image data including tags
If page and limit are provided: PaginatedImagesResult with images and pagination info
Otherwise: List of dictionaries containing all image data including tags
"""
conn = _connect()
cursor = conn.cursor()

try:
# Build the query with optional WHERE clause
# First, get total count for pagination
count_query = "SELECT COUNT(DISTINCT i.id) FROM images i"
count_params = []
if tagged is not None:
count_query += " WHERE i.isTagged = ?"
count_params.append(tagged)

cursor.execute(count_query, count_params)
total_count = cursor.fetchone()[0]

# Build the main query with optional WHERE clause
query = """
SELECT
i.id,
Expand Down Expand Up @@ -205,10 +241,43 @@ def db_get_all_images(tagged: Union[bool, None] = None) -> List[dict]:
# Sort by path
images.sort(key=lambda x: x["path"])

# Apply pagination if page and limit are provided
if page is not None and limit is not None:
# Calculate pagination values
total_pages = math.ceil(total_count / limit) if limit > 0 else 0
offset = (page - 1) * limit
paginated_images = images[offset : offset + limit]

pagination_info: PaginationInfo = {
"page": page,
"limit": limit,
"total_count": total_count,
"total_pages": total_pages,
"has_next": page < total_pages,
"has_previous": page > 1,
}

return {
"images": paginated_images,
"pagination": pagination_info,
}

return images

except Exception as e:
logger.error(f"Error getting all images: {e}")
if page is not None and limit is not None:
return {
"images": [],
"pagination": {
"page": page,
"limit": limit,
"total_count": 0,
"total_pages": 0,
"has_next": False,
"has_previous": False,
},
}
return []
finally:
conn.close()
Expand Down
105 changes: 80 additions & 25 deletions backend/app/routes/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,45 +37,100 @@ class ImageData(BaseModel):
tags: Optional[List[str]] = None


class PaginationInfo(BaseModel):
page: int
limit: int
total_count: int
total_pages: int
has_next: bool
has_previous: bool


class GetAllImagesResponse(BaseModel):
success: bool
message: str
data: List[ImageData]


class PaginatedImagesResponse(BaseModel):
success: bool
message: str
data: List[ImageData]
pagination: PaginationInfo


@router.get(
"/",
response_model=GetAllImagesResponse,
response_model=None,
responses={500: {"model": ErrorResponse}},
)
def get_all_images(
tagged: Optional[bool] = Query(None, description="Filter images by tagged status")
tagged: Optional[bool] = Query(None, description="Filter images by tagged status"),
page: Optional[int] = Query(None, ge=1, description="Page number (1-indexed)"),
limit: Optional[int] = Query(
None, ge=1, le=100, description="Number of images per page (max 100)"
),
):
"""Get all images from the database."""
"""
Get all images from the database.

- If `page` and `limit` are provided, returns paginated results.
- If `page` and `limit` are not provided, returns all images (backward compatible).
"""
try:
# Get all images with tags from database (single query with optional filter)
images = db_get_all_images(tagged=tagged)

# Convert to response format
image_data = [
ImageData(
id=image["id"],
path=image["path"],
folder_id=image["folder_id"],
thumbnailPath=image["thumbnailPath"],
metadata=image_util_parse_metadata(image["metadata"]),
isTagged=image["isTagged"],
isFavourite=image.get("isFavourite", False),
tags=image["tags"],
# Get images with optional pagination
result = db_get_all_images(tagged=tagged, page=page, limit=limit)

# Check if paginated result
if page is not None and limit is not None:
images = result["images"]
pagination = result["pagination"]

# Convert to response format
image_data = [
ImageData(
id=image["id"],
path=image["path"],
folder_id=image["folder_id"],
thumbnailPath=image["thumbnailPath"],
metadata=image_util_parse_metadata(image["metadata"]),
isTagged=image["isTagged"],
isFavourite=image.get("isFavourite", False),
tags=image["tags"],
)
for image in images
]

return PaginatedImagesResponse(
success=True,
message=f"Successfully retrieved {len(image_data)} images (page {pagination['page']} of {pagination['total_pages']})",
data=image_data,
pagination=PaginationInfo(**pagination),
)
else:
# Non-paginated response (backward compatible)
images = result

# Convert to response format
image_data = [
ImageData(
id=image["id"],
path=image["path"],
folder_id=image["folder_id"],
thumbnailPath=image["thumbnailPath"],
metadata=image_util_parse_metadata(image["metadata"]),
isTagged=image["isTagged"],
isFavourite=image.get("isFavourite", False),
tags=image["tags"],
)
for image in images
]

return GetAllImagesResponse(
success=True,
message=f"Successfully retrieved {len(image_data)} images",
data=image_data,
)
Comment on lines +68 to 133
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and read the file
find . -name "images.py" -path "*/routes/*" | head -20

Repository: AOSSIE-Org/PictoPy

Length of output: 92


🏁 Script executed:

# Search for the db_get_all_images function definition
rg "def db_get_all_images" -A 30 -t py

Repository: AOSSIE-Org/PictoPy

Length of output: 2254


🏁 Script executed:

# Check the structure of the backend directory
ls -la backend/app/routes/ 2>/dev/null | head -20

Repository: AOSSIE-Org/PictoPy

Length of output: 551


🏁 Script executed:

# Read the complete db_get_all_images function
cat -n backend/app/database/images.py | grep -A 80 "def db_get_all_images"

Repository: AOSSIE-Org/PictoPy

Length of output: 3287


🏁 Script executed:

# Read the complete get_all_images function in routes
cat -n backend/app/routes/images.py | grep -A 80 "def get_all_images"

Repository: AOSSIE-Org/PictoPy

Length of output: 3613


🏁 Script executed:

# Get the rest of db_get_all_images function
cat -n backend/app/database/images.py | sed -n '142,250p'

Repository: AOSSIE-Org/PictoPy

Length of output: 4576


🏁 Script executed:

# Get the rest of db_get_all_images function
cat -n backend/app/database/images.py | sed -n '250,280p'

Repository: AOSSIE-Org/PictoPy

Length of output: 1209


Guard against partial pagination params to avoid unexpected full fetches.

If a client sends only page or only limit without both parameters, the function silently falls back to returning all images. This defeats the purpose of pagination and can reintroduce the large-response risk. Both page and limit should be required together or explicitly default; partial pagination parameters should not be silently accepted.

Consider rejecting requests with mismatched parameters:

Suggested validation
 def get_all_images(
     tagged: Optional[bool] = Query(None, description="Filter images by tagged status"),
     page: Optional[int] = Query(None, ge=1, description="Page number (1-indexed)"),
     limit: Optional[int] = Query(
         None, ge=1, le=100, description="Number of images per page (max 100)"
     ),
 ):
     """
     Get all images from the database.

     - If `page` and `limit` are provided, returns paginated results.
     - If `page` and `limit` are not provided, returns all images (backward compatible).
     """
     try:
+        if (page is None) ^ (limit is None):
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="Both 'page' and 'limit' must be provided together.",
+            )
         # Get images with optional pagination
         result = db_get_all_images(tagged=tagged, page=page, limit=limit)
🤖 Prompt for AI Agents
In `@backend/app/routes/images.py` around lines 68 - 133, The route currently
allows partial pagination by accepting only page or only limit and then calling
db_get_all_images which can return all rows; add validation at the start of the
handler to reject mismatched pagination params: if (page is None) != (limit is
None) raise an HTTPException(400) (or return a 400 response) with a clear
message like "Both 'page' and 'limit' must be provided together or neither";
keep existing behavior when both are None (return GetAllImagesResponse) and when
both are present call db_get_all_images and return PaginatedImagesResponse as
before. Ensure you reference the existing parameter names page, limit and
functions db_get_all_images, GetAllImagesResponse and PaginatedImagesResponse
when implementing the check.

for image in images
]

return GetAllImagesResponse(
success=True,
message=f"Successfully retrieved {len(image_data)} images",
data=image_data,
)

except Exception as e:
raise HTTPException(
Expand Down
64 changes: 60 additions & 4 deletions frontend/src/api/api-functions/images.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,68 @@
import { imagesEndpoints } from '../apiEndpoints';
import { apiClient } from '../axiosConfig';
import { APIResponse } from '@/types/API';
import { APIResponse, PaginatedAPIResponse } from '@/types/API';
import { Image } from '@/types/Media';

export interface FetchImagesParams {
tagged?: boolean;
page?: number;
limit?: number;
}

/**
* Fetch images from the API with optional pagination.
*
* @param params - Optional parameters for filtering and pagination
* @param params.tagged - Filter by tagged status
* @param params.page - Page number (1-indexed)
* @param params.limit - Number of images per page
* @returns APIResponse or PaginatedAPIResponse depending on whether pagination params are provided
*/
export const fetchAllImages = async (
params?: FetchImagesParams | boolean,
): Promise<APIResponse | PaginatedAPIResponse<Image>> => {
// Handle backward compatibility: if params is a boolean, treat it as tagged filter
const queryParams: Record<string, unknown> = {};

if (typeof params === 'boolean') {
queryParams.tagged = params;
} else if (params) {
if (params.tagged !== undefined) {
queryParams.tagged = params.tagged;
}
if (params.page !== undefined) {
queryParams.page = params.page;
}
if (params.limit !== undefined) {
queryParams.limit = params.limit;
}
}

const response = await apiClient.get<
APIResponse | PaginatedAPIResponse<Image>
>(imagesEndpoints.getAllImages, { params: queryParams });
return response.data;
};

/**
* Fetch paginated images from the API.
*
* @param page - Page number (1-indexed)
* @param limit - Number of images per page (default: 50)
* @param tagged - Optional filter by tagged status
* @returns PaginatedAPIResponse with images and pagination info
*/
export const fetchPaginatedImages = async (
page: number,
limit: number = 50,
tagged?: boolean,
): Promise<APIResponse> => {
const params = tagged !== undefined ? { tagged } : {};
const response = await apiClient.get<APIResponse>(
): Promise<PaginatedAPIResponse<Image>> => {
const params: Record<string, unknown> = { page, limit };
if (tagged !== undefined) {
params.tagged = tagged;
}

const response = await apiClient.get<PaginatedAPIResponse<Image>>(
imagesEndpoints.getAllImages,
{ params },
);
Expand Down
Loading