Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Thumbnails Service #3

Merged
merged 22 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/go-chi/chi v4.0.2+incompatible
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
github.com/micro/cli/v2 v2.1.1
github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646
github.com/ogier/pflag v0.0.1 // indirect
github.com/oklog/run v1.0.0
github.com/openzipkin/zipkin-go v0.2.2
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,8 @@ github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw=
github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c=
github.com/nats-io/stan.go v0.5.0/go.mod h1:dYqB+vMN3C2F9pT1FRQpg9eHbjPj6mP0yYuyBNuXHZE=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646 h1:zYyBkD/k9seD2A7fsi6Oo2LfFZAehjjQMERAvZLEDnQ=
github.com/nfnt/resize v0.0.0-20180221191011-83c6a9932646/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8=
github.com/nlopes/slack v0.6.0/go.mod h1:JzQ9m3PMAqcpeCam7UaHSuBuupz7CmpjehYMayT6YOk=
github.com/nlopes/slack v0.6.1-0.20191106133607-d06c2a2b3249/go.mod h1:JzQ9m3PMAqcpeCam7UaHSuBuupz7CmpjehYMayT6YOk=
github.com/nrdcg/auroradns v1.0.0/go.mod h1:6JPXKzIRzZzMqtTDgueIhTi6rFf1QvYE/HzqidhOhjw=
Expand Down
22 changes: 17 additions & 5 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,23 @@ type Tracing struct {

// Config combines all available configuration parts.
type Config struct {
File string
Log Log
Debug Debug
HTTP HTTP
Tracing Tracing
File string
Log Log
Debug Debug
HTTP HTTP
Tracing Tracing
FileSystemStorage FileSystemStorage
WebDavSource WebDavSource
}

// FileSystemStorage defines the available filesystem storage configuration.
type FileSystemStorage struct {
RootDirectory string
}

// WebDavSource defines the available webdav source configuration.
type WebDavSource struct {
BaseURL string
}

// New initializes a new configuration with or without defaults.
Expand Down
18 changes: 16 additions & 2 deletions pkg/flagset/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
},
&cli.StringFlag{
Name: "debug-addr",
Value: "0.0.0.0:9114",
Value: "0.0.0.0:9194",
Copy link
Member

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

Copy link
Contributor Author

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.

Usage: "Address to bind debug server",
EnvVars: []string{"THUMBNAILS_DEBUG_ADDR"},
Destination: &cfg.Debug.Addr,
Expand All @@ -115,7 +115,7 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
},
&cli.StringFlag{
Name: "http-addr",
Value: "0.0.0.0:9110",
Value: "0.0.0.0:9190",
Usage: "Address to bind http server",
EnvVars: []string{"THUMBNAILS_HTTP_ADDR"},
Destination: &cfg.HTTP.Addr,
Expand All @@ -134,5 +134,19 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
EnvVars: []string{"THUMBNAILS_HTTP_ROOT"},
Destination: &cfg.HTTP.Root,
},
&cli.StringFlag{
Name: "filesystemstorage-root",
Value: "/tmp/ocis-thumbnails/",
Usage: "Root path of the filesystem storage directory",
EnvVars: []string{"THUMBNAILS_FILESYSTEMSTORAGE_ROOT"},
Destination: &cfg.FileSystemStorage.RootDirectory,
},
&cli.StringFlag{
Name: "webdavsource-baseurl",
Value: "http://localhost:9140/remote.php/webdav/",
Usage: "Base url for a webdav api",
EnvVars: []string{"THUMBNAILS_WEBDAVSOURCE_BASEURL"},
Destination: &cfg.WebDavSource.BaseURL,
},
}
}
4 changes: 2 additions & 2 deletions pkg/service/v0/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ func (i instrument) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

// Dummy implements the Service interface.
func (i instrument) Dummy(w http.ResponseWriter, r *http.Request) {
i.next.Dummy(w, r)
func (i instrument) Thumbnails(w http.ResponseWriter, r *http.Request) {
i.next.Thumbnails(w, r)
}
4 changes: 2 additions & 2 deletions pkg/service/v0/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ func (l logging) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

// Dummy implements the Service interface.
func (l logging) Dummy(w http.ResponseWriter, r *http.Request) {
l.next.Dummy(w, r)
func (l logging) Thumbnails(w http.ResponseWriter, r *http.Request) {
l.next.Thumbnails(w, r)
}
76 changes: 67 additions & 9 deletions pkg/service/v0/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change annotation to:

Thumbnails implements the business logic for Service.

Also use the singular, as in Thumbnail 💃

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"))
Copy link
Member

Choose a reason for hiding this comment

The 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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking I'd declare a struct and try to unmarshal the request query string onto it, then operate with typed fields. It is the same concept you're doing here but a bit more explicit. We could give it a try together


w.Write([]byte("Hello ocis-thumbnails!"))
encoder := thumbnails.EncoderForType(fileType)
if encoder == nil {
// TODO: better error responses
Copy link

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a logger from ocis-pkg so we have persisted logs and not only the user can see it. I'm also fond of using the passive voice, as in:

can't be encoded

Furthermore the error message is not informative enough, since you're checking whether the fileType is supported or not by fetching an encoder for it, it should read more like:

encoding for filetype: .XYZ is not supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
}
4 changes: 2 additions & 2 deletions pkg/service/v0/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ func (t tracing) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

// Dummy implements the Service interface.
func (t tracing) Dummy(w http.ResponseWriter, r *http.Request) {
t.next.Dummy(w, r)
func (t tracing) Thumbnails(w http.ResponseWriter, r *http.Request) {
t.next.Thumbnails(w, r)
}
58 changes: 58 additions & 0 deletions pkg/thumbnails/encoding.go
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
}
}
33 changes: 33 additions & 0 deletions pkg/thumbnails/imgsource/imgsource.go
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you considered using context.Context here? You can still store key-values on a typed and thread safe manner 🗡

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
}
57 changes: 57 additions & 0 deletions pkg/thumbnails/imgsource/webdav.go
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
}
Loading