Skip to content

feat: fixed SavePolicy failing when the new policy is empty#74

Open
GeminiZA wants to merge 1 commit intoapache:masterfrom
GeminiZA:master
Open

feat: fixed SavePolicy failing when the new policy is empty#74
GeminiZA wants to merge 1 commit intoapache:masterfrom
GeminiZA:master

Conversation

@GeminiZA
Copy link
Copy Markdown

When saving a policy after removing all lines a.mongo.InsertMany would fail with invalid documents: must provide at least one element in input slice. Added a guard clause to avoid this.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 82.474% (-0.3%) from 82.744%
when pulling 155c68d on GeminiZA:master
into 06d09e5 on casbin:master.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Tristan Theron seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown

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

Fixes a MongoDB driver error when persisting policies after all policy lines have been removed, by avoiding an InsertMany call with an empty input slice.

Changes:

  • Add a guard in SavePolicy to return early when there are no policy lines to insert.
  • Minor refactors/formatting: group default constants into a const (...) block and reformat loadPolicyLine slice construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines 354 to +360
ctx, cancel := context.WithTimeout(context.TODO(), a.timeout)
defer cancel()

if len(lines) < 1 {
return nil
}

Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The empty-lines guard can be checked before creating the timeout context to avoid allocating a context/defer for a no-op insert path (dropTable has already completed). This also keeps the function’s control flow a bit clearer.

Suggested change
ctx, cancel := context.WithTimeout(context.TODO(), a.timeout)
defer cancel()
if len(lines) < 1 {
return nil
}
if len(lines) < 1 {
return nil
}
ctx, cancel := context.WithTimeout(context.TODO(), a.timeout)
defer cancel()

Copilot uses AI. Check for mistakes.
Comment on lines +357 to +359
if len(lines) < 1 {
return nil
}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

Add a regression test covering the new behavior where saving an empty model results in no InsertMany call (i.e., SavePolicy succeeds and the collection is left empty after dropTable). The current test suite exercises SavePolicy in other scenarios, but doesn’t assert this empty-policy case that previously triggered the driver error.

Copilot uses AI. Check for mistakes.
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.

4 participants