fiber
fiber copied to clipboard
gofiber
📝 [v3 Proposal]: Improve Storage Interface
Feature Proposal Description
Idea One
Storage interface doesn't allow us to pass context to make operations cancellable. We should add new methods to allow context operations. Something like:
type Storage interface {
Get(key string) ([]byte, error)
Set(key string, val []byte, exp time.Duration) error
Delete(key string) error
Reset() error
Close() error
GetWithContext(ctx context.Context, key string) ([]byte, error)
SetWithContext(ctx context.Context, key string, val []byte, exp time.Duration) error
DeleteWithContext(ctx context.Context, key string) error
ResetWithContext(ctx context.Context) error
}
Idea Two
We can allow using generics to change key and value types.
type Storage[K ~comparable, V any] interface {
// Get gets the value for the given key.
// `nil, nil` is returned when the key does not exist
Get(key K) (V, error)
// Set stores the given value for the given key along
// with an expiration value, 0 means no expiration.
// Empty key or value will be ignored without an error.
Set(key K, val V, exp time.Duration) error
// Delete deletes the value for the given key.
// It returns no error if the storage does not contain the key,
Delete(key K) error
// Reset resets the storage and delete all keys.
Reset() error
// Close closes the storage and will stop any running garbage
// collectors and open connections.
Close() error
}
Alignment with Express API
Express does not have storage interface.
HTTP RFC Standards Compliance
Not related to core.
API Stability
Not related to core.
Feature Examples
.
Checklist:
- [X] I agree to follow Fiber's Code of Conduct.
- [X] I have searched for existing issues that describe my proposal before opening this one.
- [X] I understand that a proposal that does not meet these guidelines may be closed without explanation.
Current
https://github.com/gofiber/fiber/blob/847a4a959d979c46c3a0afd47409929bd158a7f0/app.go#L43-L63
i like the first idea
// Storage defines the base storage interface.
type StorageV2 interface { // `V2` this allows us to differentiate between old and new interfaces and support at least the last 3, so that we have the possibility to make further adjustments in the future
Get(key string) ([]byte, error)
Set(key string, val []byte, exp time.Duration) error
Delete(key string) error
Reset() error
Close() error
WithContext() ContextAwareStorage // Methode, gives the ContextAwareStorage-Interface
}
// ContextAwareStorage defines the storage interface with context-aware methods.
type ContextAwareStorage interface {
Get(ctx context.Context, key string) ([]byte, error)
Set(ctx context.Context, key string, val []byte, exp time.Duration) error
Delete(ctx context.Context, key string) error
Reset(ctx context.Context) error
Close(ctx context.Context) error
}
but maybe we don't need the old methods anymore
2nd idea for the storages we should have fixed input parameters for key and value, because we have to align everything in the storages of the underlying packages to them however, we could also create general conversion methods that allow the old data types as output, but this may be too complex
I feel like having a version that doesn't take a context is redundant because people can just pass context.TODO() if they dont need a context. Just my personal opinion though.
The benefit here would be more than just the ability to cancel. I'm trying to add open telemetry tracing to my redis storage, but I can't pass the context, so it's not aware of its trace parent. The redis client just has hardcoded context.Background() hardcoded in: https://github.com/gofiber/storage/blob/7479b8518e8feacd7bedd50e6dd6711c1c4c8bbb/redis/redis.go#L108
