From eb3dd467adb232c709449be93c1762dedb1be0a5 Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 22 Mar 2019 03:03:41 -0500 Subject: [PATCH] change image location to user based, make random uuid for file name --- backend/app/rest/api/rest.go | 2 +- backend/app/rest/api/rest_private.go | 5 +- backend/app/rest/api/rest_private_test.go | 5 +- backend/app/rest/api/rest_public.go | 4 +- backend/app/store/image/image.go | 70 +++++++++++++---------- backend/app/store/image/image_test.go | 48 ++++++++-------- 6 files changed, 74 insertions(+), 60 deletions(-) diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index e09a458e..c7349f84 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -221,7 +221,7 @@ func (s *Rest) routes() chi.Router { ropen.Get("/config", s.configCtrl) ropen.Post("/preview", s.previewCommentCtrl) ropen.Get("/info", s.infoCtrl) - ropen.Get("/picture/{id}", s.loadPictureCtrl) + ropen.Get("/picture/{user}/{id}", s.loadPictureCtrl) ropen.Mount("/rss", s.rssRoutes()) ropen.Mount("/img", s.ImageProxy.Routes()) diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 97e2b493..b7063656 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -301,14 +301,13 @@ func (s *Rest) savePictureCtrl(w http.ResponseWriter, r *http.Request) { } defer func() { _ = file.Close() }() - picName := fmt.Sprintf("%s_%d_%s", user.ID, time.Now().Nanosecond(), header.Filename) - id, err := s.ImageService.Save(picName, file) + id, err := s.ImageService.Save(header.Filename, user.ID, file) if err != nil { rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't save image", rest.ErrInternal) return } - render.JSON(w, r, R.JSON{"location": id}) + render.JSON(w, r, R.JSON{"id": id}) } func (s *Rest) isReadOnly(locator store.Locator) bool { diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 46a92ad6..cf053e2a 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -509,6 +509,7 @@ func TestRest_SavePictureCtrl(t *testing.T) { client := http.Client{} req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/api/v1/picture", ts.URL), bodyBuf) + require.NoError(t, err) req.Header.Add("Content-Type", contentType) req.Header.Add("X-JWT", devToken) resp, err := client.Do(req) @@ -519,10 +520,10 @@ func TestRest_SavePictureCtrl(t *testing.T) { m := map[string]string{} err = json.Unmarshal(body, &m) - assert.Contains(t, m["location"], ".png") + assert.Contains(t, m["id"], ".png") // load picture - resp, err = http.Get(fmt.Sprintf("%s/api/v1/picture/%s", ts.URL, m["location"])) + resp, err = http.Get(fmt.Sprintf("%s/api/v1/picture/%s", ts.URL, m["id"])) require.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) body, err = ioutil.ReadAll(resp.Body) diff --git a/backend/app/rest/api/rest_public.go b/backend/app/rest/api/rest_public.go index 6aae3850..3713ef53 100644 --- a/backend/app/rest/api/rest_public.go +++ b/backend/app/rest/api/rest_public.go @@ -332,7 +332,7 @@ func (s *Rest) listCtrl(w http.ResponseWriter, r *http.Request) { } } -// GET /picture/{id} - get picture +// GET /picture/{user}/{id} - get picture func (s *Rest) loadPictureCtrl(w http.ResponseWriter, r *http.Request) { imgContentType := func(img string) string { @@ -348,7 +348,7 @@ func (s *Rest) loadPictureCtrl(w http.ResponseWriter, r *http.Request) { return "image/*" } - id := chi.URLParam(r, "id") + id := chi.URLParam(r, "user") + "/" + chi.URLParam(r, "id") imgRdr, size, err := s.ImageService.Load(id) if err != nil { rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't get image "+id, rest.ErrAssetNotFound) diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 30a74ef4..2fa45bbe 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -3,25 +3,26 @@ package image import ( - "crypto/sha256" - "encoding/hex" "fmt" "hash/crc64" "io" "math" "os" "path" + "path/filepath" "strconv" + "strings" "sync" log "github.com/go-pkgz/lgr" + "github.com/google/uuid" "github.com/pkg/errors" ) // Interface defines Save and Load methods type Interface interface { - Save(name string, r io.Reader) (id string, err error) // get name and reader and returns ID of stored image - Load(id string) (io.ReadCloser, int64, error) // load image by ID. Caller has to close the reader. + Save(fileName string, userID string, r io.Reader) (id string, err error) // get name and reader and returns ID of stored image + Load(id string) (io.ReadCloser, int64, error) // load image by ID. Caller has to close the reader. } // FileSystem provides image Interface for local files. Saves and loads files from Location, restricts max size @@ -41,17 +42,17 @@ type FileSystem struct { // Save data from reader for given file name to local FS. Returns id as a hash of name // name should be passed in unique prefix, for example with userID_* // Files partitioned across multiple subdirectories. -func (f *FileSystem) Save(name string, r io.Reader) (id string, err error) { +func (f *FileSystem) Save(fileName string, userID string, r io.Reader) (id string, err error) { - h := sha256.Sum224([]byte(name)) - id = hex.EncodeToString(h[:]) - if ext := path.Ext(name); ext != "" { - id += ext + uid, err := uuid.NewUUID() + if err != nil { + return "", errors.Wrap(err, "can't make image uuid") } - location := f.location(id) - dst := path.Join(location, id) - if err = os.MkdirAll(location, 0700); err != nil { + id = path.Join(userID, uid.String()) + filepath.Ext(fileName) // make id as user/uuid.ext + dst := f.location(id) + + if err = os.MkdirAll(path.Dir(dst), 0700); err != nil { return "", errors.Wrap(err, "can't make image directory") } @@ -71,17 +72,16 @@ func (f *FileSystem) Save(name string, r io.Reader) (id string, err error) { if err = os.Remove(dst); err != nil { log.Printf("[WARN] can't remove image file %s, %v", dst, err) } - return "", errors.Errorf("file %s is too large", name) + return "", errors.Errorf("file %s is too large", fileName) } - log.Printf("[DEBUG] file %s saved for image %s", fh.Name(), name) + log.Printf("[DEBUG] file %s saved for image %s", fh.Name(), fileName) return id, nil } // Load image from FS. Uses id to get partition subdirectory. // returns ReadCloser and caller should call close after processing completed. func (f *FileSystem) Load(id string) (io.ReadCloser, int64, error) { - location := f.location(id) - imgFile := path.Join(location, id) + imgFile := f.location(id) st, err := os.Stat(imgFile) if err != nil { @@ -95,21 +95,33 @@ func (f *FileSystem) Load(id string) (io.ReadCloser, int64, error) { return fh, st.Size(), nil } -// get location (directory) for id by adding partition to the final path in order to keep files +// get location (full path) for id by adding partition to the final path in order to keep files // in different subdirectories and avoid too many files in a single place. -// the end result is a full path like this - /tmp/images/92. Number of partitions defined by FileSystem.Partitions +// the end result is a full path like this - /tmp/images/user1/92/xxx-yyy.png. Number of partitions defined by FileSystem. +// Partitions func (f *FileSystem) location(id string) string { - if f.Partitions == 0 { - return f.Location + + partition := func(id string) string { + f.crc.Do(func() { + f.crc.Table = crc64.MakeTable(crc64.ECMA) + p := int(math.Round(math.Log10(float64(f.Partitions)))) + f.crc.mask = "%0" + strconv.Itoa(p) + "d" + f.crc.divider = uint64(math.Pow(10, float64(p))) + }) + checksum64 := crc64.Checksum([]byte(id), f.crc.Table) + partition := checksum64 % f.crc.divider + return fmt.Sprintf(f.crc.mask, partition) } - f.crc.Do(func() { - f.crc.Table = crc64.MakeTable(crc64.ECMA) - p := int(math.Round(math.Log10(float64(f.Partitions)))) - f.crc.mask = "%0" + strconv.Itoa(p) + "d" - f.crc.divider = uint64(math.Pow(10, float64(p))) - }) - checksum64 := crc64.Checksum([]byte(id), f.crc.Table) - partition := checksum64 % f.crc.divider - return path.Join(f.Location, fmt.Sprintf(f.crc.mask, partition)) + user := "unknown" + file := id + elems := strings.Split(id, "/") + if len(elems) == 2 { + user, file = elems[0], elems[1] + } + + if f.Partitions == 0 { + return path.Join(f.Location, user, file) + } + return path.Join(f.Location, user, partition(id), file) } diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 3c24c239..ef99fc05 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -4,7 +4,6 @@ import ( "io/ioutil" "math/rand" "os" - "path" "strconv" "strings" "testing" @@ -17,12 +16,13 @@ func TestImage_Save(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("blah_ff1.png", strings.NewReader("blah blah")) + id, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah")) assert.NoError(t, err) - assert.Equal(t, "fc77a87ad3c898b9603119711f99305145e272e103c904d85ee2deda.png", id) + assert.Contains(t, id, "user1/") + assert.Contains(t, id, ".png") + t.Log(id) - dst := path.Join(svc.Location, "56", id) - data, err := ioutil.ReadFile(dst) + data, err := ioutil.ReadFile(svc.location(id)) assert.NoError(t, err) assert.Equal(t, "blah blah", string(data)) } @@ -31,9 +31,9 @@ func TestImage_SaveTooLarge(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() svc.MaxSize = 5 - _, err := svc.Save("blah_ff1.png", strings.NewReader("blah blah")) + _, err := svc.Save("blah_ff1.png", "user2", strings.NewReader("blah blah")) assert.Error(t, err) - assert.EqualError(t, err, "file blah_ff1.png is too large") + assert.Contains(t, err.Error(), "is too large") } func TestImage_Load(t *testing.T) { @@ -41,8 +41,9 @@ func TestImage_Load(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("blah_ff1.png", strings.NewReader("blah blah")) + id, err := svc.Save("blah_ff1.png", "user1", strings.NewReader("blah blah")) assert.NoError(t, err) + t.Log(id) r, sz, err := svc.Load(id) assert.NoError(t, err) @@ -60,15 +61,15 @@ func TestImage_location(t *testing.T) { partitions int id, res string }{ - {10, "abcdefg", "/tmp/2"}, - {10, "abcdefe", "/tmp/1"}, - {10, "12345", "/tmp/9"}, - {100, "12345", "/tmp/69"}, - {100, "xyzz", "/tmp/58"}, - {100, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/02"}, - {5, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/2"}, - {5, "xxxyz.png", "/tmp/0"}, - {0, "12345", "/tmp"}, + {10, "u1/abcdefg.png", "/tmp/u1/4/abcdefg.png"}, + {10, "abcdefe", "/tmp/unknown/1/abcdefe"}, + {10, "12345", "/tmp/unknown/9/12345"}, + {100, "12345", "/tmp/unknown/69/12345"}, + {100, "xyzz", "/tmp/unknown/58/xyzz"}, + {100, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/02/6851dcde6024e03258a66705f29e14b506048c74.png"}, + {5, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/2/6851dcde6024e03258a66705f29e14b506048c74.png"}, + {5, "xxxyz.png", "/tmp/unknown/0/xxxyz.png"}, + {0, "12345", "/tmp/unknown/12345"}, } for n, tt := range tbl { t.Run(strconv.Itoa(n), func(t *testing.T) { @@ -79,20 +80,21 @@ func TestImage_location(t *testing.T) { // generate random names and make sure partition never runs out of allowed letterRunes := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - randomString := func(n int) string { + randomID := func(n int) string { b := make([]rune, n) for i := range b { b[i] = letterRunes[rand.Intn(len(letterRunes))] } - return string(b) + return "user1" + "/" + string(b) } svc := FileSystem{Location: "/tmp", Partitions: 10} for i := 0; i < 1000; i++ { - v := randomString(rand.Intn(64)) - parts := strings.Split(svc.location(v), "/") - p, err := strconv.Atoi(parts[len(parts)-1]) - require.NoError(t, err) + v := randomID(rand.Intn(64)) + location := svc.location(v) + elems := strings.Split(location, "/") + p, err := strconv.Atoi(elems[3]) + require.NoError(t, err, location) assert.True(t, p >= 0 && p < 10) } }