Skip to content

Commit

Permalink
Merge pull request #32 from wednesday-solutions/feat/code_refactor
Browse files Browse the repository at this point in the history
Feat/code refactor according to sonar standards
  • Loading branch information
anasnadeemws authored Mar 15, 2024
2 parents 9dbe83f + 1291e48 commit effef37
Show file tree
Hide file tree
Showing 18 changed files with 166 additions and 97 deletions.
3 changes: 2 additions & 1 deletion app/config/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
from __future__ import annotations

from .redis_config import get_redis_pool


__all__ = [
"engine",
"get_redis_pool",
]
10 changes: 7 additions & 3 deletions app/daos/home.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
from __future__ import annotations

import asyncio
import random

from app.exceptions import ExternalServiceException


async def external_service_call():
# Simulate network delay
delay = random.uniform(0.1, 1.0) # Random delay between 0.1 to 1.0 seconds
delay = random.uniform(0.1, 1.0) # Random delay between 0.1 to 1.0 seconds #NOSONAR
await asyncio.sleep(delay)

# Simulate occasional failures
if random.random() < 0.2: # 20% chance of failure
raise Exception("External service failed")
if random.random() < 0.2: # 20% chance of failure #NOSONAR
raise ExternalServiceException("External Service Failed")

return "Success from external service"
67 changes: 41 additions & 26 deletions app/daos/users.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,37 @@
from __future__ import annotations

import json

from fastapi import HTTPException
from fastapi_pagination.ext.sqlalchemy import paginate
from sqlalchemy import select
from sqlalchemy.orm import Session
from werkzeug.security import check_password_hash

from app.constants import jwt_utils
from app.constants.messages.users import user_messages as messages
from app.exceptions import EmailAlreadyExistException
from app.exceptions import InvalidCredentialsException
from app.exceptions import MobileAlreadyExistException
from app.exceptions import NoUserFoundException
from app.models import User
from app.schemas.users.users_request import CreateUser, Login
from werkzeug.security import check_password_hash
from fastapi_pagination.ext.sqlalchemy import paginate
from sqlalchemy import select
from app.utils.user_utils import check_existing_field, responseFormatter
from app.wrappers.cache_wrappers import create_cache, retrieve_cache
from app.schemas.users.users_request import CreateUser
from app.schemas.users.users_request import Login
from app.utils.user_utils import check_existing_field
from app.utils.user_utils import response_formatter
from app.wrappers.cache_wrappers import create_cache
from app.wrappers.cache_wrappers import retrieve_cache


