From 909f098dcd66f29e72cad321723fbc7c7f1a668c Mon Sep 17 00:00:00 2001
From: Burcu Dogan <jbd@google.com>
Date: Mon, 26 May 2014 11:53:51 +0200
Subject: [PATCH] Cache read operation should be handled during construction.

---
 cache.go      | 41 +++++++++++++++++----------
 cache_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++---------
 transport.go  |  2 +-
 3 files changed, 91 insertions(+), 30 deletions(-)

diff --git a/cache.go b/cache.go
index 5920645..b599d37 100644
--- a/cache.go
+++ b/cache.go
@@ -7,19 +7,34 @@ package oauth2
 import (
 	"encoding/json"
 	"io/ioutil"
+	"os"
 )
 
 // Cache represents a token cacher.
 type Cache interface {
-	// Read reads a cache token from the specified file.
-	Read() (token *Token)
+	// Token returns the initial token retrieved from the cache,
+	// if there is no existing token nil value is returned.
+	Token() (token *Token)
 	// Write writes a token to the specified file.
 	Write(token *Token)
 }
 
 // NewFileCache creates a new file cache.
-func NewFileCache(filename string) *FileCache {
-	return &FileCache{filename: filename}
+func NewFileCache(filename string) (cache *FileCache, err error) {
+	data, err := ioutil.ReadFile(filename)
+	if os.IsNotExist(err) {
+		// no token has cached before, skip reading
+		return &FileCache{filename: filename}, nil
+	}
+	if err != nil {
+		return
+	}
+	var token Token
+	if err = json.Unmarshal(data, &token); err != nil {
+		return
+	}
+	cache = &FileCache{filename: filename, initialToken: &token}
+	return
 }
 
 // FileCache represents a file based token cacher.
@@ -28,19 +43,15 @@ type FileCache struct {
 	// during read or write operations.
 	ErrorHandler func(error)
 
-	filename string
+	initialToken *Token
+	filename     string
 }
 
-// Read reads a cache token from the specified file.
-func (f *FileCache) Read() (token *Token) {
-	data, err := ioutil.ReadFile(f.filename)
-	if err == nil {
-		err = json.Unmarshal(data, token)
-	}
-	if f.ErrorHandler != nil {
-		f.ErrorHandler(err)
-	}
-	return
+// Token returns the initial token read from the cache. It should be used to
+// warm the authorization mechanism, token refreshes and later writes don't
+// change the returned value. If no token is cached before, returns nil.
+func (f *FileCache) Token() (token *Token) {
+	return f.initialToken
 }
 
 // Write writes a token to the specified file.
diff --git a/cache_test.go b/cache_test.go
index 0c0e351..2fd5efe 100644
--- a/cache_test.go
+++ b/cache_test.go
@@ -5,23 +5,73 @@
 package oauth2
 
 import (
-	"os"
+	"io/ioutil"
+	"path"
 	"testing"
 )
 
-func TestFileCacheErrorHandling(t *testing.T) {
-	var lastErr error
-	fileCache := NewFileCache("/path/that/doesnt/exist")
-	fileCache.ErrorHandler = func(err error) {
-		lastErr = err
+var tokenBody = `{"access_token":"abc123","token_type":"Bearer","refresh_token":"def789","expiry":"0001-01-01T00:00:00Z"}`
+
+func TestNewFileCacheNotExist(t *testing.T) {
+	cache, err := NewFileCache("/path/that/doesnt/exist")
+	if err != nil {
+		t.Fatalf("NewFileCache shouldn't return an error for if cache file doesn't exist, but returned %v", err)
 	}
-	fileCache.Read()
-	if !os.IsNotExist(lastErr) {
-		t.Fatalf("Read should have invoked the error handling func with os.ErrNotExist, but read err is %v", lastErr)
-	}
-	lastErr = nil
-	fileCache.Write(&Token{})
-	if !os.IsNotExist(lastErr) {
-		t.Fatalf("Write should have invoked the error handling func with os.ErrNotExist, but read err is %v", lastErr)
+	if cache == nil {
+		t.Fatalf("A file cache should be inited with a non existing cache file")
+	}
+}
+
+func TestNewFileCache(t *testing.T) {
+	f, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = f.WriteString(tokenBody)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cache, err := NewFileCache(f.Name())
+	if err != nil {
+		t.Fatalf("Cache should have read the file cache at %v, but recieved %v", f.Name(), err)
+	}
+	token := cache.Token()
+	if token.AccessToken != "abc123" {
+		t.Fatalf("Cached access token is %v, expected to be abc123", token.AccessToken)
+	}
+	if token.RefreshToken != "def789" {
+		t.Fatalf("Cached refresh token is %v, expected to be def789", token.RefreshToken)
+	}
+}
+
+func TestFileCacheWrite(t *testing.T) {
+	dirName, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cache, err := NewFileCache(path.Join(dirName, "cache-file"))
+	cache.ErrorHandler = func(err error) {
+		if err != nil {
+			t.Fatalf("Cache write should have been succeeded succesfully, recieved %v", err)
+		}
+	}
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cache.Write(&Token{
+		AccessToken:  "abc123",
+		TokenType:    "Bearer",
+		RefreshToken: "def789",
+	})
+
+	data, err := ioutil.ReadFile(cache.filename)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(data) != tokenBody {
+		t.Fatalf("Written token is different than the expected, %v is found", data)
 	}
 }
diff --git a/transport.go b/transport.go
index 4346770..43f0036 100644
--- a/transport.go
+++ b/transport.go
@@ -98,7 +98,7 @@ func (t *authorizedTransport) RoundTrip(req *http.Request) (resp *http.Response,
 	// Try to initialize the token from the cache.
 	if token == nil {
 		if c := t.fetcher.Cache(); c != nil {
-			token = c.Read()
+			token = c.Token()
 		}
 	}