fix bug on cache delete (go-xorm#703)

* fix bug on cache delete

* fix cache issues and improve the tests

cache_lru.go

@@ -15,13 +15,12 @@ import (
 
 // LRUCacher implments cache object facilities
 type LRUCacher struct {
-	idList   *list.List
-	sqlList  *list.List
-	idIndex  map[string]map[string]*list.Element
-	sqlIndex map[string]map[string]*list.Element
-	store    core.CacheStore
-	mutex    sync.Mutex
-	// maxSize    int
+	idList         *list.List
+	sqlList        *list.List
+	idIndex        map[string]map[string]*list.Element
+	sqlIndex       map[string]map[string]*list.Element
+	store          core.CacheStore
+	mutex          sync.Mutex
 	MaxElementSize int
 	Expired        time.Duration
 	GcInterval     time.Duration
@@ -54,8 +53,6 @@ func (m *LRUCacher) RunGC() {
 
 // GC check ids lit and sql list to remove all element expired
 func (m *LRUCacher) GC() {
-	//fmt.Println("begin gc ...")
-	//defer fmt.Println("end gc ...")
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 	var removedNum int
@@ -64,12 +61,10 @@ func (m *LRUCacher) GC() {
 			time.Now().Sub(e.Value.(*idNode).lastVisit) > m.Expired {
 			removedNum++
 			next := e.Next()
-			//fmt.Println("removing ...", e.Value)
 			node := e.Value.(*idNode)
 			m.delBean(node.tbName, node.id)
 			e = next
 		} else {
-			//fmt.Printf("removing %d cache nodes ..., left %d\n", removedNum, m.idList.Len())
 			break
 		}
 	}
@@ -80,12 +75,10 @@ func (m *LRUCacher) GC() {
 			time.Now().Sub(e.Value.(*sqlNode).lastVisit) > m.Expired {
 			removedNum++
 			next := e.Next()
-			//fmt.Println("removing ...", e.Value)
 			node := e.Value.(*sqlNode)
 			m.delIds(node.tbName, node.sql)
 			e = next
 		} else {
-			//fmt.Printf("removing %d cache nodes ..., left %d\n", removedNum, m.sqlList.Len())
 			break
 		}
 	}
@@ -116,7 +109,6 @@ func (m *LRUCacher) GetIds(tableName, sql string) interface{} {
 	}
 
 	m.delIds(tableName, sql)
-
 	return nil
 }
 
@@ -134,7 +126,6 @@ func (m *LRUCacher) GetBean(tableName string, id string) interface{} {
 			// if expired, remove the node and return nil
 			if time.Now().Sub(lastTime) > m.Expired {
 				m.delBean(tableName, id)
-				//m.clearIds(tableName)
 				return nil
 			}
 			m.idList.MoveToBack(el)
@@ -148,7 +139,6 @@ func (m *LRUCacher) GetBean(tableName string, id string) interface{} {
 
 	// store bean is not exist, then remove memory's index
 	m.delBean(tableName, id)
-	//m.clearIds(tableName)
 	return nil
 }
 
@@ -166,8 +156,8 @@ func (m *LRUCacher) clearIds(tableName string) {
 // ClearIds clears all sql-ids mapping on table tableName from cache
 func (m *LRUCacher) ClearIds(tableName string) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	m.clearIds(tableName)
+	m.mutex.Unlock()
 }
 
 func (m *LRUCacher) clearBeans(tableName string) {
@@ -184,14 +174,13 @@ func (m *LRUCacher) clearBeans(tableName string) {
 // ClearBeans clears all beans in some table
 func (m *LRUCacher) ClearBeans(tableName string) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	m.clearBeans(tableName)
+	m.mutex.Unlock()
 }
 
 // PutIds pus ids into table
 func (m *LRUCacher) PutIds(tableName, sql string, ids interface{}) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	if _, ok := m.sqlIndex[tableName]; !ok {
 		m.sqlIndex[tableName] = make(map[string]*list.Element)
 	}
@@ -207,12 +196,12 @@ func (m *LRUCacher) PutIds(tableName, sql string, ids interface{}) {
 		node := e.Value.(*sqlNode)
 		m.delIds(node.tbName, node.sql)
 	}
+	m.mutex.Unlock()
 }
 
 // PutBean puts beans into table
 func (m *LRUCacher) PutBean(tableName string, id string, obj interface{}) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	var el *list.Element
 	var ok bool
 
@@ -229,6 +218,7 @@ func (m *LRUCacher) PutBean(tableName string, id string, obj interface{}) {
 		node := e.Value.(*idNode)
 		m.delBean(node.tbName, node.id)
 	}
+	m.mutex.Unlock()
 }
 
 func (m *LRUCacher) delIds(tableName, sql string) {
@@ -244,8 +234,8 @@ func (m *LRUCacher) delIds(tableName, sql string) {
 // DelIds deletes ids
 func (m *LRUCacher) DelIds(tableName, sql string) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	m.delIds(tableName, sql)
+	m.mutex.Unlock()
 }
 
 func (m *LRUCacher) delBean(tableName string, id string) {
@@ -261,8 +251,8 @@ func (m *LRUCacher) delBean(tableName string, id string) {
 // DelBean deletes beans in some table
 func (m *LRUCacher) DelBean(tableName string, id string) {
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
 	m.delBean(tableName, id)
+	m.mutex.Unlock()
 }
 
 type idNode struct {

cache_lru_test.go

@@ -0,0 +1,52 @@
+// Copyright 2015 The Xorm Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package xorm
+
+import (
+	"testing"
+
+	"github.com/go-xorm/core"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLRUCache(t *testing.T) {
+	type CacheObject1 struct {
+		Id int64
+	}
+
+	store := NewMemoryStore()
+	cacher := NewLRUCacher(store, 10000)
+
+	tableName := "cache_object1"
+	pks := []core.PK{
+		{1},
+		{2},
+	}
+
+	for _, pk := range pks {
+		sid, err := pk.ToString()
+		assert.NoError(t, err)
+
+		cacher.PutIds(tableName, "select * from cache_object1", sid)
+		ids := cacher.GetIds(tableName, "select * from cache_object1")
+		assert.EqualValues(t, sid, ids)
+
+		cacher.ClearIds(tableName)
+		ids2 := cacher.GetIds(tableName, "select * from cache_object1")
+		assert.Nil(t, ids2)
+
+		obj2 := cacher.GetBean(tableName, sid)
+		assert.Nil(t, obj2)
+
+		var obj = new(CacheObject1)
+		cacher.PutBean(tableName, sid, obj)
+		obj3 := cacher.GetBean(tableName, sid)
+		assert.EqualValues(t, obj, obj3)
+
+		cacher.DelBean(tableName, sid)
+		obj4 := cacher.GetBean(tableName, sid)
+		assert.Nil(t, obj4)
+	}
+}

cache_memory_store_test.go

@@ -0,0 +1,37 @@
+// Copyright 2015 The Xorm Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package xorm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMemoryStore(t *testing.T) {
+	store := NewMemoryStore()
+	var kvs = map[string]interface{}{
+		"a": "b",
+	}
+	for k, v := range kvs {
+		assert.NoError(t, store.Put(k, v))
+	}
+
+	for k, v := range kvs {
+		val, err := store.Get(k)
+		assert.NoError(t, err)
+		assert.EqualValues(t, v, val)
+	}
+
+	for k, _ := range kvs {
+		err := store.Del(k)
+		assert.NoError(t, err)
+	}
+
+	for k, _ := range kvs {
+		_, err := store.Get(k)
+		assert.EqualValues(t, ErrNotExist, err)
+	}
+}

cache_test.go

@@ -20,6 +20,7 @@ func TestCacheFind(t *testing.T) {
 		Password string
 	}
 
+	oldCacher := testEngine.Cacher
 	cacher := NewLRUCacher2(NewMemoryStore(), time.Hour, 10000)
 	testEngine.SetDefaultCacher(cacher)
 
@@ -58,7 +59,7 @@ func TestCacheFind(t *testing.T) {
 		assert.Equal(t, inserts[i].Password, box.Password)
 	}
 
-	testEngine.SetDefaultCacher(nil)
+	testEngine.SetDefaultCacher(oldCacher)
 }
 
 func TestCacheFind2(t *testing.T) {
@@ -70,6 +71,7 @@ func TestCacheFind2(t *testing.T) {
 		Password string
 	}
 
+	oldCacher := testEngine.Cacher
 	cacher := NewLRUCacher2(NewMemoryStore(), time.Hour, 10000)
 	testEngine.SetDefaultCacher(cacher)
 
@@ -108,7 +110,7 @@ func TestCacheFind2(t *testing.T) {
 		assert.Equal(t, inserts[i].Password, box.Password)
 	}
 
-	testEngine.SetDefaultCacher(nil)
+	testEngine.SetDefaultCacher(oldCacher)
 }
 
 func TestCacheGet(t *testing.T) {
@@ -120,6 +122,7 @@ func TestCacheGet(t *testing.T) {
 		Password string
 	}
 
+	oldCacher := testEngine.Cacher
 	cacher := NewLRUCacher2(NewMemoryStore(), time.Hour, 10000)
 	testEngine.SetDefaultCacher(cacher)
 
@@ -148,5 +151,5 @@ func TestCacheGet(t *testing.T) {
 	assert.EqualValues(t, "user1", box2.Username)
 	assert.EqualValues(t, "pass1", box2.Password)
 
-	testEngine.SetDefaultCacher(nil)
+	testEngine.SetDefaultCacher(oldCacher)
 }

circle.yml

@@ -21,7 +21,16 @@ database:
 test:
   override:
     # './...' is a relative pattern which means all subdirectories
-    - go test -v -race -db="sqlite3::mysql::mymysql::postgres" -conn_str="./test.db::root:@/xorm_test::xorm_test/root/::dbname=xorm_test sslmode=disable" -coverprofile=coverage.txt -covermode=atomic
+    - go get -u github.com/wadey/gocovmerge;
+    - go test -v -race -db="sqlite3" -conn_str="./test.db" -coverprofile=coverage1-1.txt -covermode=atomic
+    - go test -v -race -db="sqlite3" -conn_str="./test.db" -cache=true -coverprofile=coverage1-2.txt -covermode=atomic
+    - go test -v -race -db="mysql" -conn_str="root:@/xorm_test" -coverprofile=coverage2-1.txt -covermode=atomic
+    - go test -v -race -db="mysql" -conn_str="root:@/xorm_test" -cache=true -coverprofile=coverage2-2.txt -covermode=atomic
+    - go test -v -race -db="mymysql" -conn_str="xorm_test/root/" -coverprofile=coverage3-1.txt -covermode=atomic
+    - go test -v -race -db="mymysql" -conn_str="xorm_test/root/" -cache=true -coverprofile=coverage3-2.txt -covermode=atomic
+    - go test -v -race -db="postgres" -conn_str="dbname=xorm_test sslmode=disable" -coverprofile=coverage4-1.txt -covermode=atomic
+    - go test -v -race -db="postgres" -conn_str="dbname=xorm_test sslmode=disable" -cache=true -coverprofile=coverage4-2.txt -covermode=atomic
+    - gocovmerge coverage1-1.txt coverage1-2.txt coverage2-1.txt coverage2-2.txt coverage3-1.txt coverage3-2.txt coverage4-1.txt coverage4-2.txt > coverage.txt
     - cd /home/ubuntu/.go_workspace/src/github.com/go-xorm/tests && ./sqlite3.sh
     - cd /home/ubuntu/.go_workspace/src/github.com/go-xorm/tests && ./mysql.sh
     - cd /home/ubuntu/.go_workspace/src/github.com/go-xorm/tests && ./postgres.sh

processors_test.go

@@ -42,24 +42,24 @@ func TestBefore_Get(t *testing.T) {
 func TestBefore_Find(t *testing.T) {
 	assert.NoError(t, prepareEngine())
 
-	type BeforeTable struct {
+	type BeforeTable2 struct {
 		Id   int64
 		Name string
 		Val  string `xorm:"-"`
 	}
 
-	assert.NoError(t, testEngine.Sync2(new(BeforeTable)))
+	assert.NoError(t, testEngine.Sync2(new(BeforeTable2)))
 
-	cnt, err := testEngine.Insert([]BeforeTable{
+	cnt, err := testEngine.Insert([]BeforeTable2{
 		{Name: "test1"},
 		{Name: "test2"},
 	})
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, cnt)
 
-	var be []BeforeTable
+	var be []BeforeTable2
 	err = testEngine.Before(func(bean interface{}) {
-		bean.(*BeforeTable).Val = "val"
+		bean.(*BeforeTable2).Val = "val"
 	}).Find(&be)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, len(be))

session_cols_test.go

@@ -14,14 +14,14 @@ import (
 func TestSetExpr(t *testing.T) {
 	assert.NoError(t, prepareEngine())
 
-	type User struct {
+	type UserExpr struct {
 		Id   int64
 		Show bool
 	}
 
-	assert.NoError(t, testEngine.Sync2(new(User)))
+	assert.NoError(t, testEngine.Sync2(new(UserExpr)))
 
-	cnt, err := testEngine.Insert(&User{
+	cnt, err := testEngine.Insert(&UserExpr{
 		Show: true,
 	})
 	assert.NoError(t, err)
@@ -31,7 +31,7 @@ func TestSetExpr(t *testing.T) {
 	if testEngine.dialect.DBType() == core.MSSQL {
 		not = "~"
 	}
-	cnt, err = testEngine.SetExpr("show", not+" `show`").ID(1).Update(new(User))
+	cnt, err = testEngine.SetExpr("show", not+" `show`").ID(1).Update(new(UserExpr))
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, cnt)
 }