Filecache: remove map for tracking what is cached, read from the file…

…system directly. closes go-spatial#203.

cache/filecache/filecache.go

@@ -3,10 +3,8 @@ package filecache
 import (
 	"errors"
 	"io/ioutil"
-	"log"
 	"os"
 	"path/filepath"
-	"sync"
 
 	"github.com/terranodo/tegola/cache"
 	"github.com/terranodo/tegola/util/dict"
@@ -37,9 +35,7 @@ func New(config map[string]interface{}) (cache.Interface, error) {
 	var err error
 
 	//	new filecache
-	fc := Filecache{
-		Locker: map[string]sync.RWMutex{},
-	}
+	fc := Filecache{}
 
 	//	parse the config
 	c := dict.M(config)
@@ -69,54 +65,11 @@ func New(config map[string]interface{}) (cache.Interface, error) {
 		return nil, err
 	}
 
-	//	walk our basepath and fill the filecache Locker with keys for already cached tiles
-	err = filepath.Walk(fc.Basepath, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-
-		//	skip directories
-		if info.IsDir() {
-			return nil
-		}
-
-		//	remove the basepath for the file key
-		fileKey := path[len(fc.Basepath):]
-
-		cacheKey, err := cache.ParseKey(fileKey)
-		if err != nil {
-			log.Println("filecache: ", err.Error())
-			return nil
-		}
-
-		//	write our key
-		fc.Lock()
-		fc.Locker[cacheKey.String()] = sync.RWMutex{}
-		fc.Unlock()
-
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-
 	return &fc, nil
 }
 
 type Filecache struct {
 	Basepath string
-
-	//	we need a cache mutex to avoid concurrent writes to our Locker
-	sync.RWMutex
-
-	//	Locker tracks which cache keys are being operated on.
-	//	when the cache is being written to a Lock() is set.
-	//	when being read from an RLock() is used so we don't
-	//	block concurrent reads.
-	//
-	//	TODO: store a hash of the cache blob along with the Locker mutex
-	Locker map[string]sync.RWMutex
-
 	//	MaxZoom determins the max zoom the cache to persist. Beyond this
 	//	zoom, cache Set() calls will be ignored. This is useful if the cache
 	//	should not be leveraged for higher zooms when data changes often.
@@ -129,27 +82,8 @@ type Filecache struct {
 func (fc *Filecache) Get(key *cache.Key) ([]byte, bool, error) {
 	path := filepath.Join(fc.Basepath, key.String())
 
-	//	lookup our mutex
-	fc.RLock()
-	mutex, ok := fc.Locker[key.String()]
-	fc.RUnlock()
-	if !ok {
-		//	no entry, return a miss
-		return nil, false, nil
-	}
-
-	//	read lock
-	mutex.RLock()
-	defer mutex.RUnlock()
-
 	f, err := os.Open(path)
 	if err != nil {
-		//	something is wrong with opening this file
-		//	remove the key from the cache if it exists
-		fc.Lock()
-		delete(fc.Locker, key.String())
-		fc.Unlock()
-
 		if os.IsNotExist(err) {
 			return nil, false, nil
 		}
@@ -173,29 +107,20 @@ func (fc *Filecache) Set(key *cache.Key, val []byte) error {
 		return nil
 	}
 
-	path := filepath.Join(fc.Basepath, key.String())
+	//	the tmpPath uses the destPath with a simple "-tmp" suffix. we're going to do
+	//	a Rename at the end of this method and according to the os.Rename() docs:
+	//	"If newpath already exists and is not a directory, Rename replaces it.
+	//	OS-specific restrictions may apply when oldpath and newpath are in different directories"
+	destPath := filepath.Join(fc.Basepath, key.String())
+	tmpPath := destPath + "-tmp"
 
-	//	lookup our mutex
-	fc.RLock()
-	mutex, ok := fc.Locker[key.String()]
-	fc.RUnlock()
-	if !ok {
-		fc.Lock()
-		fc.Locker[key.String()] = sync.RWMutex{}
-		mutex = fc.Locker[key.String()]
-		fc.Unlock()
-	}
 	//	the key can have a directory syntax so we need to makeAll
-	if err = os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
+	if err = os.MkdirAll(filepath.Dir(destPath), os.ModePerm); err != nil {
 		return err
 	}
 
-	//	write lock
-	mutex.Lock()
-	defer mutex.Unlock()
-
 	//	create the file
-	f, err := os.Create(path)
+	f, err := os.Create(tmpPath)
 	if err != nil {
 		return err
 	}
@@ -207,7 +132,8 @@ func (fc *Filecache) Set(key *cache.Key, val []byte) error {
 		return err
 	}
 
-	return nil
+	//	move the temp file to the destination
+	return os.Rename(tmpPath, destPath)
 }
 
 func (fc *Filecache) Purge(key *cache.Key) error {
@@ -219,9 +145,5 @@ func (fc *Filecache) Purge(key *cache.Key) error {
 	}
 
 	//	remove the locker key on purge
-	fc.Lock()
-	delete(fc.Locker, key.String())
-	fc.Unlock()
-
 	return os.Remove(path)
 }

cache/filecache/filecache_test.go

@@ -3,7 +3,6 @@ package filecache_test
 import (
 	"fmt"
 	"reflect"
-	"sync"
 	"testing"
 
 	"github.com/terranodo/tegola/cache"
@@ -24,10 +23,6 @@ func TestNew(t *testing.T) {
 			},
 			expected: &filecache.Filecache{
 				Basepath: "testfiles/tegola-cache",
-				Locker: map[string]sync.RWMutex{
-					//	our testfiles directory has a file that will be read in when calling New
-					"0/1/12": sync.RWMutex{},
-				},
 			},
 			err: nil,
 		},
@@ -38,11 +33,7 @@ func TestNew(t *testing.T) {
 			},
 			expected: &filecache.Filecache{
 				Basepath: "testfiles/tegola-cache",
-				Locker: map[string]sync.RWMutex{
-					//	our testfiles directory has a file that will be read in when calling New
-					"0/1/12": sync.RWMutex{},
-				},
-				MaxZoom: &maxZoom,
+				MaxZoom:  &maxZoom,
 			},
 			err: nil,
 		},
@@ -133,3 +124,69 @@ func TestSetGetPurge(t *testing.T) {
 		}
 	}
 }
+
+func TestSetOverwrite(t *testing.T) {
+	testcases := []struct {
+		config   map[string]interface{}
+		key      cache.Key
+		bytes1   []byte
+		bytes2   []byte
+		expected []byte
+	}{
+		{
+			config: map[string]interface{}{
+				"basepath": "testfiles/tegola-cache",
+			},
+			key: cache.Key{
+				Z: 0,
+				X: 1,
+				Y: 1,
+			},
+			bytes1:   []byte("\x66\x6f\x6f"),
+			bytes2:   []byte("\x53\x69\x6c\x61\x73"),
+			expected: []byte("\x53\x69\x6c\x61\x73"),
+		},
+	}
+
+	for i, tc := range testcases {
+		fc, err := filecache.New(tc.config)
+		if err != nil {
+			t.Errorf("testcase (%v) failed. err: %v", i, err)
+			continue
+		}
+
+		//	test write1
+		if err = fc.Set(&tc.key, tc.bytes1); err != nil {
+			t.Errorf("testcase (%v) write failed. err: %v", i, err)
+			continue
+		}
+
+		//	test write2
+		if err = fc.Set(&tc.key, tc.bytes2); err != nil {
+			t.Errorf("testcase (%v) write failed. err: %v", i, err)
+			continue
+		}
+
+		//	fetch the cache entry
+		output, hit, err := fc.Get(&tc.key)
+		if err != nil {
+			t.Errorf("testcase (%v) read failed. err: %v", i, err)
+			continue
+		}
+		if !hit {
+			t.Errorf("testcase (%v) read failed. should have been a hit but cache reported a miss", i)
+			continue
+		}
+
+		if !reflect.DeepEqual(output, tc.expected) {
+			t.Errorf("testcase (%v) failed. output (%v) does not match expected (%v)", i, output, tc.expected)
+			continue
+		}
+
+		//	clean up
+		if err = fc.Purge(&tc.key); err != nil {
+			t.Errorf("testcase (%v) failed. purge failed. err: %v", i, err)
+			continue
+		}
+	}
+}