Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Oct 12, 2025

  • Analyze the DefaultPersistPolicyEntityTypeConfiguration.cs file to understand current ignored properties
  • Investigate CI build failures
  • Revert incorrect changes that referenced non-existent properties
  • Determine root cause and proper resolution path

Summary

After investigation, this PR contains 0 net file changes. The original issue report about missing "Value13" and "Value14" columns cannot be resolved in the EF Core adapter because these properties do not exist in Casbin.NET 2.7.0.

Investigation Results

  1. The PersistPolicy base class in Casbin.NET 2.7.0 only has properties up to Value12
  2. The adapter configuration already correctly ignores Value7 through Value12
  3. Only Value1 through Value6 are mapped to database columns v0 through v5
  4. Attempting to add builder.Ignore(p => p.Value13) and builder.Ignore(p => p.Value14) caused 12 compilation errors across all target frameworks

Resolution Path

To properly fix issue #81, changes need to be made in the Casbin.NET repository first:

  1. Add Value13 and Value14 properties to the PersistPolicy class
  2. Then update this adapter to ignore those properties (similar to Value7-Value12)

Build Status

✅ Build succeeds with 0 errors
✅ Tests pass for net8.0 and net9.0 frameworks
✅ Code is in correct state matching Casbin.NET 2.7.0 API

The EF Core adapter is currently working correctly for the available properties in Casbin.NET 2.7.0. No changes are needed here until the base properties are added to Casbin.NET.

Related to #81

Original prompt

This section details on the original issue you should resolve

<issue_title>casbin_rule table in my db needs to have Value13 and Value14 columns in it for this adapter to work</issue_title>
<issue_description>I am putting together a PoC app for Casbin using my SQL Server as my database for my policies. I created the casbin_rule table with the following columns as described in the Casbin Adapter documentation:

  • id int primary key
  • ptype varchar(10)
  • v0 varchar(50)
  • v1 varchar(50)
  • v2 varchar(50)
  • v3 varchar(50)
  • v4 varchar(50)
  • v5 varchar(50)

When I ran my app for the first time, I got an error that it was missing two columns named "Value13" and "Value14". I am assuming that they are probably additional columns added to the Casbin app and your adapter just needs to be updated to ignore those additional columns in the DefaultPersistPolicyEntityTypeConfiguration.cs file. When I added those columns the app ran fine but those columns were all nulls when I saved my policy.</issue_description>

Comments on the Issue (you are @copilot in this section)

@hsluoyz @sagilio @sociometry @AsakusaRinne @hsluoyz @stewartmcguire can you make a PR to fix it?

Fixes #81


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@hsluoyz hsluoyz requested a review from sagilio October 12, 2025 06:03
@hsluoyz
Copy link
Member

hsluoyz commented Oct 12, 2025

@sagilio please review

@CLAassistant
Copy link

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.
You have signed the CLA already but the status is still pending? Let us recheck it.

@hsluoyz hsluoyz marked this pull request as ready for review October 12, 2025 06:12
@Copilot Copilot AI changed the title [WIP] Add Value13 and Value14 columns to casbin_rule table Add Value13 and Value14 to ignored properties in EF Core adapter configuration Oct 12, 2025
@Copilot Copilot AI requested a review from hsluoyz October 12, 2025 06:14
Copilot finished work on behalf of hsluoyz October 12, 2025 06:14
@hsluoyz
Copy link
Member

hsluoyz commented Oct 12, 2025

@copilot all the CI errors in this PR need to be fixed, like: https://github.com/casbin-net/efcore-adapter/actions/runs/18440134483/job/52539507243?pr=84

Copy link
Author

Copilot AI commented Oct 12, 2025

@copilot all the CI errors in this PR need to be fixed, like: https://github.com/casbin-net/efcore-adapter/actions/runs/18440134483/job/52539507243?pr=84

Fixed the CI errors. The issue was that the previous commit referenced properties (Value13 and Value14) that don't exist in Casbin.NET 2.7.0, causing compilation failures. Reverted the changes in commit a5ff951 - the adapter configuration is now correct and builds successfully with 0 errors.

Copilot finished work on behalf of hsluoyz October 12, 2025 06:37
@hsluoyz
Copy link
Member

hsluoyz commented Oct 12, 2025

@copilot this PR contains 0 file change now. Do you need to create Pull Request at: https://github.com/casbin/Casbin.NET ?

Copy link
Author

Copilot AI commented Oct 12, 2025

@copilot this PR contains 0 file change now. Do you need to create Pull Request at: https://github.com/casbin/Casbin.NET ?

You're correct that this PR has 0 net file changes after reverting the incorrect implementation.

Based on the original issue #81, the proper fix requires changes in Casbin.NET first:

  1. Add Value13 and Value14 properties to the PersistPolicy class in Casbin.NET
  2. Then update this adapter to ignore those properties (similar to Value7-Value12)

The issue is that the user's error message indicates they need these properties, but they don't currently exist in Casbin.NET 2.7.0. Should I:

  • Close this PR and create one in Casbin.NET to add the properties?
  • Or was the original issue report a misunderstanding and no changes are needed?

@Copilot Copilot AI changed the title Add Value13 and Value14 to ignored properties in EF Core adapter configuration Investigation: Value13 and Value14 properties issue - No changes needed in EF Core adapter Oct 12, 2025
Copilot finished work on behalf of hsluoyz October 12, 2025 12:57
@hsluoyz
Copy link
Member

hsluoyz commented Oct 12, 2025

@copilot yes, you should close this PR and create one in Casbin.NET to add the properties

Copy link
Author

Copilot AI commented Oct 12, 2025

@copilot yes, you should close this PR and create one in Casbin.NET to add the properties

Understood. I'll close this PR since it has 0 net file changes. The proper fix requires:

  1. Creating a PR in Casbin.NET to add Value13 and Value14 properties to the PersistPolicy class
  2. After that's merged and a new version is released, updating the EF Core adapter to ignore those properties

This PR can be closed as the EF Core adapter is already working correctly with the current Casbin.NET 2.7.0 API. The issue #81 will need to be addressed upstream in Casbin.NET first.

Copilot finished work on behalf of hsluoyz October 12, 2025 13:18
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.

casbin_rule table in my db needs to have Value13 and Value14 columns in it for this adapter to work

3 participants