Skip to content

Feature: issue #385 - Resctructured to follow bigger apps layout #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DimitriZC wants to merge 3 commits into from
Closed

Feature: issue #385 - Resctructured to follow bigger apps layout #388

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52b47c7
Fix: issue #385 - Resctructured to follow bigger apps layout
DimitriZC Jun 2, 2025
fad6cc7
Changed app folder to src to avoid folder name conflict
DimitriZC Jun 5, 2025
3ee2c92
Merge branch 'master' into feature/issue-385-Restructure-to-bigger-ap…
DimitriZC Jun 11, 2025
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
16 changes: 8 additions & 8 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ COPY requirements.txt .
RUN pip install --no-cache-dir --no-index --find-links /app/wheelhouse -r requirements.txt && \
rm -rf /app/wheelhouse

# Copy application source code
COPY main.py ./
COPY databases/ ./databases/
COPY models/ ./models/
COPY routes/ ./routes/
COPY schemas/ ./schemas/
COPY services/ ./services/
# Copy application source code
COPY app/main.py ./
COPY app/databases/ ./databases/
COPY app/models/ ./models/
COPY app/routes/ ./routes/
COPY app/schemas/ ./schemas/
COPY app/services/ ./services/

Comment on lines +48 to 55
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Dockerfile flattens app package, breaking imports

The current COPY commands break the app package hierarchy by installing subdirectories directly under /app. Since the code uses imports like app.main and app.models, there must be a top-level app directory in the image. Without it, uvicorn app.main:app will fail at runtime.

Please replace the individual COPY instructions with one that preserves the package:

- # Copy application source code 
- COPY app/main.py            ./
- COPY app/databases/         ./databases/
- COPY app/models/            ./models/
- COPY app/routes/            ./routes/
- COPY app/schemas/           ./schemas/
- COPY app/services/          ./services/
+ # Copy entire app package with correct ownership
+ COPY --chown=fastapi:fastapi app/ ./app/
🤖 Prompt for AI Agents 
In Dockerfile lines 48 to 55, the current COPY commands flatten the app package
by copying subdirectories directly into the image root, breaking imports like
app.main. To fix this, replace all individual COPY commands with a single COPY
that copies the entire app directory into the image, preserving the app package
hierarchy so that imports remain valid at runtime.

✅ Addressed in commit 589e8e6

# https://rules.sonarsource.com/docker/RSPEC-6504/

Expand All @@ -78,4 +78,4 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \
CMD ["./healthcheck.sh"]

ENTRYPOINT ["./entrypoint.sh"]
CMD ["uvicorn", "main:app", "--host", "0.0.0.0", "--port", "9000"]
CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "9000"]
0 databases/__init__.py → app/__init__.py
View file
Open in desktop
File renamed without changes.
0 models/__init__.py → app/databases/__init__.py
View file
Open in desktop
File renamed without changes.
0 databases/player_database.py → app/databases/player.py
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions main.py → app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import logging
from typing import AsyncIterator
from fastapi import FastAPI
from routes import player_route, health_route
from app.routes import player, health

# https://github.com/encode/uvicorn/issues/562
UVICORN_LOGGER = "uvicorn.error"
Expand All @@ -35,5 +35,5 @@ async def lifespan(_: FastAPI) -> AsyncIterator[None]:
version="1.0.0",
)

app.include_router(player_route.api_router)
app.include_router(health_route.api_router)
app.include_router(player.router)
app.include_router(health.router)
0 routes/__init__.py → app/models/__init__.py
View file
Open in desktop
File renamed without changes.
0 models/player_model.py → app/models/player.py
View file
Open in desktop
File renamed without changes.
0 schemas/__init__.py → app/routes/__init__.py
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions routes/health_route.py → app/routes/health.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

from fastapi import APIRouter

api_router = APIRouter()
router = APIRouter()


@api_router.get("/health", tags=["Health"])
@router.get("/health", tags=["Health"])
async def health_check():
"""
Simple health check endpoint.
Expand Down
60 changes: 31 additions & 29 deletions routes/player_route.py → app/routes/player.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,22 @@
from sqlalchemy.ext.asyncio import AsyncSession
from aiocache import SimpleMemoryCache

from databases.player_database import generate_async_session
from models.player_model import PlayerModel
from services import player_service
from app.databases.player import generate_async_session
from app.models.player import PlayerModel
from app.services import player

api_router = APIRouter()
router = APIRouter()
simple_memory_cache = SimpleMemoryCache()

CACHE_KEY = "players"
CACHE_TTL = 600 # 10 minutes

PLAYER_TITLE = "The ID of the Player"

# POST -------------------------------------------------------------------------


@api_router.post(
@router.post(
"/players/",
status_code=status.HTTP_201_CREATED,
summary="Creates a new Player",
Expand All @@ -56,17 +58,17 @@ async def post_async(
Raises:
HTTPException: HTTP 409 Conflict error if the Player already exists.
"""
player = await player_service.retrieve_by_id_async(async_session, player_model.id)
if player:
player_res = await player.retrieve_by_id_async(async_session, player_model.id)
if player_res:
raise HTTPException(status_code=status.HTTP_409_CONFLICT)
await player_service.create_async(async_session, player_model)
await player.create_async(async_session, player_model)
await simple_memory_cache.clear(CACHE_KEY)


# GET --------------------------------------------------------------------------


