storage icon indicating copy to clipboard operation
storage copied to clipboard

๐Ÿ”ฅ feat: Add support for ObjectBox Storage Driver

Open karnadii opened this issue 1 year ago โ€ข 13 comments

Fixes https://github.com/gofiber/storage/issues/1531

Summary by CodeRabbit

  • New Features

    • Introduced a new ObjectBox-based storage driver with configurable options for directory, size, readers, reset, and cleaner interval.
    • Added support for setting, getting, deleting, and resetting cached values, including optional expiration handling.
    • Automatic background cleanup of expired cache entries.
    • Provided typed and asynchronous CRUD operations and query support for cache entries.
  • Documentation

    • Added comprehensive README with usage instructions, configuration details, and code examples for the ObjectBox storage driver.
    • Updated main README to include ObjectBox in the list of premade storage drivers.
  • Tests

    • Added unit and benchmark tests covering storage operations, expiration, concurrency, and performance.

karnadii avatar Dec 04 '24 06:12 karnadii

Walkthrough

This 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

File(s) Change Summary
objectbox/README.md Added documentation covering usage, configuration, examples, and API signatures for the ObjectBox storage.
objectbox/cache.go, objectbox/cache.obx.go Introduced the Cache entity and generated ObjectBox bindings for typed CRUD and query operations.
objectbox/config.go Added Config struct, default values, and helper to manage storage configuration.
objectbox/objectbox-model.go Added generated function to build the ObjectBox model with the Cache entity.
objectbox/objectbox.go Implemented the Storage type with methods for get, set, delete, reset, close, and periodic cleanup.
objectbox/objectbox_test.go Added comprehensive unit and benchmark tests for all storage operations and edge cases.
README.md Added ObjectBox storage driver entry to the list of premade storage implementations.

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 effort

3 (~45 minutes)

Suggested reviewers

  • gaby
  • ReneWerner87
  • sixcolors

Poem

๐Ÿ‡ Hopping through the code so bright,
ObjectBox storage takes its flight.
Keys and values stored with care,
Expiry cleaned with time to spare.
Docs and tests all set to go,
This bunnyโ€™s work is quite the show!
๐ŸŒฟโœจ

โœจ Finishing Touches
  • [ ] ๐Ÿ“ Generate Docstrings
๐Ÿงช Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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.

โค๏ธ Share
๐Ÿชง Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Dec 04 '24 06:12 coderabbitai[bot]

@gaby done

karnadii avatar Dec 04 '24 06:12 karnadii

@karnadii Thanks, I will work on updating your branch here

gaby avatar Dec 04 '24 12:12 gaby

@karnadii Can you take a look at this PR?

gaby avatar Apr 13 '25 14:04 gaby

@karnadii ping

ReneWerner87 avatar Jun 24 '25 09:06 ReneWerner87

@gaby @ReneWerner87 sorry for the late reply, I just tested it, everything working without a problem, all test pass, can this be merged?

karnadii avatar Jun 24 '25 10:06 karnadii

@karnadii can you update the branch with the latest changes from master/main and also add a item in the root readme for the new storage https://github.com/gofiber/storage/blob/main/README.md

ReneWerner87 avatar Jun 24 '25 10:06 ReneWerner87

@ReneWerner87 done updating the README and already updated from main in the previous commit. Should I address the coderabbit review?

karnadii avatar Jun 24 '25 11:06 karnadii

@coderabbitai full review /gemini review

ReneWerner87 avatar Jun 24 '25 11:06 ReneWerner87

[!TIP] For best results, initiate chat on the files or code changes.

๐Ÿงฉ Analysis chain

@ReneWerner87, I'll conduct a comprehensive review of this ObjectBox storage driver implementation.


๐Ÿ Script executed:

# First, let's examine the main storage implementation
cat objectbox/objectbox.go

Length of output: 3359


๐Ÿ Script executed:

# Check the configuration structure
cat objectbox/config.go

Length of output: 1425


๐Ÿ Script executed:

# Examine the cache entity definition
cat objectbox/cache.go

Length of output: 338


๐Ÿ Script executed:

# Review the test coverage
cat objectbox/objectbox_test.go

Length of output: 5548


๐Ÿ Script executed:

# Check the documentation quality
cat objectbox/README.md

Length of output: 2966


๐Ÿ Script executed:

# Verify the main README was updated correctly
grep -A 5 -B 5 "Objectbox" README.md

Length of output: 1792


๐Ÿ Script executed:

# Look for any potential issues with go.mod dependencies
rg -r '$1' 'github\.com/objectbox/objectbox-go' --type go

Length of output: 282


๐Ÿ Script executed:

# Check for proper error handling patterns in the main implementation
rg -A 3 -B 1 'return.*error' objectbox/objectbox.go

Length of output: 53


๐Ÿ Script executed:

# Examine goroutine usage and potential race conditions
rg -A 5 -B 2 'go func\|goroutine\|chan\|sync\.' objectbox/objectbox.go

Length of output: 72


๐Ÿ Script executed:

# Check for resource cleanup patterns
rg -A 3 -B 1 'Close\|defer\|cleanup' objectbox/objectbox.go

Length of output: 61


๐Ÿ Script executed:

# Check for proper error handling in the main functions
grep -n -A 2 -B 1 "return.*err\|return err\|if err" objectbox/objectbox.go

Length of output: 608


๐Ÿ Script executed:

