Fixes after review

- It's confusing. If there is no difference, I suggest having the route named after the protocol verb or resulting handler. So it should be either post or upload.
- Don't you find that it would be more understandable without else ifs?
- Why do we need a temporary file here?
- etc

Signed-off-by: Evgeniy Kulikov <kim@nspcc.ru>
This commit is contained in:
Evgeniy Kulikov 2021-01-26 18:36:53 +03:00
parent 3cbd4dbd09
commit ec70bfa4cc
No known key found for this signature in database
GPG key ID: BF6AEE0A2A699BF2
3 changed files with 50 additions and 32 deletions

4
app.go
View file

@ -181,8 +181,8 @@ func (a *app) Serve(ctx context.Context) {
r := router.New() r := router.New()
r.RedirectTrailingSlash = true r.RedirectTrailingSlash = true
a.log.Info("enabled /put/{cid}") a.log.Info("enabled /upload/{cid}")
r.POST("/put/{cid}", a.upload) r.POST("/upload/{cid}", a.upload)
a.log.Info("enabled /get/{cid}/{oid}") a.log.Info("enabled /get/{cid}/{oid}")
r.GET("/get/{cid}/{oid}", a.byAddress) r.GET("/get/{cid}/{oid}", a.byAddress)

View file

@ -61,13 +61,24 @@ func (h *headerFilter) Filter(header *fasthttp.RequestHeader) map[string]string
prefix := []byte(userAttributeHeaderPrefix) prefix := []byte(userAttributeHeaderPrefix)
header.VisitAll(func(key, val []byte) { header.VisitAll(func(key, val []byte) {
// checks that key and val not empty
if len(key) == 0 || len(val) == 0 { if len(key) == 0 || len(val) == 0 {
return return
} else if !bytes.HasPrefix(key, prefix) { }
// checks that key has attribute prefix
if !bytes.HasPrefix(key, prefix) {
return return
} else if key = bytes.TrimPrefix(key, prefix); len(key) == 0 { }
// checks that after removing attribute prefix we had not empty key
if key = bytes.TrimPrefix(key, prefix); len(key) == 0 {
return return
} else if name, ok := h.mapping[string(key)]; ok { }
// checks mapping table and if we found record store it
// at resulting hashmap
if name, ok := h.mapping[string(key)]; ok {
result[name] = string(val) result[name] = string(val)
h.logger.Debug("add attribute to result object", h.logger.Debug("add attribute to result object",
@ -77,6 +88,7 @@ func (h *headerFilter) Filter(header *fasthttp.RequestHeader) map[string]string
return return
} }
// otherwise inform that attribute will be ignored
h.logger.Debug("ignore attribute", h.logger.Debug("ignore attribute",
zap.String("key", string(key)), zap.String("key", string(key)),
zap.String("val", string(val))) zap.String("val", string(val)))

View file

@ -3,9 +3,7 @@ package main
import ( import (
"encoding/json" "encoding/json"
"io" "io"
"io/ioutil"
"mime/multipart" "mime/multipart"
"os"
"strconv" "strconv"
"time" "time"
@ -40,7 +38,7 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
var ( var (
err error err error
name string name string
tmp *os.File tmp io.Reader
addr *object.Address addr *object.Address
form *multipart.Form form *multipart.Form
cid = container.NewID() cid = container.NewID()
@ -55,50 +53,51 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
return return
} }
if tmp, err = ioutil.TempFile("", "http-gate-upload-*"); err != nil {
log.Error("could not prepare temporary file", zap.Error(err))
c.Error("could not prepare temporary file", fasthttp.StatusBadRequest)
return
}
defer func() { defer func() {
tmpName := tmp.Name() // if temporary reader can be closed - close it
if closer := tmp.(io.Closer); closer != nil {
log.Debug("close temporary file", zap.Error(tmp.Close())) log.Debug("close temporary multipart/form file", zap.Error(closer.Close()))
log.Debug("remove temporary file", zap.Error(os.RemoveAll(tmpName))) }
if form == nil { if form == nil {
return return
} }
log.Debug("cleanup multipart form", zap.Error(form.RemoveAll())) log.Debug("cleanup multipart/form", zap.Error(form.RemoveAll()))
}() }()
// tries to receive multipart/form or throw error
if form, err = c.MultipartForm(); err != nil { if form, err = c.MultipartForm(); err != nil {
log.Error("could not receive multipart form", zap.Error(err)) log.Error("could not receive multipart/form", zap.Error(err))
c.Error("could not receive multipart form: "+err.Error(), fasthttp.StatusBadRequest) c.Error("could not receive multipart/form: "+err.Error(), fasthttp.StatusBadRequest)
return return
} else if ln := len(form.File); ln != 1 { }
log.Error("received multipart form with more then one file", zap.Int("count", ln))
c.Error("received multipart form with more then one file", fasthttp.StatusBadRequest) // checks that received multipart/form contains only one `file` per request
if ln := len(form.File); ln != 1 {
log.Error("received multipart/form with more then one file", zap.Int("count", ln))
c.Error("received multipart/form with more then one file", fasthttp.StatusBadRequest)
return return
} }
for _, file := range form.File { for _, file := range form.File {
// because multipart/form can contains multiple FileHeader records
// we should check that we have only one per request or throw error
if ln := len(file); ln != 1 { if ln := len(file); ln != 1 {
log.Error("received multipart form file should contains one record", zap.Int("count", ln)) log.Error("received multipart/form file should contains one record", zap.Int("count", ln))
c.Error("received multipart form file should contains one record", fasthttp.StatusBadRequest) c.Error("received multipart/form file should contains one record", fasthttp.StatusBadRequest)
return return
} }
name = file[0].Filename name = file[0].Filename
if err = fasthttp.SaveMultipartFile(file[0], tmp.Name()); err != nil { // opens multipart/form file to work within or throw error
log.Error("could not store uploaded file into temporary", zap.Error(err)) if tmp, err = file[0].Open(); err != nil {
c.Error("could not store uploaded file into temporary", fasthttp.StatusBadRequest) log.Error("could not prepare uploaded file", zap.Error(err))
c.Error("could not prepare uploaded file", fasthttp.StatusBadRequest)
return return
} }
@ -107,6 +106,7 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
filtered := a.hdr.Filter(&c.Request.Header) filtered := a.hdr.Filter(&c.Request.Header)
attributes := make([]*object.Attribute, 0, len(filtered)) attributes := make([]*object.Attribute, 0, len(filtered))
// prepares attributes from filtered headers
for key, val := range filtered { for key, val := range filtered {
attribute := object.NewAttribute() attribute := object.NewAttribute()
attribute.SetKey(key) attribute.SetKey(key)
@ -115,7 +115,7 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
attributes = append(attributes, attribute) attributes = append(attributes, attribute)
} }
// Attribute FileName wasn't set from header // sets FileName attribute if it wasn't set from header
if _, ok := filtered[object.AttributeFileName]; ok { if _, ok := filtered[object.AttributeFileName]; ok {
filename := object.NewAttribute() filename := object.NewAttribute()
filename.SetKey(object.AttributeFileName) filename.SetKey(object.AttributeFileName)
@ -124,7 +124,7 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
attributes = append(attributes, filename) attributes = append(attributes, filename)
} }
// Attribute Timestamp wasn't set from header and enabled by settings // sets Timestamp attribute if it wasn't set from header and enabled by settings
if _, ok := filtered[object.AttributeTimestamp]; ok && a.enableDefaultTimestamp { if _, ok := filtered[object.AttributeTimestamp]; ok && a.enableDefaultTimestamp {
timestamp := object.NewAttribute() timestamp := object.NewAttribute()
timestamp.SetKey(object.AttributeTimestamp) timestamp.SetKey(object.AttributeTimestamp)
@ -133,23 +133,29 @@ func (a *app) upload(c *fasthttp.RequestCtx) {
attributes = append(attributes, timestamp) attributes = append(attributes, timestamp)
} }
// prepares new object and fill it
raw := object.NewRaw() raw := object.NewRaw()
raw.SetContainerID(cid) raw.SetContainerID(cid)
raw.SetOwnerID(a.cli.Owner()) // should be various: from sdk / BearerToken raw.SetOwnerID(a.cli.Owner()) // should be various: from sdk / BearerToken
raw.SetAttributes(attributes...) raw.SetAttributes(attributes...)
// tries to put file into NeoFS or throw error
if addr, err = a.cli.Object().Put(c, raw.Object(), sdk.WithPutReader(tmp)); err != nil { if addr, err = a.cli.Object().Put(c, raw.Object(), sdk.WithPutReader(tmp)); err != nil {
log.Error("could not store file in NeoFS", zap.Error(err)) log.Error("could not store file in NeoFS", zap.Error(err))
c.Error("could not store file in NeoFS", fasthttp.StatusBadRequest) c.Error("could not store file in NeoFS", fasthttp.StatusBadRequest)
return return
} else if err = newPutResponse(addr).Encode(c); err != nil { }
// tries to return response, otherwise, if something went wrong throw error
if err = newPutResponse(addr).Encode(c); err != nil {
log.Error("could not prepare response", zap.Error(err)) log.Error("could not prepare response", zap.Error(err))
c.Error("could not prepare response", fasthttp.StatusBadRequest) c.Error("could not prepare response", fasthttp.StatusBadRequest)
return return
} }
// reports status code and content type
c.Response.SetStatusCode(fasthttp.StatusOK) c.Response.SetStatusCode(fasthttp.StatusOK)
c.Response.Header.SetContentType(jsonHeader) c.Response.Header.SetContentType(jsonHeader)
} }