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

@@ -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,
+		Url:        fmt.Sprintf(attachmentURL, in.ID, url.PathEscape(in.Name)),
-		PreviewUrl: in.PreviewUrl,
+		PreviewUrl: preview,
-		Size:       in.Size,
+		Meta:       in.Meta,
-		Mimetype:   in.Mimetype,
 		Name:       in.Name,
 		CreatedAt:  in.CreatedAt,
 		UpdatedAt:  in.UpdatedAt,

internal/payload/outgoing/attachment.go

@@ -6,15 +6,14 @@ import (
 
 type (
 	Attachment struct {
-		ID         string     `json:"ID"`
+		ID         string      `json:"ID"`
-		UserID     string     `json:"userID"`
+		UserID     string      `json:"userID"`
-		Url        string     `json:"url"`
+		Url        string      `json:"url"`
-		PreviewUrl string     `json:"previewUrl"`
+		PreviewUrl string      `json:"previewUrl,omitempty"`
-		Size       int64      `json:"size"`
+		Meta       interface{} `json:"meta"`
-		Mimetype   string     `json:"mimetype"`
+		Name       string      `json:"name"`
-		Name       string     `json:"name"`
+		CreatedAt  time.Time   `json:"createdAt,omitempty"`
-		CreatedAt  time.Time  `json:"createdAt,omitempty"`
+		UpdatedAt  *time.Time  `json:"updatedAt,omitempty"`
-		UpdatedAt  *time.Time `json:"updatedAt,omitempty"`
 	}
 
 	AttachmentSet []*Attachment

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,

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
 

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"
 			}

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
     }
   ]

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
+	if sql, args, err := sqlx.In(sqlAttachmentByMessageID, IDs); err != nil {
-		      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 {
 		return nil, err
 	} else {
 		return rval, r.db().Select(&rval, sql, args...)

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
+	}
 }

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("/original/{name}", ah.Original)
-			r.Get("/{name}/preview", ah.Preview)
+			r.Get("/preview.{ext}", ah.Preview)
 		})
 	})
 }

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())

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(),
+		ID:     factory.Sonyflake.NextID(),
-		UserID:   currentUserID,
+		UserID: currentUserID,
-		Name:     strings.TrimSpace(name),
+		Name:   strings.TrimSpace(name),
-		Mimetype: "application/octet-stream",
-		Size:     size,
 	}
 
 	// 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 {
+	att.Meta.Original.Size = size
-		// @todo logmeta extraction failure
+	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, att.Meta.Original.Extension)
-		att.Url = svc.store.Original(att.ID, ext)
+	if err = svc.store.Save(att.Url, fh); err != nil {
-		if err = svc.store.Save(att.Url, fh); err != nil {
+		log.Print(err.Error())
-			log.Print(err.Error())
+		return
-			return
-		}
-
-		// Try to make preview
-		svc.makePreview(att, fh)
 	}
 
+	// 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)
+	return http.DetectContentType(buf), nil
-
-	// @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
 }
 
-func (svc *attachment) makePreview(att *types.Attachment, original io.ReadSeeker) (err error) {
+func (svc *attachment) processImage(original io.ReadSeeker, att *types.Attachment) (err error) {
-	if true {
+	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 (
-	var ext = "jpg"
+		preview       image.Image
-	att.PreviewUrl = svc.store.Preview(att.ID, ext)
+		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

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()
 				}
 			}
 

sam/types/attachment.go

@@ -1,28 +1,42 @@
 package types
 
 import (
-	"fmt"
+	"database/sql/driver"
-	"net/url"
+	"encoding/json"
 	"time"
-)
 
-const (
+	"github.com/pkg/errors"
-	attachmentURL        = "/attachment/%d/%s"
-	attachmentPreviewURL = "/attachment/%d/%s/preview"
 )
 
 type (
 	Attachment struct {
-		ID         uint64     `db:"id"         json:"ID,omitempty"`
+		ID         uint64         `db:"id"         json:"ID,omitempty"`
-		UserID     uint64     `db:"rel_user"   json:"userID,omitempty"`
+		UserID     uint64         `db:"rel_user"   json:"userID,omitempty"`
-		Url        string     `db:"url"        json:"url,omitempty"`
+		Url        string         `db:"url"        json:"url,omitempty"`
-		PreviewUrl string     `db:"preview_url"json:"previewUrl,omitempty"`
+		PreviewUrl string         `db:"preview_url"json:"previewUrl,omitempty"`
-		Size       int64      `db:"size"       json:"size,omitempty"`
+		Name       string         `db:"name"       json:"name,omitempty"`
-		Mimetype   string     `db:"mimetype"   json:"mimetype,omitempty"`
+		Meta       attachmentMeta `db:"meta"       json:"meta"`
-		Name       string     `db:"name"       json:"name,omitempty"`
+		CreatedAt  time.Time      `db:"created_at" json:"createdAt,omitempty"`
-		CreatedAt  time.Time  `db:"created_at" json:"createdAt,omitempty"`
+		UpdatedAt  *time.Time     `db:"updated_at" json:"updatedAt,omitempty"`
-		UpdatedAt  *time.Time `db:"updated_at" json:"updatedAt,omitempty"`
+		DeletedAt  *time.Time     `db:"deleted_at" json:"deletedAt,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() {
+func (a *Attachment) SetOriginalImageMeta(width, height int, animated bool) *attachmentFileMeta {
-	a.Url = fmt.Sprintf(attachmentURL, a.ID, url.PathEscape(a.Name))
+	a.imageMeta(&a.Meta.Original, width, height, animated)
-	a.PreviewUrl = fmt.Sprintf(attachmentPreviewURL, a.ID, url.PathEscape(a.Name))
+	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)
 }