Update with optimistic locking

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,17 @@ 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
+```
+
+## Generating an Update Conflict
+
+We can try to use the xargs tool to submit multiple requests to our webserver. Hopefully this results in the 409 that we are looking to create.
+
+```bash
+xargs -I % -P8 curl -X PATCH -d '{"runtime": "97 mins"}' "localhost:5002/v1/movies/4" < <(printf '%s\n' {1..8})
+```

cmd/api/errors.go

@@ -64,3 +64,8 @@ func (app *application) badRequestResponse(w http.ResponseWriter, r *http.Reques
 func (app *application) failedValidationResponse(w http.ResponseWriter, r *http.Request, errors map[string]string) {
 	app.errorResponse(w, r, http.StatusUnprocessableEntity, errors)
 }
+
+func (app *application) editConflictResponse(w http.ResponseWriter, r *http.Request) {
+	message := "unable to update the record due to an edit conflict, please review new version and try again"
+	app.errorResponse(w, r, http.StatusConflict, message)
+}

cmd/api/handlers.go

@@ -122,11 +122,20 @@ func (app *application) updateMovieHandler(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
+	// var input struct {
+	// 	Title   string       `json:"title"`
+	// 	Year    int32        `json:"year"`
+	// 	Runtime data.Runtime `json:"runtime"`
+	// 	Genres  []string     `json:"genres"`
+	// }
+
+	// Choosing to use pointers to allow us to determine if the zero value was sent
+	// by the client or if it was excluded from the client submission
 	var input struct {
-		Title   string       `json:"title"`
-		Year    int32        `json:"year"`
-		Runtime data.Runtime `json:"runtime"`
-		Genres  []string     `json:"genres"`
+		Title   *string       `json:"title"`
+		Year    *int32        `json:"year"`
+		Runtime *data.Runtime `json:"runtime"`
+		Genres  []string      `json:"genres"`
 	}
 
 	err = app.readJSON(w, r, &input)
@@ -135,10 +144,19 @@ func (app *application) updateMovieHandler(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	movie.Title = input.Title
-	movie.Year = input.Year
-	movie.Runtime = input.Runtime
-	movie.Genres = input.Genres
+	if input.Title != nil {
+		movie.Title = *input.Title
+	}
+	if input.Year != nil {
+		movie.Year = *input.Year
+	}
+	if input.Runtime != nil {
+		movie.Runtime = *input.Runtime
+	}
+
+	if input.Genres != nil {
+		movie.Genres = input.Genres
+	}
 
 	v := validator.New()
 
@@ -149,7 +167,12 @@ func (app *application) updateMovieHandler(w http.ResponseWriter, r *http.Reques
 
 	err = app.models.Movies.Update(r.Context(), movie)
 	if err != nil {
-		app.serverErrorResponse(w, r, err)
+		switch {
+		case errors.Is(err, data.ErrEditConflict):
+			app.editConflictResponse(w, r)
+		default:
+			app.serverErrorResponse(w, r, err)
+		}
 		return
 	}
 
@@ -167,7 +190,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 +214,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) {

cmd/api/routes.go

@@ -25,7 +25,7 @@ func (app *application) routes() http.Handler {
 	router.HandlerFunc(http.MethodGet, "/v1/healthcheck", app.healthCheckHandler)
 	router.HandlerFunc(http.MethodPost, "/v1/movies", app.createMovieHandler)
 	router.HandlerFunc(http.MethodGet, "/v1/movies/:id", app.getMovieHandler)
-	router.HandlerFunc(http.MethodPut, "/v1/movies/:id", app.updateMovieHandler)
+	router.HandlerFunc(http.MethodPatch, "/v1/movies/:id", app.updateMovieHandler)
 	router.HandlerFunc(http.MethodDelete, "/v1/movies/:id", app.deleteMovieHandler)
 
 	// middleware

internal/data/models.go

@@ -8,6 +8,7 @@ import (
 
 var (
 	ErrRecordNotFound = errors.New("record not found")
+	ErrEditConflict   = errors.New("edit conflict")
 )
 
 // Models is a wrapper around all model repository objects

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"
 )
 
@@ -80,10 +81,17 @@ func (m MovieModel) Get(ctx context.Context, id int64) (*Movie, error) {
 }
 
 func (m MovieModel) Update(ctx context.Context, movie *Movie) error {
+	// Using version here as an optimistic lock. Look up optimistic vs pessimistic locking
+	// https://stackoverflow.com/questions/129329/optimistic-vs-pessimistic-locking/129397#129397
+
+	// version incrementing with an integer is cheap. Don't try with a last_updated timestamp...that could
+	// cause issues. If you don't want version to be guessable then a UUID generated by the DB is suitable.
+	// Example: SET ... version = uuid_generate_v4()
+
 	query := `
 		UPDATE movies 
 		SET title = $1, year = $2, runtime = $3, genres = $4, version = version + 1
-		WHERE id = $5
+		WHERE id = $5 and version = $6
 		RETURNING version	
 	`
 
@@ -93,12 +101,35 @@ func (m MovieModel) Update(ctx context.Context, movie *Movie) error {
 		movie.Runtime,
 		movie.Genres,
 		movie.ID,
+		movie.Version,
 	}
 
-	return m.pool.QueryRow(ctx, query, args...).Scan(&movie.Version)
+	// Will not return any rows if the version number has already changed.
+	err := m.pool.QueryRow(ctx, query, args...).Scan(&movie.Version)
+	if err != nil {
+		switch {
+		case errors.Is(err, pgx.ErrNoRows):
+			return ErrEditConflict
+		default:
+			return err
+		}
+	}
+
+	return nil
 }
 
-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 +137,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
 }