-
Notifications
You must be signed in to change notification settings - Fork 1
Thumbnails Service #3
Changes from 13 commits
a98df0b
34dcaf9
0978653
f55199f
9753878
4647168
d8c3593
2a587b3
e0354f2
61d0e52
78b5231
e36a7ef
261429e
985c118
1389cd7
f9d0f7e
0164715
f5f68e2
78a92cf
35141c8
06151de
024e466
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,19 @@ package svc | |
|
||
import ( | ||
"net/http" | ||
"strconv" | ||
|
||
"github.com/go-chi/chi" | ||
"github.com/owncloud/ocis-thumbnails/pkg/config" | ||
"github.com/owncloud/ocis-thumbnails/pkg/thumbnails" | ||
"github.com/owncloud/ocis-thumbnails/pkg/thumbnails/imgsource" | ||
"github.com/owncloud/ocis-thumbnails/pkg/thumbnails/storage" | ||
) | ||
|
||
// Service defines the extension handlers. | ||
type Service interface { | ||
ServeHTTP(http.ResponseWriter, *http.Request) | ||
Dummy(http.ResponseWriter, *http.Request) | ||
Thumbnails(http.ResponseWriter, *http.Request) | ||
} | ||
|
||
// NewService returns a service implementation for Service. | ||
|
@@ -23,30 +27,84 @@ func NewService(opts ...Option) Service { | |
svc := Thumbnails{ | ||
config: options.Config, | ||
mux: m, | ||
manager: thumbnails.SimpleManager{ | ||
Storage: storage.NewFileSystemStorage(options.Config.FileSystemStorage), | ||
}, | ||
source: imgsource.NewWebDavSource(options.Config.WebDavSource), | ||
} | ||
|
||
m.Route(options.Config.HTTP.Root, func(r chi.Router) { | ||
r.Get("/", svc.Dummy) | ||
r.Get("/thumbnails", svc.Thumbnails) | ||
}) | ||
|
||
return svc | ||
} | ||
|
||
// Thumbnails defines implements the business logic for Service. | ||
type Thumbnails struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change annotation to:
Also use the singular, as in |
||
config *config.Config | ||
mux *chi.Mux | ||
config *config.Config | ||
mux *chi.Mux | ||
manager thumbnails.Manager | ||
source imgsource.Source | ||
} | ||
|
||
// ServeHTTP implements the Service interface. | ||
func (g Thumbnails) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
g.mux.ServeHTTP(w, r) | ||
} | ||
|
||
// Dummy implements the Service interface. | ||
func (g Thumbnails) Dummy(w http.ResponseWriter, r *http.Request) { | ||
w.Header().Set("Content-Type", "text/plain") | ||
w.WriteHeader(http.StatusOK) | ||
// Thumbnails provides the endpoint to retrieve a thumbnail for an image | ||
func (g Thumbnails) Thumbnails(w http.ResponseWriter, r *http.Request) { | ||
query := r.URL.Query() | ||
width, _ := strconv.Atoi(query.Get("width")) | ||
height, _ := strconv.Atoi(query.Get("height")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add error handling here, make it more explicit where a failure might happen |
||
fileType := query.Get("type") | ||
filePath := query.Get("file_path") | ||
etag := query.Get("etag") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally speaking I'd declare a |
||
|
||
w.Write([]byte("Hello ocis-thumbnails!")) | ||
encoder := thumbnails.EncoderForType(fileType) | ||
if encoder == nil { | ||
// TODO: better error responses | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use https://golang.org/pkg/net/http/#Error here and in all subsequent cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. But since the API will be changed in another PR anyways I would leave it like that for now. |
||
w.WriteHeader(http.StatusBadRequest) | ||
w.Write([]byte("can't encode that")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use a logger from
Furthermore the error message is not informative enough, since you're checking whether the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to replace that when the proper API is implemented. |
||
return | ||
} | ||
ctx := thumbnails.Context{ | ||
Width: width, | ||
Height: height, | ||
ImagePath: filePath, | ||
Encoder: encoder, | ||
ETag: etag, | ||
} | ||
|
||
thumbnail := g.manager.GetStored(ctx) | ||
if thumbnail != nil { | ||
w.Write(thumbnail) | ||
return | ||
} | ||
|
||
auth := r.Header.Get("Authorization") | ||
|
||
sCtx := imgsource.NewContext() | ||
sCtx.Set(imgsource.WebDavAuth, auth) | ||
// TODO: clean up error handling | ||
img, err := g.source.Get(ctx.ImagePath, sCtx) | ||
if err != nil { | ||
w.WriteHeader(http.StatusInternalServerError) | ||
w.Write([]byte(err.Error())) | ||
return | ||
} | ||
if img == nil { | ||
w.WriteHeader(http.StatusInternalServerError) | ||
w.Write([]byte("img is nil")) | ||
return | ||
} | ||
thumbnail, err = g.manager.Get(ctx, img) | ||
if err != nil { | ||
w.Write([]byte(err.Error())) | ||
return | ||
} | ||
|
||
w.WriteHeader(http.StatusCreated) | ||
w.Write(thumbnail) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
package thumbnails | ||
|
||
import ( | ||
"image" | ||
"image/jpeg" | ||
"image/png" | ||
"io" | ||
"strings" | ||
) | ||
|
||
// Encoder encodes the thumbnail to a specific format. | ||
type Encoder interface { | ||
// Encode encodes the image to a format. | ||
Encode(io.Writer, image.Image) error | ||
// Types returns the formats suffixes. | ||
Types() []string | ||
} | ||
|
||
// PngEncoder encodes to png | ||
type PngEncoder struct{} | ||
|
||
// Encode encodes to png format | ||
func (e PngEncoder) Encode(w io.Writer, i image.Image) error { | ||
return png.Encode(w, i) | ||
} | ||
|
||
// Types returns the png suffix | ||
func (e PngEncoder) Types() []string { | ||
return []string{"png"} | ||
} | ||
|
||
// JpegEncoder encodes to jpg. | ||
type JpegEncoder struct{} | ||
|
||
// Encode encodes to jpg | ||
func (e JpegEncoder) Encode(w io.Writer, i image.Image) error { | ||
return jpeg.Encode(w, i, nil) | ||
} | ||
|
||
// Types returns the jpg suffixes. | ||
func (e JpegEncoder) Types() []string { | ||
return []string{"jpeg", "jpg"} | ||
} | ||
|
||
// EncoderForType returns the encoder for a given file type | ||
// or nil if the type is not supported. | ||
func EncoderForType(fileType string) Encoder { | ||
switch strings.ToLower(fileType) { | ||
case "png": | ||
return PngEncoder{} | ||
case "jpg": | ||
fallthrough | ||
case "jpeg": | ||
return JpegEncoder{} | ||
default: | ||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package imgsource | ||
|
||
import "image" | ||
|
||
// Source defines the interface for image sources | ||
type Source interface { | ||
Get(path string, ctx SourceContext) (image.Image, error) | ||
} | ||
|
||
// NewContext creates a new SourceContext instance | ||
func NewContext() SourceContext { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have you considered using |
||
return SourceContext{ | ||
m: make(map[string]interface{}), | ||
} | ||
} | ||
|
||
// SourceContext is used to pass source specific parameters | ||
type SourceContext struct { | ||
m map[string]interface{} | ||
} | ||
|
||
// GetString tries to cast the value to a string | ||
func (s SourceContext) GetString(key string) string { | ||
if s, ok := s.m[key].(string); ok { | ||
return s | ||
} | ||
return "" | ||
} | ||
|
||
// Set sets a value | ||
func (s SourceContext) Set(key string, val interface{}) { | ||
s.m[key] = val | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package imgsource | ||
|
||
import ( | ||
"fmt" | ||
"image" | ||
"net/http" | ||
"net/url" | ||
"path" | ||
|
||
"github.com/owncloud/ocis-thumbnails/pkg/config" | ||
) | ||
|
||
// NewWebDavSource creates a new webdav instance. | ||
func NewWebDavSource(cfg config.WebDavSource) WebDav { | ||
return WebDav{ | ||
baseURL: cfg.BaseURL, | ||
} | ||
} | ||
|
||
// WebDav implements the Source interface for webdav services | ||
type WebDav struct { | ||
baseURL string | ||
} | ||
|
||
const ( | ||
// WebDavAuth is the parameter name for the autorization token | ||
WebDavAuth = "Authorization" | ||
) | ||
|
||
// Get downloads the file from a webdav service | ||
func (s WebDav) Get(file string, ctx SourceContext) (image.Image, error) { | ||
u, _ := url.Parse(s.baseURL) | ||
u.Path = path.Join(u.Path, file) | ||
req, err := http.NewRequest(http.MethodGet, u.String(), nil) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not get the image \"%s\" error: %s", file, err.Error()) | ||
} | ||
|
||
auth := ctx.GetString(WebDavAuth) | ||
req.Header.Add("Authorization", auth) | ||
|
||
client := &http.Client{} | ||
resp, err := client.Do(req) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not get the image \"%s\" error: %s", file, err.Error()) | ||
} | ||
|
||
if resp.StatusCode != http.StatusOK { | ||
return nil, fmt.Errorf("could not get the image \"%s\". Request returned with statuscode %d ", file, resp.StatusCode) | ||
} | ||
|
||
img, _, err := image.Decode(resp.Body) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not decode the image \"%s\". error: %s", file, err.Error()) | ||
} | ||
return img, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure to reserve a port range here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I wanted to change that back... But yes we should define a port for the service.