-
Notifications
You must be signed in to change notification settings - Fork 16
Add migration for KeyLookup to Extractor in session config #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughThe changes simplify a string replacement operation in the session configuration migration logic and introduce five new parallelized tests. These tests verify that the migration function correctly transforms the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant Migration as MigrateSessionExtractor
participant Source as Go Source File
Test->>Source: Write session.Config with KeyLookup
Test->>Migration: Call MigrateSessionExtractor
Migration->>Source: Read and transform config
Source-->>Migration: Return migrated config (Extractor field)
Migration-->>Test: Return result and log output
Test->>Test: Assert correct migration and log
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningscmd/internal/migrations/v3/common.go (8)Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: dave-gray101 Learnt from: dave-gray101 cmd/internal/migrations/v3/common_test.go (23)Learnt from: sixcolors Learnt from: sixcolors Learnt from: gaby Learnt from: gaby Learnt from: gaby Learnt from: gaby Learnt from: sixcolors Learnt from: ReneWerner87 Learnt from: gaby Learnt from: gaby Learnt from: efectn Learnt from: juls0730 Learnt from: norri Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: norri Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors 🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @sixcolors, 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 crucial migration step for session middleware configurations, updating the deprecated KeyLookup field to the new Extractor field. This change is essential for maintaining forward compatibility and simplifying the upgrade path for users. The implementation includes robust parsing and conversion logic, backed by comprehensive test coverage to ensure the reliability and correctness of the migration process across various scenarios.
Highlights
- Session Configuration Migration: I've implemented a new migration step within
MigrateSessionConfigto automatically convertKeyLookupfields insession.Configstructures to the newExtractorformat. This handles common sources likecookie,header, andquery, transformingKeyLookup: "source:name"intoExtractor: session.FromSource("name"). - Robust Migration Handling: The migration logic includes robust handling for invalid or unsupported
KeyLookupformats. If aKeyLookupentry does not conform to the expectedsource:namepattern or uses an unsupported source, it will be safely removed from the configuration to prevent potential errors during runtime. - Comprehensive Test Coverage: I've added a suite of new unit tests to
common_test.goto thoroughly verify theKeyLookuptoExtractormigration. These tests cover all supported sources (cookie, header, query) and confirm the correct behavior when encountering unsupported or malformedKeyLookupinputs, ensuring the migration's reliability.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a migration script to convert the deprecated KeyLookup field to the new Extractor field in session middleware configurations. It includes regex-based replacement logic and corresponding unit tests for various scenarios. My review focuses on improving the robustness of the parsing logic and the maintainability of the new tests.
|
i combined it with a5e7ef7 |
|
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
Implement migration logic to convert
KeyLookupconfigurations toExtractorin session middleware. Include tests for various scenarios, ensuring proper handling of cookie, header, and query sources, as well as unsupported formats.Re PR: gofiber/fiber#3625
Summary by CodeRabbit