Implement POST /upload endpoint for multipart image upload with consumer validation#1
Implement POST /upload endpoint for multipart image upload with consumer validation#1
Conversation
…pload Co-authored-by: drdule <204492158+drdule@users.noreply.github.com>
Co-authored-by: drdule <204492158+drdule@users.noreply.github.com>
Co-authored-by: drdule <204492158+drdule@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a FastAPI-based image upload endpoint for a meter reading application. The implementation includes MySQL database integration, file validation, and proper error handling. The PR creates a complete working application from scratch with all necessary configuration files and documentation.
Key Changes
- Added POST /upload endpoint with multipart form support for consumer ID and image file
- Implemented MySQL database layer with connection management and helper functions for consumer validation and image record insertion
- Added comprehensive validation for file formats (JPEG, JPG, PNG) and consumer existence
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Specifies Python dependencies including FastAPI, uvicorn, mysql-connector-python, and python-multipart |
| main.py | Implements the FastAPI application with /upload endpoint, file validation, and error handling |
| database.py | Provides Database class for MySQL connection management and query execution |
| README.md | Comprehensive documentation including installation, API usage, and database schema |
| .env.example | Template for environment variables including database credentials and upload directory |
| .gitignore | Standard Python gitignore rules to exclude environment files, caches, and IDE files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| contents = await file.read() | ||
| with open(file_path, "wb") as f: | ||
| f.write(contents) |
There was a problem hiding this comment.
The file is read entirely into memory before writing. For large image files, this could cause memory issues. Consider using chunked reading and writing to handle large files more efficiently, especially for a production upload endpoint.
| contents = await file.read() | |
| with open(file_path, "wb") as f: | |
| f.write(contents) | |
| CHUNK_SIZE = 1024 * 1024 # 1MB per chunk | |
| with open(file_path, "wb") as f: | |
| while True: | |
| chunk = await file.read(CHUNK_SIZE) | |
| if not chunk: | |
| break | |
| f.write(chunk) |
There was a problem hiding this comment.
slika je mala može da bude do 2 mega pixela, uslovljeni smo uređajem koje imamo.
|
|
||
| # Save file to disk | ||
| try: | ||
| contents = await file.read() |
There was a problem hiding this comment.
There is no limit on file size, which could allow users to upload extremely large files and cause disk space exhaustion or DoS attacks. Add a file size validation check before reading the file contents.
There was a problem hiding this comment.
možemo da ograničimo kada budemo znali koliku sliku šalje uređaj
| upload_path.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Generate filename with timestamp | ||
| timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") |
There was a problem hiding this comment.
The timestamp format lacks milliseconds or microseconds, which could lead to filename collisions if multiple uploads occur within the same second for the same consumer. Consider adding milliseconds to the timestamp format or using a UUID to ensure uniqueness.
There was a problem hiding this comment.
nisu potrebne milisekunde jer uređaj ne može da napravi više do 1 slike u post 1 slanju
| status_code=500, | ||
| detail=f"Database error: {str(e)}" |
There was a problem hiding this comment.
The error message exposes internal database error details with str(e). In production, this could leak sensitive information about database structure or queries. Consider logging the full error internally and returning a generic error message to the user.
| self.user = os.getenv('DB_USER', 'ecitko_user') | ||
| self.password = os.getenv('DB_PASSWORD', 'strongpassword') | ||
| self.database = os.getenv('DB_NAME', 'ecitko_db') | ||
| self.connection = None |
There was a problem hiding this comment.
The Database class stores a single connection instance that is shared across methods. In a web application with concurrent requests, this could lead to race conditions or connection state issues. Consider using a connection pool or ensuring thread-safety, especially since FastAPI is async by default.
| status_code=500, | ||
| detail=f"Failed to save file: {str(e)}" |
There was a problem hiding this comment.
The error message exposes internal exception details with str(e). In production, this could leak sensitive information about the file system structure or permissions. Consider logging the full error internally and returning a generic error message to the user.
| if image_id is None: | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail="Failed to insert image record into database" | ||
| ) |
There was a problem hiding this comment.
The check for image_id being None is redundant since the previous except block would catch any database errors. If insert_image returns None without raising an exception, that would indicate a logic error in the database module. Consider removing this check or documenting why insert_image might return None without raising an exception.
| if image_id is None: | |
| raise HTTPException( | |
| status_code=500, | |
| detail="Failed to insert image record into database" | |
| ) |
| password=self.password, | ||
| database=self.database | ||
| ) | ||
| return self.connection |
There was a problem hiding this comment.
The connect method returns the connection, but this return value is not used anywhere (the connection is already stored in self.connection). Consider removing the return statement to simplify the method signature, or document why it might be needed.
| return self.connection |
| query = "SELECT id FROM consumers WHERE id = %s" | ||
| cursor.execute(query, (consumer_id,)) | ||
| result = cursor.fetchone() | ||
| cursor.close() |
There was a problem hiding this comment.
The cursor is not closed in an exception scenario. If an error occurs after cursor creation but before cursor.close(), the cursor will leak. Use a try-finally block or context manager to ensure the cursor is always closed.
|
|
||
| # Save file to disk | ||
| try: | ||
| contents = await file.read() |
There was a problem hiding this comment.
možemo da ograničimo kada budemo znali koliku sliku šalje uređaj
|
|
||
| CREATE TABLE IF NOT EXISTS consumers ( | ||
| id INT PRIMARY KEY AUTO_INCREMENT, | ||
| name VARCHAR(255) NOT NULL, |
There was a problem hiding this comment.
treba mi ispred name šifra korisnika, ili kako da rešimo jer jedan korisnik može da ima više vodomera koje treba da očitamo
| upload_path.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Generate filename with timestamp | ||
| timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") |
There was a problem hiding this comment.
nisu potrebne milisekunde jer uređaj ne može da napravi više do 1 slike u post 1 slanju
|
|
||
| # Generate filename with timestamp | ||
| timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") | ||
| new_filename = f"{consumerId}_{timestamp}_{file.filename}" |
There was a problem hiding this comment.
ok je ovo ali ako ne opterećuje sistem možemo da koristimo čisto radi bezbednosti jer post slanjem mi kreiramo ime datoteke
| contents = await file.read() | ||
| with open(file_path, "wb") as f: | ||
| f.write(contents) |
There was a problem hiding this comment.
slika je mala može da bude do 2 mega pixela, uslovljeni smo uređajem koje imamo.
| @app.post("/upload") | ||
| async def upload_image( | ||
| consumerId: int = Form(...), | ||
| file: UploadFile = File(...), | ||
| db: Database = Depends(get_database) | ||
| ): | ||
| """ | ||
| Upload an image for a consumer. | ||
|
|
||
| Args: | ||
| consumerId: The ID of the consumer | ||
| file: The image file to upload (JPEG, JPG, or PNG) | ||
| db: Database dependency | ||
|
|
||
| Returns: | ||
| JSON response with message, image_id, and image_url | ||
| """ | ||
|
|
||
| # Validate file format | ||
| if not file.filename: | ||
| raise HTTPException(status_code=400, detail="No file provided") | ||
|
|
||
| if not is_allowed_file(file.filename): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"Invalid file format. Allowed formats: {', '.join(ALLOWED_EXTENSIONS).upper()}" | ||
| ) | ||
|
|
||
| # Validate consumer exists | ||
| if not db.consumer_exists(consumerId): | ||
| raise HTTPException( | ||
| status_code=404, | ||
| detail=f"Consumer with ID {consumerId} not found" | ||
| ) | ||
|
|
||
| # Create upload directory if it doesn't exist | ||
| upload_path = Path(UPLOAD_DIR) | ||
| upload_path.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Generate filename with timestamp | ||
| timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") | ||
| new_filename = f"{consumerId}_{timestamp}_{file.filename}" | ||
| file_path = upload_path / new_filename | ||
|
|
||
| # Save file to disk | ||
| try: | ||
| contents = await file.read() | ||
| with open(file_path, "wb") as f: | ||
| f.write(contents) | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=f"Failed to save file: {str(e)}" | ||
| ) | ||
|
|
||
| # Insert record into database | ||
| image_url = str(file_path) | ||
| try: | ||
| image_id = db.insert_image(consumerId, image_url) | ||
| if image_id is None: | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail="Failed to insert image record into database" | ||
| ) | ||
| except Exception as e: | ||
| # Clean up file if database insert fails | ||
| if file_path.exists(): | ||
| file_path.unlink() | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=f"Database error: {str(e)}" | ||
| ) | ||
|
|
||
| return JSONResponse( | ||
| status_code=200, | ||
| content={ | ||
| "message": "Image uploaded successfully", | ||
| "image_id": image_id, | ||
| "image_url": image_url | ||
| } | ||
| ) |
There was a problem hiding this comment.
može samo greške sistema ili baze podataka.
Implements FastAPI endpoint for uploading consumer meter reading images with MySQL persistence and validation.
Implementation
Core endpoint (
main.py):/uploadacceptsconsumerId(int) andfile(UploadFile) as multipart/form-data/srv/ecitko/uploadswith naming:{consumerId}_{timestamp}_{filename}imagestable withconsumer_id,image_url,processed=0message,image_id,image_urlon successDatabase layer (
database.py):consumer_exists()validates consumer_idinsert_image()inserts record, returns generated idInfrastructure:
init.sqlcreates schema (consumers, images, readings) with test dataExample Usage
Response:
{ "message": "Image uploaded successfully", "image_id": 42, "image_url": "/srv/ecitko/uploads/1_20231215_143022_meter_reading.jpg" }Interactive docs available at
/docs(Swagger) and/redoc.Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.