Skip to content

feat: optimize facades.Auth().Guard() return value#967

Merged
hwbrzzl merged 8 commits intomasterfrom
bowen/optimize-auth
Mar 27, 2025
Merged

feat: optimize facades.Auth().Guard() return value#967
hwbrzzl merged 8 commits intomasterfrom
bowen/optimize-auth

Conversation

@hwbrzzl
Copy link
Contributor

@hwbrzzl hwbrzzl commented Mar 23, 2025

📑 Description

This pull request includes significant changes to the auth package, focusing on refactoring the authentication system and improving the JWT guard implementation. The most important changes include restructuring the Auth and JwtGuard classes, updating the OrmUserProvider, and adding new tests for the OrmUserProvider.

Refactoring and Improvements to Auth and JwtGuard:

  • auth/auth.go: Refactored the Auth class to use sync.Map for storing guard and provider functions, removed unused dependencies, and simplified the constructor by eliminating the need for cache and orm parameters. Added log as a new dependency.
  • auth/jwt_guard.go: Introduced a new NewJwtGuard function to initialize JwtGuard with context and configuration. Added new fields for secret, ttl, and refreshTtl to JwtGuard. Updated methods to use these fields and replaced the old authToken method with jwtToken. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Updates to OrmUserProvider:

  • auth/orm_user_provider.go: Refactored OrmUserProvider to use the global ormFacade and added a context parameter to its methods. Implemented error handling for missing ORM facade.
  • auth/orm_user_provider_test.go: Added new test cases for OrmUserProvider to ensure proper functionality and error handling.

Dependency Management:

  • auth/service_provider.go: Updated the service provider to set global facades for cache, config, and orm. Simplified the Register method to handle optional context parameters.

Contract Changes:

✅ Checks

  • Added test cases for my code

Copilot AI review requested due to automatic review settings March 23, 2025 04:30
@hwbrzzl hwbrzzl requested a review from a team as a code owner March 23, 2025 04:30
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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: ef9e859 Previous: aec2bbb Ratio
BenchmarkFile_ReadWrite 346532 ns/op 2073 B/op 28 allocs/op 228545 ns/op 2072 B/op 28 allocs/op 1.52
BenchmarkFile_ReadWrite - ns/op 346532 ns/op 228545 ns/op 1.52

This comment was automatically generated by workflow using github-action-benchmark.

@codecov
Copy link

codecov bot commented Mar 23, 2025

Codecov Report

Attention: Patch coverage is 79.56204% with 28 lines in your changes missing coverage. Please review.

Project coverage is 69.25%. Comparing base (aec2bbb) to head (935bbfd).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
auth/jwt_guard.go 79.71% 9 Missing and 5 partials ⚠️
auth/service_provider.go 0.00% 12 Missing ⚠️
auth/auth.go 95.12% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #967      +/-   ##
==========================================
+ Coverage   69.15%   69.25%   +0.10%     
==========================================
  Files         160      160              
  Lines       10718    10712       -6     
==========================================
+ Hits         7412     7419       +7     
+ Misses       2970     2958      -12     
+ Partials      336      335       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +15 to +16
guardFuncs = sync.Map{}
providersFuncs = sync.Map{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the previous version, custom guards and custom user providers were missed due to facades.Auth() not being a singleton. Hence, we need to save them to global variables. cc @vendion

Comment on lines +34 to +35
auth.Extend("jwt", NewJwtGuard)
auth.Provider("orm", NewOrmUserProvider)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the previous version, NewJwtGuard and NewOrmUserProvider do not implement GuardFunc and UserProviderFunc, but they should. cc @vendion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the session guard driver and the current test cases, there are many test cases that can't be used in the session guard driver, so move them to the jwt_guard_test.go.

@hwbrzzl hwbrzzl force-pushed the bowen/optimize-auth branch from a0961c8 to ef9e859 Compare March 27, 2025 03:57
@hwbrzzl hwbrzzl merged commit 14d38dd into master Mar 27, 2025
13 checks passed
@hwbrzzl hwbrzzl deleted the bowen/optimize-auth branch March 27, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant