code review 5768043: oauth.go: Add functionality to cache token data.

oauth.go: Add functionality to cache token data.

[manugarg1, 2012-03-07 16:38:22]

Hello adg@golang.org, adg@google.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/goauth2/

[manugarg1, 2012-03-07 16:41:31]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[adg, 2012-03-08 08:31:14]

http://codereview.appspot.com/5768043/diff/5/oauth/oauth.go
File oauth/oauth.go (right):

http://codereview.appspot.com/5768043/diff/5/oauth/oauth.go#newcode53
oauth/oauth.go:53: Get() ([]byte, error)
This should be

type Cache interface {
    Token() (*Token, error)
    PutToken(*Token) error
}

and the rest of the design should fall out from there. Not everyone will want to
store their tokens as a JSON blob.

[manugarg1, 2012-03-08 18:39:35]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[adg, 2012-03-12 02:51:29]

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go
File oauth/oauth.go (right):

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode49
oauth/oauth.go:49: // Used for token cache.
// Cache specifies the methods that implement a Token cache.

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode55
oauth/oauth.go:55: type CacheFile string
// CacheFile implements Cache. Its value is the name of the file in which the
Token is stored in JSON format.

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode60
oauth/oauth.go:60: return nil, err
s/err/fmt.Errorf("CacheFile: %v")/
in the return statements of this function and in the PutToken function

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode144
oauth/oauth.go:144: func (t *Transport) LoadFromCache() error {
These functions are overkill. Drop them.

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode196
oauth/oauth.go:196: if err == nil {
change this whole bit

if err != nil {
  return err
}
if t.TokenCache != nil {
  return t.TokenCache.PutToken(t.Token)
}
return nil

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode220
oauth/oauth.go:220: } else if err := t.LoadFromCache(); err != nil {
} // drop the else
t.Token, err = t.TokenCache.Token()
if err != nil {
  return nil, err
}

http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode249
oauth/oauth.go:249: if err == nil {
if err != nil {
    return err
}
if t.TokenCache != nil {
    return.TokenCache.PutToken(t.Token)
}
return nil

[manugarg1, 2012-03-12 05:15:55]

Thanks a lot Andrew for the review. I have done the changes as per your
suggestion. Please take a look again.

On Mon, Mar 12, 2012 at 8:21 AM, <adg@golang.org> wrote:

>
>
http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**go<http://cod...
> File oauth/oauth.go (right):
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode49<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode49>
> oauth/oauth.go:49: // Used for token cache.
> // Cache specifies the methods that implement a Token cache.
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode55<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode55>
> oauth/oauth.go:55: type CacheFile string
> // CacheFile implements Cache. Its value is the name of the file in
> which the Token is stored in JSON format.
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode60<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode60>
> oauth/oauth.go:60: return nil, err
> s/err/fmt.Errorf("CacheFile: %v")/
> in the return statements of this function and in the PutToken function
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode144<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode144>
> oauth/oauth.go:144: func (t *Transport) LoadFromCache() error {
> These functions are overkill. Drop them.
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode196<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode196>
> oauth/oauth.go:196: if err == nil {
> change this whole bit
>
> if err != nil {
>  return err
> }
> if t.TokenCache != nil {
>  return t.TokenCache.PutToken(t.Token)
> }
> return nil
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode220<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode220>
> oauth/oauth.go:220: } else if err := t.LoadFromCache(); err != nil {
> } // drop the else
> t.Token, err = t.TokenCache.Token()
> if err != nil {
>  return nil, err
> }
>
> http://codereview.appspot.com/**5768043/diff/9001/oauth/oauth.**
>
go#newcode249<http://codereview.appspot.com/5768043/diff/9001/oauth/oauth.go#newcode249>
> oauth/oauth.go:249: if err == nil {
> if err != nil {
>    return err
> }
> if t.TokenCache != nil {
>    return.TokenCache.PutToken(t.**Token)
> }
> return nil
>
>
http://codereview.appspot.com/**5768043/<http://codereview.appspot.com/5768043/>
>



-- 
Manu Garg <http://www.manugarg.com>
Page
Notes<https://chrome.google.com/webstore/detail/omjdheidbhoghpfdnndkgoelfiogjfla>-
Chrome extension to take notes for the web pages that you visit!
"Journey is the destination of life."

[adg, 2012-03-12 05:22:21]

*** Submitted as http://code.google.com/p/goauth2/source/detail?r=3af366e7c181
***

oauth.go: Add functionality to cache token data.

R=adg
CC=golang-dev
http://codereview.appspot.com/5768043

Committer: Andrew Gerrand <adg@golang.org>

[adg, 2012-03-12 05:22:52]

LGTM

[manugarg1, 2012-09-05 12:32:25]

*** Abandoned ***