@api_router.get(
@router.get(
"/players/",
response_model=List[PlayerModel],
status_code=status.HTTP_200_OK,
Expand All @@ -88,21 +90,21 @@ async def get_all_async(
players = await simple_memory_cache.get(CACHE_KEY)
response.headers["X-Cache"] = "HIT"
if not players:
players = await player_service.retrieve_all_async(async_session)
players = await player.retrieve_all_async(async_session)
await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
response.headers["X-Cache"] = "MISS"
return players
Comment on lines 90 to
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix cache-miss logic to distinguish empty lists

Using if not players: treats an empty list as a miss. Switch to a None check so that an empty result still counts as a valid cache hit:

-    players = await simple_memory_cache.get(CACHE_KEY)
-    response.headers["X-Cache"] = "HIT"
-    if not players:
+    players = await simple_memory_cache.get(CACHE_KEY)
+    response.headers["X-Cache"] = "HIT"
+    if players is None:
         players = await player.retrieve_all_async(async_session)
         await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
         response.headers["X-Cache"] = "MISS"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
players = await simple_memory_cache.get(CACHE_KEY)
response.headers["X-Cache"] = "HIT"
if not players:
players = await player_service.retrieve_all_async(async_session)
players = await player.retrieve_all_async(async_session)
await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
response.headers["X-Cache"] = "MISS"
return players
players = await simple_memory_cache.get(CACHE_KEY)
response.headers["X-Cache"] = "HIT"
if players is None:
players = await player.retrieve_all_async(async_session)
await simple_memory_cache.set(CACHE_KEY, players, ttl=CACHE_TTL)
response.headers["X-Cache"] = "MISS"
return players
🤖 Prompt for AI Agents 
In app/routes/player.py around lines 90 to 96, the cache-miss logic incorrectly
treats an empty list as a cache miss by using 'if not players:'. Change this
condition to explicitly check for None instead, so that an empty list is
considered a valid cache hit and does not trigger a cache refresh.



@api_router.get(
@router.get(
"/players/{player_id}",
response_model=PlayerModel,
status_code=status.HTTP_200_OK,
summary="Retrieves a Player by its Id",
tags=["Players"],
)
async def get_by_id_async(
player_id: int = Path(..., title="The ID of the Player"),
player_id: int = Path(..., title=PLAYER_TITLE),
async_session: AsyncSession = Depends(generate_async_session),
):
"""
Expand All @@ -119,13 +121,13 @@ async def get_by_id_async(
HTTPException: Not found error if the Player with the specified ID does not
exist.
"""
player = await player_service.retrieve_by_id_async(async_session, player_id)
if not player:
player_res = await player.retrieve_by_id_async(async_session, player_id)
if not player_res:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
return player
return player_res


@api_router.get(
@router.get(
"/players/squadnumber/{squad_number}",
response_model=PlayerModel,
status_code=status.HTTP_200_OK,
Expand All @@ -150,25 +152,25 @@ async def get_by_squad_number_async(
HTTPException: HTTP 404 Not Found error if the Player with the specified
Squad Number does not exist.
"""
player = await player_service.retrieve_by_squad_number_async(
player_res = await player.retrieve_by_squad_number_async(
async_session, squad_number
)
if not player:
if not player_res:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
return player
return player_res


# PUT --------------------------------------------------------------------------


@api_router.put(
@router.put(
"/players/{player_id}",
status_code=status.HTTP_204_NO_CONTENT,
summary="Updates an existing Player",
tags=["Players"],
)
async def put_async(
player_id: int = Path(..., title="The ID of the Player"),
player_id: int = Path(..., title=PLAYER_TITLE),
player_model: PlayerModel = Body(...),
async_session: AsyncSession = Depends(generate_async_session),
):
Expand All @@ -185,24 +187,24 @@ async def put_async(
HTTPException: HTTP 404 Not Found error if the Player with the specified ID
does not exist.
"""
player = await player_service.retrieve_by_id_async(async_session, player_id)
if not player:
player_res = await player.retrieve_by_id_async(async_session, player_id)
if not player_res:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
await player_service.update_async(async_session, player_model)
await player.update_async(async_session, player_model)
await simple_memory_cache.clear(CACHE_KEY)


# DELETE -----------------------------------------------------------------------


@api_router.delete(
@router.delete(
"/players/{player_id}",
status_code=status.HTTP_204_NO_CONTENT,
summary="Deletes an existing Player",
tags=["Players"],
)
async def delete_async(
player_id: int = Path(..., title="The ID of the Player"),
player_id: int = Path(..., title=PLAYER_TITLE),
async_session: AsyncSession = Depends(generate_async_session),
):
"""
Expand All @@ -216,8 +218,8 @@ async def delete_async(
HTTPException: HTTP 404 Not Found error if the Player with the specified ID
does not exist.
"""
player = await player_service.retrieve_by_id_async(async_session, player_id)
if not player:
player_res = await player.retrieve_by_id_async(async_session, player_id)
if not player_res:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
await player_service.delete_async(async_session, player_id)
await player.delete_async(async_session, player_id)
await simple_memory_cache.clear(CACHE_KEY)
0 services/__init__.py → app/schemas/__init__.py
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion schemas/player_schema.py → app/schemas/player.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"""

from sqlalchemy import Column, String, Integer, Boolean
from databases.player_database import Base
from app.databases.player import Base


class Player(Base):
Expand Down
Empty file added app/services/__init__.py
View file
Open in desktop
Empty file.
4 changes: 2 additions & 2 deletions services/player_service.py → app/services/player.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.exc import SQLAlchemyError
from models.player_model import PlayerModel
from schemas.player_schema import Player
from app.models.player import PlayerModel
from app.schemas.player import Player

# Create -----------------------------------------------------------------------

Expand Down
Binary file modified storage/players-sqlite3.db
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletion tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import warnings
import pytest
from fastapi.testclient import TestClient
from main import app
from app.main import app

# Suppress the DeprecationWarning from httpx
warnings.filterwarnings("ignore", category=DeprecationWarning)
Expand Down
Loading