🔥 feat: Add LevelDB driver #1565
Conversation
SadikSunbul
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
WalkthroughThis pull request introduces a new LevelDB storage implementation for the GoFiber framework. It adds comprehensive documentation, a configuration structure with default values, and a set of CRUD methods (New, Get, Set, Delete, Reset, Close, Conn) for managing key-value pairs with expiration support. Additionally, it includes a garbage collection mechanism to remove expired items and extensive unit tests to verify configuration settings and storage operations. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
thx
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
leveldb/config_test.go (1)
9-19: Consider adding more test cases for configuration validation.The test only validates the
MaxOpenFilessetting. Consider adding test cases for other configuration options likeCacheSize,BlockSize, etc.leveldb/config.go (1)
110-163: Add validation for configuration values.The
configDefaultfunction only checks if values are <= 0, but some fields might need additional validation (e.g., MaxOpenFiles should have a reasonable minimum value).func configDefault(config ...Config) Config { // ... existing code ... + // Validate minimum values + if cfg.MaxOpenFiles < 50 { + cfg.MaxOpenFiles = 50 // Ensure a reasonable minimum + } + if cfg.CacheSize > 1024 { + cfg.CacheSize = 1024 // Limit maximum cache size to 1GB + } + if cfg.GCInterval < time.Minute { + cfg.GCInterval = time.Minute // Ensure reasonable GC interval + } return cfg }leveldb/README.md (2)
12-15: Introduction and Go Version NoteThe description succinctly introduces LevelDB as a fast key-value store using the
syndtr/goleveldbpackage. The Go version requirement is prominently noted. However, instead of using bold text for the note (line 14), consider using a proper heading (e.g.,## Note: Requires Go 1.23.1 and above) to align with common Markdown practices.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
72-151: Configuration Struct DocumentationThe
Configstruct is well-documented with inline comments explaining each field and its default value. However, there are multiple static analysis warnings regarding the use of hard tabs in this code block. Replacing these hard tabs with spaces would improve readability and ensure compliance with Markdown linting (MD010) guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
76-76: Hard tabs
Column: 1(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 1(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1(MD010, no-hard-tabs)
146-146: Hard tabs
Column: 1(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
leveldb/go.modis excluded by!**/*.modleveldb/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (5)
leveldb/README.md(1 hunks)leveldb/config.go(1 hunks)leveldb/config_test.go(1 hunks)leveldb/leveldb.go(1 hunks)leveldb/leveldb_test.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint (leveldb)
leveldb/leveldb.go
[failure] 61-61:
[golangci] reported by reviewdog 🐶
Error return value of s.db.Delete is not checked (errcheck)
Raw Output:
leveldb.go:61:14: Error return value of s.db.Delete is not checked (errcheck)
s.db.Delete(key, nil)
^
🪛 golangci-lint (1.62.2)
leveldb/leveldb.go
61-61: Error return value of s.db.Delete is not checked
(errcheck)
leveldb/leveldb_test.go
41-41: Error return value of db.Set is not checked
(errcheck)
42-42: Error return value of db.Set is not checked
(errcheck)
55-55: Error return value of db.Set is not checked
(errcheck)
103-103: Error return value of db.Reset is not checked
(errcheck)
🪛 GitHub Actions: Golangci-Lint Check
leveldb/leveldb.go
[error] 61-61: Error return value of s.db.Delete is not checked (errcheck)
🪛 LanguageTool
leveldb/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1020 characters long)
Context: .../v/tag/gofiber/storage?filter=leveldb*) 
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
leveldb/README.md
14-14: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
76-76: Hard tabs
Column: 1
(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1
(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1
(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1
(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1
(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1
(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1
(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1
(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1
(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1
(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1
(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1
(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1
(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1
(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1
(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1
(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1
(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1
(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1
(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1
(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1
(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1
(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 1
(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1
(MD010, no-hard-tabs)
146-146: Hard tabs
Column: 1
(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1
(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1
(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1
(MD010, no-hard-tabs)
157-157: Hard tabs
Column: 1
(MD010, no-hard-tabs)
158-158: Hard tabs
Column: 1
(MD010, no-hard-tabs)
159-159: Hard tabs
Column: 1
(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1
(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1
(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1
(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1
(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1
(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1
(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1
(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1
(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1
(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 1
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 1
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 1
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (9)
leveldb/leveldb.go (1)
126-159:⚠️ Potential issueProtect against potential data races in gc routine.
The garbage collection routine accesses the database without synchronization, which could lead to data races. Consider using a mutex to protect database operations.
type Storage struct { db *leveldb.DB gcInterval time.Duration done chan struct{} + mu sync.RWMutex } func (s *Storage) gc() { ticker := time.NewTicker(s.gcInterval) defer ticker.Stop() for { select { case <-s.done: return case <-ticker.C: + s.mu.Lock() iter := s.db.NewIterator(nil, nil) batch := new(leveldb.Batch) // ... rest of the code ... if batch.Len() > 0 { _ = s.db.Write(batch, nil) } + s.mu.Unlock() } } }Likely invalid or redundant comment.
leveldb/README.md (8)
1-5: YAML Front Matter is well defined.The front matter clearly specifies the module's ID and title using YAML, which will help in proper metadata management.
6-11: Badge Section ReviewThe badge links effectively convey project status (release, test, security, linter) and community information. Ensure that these URLs remain up-to-date with the repository's status.
🧰 Tools
🪛 LanguageTool
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1020 characters long)
Context: .../v/tag/gofiber/storage?filter=leveldb*)
16-23: Table of Contents ClarityThe Table of Contents is well-organized, making it easy for readers to navigate through the document.
24-34: Function Signatures OverviewThe Signatures section clearly lists the public API functions available in the LevelDB storage implementation. It’s important to verify that these signatures match the corresponding implementations in
leveldb/leveldb.go.
36-42: Installation InstructionsThe installation steps, including module initialization and the setup commands, are concise and clear. This should help new users get started quickly.
44-48: Package Installation CommandThe instruction to install the leveldb implementation using
go getis straightforward and correct. No issues found.
50-70: Examples Section ReviewThe examples demonstrate how to initialize the storage using both default and custom configurations. One point to note: the custom configuration snippet shows
GCIntervalset to10 * time.Second, whereas in the default configuration later, it is set to10 * time.Minute. Please verify that this discrepancy is intentional.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
66-66: Hard tabs
Column: 1(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1(MD010, no-hard-tabs)
153-178: Default Configuration SettingsThe
ConfigDefaultvariable provides clear default settings, including a dynamic assignment forMaxOpenFilesbased on the operating system. Similar to the Config struct, several hard tabs are used in this block. Converting these to spaces will help maintain consistency with Markdown styling best practices. Additionally, please ensure the defaultGCIntervalof10 * time.Minuteis the intended value, especially in light of the shorter interval shown in the custom example.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
157-157: Hard tabs
Column: 1(MD010, no-hard-tabs)
158-158: Hard tabs
Column: 1(MD010, no-hard-tabs)
159-159: Hard tabs
Column: 1(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 1(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 1(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 1(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 1(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
leveldb/leveldb_test.go (1)
70-70: 🛠️ Refactor suggestionReplace time.Sleep with deterministic approaches.
Using
time.Sleepcan make tests flaky. Consider using a more deterministic approach with shorter intervals and polling.Also applies to: 142-142
🧹 Nitpick comments (1)
leveldb/leveldb_test.go (1)
182-182: Document error handling in benchmarks.While ignoring errors in benchmarks is common for performance reasons, it should be documented to avoid confusion.
Add comments to explain why errors are ignored:
- _ = db.Set(key, value, 0) + // Errors are ignored in benchmarks for performance + _ = db.Set(key, value, 0)Also applies to: 202-202, 220-220
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/test-leveldb.ymlis excluded by!**/*.yml
📒 Files selected for processing (2)
README.md(1 hunks)leveldb/leveldb_test.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
80-80: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
42-42: Error return value of db.Set is not checked
(errcheck)
43-43: Error return value of db.Set is not checked
(errcheck)
56-56: Error return value of db.Set is not checked
(errcheck)
104-104: Error return value of db.Reset is not checked
(errcheck)
🔇 Additional comments (1)
leveldb/leveldb_test.go (1)
12-14: LGTM!The helper function is well-designed for test cleanup.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
leveldb/leveldb_test.go (1)
39-121: 🛠️ Refactor suggestionAdd error checks and make expiration tests more reliable.
Several improvements needed:
- Missing error checks for db.Set operations (as previously noted)
- Flaky tests using time.Sleep
- Cleanup should use t.Cleanup()
func Test_Set_Overwrite(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() - db.Set([]byte("key"), []byte("value"), time.Second*1) - db.Set([]byte("key"), []byte("value2"), time.Second*1) + require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1)) + require.NoError(t, db.Set([]byte("key"), []byte("value2"), time.Second*1)) value, err := db.Get([]byte("key")) require.Nil(t, err) require.Equal(t, []byte("value2"), value) - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) } func Test_Get_ForExpired100Millisecond(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() - db.Set([]byte("key"), []byte("value"), time.Millisecond*100) + require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Millisecond*100)) - time.Sleep(time.Millisecond * 150) + // Wait for expiration with exponential backoff + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + value, err := db.Get([]byte("key")) + if err == nil && value == nil { + break + } + time.Sleep(time.Millisecond * 10) + } value, err := db.Get([]byte("key")) require.Nil(t, err) require.Nil(t, value) - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) }🧰 Tools
🪛 golangci-lint (1.62.2)
42-42: Error return value of
db.Setis not checked(errcheck)
43-43: Error return value of
db.Setis not checked(errcheck)
56-56: Error return value of
db.Setis not checked(errcheck)
104-104: Error return value of
db.Resetis not checked(errcheck)
🧹 Nitpick comments (4)
leveldb/leveldb_test.go (4)
12-14: Consider enhancing test cleanup reliability.The cleanup function could be more robust by:
- Ensuring the directory exists before removal
- Adding retries for Windows systems where file handles might not be immediately released
func removeAllFiles(dir string) error { + // Ensure cleanup even if directory doesn't exist + if _, err := os.Stat(dir); os.IsNotExist(err) { + return nil + } + // Add retries for Windows systems + for i := 0; i < 3; i++ { + err := os.RemoveAll(dir) + if err == nil || os.IsNotExist(err) { + return nil + } + time.Sleep(time.Millisecond * 100) + } return os.RemoveAll(dir) }
16-37: Enhance configuration tests with edge cases and proper cleanup.The configuration tests could be improved by:
- Testing invalid paths
- Using
t.Cleanup()for reliable cleanup- Testing more configuration options
func Test_New_EmptyConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() require.NotNil(t, db) _, err := os.Stat("./fiber.leveldb") require.Nil(t, err) - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) } func Test_New_WithConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./testdb")) + }) + // Test invalid path + invalidDB := New(Config{ + DBPath: "/invalid/path/that/doesnt/exist", + }) + require.NotNil(t, invalidDB) + db := New(Config{ DBPath: "./testdb", }) require.NotNil(t, db) _, err := os.Stat("./testdb") require.Nil(t, err) - err = removeAllFiles("./testdb") - require.Nil(t, err) }
170-185: Consider adding more channel-related tests.The channel test is good but could be expanded to cover:
- Multiple close calls
- Concurrent operations during close
- GC channel behavior
+func Test_Close_Multiple(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) + db := New() + require.NoError(t, db.Close()) + require.Error(t, db.Close(), "second close should error") +} + +func Test_Close_Concurrent(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) + db := New() + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + _ = db.Set([]byte("key"), []byte("value"), 0) + }() + go func() { + defer wg.Done() + _ = db.Close() + }() + wg.Wait() +}
187-252: Enhance benchmarks with more scenarios.The benchmarks are good but could be improved by:
- Testing different value sizes
- Adding batch operation benchmarks
- Testing with different expiration times
+func Benchmark_Set_LargeValue(b *testing.B) { + db := New() + defer func() { + _ = db.Close() + _ = removeAllFiles("./fiber.leveldb") + }() + + value := make([]byte, 1024*1024) // 1MB value + rand.Read(value) + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + var i int + for pb.Next() { + key := []byte(fmt.Sprintf("key_%d", i)) + if err := db.Set(key, value, 0); err != nil { + b.Fatal(err) + } + i++ + } + }) +} + +func Benchmark_Set_WithExpiration(b *testing.B) { + db := New() + defer func() { + _ = db.Close() + _ = removeAllFiles("./fiber.leveldb") + }() + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + var i int + for pb.Next() { + key := []byte(fmt.Sprintf("key_%d", i)) + value := []byte(fmt.Sprintf("value_%d", i)) + if err := db.Set(key, value, time.Hour); err != nil { + b.Fatal(err) + } + i++ + } + }) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)leveldb/config_test.go(1 hunks)leveldb/leveldb.go(1 hunks)leveldb/leveldb_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- README.md
- leveldb/config_test.go
- leveldb/leveldb.go
🧰 Additional context used
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
42-42: Error return value of db.Set is not checked
(errcheck)
43-43: Error return value of db.Set is not checked
(errcheck)
56-56: Error return value of db.Set is not checked
(errcheck)
104-104: Error return value of db.Reset is not checked
(errcheck)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
leveldb/leveldb_test.go (1)
39-51:⚠️ Potential issueAdd error checking for Set operations.
The test is missing error checks for
db.Setoperations which could fail silently.Apply this pattern throughout the tests:
-db.Set([]byte("key"), []byte("value"), time.Second*1) -db.Set([]byte("key"), []byte("value2"), time.Second*1) +require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1)) +require.NoError(t, db.Set([]byte("key"), []byte("value2"), time.Second*1))🧰 Tools
🪛 golangci-lint (1.62.2)
42-42: Error return value of
db.Setis not checked(errcheck)
43-43: Error return value of
db.Setis not checked(errcheck)
🧹 Nitpick comments (4)
leveldb/leveldb_test.go (4)
12-14: Use t.Cleanup consistently across all tests.Some tests use
t.Cleanupwhile others manually callremoveAllFiles. For consistency and to ensure cleanup even if tests fail, uset.Cleanupin all tests.Example pattern to apply:
func Test_New_EmptyConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() require.NotNil(t, db) - _, err := os.Stat("./fiber.leveldb") require.Nil(t, err) - - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) }
16-37: Enhance initialization test coverage.The initialization tests could be more comprehensive:
- Verify default configuration values when using empty config
- Add error handling for New() which might fail
- Test invalid configurations (e.g., invalid paths)
Example additions:
func Test_New_DefaultConfig(t *testing.T) { t.Cleanup(func() { require.NoError(t, removeAllFiles("./fiber.leveldb")) }) db := New() require.NotNil(t, db) // Verify default values cfg := db.config require.Equal(t, "./fiber.leveldb", cfg.DBPath) require.Equal(t, time.Minute*10, cfg.GCInterval) } func Test_New_InvalidConfig(t *testing.T) { db := New(Config{ DBPath: "/invalid/path/that/doesnt/exist", }) require.NotNil(t, db) // Verify that operations fail gracefully err := db.Set([]byte("key"), []byte("value"), 0) require.Error(t, err) }
70-70: Translate comments to English.For consistency and maintainability, comments should be in English:
-// Anahtarın silinmesini bekle +// Wait for the key to be deleted -GCInterval: time.Hour, // Uzun aralık +GCInterval: time.Hour, // Long interval -// GC çalışmadığı için değer hala var olmalı +// Value should still exist as GC hasn't run yetAlso applies to: 188-188, 192-192
222-287: Consider adding benchmark cases with different payload sizes.The benchmarks effectively test parallel operations but could be more comprehensive by testing different payload sizes to understand performance characteristics better.
Example additions:
func Benchmark_Set_LargePayload(b *testing.B) { db := New() defer func() { _ = db.Close() _ = removeAllFiles("./fiber.leveldb") }() largeValue := make([]byte, 1024*1024) // 1MB payload b.ResetTimer() b.RunParallel(func(pb *testing.PB) { var i int for pb.Next() { key := []byte(fmt.Sprintf("key_%d", i)) if err := db.Set(key, largeValue, 0); err != nil { b.Fatal(err) } i++ } }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
leveldb/leveldb_test.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
42-42: Error return value of db.Set is not checked
(errcheck)
43-43: Error return value of db.Set is not checked
(errcheck)
56-56: Error return value of db.Set is not checked
(errcheck)
112-112: Error return value of db.Reset is not checked
(errcheck)
🔇 Additional comments (2)
leveldb/leveldb_test.go (2)
131-141: LGTM! Close tests are thorough.The tests effectively verify both the functional aspect (operations failing after close) and implementation details (channel closure).
Also applies to: 205-220
143-203: LGTM! GC tests follow best practices.The garbage collection tests effectively verify the functionality using polling with deadlines instead of fixed sleeps, which makes the tests more reliable.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
leveldb/leveldb_test.go (1)
42-43:⚠️ Potential issueAdd error checking for Set operations.
The Set operations should check for errors to ensure test validity.
🧰 Tools
🪛 golangci-lint (1.62.2)
42-42: Error return value of
db.Setis not checked(errcheck)
43-43: Error return value of
db.Setis not checked(errcheck)
🧹 Nitpick comments (4)
leveldb/leveldb_test.go (4)
12-14: Use t.Cleanup consistently across all tests.Some tests use
t.Cleanup()while others manually clean up. For consistency and to ensure cleanup even if tests fail, consider usingt.Cleanup()throughout.Example pattern to apply:
func Test_New_EmptyConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() require.NotNil(t, db) - _, err := os.Stat("./fiber.leveldb") require.Nil(t, err) - - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) }
65-86: Improve test readability by removing Turkish comments.The comment "Anahtarın silinmesini bekle" should be in English for consistency and maintainability.
- // Anahtarın silinmesini bekle + // Wait for the key to be deleted
88-103: Enhance Delete test coverage.The Delete test should also verify behavior with:
- Non-existent keys
- Empty keys
- Special characters in keys
Example addition:
func Test_Delete_WhileThereIsData(t *testing.T) { db := New() + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) // Test existing key require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1)) require.NoError(t, db.Delete("key")) value, err := db.Get([]byte("key")) require.Nil(t, err) require.Nil(t, value) + // Test non-existent key + err = db.Delete("non-existent") + require.NoError(t, err) + + // Test empty key + err = db.Delete("") + require.NoError(t, err) + + // Test special characters + require.NoError(t, db.Set([]byte("key!@#$"), []byte("value"), time.Second*1)) + require.NoError(t, db.Delete("key!@#$")) + value, err = db.Get([]byte("key!@#$")) + require.Nil(t, err) + require.Nil(t, value) }
182-203: Translate Turkish comments to English.For consistency and maintainability, all comments should be in English.
- GCInterval: time.Hour, // Uzun aralık + GCInterval: time.Hour, // Long interval - // GC çalışmadığı için değer hala var olmalı + // Value should still exist as GC hasn't run yet
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
leveldb/leveldb_test.go(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
42-42: Error return value of db.Set is not checked
(errcheck)
43-43: Error return value of db.Set is not checked
(errcheck)
56-56: Error return value of db.Set is not checked
(errcheck)
🔇 Additional comments (1)
leveldb/leveldb_test.go (1)
222-287: Well-structured benchmarks!The benchmarks are well-implemented with:
- Proper parallel execution
- Good error handling
- Clean cleanup using defer
- Appropriate use of ResetTimer
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
leveldb/README.md (2)
14-14: Add specific Go version requirements.The note should specify both minimum and maximum supported Go versions.
Apply this change:
-**Note: Requires Go 1.21 or later** +**Note: Requires Go 1.21 or 1.22**🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
74-150: Add configuration validation information.The Config section should include information about validation of configuration values and any constraints.
Consider adding a note about:
- Valid ranges for numeric values (e.g., minimum/maximum for CacheSize, BlockSize)
- Validation behavior (e.g., what happens if invalid values are provided)
- Impact of different configurations on performance
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
76-76: Hard tabs
Column: 1(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 1(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1(MD010, no-hard-tabs)
146-146: Hard tabs
Column: 1(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
leveldb/go.modis excluded by!**/*.mod
📒 Files selected for processing (4)
leveldb/README.md(1 hunks)leveldb/config_test.go(1 hunks)leveldb/leveldb.go(1 hunks)leveldb/leveldb_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- leveldb/config_test.go
- leveldb/leveldb.go
🧰 Additional context used
🪛 LanguageTool
leveldb/README.md
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1017 characters long)
Context: .../v/tag/gofiber/storage?filter=leveldb*)
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
leveldb/README.md
14-14: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
76-76: Hard tabs
Column: 1
(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1
(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1
(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1
(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1
(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1
(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1
(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1
(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1
(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1
(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1
(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1
(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1
(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1
(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1
(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1
(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1
(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1
(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1
(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1
(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1
(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1
(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1
(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1
(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1
(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 1
(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1
(MD010, no-hard-tabs)
146-146: Hard tabs
Column: 1
(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1
(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1
(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1
(MD010, no-hard-tabs)
157-157: Hard tabs
Column: 1
(MD010, no-hard-tabs)
158-158: Hard tabs
Column: 1
(MD010, no-hard-tabs)
159-159: Hard tabs
Column: 1
(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1
(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1
(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1
(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1
(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1
(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1
(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1
(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1
(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1
(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1
(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 1
(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 1
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1
(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 1
(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
41-41: Error return value of db.Set is not checked
(errcheck)
42-42: Error return value of db.Set is not checked
(errcheck)
55-55: Error return value of db.Set is not checked
(errcheck)
🔇 Additional comments (8)
leveldb/leveldb_test.go (7)
41-42: Add error checking for db.Set operations.Test validity could be compromised if Set operations fail silently.
Apply this pattern:
-db.Set([]byte("key"), []byte("value"), time.Second*1) -db.Set([]byte("key"), []byte("value2"), time.Second*1) +require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1)) +require.NoError(t, db.Set([]byte("key"), []byte("value2"), time.Second*1))🧰 Tools
🪛 golangci-lint (1.62.2)
41-41: Error return value of
db.Setis not checked(errcheck)
42-42: Error return value of
db.Setis not checked(errcheck)
55-55: Add error checking for db.Set operation.Test validity could be compromised if Set operation fails silently.
Apply this pattern:
-db.Set([]byte("key"), []byte("value"), 0) +require.NoError(t, db.Set([]byte("key"), []byte("value"), 0))🧰 Tools
🪛 golangci-lint (1.62.2)
55-55: Error return value of
db.Setis not checked(errcheck)
69-77: LGTM! Good use of polling with deadline.The implementation correctly uses a polling approach with deadline instead of time.Sleep, which helps avoid flaky tests.
90-90: Add error checking for db.Set operation.Test validity could be compromised if Set operation fails silently.
Apply this pattern:
-db.Set([]byte("key"), []byte("value"), time.Second*1) +require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1))
107-109: Add error checking for db.Set operations.Test validity could be compromised if Set operations fail silently.
Apply this pattern:
-db.Set([]byte("key1"), []byte("value1"), time.Second*1) -db.Set([]byte("key2"), []byte("value2"), time.Second*1) -db.Set([]byte("key3"), []byte("value3"), time.Second*1) +require.NoError(t, db.Set([]byte("key1"), []byte("value1"), time.Second*1)) +require.NoError(t, db.Set([]byte("key2"), []byte("value2"), time.Second*1)) +require.NoError(t, db.Set([]byte("key3"), []byte("value3"), time.Second*1))
11-13: LGTM!The helper function is well-implemented and consistently used across tests for cleanup.
38-50: Add error checking for db.Set operations.Test validity could be compromised if Set operations fail silently. Add error checking using
require.NoError.🧰 Tools
🪛 golangci-lint (1.62.2)
41-41: Error return value of
db.Setis not checked(errcheck)
42-42: Error return value of
db.Setis not checked(errcheck)
leveldb/README.md (1)
1-179: LGTM! Documentation is comprehensive and well-structured.The README provides clear installation instructions, usage examples, and detailed configuration options. All essential aspects of the LevelDB driver are well documented.
🧰 Tools
🪛 LanguageTool
[style] ~6-~6: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1017 characters long)
Context: .../v/tag/gofiber/storage?filter=leveldb*)(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
14-14: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
66-66: Hard tabs
Column: 1(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1(MD010, no-hard-tabs)
76-76: Hard tabs
Column: 1(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1(MD010, no-hard-tabs)
82-82: Hard tabs
Column: 1(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1(MD010, no-hard-tabs)
107-107: Hard tabs
Column: 1(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1(MD010, no-hard-tabs)
141-141: Hard tabs
Column: 1(MD010, no-hard-tabs)
142-142: Hard tabs
Column: 1(MD010, no-hard-tabs)
143-143: Hard tabs
Column: 1(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1(MD010, no-hard-tabs)
146-146: Hard tabs
Column: 1(MD010, no-hard-tabs)
147-147: Hard tabs
Column: 1(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1(MD010, no-hard-tabs)
157-157: Hard tabs
Column: 1(MD010, no-hard-tabs)
158-158: Hard tabs
Column: 1(MD010, no-hard-tabs)
159-159: Hard tabs
Column: 1(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1(MD010, no-hard-tabs)
164-164: Hard tabs
Column: 1(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1(MD010, no-hard-tabs)
172-172: Hard tabs
Column: 1(MD010, no-hard-tabs)
173-173: Hard tabs
Column: 1(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1(MD010, no-hard-tabs)
175-175: Hard tabs
Column: 1(MD010, no-hard-tabs)
176-176: Hard tabs
Column: 1(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
leveldb/leveldb_test.go (2)
11-13: Improve error handling in removeAllFiles helper.Consider enhancing the helper function with better error handling and path validation.
func removeAllFiles(dir string) error { + if dir == "" { + return fmt.Errorf("directory path cannot be empty") + } + if _, err := os.Stat(dir); os.IsNotExist(err) { + return nil // Already doesn't exist + } return os.RemoveAll(dir) }
69-69: Use English comments consistently.Replace Turkish comments with English equivalents:
- // Anahtarın silinmesini bekle + // Wait for the key to be deleted - GCInterval: time.Hour, // Uzun aralık + GCInterval: time.Hour, // Long interval - // GC çalışmadığı için değer hala var olmalı + // Value should still exist as GC hasn't run yetAlso applies to: 187-187, 191-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
leveldb/config.go(1 hunks)leveldb/leveldb.go(1 hunks)leveldb/leveldb_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- leveldb/config.go
- leveldb/leveldb.go
🧰 Additional context used
🪛 golangci-lint (1.62.2)
leveldb/leveldb_test.go
41-41: Error return value of db.Set is not checked
(errcheck)
42-42: Error return value of db.Set is not checked
(errcheck)
55-55: Error return value of db.Set is not checked
(errcheck)
🔇 Additional comments (3)
leveldb/leveldb_test.go (3)
15-36: Use t.Cleanup() for consistent test cleanup.Previous review comments suggested using t.Cleanup() for reliable cleanup. Let's maintain consistency.
func Test_New_EmptyConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./fiber.leveldb")) + }) db := New() require.NotNil(t, db) _, err := os.Stat("./fiber.leveldb") require.Nil(t, err) - - err = removeAllFiles("./fiber.leveldb") - require.Nil(t, err) } func Test_New_WithConfig(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, removeAllFiles("./testdb")) + }) db := New(Config{ Path: "./testdb", }) require.NotNil(t, db) _, err := os.Stat("./testdb") require.Nil(t, err) - - err = removeAllFiles("./testdb") - require.Nil(t, err) }
38-102: Add error checking for database operations.Static analysis and previous reviews indicate missing error checks for Set operations.
func Test_Set_Overwrite(t *testing.T) { db := New() - db.Set([]byte("key"), []byte("value"), time.Second*1) - db.Set([]byte("key"), []byte("value2"), time.Second*1) + require.NoError(t, db.Set([]byte("key"), []byte("value"), time.Second*1)) + require.NoError(t, db.Set([]byte("key"), []byte("value2"), time.Second*1)) // ... } func Test_Get_For0Second(t *testing.T) { db := New() - db.Set([]byte("key"), []byte("value"), 0) + require.NoError(t, db.Set([]byte("key"), []byte("value"), 0)) // ... }🧰 Tools
🪛 golangci-lint (1.62.2)
41-41: Error return value of
db.Setis not checked(errcheck)
42-42: Error return value of
db.Setis not checked(errcheck)
55-55: Error return value of
db.Setis not checked(errcheck)
221-284: Simplify benchmark loops as suggested in previous reviews.Previous reviews recommended simplifying the benchmark loops.
func Benchmark_Set(b *testing.B) { // ... b.RunParallel(func(pb *testing.PB) { for pb.Next() { - if err := db.Set(key, value, 0); err != nil { - b.Fatal(err) - } + require.NoError(b, db.Set(key, value, 0)) } }) } func Benchmark_Delete(b *testing.B) { // ... b.RunParallel(func(pb *testing.PB) { for pb.Next() { - if err := db.Delete(key); err != nil { - b.Fatal(err) - } + require.NoError(b, db.Delete(key)) } }) }
This PR aims to add a new storage implementation for the LevelDB database.
Thank you for your reviews 🙏
Summary by CodeRabbit