Skip to content
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.

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

Solution #44

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
10 changes: 6 additions & 4 deletions src/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
class BaseAppSettings(BaseSettings):
BASE_DIR: Path = Path(__file__).parent.parent
PATH_TO_DB: str = str(BASE_DIR / "database" / "source" / "theater.db")
PATH_TO_MOVIES_CSV: str = str(BASE_DIR / "database" / "seed_data" / "imdb_movies.csv")
PATH_TO_MOVIES_CSV: str = str(
BASE_DIR / "database" / "seed_data" / "imdb_movies.csv"
)
Comment on lines +11 to +13

Choose a reason for hiding this comment

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

There is a syntax error in the path construction for PATH_TO_MOVIES_CSV. The path should not end with a slash, as it is not a directory. Ensure the path is correctly constructed to point to the imdb_movies.csv file.



class Settings(BaseAppSettings):
Expand All @@ -22,11 +24,11 @@ class Settings(BaseAppSettings):
class TestingSettings(BaseAppSettings):

def model_post_init(self, __context: dict[str, Any] | None = None) -> None:
object.__setattr__(self, 'PATH_TO_DB', ":memory:")
object.__setattr__(self, "PATH_TO_DB", ":memory:")
object.__setattr__(
self,
'PATH_TO_MOVIES_CSV',
str(self.BASE_DIR / "database" / "seed_data" / "test_data.csv")
"PATH_TO_MOVIES_CSV",
str(self.BASE_DIR / "database" / "seed_data" / "test_data.csv"),
)


Expand Down
Empty file added src/crud/__init__.py
Empty file.
120 changes: 120 additions & 0 deletions src/crud/movies.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
from typing import List

from fastapi import HTTPException
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.orm import Session

from database.models import (
MovieModel,
CountryModel,
GenreModel,
ActorModel,
LanguageModel,
)

from schemas.movies import MovieCreateSchema, MovieUpdateSchema


def get_movies(db: Session, offset, per_page):
return (
db.query(MovieModel)
.order_by(MovieModel.id.desc())
.offset(offset)
.limit(per_page)
.all()
)


def get_movie(db: Session, movie_id):
return db.query(MovieModel).filter(MovieModel.id == movie_id).first()


def create_objects(data: List[str], model, db: Session):
objects = [model(name=element) for element in data]
db.add_all(objects)
db.commit()

for obj in objects:
db.refresh(obj)

return objects


def create_movie(db: Session, movie: MovieCreateSchema):
try:

country_data = (
db.query(CountryModel).filter(CountryModel.code == movie.country).first()
)
if not country_data:
country_data = CountryModel(code=movie.country)
db.add(country_data)
db.commit()
db.refresh(country_data)

genres_data = (
db.query(GenreModel).filter(GenreModel.name.in_(movie.genres)).all()
)
if not genres_data:
genres_data = create_objects(movie.genres, GenreModel, db)
Comment on lines +58 to +59

Choose a reason for hiding this comment

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

The current logic for handling genres, actors, and languages may lead to data integrity issues. If any of these entities partially exist, the create_objects function will create duplicates for the non-existing ones. Consider modifying the logic to only create missing entities.


actors_data = (
db.query(ActorModel).filter(ActorModel.name.in_(movie.actors)).all()
)
if not actors_data:
actors_data = create_objects(movie.actors, ActorModel, db)
Comment on lines +64 to +65

Choose a reason for hiding this comment

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

Similar to the genres handling, the logic for actors may lead to duplicates if some actors already exist. Ensure that only missing actors are created.


languages_data = (
db.query(LanguageModel)
.filter(LanguageModel.name.in_(movie.languages))
.all()
)
if not languages_data:
languages_data = create_objects(movie.languages, LanguageModel, db)
Comment on lines +72 to +73

Choose a reason for hiding this comment

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

The logic for handling languages can also lead to duplicates if some languages already exist. Modify the logic to create only the missing languages.


db_movie = MovieModel(
name=movie.name,
date=movie.date,
score=movie.score,
overview=movie.overview,
status=movie.status,
budget=movie.budget,
revenue=movie.revenue,
country=country_data,
genres=genres_data,
actors=actors_data,
languages=languages_data,
)

db.add(db_movie)
db.commit()
db.refresh(db_movie)
return db_movie

except SQLAlchemyError:
db.rollback()
raise HTTPException(status_code=400, detail="Invalid input data.")
Comment on lines +94 to +96

Choose a reason for hiding this comment

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

Consider logging the exception details for better debugging and error tracking, rather than just raising a generic HTTPException.

Choose a reason for hiding this comment

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

Consider adding logging for the SQLAlchemyError to improve error tracking and debugging.



def delete_movie(db: Session, movie_id: int):
db_movie = db.query(MovieModel).filter(MovieModel.id == movie_id).first()
if not db_movie:
raise HTTPException(
status_code=404, detail="Movie with the given ID was not found."
)
db.delete(db_movie)
db.commit()


def update_movie(db: Session, movie_id: int, movie: MovieUpdateSchema):
db_movie = db.query(MovieModel).filter(MovieModel.id == movie_id).first()
if not db_movie:
raise HTTPException(
status_code=404, detail="Movie with the given ID was not found."
)

for key, value in movie.dict(exclude_unset=True).items():
setattr(db_movie, key, value)

db.commit()
db.refresh(db_movie)
6 changes: 3 additions & 3 deletions src/database/migrations/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from alembic import context

from database import models # noqa: F401
from database import models # noqa: F401
from database.models import Base
from database.session_postgresql import postgresql_engine

Expand Down Expand Up @@ -47,7 +47,7 @@ def run_migrations_offline() -> None:
connection=connection,
target_metadata=target_metadata,
compare_type=True,
compare_server_default=True
compare_server_default=True,

