diff --git a/NOTES.md b/NOTES.md index 9603952..546d9fb 100644 --- a/NOTES.md +++ b/NOTES.md @@ -5,6 +5,13 @@ send_interrupt = true ``` + +## Get Movie + +```bash +curl -i http://0.0.0.0:5002/v1/movies/1 +``` + ## Creating a movie ```bash @@ -19,3 +26,9 @@ curl -i -X POST -d '{"title":"The Batman","year":2022,"runtime":"177 mins", "gen ```bash curl -i -X POST -d '{"title":"Death of a Unicorn","year":2025,"runtime":"126 mins", "genres":["comedy","satire"]}' http://0.0.0.0:5002/v1/movies ``` + + +```bash +BODY='{"title":"Black Panther","year":2018,"runtime":"134 mins","genres":["sci-fi","action","adventure"]}' +curl -X PUT -d "$BODY" http://0.0.0.0:5002/v1/movies/1 +``` \ No newline at end of file diff --git a/cmd/api/handlers.go b/cmd/api/handlers.go index d8a8fad..f899ca4 100644 --- a/cmd/api/handlers.go +++ b/cmd/api/handlers.go @@ -167,7 +167,20 @@ func (app *application) deleteMovieHandler(w http.ResponseWriter, r *http.Reques } // Maybe should be passing a timeout context - _, err = app.models.Movies.Get(r.Context(), id) + // WE DON"T NEED THIS BECAUSE EXEC RETURNS A RESULT OBJECT WITH THE NUMBER + // OF ROWS AFFECTED. IF THE RESULT IS 0 THEN WE 404 + // _, err = app.models.Movies.Get(r.Context(), id) + // if err != nil { + // switch { + // case errors.Is(err, data.ErrRecordNotFound): + // app.notFoundResponse(w, r) + // default: + // app.serverErrorResponse(w, r, err) + // } + // return + // } + + err = app.models.Movies.Delete(r.Context(), id) if err != nil { switch { case errors.Is(err, data.ErrRecordNotFound): @@ -178,13 +191,15 @@ func (app *application) deleteMovieHandler(w http.ResponseWriter, r *http.Reques return } - err = app.models.Movies.Delete(r.Context(), id) + // I personally think this should be a 202 or a 204 but Alex thinks otherwise. + // his rule of thumb is if the clients are humans send human readable messages. If they + // are machines then status code is alright. + //w.WriteHeader(http.StatusAccepted) + + err = app.writeJSON(w, http.StatusOK, envelope{"message": "movie successfully deleted"}, nil) if err != nil { app.serverErrorResponse(w, r, err) - return } - - w.WriteHeader(http.StatusAccepted) } func (app *application) healthCheckHandler(w http.ResponseWriter, r *http.Request) { diff --git a/internal/data/movies.go b/internal/data/movies.go index cf61f1d..5b75bda 100644 --- a/internal/data/movies.go +++ b/internal/data/movies.go @@ -6,6 +6,7 @@ import ( "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" ) @@ -98,7 +99,18 @@ func (m MovieModel) Update(ctx context.Context, movie *Movie) error { return m.pool.QueryRow(ctx, query, args...).Scan(&movie.Version) } -func (m MovieModel) Delete(ctx context.Context, id int64) error { +// Here I decided to attempt an idiomatic version of the Delete service with +// a database transaction. Since this is updating just one entry in a single table +// an explicit db transaction isn't really necessary but I thought I'd practice it +// none the less. If using sqlc you'd probably not even worry about implementing +// it this way, but I am here to learn so. + +func (m MovieModel) Delete(ctx context.Context, id int64) (err error) { + + if id < 1 { + return ErrRecordNotFound + } + query := `DELETE FROM movies WHERE id = $1` tx, err := m.pool.BeginTx(ctx, pgx.TxOptions{}) @@ -106,9 +118,24 @@ func (m MovieModel) Delete(ctx context.Context, id int64) error { return err } defer func() { - tx.Rollback(ctx) + // If it's in error or the context was canceled, rollback + if err != nil || ctx.Err() != nil { + _ = tx.Rollback(ctx) + return + } + // if the commit fails raises the error. + err = tx.Commit(ctx) }() - _, err = tx.Exec(ctx, query, id) - return err + var cmd pgconn.CommandTag + cmd, err = tx.Exec(ctx, query, id) + if err != nil { + return err + } + + if cmd.RowsAffected() == 0 { + return ErrRecordNotFound + } + + return nil }