🔥 feat: Add support for ObjectBox Storage Driver #1534
Conversation
WalkthroughThis change introduces a new ObjectBox-based storage driver, including its implementation, configuration, entity model, and test suite. It provides full CRUD operations, expiration handling, periodic cleanup, and comprehensive documentation. The update also includes generated ObjectBox bindings and benchmarks to validate performance and correctness. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Storage
participant ObjectBox
participant CacheBox
App->>Storage: New(config)
Storage->>ObjectBox: Initialize DB with config
ObjectBox->>CacheBox: Create entity box
App->>Storage: Set(key, value, exp)
Storage->>CacheBox: Query for key
alt Key exists
Storage->>CacheBox: Update entry
else Key does not exist
Storage->>CacheBox: Insert entry
end
App->>Storage: Get(key)
Storage->>CacheBox: Query for key
alt Entry found and not expired
CacheBox-->>Storage: Return value
else Not found or expired
CacheBox-->>Storage: Return nil
end
App->>Storage: Delete(key)
Storage->>CacheBox: Remove entry
App->>Storage: Reset()
Storage->>CacheBox: Remove all entries
Note over Storage: Background goroutine
loop Every cleanerInterval
Storage->>CacheBox: Remove expired entries
end
App->>Storage: Close()
Storage->>ObjectBox: Close DB
Estimated code review effort3 (~45 minutes) Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@gaby done |
|
@karnadii Thanks, I will work on updating your branch here |
gaby
changed the title
|
@karnadii Can you take a look at this PR? |
|
@karnadii ping |
|
@gaby @ReneWerner87 sorry for the late reply, I just tested it, everything working without a problem, all test pass, can this be merged? |
karnadii
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
objectbox/config.go (1)
57-59: Improve duration validation to handle edge cases.The current check
int(cfg.CleanerInterval.Seconds()) == 0may not properly handle very small positive durations that round down to 0 seconds. This could cause valid but very small durations to be replaced with the default.Consider using a more precise check:
- if int(cfg.CleanerInterval.Seconds()) == 0 { + if cfg.CleanerInterval == 0 { cfg.CleanerInterval = DefaultConfig.CleanerInterval }objectbox/objectbox_test.go (1)
87-87: Avoid hardcoded sleep times in tests to prevent flakiness.Using
time.Sleep()with fixed durations can make tests flaky, especially in CI environments with variable performance.Consider using a more deterministic approach for expiration testing:
- time.Sleep(2 * time.Second) + // Poll for expiration with timeout + timeout := time.After(3 * time.Second) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case <-timeout: + t.Fatal("Key did not expire within expected time") + case <-ticker.C: + got, err := store.Get(key) + require.NoError(t, err) + if got == nil { + return // Key expired as expected + } + } + }Also applies to: 167-168
objectbox/README.md (1)
35-35: Fix typo in installation instruction.There's a typo in "ojectbox" - it should be "objectbox".
-[Install ojectbox](https://golang.objectbox.io/install) +[Install ObjectBox](https://golang.objectbox.io/install)objectbox/objectbox.go (3)
48-69: Optimize query performance and improve readability.The current query combines multiple conditions which may not be optimal. Consider breaking this into separate queries or using ObjectBox's more efficient query patterns.
func (s *Storage) Get(key string) ([]byte, error) { if len(key) < 1 { return nil, nil } - query := s.box.Query(Cache_.Key.Equals(key, true), - objectbox.Any( - Cache_.ExpiresAt.Equals(0), - Cache_.ExpiresAt.GreaterThan(time.Now().Unix()), - )) - caches, err := query.Find() + // First try to find by key, then check expiration in application logic + query := s.box.Query(Cache_.Key.Equals(key, true)) + caches, err := query.Find() if err != nil { return nil, err } - if len(caches) < 1 { + if len(caches) == 0 { return nil, nil } - return caches[0].Value, nil + cache := caches[0] + // Check expiration + if cache.ExpiresAt > 0 && cache.ExpiresAt <= time.Now().Unix() { + return nil, nil + } + + return cache.Value, nil }
74-76: Inconsistent validation logic for empty values.The validation uses different comparison operators (
<=vs<) for key and value length checks, and the key validation inGetuses< 1while here it uses<= 0. Consider standardizing the validation logic.- if len(key) <= 0 || len(value) <= 0 { + if len(key) == 0 || len(value) == 0 { return nil }
146-146: Document the decision to ignore cleanup errors.While ignoring errors during cleanup might be acceptable, it would be helpful for maintainability to document why this decision was made.
-func (s *Storage) cleanStorage() { - s.box.Query(Cache_.ExpiresAt.LessThan(time.Now().Unix())).Remove() //nolint:errcheck // It is fine to ignore the error +func (s *Storage) cleanStorage() { + // Ignore errors during cleanup as it's a best-effort operation that shouldn't affect the main functionality + s.box.Query(Cache_.ExpiresAt.LessThan(time.Now().Unix())).Remove() //nolint:errcheck }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
objectbox/go.modis excluded by!**/*.modobjectbox/go.sumis excluded by!**/*.sum,!**/*.sumobjectbox/objectbox-model.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
objectbox/README.md(1 hunks)objectbox/cache.go(1 hunks)objectbox/cache.obx.go(1 hunks)objectbox/config.go(1 hunks)objectbox/objectbox-model.go(1 hunks)objectbox/objectbox.go(1 hunks)objectbox/objectbox_test.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
objectbox/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 861 characters long)
Context: .../tag/gofiber/storage?filter=objectbox*) 
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (8)
objectbox/objectbox-model.go (1)
1-20: Generated code looks correct.This file is properly marked as generated code and should not be manually edited. The ObjectBox model setup appears correct with proper entity registration and ID management.
objectbox/cache.go (1)
1-11: Well-designed entity structure.The Cache entity is properly structured with appropriate ObjectBox tags:
- Unique index on Key for efficient key-value lookups
- Index on ExpiresAt for efficient cleanup of expired entries
- Proper use of date type for timestamp field
The go:generate directive ensures the ObjectBox bindings are generated correctly.
objectbox/objectbox_test.go (1)
196-196: ```shell
#!/bin/bashDetermine the package declaration in the test file
rg -m 1 '^package ' objectbox/objectbox_test.go
</details> <details> <summary>objectbox/README.md (3)</summary> `6-6`: **Badge markdown formatting is acceptable.** The static analysis tool flagged "excessive exclamation marks" in the badges, but this is standard markdown badge syntax and not actually excessive punctuation in the content. --- `87-87`: **Ensure documentation matches actual default values.** The documentation states the default MaxSizeInKb is "1GB (1024 * 1024)" but this should be consistent with the actual implementation. If the config.go issue is fixed to use 1GB, this documentation is correct. Verify that the documentation matches the final default values after fixing the config.go inconsistency. Also applies to: 112-112 --- `1-117`: **Comprehensive and well-structured documentation.** The README provides excellent coverage of: - Installation instructions (including ObjectBox setup) - Usage examples with both default and custom configurations - Complete API documentation - Detailed configuration options This will help users effectively adopt the ObjectBox storage driver. </details> <details> <summary>objectbox/objectbox.go (1)</summary> `149-162`: **LGTM: Well-implemented cleanup routine.** The cleanup routine follows good practices with proper goroutine lifecycle management, ticker cleanup, and graceful shutdown via the done channel. </details> <details> <summary>objectbox/cache.obx.go (1)</summary> `1-365`: **Generated code follows ObjectBox conventions correctly.** This is a properly generated ObjectBox binding file that provides type-safe CRUD operations for the `Cache` entity. The generated code includes: - Entity metadata and property definitions - Typed box operations (CacheBox) - Asynchronous operations (CacheAsyncBox) - Query functionality (CacheQuery) - Proper FlatBuffer serialization/deserialization The integration points with the main `objectbox.go` file are correct, particularly the use of `BoxForCache()` and the `Cache_` property helpers for queries. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
objectbox/config.go
Outdated
| // Optional, defaults to 1GB (1024 * 1024 * 1024) | ||
| MaxSizeInKb uint64 |
There was a problem hiding this comment.
Fix inconsistency between comment and default value.
The comment states "defaults to 1GB (1024 * 1024 * 1024)" but the actual default value is 1024 * 1024 which equals 1MB, not 1GB.
Apply this diff to fix the inconsistency:
- // MaxSizeInKb sets the maximum size of the database in kilobytes.
- // Optional, defaults to 1GB (1024 * 1024 * 1024)
+ // MaxSizeInKb sets the maximum size of the database in kilobytes.
+ // Optional, defaults to 1GB (1024 * 1024)Or if 1GB was intended, update the default value:
- MaxSizeInKb: 1024 * 1024, // 1GB
+ MaxSizeInKb: 1024 * 1024 * 1024, // 1GBAlso applies to: 30-30
🤖 Prompt for AI Agents
In objectbox/config.go around lines 12 to 13 and line 30, the comment states the
default MaxSizeInKb is 1GB (1024 * 1024 * 1024), but the actual default value is
1024 * 1024 (1MB). To fix this inconsistency, either update the comment to
reflect the actual default of 1MB or change the default value to 1024 * 1024 *
1024 to match the 1GB stated in the comment. Ensure both the comment and the
default value are consistent.
| if err != nil { | ||
| t.Errorf("Failed concurrent set: %v", err) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in concurrent test goroutines.
The current error handling in goroutines uses t.Errorf which is not safe for concurrent access and may not properly fail the test.
Use proper error handling for concurrent tests:
- err := store.Set(key, value, time.Hour)
- if err != nil {
- t.Errorf("Failed concurrent set: %v", err)
- }
-
- _, err = store.Get(key)
- if err != nil {
- t.Errorf("Failed concurrent get: %v", err)
- }
+ err := store.Set(key, value, time.Hour)
+ if err != nil {
+ select {
+ case done <- false:
+ default:
+ }
+ return
+ }
+
+ _, err = store.Get(key)
+ if err != nil {
+ select {
+ case done <- false:
+ default:
+ }
+ return
+ }And update the collection logic to handle failures:
for i := 0; i < goroutines; i++ {
- <-done
+ success := <-done
+ if !success {
+ t.Fatal("Concurrent operation failed")
+ }
}Also applies to: 147-149
🤖 Prompt for AI Agents
In objectbox/objectbox_test.go around lines 142 to 144 and also 147 to 149, the
use of t.Errorf inside concurrent goroutines is unsafe and may not correctly
fail the test. Replace t.Errorf with a thread-safe mechanism such as sending
errors to a channel or using sync/atomic to record errors, then check and fail
the test after all goroutines complete. Update the test logic to collect and
handle these errors properly outside the goroutines to ensure reliable test
failure reporting.
objectbox/objectbox.go
Outdated
| ob, err := objectbox.NewBuilder().Model(ObjectBoxModel()).MaxSizeInKb(cfg.MaxSizeInKb).MaxReaders(cfg.MaxReaders).Directory(cfg.Directory).Build() | ||
| if err != nil { | ||
| panic(err) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider returning errors instead of panicking during initialization.
While panicking ensures the application fails fast on configuration errors, it prevents graceful error handling and recovery. Consider returning an error from New() to allow callers to handle initialization failures appropriately.
-func New(config ...Config) *Storage {
+func New(config ...Config) (*Storage, error) {
cfg := getConfig(config...)
ob, err := objectbox.NewBuilder().Model(ObjectBoxModel()).MaxSizeInKb(cfg.MaxSizeInKb).MaxReaders(cfg.MaxReaders).Directory(cfg.Directory).Build()
if err != nil {
- panic(err)
+ return nil, err
}🤖 Prompt for AI Agents
In objectbox/objectbox.go around lines 21 to 24, the code panics on error during
initialization which prevents graceful error handling. Modify the New() function
to return an error instead of panicking. Change the function signature to return
(objectbox, error), and on error, return nil and the error to allow callers to
handle initialization failures properly.
objectbox/objectbox.go
Outdated
| if cfg.Reset { | ||
| box := BoxForCache(ob) | ||
| err = box.RemoveAll() | ||
|
|
||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle reset operation errors more gracefully.
Similar to the initialization, consider returning the error instead of panicking to allow callers to handle reset failures.
if cfg.Reset {
box := BoxForCache(ob)
err = box.RemoveAll()
if err != nil {
- panic(err)
+ return nil, err
}
}🤖 Prompt for AI Agents
In objectbox/objectbox.go around lines 26 to 33, the reset operation currently
panics on error, which is not graceful. Modify the code to return the error
instead of panicking, so that callers can handle reset failures properly. Update
the function signature if needed to propagate the error upwards.
|
@karnadii can you update the branch with the latest changes from master/main |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new storage driver for ObjectBox. I've found a critical issue related to the cleanup of non-expiring items, along with several medium-severity suggestions to improve code clarity, test isolation, and error handling. Once the critical issue is addressed, this will be a great addition.
objectbox/objectbox.go
Outdated
|
|
||
| // cleanStorage removes all expired cache entries. | ||
| func (s *Storage) cleanStorage() { | ||
| s.box.Query(Cache_.ExpiresAt.LessThan(time.Now().Unix())).Remove() //nolint:errcheck // It is fine to ignore the error |
There was a problem hiding this comment.
The current implementation of cleanStorage will incorrectly remove items that are meant to never expire. Items with ExpiresAt set to 0 are considered non-expiring by the Get method, but the query in cleanStorage will select them for deletion because 0 is always less than the current Unix timestamp.
To fix this, the query should explicitly exclude items where ExpiresAt is 0.
| s.box.Query(Cache_.ExpiresAt.LessThan(time.Now().Unix())).Remove() //nolint:errcheck // It is fine to ignore the error | |
| s.box.Query(Cache_.ExpiresAt.NotEquals(0), Cache_.ExpiresAt.LessThan(time.Now().Unix())).Remove() |
objectbox/objectbox_test.go
Outdated
| }{ | ||
| {"expired1", []byte("value1"), -1 * time.Second}, | ||
| {"expired2", []byte("value2"), -2 * time.Second}, | ||
| {"valid1", []byte("value3"), 1 * time.Hour}, | ||
| {"valid2", []byte("value4"), 2 * time.Hour}, | ||
| } |
There was a problem hiding this comment.
The test suite for the cleaner is missing a case for non-expiring items (i.e., where expiry is 0). Adding this case is crucial as it would help identify the bug in the cleanStorage function.
Please add a test case for a non-expiring item and assert that it is not removed after the cleanup process.
tests := []struct {
key string
value []byte
expiry time.Duration
}{
{"expired1", []byte("value1"), -1 * time.Second},
{"expired2", []byte("value2"), -2 * time.Second},
{"no_expiry", []byte("value_no_expiry"), 0},
{"valid1", []byte("value3"), 1 * time.Hour},
{"valid2", []byte("value4"), 2 * time.Hour},
}
objectbox/objectbox.go
Outdated
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| if cfg.Reset { | ||
| box := BoxForCache(ob) | ||
| err = box.RemoveAll() | ||
|
|
||
| if err != nil { | ||
| panic(err) | ||
| } |
There was a problem hiding this comment.
The New function panics if the ObjectBox database fails to initialize or reset. It's more idiomatic in Go to return an error. This allows the calling code to decide how to handle the initialization failure.
func New(config ...Config) (*Storage, error) {
ob, err := objectbox.NewBuilder().Model(ObjectBoxModel()).MaxSizeInKb(cfg.MaxSizeInKb).MaxReaders(cfg.MaxReaders).Directory(cfg.Directory).Build()
if err != nil {
return nil, err
}
if cfg.Reset {
box := BoxForCache(ob)
err = box.RemoveAll()
if err != nil {
return nil, err
}
}
storage := &Storage{
ob: ob,
box: BoxForCache(ob),
done: make(chan struct{}),
}
go storage.cleanerTicker(cfg.CleanerInterval)
return storage, nil
}| // Set stores a value in cache with the specified key and expiration. | ||
| // If expiration is 0, the entry won't expire. | ||
| func (s *Storage) Set(key string, value []byte, exp time.Duration) error { | ||
| if len(key) <= 0 || len(value) <= 0 { |
There was a problem hiding this comment.
The current implementation of Set ignores calls with an empty value. Confirm if this is the intended behavior across all storage drivers in this repository. If storing empty values should be supported, the check for len(value) <= 0 should be removed.
if len(key) <= 0 {
return nil
}
objectbox/config.go
Outdated
| cfg.MaxReaders = DefaultConfig.MaxReaders | ||
| } | ||
|
|
||
| if int(cfg.CleanerInterval.Seconds()) == 0 { |
There was a problem hiding this comment.
The check for a zero CleanerInterval can be simplified by comparing directly to 0.
| if int(cfg.CleanerInterval.Seconds()) == 0 { | |
| if cfg.CleanerInterval == 0 { | |
| cfg.CleanerInterval = DefaultConfig.CleanerInterval | |
| } |
objectbox/objectbox_test.go
Outdated
| var store = New(Config{ | ||
| Reset: true, | ||
| Directory: "test_db", | ||
| MaxSizeInKb: 1024, | ||
| MaxReaders: 10, | ||
| CleanerInterval: 60 * time.Second, | ||
| }) |
There was a problem hiding this comment.
The tests share a single global store instance. This can lead to tests interfering with each other. Ensure tests are isolated by creating a new store instance for each test function or by using t.Cleanup to reset the state after each test.
func newTestStore(t *testing.T) *Storage {
t.Helper()
store := New(Config{
Reset: true,
Directory: t.TempDir(), // Use a temporary directory for each test
})
t.Cleanup(func() {
store.Close()
})
return store
}
func Test_ObjectBox_Set_And_Get(t *testing.T) {
store := newTestStore(t)
// ... test logic ...
}
|
@karnadii I have had our 3 ai bots analyze your code, you can look at their tips and implement them if they make sense |
…implementation - Add objectbox module to go.work file - Fix typo in README installation link - Refactor objectbox.go for better readability and maintainability - Optimize storage operations by reducing redundant queries - Add input validation for empty keys/values in Set method
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
objectbox/README.md (3)
56-58: Importtimein the example so it compiles out-of-the-box
time.Secondis referenced in the custom-config snippet, but the corresponding import is missing. Readers copy-pasting this code will hit an “undefined: time” compile error.-import "github.com/gofiber/storage/objectbox/v2" +import ( + "time" + "github.com/gofiber/storage/objectbox/v2" +)
38-40: Consider warning users about piping curl to bashThe one-liner installer is convenient, but it silently executes remote code. A brief note (or link to verification instructions) would help security-conscious users.
Example wording:
⚠️ Review the script before executing or pass--dry-runto inspect the steps.
22-31: Clarify receiver notation in the signatures blockThe list mixes a free function (
New) with pointer-receiver methods but omits the package prefix, which may confuse newcomers skimming the docs. Adding the package alias (e.g.objectbox.Storage) or a short preface (“The methods below are defined on*Storage”) would improve clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.workis excluded by!**/*.work,!**/*.work
📒 Files selected for processing (2)
objectbox/README.md(1 hunks)objectbox/objectbox.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- objectbox/objectbox.go
🧰 Additional context used
🪛 LanguageTool
objectbox/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 862 characters long)
Context: .../tag/gofiber/storage?filter=objectbox*)
(EN_EXCESSIVE_EXCLAMATION)
refactor: reorganize go.mod dependencies docs: update default config values for clarity
- Fix cleanStorage bug to prevent removal of non-expiring entries. - Ensure Set operations are atomic by using a write transaction. - Correctly handle negative and zero-value expirations. - Enhance tests to verify fixes and preserve non-expiring entries.
…ssary benchmark - Improved getConfig to safely merge user settings without side effects. - Removed Benchmark_ObjectBox_Cleaner as it's not a primary API operation. - Reverted Set function to transactional for data integrity.
Refactor comments in config.go, objectbox.go, and objectbox_test.go to align with Go best practices. Update README.md for clarity and remove redundant information.
|
@ReneWerner87 could you take a look again? I have made a lot of changes |
|
Ok |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
objectbox/README.md (2)
35-37: Replace bare URL with a proper Markdown linkMarkdown-lint (MD034) complains about the naked link. Converting it to an inline link also makes the doc look more polished.
-https://golang.objectbox.io/install +[ObjectBox installation guide](https://golang.objectbox.io/install)
53-65: Example snippet is missing thetimeimport
CleanerInterval: 60 * time.Secondwon’t compile without importing thetimepackage. Either add a second import line or wrap the example in a full import block.-import "github.com/gofiber/storage/objectbox" +import ( + "github.com/gofiber/storage/objectbox" + "time" +)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/test-objectbox.ymlis excluded by!**/*.ymlobjectbox/go.modis excluded by!**/*.mod
📒 Files selected for processing (1)
objectbox/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
objectbox/README.md (1)
Learnt from: ReneWerner87
PR: gofiber/storage#0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
🪛 LanguageTool
objectbox/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 949 characters long)
Context: .../tag/gofiber/storage?filter=objectbox*)
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
objectbox/README.md
36-36: Bare URL used
(MD034, no-bare-urls)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
objectbox/README.md (3)
6-10: Release badge regex likely incorrect – badge will not resolve
Thefilter=objectbox*query parameter is not a valid regex for Shields-IO.
Use a proper regex such asfilter=^objectboxor remove the filter entirely to avoid a broken image link.- +
56-68: Minor: show zero expiration semantics inSetexample
Readers may wonder what to pass when no TTL is desired. A one-liner comment clarifies intent and mirrors behaviour in other Fiber storage docs.- CleanerInterval: 60 * time.Second, + CleanerInterval: 60 * time.Second, // delete expired keys every minute } // store.Set("foo", []byte("bar"), 0) // 0 means no expiration
73-99: Consider clarifying units forMaxSizeInKb
Strict “KB vs KiB” wording avoids ambiguity and helps users pick correct values under quotas.Suggestion:
// MaxSizeInKb sets the maximum database size in kibibytes (KiB).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
objectbox/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
objectbox/README.md (1)
Learnt from: ReneWerner87
PR: gofiber/storage#0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
🪛 LanguageTool
objectbox/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 946 characters long)
Context: .../tag/gofiber/storage?filter=objectbox*)
(EN_EXCESSIVE_EXCLAMATION)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
objectbox/README.md (2)
35-41: Mention CGO & generator prerequisites in the Installation sectionUsers who have never worked with ObjectBox may hit build errors because
objectbox-gorelies on native libraries (CGO) and theobjectbox-gogencode-generator. Adding a short note (and the standard install commands) prevents confusion.-First, ensure you have ObjectBox installed by following the official installation guide: -[ObjectBox installation guide](https://golang.objectbox.io/install) +First, make sure the native ObjectBox binaries are available (CGO is required) and install the +ObjectBox code-generator: + +```bash +# Install native libraries – see the official docs for your platform +# https://golang.objectbox.io/install + +# Install the ObjectBox Go code-generator (only needed when you define your own entities) +go install github.com/objectbox/objectbox-go/cmd/objectbox-gogen@latest +``` + +For platform-specific steps, refer to the official installation guide.
73-88: PreferKB(all-caps) for acronym consistency
MaxSizeInKbmixes upper- and lower-case letters for the same acronym. Renaming it toMaxSizeInKBaligns with typical Go naming (e.g.,MiB,KB) and avoids the “kilobit vs kilobyte” ambiguity.- MaxSizeInKb uint64 + MaxSizeInKB uint64This change also requires updating the corresponding struct field in code and any references in tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
objectbox/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
objectbox/README.md (10)
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db_mock.go:13-19
Timestamp: 2024-11-23T19:50:06.387Z
Learning: In test code within `clean-code/app/datasources/database/db_mock.go`, adding safety checks like context validation, safe type assertions, and extra documentation is not necessary.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/server.go:14-16
Timestamp: 2024-11-23T19:50:41.765Z
Learning: In the `clean-code` example at `clean-code/app/server/server.go` using the Go Fiber framework, it's acceptable to omit production-level features like context usage and graceful shutdown handling to keep the example simple.
Learnt from: juls0730
PR: gofiber/recipes#2710
File: tableflip/main.go:61-62
Timestamp: 2024-12-01T01:15:48.126Z
Learning: In the GoFiber `tableflip` recipe (`tableflip/main.go`), the implementation matches the upstream reference implementation. Future code suggestions should consider maintaining this alignment to ensure consistency.
Learnt from: itssimon
PR: gofiber/recipes#3108
File: monitoring-with-apitally/main.go:90-105
Timestamp: 2025-05-14T01:04:10.042Z
Learning: In recipe examples for the gofiber/recipes repository, it's acceptable to have placeholder comments for business logic (like data storage) when the primary focus is on demonstrating a specific integration or feature (such as Apitally monitoring).
Learnt from: ReneWerner87
PR: gofiber/storage#0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
Learnt from: efectn
PR: gofiber/fiber#3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the `require` methods from the `testify` package for assertions instead of manual comparisons and calls to `t.Fatal` or `t.Fatalf`.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/memory_db.go:0-0
Timestamp: 2024-11-25T19:31:39.216Z
Learning: The `memoryDB` implementation in `clean-code/app/datasources/database/memory_db.go` is not intended for production use, so input validation, concurrency protection, and context usage are not required in this file.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/handlers/books.go:28-43
Timestamp: 2024-11-23T19:54:01.389Z
Learning: In 'clean-code/app/server/handlers/books.go', validation is out of scope and should not be suggested.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/main.go:0-0
Timestamp: 2024-11-23T19:35:36.767Z
Learning: In the Go `clean-code` example (`clean-code/app/main.go`) in the `gofiber/recipes` repository, it's acceptable to omit graceful shutdown handling, as the example code prioritizes simplicity over production-level practices.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db.go:16-19
Timestamp: 2024-11-23T19:43:14.861Z
Learning: In the Go file `clean-code/app/datasources/database/db.go`, the `NewBook` struct should not include validation constraints as validation is not in scope.
🪛 LanguageTool
objectbox/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 946 characters long)
Context: .../tag/gofiber/storage?filter=objectbox*)
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (1)
objectbox/README.md (1)
47-52: Example code block is now clean – diff markers removedThe import snippet compiles as-is and no longer contains stray
+prefixes. 👍
Rename CleanerInterval to GCInterval and related functions to better reflect their purpose as garbage collection operations. This change improves code clarity and maintains consistent terminology.
Add dummy context-aware methods to comply with Fiber storage interface requirements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
objectbox/README.md (2)
41-47: Mention thego generatestep required by ObjectBoxObjectBox relies on code generation (
objectbox-generators→go generate ./...). Without running the generator the example will not compile/run at runtime. Consider appending the generator command right aftergo getso newcomers don’t stumble over the missing step.go get github.com/gofiber/storage/objectbox +# generate ObjectBox bindings +go generate ./...
53-58: Import list missingcontextused later in READMEThe snippets below call
objectbox.New()without context, but the signature list shows context–aware methods. Adding the import now avoids copy-paste edits when readers expand the example.import ( "github.com/gofiber/storage/objectbox" + "context" "time" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
objectbox/README.md(1 hunks)objectbox/objectbox.go(1 hunks)
🧠 Learnings (1)
objectbox/README.md (10)
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db_mock.go:13-19
Timestamp: 2024-11-23T19:50:06.387Z
Learning: In test code within clean-code/app/datasources/database/db_mock.go, adding safety checks like context validation, safe type assertions, and extra documentation is not necessary.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/server.go:14-16
Timestamp: 2024-11-23T19:50:41.765Z
Learning: In the clean-code example at clean-code/app/server/server.go using the Go Fiber framework, it's acceptable to omit production-level features like context usage and graceful shutdown handling to keep the example simple.
Learnt from: juls0730
PR: gofiber/recipes#2710
File: tableflip/main.go:61-62
Timestamp: 2024-12-01T01:15:48.126Z
Learning: In the GoFiber tableflip recipe (tableflip/main.go), the implementation matches the upstream reference implementation. Future code suggestions should consider maintaining this alignment to ensure consistency.
Learnt from: itssimon
PR: gofiber/recipes#3108
File: monitoring-with-apitally/main.go:90-105
Timestamp: 2025-05-14T01:04:10.042Z
Learning: In recipe examples for the gofiber/recipes repository, it's acceptable to have placeholder comments for business logic (like data storage) when the primary focus is on demonstrating a specific integration or feature (such as Apitally monitoring).
Learnt from: ReneWerner87
PR: gofiber/storage#0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
Learnt from: efectn
PR: gofiber/fiber#3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the require methods from the testify package for assertions instead of manual comparisons and calls to t.Fatal or t.Fatalf.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/memory_db.go:0-0
Timestamp: 2024-11-25T19:31:39.216Z
Learning: The memoryDB implementation in clean-code/app/datasources/database/memory_db.go is not intended for production use, so input validation, concurrency protection, and context usage are not required in this file.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/handlers/books.go:28-43
Timestamp: 2024-11-23T19:54:01.389Z
Learning: In 'clean-code/app/server/handlers/books.go', validation is out of scope and should not be suggested.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/main.go:0-0
Timestamp: 2024-11-23T19:35:36.767Z
Learning: In the Go clean-code example (clean-code/app/main.go) in the gofiber/recipes repository, it's acceptable to omit graceful shutdown handling, as the example code prioritizes simplicity over production-level practices.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db.go:16-19
Timestamp: 2024-11-23T19:43:14.861Z
Learning: In the Go file clean-code/app/datasources/database/db.go, the NewBook struct should not include validation constraints as validation is not in scope.
🚧 Files skipped from review as they are similar to previous changes (1)
- objectbox/objectbox.go
🧰 Additional context used
🧠 Learnings (1)
objectbox/README.md (10)
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db_mock.go:13-19
Timestamp: 2024-11-23T19:50:06.387Z
Learning: In test code within clean-code/app/datasources/database/db_mock.go, adding safety checks like context validation, safe type assertions, and extra documentation is not necessary.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/server.go:14-16
Timestamp: 2024-11-23T19:50:41.765Z
Learning: In the clean-code example at clean-code/app/server/server.go using the Go Fiber framework, it's acceptable to omit production-level features like context usage and graceful shutdown handling to keep the example simple.
Learnt from: juls0730
PR: gofiber/recipes#2710
File: tableflip/main.go:61-62
Timestamp: 2024-12-01T01:15:48.126Z
Learning: In the GoFiber tableflip recipe (tableflip/main.go), the implementation matches the upstream reference implementation. Future code suggestions should consider maintaining this alignment to ensure consistency.
Learnt from: itssimon
PR: gofiber/recipes#3108
File: monitoring-with-apitally/main.go:90-105
Timestamp: 2025-05-14T01:04:10.042Z
Learning: In recipe examples for the gofiber/recipes repository, it's acceptable to have placeholder comments for business logic (like data storage) when the primary focus is on demonstrating a specific integration or feature (such as Apitally monitoring).
Learnt from: ReneWerner87
PR: gofiber/storage#0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.
Learnt from: efectn
PR: gofiber/fiber#3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the require methods from the testify package for assertions instead of manual comparisons and calls to t.Fatal or t.Fatalf.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/memory_db.go:0-0
Timestamp: 2024-11-25T19:31:39.216Z
Learning: The memoryDB implementation in clean-code/app/datasources/database/memory_db.go is not intended for production use, so input validation, concurrency protection, and context usage are not required in this file.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/server/handlers/books.go:28-43
Timestamp: 2024-11-23T19:54:01.389Z
Learning: In 'clean-code/app/server/handlers/books.go', validation is out of scope and should not be suggested.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/main.go:0-0
Timestamp: 2024-11-23T19:35:36.767Z
Learning: In the Go clean-code example (clean-code/app/main.go) in the gofiber/recipes repository, it's acceptable to omit graceful shutdown handling, as the example code prioritizes simplicity over production-level practices.
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/datasources/database/db.go:16-19
Timestamp: 2024-11-23T19:43:14.861Z
Learning: In the Go file clean-code/app/datasources/database/db.go, the NewBook struct should not include validation constraints as validation is not in scope.
Fixes #1531
Summary by CodeRabbit
New Features
Documentation
Tests