Choose a reason for hiding this comment

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

The run_migrations_offline function incorrectly uses postgresql_engine to establish a connection. In offline mode, you should configure the context with just a URL, not an engine. This needs to be corrected to ensure offline migrations work as intended.

)

with context.begin_transaction():
Expand All @@ -68,7 +68,7 @@ def run_migrations_online() -> None:
connection=connection,
target_metadata=target_metadata,
compare_type=True,
compare_server_default=True
compare_server_default=True,
)

with context.begin_transaction():
Expand Down
140 changes: 79 additions & 61 deletions src/database/migrations/versions/ea3a65568bd9_initial_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,92 +5,110 @@
Create Date: 2025-01-02 21:07:13.284837

"""

from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision: str = 'ea3a65568bd9'
revision: str = "ea3a65568bd9"
down_revision: Union[str, None] = None

Choose a reason for hiding this comment

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

The down_revision is set to None, which is appropriate for an initial migration. Ensure that subsequent migrations correctly reference this revision ID.

branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.create_table('actors',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('name', sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('name')
op.create_table(
"actors",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("name", sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("name"),
)
op.create_table('countries',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('code', sa.String(length=3), nullable=False),
sa.Column('name', sa.String(length=255), nullable=True),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('code')
op.create_table(
"countries",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("code", sa.String(length=3), nullable=False),
sa.Column("name", sa.String(length=255), nullable=True),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("code"),
)
op.create_table('genres',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('name', sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('name')
op.create_table(
"genres",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("name", sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("name"),
)
op.create_table('languages',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('name', sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('name')
op.create_table(
"languages",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("name", sa.String(length=255), nullable=False),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("name"),
)
op.create_table('movies',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('name', sa.String(length=255), nullable=False),
sa.Column('date', sa.Date(), nullable=False),
sa.Column('score', sa.Float(), nullable=False),
sa.Column('overview', sa.Text(), nullable=False),
sa.Column('status', sa.Enum('RELEASED', 'POST_PRODUCTION', 'IN_PRODUCTION', name='moviestatusenum'), nullable=False),
sa.Column('budget', sa.DECIMAL(precision=15, scale=2), nullable=False),
sa.Column('revenue', sa.Float(), nullable=False),
sa.Column('country_id', sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(['country_id'], ['countries.id'], ),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('name', 'date', name='unique_movie_constraint')
op.create_table(
"movies",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("name", sa.String(length=255), nullable=False),
sa.Column("date", sa.Date(), nullable=False),
sa.Column("score", sa.Float(), nullable=False),
sa.Column("overview", sa.Text(), nullable=False),
sa.Column(
"status",
sa.Enum(
"RELEASED", "POST_PRODUCTION", "IN_PRODUCTION", name="moviestatusenum"
),
nullable=False,
),
sa.Column("budget", sa.DECIMAL(precision=15, scale=2), nullable=False),
sa.Column("revenue", sa.Float(), nullable=False),
sa.Column("country_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(
["country_id"],
["countries.id"],
),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("name", "date", name="unique_movie_constraint"),

Choose a reason for hiding this comment

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

The unique constraint on the movies table ensures that no two movies can have the same name and date. This is a good practice to prevent duplicate entries.

)
op.create_table('actors_movies',
sa.Column('movie_id', sa.Integer(), nullable=False),
sa.Column('actor_id', sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(['actor_id'], ['actors.id'], ondelete='CASCADE'),
sa.ForeignKeyConstraint(['movie_id'], ['movies.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('movie_id', 'actor_id')
op.create_table(
"actors_movies",
sa.Column("movie_id", sa.Integer(), nullable=False),
sa.Column("actor_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(["actor_id"], ["actors.id"], ondelete="CASCADE"),
sa.ForeignKeyConstraint(["movie_id"], ["movies.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("movie_id", "actor_id"),
)
op.create_table('movies_genres',
sa.Column('movie_id', sa.Integer(), nullable=False),
sa.Column('genre_id', sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(['genre_id'], ['genres.id'], ondelete='CASCADE'),
sa.ForeignKeyConstraint(['movie_id'], ['movies.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('movie_id', 'genre_id')
op.create_table(
"movies_genres",
sa.Column("movie_id", sa.Integer(), nullable=False),
sa.Column("genre_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(["genre_id"], ["genres.id"], ondelete="CASCADE"),
sa.ForeignKeyConstraint(["movie_id"], ["movies.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("movie_id", "genre_id"),
)
op.create_table('movies_languages',
sa.Column('movie_id', sa.Integer(), nullable=False),
sa.Column('language_id', sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(['language_id'], ['languages.id'], ondelete='CASCADE'),
sa.ForeignKeyConstraint(['movie_id'], ['movies.id'], ondelete='CASCADE'),
sa.PrimaryKeyConstraint('movie_id', 'language_id')
op.create_table(
"movies_languages",
sa.Column("movie_id", sa.Integer(), nullable=False),
sa.Column("language_id", sa.Integer(), nullable=False),
sa.ForeignKeyConstraint(["language_id"], ["languages.id"], ondelete="CASCADE"),
sa.ForeignKeyConstraint(["movie_id"], ["movies.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("movie_id", "language_id"),
)
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.drop_table('movies_languages')
op.drop_table('movies_genres')
op.drop_table('actors_movies')
op.drop_table('movies')
op.drop_table('languages')
op.drop_table('genres')
op.drop_table('countries')
op.drop_table('actors')
op.drop_table("movies_languages")
op.drop_table("movies_genres")
op.drop_table("actors_movies")
op.drop_table("movies")
op.drop_table("languages")
op.drop_table("genres")
op.drop_table("countries")
op.drop_table("actors")
# ### end Alembic commands ###
Loading
Loading