Database grows unexpectedly using EFCore adapter

[thoraj]

In our solution we're noticing that the Casbin table grows "unexpectedly". Before I start producing a simpler repro I thought I should check if my expectations make sense.

I would expect there not to be any duplicate rows in the table, and that the rows correspond to individual AddXXC calls. But when we're adding entries using either enforcer.AddNamedPolicyAsync() or enforcer.AddRoleForUserAsync() we're seeing that a single call to enforcer.AddXXX() produces duplicate (rows that differ only in the PK).

Is this to be expected?

Not sure if it's relevant but we're using a custom CasbinDbContext, and a Postgres database with the adapter.

[hsluoyz]

@sagilio

[sagilio]

It may be a bug, casbin should keep no duplicate rules in the model.
I will check about it and wait for the simpler repo.

[thoraj]

While working on the repro I discovered what the issue is/was. I was calling enforcer.SavePolicyAsync() while AutoSave was enabled.

So the issue was really a couple of things:

1. Assumption that save SavePolicy() would make the database correspond to the rules in the enforcer. Instead it seems to simply create a new row for each rule in the enforcer, which leads to duplicates.
2. AutoSave is enabled by default.

Not sure if item 1. is by design? If so it makes for a very crude mechanism to it hard to keep the database in sync with the enforcer. Also it means that calling SavePolicy with AutoSave enabled is always (?) a mistake.

So perhaps this is not considered a bug, and the ticket can be closed, but I would like your comment on:

• Is the SavePolicy() supposed to avoid creating duplicates, or does it work as intended?
• Are there plans to improve the api/interface to allow more flexibility in how the backend database is kept in sync with the enforcer?

[sagilio]

Thank you for the feedback.

• I think we should not add duplicates to model and storage (like DB or others), and we also need to ensure the idempotence of repeated operations.
• I plan to improve the IAdapter on v2.x, I hope it can use a more simple way to test whether the need to sync the policy and provide more flexibility on sync workflow. Do you have any suggestions?

[thoraj]

Thanks @sagilio
IMHO I think getting duplicates on enforcer.SavePolicy() needs to be fixed. Otherwise the documented usage will always produce duplicates. I think the only scenario which will not produce duplicates is the case where the database is empty to start with.

So in the short term I think adding "de-duplication' on SavePolicy() is necessary and will make SavePolicy() behave closer to the expected . (How does it behave in other implementations on other platforms?)

As for other improvement for IAdapter v2.x, I don't have anything concrete right now, but I believe the lack of rule identity may cause some friction. In the next few days we will be implementing a couple of synchronization scenarios, and I will let you know if we discover any "friction" implementing those. Perhaps we will discover if/how IAdapter can be improved?

[sagilio]

Look forward to your discoveries and welcome to comment at #184 anytime.