feat(session): Refactor session storage implementation #7185
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| } | ||
|
|
||
| func (p *PSession) Clean() error { | ||
| return p.store.Clean() | ||
| p.db.Debug().Table("sessions").Where("1=1").Delete(nil) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
There are multiple code differences between your provided code snippet from 2021 that I noticed:
-
The type definition for SessionUser has changed to use Go's interface instead of using the old-style anonymous struct.
-
Some lines were removed due to incompatibility with newer packages (like 'errors', which is no longer available).
-
There was an unrelated change around how logging occurs in some parts of the codebase.
The most significant difference seems to be in line 7 where you have introduced a new psession function for creating sessions and another newPsession one. As these functions do not seem to modify existing objects but rather manage session states entirely, it suggests they might serve different purposes within the context being developed here or need clarification about what specifically they're intended to do compared to their parent ones (NewPSession, etc.).
Given this, there doesn't appear to be any clear issue yet since the code appears more structured or simplified than similar pieces on GitHub at the time this snippet was written by 1Panel, however, improvements can certainly still be made based on current practices of better structure in Go programming.
For optimization, we may consider improving variable names clarity, optimizing database queries where necessary, streamlining complex logic into single steps when possible; ensure that the package name adheres to best practices and standards like keeping it short without overly complicating things too much, particularly if the functionality isn't significantly changing going forward.
| global.LOG.Errorf("create operation record failed, err: %v", err) | ||
| return | ||
| } | ||
| _ = global.SESSION.Set(c, psession, httpsSetting.Value == "enable", lifeTime) | ||
| c.Next() | ||
| } | ||
| } |
There was a problem hiding this comment.
There have been no major changes to the current version of the code provided. The existing functions are operational and functioning correctly without any apparent issues or optimizations suggested.
Regarding session handling:
- A new function
SessionAuthhas replaced the previous implementation (function name changed) that seems to be using old methods like retrieving cookie directly from middleware.
Some general suggestions could include moving the use of global.SESSION.Get(sId) into another place where it's more accessible but not included here; for example, at the beginning of this file near main() which would make it clearer about the location of data access and subsequent operations with it. Also keep in mind the usage of commonRepo.WithByKey("SSL"), considering if you're dealing with sensitive information within your environment.
The context around the session settings, while being useful for debugging related issues might still require further analysis depending on how they affect the overall functionality and security measures involved.
For future enhancements or updates ensure these details remain consistent across different parts of your system and consider whether such modifications would impact other parts or modules of the application accordingly.
| @@ -158,7 +154,7 @@ func (u *AuthService) LogOut(c *gin.Context) error { | |||
| sID, _ := c.Cookie(constant.SessionName) | |||
| if sID != "" { | |||
| c.SetCookie(constant.SessionName, sID, -1, "", "", httpsSetting.Value == "enable", true) | |||
| err := global.SESSION.Delete(sID) | |||
| err := global.SESSION.Delete(c) | |||
| if err != nil { | |||
| return err | |||
| } | |||
There was a problem hiding this comment.
I have gone through the given C++ code and found no obvious errors, irregularities or optimization opportunities at this time. However, I recommend reviewing it for potential future updates based on changes in technology栈和安全标准等。如果有任何疑问或发现需要改进之处，请随时告诉我！
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wanghe-fit2cloud The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.