Behaviour of NewAdapterByDB() has a breaking change
d1ss0nanz opened this issue · 17 comments
Hi,
Just a heads up for other users:
I've updated from gorm-adapter 3.0.3 to 3.2.0 today and found that NewAdapterByDB() is now using "casbin_rule" as table name vs. "casbin_rules" in 3.0.3.
Therefore the user group memberships where missing in the new "casbin_rule" table.
The reason is probably (didn't verify) the use of the Gorm Migrator in 3.0.3:
return a.db.Migrator().CreateTable(a.getTableInstance())
In Gorm "CasbinRule" will result in a table name "casbin_rules".
This fixed if for me:
insert into casbin_rule(ptype,v0,v1) select p_type,v0,v1 from casbin_rules where p_type = 'g';
@d1ss0nanz I think casbin_rule has always been the rule name? Even before v3.0.3, we have:
Yes, but that's not the cause. I've checked my old logs and my database initialisation scripts.
It was "casbin_rules" in 3.0.3.
Please see my note about the use of the Migrator.
The reason is probably (didn't verify) the use of the Gorm Migrator in 3.0.3:
return a.db.Migrator().CreateTable(a.getTableInstance())
In Gorm "CasbinRule" will result in a table name "casbin_rules".
@closetool is it possible to fix this bug?
In v3.0.3 NewAdapterByDB will create table named casbin_rules, because of this
And NewAdapter will create table named casbin_rules
In v3.2.0 they both create table named casbin_rule,
I think it works fine now, nothing needed to change.
@d1ss0nanz What can I do for you?
@closetool if someone is running our migration code (the following line):
return a.db.Migrator().CreateTable(a.getTableInstance())Will they end up with the wrong table name: casbin_rules ?
@hsluoyz In v3.2.0, there will be no casbin_rules default, in spite of that, I changed the CasbinRule's name mapping
Ok, so up to at least 3.0.3, in some code paths, the created table is named "casbin_rules".
NewAdapterByDB() is behaving this way in 3.0.3.
Now, when updating to a more recent version, gorm-adapter is creating a new table "casbin_rule" besides the fact that there already exists a table "casbin_rules".
All authorization attempts silently fail, since the group membership data is in the old table.
I would suggest that you document the breaking change, so one doesn't have to understand how Gorm derives table names from struct names, to understand what's going on.
Optionally, there should be migration code that checks for a table "casbin_rules" and moves the data over when creating "casbin_rule".
@hsluoyz @closetool Hello, I think it was a new bug with new version gorm.
We use the scopes function to realize the dynamic table name. like this
a.db = db.Scopes(a.casbinRuleTable()).Session(&gorm.Session{})
err := a.createTable()It was successful work at least version 1.20.8.
But now, even if we use this version too, new users also will use new version gorm, which make us have to use new version to run. I'm already asking the author this bug at here. I think it's necessary to remind users that not use the latest gorm version in their projects on readme.md before the author has a new reply.
@closetool But I get the same error, why?
If I update the version of gorm on go.mod, it was work. I think it was a bug of gorm rather than ours
@00LT00 For me this was caused by using NewAdapterByDB(db) and from looking at the code I assume that NewAdapterByDBUseTableName(db, "", "casbin_rules") should do the trick.
But I've decided to just migrate the data into "casbin_rule" and go with the default.
@00LT00 For me this was caused by using
NewAdapterByDB(db)and from looking at the code I assume thatNewAdapterByDBUseTableName(db, "", "casbin_rules")should do the trick.But I've decided to just migrate the data into "casbin_rule" and go with the default.
It just fix your error, but the bug is there.
@closetool But it just fix the defalt table names. If user use dynamic table name, and the table is not exist. Then the table will not be create.
So I still suggest we should remind users not to update version of gorm more than 1.20.12.
@00LT00 thanks! I saw Jinzhu has fixed that bug. So updating to Gorm master HEAD will fix the issue.
@d1ss0nanz Bug fixed in: https://github.com/casbin/gorm-adapter/releases/tag/v3.2.2