🧹 chore: Add CBOR support to AutoFormat()#3665
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughCBOR (Concise Binary Object Representation) support was added to the context's auto-formatting logic, tests, and documentation. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FiberCtx
participant Responder
Client->>FiberCtx: HTTP request with Accept: application/cbor
FiberCtx->>Responder: Call AutoFormat(body)
Responder->>FiberCtx: Serializes body to CBOR
FiberCtx-->>Client: HTTP response with Content-Type: application/cbor, CBOR body
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds CBOR (Concise Binary Object Representation) support to the AutoFormat functionality, enabling automatic content negotiation for CBOR responses when the Accept header is application/cbor.
Key Changes:
- Extended AutoFormat to handle CBOR content type alongside existing formats (JSON, XML, MessagePack, etc.)
- Updated documentation to reflect CBOR support in AutoFormat
- Added comprehensive test coverage for CBOR AutoFormat functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| res.go | Added CBOR to accepted content types and case handling in AutoFormat method |
| docs/guide/advance-format.md | Added documentation for CBOR usage with AutoFormat |
| docs/api/ctx.md | Updated AutoFormat documentation to include CBOR and added usage example |
| ctx_test.go | Added test cases for CBOR AutoFormat with both string and struct data |
There was a problem hiding this comment.
Summary of Changes
Hello @gaby, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant enhancement by integrating CBOR (Concise Binary Object Representation) support into the existing AutoFormat functionality. This allows the application to automatically serve responses in CBOR format when requested via the Accept header, improving content negotiation capabilities for binary data. The changes include core logic updates, comprehensive testing, and corresponding documentation to guide users on this new feature.
Highlights
- CBOR Autoformat Support: The
AutoFormatfunction now supportsapplication/cborcontent type, enabling automatic encoding of responses into CBOR format based on theAcceptHTTP header. - Documentation Updates: The API documentation for
AutoFormat(docs/api/ctx.md) and the advanced formats guide (docs/guide/advance-format.md) have been updated to reflect the new CBOR support, including usage examples. - Comprehensive Testing: New test cases have been added to
ctx_test.goto verify the correct handling of CBOR encoding for both simple strings and complex struct data when usingAutoFormat.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces CBOR support to the Fiber framework's AutoFormat functionality. The changes include updates to the core logic, addition of test cases, and documentation enhancements. The addition of CBOR support enhances the framework's ability to handle different content types, providing more flexibility for developers.
I've left a few comments:
- A suggestion to improve the wording in the advanced formats guide for clarity.
- A note about updating the comment in
res.goto reflect the addition of CBOR as a supported content type. - A suggestion to remove the redundant
SetBodyRawcall in theCBORfunction.
Once these points are addressed, the PR should be in great shape to merge.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3665 +/- ##
=======================================
Coverage 91.84% 91.84%
=======================================
Files 113 113
Lines 11444 11447 +3
=======================================
+ Hits 10511 10514 +3
Misses 669 669
Partials 264 264
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/api/ctx.md (1)
1474-1513: Consider tightening CBOR example wording and clarifying setup prereq
- The new list correctly includes
application/cbor, 👍.- In the added CBOR example, we jump straight into calling
c.AutoFormat(user)without reminding users that they must have configured an encoder/decoder (as described in the advanced-format guide). A short forward reference like “… after registering a CBOR encoder/decoder (see advanced-format guide) …” would avoid confusion for first-time readers.Purely editorial—no functional issue.
docs/guide/advance-format.md (1)
70-70: Bullet mixes up request binding vs response encoding and has minor punctuation issue
Ctx.CBOR()is a response helper, whereas request binding is done viac.Bind().CBOR().
Recommend:- - Use `Ctx.CBOR()` to bind CBOR data directly to structs, similar to how you would use JSON binding. Alternatively, use `Ctx.AutoFormat()` to send response as CBOR when the Accept HTTP header is `application/cbor`, For more details, see the [AutoFormat documentation](../api/ctx.md#autoformat). + - Use `c.Bind().CBOR()` to bind CBOR request bodies to structs, similar to JSON binding. + To *respond* with CBOR, call `c.CBOR()` directly or rely on `c.AutoFormat()` when the client’s `Accept` header is `application/cbor`. + For more details, see the [AutoFormat documentation](../api/ctx.md#autoformat).This also fixes the comma splice after “…
application/cbor”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ctx_test.go(4 hunks)docs/api/ctx.md(2 hunks)docs/guide/advance-format.md(1 hunks)res.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/advance-format.mddocs/api/ctx.md
🧠 Learnings (17)
📓 Common learnings
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Fiber currently supports square bracket notation in form data with 'application/x-www-form-urlencoded' content type, but this feature is not available for multipart form data. The implementation for form-urlencoded is in ctx.go (BodyParser method), while multipart form data handling needs enhancement to support this feature.
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
Learnt from: ksw2000
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2025-07-08T13:41:20.627Z
Learning: ksw2000 suggests implementing a pluggable encoder/decoder registry system in Fiber instead of hardcoding all encoders in app.config. This would solve dependency management issues by allowing users to opt-in only to the formats they need, keeping JSON and XML as defaults while making CBOR and MsgPack explicit registrations.
📚 Learning: in the fiber framework, breaking changes are acceptable when moving from version 2 to version 3, inc...
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Applied to files:
docs/guide/advance-format.mddocs/api/ctx.mdctx_test.go
📚 Learning: fiber currently supports square bracket notation in form data with 'application/x-www-form-urlencode...
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Fiber currently supports square bracket notation in form data with 'application/x-www-form-urlencoded' content type, but this feature is not available for multipart form data. The implementation for form-urlencoded is in ctx.go (BodyParser method), while multipart form data handling needs enhancement to support this feature.
Applied to files:
docs/guide/advance-format.mddocs/api/ctx.md
📚 Learning: in the `middleware/adaptor/adaptor.go` file of the fiber framework, when updating context handling, ...
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
docs/guide/advance-format.mddocs/api/ctx.mdctx_test.go
📚 Learning: in the fiber framework, `context()` is being renamed to `requestctx()`, and `usercontext()` to `cont...
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
Applied to files:
docs/guide/advance-format.mddocs/api/ctx.md
📚 Learning: in documentation files for the fiber framework, code examples are often partial and don't repeat imp...
Learnt from: sixcolors
PR: gofiber/fiber#3446
File: docs/middleware/logger.md:44-44
Timestamp: 2025-05-13T00:19:16.407Z
Learning: In documentation files for the Fiber framework, code examples are often partial and don't repeat import statements that were shown in earlier examples, focusing instead on demonstrating specific usage patterns.
Applied to files:
docs/guide/advance-format.md
📚 Learning: ksw2000 suggests implementing a pluggable encoder/decoder registry system in fiber instead of hardco...
Learnt from: ksw2000
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2025-07-08T13:41:20.627Z
Learning: ksw2000 suggests implementing a pluggable encoder/decoder registry system in Fiber instead of hardcoding all encoders in app.config. This would solve dependency management issues by allowing users to opt-in only to the formats they need, keeping JSON and XML as defaults while making CBOR and MsgPack explicit registrations.
Applied to files:
docs/guide/advance-format.md
📚 Learning: when documenting go interface methods in the fiber project, avoid showing method signatures with the...
Learnt from: mdelapenya
PR: gofiber/fiber#3434
File: docs/api/services.md:39-43
Timestamp: 2025-05-07T13:07:33.899Z
Learning: When documenting Go interface methods in the Fiber project, avoid showing method signatures with the interface type as the receiver (e.g., `func (d *Service) Method()`) since interfaces cannot be used as receivers in Go. Instead, show just the method signature without a receiver or use a placeholder implementation name.
Applied to files:
docs/guide/advance-format.md
📚 Learning: feature request #3224 has been created to add support for square bracket notation and comma-separate...
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
Applied to files:
docs/guide/advance-format.md
📚 Learning: in the fiber framework tests, using `ctx.response.header.cookie` may not be suitable for parsing coo...
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
docs/guide/advance-format.mddocs/api/ctx.mdctx_test.go
📚 Learning: in `defaulterrorhandler(c *fiber.ctx, err error)`, since `c` is a pointer to an interface, we need t...
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Applied to files:
docs/guide/advance-format.mdctx_test.go
📚 Learning: in the go unit tests in `ctx_test.go`, it is acceptable to use invalid cidr notation such as `"0.0.0...
Learnt from: gaby
PR: gofiber/fiber#3170
File: ctx_test.go:1721-1724
Timestamp: 2024-10-16T12:12:30.506Z
Learning: In the Go unit tests in `ctx_test.go`, it is acceptable to use invalid CIDR notation such as `"0.0.0.1/31junk"` for testing purposes.
Applied to files:
ctx_test.go
📚 Learning: the project uses the testify/assert package for assertions in unit tests....
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Applied to files:
ctx_test.go
📚 Learning: in go test files, prefer using the `require` methods from the `testify` package for assertions inste...
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`.
Applied to files:
ctx_test.go
📚 Learning: parallel tests for `session.save` already exist in the `middleware/session/session_test.go` file, sp...
Learnt from: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-06-30T00:38:06.580Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Applied to files:
ctx_test.go
📚 Learning: in the `test_app_shutdownwithcontext` function in `app_test.go`, the `clientdone` channel is used to...
Learnt from: efectn
PR: gofiber/fiber#3162
File: app_test.go:893-895
Timestamp: 2024-11-29T12:37:27.581Z
Learning: In the `Test_App_ShutdownWithContext` function in `app_test.go`, the `clientDone` channel is used to synchronize the client's request completion before proceeding, eliminating the need for additional `time.Sleep` calls.
Applied to files:
ctx_test.go
📚 Learning: in the session middleware `config` struct, the `extractor` field uses function closures (like `fromc...
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
ctx_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Compare
- GitHub Check: lint
- GitHub Check: Analyse
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: repeated
- GitHub Check: unit (1.24.x, windows-latest)
🔇 Additional comments (8)
res.go (3)
363-365: Doc-string correctly updated – thanks for keeping it in sync.
The newly listedapplication/cborandapplication/vnd.msgpackcontent types match the implementation below.
390-392: Default CBOR encoder is already registeredThe
Configstruct declaresCBOREncoder(app.go:318), and inNew()we guard against it being nil:
- app.go #590–592:
if app.config.CBOREncoder == nil { app.config.CBOREncoder = binder.UnimplementedCborMarshal }This ensures
r.CBOR()never dereferences a nil function pointer. No further changes are needed.
368-368: Verify CBOR MIME mappingI wasn’t able to find a
GetMIMEimplementation or a"cbor"→"application/cbor"entry in the codebase. Please confirm that your extension-to-MIME map includes:"cbor": "application/cbor"
so that
Type("cbor")correctly setsContent-Type: application/cbor. Add this mapping if it’s missing before merging.ctx_test.go (5)
46-48: LGTM! Proper CBOR configuration in test setup.The CBOR encoder and decoder configuration is correctly added to the app configuration in multiple test functions. This ensures that the CBOR functionality is properly initialized for testing.
Also applies to: 79-81, 164-166
531-532: LGTM! Consistent CBOR configuration for AutoFormat test.The CBOR encoder and decoder are properly configured alongside the existing MsgPack configuration, maintaining consistency with the test pattern.
559-565: LGTM! Well-structured CBOR test case in AutoFormat.The CBOR test case follows the established pattern:
- Sets the Accept header to
MIMEApplicationCBOR- Calls
c.AutoFormat()with test data- Verifies the response body matches the expected CBOR byte sequence
The expected byte sequence
[]byte{0x6d, 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x2c, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x21}appears to be the correct CBOR encoding for the string "Hello, World!".
598-599: LGTM! Consistent CBOR configuration for struct AutoFormat test.The CBOR encoder and decoder are properly configured in the Test_Ctx_AutoFormat_Struct function, maintaining consistency with the previous test function.
636-640: LGTM! Well-structured CBOR struct test case.The CBOR test case for struct serialization is properly implemented:
- Sets the Accept header to
MIMEApplicationCBOR- Calls
c.AutoFormat()with the test struct- Uses
hex.EncodeToString()to verify the CBOR-encoded response bodyThe expected hex string
"a36653656e646572654361726f6c6a526563697069656e74738265416c69636563426f6267557267656e637903"represents the CBOR encoding of the Message struct with the test data.
|
I'm confused why is CBOR encoding different between OS's ? |
yes this is indeed confusing |
|
@coderabbitai Can you help clear up the confusion? |
Summary
application/cborinAutoFormatAutoFormatFixes #3664