suggestion: allow to query enforcer for number of expected parameters for enforce call

[DemianTinkiel]

Given the different styles of supported models and the fact that the data source might be unknown/out of our control it would be useful to be able to determine the number of parameters required for a call to `enforcer#enforce(...)#.

Or at least fail with something better than Method threw 'java.lang.ArrayIndexOutOfBoundsException' exception.

e.g.

enforcer.enforce("alice","data1","read")

would thrown an exception for

p, admin, domain1, data1, read
p, admin, domain1, data1, write
p, admin, domain2, data2, read
p, admin, domain2, data2, write

g, alice, admin, domain1
g, bob, admin, domain2

but would succeed for

p, alice, data1, read
p, bob, data2, write
p, data_group_admin, data_group, write

g, alice, data_group_admin
g2, data1, data_group
g2, data2, data_group

I thought I could guess it from enforcer.getAllActions() but in the 1st case this returns
actions:[data1, data1, data2, data2]
while in the 2nd it (correctly) returns
actions:[read, write, write]

I think this particular scenario would be solvable with enforcer.getAllDomains() but is not very scalable as a solution for future changes.

[hsluoyz]

Hi @DemianTinkiel , you can call getPolicy() to get all the policy rules, then check the length of one rule like p, admin, domain1, data1, read. length - 1 will be the number of parameters in enforce() (the first p removed).

jcasbin/src/test/java/org/casbin/jcasbin/main/EnforcerUnitTest.java

Lines 214 to 220 in 7db9d32

	@Test
	public void testReloadPolicy() {
	Enforcer e = new Enforcer("examples/rbac_model.conf", "examples/rbac_policy.csv");
	e.loadPolicy();
	testGetPolicy(e, asList(asList("alice", "data1", "read"), asList("bob", "data2", "write"), asList("data2_admin", "data2", "read"), asList("data2_admin", "data2", "write")));
	}

Of course, we can do better by checking it in enforce() and providing a better exception. Would you propose a PR along this way?

[DemianTinkiel]

I see. Sure I'll submit a pr tomorrow (dev laptop currently unavailable).

However in the case of getAllActions() should we also not check policy and shift answer accordingly?

[DemianTinkiel]

@hsluoyz I'm creating the PR now but looking at org.casbin.jcasbin.model.Policy#getPolicy I wonder if is best not to check against model instead of policy. My reasoning is that you can just load the model as in org.casbin.jcasbin.main.EnforcerUnitTest#testRoleLinks and you should be able to do the check anyway. What do you think?

[hsluoyz]

I didn't quite get your idea. Can be be more specific?

[DemianTinkiel]

update: reading the Syntax for Models, it sounds like request_definition is the one I should check against

lets say I do

Enforcer e = new Enforcer("examples/rbac_model.conf");
e.enforce("user501", "data9"); //Not enough params throw meaningful exception

then getPolicy().size()==0 instead I should do something like

getModel().model.get("p").get("p").tokens.length;

[DemianTinkiel]

@hsluoyz I submitted a PR perhaps that would make it more clear as to what I mean.

[hsluoyz]

Yes. You can do it with getModel().model.get("p").get("p").tokens.length;. It also works. I have responded to your PR.

[DemianTinkiel]

Yes. You can do it with getModel().model.get("p").get("p").tokens.length;

what I see is that the check should be done against request_definition so it should be more like getModel().model.get("r").get("r").tokens.length

[hsluoyz]

I think you are right. You should check r instead of p.

[DemianTinkiel]

I've updated the PR #34
Note File CoreEnforcer.java has 251 lines of code (exceeds 250 allowed). Not sure what to do about that.

What is your release cycle? ie. when can I use a new version in maven that has these changes?

[hsluoyz]

Note File CoreEnforcer.java has 251 lines of code (exceeds 250 allowed). Not sure what to do about that.

codeclimate is not a hard requirement. Never mind.

What is your release cycle? ie. when can I use a new version in maven that has these changes?

I will make a release right after your PRs are finished.