From 9d9c10044be1218e8d61c8d1c9cee7a8985ae335 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Sat, 29 Sep 2018 12:02:19 +0200 Subject: [PATCH] Improve attachment handling - flexible meta data struct for uploaded files - prevew generator (images only for now) - using outgoing types for return data (to cover attachment url generation in one place) --- internal/payload/outgoing.go | 21 +- internal/payload/outgoing/attachment.go | 17 +- .../schema/mysql/20180704080000.base.up.sql | 2 + sam/docs/README.md | 4 +- sam/docs/src/spec.json | 4 +- sam/docs/src/spec/attachment.json | 4 +- sam/repository/attachment.go | 24 ++- sam/rest/channel.go | 14 +- sam/rest/handlers/attachment.go | 4 +- sam/rest/handlers/attachment_custom.go | 2 + sam/service/attachment.go | 185 +++++++++++++----- sam/service/message.go | 1 - sam/types/attachment.go | 90 +++++++-- 13 files changed, 269 insertions(+), 103 deletions(-) diff --git a/internal/payload/outgoing.go b/internal/payload/outgoing.go index 22c5551a1..acd0bd161 100644 --- a/internal/payload/outgoing.go +++ b/internal/payload/outgoing.go @@ -1,11 +1,19 @@ package payload import ( + "fmt" + "net/url" + auth "github.com/crusttech/crust/auth/types" "github.com/crusttech/crust/internal/payload/outgoing" sam "github.com/crusttech/crust/sam/types" ) +const ( + attachmentURL = "/attachment/%d/original/%s" + attachmentPreviewURL = "/attachment/%d/preview.%s" +) + func Message(msg *sam.Message) *outgoing.Message { return &outgoing.Message{ ID: Uint64toa(msg.ID), @@ -85,13 +93,18 @@ func Attachment(in *sam.Attachment) *outgoing.Attachment { return nil } + var preview string + + if in.Meta.Preview != nil { + preview = fmt.Sprintf(attachmentPreviewURL, in.ID, in.Meta.Preview.Extension) + } + return &outgoing.Attachment{ ID: Uint64toa(in.ID), UserID: Uint64toa(in.UserID), - Url: in.Url, - PreviewUrl: in.PreviewUrl, - Size: in.Size, - Mimetype: in.Mimetype, + Url: fmt.Sprintf(attachmentURL, in.ID, url.PathEscape(in.Name)), + PreviewUrl: preview, + Meta: in.Meta, Name: in.Name, CreatedAt: in.CreatedAt, UpdatedAt: in.UpdatedAt, diff --git a/internal/payload/outgoing/attachment.go b/internal/payload/outgoing/attachment.go index 645188783..fc9ef700c 100644 --- a/internal/payload/outgoing/attachment.go +++ b/internal/payload/outgoing/attachment.go @@ -6,15 +6,14 @@ import ( type ( Attachment struct { - ID string `json:"ID"` - UserID string `json:"userID"` - Url string `json:"url"` - PreviewUrl string `json:"previewUrl"` - Size int64 `json:"size"` - Mimetype string `json:"mimetype"` - Name string `json:"name"` - CreatedAt time.Time `json:"createdAt,omitempty"` - UpdatedAt *time.Time `json:"updatedAt,omitempty"` + ID string `json:"ID"` + UserID string `json:"userID"` + Url string `json:"url"` + PreviewUrl string `json:"previewUrl,omitempty"` + Meta interface{} `json:"meta"` + Name string `json:"name"` + CreatedAt time.Time `json:"createdAt,omitempty"` + UpdatedAt *time.Time `json:"updatedAt,omitempty"` } AttachmentSet []*Attachment diff --git a/sam/db/schema/mysql/20180704080000.base.up.sql b/sam/db/schema/mysql/20180704080000.base.up.sql index 03dbce023..5c8b54c09 100644 --- a/sam/db/schema/mysql/20180704080000.base.up.sql +++ b/sam/db/schema/mysql/20180704080000.base.up.sql @@ -131,6 +131,8 @@ CREATE TABLE attachments ( mimetype VARCHAR(255), name TEXT, + meta JSON, + created_at DATETIME NOT NULL DEFAULT NOW(), updated_at DATETIME NULL, deleted_at DATETIME NULL, diff --git a/sam/docs/README.md b/sam/docs/README.md index 074978265..ea33e6e92 100644 --- a/sam/docs/README.md +++ b/sam/docs/README.md @@ -561,7 +561,7 @@ The following event types may be sent with a message event: | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/attachment/{attachmentID}/{name}` | HTTP/S | GET | Client ID, Session ID | +| `/attachment/{attachmentID}/original/{name}` | HTTP/S | GET | Client ID, Session ID | #### Request parameters @@ -575,7 +575,7 @@ The following event types may be sent with a message event: | URI | Protocol | Method | Authentication | | --- | -------- | ------ | -------------- | -| `/attachment/{attachmentID}/{name}/preview` | HTTP/S | GET | Client ID, Session ID | +| `/attachment/{attachmentID}/preview.{ext}` | HTTP/S | GET | Client ID, Session ID | #### Request parameters diff --git a/sam/docs/src/spec.json b/sam/docs/src/spec.json index 518955f6e..aafa7c87d 100644 --- a/sam/docs/src/spec.json +++ b/sam/docs/src/spec.json @@ -483,7 +483,7 @@ "apis": [ { "name": "original", - "path": "/{name}", + "path": "/original/{name}", "method": "GET", "title": "Serves attached file", @@ -495,7 +495,7 @@ }, { "name": "preview", - "path": "/{name}/preview", + "path": "/preview.{ext}", "method": "GET", "title": "Serves preview of an attached file" } diff --git a/sam/docs/src/spec/attachment.json b/sam/docs/src/spec/attachment.json index 5267c0f6d..6029d7491 100644 --- a/sam/docs/src/spec/attachment.json +++ b/sam/docs/src/spec/attachment.json @@ -24,7 +24,7 @@ "Name": "original", "Method": "GET", "Title": "Serves attached file", - "Path": "/{name}", + "Path": "/original/{name}", "Parameters": { "GET": [ { @@ -40,7 +40,7 @@ "Name": "preview", "Method": "GET", "Title": "Serves preview of an attached file", - "Path": "/{name}/preview", + "Path": "/preview.{ext}", "Parameters": null } ] diff --git a/sam/repository/attachment.go b/sam/repository/attachment.go index 816da1a60..8c1dba52a 100644 --- a/sam/repository/attachment.go +++ b/sam/repository/attachment.go @@ -27,8 +27,22 @@ type ( ) const ( + sqlAttachmentColumns = ` + a.id, a.rel_user, + a.url, a.preview_url, + a.name, + a.meta, + a.created_at, a.updated_at, a.deleted_at + ` sqlAttachmentScope = "deleted_at IS NULL" + sqlAttachmentByID = `SELECT ` + sqlAttachmentColumns + ` FROM attachments AS a WHERE id = ? AND ` + sqlAttachmentScope + + sqlAttachmentByMessageID = `SELECT ` + sqlAttachmentColumns + `, rel_message + FROM attachments AS a + INNER JOIN message_attachment AS ma ON a.id = ma.rel_attachment + WHERE ma.rel_message IN (?) AND ` + sqlAttachmentScope + ErrAttachmentNotFound = repositoryError("AttachmentNotFound") ) @@ -43,10 +57,9 @@ func (r *attachment) With(ctx context.Context, db *factory.DB) AttachmentReposit } func (r *attachment) FindAttachmentByID(id uint64) (*types.Attachment, error) { - sql := "SELECT * FROM attachments WHERE id = ? AND " + sqlAttachmentScope mod := &types.Attachment{} - return mod, isFound(r.db().Get(mod, sql, id), mod.ID > 0, ErrAttachmentNotFound) + return mod, isFound(r.db().Get(mod, sqlAttachmentByID, id), mod.ID > 0, ErrAttachmentNotFound) } func (r *attachment) FindAttachmentByMessageID(IDs ...uint64) (rval types.MessageAttachmentSet, err error) { @@ -56,12 +69,7 @@ func (r *attachment) FindAttachmentByMessageID(IDs ...uint64) (rval types.Messag return } - sql := `SELECT a.*, rel_message - FROM attachments AS a - INNER JOIN message_attachment AS ma ON a.id = ma.rel_attachment - WHERE ma.rel_message IN (?) AND ` + sqlAttachmentScope - - if sql, args, err := sqlx.In(sql, IDs); err != nil { + if sql, args, err := sqlx.In(sqlAttachmentByMessageID, IDs); err != nil { return nil, err } else { return rval, r.db().Select(&rval, sql, args...) diff --git a/sam/rest/channel.go b/sam/rest/channel.go index bafb1d716..eccb0a374 100644 --- a/sam/rest/channel.go +++ b/sam/rest/channel.go @@ -3,6 +3,8 @@ package rest import ( "context" + "github.com/crusttech/crust/internal/payload" + "github.com/crusttech/crust/internal/payload/outgoing" "github.com/crusttech/crust/sam/rest/request" "github.com/crusttech/crust/sam/service" "github.com/crusttech/crust/sam/types" @@ -84,9 +86,17 @@ func (ctrl *Channel) Attach(ctx context.Context, r *request.ChannelAttach) (inte defer file.Close() - return ctrl.svc.att.With(ctx).Create( + return ctrl.wrapAttachment(ctrl.svc.att.With(ctx).Create( r.ChannelID, r.Upload.Filename, r.Upload.Size, - file) + file)) +} + +func (ctrl *Channel) wrapAttachment(attachment *types.Attachment, err error) (*outgoing.Attachment, error) { + if err != nil { + return nil, err + } else { + return payload.Attachment(attachment), nil + } } diff --git a/sam/rest/handlers/attachment.go b/sam/rest/handlers/attachment.go index 5dec6b138..c4d3296b6 100644 --- a/sam/rest/handlers/attachment.go +++ b/sam/rest/handlers/attachment.go @@ -60,8 +60,8 @@ func (ah *Attachment) MountRoutes(r chi.Router, middlewares ...func(http.Handler r.Group(func(r chi.Router) { r.Use(middlewares...) r.Route("/attachment/{attachmentID}", func(r chi.Router) { - r.Get("/{name}", ah.Original) - r.Get("/{name}/preview", ah.Preview) + r.Get("/original/{name}", ah.Original) + r.Get("/preview.{ext}", ah.Preview) }) }) } diff --git a/sam/rest/handlers/attachment_custom.go b/sam/rest/handlers/attachment_custom.go index 395eb86b2..6d176c810 100644 --- a/sam/rest/handlers/attachment_custom.go +++ b/sam/rest/handlers/attachment_custom.go @@ -38,6 +38,8 @@ func NewAttachmentDownloadable(ah AttachmentAPI) *Attachment { } else { if dl.Download() { w.Header().Add("Content-Disposition", "attachment; filename="+url.QueryEscape(dl.Name())) + } else { + w.Header().Add("Content-Disposition", "inline; filename="+url.QueryEscape(dl.Name())) } http.ServeContent(w, r, dl.Name(), dl.ModTime(), dl.Content()) diff --git a/sam/service/attachment.go b/sam/service/attachment.go index 95e36bbe7..a998b784e 100644 --- a/sam/service/attachment.go +++ b/sam/service/attachment.go @@ -1,13 +1,19 @@ package service import ( + "bytes" "context" + "image" + "image/gif" "io" "log" "net/http" "path" "strings" + "github.com/disintegration/imaging" + "github.com/edwvee/exiffix" + "github.com/pkg/errors" "github.com/titpetric/factory" authService "github.com/crusttech/crust/auth/service" @@ -16,6 +22,11 @@ import ( "github.com/crusttech/crust/sam/types" ) +const ( + attachmentPreviewMaxWidth = 800 + attachmentPreviewMaxHeight = 400 +) + type ( attachment struct { db *factory.DB @@ -83,43 +94,47 @@ func (svc *attachment) OpenPreview(att *types.Attachment) (io.ReadSeeker, error) } func (svc *attachment) Create(channelId uint64, name string, size int64, fh io.ReadSeeker) (att *types.Attachment, err error) { + if svc.store == nil { + return nil, errors.New("Can not create attachment: store handler not set") + } + var currentUserID uint64 = repository.Identity(svc.ctx) // @todo verify if current user can access this channel // @todo verify if current user can upload to this channel att = &types.Attachment{ - ID: factory.Sonyflake.NextID(), - UserID: currentUserID, - Name: strings.TrimSpace(name), - Mimetype: "application/octet-stream", - Size: size, + ID: factory.Sonyflake.NextID(), + UserID: currentUserID, + Name: strings.TrimSpace(name), } // Extract extension but make sure path.Ext is not confused by any leading/trailing dots - var ext = strings.Trim(path.Ext(strings.Trim(name, ".")), ".") + att.Meta.Original.Extension = strings.Trim(path.Ext(strings.Trim(name, ".")), ".") - if err := svc.extractMeta(att, fh); err != nil { - // @todo logmeta extraction failure + att.Meta.Original.Size = size + if att.Meta.Original.Mimetype, err = svc.extractMimetype(fh); err != nil { + return } - log.Printf("Processing uploaded file (name: %s, size: %d, mime: %s)", att.Name, att.Size, att.Mimetype) + log.Printf( + "Processing uploaded file (name: %s, size: %d, mimetype: %s)", + att.Name, + att.Meta.Original.Size, + att.Meta.Original.Mimetype) - if svc.store != nil { - att.Url = svc.store.Original(att.ID, ext) - if err = svc.store.Save(att.Url, fh); err != nil { - log.Print(err.Error()) - return - } - - // Try to make preview - svc.makePreview(att, fh) + att.Url = svc.store.Original(att.ID, att.Meta.Original.Extension) + if err = svc.store.Save(att.Url, fh); err != nil { + log.Print(err.Error()) + return } + // Process image: extract width, height, make preview + log.Printf("Image processed, error: %v", svc.processImage(fh, att)) + log.Printf("File %s stored as %s", att.Name, att.Url) return att, svc.db.Transaction(func() (err error) { - if att, err = svc.attachment.CreateAttachment(att); err != nil { return } @@ -131,7 +146,7 @@ func (svc *attachment) Create(channelId uint64, name string, size int64, fh io.R UserID: currentUserID, } - if strings.HasPrefix(att.Mimetype, "image/") { + if strings.HasPrefix(att.Meta.Original.Mimetype, "image/") { msg.Type = types.MessageTypeInlineImage } @@ -151,12 +166,12 @@ func (svc *attachment) Create(channelId uint64, name string, size int64, fh io.R }) } -func (svc *attachment) extractMeta(att *types.Attachment, file io.ReadSeeker) (err error) { +func (svc *attachment) extractMimetype(file io.ReadSeeker) (mimetype string, err error) { if _, err = file.Seek(0, 0); err != nil { - return err + return } - // Make sure we rewind... + // Make sure we rewind when we're done defer file.Seek(0, 0) // See http.DetectContentType about 512 bytes @@ -165,40 +180,107 @@ func (svc *attachment) extractMeta(att *types.Attachment, file io.ReadSeeker) (e return } - att.Mimetype = http.DetectContentType(buf) - - // @todo compare mime with extension (or better, enforce extension from mimetype) - //if extensions, err := mime.ExtensionsByType(att.Mimetype); err == nil { - // extensions[0] - //} - - // @todo extract image info so we can provide additional features if needed - //if strings.HasPrefix(att.Mimetype, "image/gif") { - // if cfg, err := gif.DecodeAll(file); err == nil { - // m.Width = cfg.Config.Width - // m.Height = cfg.Config.Height - // m.Animated = cfg.LoopCount > 0 || len(cfg.Delay) > 1 - // } - //} else if strings.HasPrefix(att.Mimetype, "image") { - // if cfg, _, err := image.DecodeConfig(file); err == nil { - // m.Width = cfg.Width - // m.Height = cfg.Height - // } - //} - - return + return http.DetectContentType(buf), nil } -func (svc *attachment) makePreview(att *types.Attachment, original io.ReadSeeker) (err error) { - if true { +func (svc *attachment) processImage(original io.ReadSeeker, att *types.Attachment) (err error) { + if !strings.HasPrefix(att.Meta.Original.Mimetype, "image/") { + // Only supporting previews from images (for now) return } - // Can and how we make a preview of this attachment? - var ext = "jpg" - att.PreviewUrl = svc.store.Preview(att.ID, ext) + var ( + preview image.Image + opts []imaging.EncodeOption + format imaging.Format + previewFormat imaging.Format + animated bool + f2m = map[imaging.Format]string{ + imaging.JPEG: "image/jpeg", + imaging.GIF: "image/gif", + } - return svc.store.Save(att.PreviewUrl, original) + f2e = map[imaging.Format]string{ + imaging.JPEG: "jpg", + imaging.GIF: "gif", + } + ) + + if _, err = original.Seek(0, 0); err != nil { + return + } + + if format, err = imaging.FormatFromExtension(att.Meta.Original.Extension); err != nil { + return errors.Wrapf(err, "Could not get format from extension '%s'", att.Meta.Original.Extension) + } + + previewFormat = format + + if imaging.JPEG == format { + // Rotate image if needed + if preview, _, err = exiffix.Decode(original); err != nil { + //return errors.Wrapf(err, "Could not decode EXIF from JPEG") + } + + } + + if imaging.GIF == format { + // Decode all and check loops & delay to determine if GIF is animated or not + if cfg, err := gif.DecodeAll(original); err == nil { + animated = cfg.LoopCount > 0 || len(cfg.Delay) > 1 + + // Use first image for the preview + preview = cfg.Image[0] + } else { + return errors.Wrapf(err, "Could not decode gif config") + } + + } else { + // Use GIF preview for GIFs and JPEG for everything else! + previewFormat = imaging.JPEG + + // Store with a bit lower quality + opts = append(opts, imaging.JPEGQuality(85)) + } + + // In case of JPEG we decode the image and rotate it beforehand + // other cases are handled here + if preview == nil { + if preview, err = imaging.Decode(original); err != nil { + return errors.Wrapf(err, "Could not decode original image") + } + } + + var width, height = preview.Bounds().Max.X, preview.Bounds().Max.Y + att.SetOriginalImageMeta(width, height, animated) + + if width > attachmentPreviewMaxWidth && width > height { + // Landscape does not fit + preview = imaging.Resize(preview, attachmentPreviewMaxWidth, 0, imaging.Lanczos) + } else if height > attachmentPreviewMaxHeight { + // Height does not fit + preview = imaging.Resize(preview, 0, attachmentPreviewMaxHeight, imaging.Lanczos) + } + + // Get dimensions from the preview + width, height = preview.Bounds().Max.X, preview.Bounds().Max.Y + + log.Printf("Generated preview %s (%dx%dpx)", previewFormat, width, height) + + var buf = &bytes.Buffer{} + if err = imaging.Encode(buf, preview, previewFormat); err != nil { + return + } + + meta := att.SetPreviewImageMeta(width, height, false) + meta.Size = int64(buf.Len()) + meta.Mimetype = f2m[previewFormat] + meta.Extension = f2e[previewFormat] + + // Can and how we make a preview of this attachment? + att.PreviewUrl = svc.store.Preview(att.ID, meta.Extension) + + return svc.store.Save(att.PreviewUrl, buf) } // Sends message to event loop @@ -206,7 +288,6 @@ func (svc *attachment) makePreview(att *types.Attachment, original io.ReadSeeker // It also preloads user func (svc *attachment) sendEvent(msg *types.Message, att *types.Attachment) (err error) { msg.Attachment = att - msg.Attachment.GenerateURLs() if msg.User == nil { // @todo pull user from cache diff --git a/sam/service/message.go b/sam/service/message.go index 6bb69ebe3..8af632c71 100644 --- a/sam/service/message.go +++ b/sam/service/message.go @@ -108,7 +108,6 @@ func (svc *message) loadAttachments(mm types.MessageSet) (err error) { if a.MessageID > 0 { if m := mm.FindById(a.MessageID); m != nil { m.Attachment = &a.Attachment - m.Attachment.GenerateURLs() } } diff --git a/sam/types/attachment.go b/sam/types/attachment.go index 0c1c8bc59..35ff232a7 100644 --- a/sam/types/attachment.go +++ b/sam/types/attachment.go @@ -1,28 +1,42 @@ package types import ( - "fmt" - "net/url" + "database/sql/driver" + "encoding/json" "time" -) -const ( - attachmentURL = "/attachment/%d/%s" - attachmentPreviewURL = "/attachment/%d/%s/preview" + "github.com/pkg/errors" ) type ( Attachment struct { - ID uint64 `db:"id" json:"ID,omitempty"` - UserID uint64 `db:"rel_user" json:"userID,omitempty"` - Url string `db:"url" json:"url,omitempty"` - PreviewUrl string `db:"preview_url"json:"previewUrl,omitempty"` - Size int64 `db:"size" json:"size,omitempty"` - Mimetype string `db:"mimetype" json:"mimetype,omitempty"` - Name string `db:"name" json:"name,omitempty"` - CreatedAt time.Time `db:"created_at" json:"createdAt,omitempty"` - UpdatedAt *time.Time `db:"updated_at" json:"updatedAt,omitempty"` - DeletedAt *time.Time `db:"deleted_at" json:"deletedAt,omitempty"` + ID uint64 `db:"id" json:"ID,omitempty"` + UserID uint64 `db:"rel_user" json:"userID,omitempty"` + Url string `db:"url" json:"url,omitempty"` + PreviewUrl string `db:"preview_url"json:"previewUrl,omitempty"` + Name string `db:"name" json:"name,omitempty"` + Meta attachmentMeta `db:"meta" json:"meta"` + CreatedAt time.Time `db:"created_at" json:"createdAt,omitempty"` + UpdatedAt *time.Time `db:"updated_at" json:"updatedAt,omitempty"` + DeletedAt *time.Time `db:"deleted_at" json:"deletedAt,omitempty"` + } + + attachmentImageMeta struct { + Width int `json:"width,omitempty"` + Height int `json:"height,omitempty"` + Animated bool `json:"animated"` + } + + attachmentFileMeta struct { + Size int64 `json:"size"` + Extension string `json:"ext"` + Mimetype string `json:"mimetype"` + Image *attachmentImageMeta `json:"image,omitempty"` + } + + attachmentMeta struct { + Original attachmentFileMeta `json:"original"` + Preview *attachmentFileMeta `json:"preview,omitempty"` } MessageAttachment struct { @@ -43,7 +57,45 @@ func (aa MessageAttachmentSet) Walk(w func(*MessageAttachment) error) (err error return } -func (a *Attachment) GenerateURLs() { - a.Url = fmt.Sprintf(attachmentURL, a.ID, url.PathEscape(a.Name)) - a.PreviewUrl = fmt.Sprintf(attachmentPreviewURL, a.ID, url.PathEscape(a.Name)) +func (a *Attachment) SetOriginalImageMeta(width, height int, animated bool) *attachmentFileMeta { + a.imageMeta(&a.Meta.Original, width, height, animated) + return &a.Meta.Original +} + +func (a *Attachment) SetPreviewImageMeta(width, height int, animated bool) *attachmentFileMeta { + if a.Meta.Preview == nil { + a.Meta.Preview = &attachmentFileMeta{} + } + + a.imageMeta(a.Meta.Preview, width, height, animated) + return a.Meta.Preview +} + +func (a *Attachment) imageMeta(in *attachmentFileMeta, width, height int, animated bool) { + if in.Image == nil { + in.Image = &attachmentImageMeta{} + } + + if width > 0 && height > 0 { + in.Image.Animated = animated + in.Image.Width = width + in.Image.Height = height + } +} + +func (meta *attachmentMeta) Scan(value interface{}) error { + switch value.(type) { + case nil: + *meta = attachmentMeta{} + case []uint8: + if err := json.Unmarshal(value.([]byte), meta); err != nil { + return errors.Wrapf(err, "Can not scan '%v' into attachmentMeta", value) + } + } + + return nil +} + +func (meta attachmentMeta) Value() (driver.Value, error) { + return json.Marshal(meta) }