📝 [v3 Proposal]: Improve Storage Interface

[efectn]

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:

• I agree to follow Fiber's Code of Conduct.
• I have searched for existing issues that describe my proposal before opening this one.
• I understand that a proposal that does not meet these guidelines may be closed without explanation.

[ReneWerner87]

Current

fiber/app.go

Lines 43 to 63 in 847a4a9

	type Storage interface {
	// Get gets the value for the given key.
	// `nil, nil` is returned when the key does not exist
	Get(key string) ([]byte, 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 string, val []byte, 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 string) 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
	}

[ReneWerner87]

related #2300

we should then continue

[ReneWerner87]

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

[nickajacks1]

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.

[half2me]

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