Skip to content

feat: add session manager and update README#6

Merged
hsluoyz merged 7 commits intoapache:masterfrom
bugoutianzhen123:master
Sep 15, 2025
Merged

feat: add session manager and update README#6
hsluoyz merged 7 commits intoapache:masterfrom
bugoutianzhen123:master

Conversation

@bugoutianzhen123
Copy link
Contributor

@bugoutianzhen123 bugoutianzhen123 commented Sep 10, 2025

Fix: #1

Added SessionManager to handle sessions and their lifecycle.
Added README content to explain and demonstrate continuous authorization usage.

These additions introduce robust session management and guide users on using continuous authorization.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a robust session management system and comprehensive documentation to the casbin-ucon project. The changes introduce a dedicated SessionManager class to handle session lifecycle operations and provide detailed README content explaining continuous authorization behavior.

  • Introduces SessionManager with thread-safe session operations and dedicated Session struct
  • Updates the UconEnforcer to use the new session management system
  • Adds comprehensive README documentation with usage examples and continuous authorization guidelines

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
session.go New file implementing Session struct and SessionManager with thread-safe operations
ucon_enforcer.go Refactored to use SessionManager instead of direct map operations
ucon_enforcer_interface.go Updated EnforceWithSession return type from bool to *Session
ucon_enforcer_test.go Updated tests to work with new Session API methods
ucon_enforcer_b_test.go Added placeholder benchmark file
README.md Added comprehensive documentation with usage examples and behavior explanations
Comments suppressed due to low confidence (1)

ucon_enforcer_test.go:1

  • The comment 'reused' should be 'refused' to match the context of session denial.
// Copyright 2025 The casbin Authors. All Rights Reserved.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

hsluoyz and others added 4 commits September 14, 2025 15:25
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@hsluoyz hsluoyz requested a review from Copilot September 14, 2025 07:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

bugoutianzhen123 and others added 2 commits September 15, 2025 10:22
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@hsluoyz hsluoyz requested a review from Copilot September 15, 2025 15:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if session.Active {
t.Error("Session should be inactive after revocation")
if session != nil {
t.Fatalf("session revocation failed: %v", err)
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

The error variable 'err' is being used in the error message but 'err' could be nil at this point. The test should use a more appropriate message like 'Expected session to be nil after revocation, but got non-nil session'.

Suggested change
t.Fatalf("session revocation failed: %v", err)
t.Fatalf("Expected session to be nil after revocation, but got non-nil session")

Copilot uses AI. Check for mistakes.
Comment on lines +235 to 239
session, err := uconE.EnforceWithSession(sessionID)
if session == nil {
// refused
t.Fatalf("Failed to enforce: %v", err)
}
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

When session is nil, err might also be nil (based on the EnforceWithSession implementation), which would result in 'Failed to enforce: '. The error message should handle this case properly or check err separately.

Copilot uses AI. Check for mistakes.
session, err := uconE.EnforceWithSession(sessionID)
if session == nil {
// refused
t.Fatalf("Failed to enforce: %v", err)
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

Same issue as Comment 3 - when session is nil, err might also be nil, leading to an unhelpful error message. The error handling should be improved to provide meaningful feedback.

Suggested change
t.Fatalf("Failed to enforce: %v", err)
if err == nil {
t.Fatalf("Failed to enforce: session and error are both nil, unexpected state")
} else {
t.Fatalf("Failed to enforce: %v", err)
}

Copilot uses AI. Check for mistakes.
@hsluoyz hsluoyz merged commit de2a1e2 into apache:master Sep 15, 2025
10 checks passed
@github-actions
Copy link

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] support UCON model in Casbin as an extended lib

3 participants