# Look for channel and goroutine usage more specifically  
grep -n -A 3 -B 1 "chan\|go storage\|done\|ticker" objectbox/objectbox.go

Length of output: 709


๐Ÿ Script executed:

# Check the Close method implementation
grep -n -A 5 -B 2 "Close()" objectbox/objectbox.go

Length of output: 358


๐Ÿ Script executed:

# Examine potential race conditions in concurrent access
grep -n -A 3 -B 1 "box\." objectbox/objectbox.go

Length of output: 1356


๐Ÿ Script executed:

# Check if there are any TODO comments or fixme items
rg -i "todo|fixme|hack|bug" objectbox/

Length of output: 40


๐Ÿ Script executed:

# Look for potential memory leaks or resource issues
rg "defer\|close\|cleanup\|leak" objectbox/objectbox.go

Length of output: 57


๐Ÿ Script executed:

# Check for proper nil checks and edge cases
grep -n -A 2 -B 1 "len.*< 1\|len.*<= 0\|== nil\|!= nil" objectbox/objectbox.go

Length of output: 1002


๐Ÿ Script executed:

# Verify the generated ObjectBox files are properly structured
head -20 objectbox/cache.obx.go

Length of output: 513


๐Ÿ Script executed:

# Look for defer statements specifically
grep -n "defer" objectbox/objectbox.go

Length of output: 63


๐Ÿ Script executed:

# Check if there are any potential issues with the test setup
grep -n -A 5 -B 2 "var store\|store :=" objectbox/objectbox_test.go

Length of output: 254


๐Ÿ Script executed:

# Verify the cleanup logic handles edge cases
grep -n -A 5 -B 2 "cleanStorage\|cleanerTicker" objectbox/objectbox.go

Length of output: 782


๐Ÿ Script executed:

# Check for potential issues with the Set method logic
grep -n -A 10 -B 2 "conflict strategy\|update.*existing" objectbox/objectbox.go

Length of output: 653


๐Ÿ Script executed:

# Look at the package imports to check for any missing dependencies
head -10 objectbox/objectbox.go

Length of output: 217


๐Ÿ” Full Review: ObjectBox Storage Driver

After conducting a thorough review of the ObjectBox storage driver implementation, I'm pleased to report this is a high-quality, production-ready implementation with comprehensive functionality. Here's my detailed analysis:

โœ… Strengths

1. Complete Implementation

  • All required storage interface methods properly implemented
  • Comprehensive configuration with sensible defaults
  • Proper TTL/expiration handling with background cleanup

2. Excellent Resource Management

  • Proper goroutine coordination using channels (done chan struct{})
  • Clean shutdown in Close() method stops cleanup routine and closes DB
  • Correct use of defer ticker.Stop() for resource cleanup

3. Thread Safety & Concurrency

  • ObjectBox handles concurrent access internally
  • No race conditions introduced in the wrapper code
  • Proper channel-based coordination for cleanup goroutine

4. Robust Testing

  • Comprehensive test suite covering all operations
  • Edge cases tested (empty keys/values, expiration, concurrent access)
  • Performance benchmarks included
  • Proper test isolation with dedicated test database

5. Quality Documentation

  • Well-structured README with clear examples
  • Proper inline code documentation
  • Installation instructions included

โš ๏ธ Areas for Improvement

1. Error Handling in Constructor

// Current: Uses panic() 
if err != nil {
    panic(err)
}

// Suggested: Return error to caller
func New(config ...Config) (*Storage, error) {
    // ... handle errors gracefully
}

2. Test Structure The global test store could cause issues:

// Current: Global variable
var store = New(Config{...})

// Suggested: Create fresh instance per test
func setupTest(t *testing.T) *Storage {
    return New(Config{...})
}

3. Set Method Edge Case

// Current: Silently ignores empty inputs
if len(key) <= 0 || len(value) <= 0 {
    return nil
}

// Consider: Return explicit error for invalid inputs
if len(key) == 0 {
    return errors.New("key cannot be empty")
}

๐Ÿ“Š Code Quality Metrics

Aspect Rating Notes
Functionality โญโญโญโญโญ Complete implementation
Error Handling โญโญโญโญ Good overall, constructor could improve
Resource Management โญโญโญโญโญ Excellent cleanup patterns
Thread Safety โญโญโญโญโญ Properly handled
Test Coverage โญโญโญโญโญ Comprehensive with benchmarks
Documentation โญโญโญโญโญ Clear and complete

๐Ÿ Recommendation

โœ… APPROVED FOR MERGE

This implementation demonstrates excellent software engineering practices and is ready for production use. The identified improvements are minor and don't impact the core functionality or safety of the implementation.

The ObjectBox driver will be a valuable addition to the Fiber storage ecosystem, providing users with a high-performance embedded database option with proper expiration handling and cleanup.

Great work @karnadii! This is a solid contribution that follows the project's standards and provides comprehensive functionality. ๐ŸŽ‰

coderabbitai[bot] avatar Jun 24 '25 11:06 coderabbitai[bot]

/gemini review

ReneWerner87 avatar Jun 24 '25 11:06 ReneWerner87

@karnadii I have had our 3 ai bots analyze your code, you can look at their tips and implement them if they make sense then it will be a perfect start

ReneWerner87 avatar Jun 24 '25 11:06 ReneWerner87

@ReneWerner87 could you take a look again? I have made a lot of changes

karnadii avatar Jul 03 '25 04:07 karnadii

Ok

ReneWerner87 avatar Jul 03 '25 05:07 ReneWerner87