🧹 chore: Refactor Opt-in support for CBOR #3580
Conversation
WalkthroughThis change removes the default CBOR encoder/decoder implementation and its dependency, replacing them with unimplemented stub functions that panic if called. All test and example code is updated to explicitly configure CBOR support. Documentation is revised to reflect the need for explicit CBOR setup and provides guidance for enabling CBOR functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FiberApp
participant CBORLib
User->>FiberApp: Initialize app (without CBOR config)
FiberApp->>FiberApp: Use UnimplementedCborMarshal/Unmarshal
User->>FiberApp: Attempt CBOR operation
FiberApp-->>User: Panic with setup instruction
User->>FiberApp: Initialize app (with CBOR encoder/decoder)
FiberApp->>CBORLib: Use cbor.Marshal/Unmarshal for CBOR ops
User->>FiberApp: Perform CBOR operation
FiberApp-->>User: Successful CBOR encoding/decoding
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 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). (5)
✨ Finishing Touches
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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ReneWerner87, 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 significantly refactors how CBOR (Concise Binary Object Representation) support is handled within the framework. The primary goal is to transition CBOR functionality from an implicit, built-in dependency to an explicit, opt-in feature. This change provides greater flexibility for users to integrate their preferred CBOR libraries and reduces the default dependency footprint of the core framework.
Highlights
- Opt-in CBOR Support: I've refactored the CBOR encoding and decoding mechanisms to be opt-in rather than implicitly available. This means that by default, Fiber will no longer include a CBOR implementation, allowing users to choose their preferred CBOR library or avoid the dependency entirely if not needed.
- New Unimplemented Stubs: To facilitate the opt-in behavior, I've introduced
UnimplementedCborMarshalandUnimplementedCborUnmarshalfunctions. These functions are now the default forCBOREncoderandCBORDecoderinfiber.Configand will panic with a helpful message if CBOR functionality is attempted without explicit configuration. - Test and Documentation Updates: I've updated all relevant tests and benchmarks to explicitly configure CBOR encoders and decoders where CBOR functionality is required. Additionally, the documentation has been updated to clearly guide users on how to set up and enable CBOR support in their Fiber applications.
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 is currently in preview and 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 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.
Pull Request Overview
This PR refactors CBOR support to be opt-in by introducing unimplemented stubs that panic when CBOR isn’t explicitly configured. It updates configuration defaults, documentation, and tests accordingly.
- Introduce
UnimplementedCborMarshal/UnimplementedCborUnmarshalas default CBOR handlers - Update Fiber docs with CBOR setup instructions and adjust default encoder/decoder entries
- Modify tests to explicitly pass CBOR encoder/decoder in
New(Config{…})
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/guide/advance-format.md | Added CBOR section with setup and usage instructions |
| docs/api/fiber.md | Updated default encoder/decoder entries for CBOR and MsgPack |
| docs/api/ctx.md | Added note referencing CBOR setup guide |
| docs/api/bind.md | Added note referencing CBOR setup guide |
| ctx_test.go | Updated tests to pass CBOR encoder/decoder in app configuration |
| client/helper_test.go | Updated helper tests to configure CBOR encoder/decoder |
| binder/cbor.go | Implemented unimplemented stubs with panic for CBOR defaults |
| binder/cbor_test.go | Added tests for unimplemented CBOR marshal/unmarshal panic cases |
| bind_test.go | Updated bind tests to configure CBOR encoder/decoder |
| app.go | Changed default CBOR initialization to unimplemented stubs |
Comments suppressed due to low confidence (2)
docs/api/fiber.md:67
- The API docs list the default values as "*
Unimplement". Please replace with the exact function names (e.g.,UnimplementedCborMarshal,UnimplementedCborUnmarshal,UnimplementedMsgpackMarshal,UnimplementedMsgpackUnmarshal) to accurately reflect the implemented defaults.
| <Reference id="cborencoder">CBOREncoder</Reference> | `utils.CBORMarshal` | Allowing for flexibility in using another cbor library for encoding. | *`Unimplement` |
ctx_test.go:45
- [nitpick] Indentation mixes spaces and tabs. Align with the existing project style (tabs only) to keep formatting consistent across tests.
CBOREncoder: cbor.Marshal,
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3580 +/- ##
=======================================
Coverage 90.96% 90.96%
=======================================
Files 111 111
Lines 11124 11128 +4
=======================================
+ Hits 10119 10123 +4
Misses 753 753
Partials 252 252
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:
|
There was a problem hiding this comment.
Code Review
The code changes introduce optional CBOR support, enhancing modularity. I've identified a few areas for improvement, mainly related to typos in user-facing panic messages and documentation, and enhancing the new tests for better clarity and correctness. Addressing these points will improve the overall quality and developer experience.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
binder/cbor_test.go (2)
112-123: Remove unreachable code in panic message test.The test correctly verifies the panic message, but line 122 (
require.NoError(t, err)) will never be reached because the function panics before returning.- _, err := UnimplementedCborMarshal(struct{ Name string }{Name: "test"}) - - require.NoError(t, err) + UnimplementedCborMarshal(struct{ Name string }{Name: "test"})
125-137: Remove unreachable code in panic message test.The test correctly verifies the panic message, but lines 134-136 (
err := UnimplementedCborUnmarshal([]byte{0xa0}, &out)andrequire.NoError(t, err)) will never be reached because the function panics before returning.- var out any - err := UnimplementedCborUnmarshal([]byte{0xa0}, &out) - - require.NoError(t, err) + var out any + UnimplementedCborUnmarshal([]byte{0xa0}, &out)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app.go(2 hunks)bind_test.go(4 hunks)binder/cbor.go(1 hunks)binder/cbor_test.go(1 hunks)client/helper_test.go(2 hunks)ctx_test.go(6 hunks)docs/api/bind.md(1 hunks)docs/api/ctx.md(1 hunks)docs/api/fiber.md(1 hunks)docs/guide/advance-format.md(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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: 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`.
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.
client/helper_test.go (14)
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`.
Learnt from: mdelapenya
PR: gofiber/fiber#3434
File: services_test.go:450-464
Timestamp: 2025-05-15T12:56:45.397Z
Learning: The `New` function in the Fiber framework automatically calls `startServices` at initialization time when services are configured, making explicit calls to `startServices` unnecessary in code that creates an App instance with `New()`.
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`.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
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.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
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.
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: 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.
Learnt from: mdelapenya
PR: gofiber/fiber#3434
File: services_test.go:450-464
Timestamp: 2025-05-15T12:56:45.397Z
Learning: In the Fiber framework, the `New` function automatically calls `startServices` when services are configured, so there's no need to explicitly call `startServices` after creating a new app with `New(Config{Services: ...})`.
Learnt from: mdelapenya
PR: gofiber/fiber#3434
File: app.go:623-636
Timestamp: 2025-05-08T08:14:37.302Z
Learning: In the gofiber/fiber framework, service startup failures should panic rather than allowing the application to continue running with degraded functionality, as this is the agreed-upon design decision.
docs/api/fiber.md (12)
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`.
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.
Learnt from: sigmundxia
PR: gofiber/fiber#3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In Fiber, the `c.Set` method does not return an error.
Learnt from: sigmundxia
PR: gofiber/fiber#3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-09-26T03:08:50.917Z
Learning: In Fiber, the `c.Set` method does not return an error.
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.
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.
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.
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`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
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.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
docs/api/ctx.md (8)
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.
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`.
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`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
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.
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.
Learnt from: sigmundxia
PR: gofiber/fiber#3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-09-26T03:08:50.917Z
Learning: In Fiber, the `c.Set` method does not return an error.
Learnt from: sigmundxia
PR: gofiber/fiber#3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In Fiber, the `c.Set` method does not return an error.
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.
docs/guide/advance-format.md (11)
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.
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`.
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.
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: 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`.
Learnt from: mdelapenya
PR: gofiber/fiber#3434
File: app.go:623-636
Timestamp: 2025-05-08T08:14:37.302Z
Learning: In the gofiber/fiber framework, service startup failures should panic rather than allowing the application to continue running with degraded functionality, as this is the agreed-upon design decision.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
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.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
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`.
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`.
app.go (9)
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`.
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`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
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`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
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`.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
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.
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.
ctx_test.go (16)
undefined
<retrieved_learning>
Learnt from: ReneWerner87
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: gaby
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: gaby
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: efectn
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: gaby
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: efectn
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware Config struct, Store is backed by fiber.Storage; they are different entities serving distinct purposes in session management.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware Config struct, Store is backed by fiber.Storage; they are different entities serving distinct purposes in session management.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the Test_CSRF_WithSession_Middleware function, calling session.NewWithStore() without arguments is acceptable, as the default configuration is sufficient.
</retrieved_learning>
<retrieved_learning>
Learnt from: sixcolors
PR: #3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Test_CSRF_WithSession_Middleware function, calling session.NewWithStore() without arguments is acceptable, as the default configuration is sufficient.
</retrieved_learning>
bind_test.go (8)
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`.
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`.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
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`.
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`.
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.
binder/cbor_test.go (11)
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: 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`.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-10-08T19:06:06.583Z
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.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-07-02T13:29:56.992Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
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.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.
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.
🧬 Code Graph Analysis (2)
app.go (1)
binder/cbor.go (2)
UnimplementedCborMarshal(27-29)UnimplementedCborUnmarshal(31-33)
binder/cbor_test.go (1)
binder/cbor.go (2)
UnimplementedCborMarshal(27-29)UnimplementedCborUnmarshal(31-33)
🪛 GitHub Check: lint
ctx_test.go
[failure] 162-162:
File is not properly formatted (gofmt)
bind_test.go
[failure] 1221-1221:
File is not properly formatted (gofmt)
🪛 GitHub Actions: golangci-lint
bind_test.go
[error] 1221-1221: File is not properly formatted (gofmt)
⏰ 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). (4)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (16)
docs/api/bind.md (1)
130-131: Good documentation update for opt-in CBOR support.The note clearly informs users about the need for explicit CBOR setup, which aligns perfectly with the PR objective of making CBOR support opt-in. The formatting and placement are consistent with the existing MsgPack documentation above.
docs/api/ctx.md (1)
1900-1901: Consistent documentation update for CBOR setup requirement.The note maintains consistency with the similar update in bind.md and provides clear guidance to users about the need for explicit CBOR setup. The formatting and messaging are appropriate.
client/helper_test.go (1)
8-8: Proper implementation of explicit CBOR setup for tests.The test helper correctly demonstrates the new explicit CBOR setup pattern by importing the external library and configuring the Fiber app with CBOR encoder/decoder functions. This ensures tests continue to work while showcasing the required setup that users will need to implement.
Also applies to: 26-29
ctx_test.go (2)
30-30: LGTM: Appropriate import additionThe import of the CBOR package is correctly added to support the explicit CBOR configuration in the tests.
44-47: LGTM: Consistent CBOR configuration patternThe explicit CBOR encoder/decoder configuration aligns well with the PR objective to make CBOR support opt-in. The implementation is consistent across test functions.
Also applies to: 77-80
app.go (4)
341-342: LGTM! Comment correctly updated to reflect the new default.The comment accurately reflects the change from
cbor.Marshaltobinder.UnimplementedCborMarshal, maintaining consistency with the actual implementation.
348-349: LGTM! Comment correctly updated to reflect the new default.The comment accurately reflects the change from
cbor.Unmarshaltobinder.UnimplementedCborUnmarshal, maintaining consistency with the actual implementation.
615-616: LGTM! Proper initialization of opt-in CBOR encoder.The change from external CBOR library to the unimplemented binder function successfully makes CBOR support opt-in, aligning with the PR objectives to reduce dependencies.
618-619: LGTM! Proper initialization of opt-in CBOR decoder.The change from external CBOR library to the unimplemented binder function successfully makes CBOR support opt-in, aligning with the PR objectives to reduce dependencies.
docs/guide/advance-format.md (2)
3-6: LGTM! Title and description correctly updated to include CBOR.The title and description now accurately reflect that both MsgPack and CBOR are covered in this guide, providing clarity for users.
66-105: Excellent documentation for CBOR support.The new CBOR section is well-structured and comprehensive, covering:
- Clear explanation that CBOR is opt-in
- Installation instructions for the recommended library
- Complete configuration example with proper imports
- Usage example demonstrating binding and response handling
- Consistent formatting with the existing MsgPack section
The documentation effectively guides users through enabling CBOR support after the change to opt-in configuration.
binder/cbor_test.go (2)
93-100: Good test structure for panic verification.The test properly verifies that
UnimplementedCborMarshalpanics when called, usingrequire.Panicsappropriately. The parallel execution and testify usage align with project conventions.
102-110: Good test structure for panic verification.The test properly verifies that
UnimplementedCborUnmarshalpanics when called, usingrequire.Panicsappropriately. The parallel execution and testify usage align with project conventions.bind_test.go (3)
882-883: LGTM! Correctly configured CBOR support for body binding tests.The explicit CBOR encoder/decoder configuration aligns with the opt-in approach and ensures the test can properly validate CBOR functionality.
1170-1171: LGTM! Consistent CBOR configuration in benchmark setup.The explicit CBOR configuration ensures all encoding formats are properly available for comprehensive benchmarking scenarios.
1877-1880: LGTM! Proper CBOR configuration for comprehensive parser testing.The explicit CBOR configuration is essential since this test validates CBOR functionality alongside other parsers in the same struct binding scenarios.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 4ad87db | Previous: c1e9ae3 | Ratio |
|---|---|---|---|
Benchmark_SlashRecognition/forEach |
5.804 ns/op 0 B/op 0 allocs/op |
3.685 ns/op 0 B/op 0 allocs/op |
1.58 |
Benchmark_SlashRecognition/forEach - ns/op |
5.804 ns/op |
3.685 ns/op |
1.58 |
This comment was automatically generated by workflow using github-action-benchmark.
efectn
left a comment
efectn
left a comment
There was a problem hiding this comment.
seems ok except some grammar issues mentioned by gemini
gaby
changed the title
Summary
Testing
go test ./...https://chatgpt.com/codex/tasks/task_e_6874fea1d5c88326b7cb049c040e806a