8000 Add Hash and Crypt Facades and limiter middleware by devhaozi · Pull Request #33 · goravel/framework · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Hash and Crypt Facades and limiter middleware #33

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 40 commits into from
Feb 28, 2023
Merged

Add Hash and Crypt Facades and limiter middleware #33

merged 40 commits into from
Feb 28, 2023

Conversation

devhaozi
Copy link
Member

尝试写了一下Hash的Facades，还缺个mock的测试。
第一次写，可能会有些小问题需要改进。

@devhaozi devhaozi changed the title Add Hash Facades Add Hash and Crypt Facades Feb 10, 2023
@devhaozi
Copy link
Member Author

补上了Crypt，实现AES-256-GCM加密

@devhaozi devhaozi mentioned this pull request Feb 10, 2023
Closed
@hwbrzzl hwbrzzl self-requested a review February 11, 2023 03:18
@devhaozi
Copy link
Member Author

这个Actions似乎有点问题，不会自动运行

@hwbrzzl
Copy link
Contributor
hwbrzzl commented Feb 11, 2023

这个Actions似乎有点问题，不会自动运行

嗯，调整一个设置，观察下

@devhaozi
Copy link
Member Author

这个Actions似乎有点问题，不会自动运行

嗯，调整一个设置，观察下

Actions好像没有复制.env，导致测试挂了

@devhaozi devhaozi changed the title Add Hash and Crypt Facades Add Hash and Crypt Facades and limiter middleware Feb 11, 2023
@devhaozi devhaozi mentioned this pull request Feb 11, 2023
Closed
@hwbrzzl
Copy link
Contributor
hwbrzzl commented Feb 11, 2023

这个Actions似乎有点问题，不会自动运行

嗯，调整一个设置，观察下

Actions好像没有复制.env，导致测试挂了

目前 Unit tests 不能依赖 .env 运行，需要使用 mock.Config() 来进行配置，除非一些特殊情况，例如 config, filesystem, mail ，单元测试中会进行 .env 文件的判断，存在 .env 才执行单测，这三个模块的单测目前是在本地跑的。

下一步会优化这里，将一些特殊配置写入到 github 的 secret 中，以达到 Action 能够运行所有测试用例的目的。

hwbrzzl
hwbrzzl reviewed Feb 11, 2023
View reviewed changes
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.

Hey, 感谢 PR，提供了非常适用的功能。👍🏻
内容比较多，还没有来及确认具体逻辑，这几天我会继续 Review，目前只是提了一些格式上的小建议。

hwbrzzl
hwbrzzl reviewed Feb 14, 2023
View reviewed changes
@devhaozi devhaozi requested a review from hwbrzzl February 22, 2023 13:57
@hwbrzzl
Copy link
Contributor
hwbrzzl commented Feb 24, 2023 •
edited
Loading

Hi @devhaozi Is the optimization finished? Can we review this PR again, please?

@devhaozi
Copy link
Member Author

Hi @devhaozi Is the optimization finished? Can we review this PR again, please?

maybe, I haven't test.

@hwbrzzl
Copy link
Contributor
hwbrzzl commented Feb 25, 2023

Hi @devhaozi Is the optimization finished? Can we review this PR again, please?

maybe, I haven't test.

Thanks! Please tell me when you have confidence in it. 😃

@devhaozi
Copy link
Member Author

Hi @devhaozi Is the optimization finished? Can we review this PR again, please?

maybe, I haven't test.

Thanks! Please tell me when you have confidence in it. 😃

Test passed! Ready for review.

@codecov
Copy link
codecov bot commented Feb 25, 2023 •
edited
Loading

Codecov Report

Base: 48.15% // Head: 48.35% // Increases project coverage by +0.20% 🎉

Coverage data is based on head (f26f9e5) compared to base (5dafe40).
Patch coverage: 54.85% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   48.15%   48.35%   +0.20%     
==========================================
  Files          95      102       +7     
  Lines        6149     6320     +171     
==========================================
+ Hits         2961     3056      +95     
- Misses       3014     3070      +56     
- Partials      174      194      +20
Impacted Files Coverage Δ
crypt/service_provider.go 0.00% <0.00%> (ø)
hash/application.go 0.00% <0.00%> (ø)
hash/service_provider.go 0.00% <0.00%> (ø)
crypt/aes.go 50.00% <50.00%> (ø)
hash/argon2id.go 59.09% <59.09%> (ø)
hash/bcrypt.go 85.00% <85.00%> (ø)
auth/auth.go 84.73% <100.00%> (+2.29%) ⬆️
crypt/application.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

hwbrzzl
hwbrzzl reviewed Feb 25, 2023
View reviewed changes
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.

Hi 感谢 PR，当前 PR 有了巨大的进步！👍
这次提出了一些疑问，我们进一步讨论哈。另外我看之前提的一些 comments 好像有些没有处理，辛苦再看一下，解决后，可以点下 Resolve 按钮。master 有了更新，可以 rebase 下分支。

@devhaozi devhaozi requested a review from hwbrzzl February 26, 2023 07:51
hwbrzzl
hwbrzzl reviewed Feb 27, 2023
View reviewed changes
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.

A perfect PR! 👍
Just a few nitpicks, I think we can merge this PR after the optimization.

@hwbrzzl
Copy link
Contributor
hwbrzzl commented Feb 27, 2023

Please fix the unit test error.

hwbrzzl
hwbrzzl approved these changes Feb 28, 2023
View reviewed changes
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.

Perfect

@hwbrzzl hwbrzzl merged commit 8d212bc into goravel:master Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwbrzzl hwbrzzl hwbrzzl approved these changes

Assignees
No one assigned
Labels
✏️ Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

✨ [Feature] 增加hash和crypt的facades ✨ [Feature] 增加限流中间件
2 participants
@devhaozi @hwbrzzl
0