Repository: casbin/casbin
Pull request #241 (2019-06-14 20:09:42)

Added errors in rbac_api.go
#### What wasn't there

* An issue referenced [here](https://github.com/casbin/casbin/issues/236#issue-451381779) stated that rbac_api.go does not return errors. This is not ideal.
* vendor wasnt in `.gitignore`

#### What has been added
- [X] Errors package
- [X] Errors in rbac_api.go
- [X] Corresponding errror switch in rbac_api_test.go
- [X] vendor in .gitignore

hsluoyz: Hi @L04DB4L4NC3R , can you merge/rework your commits into the final state? For example, you did something wrong in commit 1, then fix it in commit 2. Then you should merge them into 1 commit in the PR.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/24015073/badge)](https://coveralls.io/builds/24015073)

Coverage decreased (-0.08%) to 80.899% when pulling **bedb0a92163fbd5fbdd89ddf44aeb2616877e94b on L04DB4L4NC3R:master** into **ccd536f4ce692763de9459f1cfb8889bf4df16cd on casbin:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/24015073/badge)](https://coveralls.io/builds/24015073)

Coverage decreased (-0.08%) to 80.899% when pulling **bedb0a92163fbd5fbdd89ddf44aeb2616877e94b on L04DB4L4NC3R:master** into **ccd536f4ce692763de9459f1cfb8889bf4df16cd on casbin:master**.


Patch:
diff --git a/.gitignore b/.gitignore
index 72ed6f4..da27805 100644
--- a/.gitignore
+++ b/.gitignore
@@ -24,4 +24,7 @@ _testmain.go
 *.prof
 
 .idea/
-*.iml
\ No newline at end of file
+*.iml
+
+# vendor files
+vendor
diff --git a/.travis.yml b/.travis.yml
index 39f9bd3..be7f634 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,14 +1,12 @@
-language: go
-
-sudo: false
-
-go:
-  - "1.10"
-  - "1.11"
-  - "1.12"
-
-before_install:
-  - go get github.com/mattn/goveralls
-
-script:
+language: go
+
+sudo: false
+
+go:
+  - tip
+
+before_install:
+  - go get github.com/mattn/goveralls
+
+script:
   - $HOME/gopath/bin/goveralls -service=travis-ci
\ No newline at end of file
diff --git a/README.md b/README.md
index ac19a00..b4ae31c 100644
--- a/README.md
+++ b/README.md
@@ -7,6 +7,7 @@ Casbin
 [![Godoc](https://godoc.org/github.com/casbin/casbin?status.svg)](https://godoc.org/github.com/casbin/casbin)
 [![Release](https://img.shields.io/github/release/casbin/casbin.svg)](https://github.com/casbin/casbin/releases/latest)
 [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/casbin/lobby)
+[![Patreon](https://img.shields.io/badge/patreon-donate-yellow.svg)](http://www.patreon.com/yangluo)
 
 **News**: still worry about how to write the correct Casbin policy? ``Casbin online editor`` is coming to help! Try it at: http://casbin.org/editor/
 
diff --git a/errors/rbac_errors.go b/errors/rbac_errors.go
new file mode 100644
index 0000000..68b1450
--- /dev/null
+++ b/errors/rbac_errors.go
@@ -0,0 +1,24 @@
+// Copyright 2018 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package errors
+
+import "errors"
+
+// Global errors for rbac defined here
+var (
+	ERR_NAME_NOT_FOUND    = errors.New("error: name does not exist")
+	ERR_DOMAIN_PARAMETER  = errors.New("error: domain should be 1 parameter")
+	ERR_NAMES12_NOT_FOUND = errors.New("error: name1 or name2 does not exist")
+)
diff --git a/examples/rbac_policy.csv b/examples/rbac_policy.csv
index 9bbfa7c..f93d6df 100644
--- a/examples/rbac_policy.csv
+++ b/examples/rbac_policy.csv
@@ -2,5 +2,4 @@ p, alice, data1, read
 p, bob, data2, write
 p, data2_admin, data2, read
 p, data2_admin, data2, write
-
 g, alice, data2_admin
\ No newline at end of file
diff --git a/examples/rbac_with_domains_policy.csv b/examples/rbac_with_domains_policy.csv
index 8810a7c..8558d17 100644
--- a/examples/rbac_with_domains_policy.csv
+++ b/examples/rbac_with_domains_policy.csv
@@ -2,6 +2,5 @@ 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
\ No newline at end of file
diff --git a/rbac/default-role-manager/role_manager.go b/rbac/default-role-manager/role_manager.go
index 5b7a155..d333cb6 100644
--- a/rbac/default-role-manager/role_manager.go
+++ b/rbac/default-role-manager/role_manager.go
@@ -15,9 +15,9 @@
 package defaultrolemanager
 
 import (
-	"errors"
 	"sync"
 
+	"github.com/casbin/casbin/errors"
 	"github.com/casbin/casbin/log"
 	"github.com/casbin/casbin/rbac"
 )
@@ -91,7 +91,7 @@ func (rm *RoleManager) AddLink(name1 string, name2 string, domain ...string) err
 		name1 = domain[0] + "::" + name1
 		name2 = domain[0] + "::" + name2
 	} else if len(domain) > 1 {
-		return errors.New("error: domain should be 1 parameter")
+		return errors.ERR_DOMAIN_PARAMETER
 	}
 
 	role1 := rm.createRole(name1)
@@ -108,11 +108,11 @@ func (rm *RoleManager) DeleteLink(name1 string, name2 string, domain ...string)
 		name1 = domain[0] + "::" + name1
 		name2 = domain[0] + "::" + name2
 	} else if len(domain) > 1 {
-		return errors.New("error: domain should be 1 parameter")
+		return errors.ERR_DOMAIN_PARAMETER
 	}
 
 	if !rm.hasRole(name1) || !rm.hasRole(name2) {
-		return errors.New("error: name1 or name2 does not exist")
+		return errors.ERR_NAMES12_NOT_FOUND
 	}
 
 	role1 := rm.createRole(name1)
@@ -128,7 +128,7 @@ func (rm *RoleManager) HasLink(name1 string, name2 string, domain ...string) (bo
 		name1 = domain[0] + "::" + name1
 		name2 = domain[0] + "::" + name2
 	} else if len(domain) > 1 {
-		return false, errors.New("error: domain should be 1 parameter")
+		return false, errors.ERR_DOMAIN_PARAMETER
 	}
 
 	if name1 == name2 {
@@ -149,7 +149,7 @@ func (rm *RoleManager) GetRoles(name string, domain ...string) ([]string, error)
 	if len(domain) == 1 {
 		name = domain[0] + "::" + name
 	} else if len(domain) > 1 {
-		return nil, errors.New("error: domain should be 1 parameter")
+		return nil, errors.ERR_DOMAIN_PARAMETER
 	}
 
 	if !rm.hasRole(name) {
@@ -171,11 +171,11 @@ func (rm *RoleManager) GetUsers(name string, domain ...string) ([]string, error)
 	if len(domain) == 1 {
 		name = domain[0] + "::" + name
 	} else if len(domain) > 1 {
-		return nil, errors.New("error: domain should be 1 parameter")
+		return nil, errors.ERR_DOMAIN_PARAMETER
 	}
 
 	if !rm.hasRole(name) {
-		return nil, errors.New("error: name does not exist")
+		return nil, errors.ERR_NAME_NOT_FOUND
 	}
 
 	names := []string{}
diff --git a/rbac_api.go b/rbac_api.go
index fef0b63..616f958 100644
--- a/rbac_api.go
+++ b/rbac_api.go
@@ -15,21 +15,23 @@
 package casbin
 
 // GetRolesForUser gets the roles that a user has.
-func (e *Enforcer) GetRolesForUser(name string) []string {
-	res, _ := e.model["g"]["g"].RM.GetRoles(name)
-	return res
+func (e *Enforcer) GetRolesForUser(name string) ([]string, error) {
+	res, err := e.model["g"]["g"].RM.GetRoles(name)
+	return res, err
 }
 
 // GetUsersForRole gets the users that has a role.
-func (e *Enforcer) GetUsersForRole(name string) []string {
-	res, _ := e.model["g"]["g"].RM.GetUsers(name)
-	return res
+func (e *Enforcer) GetUsersForRole(name string) ([]string, error) {
+	res, err := e.model["g"]["g"].RM.GetUsers(name)
+	return res, err
 }
 
 // HasRoleForUser determines whether a user has a role.
-func (e *Enforcer) HasRoleForUser(name string, role string) bool {
-	roles := e.GetRolesForUser(name)
-
+func (e *Enforcer) HasRoleForUser(name string, role string) (bool, error) {
+	roles, err := e.GetRolesForUser(name)
+	if err != nil {
+		return false, err
+	}
 	hasRole := false
 	for _, r := range roles {
 		if r == role {
@@ -38,7 +40,7 @@ func (e *Enforcer) HasRoleForUser(name string, role string) bool {
 		}
 	}
 
-	return hasRole
+	return hasRole, nil
 }
 
 // AddRoleForUser adds a role for a user.
diff --git a/rbac_api_synced.go b/rbac_api_synced.go
index 119ce9f..c75cc96 100644
--- a/rbac_api_synced.go
+++ b/rbac_api_synced.go
@@ -15,21 +15,21 @@
 package casbin
 
 // GetRolesForUser gets the roles that a user has.
-func (e *SyncedEnforcer) GetRolesForUser(name string) []string {
+func (e *SyncedEnforcer) GetRolesForUser(name string) ([]string, error) {
 	e.m.Lock()
 	defer e.m.Unlock()
 	return e.Enforcer.GetRolesForUser(name)
 }
 
 // GetUsersForRole gets the users that has a role.
-func (e *SyncedEnforcer) GetUsersForRole(name string) []string {
+func (e *SyncedEnforcer) GetUsersForRole(name string) ([]string, error) {
 	e.m.Lock()
 	defer e.m.Unlock()
 	return e.Enforcer.GetUsersForRole(name)
 }
 
 // HasRoleForUser determines whether a user has a role.
-func (e *SyncedEnforcer) HasRoleForUser(name string, role string) bool {
+func (e *SyncedEnforcer) HasRoleForUser(name string, role string) (bool, error) {
 	e.m.Lock()
 	defer e.m.Unlock()
 	return e.Enforcer.HasRoleForUser(name, role)
diff --git a/util/builtin_operators.go b/util/builtin_operators.go
index 53011e7..45110a6 100644
--- a/util/builtin_operators.go
+++ b/util/builtin_operators.go
@@ -83,7 +83,7 @@ func KeyMatch3(key1 string, key2 string) bool {
 		key2 = re.ReplaceAllString(key2, "$1[^/]+$2")
 	}
 
-	return RegexMatch(key1, "^" + key2 + "$")
+	return RegexMatch(key1, key2)
 }
 
 // KeyMatch3Func is the wrapper for KeyMatch3.


Test patch:
diff --git a/rbac_api_test.go b/rbac_api_test.go
index 9b5f2d2..05c14f4 100644
--- a/rbac_api_test.go
+++ b/rbac_api_test.go
@@ -17,12 +17,16 @@ package casbin
 import (
 	"testing"
 
+	"github.com/casbin/casbin/errors"
 	"github.com/casbin/casbin/util"
 )
 
 func testGetRoles(t *testing.T, e *Enforcer, name string, res []string) {
 	t.Helper()
-	myRes := e.GetRolesForUser(name)
+	myRes, err := e.GetRolesForUser(name)
+	if err != nil {
+		t.Error("Roles for ", name, " could not be fetched: ", err.Error())
+	}
 	t.Log("Roles for ", name, ": ", myRes)
 
 	if !util.SetEquals(res, myRes) {
@@ -32,7 +36,15 @@ func testGetRoles(t *testing.T, e *Enforcer, name string, res []string) {
 
 func testGetUsers(t *testing.T, e *Enforcer, name string, res []string) {
 	t.Helper()
-	myRes := e.GetUsersForRole(name)
+	myRes, err := e.GetUsersForRole(name)
+	switch err {
+	case nil:
+		break
+	case errors.ERR_NAME_NOT_FOUND:
+		t.Log("No name found")
+	default:
+		t.Error("Users for ", name, " could not be fetched: ", err.Error())
+	}
 	t.Log("Users for ", name, ": ", myRes)
 
 	if !util.SetEquals(res, myRes) {
@@ -42,7 +54,10 @@ func testGetUsers(t *testing.T, e *Enforcer, name string, res []string) {
 
 func testHasRole(t *testing.T, e *Enforcer, name string, role string, res bool) {
 	t.Helper()
-	myRes := e.HasRoleForUser(name, role)
+	myRes, err := e.HasRoleForUser(name, role)
+	if err != nil {
+		t.Error("HasRoleForUser returned an error: ", err.Error())
+	}
 	t.Log(name, " has role ", role, ": ", myRes)
 
 	if res != myRes {
diff --git a/util/builtin_operators_test.go b/util/builtin_operators_test.go
index a478971..7ef164f 100644
--- a/util/builtin_operators_test.go
+++ b/util/builtin_operators_test.go
@@ -90,15 +90,14 @@ func testKeyMatch3(t *testing.T, key1 string, key2 string, res bool) {
 }
 
 func TestKeyMatch3(t *testing.T) {
-	// keyMatch3() is similar with KeyMatch2(), except using "/proxy/{id}" instead of "/proxy/:id".
 	testKeyMatch3(t, "/foo", "/foo", true)
 	testKeyMatch3(t, "/foo", "/foo*", true)
 	testKeyMatch3(t, "/foo", "/foo/*", false)
-	testKeyMatch3(t, "/foo/bar", "/foo", false)
-	testKeyMatch3(t, "/foo/bar", "/foo*", false)
+	testKeyMatch3(t, "/foo/bar", "/foo", true) // different with KeyMatch.
+	testKeyMatch3(t, "/foo/bar", "/foo*", true)
 	testKeyMatch3(t, "/foo/bar", "/foo/*", true)
-	testKeyMatch3(t, "/foobar", "/foo", false)
-	testKeyMatch3(t, "/foobar", "/foo*", false)
+	testKeyMatch3(t, "/foobar", "/foo", true) // different with KeyMatch.
+	testKeyMatch3(t, "/foobar", "/foo*", true)
 	testKeyMatch3(t, "/foobar", "/foo/*", false)
 
 	testKeyMatch3(t, "/", "/{resource}", false)