casbin-cpp icon indicating copy to clipboard operation
casbin-cpp copied to clipboard

fix: add KeyGet for built_in_functions

Open cs1137195420 opened this issue 3 years ago • 10 comments

part of: #209 add KeyGet(), KeyGet2() and KeyGet3() for built_in_functions

cs1137195420 avatar Jul 30 '22 07:07 cs1137195420

@EmperorYP7 @sheny1xuan please review

casbin-bot avatar Jul 30 '22 07:07 casbin-bot

@cs1137195420 plz also port all related tests from Go to CPP

hsluoyz avatar Jul 30 '22 07:07 hsluoyz

@JalinWang plz review

hsluoyz avatar Jul 30 '22 07:07 hsluoyz

There are too many if-else statements, making the code hard to understand. Would it be better if you add some comments to illustrate what the case the code is dealing with?

JalinWang avatar Aug 01 '22 13:08 JalinWang

@hsluoyz @JalinWang I tested the functions in built_in_functions with Casbin's test cases, and found that some of the original keymatch functions still failed in some examples. I will implement these error functions with regular matching later, and upload them together with the test case. For example, KeyMatch3 will return false for the regular expression /proxy/{id}/*/{res} and the string /proxy/myid/res/res2/res3.

cs1137195420 avatar Aug 01 '22 14:08 cs1137195420

@cs1137195420 a good practice is using exactly the same logic as Golang, just port Go code to C++.

hsluoyz avatar Aug 01 '22 15:08 hsluoyz

@cs1137195420 a good practice is using exactly the same logic as Golang, just port Go code to C++.

I agree with you. That is exactly what I am going to do next.

cs1137195420 avatar Aug 01 '22 15:08 cs1137195420

@cs1137195420 fix all errors:

image

hsluoyz avatar Aug 08 '22 02:08 hsluoyz

@JalinWang @sheny1xuan @EmperorYP7 plz review

hsluoyz avatar Aug 10 '22 15:08 hsluoyz

@sheny1xuan

hsluoyz avatar Aug 11 '22 15:08 hsluoyz

@cs1137195420 fix:

image

hsluoyz avatar Aug 14 '22 02:08 hsluoyz

:tada: This PR is included in version 1.50.4 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Aug 14 '22 16:08 github-actions[bot]