Skip to content
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

refactor: change WithValue key type to any #88

Merged
merged 11 commits into from
Sep 15, 2024

Conversation

mdanialr
Copy link
Contributor

@mdanialr mdanialr commented Sep 13, 2024

📑 Description

Implementation of #630

Since the underlying gin does not support any as the key type, we use Ctx which is context.Context as the bag instead and only used as a bag. Because the cancellation signal should only come from the underlying gin instead of the goravel itself.

Summary by CodeRabbit

  • New Features

    • Introduced a new context management system for improved handling of key-value pairs.
    • Enhanced flexibility in storing and retrieving various data types within the context.
  • Bug Fixes

    • Updated context retrieval methods to prevent runtime errors from type mismatches.
  • Tests

    • Expanded test coverage to include new key-value pairs and custom types for context validation.
  • Chores

    • Updated dependency version for improved performance and potential new features.

✅ Checks

  • Added test cases for my code

Copy link

coderabbitai bot commented Sep 13, 2024

Important

Review skipped

Auto reviews are limited to specific labels.

Labels to auto review (1)
  • 🚀 Review Ready

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes introduced in this pull request enhance the context management capabilities within the application. A new type Ctx is added to wrap the standard context.Context, and the Context struct is modified to utilize this new type effectively. The WithValue and Value methods are updated to support more flexible key types, and the go.mod file reflects an upgrade to a newer version of the github.com/goravel/framework dependency. Additionally, test cases are improved to validate the handling of various key-value pairs in context.

Changes

Files Change Summary
context.go Introduced Ctx type, modified NewContext to initialize Ctx with context.Background(), updated WithValue to accept any type for keys, and adjusted Context and Value methods to utilize Ctx.
context_test.go Enhanced TestContext by adding custom types for key-value pairs, modified assertions for clarity, and ensured proper validation of various types stored in context.
go.mod Updated github.com/goravel/framework dependency version from v1.14.5 to v1.14.6-0.20240912145232-6543aeadf1bc.
group_test.go Modified TestGroup to retrieve context values without type assertions, allowing for greater flexibility in the types of values stored in context.

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?

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>.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @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.

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 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 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.

@mdanialr
Copy link
Contributor Author

@coderabbitai review

Copy link

coderabbitai bot commented Sep 13, 2024

Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 13, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (v1.2.x@b7169e5). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             v1.2.x      #88   +/-   ##
=========================================
  Coverage          ?   80.07%           
=========================================
  Files             ?       14           
  Lines             ?      813           
  Branches          ?        0           
=========================================
  Hits              ?      651           
  Misses            ?      125           
  Partials          ?       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

context.go Outdated Show resolved Hide resolved
context.go Outdated Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 13, 2024
@mdanialr mdanialr requested a review from hwbrzzl September 13, 2024 09:44
context.go Outdated Show resolved Hide resolved
@mdanialr mdanialr requested a review from hwbrzzl September 15, 2024 06:23
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR 👍

@hwbrzzl hwbrzzl merged commit 3c0efad into goravel:v1.2.x Sep 15, 2024
7 checks passed
@hwbrzzl
Copy link
Contributor

hwbrzzl commented Sep 15, 2024

Hey @mdanialr Added you to the contributor list, thanks for your great PRs! Hope to receive more of your perfect contributions. And I'll release v1.14.6 recently.

@mdanialr mdanialr deleted the ref/change-withvalue-key-type-to-any branch September 15, 2024 12:59
hwbrzzl added a commit that referenced this pull request Sep 21, 2024
* fix: [#455] Session same_site config not working (#71)

* add sameSite in cookie

* check if sameSite exist in map

* feat: add Shutdown to support graceful quit of HTTP Server (#83)

* 1. add Shutdown to support graceful quit of HTTP Server
2. Unify the behavior of Run/RunTLS/RunTLSWithCert. The original RunTLS/RunTLSWithCert did not apply the http.AllowQuerySemicolons and MaxHeaderBytes configuration.

* compatible with multiple servers

* improve unit testing

* improve unit testing

* improve unit testing

---------

Co-authored-by: 朱祥东 <xiangdong.zhu@maitang001.com>

* feat: Optmize Shutdown (#84)

* feat: Optmize Shutdown

* fix test cases

* refactor: change `WithValue` key type to any (#88)

* bump version goravel framework

* ref: use any as the key type for WithValue

* test: more test cases for WithValue and Context

* test: swap the expected and actual value in assertion

* fix: need to store the key-val to underlying gin too

* test: remove type assert

* ref: immediately use `context.Context` as field ctx instead

* feat: helper to get goravel data from gin context

* ref: use helper to get goravel data from gin context instead

* test: add custom key to `WithValue` in group_test

* ref: remove the unnecessary ctx of `context.Context`

* upgrade: v1.2.5 (#89)

---------

Co-authored-by: krishan kumar <84431594+kkumar-gcc@users.noreply.github.com>
Co-authored-by: zxdstyle <1430822515@qq.com>
Co-authored-by: 朱祥东 <xiangdong.zhu@maitang001.com>
Co-authored-by: Danial <48054961+mdanialr@users.noreply.github.com>
@coderabbitai coderabbitai bot mentioned this pull request Dec 15, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants