diff --git a/handler-details.go b/handler-details.go index b176b376f016662d8354e176a9ee043909c0e908..30813c5ab3dd6133fe6bc10c3d3c73d3a6b692c1 100644 --- a/handler-details.go +++ b/handler-details.go @@ -15,6 +15,7 @@ import ( "git.c3pb.de/gbe/invinoveritas/auth" "git.c3pb.de/gbe/invinoveritas/log" + "git.c3pb.de/gbe/invinoveritas/vino" ) func (h Handler) img() http.HandlerFunc { @@ -32,7 +33,7 @@ func (h Handler) img() http.HandlerFunc { return } - img, err := LoadPictureData(r.Context(), h.db, id) + img, err := h.storage.LoadPictureData(r.Context(), id) if err != nil { httpError(w, r, "can't load image", err, http.StatusNotFound) return @@ -64,7 +65,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { return } - err = DeleteComment(r.Context(), h.db, commentID) + err = h.storage.DeleteComment(r.Context(), commentID) if err != nil { httpError(w, r, "internal server error", err, http.StatusInternalServerError) return @@ -92,7 +93,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { } if action == "delete" { - err = DeleteVino(r.Context(), h.db, id) + err = h.storage.DeleteVino(r.Context(), id) if err != nil { httpError(w, r, "can't delete wine", err, http.StatusInternalServerError) return @@ -102,7 +103,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { return } - v, err := LoadVino(r.Context(), h.db, id) + v, err := h.storage.LoadVino(r.Context(), id) if err != nil { // Only log error here, just create a new entry. level.Error(log.Get(r)). @@ -123,7 +124,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { return } - country, err := ISO2CountryCodeFromString(r.FormValue("country")) + country, err := vino.ISO2CountryCodeFromString(r.FormValue("country")) if err != nil { httpError(w, r, "can't parse country", nil, http.StatusBadRequest) return @@ -139,7 +140,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { comment = comment[:1024] + "..." } - err = v.StoreComment(r.Context(), h.db, comment) + err = h.storage.StoreComment(r.Context(), &v, comment) if err != nil { httpError(w, r, "can't add comment", err, http.StatusInternalServerError) return @@ -168,7 +169,7 @@ func (h Handler) detailsPost(w http.ResponseWriter, r *http.Request) { } } - err = v.Store(r.Context(), h.db) + err = h.storage.Store(r.Context(), &v) if err != nil { httpError(w, r, "can't store wine", err, http.StatusInternalServerError) return @@ -184,7 +185,7 @@ func (h Handler) details() http.Handler { ) type templateData struct { - Wine Vino + Wine vino.Vino User auth.User } @@ -211,7 +212,7 @@ func (h Handler) details() http.Handler { return } - v, err := LoadVino(r.Context(), h.db, id) + v, err := h.storage.LoadVino(r.Context(), id) if err != nil { httpError(w, r, "can't load wine data", err, http.StatusInternalServerError) return diff --git a/handler-index.go b/handler-index.go index d02402939b5c01af177a7b0733a46bb7b7a281f4..33006f1096dacd026f18e54643818b295b91262e 100644 --- a/handler-index.go +++ b/handler-index.go @@ -12,6 +12,7 @@ import ( "git.c3pb.de/gbe/invinoveritas/auth" "git.c3pb.de/gbe/invinoveritas/log" + "git.c3pb.de/gbe/invinoveritas/vino" ) func (h Handler) index() http.Handler { @@ -21,7 +22,7 @@ func (h Handler) index() http.Handler { ) type templateData struct { - Wines []Vino + Wines []vino.Vino User auth.User } @@ -31,7 +32,7 @@ func (h Handler) index() http.Handler { }) if r.Method == "GET" { - wines, err := ListWines(r.Context(), h.db) + wines, err := h.storage.ListWines(r.Context()) if err != nil { httpError(w, r, "can't list wines", err, http.StatusInternalServerError) return @@ -82,17 +83,16 @@ func (h Handler) index() http.Handler { err = r.ParseMultipartForm(16 * 1024 * 1024) if err != nil { - httpError(w, r, "can't parse multipart form data", err, http.StatusInternalServerError) - return + level.Warn(log.Get(r)).Log("error", err, "msg", "can't parse multipart form data") } - vino := Vino{ + vino := vino.Vino{ Name: name, Rating: rating, } // See if there's an image file uploaded - if len(r.MultipartForm.File["picture"]) != 0 { + if r.MultipartForm != nil && len(r.MultipartForm.File["picture"]) != 0 { picFile, picHdr, err := r.FormFile("picture") if err != nil { httpError(w, r, "can't open picture file", err, http.StatusInternalServerError) @@ -107,7 +107,7 @@ func (h Handler) index() http.Handler { } } - err = vino.Store(r.Context(), h.db) + err = h.storage.Store(r.Context(), &vino) if err != nil { httpError(w, r, fmt.Sprintf("can't store wine %q", vino), err, http.StatusInternalServerError) return diff --git a/handler-index_test.go b/handler-index_test.go new file mode 100644 index 0000000000000000000000000000000000000000..c0d9c49ef424668e4216adf8acd42b9d9c4cd0f3 --- /dev/null +++ b/handler-index_test.go @@ -0,0 +1,41 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "git.c3pb.de/gbe/invinoveritas/log" +) + +type testLogger struct { + t *testing.T +} + +func (tl testLogger) Log(vals ...interface{}) error { + tl.t.Log(vals...) + return nil +} + +func TestHandler_Index_Add(t *testing.T) { + w := httptest.NewRecorder() + + data := make(url.Values) + data.Set("name", "unit-test") + + r := httptest.NewRequest("POST", "/add", strings.NewReader(data.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + h := Handler{} + + logger := testLogger{t} + + hdlr := log.Request(h.index(), logger) + hdlr.ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Error("unexpected HTTP status code:", w.Code) + } +} diff --git a/main.go b/main.go index 04fa2d165c2d514cbe111524d70e17d3eb7a2ac0..ae25b3e5e46b4cf5e8a194d271fc270be9759c07 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,8 @@ import ( "git.c3pb.de/gbe/invinoveritas/auth" "git.c3pb.de/gbe/invinoveritas/log" "git.c3pb.de/gbe/invinoveritas/session" + "git.c3pb.de/gbe/invinoveritas/storage" + "git.c3pb.de/gbe/invinoveritas/vino" ) //go:embed templates/*.tpl @@ -58,7 +60,22 @@ type userProvider interface { UpdatePassword(ctx context.Context, name string, oldPW, newPW string) error } +type Storage interface { + DeleteComment(ctx context.Context, id int) error + DeleteVino(ctx context.Context, id int) error + + ListWines(ctx context.Context) ([]vino.Vino, error) + + LoadVino(ctx context.Context, id int) (vino.Vino, error) + LoadPictureData(ctx context.Context, id int) ([]byte, error) + + StoreComment(ctx context.Context, v *vino.Vino, txt string) error + Store(ctx context.Context, v *vino.Vino) error +} + type Handler struct { + storage Storage + db *sqlx.DB sp sessionProvider up userProvider @@ -167,6 +184,10 @@ func main() { } handler := Handler{ + storage: storage.Backend{ + DB: db, + }, + db: db, sp: sessions, up: sessions, diff --git a/vino.go b/storage/storage.go similarity index 59% rename from vino.go rename to storage/storage.go index 7885c09b491df66ce7114447caa16e5c33957965..7b93892238272732cbedae744c909b1bb233a120 100644 --- a/vino.go +++ b/storage/storage.go @@ -1,38 +1,31 @@ -package main +package storage import ( "bytes" "context" "database/sql" - "database/sql/driver" "errors" "fmt" "image" "image/png" - "io" "sort" - // Imported for side effects to register format handlers - _ "image/jpeg" - _ "image/png" - + "git.c3pb.de/gbe/invinoveritas/log" + "git.c3pb.de/gbe/invinoveritas/vino" "github.com/Masterminds/squirrel" "github.com/go-kit/kit/log/level" "github.com/jmoiron/sqlx" "golang.org/x/image/draw" - - "git.c3pb.de/gbe/invinoveritas/log" ) var errNotFound = errors.New("not found") -type Comment struct { - ID int `db:"id"` - Content string `db:"content"` +type Backend struct { + DB *sqlx.DB } -func DeleteComment(ctx context.Context, db *sqlx.DB, id int) (err error) { - tx, err := db.Begin() +func (b Backend) DeleteComment(ctx context.Context, id int) (err error) { + tx, err := b.DB.Begin() if err != nil { return err } @@ -59,84 +52,8 @@ func DeleteComment(ctx context.Context, db *sqlx.DB, id int) (err error) { return nil } -type ISO2CountryCode [2]byte - -var UnknownCountry = ISO2CountryCode{'X', 'X'} - -func ISO2CountryCodeFromString(s string) (ISO2CountryCode, error) { - if len(s) == 0 { - return UnknownCountry, nil - } - - if len(s) != 2 { - return UnknownCountry, errors.New("invalid length") - } - - return ISO2CountryCode{s[0], s[1]}, nil -} - -func (i ISO2CountryCode) String() string { - if i[0] == 0 || i[1] == 0 { - return "XX" - } - - return fmt.Sprintf("%c%c", i[0], i[1]) -} - -func (i *ISO2CountryCode) UnmarshalBinary(d []byte) error { - if len(d) == 0 { - *i = UnknownCountry - return nil - } - - if len(d) != 2 { - *i = UnknownCountry - return nil - } - - copy(i[:], d) - - return nil -} - -func (i ISO2CountryCode) Value() (driver.Value, error) { - return i.String(), nil -} - -func (i *ISO2CountryCode) Scan(data interface{}) error { - var raw string - - switch data := data.(type) { - case string: - raw = data - case []byte: - raw = string(data) - default: - return fmt.Errorf("can't convert from %T", data) - } - - c, err := ISO2CountryCodeFromString(raw) - if err != nil { - return err - } - - *i = c - - return nil -} - -type Vino struct { - ID int `db:"id"` - Name string `db:"name"` - Rating int `db:"rating"` - Country ISO2CountryCode `db:"country"` - Picture image.Image `db:"-"` - HasPicture bool `db:"has_picture"` // Set to true if there's picture data for this vino - Comments []Comment `db:"-"` -} - -func DeleteVino(ctx context.Context, db *sqlx.DB, id int) (err error) { - tx, err := db.Begin() +func (b Backend) DeleteVino(ctx context.Context, id int) (err error) { + tx, err := b.DB.Begin() if err != nil { return err } @@ -169,42 +86,53 @@ func DeleteVino(ctx context.Context, db *sqlx.DB, id int) (err error) { return nil } -func (v *Vino) loadComments(ctx context.Context, tx *sqlx.Tx) error { - err := tx.SelectContext(ctx, &v.Comments, ` - SELECT id() as id, content - FROM comments - WHERE wine = ?1`, v.ID) +func (b Backend) ListWines(ctx context.Context) ([]vino.Vino, error) { + var wines []vino.Vino + + tx, err := b.DB.Beginx() if err != nil { - return err + return nil, err } + defer tx.Rollback() //nolint:errcheck // No write intended in this tx, it's only for read consistency - return nil -} - -// loadVino uses the given read-only bolt transaction to load the data for the wine with the given id. When -// there are no wines at all, or there is no wine with the given ID, loadVino returns errNotFound. -func loadVino(ctx context.Context, tx *sqlx.Tx, id int) (Vino, error) { - var v Vino - err := tx.GetContext(ctx, &v, ` + err = tx.SelectContext(ctx, &wines, ` SELECT id() as id, name, rating, country, picture IS NOT NULL AS has_picture - FROM wines - WHERE id() = ?1`, id) + FROM wines`) if err != nil { - return v, err + return nil, err } - err = v.loadComments(ctx, tx) + // Load comments + for _, v := range wines { + err = loadComments(ctx, &v, tx) + if err != nil { + return nil, err + } + } + + sort.Slice(wines, func(i, j int) bool { + return wines[i].Rating > wines[j].Rating + }) + + return wines, nil +} + +func loadComments(ctx context.Context, v *vino.Vino, tx *sqlx.Tx) error { + err := tx.SelectContext(ctx, &v.Comments, ` + SELECT id() as id, content + FROM comments + WHERE wine = ?1`, v.ID) if err != nil { - return v, err + return err } - return v, nil + return nil } -func LoadVino(ctx context.Context, db *sqlx.DB, id int) (Vino, error) { - tx, err := db.Beginx() +func (b Backend) LoadVino(ctx context.Context, id int) (vino.Vino, error) { + tx, err := b.DB.Beginx() if err != nil { - return Vino{}, err + return vino.Vino{}, err } defer tx.Rollback() //nolint:errcheck // No change in tx intended, it's only used for read consistency @@ -216,40 +144,30 @@ func LoadVino(ctx context.Context, db *sqlx.DB, id int) (Vino, error) { return v, nil } -func ListWines(ctx context.Context, db *sqlx.DB) ([]Vino, error) { - var wines []Vino - - tx, err := db.Beginx() - if err != nil { - return nil, err - } - defer tx.Rollback() //nolint:errcheck // No write intended in this tx, it's only for read consistency +// loadVino uses the given read-only bolt transaction to load the data for the wine with the given id. When +// there are no wines at all, or there is no wine with the given ID, loadVino returns errNotFound. +func loadVino(ctx context.Context, tx *sqlx.Tx, id int) (vino.Vino, error) { + var v vino.Vino - err = tx.SelectContext(ctx, &wines, ` + err := tx.GetContext(ctx, &v, ` SELECT id() as id, name, rating, country, picture IS NOT NULL AS has_picture - FROM wines`) + FROM wines + WHERE id() = ?1`, id) if err != nil { - return nil, err + return v, err } - // Load comments - for _, v := range wines { - err = v.loadComments(ctx, tx) - if err != nil { - return nil, err - } + err = loadComments(ctx, &v, tx) + if err != nil { + return v, err } - sort.Slice(wines, func(i, j int) bool { - return wines[i].Rating > wines[j].Rating - }) - - return wines, nil + return v, nil } -func LoadPictureData(ctx context.Context, db *sqlx.DB, id int) ([]byte, error) { +func (b Backend) LoadPictureData(ctx context.Context, id int) ([]byte, error) { var data []byte - err := db.GetContext(ctx, &data, `SELECT picture FROM wines WHERE id() = ?1`, id) + err := b.DB.GetContext(ctx, &data, `SELECT picture FROM wines WHERE id() = ?1`, id) if errors.Is(err, sql.ErrNoRows) { return nil, errNotFound } @@ -267,32 +185,28 @@ func LoadPictureData(ctx context.Context, db *sqlx.DB, id int) ([]byte, error) { return data, nil } -// AddPicture loads picture data (PNG or JPEG) from fh and sets v's picture to it. -// If something goes wrong during loading, or the image is neither PNG nor JPEG, an error -// is returned. If contentType is not the empty string, it is validated to be either -// image/png or image/jpeg. -func (v *Vino) AddPicture(fh io.Reader, contentType string) error { - switch contentType { - case "", "image/jpeg", "image/png": - default: - return fmt.Errorf("unexpected content type for image: %q", contentType) - } - - img, _, err := image.Decode(fh) +func (b Backend) StoreComment(ctx context.Context, v *vino.Vino, text string) (err error) { + tx, err := b.DB.Begin() if err != nil { return err } - v.Picture = img + _, err = tx.ExecContext(ctx, `INSERT INTO comments (wine, content) VALUES (?1, ?2)`, v.ID, text) + if err != nil { + rerr := tx.Rollback() + if rerr != nil { + level.Error(log.GetContext(ctx)). + Log("error", rerr, + "msg", "can't roll back transaction") + } - return nil -} + return err + } -func (v Vino) String() string { - return fmt.Sprintf("{Name: %q, Rating: %d}", v.Name, v.Rating) + return tx.Commit() } -func (v *Vino) Store(ctx context.Context, db *sqlx.DB) (err error) { +func (b Backend) Store(ctx context.Context, v *vino.Vino) (err error) { // Encode scaled image as PNG, will contain image data if there is any values := map[string]interface{}{ @@ -352,7 +266,7 @@ func (v *Vino) Store(ctx context.Context, db *sqlx.DB) (err error) { return err } - tx, err := db.Beginx() + tx, err := b.DB.Beginx() if err != nil { return err } @@ -388,24 +302,3 @@ func (v *Vino) Store(ctx context.Context, db *sqlx.DB) (err error) { return nil } - -func (v *Vino) StoreComment(ctx context.Context, db *sqlx.DB, text string) (err error) { - tx, err := db.Begin() - if err != nil { - return err - } - - _, err = tx.ExecContext(ctx, `INSERT INTO comments (wine, content) VALUES (?1, ?2)`, v.ID, text) - if err != nil { - rerr := tx.Rollback() - if rerr != nil { - level.Error(log.GetContext(ctx)). - Log("error", rerr, - "msg", "can't roll back transaction") - } - - return err - } - - return tx.Commit() -} diff --git a/vino/vino.go b/vino/vino.go new file mode 100644 index 0000000000000000000000000000000000000000..9e07f3b673bbd55d1b103f5143ac70d5c9558844 --- /dev/null +++ b/vino/vino.go @@ -0,0 +1,119 @@ +package vino + +import ( + "database/sql/driver" + "errors" + "fmt" + "image" + "io" + + // Imported for side effects to register format handlers + _ "image/jpeg" + _ "image/png" +) + +type Comment struct { + ID int `db:"id"` + Content string `db:"content"` +} + +type ISO2CountryCode [2]byte + +var UnknownCountry = ISO2CountryCode{'X', 'X'} + +func ISO2CountryCodeFromString(s string) (ISO2CountryCode, error) { + if len(s) == 0 { + return UnknownCountry, nil + } + + if len(s) != 2 { + return UnknownCountry, errors.New("invalid length") + } + + return ISO2CountryCode{s[0], s[1]}, nil +} + +func (i ISO2CountryCode) String() string { + if i[0] == 0 || i[1] == 0 { + return "XX" + } + + return fmt.Sprintf("%c%c", i[0], i[1]) +} + +func (i *ISO2CountryCode) UnmarshalBinary(d []byte) error { + if len(d) == 0 { + *i = UnknownCountry + return nil + } + + if len(d) != 2 { + *i = UnknownCountry + return nil + } + + copy(i[:], d) + + return nil +} + +func (i ISO2CountryCode) Value() (driver.Value, error) { + return i.String(), nil +} + +func (i *ISO2CountryCode) Scan(data interface{}) error { + var raw string + + switch data := data.(type) { + case string: + raw = data + case []byte: + raw = string(data) + default: + return fmt.Errorf("can't convert from %T", data) + } + + c, err := ISO2CountryCodeFromString(raw) + if err != nil { + return err + } + + *i = c + + return nil +} + +type Vino struct { + ID int `db:"id"` + Name string `db:"name"` + Rating int `db:"rating"` + Country ISO2CountryCode `db:"country"` + Picture image.Image `db:"-"` + HasPicture bool `db:"has_picture"` // Set to true if there's picture data for this vino + Comments []Comment `db:"-"` +} + +// AddPicture loads picture data (PNG or JPEG) from fh and sets v's picture to it. +// If something goes wrong during loading, or the image is neither PNG nor JPEG, an error +// is returned. If contentType is not the empty string, it is validated to be either +// image/png or image/jpeg. +func (v *Vino) AddPicture(fh io.Reader, contentType string) error { + switch contentType { + case "", "image/jpeg", "image/png": + default: + return fmt.Errorf("unexpected content type for image: %q", contentType) + } + + img, _, err := image.Decode(fh) + if err != nil { + return err + } + + v.Picture = img + + return nil +} + +func (v Vino) String() string { + return fmt.Sprintf("{Name: %q, Rating: %d}", v.Name, v.Rating) +}