async def get_user(user_id: int, dbSession: Session):
async def get_user(user_id: int, db_session: Session):
try:
cache_key = f"user_{user_id}"
cached_user, expire = await retrieve_cache(cache_key)
cached_user, _ = await retrieve_cache(cache_key)
if cached_user:
return json.loads(cached_user)
# Check if the user already exists in the database
user = (
dbSession.query(User)
db_session.query(User)
.where(User.id == user_id)
.with_entities(
User.id,
Expand All @@ -35,7 +45,7 @@ async def get_user(user_id: int, dbSession: Session):
.first()
)
if not user:
raise Exception(messages["NO_USER_FOUND_FOR_ID"])
raise NoUserFoundException(messages['NO_USER_FOUND_FOR_ID'])

await create_cache(json.dumps(user._asdict(), default=str), cache_key, 60)
return user
Expand All @@ -45,12 +55,12 @@ async def get_user(user_id: int, dbSession: Session):
raise HTTPException(status_code=400, detail=f"{str(e)}")


def list_users(dbSession: Session):
def list_users(db_session: Session):
try:
query = select(User.id, User.name, User.email, User.mobile).order_by(User.created_at)

# Pass the Select object to the paginate function
users = paginate(dbSession, query=query)
users = paginate(db_session, query=query)

return users

Expand All @@ -60,54 +70,59 @@ def list_users(dbSession: Session):
raise HTTPException(status_code=400, detail=f"{str(e)}")


def create_user(data: CreateUser, dbSession: Session):
def create_user(data: CreateUser, db_session: Session):
try:
user_data = data.dict()
# Check if the email already exists in the db
email_exists = check_existing_field(dbSession=dbSession, model=User, field="email", value=user_data["email"])
email_exists = check_existing_field(db_session=db_session, model=User, field="email", value=user_data["email"])
if email_exists:
raise Exception(messages["EMAIL_ALREADY_EXIST"])
raise EmailAlreadyExistException(messages['EMAIL_ALREADY_EXIST'])

# Check if the mobile already exists in the db
mobile_exists = check_existing_field(dbSession=dbSession, model=User, field="mobile", value=user_data["mobile"])
mobile_exists = check_existing_field(
db_session=db_session,
model=User,
field="mobile",
value=user_data["mobile"],
)
if mobile_exists:
raise Exception(messages["MOBILE_ALREADY_EXIST"])
raise MobileAlreadyExistException(messages['MOBILE_ALREADY_EXIST'])

user = User(**user_data)

dbSession.add(user)
dbSession.commit()
dbSession.refresh(user)
db_session.add(user)
db_session.commit()
db_session.refresh(user)

return responseFormatter(messages["CREATED_SUCCESSFULLY"])
return response_formatter(messages["CREATED_SUCCESSFULLY"])

except Exception as e:
# Return a user-friendly error message to the client
raise HTTPException(status_code=400, detail=f"{str(e)}")


def login(data: Login, dbSession: Session):
def login(data: Login, db_session: Session):
try:
user_data = data.dict()

# Check if the course already exists in the db
user_details = (
dbSession.query(User)
db_session.query(User)
.where(
User.email == user_data["email"],
)
.first()
)

if not user_details:
raise Exception(messages["INVALID_CREDENTIALS"])
raise InvalidCredentialsException(messages['INVALID_CREDENTIALS'])

if not check_password_hash(user_details.password, user_data["password"]):
raise Exception(messages["INVALID_CREDENTIALS"])
raise InvalidCredentialsException(messages['INVALID_CREDENTIALS'])

del user_details.password
token = jwt_utils.create_access_token({"sub": user_details.email, "id": user_details.id})
return responseFormatter(messages["LOGIN_SUCCESSFULLY"], {"token": token})
return response_formatter(messages["LOGIN_SUCCESSFULLY"], {"token": token})

except Exception as e:
print(e)
Expand Down
24 changes: 24 additions & 0 deletions app/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class ExternalServiceException(Exception):
pass


class NoUserFoundException(Exception):
pass
class EmailAlreadyExistException(Exception):
pass

class MobileAlreadyExistException(Exception):
pass

class InvalidCredentialsException(Exception):
pass

class CentryTestException(Exception):
pass

class DatabaseConnectionException(Exception):
pass


class RedisUrlNotFoundException(Exception):
pass
13 changes: 8 additions & 5 deletions app/middlewares/cache_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@

class CacheMiddleware(BaseHTTPMiddleware):
def __init__(
self,
app,
cached_endpoints: list[str],
self,
app,
cached_endpoints: list[str],
):
super().__init__(app)
self.cached_endpoints = cached_endpoints
Expand All @@ -28,6 +28,10 @@ def matches_any_path(self, path_url):
return True
return False

async def handle_max_age(self, max_age, response_body, key):
if max_age:
await create_cache(response_body[0].decode(), key, max_age)

async def dispatch(self, request: Request, call_next) -> Response:
path_url = request.url.path
request_type = request.method
Expand Down Expand Up @@ -58,8 +62,7 @@ async def dispatch(self, request: Request, call_next) -> Response:
max_age_match = re.search(r"max-age=(\d+)", cache_control)
if max_age_match:
max_age = int(max_age_match.group(1))
if max_age:
await create_cache(response_body[0].decode(), key, max_age)
await self.handle_max_age(max_age, response_body, key)
elif matches:
await create_cache(response_body[0].decode(), key, max_age)
return response
2 changes: 1 addition & 1 deletion app/middlewares/rate_limiter_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ async def dispatch(self, request: Request, call_next):
pipe.expire(client_ip, TIME_WINDOW)
await pipe.execute()
finally:
pass
print("Finally Block in Rate Limit exceeded")

response = await call_next(request)
return response
6 changes: 4 additions & 2 deletions app/middlewares/request_id_injection.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
from __future__ import annotations

import contextvars
import uuid

from fastapi.responses import JSONResponse
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.requests import Request
from fastapi.responses import JSONResponse

request_id_contextvar = contextvars.ContextVar("request_id", default=None)


class RequestIdInjection(BaseHTTPMiddleware):
def dispatch(self, request: Request, call_next):
request_id = str(uuid.uuid4())
request_id_contextvar.set(request_id)
request_id_contextvar.set(request_id) # noqa
try:
return call_next(request)

Expand Down
10 changes: 1 addition & 9 deletions app/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,4 @@
# import glob
# from os.path import basename
# from os.path import dirname
# from os.path import isfile
# from os.path import join

# modules = glob.glob(join(dirname(__file__), "*.py"))

# __all__ = [basename(f)[:-3] for f in modules if isfile(f) and not f.endswith("__init__.py")]
from __future__ import annotations

from .users import User

Expand Down
9 changes: 6 additions & 3 deletions app/models/users.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
from sqlalchemy import Column, event
from __future__ import annotations

from sqlalchemy import Column
from sqlalchemy import event
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.sql import func
from sqlalchemy.sql.sqltypes import DateTime
from sqlalchemy.sql.sqltypes import Integer
from sqlalchemy.sql.sqltypes import String
from sqlalchemy.ext.declarative import declarative_base
from werkzeug.security import generate_password_hash

from app.sessions.db import engine

Base = declarative_base()


class User(Base):
class User(Base): # noqa
__tablename__ = "user"

id = Column(Integer, primary_key=True, index=True)
Expand Down
2 changes: 1 addition & 1 deletion app/routes/cache_router/cache_samples.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
@cache_sample_router.get("/get-cache", tags=["Cache-Sample"])
def get_cache():
print("Request ID:", request_id_contextvar.get())
response = random.randint(100, 1000)
response = random.randint(100, 1000) #NOSONAR
return {"random value is": response}
15 changes: 10 additions & 5 deletions app/routes/home/home.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
from dependencies import circuit_breaker
from fastapi import APIRouter, HTTPException
from __future__ import annotations

from fastapi import APIRouter
from fastapi import HTTPException
from fastapi.responses import JSONResponse
from pybreaker import CircuitBreakerError

from app.config.base import settings
from app.middlewares.request_id_injection import request_id_contextvar
from app.daos.home import external_service_call
from pybreaker import CircuitBreakerError
from app.exceptions import CentryTestException
from app.middlewares.request_id_injection import request_id_contextvar
from dependencies import circuit_breaker

home_router = APIRouter()

Expand Down Expand Up @@ -32,7 +37,7 @@ async def external_service_endpoint():
def sentry_endpoint():
if not settings.SENTRY_DSN:
raise HTTPException(status_code=503, detail="Sentry DSN not found")
raise Exception("Testing Sentry")
raise CentryTestException("Centry Test")


@home_router.get("/{path:path}", include_in_schema=False)
Expand Down
Loading

1 comment on commit effef37

@github-actions
Copy link

Choose a reason for hiding this comment

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

Coverage

Coverage Report
FileStmtsMissCoverMissing
app
   app.py312423%10–61
   exceptions.py16160%1–24
app/config
   __init__.py3167%6
   base.py32391%44–46
   celery_config.py16160%1–25
   celery_utils.py20200%1–28
   redis_config.py4325%2–6
app/constants
   jwt_utils.py15150%1–19
app/daos
   home.py10100%1–18
   users.py71710%1–130
app/middlewares
   cache_middleware.py52520%1–68
   rate_limiter_middleware.py24240%1–32
   request_id_injection.py17170%1–25
app/models
   __init__.py330%1–5
   users.py27270%1–38
app/routes
   __init__.py11110%1–13
app/routes/cache_router
   __init__.py220%1–3
   cache_samples.py11110%1–15
app/routes/celery_router
   __init__.py220%1–3
   celery_samples.py11110%1–14
app/routes/home
   __init__.py220%1–3
   home.py33330%1–45
app/routes/users
   __init__.py220%1–3
   users.py38380%1–57
app/schemas/users
   users_request.py41410%1–70
   users_response.py880%1–11
app/sessions
   db.py53530%1–82
app/tests
   test_basic.py181517%7–31
app/utils
   exception_handler.py19190%1–36
   redis_utils.py330%1–5
   slack_notification_utils.py13130%1–29
   user_utils.py25250%1–36
app/wrappers
   cache_wrappers.py19190%1–27
TOTAL6526106% 

Tests Skipped Failures Errors Time
1 0 💤 0 ❌ 1 🔥 0.573s ⏱️

Please sign in to comment.