Allow empty string policy field

[vzts]

Hello, I was giving some update to SQLAlchemy adapter, but I found out that there's no set standard on handling the empty string policy field.

I read #171 (comment) and the original author @hsluoyz doesn't seem to have a strong opinion on whether to allow empty string or not.

However, I think we should whether 1) do not allow empty string in the first place when writing rules, or 2) explicitly separate empty string and null in the persistence. This is because if we allow empty string on rules, we cannot differentiate whether it's a valid rule element when we do the loading.

Consider a following rule example:

'p', 'x', '', 'y', 'z'

Above can be persisted in several ways as below:

| v0 | v1   | v2 | v3 | v4   | v5   |
|----|------|----|----|------|------|
| x  |      | y  | z  | null | null |
| x  | null | y  | z  | null | null |
| x  |      | y  | z  |      |      |

The first option is okay, we can differentiate between empty and nil, so that when loading we can decide that it's 'x', '', 'y', 'z'.
The second and third option is not okay, because we cannot differentiate between the two, which lead to a situation where we can't decide it should whether be 'x', '', 'y', 'z' or 'x', '', 'y', 'z', '', '', or even 'x'.. when loading.

If you look at current implementation of one most famous orm adapter(xorm-adapter in Go), this behavior is actually giving a false result.
Here, it sets the field as nonnull, and give empty string '' by default.
Here, it does not forbid an empty string input.
However, here when loading, it ignores the value if it's an empty string.
So if we've ever inputted 'x', '', 'y', 'z', it will save in db with third option illustrated above, and be loaded as 'x', 'y', 'z', which is wrong.

So it might seem trivial, and ever question whoever will input an empty string field, it's important for the adapter library to have a standard behavior so that all adapters can be compatible (e.g. when switching from lang A to lang B(change of adapter) while staying with same DB, it should just work).

Also, in my case it happened to me, when I was working on 'RBAC with domains' model, where I was tempted to put empty string '' on tenant of a 'root' role. So I wish there would be a standard behavior on handling empty string.

I think @hsluoyz you can make a judgement call. What do you think?
In my opinion, it's simpler to just forbid the use of empty string field on the policy, raising invalid argument error when input with empty string comes in.

[leeqvip]

This is determined by the go language itself. It can distinguish between null and "" in other languages, such as Java, Python, and PHP.

[vzts]

@techoner What do you mean? You mean golang cannot distinguish null and "", or other languages? I believe all major languages should be able to do that.

[leeqvip]

I mean the string type cannot be nil in GoLang.

[hsluoyz]

@vzts thanks for bringing this issue on the surface.

I think we should allow empty string "" in Casbin rule elements. And we should use the 1st persistent way:

v0	v1	v2	v3	v4	v5
x		y	z	null	null

If any adapters handle this wrong, we should fix them.

[leeqvip]

If so, most of our adapters need to be fixed.

[vzts]

@techoner Thanks for the clarification. I don't know much about golang. In that case, I think there should be nullable string mapping in the orm for null field, to check null.

@hsluoyz Thank you. That is very clear. I can fix the sqlalchemy adapter if everyone agrees. @techoner what do you think would be the proper solution on empty string? Do you agree with @hsluoyz ?

[leeqvip]

I agree with the solution proposed by @hsluoyz , We should allow empty string "" and filter out null .
You can fix it in the sqlalchemy adapter.

[vzts]

Sounds good and will fix accordingly!