From 665b206709787007be33d2da091a2a668739eb9c Mon Sep 17 00:00:00 2001 From: Conrad Hoffmann Date: Thu, 7 Nov 2024 17:50:09 +0100 Subject: [PATCH] storage: streamline ETag calculation This commit introduces some helpers so that ETags can be calculated at the same time that files get read or written. Besides looking nicer, it should also help reduce lock contention around file access, as files do not need to be opened twice anymore. --- storage/etag.go | 58 +++++++++++++++++++++++++++++++++++ storage/filesystem.go | 9 ++---- storage/filesystem_caldav.go | 37 +++++++++------------- storage/filesystem_carddav.go | 39 +++++++++-------------- 4 files changed, 89 insertions(+), 54 deletions(-) create mode 100644 storage/etag.go diff --git a/storage/etag.go b/storage/etag.go new file mode 100644 index 0000000..1d22a5e --- /dev/null +++ b/storage/etag.go @@ -0,0 +1,58 @@ +package storage + +import ( + "crypto/sha1" + "encoding/base64" + "hash" + "io" +) + +type ETagWriter struct { + io.Writer + output io.Writer + csum hash.Hash +} + +// NewETagWriter creates a new io.Writer that pipes writes to the provided +// output while also calculating the contents ETag. +func NewETagWriter(output io.Writer) *ETagWriter { + csum := sha1.New() + return &ETagWriter{ + output: io.MultiWriter(output, csum), + csum: csum, + } +} + +func (e *ETagWriter) Write(p []byte) (n int, err error) { + return e.output.Write(p) +} + +func (e *ETagWriter) ETag() string { + csum := e.csum.Sum(nil) + return base64.StdEncoding.EncodeToString(csum[:]) +} + +type ETagReader struct { + io.Reader + input io.Reader + csum hash.Hash +} + +// NewETagReader creates a new io.Reader that pipes reads from the provided +// input while also calculating the contents ETag. +func NewETagReader(input io.Reader) *ETagReader { + csum := sha1.New() + return &ETagReader{ + input: io.TeeReader(input, csum), + csum: csum, + } +} + +func (e *ETagReader) Read(p []byte) (n int, err error) { + return e.input.Read(p) +} + +func (e *ETagReader) ETag() string { + csum := e.csum.Sum(nil) + return base64.StdEncoding.EncodeToString(csum[:]) +} diff --git a/storage/filesystem.go b/storage/filesystem.go index c6f3887..301c8f7 100644 --- a/storage/filesystem.go +++ b/storage/filesystem.go @@ -1,8 +1,6 @@ package storage import ( - "crypto/sha1" - "encoding/base64" "fmt" "io" "os" @@ -144,11 +142,10 @@ func etagForFile(path string) (string, error) { } defer f.Close() - h := sha1.New() - if _, err := io.Copy(h, f); err != nil { + src := NewETagReader(f) + if _, err := io.ReadAll(src); err != nil { return "", err } - csum := h.Sum(nil) - return base64.StdEncoding.EncodeToString(csum[:]), nil + return src.ETag(), nil } diff --git a/storage/filesystem_caldav.go b/storage/filesystem_caldav.go index a495ba5..fdab921 100644 --- a/storage/filesystem_caldav.go +++ b/storage/filesystem_caldav.go @@ -47,22 +47,23 @@ func (b *filesystemBackend) safeLocalCalDAVPath(ctx context.Context, urlPath str return b.safeLocalPath(homeSetPath, urlPath) } -func calendarFromFile(path string, propFilter []string) (*ical.Calendar, error) { +func calendarAndEtagFromFile(path string, propFilter []string) (*ical.Calendar, string, error) { f, err := os.Open(path) if err != nil { - return nil, err + return nil, "", err } defer f.Close() - dec := ical.NewDecoder(f) + src := NewETagReader(f) + dec := ical.NewDecoder(src) cal, err := dec.Decode() if err != nil { - return nil, err + return nil, "", err } - return cal, nil // TODO implement //return icalPropFilter(cal, propFilter), nil + return cal, src.ETag(), nil } func (b *filesystemBackend) loadAllCalendarObjects(ctx context.Context, urlPath string, propFilter []string) ([]caldav.CalendarObject, error) { @@ -88,17 +89,12 @@ func (b *filesystemBackend) loadAllCalendarObjects(ctx context.Context, urlPath b.RLock(filename) defer b.RUnlock(filename) - cal, err := calendarFromFile(filename, propFilter) + cal, etag, err := calendarAndEtagFromFile(filename, propFilter) if err != nil { fmt.Printf("load calendar error for %s: %v\n", filename, err) return err } - etag, err := etagForFile(filename) - if err != nil { - return err - } - // TODO can this potentially be called on a calendar object resource? // Would work (as Walk() includes root), except for the path construction below obj := caldav.CalendarObject{ @@ -108,6 +104,7 @@ func (b *filesystemBackend) loadAllCalendarObjects(ctx context.Context, urlPath ETag: etag, Data: cal, } + log.Debug().Str("path", obj.Path).Str("etag", etag).Int64("size", info.Size()).Msg("calendar object loaded") result = append(result, obj) return nil }) @@ -268,17 +265,12 @@ func (b *filesystemBackend) GetCalendarObject(ctx context.Context, objPath strin propFilter = req.Props } - calendar, err := calendarFromFile(localPath, propFilter) + calendar, etag, err := calendarAndEtagFromFile(localPath, propFilter) if err != nil { log.Debug().Str("path", localPath).Err(err).Msg("error reading calendar") return nil, err } - etag, err := etagForFile(localPath) - if err != nil { - return nil, err - } - obj := caldav.CalendarObject{ Path: objPath, ModTime: info.ModTime(), @@ -286,6 +278,7 @@ func (b *filesystemBackend) GetCalendarObject(ctx context.Context, objPath strin ETag: etag, Data: calendar, } + log.Debug().Str("path", objPath).Str("etag", etag).Int64("size", info.Size()).Msg("returning calendar object") return &obj, nil } @@ -373,16 +366,13 @@ func (b *filesystemBackend) PutCalendarObject(ctx context.Context, objPath strin } defer f.Close() - enc := ical.NewEncoder(f) + out := NewETagWriter(f) + enc := ical.NewEncoder(out) err = enc.Encode(calendar) if err != nil { return nil, err } - etag, err := etagForFile(localPath) - if err != nil { - return nil, err - } info, err := f.Stat() if err != nil { return nil, err @@ -392,9 +382,10 @@ func (b *filesystemBackend) PutCalendarObject(ctx context.Context, objPath strin Path: objPath, ModTime: info.ModTime(), ContentLength: info.Size(), - ETag: etag, + ETag: out.ETag(), Data: calendar, } + log.Debug().Str("path", r.Path).Str("etag", r.ETag).Int64("size", info.Size()).Msg("calendar object updated") return &r, nil } diff --git a/storage/filesystem_carddav.go b/storage/filesystem_carddav.go index 01f0c89..6cdc49c 100644 --- a/storage/filesystem_carddav.go +++ b/storage/filesystem_carddav.go @@ -68,21 +68,21 @@ func vcardPropFilter(card vcard.Card, props []string) vcard.Card { return result } -func vcardFromFile(path string, propFilter []string) (vcard.Card, error) { - +func vcardAndEtagFromFile(path string, propFilter []string) (vcard.Card, string, error) { f, err := os.Open(path) if err != nil { - return nil, err + return nil, "", err } defer f.Close() - dec := vcard.NewDecoder(f) + src := NewETagReader(f) + dec := vcard.NewDecoder(src) card, err := dec.Decode() if err != nil { - return nil, err + return nil, "", err } - return vcardPropFilter(card, propFilter), nil + return vcardPropFilter(card, propFilter), src.ETag(), nil } func (b *filesystemBackend) loadAllAddressObjects(ctx context.Context, urlPath string, propFilter []string) ([]carddav.AddressObject, error) { @@ -104,16 +104,10 @@ func (b *filesystemBackend) loadAllAddressObjects(ctx context.Context, urlPath s return nil } - // TODO use single file read for data & etag b.RLock(filename) defer b.RUnlock(filename) - card, err := vcardFromFile(filename, propFilter) - if err != nil { - return err - } - - etag, err := etagForFile(filename) + card, etag, err := vcardAndEtagFromFile(filename, propFilter) if err != nil { return err } @@ -127,6 +121,7 @@ func (b *filesystemBackend) loadAllAddressObjects(ctx context.Context, urlPath s ETag: etag, Card: card, } + log.Debug().Str("path", obj.Path).Str("etag", etag).Int64("size", info.Size()).Msg("address object loaded") result = append(result, obj) return nil }) @@ -335,17 +330,12 @@ func (b *filesystemBackend) GetAddressObject(ctx context.Context, objPath string propFilter = req.Props } - card, err := vcardFromFile(localPath, propFilter) + card, etag, err := vcardAndEtagFromFile(localPath, propFilter) if err != nil { log.Debug().Str("path", localPath).Err(err).Msg("error reading calendar") return nil, err } - etag, err := etagForFile(localPath) - if err != nil { - return nil, err - } - obj := carddav.AddressObject{ Path: objPath, ModTime: info.ModTime(), @@ -353,6 +343,7 @@ func (b *filesystemBackend) GetAddressObject(ctx context.Context, objPath string ETag: etag, Card: card, } + log.Debug().Str("path", objPath).Str("etag", etag).Int64("size", info.Size()).Msg("returning address object") return &obj, nil } @@ -435,15 +426,12 @@ func (b *filesystemBackend) PutAddressObject(ctx context.Context, objPath string } defer f.Close() - enc := vcard.NewEncoder(f) + out := NewETagWriter(f) + enc := vcard.NewEncoder(out) if err := enc.Encode(card); err != nil { return nil, err } - etag, err := etagForFile(localPath) - if err != nil { - return nil, err - } info, err := f.Stat() if err != nil { return nil, err @@ -453,9 +441,10 @@ func (b *filesystemBackend) PutAddressObject(ctx context.Context, objPath string Path: objPath, ModTime: info.ModTime(), ContentLength: info.Size(), - ETag: etag, + ETag: out.ETag(), Card: card, } + log.Debug().Str("path", r.Path).Str("etag", r.ETag).Int64("size", info.Size()).Msg("address object updated") return &r, nil }