Fix RemoveFilteredPolicy

README.md

@@ -45,6 +45,14 @@ func main() {
 }
 ```
 
+## Run all tests
+
+    docker-compose run --rm go
+
+## Debug tests
+
+    docker-compose run --rm go dlv test github.com/pckhoi/casbin-pg-adapter
+
 ## Getting Help
 
 -   [Casbin](https://github.com/casbin/casbin)

adapter.go

@@ -32,9 +32,6 @@ type Adapter struct {
 	db *pg.DB
 }
 
-// finalizer is the destructor for Adapter.
-func finalizer(a *Adapter) {}
-
 // NewAdapter is the constructor for Adapter.
 // arg should be a PostgreS URL string or of type *pg.Options
 // The adapter will create a DB named "casbin" if it doesn't exist
@@ -149,6 +146,13 @@ func (a *Adapter) LoadPolicy(model model.Model) error {
 	return nil
 }
 
+func policyID(ptype string, rule []string) string {
+	data := strings.Join(append([]string{ptype}, rule...), ",")
+	sum := make([]byte, 64)
+	sha3.ShakeSum128(sum, []byte(data))
+	return fmt.Sprintf("%x", sum)
+}
+
 func savePolicyLine(ptype string, rule []string) *CasbinRule {
 	line := &CasbinRule{PType: ptype}
 
@@ -172,10 +176,7 @@ func savePolicyLine(ptype string, rule []string) *CasbinRule {
 		line.V5 = rule[5]
 	}
 
-	data := strings.Join(append([]string{ptype}, rule...), ",")
-	sum := make([]byte, 64)
-	sha3.ShakeSum128(sum, []byte(data))
-	line.ID = fmt.Sprintf("%x", sum)
+	line.ID = policyID(ptype, rule)
 
 	return line
 }
@@ -222,28 +223,28 @@ func (a *Adapter) RemovePolicy(sec string, ptype string, rule []string) error {
 
 // RemoveFilteredPolicy removes policy rules that match the filter from the storage.
 func (a *Adapter) RemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) error {
-	line := &CasbinRule{PType: ptype}
+	query := a.db.Model((*CasbinRule)(nil)).Where("p_type = ?", ptype)
 
 	idx := fieldIndex + len(fieldValues)
-	if fieldIndex <= 0 && idx > 0 {
-		line.V0 = fieldValues[0-fieldIndex]
+	if fieldIndex <= 0 && idx > 0 && fieldValues[0-fieldIndex] != "" {
+		query = query.Where("v0 = ?", fieldValues[0-fieldIndex])
 	}
-	if fieldIndex <= 1 && idx > 1 {
-		line.V1 = fieldValues[1-fieldIndex]
+	if fieldIndex <= 1 && idx > 1 && fieldValues[1-fieldIndex] != "" {
+		query = query.Where("v1 = ?", fieldValues[1-fieldIndex])
 	}
-	if fieldIndex <= 2 && idx > 2 {
-		line.V2 = fieldValues[2-fieldIndex]
+	if fieldIndex <= 2 && idx > 2 && fieldValues[2-fieldIndex] != "" {
+		query = query.Where("v2 = ?", fieldValues[2-fieldIndex])
 	}
-	if fieldIndex <= 3 && idx > 3 {
-		line.V3 = fieldValues[3-fieldIndex]
+	if fieldIndex <= 3 && idx > 3 && fieldValues[3-fieldIndex] != "" {
+		query = query.Where("v3 = ?", fieldValues[3-fieldIndex])
 	}
-	if fieldIndex <= 4 && idx > 4 {
-		line.V4 = fieldValues[4-fieldIndex]
+	if fieldIndex <= 4 && idx > 4 && fieldValues[4-fieldIndex] != "" {
+		query = query.Where("v4 = ?", fieldValues[4-fieldIndex])
 	}
-	if fieldIndex <= 5 && idx > 5 {
-		line.V5 = fieldValues[5-fieldIndex]
+	if fieldIndex <= 5 && idx > 5 && fieldValues[5-fieldIndex] != "" {
+		query = query.Where("v5 = ?", fieldValues[5-fieldIndex])
 	}
 
-	err := a.db.Delete(line)
+	_, err := query.Delete()
 	return err
 }

adapter_test.go

@@ -18,6 +18,7 @@ type AdapterTestSuite struct {
 }
 
 func (s *AdapterTestSuite) testGetPolicy(res [][]string) {
+	s.T().Helper()
 	myRes := s.e.GetPolicy()
 	s.Assert().True(util.Array2DEquals(res, myRes), "Policy Got: %v, supposed to be %v", myRes, res)
 }
@@ -27,8 +28,7 @@ func (s *AdapterTestSuite) dropCasbinDB() {
 	s.Require().NoError(err)
 	db := pg.Connect(opts)
 	defer db.Close()
-	_, err = db.Exec("DROP DATABASE casbin")
-	s.Require().NoError(err)
+	db.Exec("DROP DATABASE casbin")
 }
 
 func (s *AdapterTestSuite) SetupTest() {
@@ -41,9 +41,9 @@ func (s *AdapterTestSuite) SetupTest() {
 	// This is a trick to save the current policy to the DB.
 	// We can't call e.SavePolicy() because the adapter in the enforcer is still the file adapter.
 	// The current policy means the policy in the Casbin enforcer (aka in memory).
-	s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+	e, err := casbin.NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
 	s.Require().NoError(err)
-	err = s.a.SavePolicy(s.e.GetModel())
+	err = s.a.SavePolicy(e.GetModel())
 	s.Require().NoError(err)
 
 	s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", s.a)
@@ -79,11 +79,59 @@ func (s *AdapterTestSuite) TestAutoSave() {
 
 	// Because AutoSave is enabled, the policy change not only affects the policy in Casbin enforcer,
 	// but also affects the policy in the storage.
-	s.e.AddPolicy("alice", "data1", "write")
+	_, err = s.e.AddPolicy("alice", "data1", "write")
+	s.Require().NoError(err)
 	// Reload the policy from the storage to see the effect.
-	s.e.LoadPolicy()
+	err = s.e.LoadPolicy()
+	s.Require().NoError(err)
 	// The policy has a new rule: {"alice", "data1", "write"}.
 	s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}, {"alice", "data1", "write"}})
+
+	// Aditional AddPolicy have no effect
+	_, err = s.e.AddPolicy("alice", "data1", "write")
+	s.Require().NoError(err)
+	err = s.e.LoadPolicy()
+	s.Require().NoError(err)
+	s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}, {"alice", "data1", "write"}})
+	s.Require().NoError(err)
+}
+
+func (s *AdapterTestSuite) TestConstructorOptions() {
+	opts, err := pg.ParseURL(os.Getenv("PG_CONN"))
+	s.Require().NoError(err)
+
+	a, err := NewAdapter(opts)
+	s.Require().NoError(err)
+	defer a.Close()
+
+	s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", a)
+	s.Require().NoError(err)
+
+	s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}})
+}
+
+func (s *AdapterTestSuite) TestRemovePolicy() {
+	_, err := s.e.RemovePolicy("alice", "data1", "read")
+	s.Require().NoError(err)
+
+	s.testGetPolicy([][]string{{"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}})
+
+	err = s.e.LoadPolicy()
+	s.Require().NoError(err)
+
+	s.testGetPolicy([][]string{{"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}})
+}
+
+func (s *AdapterTestSuite) TestRemoveFilteredPolicy() {
+	_, err := s.e.RemoveFilteredPolicy(0, "", "data2")
+	s.Require().NoError(err)
+
+	s.testGetPolicy([][]string{{"alice", "data1", "read"}})
+
+	err = s.e.LoadPolicy()
+	s.Require().NoError(err)
+
+	s.testGetPolicy([][]string{{"alice", "data1", "read"}})
 }
 
 func TestAdapterTestSuite(t *testing.T) {

docker-compose.yml

@@ -2,7 +2,8 @@
 version: "3.4"
 services:
   postgres:
-    image: postgres:12
+    build:
+      context: ./postgres
     ports:
       - 5432:5432
     environment:

postgres/Dockerfile

@@ -0,0 +1,3 @@
+FROM postgres:12-alpine
+
+ADD . /docker-entrypoint-initdb.d

postgres/enable_log_statement.sh

@@ -0,0 +1,5 @@
+#!/bin/sh
+set -x
+
+PGCONF=/var/lib/postgresql/data/postgresql.conf
+sed -i "s/#log_statement = 'none'.*/log_statement = 'all'/g" $PGCONF