Skip to content

🐛 [RTY-260012]: Duplicate Code #12

@recursivezero

Description

@recursivezero

What happened?

  • load_env(): Reduced from 3 calls to 1.
  • Redirect Route: Consolidate into main.py only.
  • Duplicate logic in create_short_url: Flattened the if/else tree so the session is updated only once at the end.
  • DB Re-connects: Removed the manual try: connect_db() checks inside every route; the lifespan ensures it's handled at boot.

What did you expect to happen?

You have some redundant logic and "racing" configurations that can be simplified. The biggest issue is that you are calling load_env() in every single file and manually managing database connections in multiple places.

Here are the specific redundant and unnecessary parts of your code:

1. Redundant load_env() Calls

You are calling load_env() in app/db/data.py, app/main.py, and app/api/fast_api.py.

  • The Fix: Call it once at the very top of your entry point (app/main.py). Since the other modules are imported by main.py, the environment variables will already be available to them.

2. Duplicate Redirection Logic

You have a /{short_code} redirect route in both app/main.py and app/api/fast_api.py.

  • The Redundancy: Since you are mounting the API app inside the main app (app.mount("/api", api_app)), having the same route in both places causes confusion and code duplication.
  • The Fix: Keep the redirection in the main app (app/main.py) for the frontend users, and remove it from the API file unless you want the API to specifically handle its own redirects under /api/v1/{short_code}.

3. "Connection on Import" Pattern

In app/db/data.py, you call connect_db() at the bottom of the file.

  • The Risk: This runs as soon as any file imports data.py. If the database is slow to respond, it can hang your app startup or cause issues during testing.
  • The Fix: Use FastAPI's Lifespan events to connect to the database once when the app starts.

4. Code Duplication in create_short_url

In app/main.py, inside create_short_url, you have the exact same logic (generating a code and updating the session) repeated three times: once for no-db mode, once if find_one fails, and once at the end.


Optimized Code Structure

1. Simplified app/db/data.py

Remove the connect_db() call from the bottom and handle the pymongo import safely.

import os
from typing import Any

# SAFE IMPORT: App won't crash if pymongo is missing
try:
    from pymongo import MongoClient
    from pymongo.errors import ServerSelectionTimeoutError
    MONGO_INSTALLED = True
except ImportError:
    MONGO_INSTALLED = False

client: Any = None
db: Any = None
urls: Any = None
url_stats: Any = None

def connect_db():
    global client, db, urls, url_stats
    
    if not MONGO_INSTALLED:
        print("⚠️ pymongo is not installed. Running in NO-DB mode.")
        return False

    MONGO_URI = os.getenv("MONGO_URI")
    DB_NAME = os.getenv("DATABASE_NAME", "tiny_url")

    if not MONGO_URI:
        return False

    try:
        client = MongoClient(MONGO_URI, serverSelectionTimeoutMS=2000)
        client.admin.command("ping")
        db = client[DB_NAME]
        urls = db["urls"]
        url_stats = db["url_stats"]
        return True
    except Exception:
        return False

2. Cleaned up app/main.py

Use the lifespan to connect once and clean up the redundancy.

from contextlib import asynccontextmanager
from app.db.data import connect_db, urls

@asynccontextmanager
async def lifespan(app: FastAPI):
    # Load env once at the start
    load_env() 
    # Connect once
    connect_db()
    yield

app = FastAPI(title="TinyURL", lifespan=lifespan)

# Simplify your creation logic to avoid repeating session.update()
@app.post("/", response_class=RedirectResponse)
async def create_short_url(request: Request, original_url: str = Form(""), ...):
    # 1. Validation Logic...
    
    # 2. Logic to get short_code
    short_code = None
    if urls is not None:
        try:
            existing = urls.find_one({"original_url": original_url})
            short_code = existing["short_code"] if existing else None
        except Exception:
            pass

    if not short_code:
        short_code = generate_code()
        # Save to DB if possible
        if urls is not None:
            try:
                urls.insert_one({"short_code": short_code, ...})
            except Exception:
                pass

    # 3. One single session update at the end
    request.session.update({"short_code": short_code, ...})
    return RedirectResponse("/", status_code=303)

Anything else we need to know?

No response

What browsers are you seeing the problem on?

No response

Relevant log output

Contact Details

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Metadata

Metadata

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions