-
Notifications
You must be signed in to change notification settings - Fork 112
Feature: conditional upload API #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 12 commits
186f3f3
fe792ef
e63bf4b
1bb876b
5d10115
b395c1c
e5c31be
328eae5
9ee1894
dd65867
d369478
b0814c7
ee34e75
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 |
|---|---|---|
|
|
@@ -6,8 +6,10 @@ package objstore | |
| import ( | ||
| "bytes" | ||
| "context" | ||
| "fmt" | ||
| "io" | ||
| "sort" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
|
|
@@ -16,6 +18,7 @@ import ( | |
| ) | ||
|
|
||
| var errNotFound = errors.New("inmem: object not found") | ||
| var errConditionNotMet = errors.New("inmem: condition not met") | ||
|
|
||
| // InMemBucket implements the objstore.Bucket interfaces against local memory. | ||
| // Methods from Bucket interface are thread-safe. Objects are assumed to be immutable. | ||
|
|
@@ -148,6 +151,10 @@ func (i *InMemBucket) SupportedIterOptions() []IterOptionType { | |
| return []IterOptionType{Recursive, UpdatedAt} | ||
| } | ||
|
|
||
| func (b *InMemBucket) SupportedObjectUploadOptions() []ObjectUploadOptionType { | ||
| return []ObjectUploadOptionType{IfNotExists, IfMatch, IfNotMatch} | ||
| } | ||
|
|
||
| func (b *InMemBucket) IterWithAttributes(ctx context.Context, dir string, f func(attrs IterObjectAttributes) error, options ...IterOption) error { | ||
| if err := ValidateIterOptions(b.SupportedIterOptions(), options...); err != nil { | ||
| return err | ||
|
|
@@ -252,9 +259,40 @@ func (b *InMemBucket) Attributes(_ context.Context, name string) (ObjectAttribut | |
| } | ||
|
|
||
| // Upload writes the file specified in src to into the memory. | ||
| func (b *InMemBucket) Upload(_ context.Context, name string, r io.Reader, _ ...ObjectUploadOption) error { | ||
| func (b *InMemBucket) Upload(_ context.Context, name string, r io.Reader, opts ...ObjectUploadOption) error { | ||
| if err := ValidateUploadOptions(b.SupportedObjectUploadOptions(), opts...); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| b.mtx.Lock() | ||
| defer b.mtx.Unlock() | ||
|
|
||
| params := ApplyObjectUploadOptions(opts...) | ||
|
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. shall we validate the options here?
Author
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. Whoops - done! |
||
| generation := 0 | ||
|
|
||
| if prev, ok := b.attrs[name]; ok { | ||
| if prev.Version == nil || prev.Version.Type != Generation { | ||
| return fmt.Errorf("inmem object should always have a generational version") | ||
| } | ||
| if params.IfNotExists { | ||
| return errConditionNotMet | ||
| } | ||
| var err error | ||
| if generation, err = strconv.Atoi(prev.Version.Value); err != nil { | ||
| return err | ||
| } | ||
| if params.Condition != nil { | ||
| if params.Condition.Value != prev.Version.Value && !params.IfNotMatch { | ||
| return errConditionNotMet | ||
| } else if params.Condition.Value == prev.Version.Value && params.IfNotMatch { | ||
| return errConditionNotMet | ||
| } | ||
| } | ||
| } else if params.Condition != nil && !params.IfNotMatch { | ||
| return errConditionNotMet | ||
| } | ||
| generation++ | ||
|
|
||
| body, err := io.ReadAll(r) | ||
| if err != nil { | ||
| return err | ||
|
|
@@ -263,6 +301,7 @@ func (b *InMemBucket) Upload(_ context.Context, name string, r io.Reader, _ ...O | |
| b.attrs[name] = ObjectAttributes{ | ||
| Size: int64(len(body)), | ||
| LastModified: time.Now(), | ||
| Version: &ObjectVersion{Type: Generation, Value: strconv.Itoa(generation)}, | ||
| } | ||
| return nil | ||
| } | ||
|
|
@@ -289,6 +328,8 @@ func (b *InMemBucket) IsAccessDeniedErr(err error) bool { | |
| return false | ||
| } | ||
|
|
||
| func (b *InMemBucket) IsConditionNotMetErr(err error) bool { return errors.Is(err, errConditionNotMet) } | ||
|
|
||
| func (b *InMemBucket) Close() error { return nil } | ||
|
|
||
| // Name returns the bucket name. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,9 @@ type Bucket interface { | |
| // Upload should be idempotent. | ||
| Upload(ctx context.Context, name string, r io.Reader, opts ...ObjectUploadOption) error | ||
|
|
||
| // SupportedObjectUploadOptions returns a list of ObjectUploadOptions supported by the underlying provider. | ||
| SupportedObjectUploadOptions() []ObjectUploadOptionType | ||
|
|
||
| // Delete removes the object with the given name. | ||
| // If object does not exist in the moment of deletion, Delete should throw error. | ||
| Delete(ctx context.Context, name string) error | ||
|
|
@@ -119,6 +122,9 @@ type BucketReader interface { | |
| // IsAccessDeniedErr returns true if access to object is denied. | ||
| IsAccessDeniedErr(err error) bool | ||
|
|
||
| // IsConditionNotMetErr returns true if an ObjectUploadOption condition parameter (IfNotExists, IfMatch, IfNotMatch) was not met. | ||
| IsConditionNotMetErr(err error) bool | ||
|
|
||
| // Attributes returns information about the specified object. | ||
| Attributes(ctx context.Context, name string) (ObjectAttributes, error) | ||
| } | ||
|
|
@@ -196,26 +202,6 @@ func ApplyIterOptions(options ...IterOption) IterParams { | |
| return out | ||
| } | ||
|
|
||
| type UploadObjectParams struct { | ||
| ContentType string | ||
| } | ||
|
|
||
| type ObjectUploadOption func(f *UploadObjectParams) | ||
|
|
||
| func WithContentType(contentType string) ObjectUploadOption { | ||
| return func(f *UploadObjectParams) { | ||
| f.ContentType = contentType | ||
| } | ||
| } | ||
|
|
||
| func ApplyObjectUploadOptions(opts ...ObjectUploadOption) UploadObjectParams { | ||
| out := UploadObjectParams{} | ||
| for _, opt := range opts { | ||
| opt(&out) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| // DownloadOption configures the provided params. | ||
| type DownloadOption func(params *downloadParams) | ||
|
|
||
|
|
@@ -274,12 +260,139 @@ func applyUploadOptions(options ...UploadOption) uploadParams { | |
| return out | ||
| } | ||
|
|
||
| var ErrUploadOptionNotSupported = errors.New("upload option is not supported") | ||
| var ErrUploadOptionInvalid = errors.New("upload option is invalid") | ||
|
|
||
| // ObjectUploadOptionType is used for type-safe option support checking of ObjectUpload options. | ||
| type ObjectUploadOptionType int | ||
|
|
||
| const ( | ||
| ContentType ObjectUploadOptionType = iota | ||
| IfNotExists | ||
| IfMatch | ||
| IfNotMatch | ||
| ) | ||
|
|
||
| // ObjectUploadOption configures UploadObjectParams. | ||
| type ObjectUploadOption struct { | ||
| Type ObjectUploadOptionType | ||
|
Member
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 don't like that these two are disjointed now and users can misuse this ie create some
Author
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. No fair enough, although I think there may be other places where symbols are overexposed maybe. Either way, easy change to make and very happy to do. |
||
| Apply func(params *UploadObjectParams) | ||
| } | ||
|
|
||
| // UploadObjectParams hold content-type and conditional write attribute metadata for upload operations that are | ||
| // supported by some provider implementations. | ||
| type UploadObjectParams struct { | ||
| ContentType string | ||
| IfNotExists bool | ||
| IfNotMatch bool | ||
| Condition *ObjectVersion | ||
| } | ||
|
|
||
| // WithContentType sets the content type of the object upload operation. | ||
| func WithContentType(contentType string) ObjectUploadOption { | ||
| return ObjectUploadOption{ | ||
| Type: ContentType, | ||
| Apply: func(params *UploadObjectParams) { | ||
| params.ContentType = contentType | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // WithIfNotExists if supported by the provider, only writes the object if the object does not already exist. | ||
| // When supported by providers this operation is usually atomic, however this is dependent on the provider. | ||
| func WithIfNotExists() ObjectUploadOption { | ||
| return ObjectUploadOption{ | ||
| Type: IfNotExists, | ||
| Apply: func(params *UploadObjectParams) { | ||
| params.IfNotExists = true | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // WithIfMatch if supported by the provider, only writes the object if the ETag value of the object in S3 matches the provided value, | ||
| // otherwise, the operation fails. | ||
| func WithIfMatch(ver *ObjectVersion) ObjectUploadOption { | ||
| return ObjectUploadOption{ | ||
| Type: IfMatch, | ||
| Apply: func(params *UploadObjectParams) { | ||
| params.Condition = ver | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // WithIfNotMatch if supported by the provider, only writes the object if the ETag value of the object in S3 does *not* match the provided value, | ||
| // otherwise, the operation fails. | ||
| func WithIfNotMatch(ver *ObjectVersion) ObjectUploadOption { | ||
| return ObjectUploadOption{ | ||
| Type: IfNotMatch, | ||
| Apply: func(params *UploadObjectParams) { | ||
| params.Condition = ver | ||
| params.IfNotMatch = true | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // ValidateUploadOptions ensures that only supported options are passed as options, and that options used simultaneously are valid. | ||
| func ValidateUploadOptions(supportedOptions []ObjectUploadOptionType, opts ...ObjectUploadOption) error { | ||
|
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. maybe we should also validate
Author
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. Sounds sensible - done. |
||
|
|
||
|
Member
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. Typically we don't put a empty line just after the definition.
Author
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. Will do |
||
| for _, opt := range opts { | ||
| if !slices.Contains(supportedOptions, opt.Type) { | ||
| return fmt.Errorf("%w: %d", ErrUploadOptionNotSupported, opt.Type) | ||
| } | ||
| if opt.Type == IfMatch || opt.Type == IfNotMatch { | ||
| candidate := &UploadObjectParams{} | ||
| opt.Apply(candidate) | ||
| if candidate.Condition == nil { | ||
| return fmt.Errorf("%w: Condition nil", ErrUploadOptionInvalid) | ||
| } | ||
| } | ||
| if opt.Type == IfNotExists { | ||
| // If IfNotExists provided alongside IfMatch or IfNotMatch. | ||
| if slices.ContainsFunc(opts, func(opt ObjectUploadOption) bool { | ||
| return opt.Type == IfMatch || opt.Type == IfNotMatch | ||
| }) { | ||
| return fmt.Errorf("%w: IfNotExists not valid with IfMatch or IfNotMatch", ErrUploadOptionInvalid) | ||
| } | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // ApplyObjectUploadOptions creates UploadObjectParams from the options. | ||
| func ApplyObjectUploadOptions(opts ...ObjectUploadOption) UploadObjectParams { | ||
| out := UploadObjectParams{} | ||
| for _, opt := range opts { | ||
| opt.Apply(&out) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| type ObjectAttributes struct { | ||
| // Size is the object size in bytes. | ||
| Size int64 `json:"size"` | ||
|
|
||
| // LastModified is the timestamp the object was last modified. | ||
| LastModified time.Time `json:"last_modified"` | ||
|
|
||
| // ObjectVersion represents an etag, generation or revision that can be used as a version in conditional updates, if supported. | ||
| Version *ObjectVersion `json:"version,omitempty"` | ||
| } | ||
|
|
||
| // ObjectVersionType is used to specify the type of object version used by the underlying provider. | ||
| type ObjectVersionType int | ||
|
|
||
| const ( | ||
| // Generation the provider supports a monotonically increasing integer version. | ||
| Generation ObjectVersionType = iota | ||
| // ETag the provider supports a hash or checksum version. | ||
| ETag ObjectVersionType = iota | ||
| ) | ||
|
|
||
| type ObjectVersion struct { | ||
| // Type is the type of object version supported by the provider. | ||
| Type ObjectVersionType | ||
| // Value is a string representation of the version data from the provider. | ||
| Value string | ||
| } | ||
|
|
||
| type IterObjectAttributes struct { | ||
|
|
@@ -387,14 +500,14 @@ func UploadDir(ctx context.Context, logger log.Logger, bkt Bucket, srcdir, dstdi | |
|
|
||
| // UploadFile uploads the file with the given name to the bucket. | ||
| // It is a caller responsibility to clean partial upload in case of failure. | ||
| func UploadFile(ctx context.Context, logger log.Logger, bkt Bucket, src, dst string) error { | ||
| func UploadFile(ctx context.Context, logger log.Logger, bkt Bucket, src, dst string, opts ...ObjectUploadOption) error { | ||
| r, err := os.Open(filepath.Clean(src)) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "open file %s", src) | ||
| } | ||
| defer logerrcapture.Do(logger, r.Close, "close file %s", src) | ||
|
|
||
| if err := bkt.Upload(ctx, dst, r); err != nil { | ||
| if err := bkt.Upload(ctx, dst, r, opts...); err != nil { | ||
| return errors.Wrapf(err, "upload file %s as %s", src, dst) | ||
| } | ||
| level.Debug(logger).Log("msg", "uploaded file", "from", src, "dst", dst, "bucket", bkt.Name()) | ||
|
|
@@ -681,6 +794,10 @@ func (b *metricBucket) SupportedIterOptions() []IterOptionType { | |
| return b.bkt.SupportedIterOptions() | ||
| } | ||
|
|
||
| func (b *metricBucket) SupportedObjectUploadOptions() []ObjectUploadOptionType { | ||
| return b.bkt.SupportedObjectUploadOptions() | ||
| } | ||
|
|
||
| func (b *metricBucket) Attributes(ctx context.Context, name string) (ObjectAttributes, error) { | ||
| const op = OpAttributes | ||
| b.metrics.ops.WithLabelValues(op).Inc() | ||
|
|
@@ -821,6 +938,8 @@ func (b *metricBucket) IsAccessDeniedErr(err error) bool { | |
| return b.bkt.IsAccessDeniedErr(err) | ||
| } | ||
|
|
||
| func (b *metricBucket) IsConditionNotMetErr(err error) bool { return b.bkt.IsConditionNotMetErr(err) } | ||
|
|
||
| func (b *metricBucket) Close() error { | ||
| return b.bkt.Close() | ||
| } | ||
|
|
||
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.
Some random whitespace issues 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.
Will resolve.