[oauth2] all: deprecate NoContext

[Jaana Burcu Dogan (Gerrit)]

Reviewers: Brad Fitzpatrick, Chris Broadfoot

Jaana Burcu Dogan uploaded a change:
https://go-review.googlesource.com/27690

all: deprecate NoContext

There is no good reason why we suggest NoContext rather than
context.Background(). When the oauth2 library first came around, the
community was not familiar with the x/net/context package. for
documentation reasons, we decided to add NoContext to the oauth2
package. It was not a good idea even back then, and given that context
package is fairly popular, there is no good reason why we are
depending on this.

Updating all the references of NoContext with context.Background
and documenting it as deprecated.

Change-Id: I18e390f1351023a29b567777a3f963dd550cf657
---
M clientcredentials/clientcredentials_test.go
M example_test.go
M jwt/example_test.go
M jwt/jwt_test.go
M oauth2.go
M oauth2_test.go
6 files changed, 25 insertions(+), 23 deletions(-)



diff --git a/clientcredentials/clientcredentials_test.go
b/clientcredentials/clientcredentials_test.go
index 5a0170a..061b43b 100644
--- a/clientcredentials/clientcredentials_test.go
+++ b/clientcredentials/clientcredentials_test.go
@@ -5,12 +5,11 @@
package clientcredentials

import (
+ "context"
"io/ioutil"
"net/http"
"net/http/httptest"
"testing"
-
- "golang.org/x/oauth2"
)

func newConf(url string) *Config {
@@ -57,7 +56,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- tok, err := conf.Token(oauth2.NoContext)
+ tok, err := conf.Token(context.Background())
if err != nil {
t.Error(err)
}
@@ -91,6 +90,6 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- c := conf.Client(oauth2.NoContext)
+ c := conf.Client(context.Background())
c.Get(ts.URL + "/somethingelse")
}
diff --git a/example_test.go b/example_test.go
index f5ac863..7890913 100644
--- a/example_test.go
+++ b/example_test.go
@@ -5,6 +5,7 @@
package oauth2_test

import (
+ "context"
"fmt"
"log"

@@ -35,11 +36,12 @@
if _, err := fmt.Scan(&code); err != nil {
log.Fatal(err)
}
- tok, err := conf.Exchange(oauth2.NoContext, code)
+ ctx := context.Background()
+ tok, err := conf.Exchange(ctx, code)
if err != nil {
log.Fatal(err)
}

- client := conf.Client(oauth2.NoContext, tok)
+ client := conf.Client(ctx, tok)
client.Get("...")
}
diff --git a/jwt/example_test.go b/jwt/example_test.go
index a9533e8..a7e0027 100644
--- a/jwt/example_test.go
+++ b/jwt/example_test.go
@@ -5,7 +5,8 @@
package jwt_test

import (
- "golang.org/x/oauth2"
+ "context"
+
"golang.org/x/oauth2/jwt"
)

@@ -26,6 +27,6 @@
}
// Initiate an http.Client, the following GET request will be
// authorized and authenticated on the behalf of us...@example.com.
- client := conf.Client(oauth2.NoContext)
+ client := conf.Client(context.Background())
client.Get("...")
}
diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go
index a9c126b..b79b816 100644
--- a/jwt/jwt_test.go
+++ b/jwt/jwt_test.go
@@ -5,11 +5,10 @@
package jwt

import (
+ "context"
"net/http"
"net/http/httptest"
"testing"
-
- "golang.org/x/oauth2"
)

var dummyPrivateKey = []byte(`-----BEGIN RSA PRIVATE KEY-----
@@ -57,7 +56,7 @@
PrivateKey: dummyPrivateKey,
TokenURL: ts.URL,
}
- tok, err := conf.TokenSource(oauth2.NoContext).Token()
+ tok, err := conf.TokenSource(context.Background()).Token()
if err != nil {
t.Fatal(err)
}
@@ -91,7 +90,7 @@
PrivateKey: dummyPrivateKey,
TokenURL: ts.URL,
}
- tok, err := conf.TokenSource(oauth2.NoContext).Token()
+ tok, err := conf.TokenSource(context.Background()).Token()
if err != nil {
t.Fatal(err)
}
@@ -124,7 +123,7 @@
PrivateKey: dummyPrivateKey,
TokenURL: ts.URL,
}
- tok, err := conf.TokenSource(oauth2.NoContext).Token()
+ tok, err := conf.TokenSource(context.Background()).Token()
if err == nil {
t.Error("got a token; expected error")
if tok.AccessToken != "" {
diff --git a/oauth2.go b/oauth2.go
index 0505263..33aa8a5 100644
--- a/oauth2.go
+++ b/oauth2.go
@@ -21,6 +21,7 @@

// NoContext is the default context you should supply if not using
// your own context.Context (see https://golang.org/x/net/context).
+// Deprecated: use context.TODO() or context.Background()
var NoContext = context.TODO()

// RegisterBrokenAuthHeaderProvider registers an OAuth2 server
diff --git a/oauth2_test.go b/oauth2_test.go
index a9aecd6..e6f5cac 100644
--- a/oauth2_test.go
+++ b/oauth2_test.go
@@ -97,7 +97,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- tok, err := conf.Exchange(NoContext, "exchange-code")
+ tok, err := conf.Exchange(context.Background(), "exchange-code")
if err != nil {
t.Error(err)
}
@@ -141,7 +141,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- tok, err := conf.Exchange(NoContext, "exchange-code")
+ tok, err := conf.Exchange(context.Background(), "exchange-code")
if err != nil {
t.Error(err)
}
@@ -236,7 +236,7 @@
defer ts.Close()
conf := newConf(ts.URL)
t1 := time.Now().Add(day)
- tok, err := conf.Exchange(NoContext, "exchange-code")
+ tok, err := conf.Exchange(context.Background(), "exchange-code")
t2 := time.Now().Add(day)
// Do a fmt.Sprint comparison so either side can be
// nil. fmt.Sprint just stringifies them to "<nil>", and no
@@ -269,7 +269,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- tok, err := conf.Exchange(NoContext, "code")
+ tok, err := conf.Exchange(context.Background(), "code")
if err != nil {
t.Fatal(err)
}
@@ -285,7 +285,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- _, err := conf.Exchange(NoContext, "exchange-code")
+ _, err := conf.Exchange(context.Background(), "exchange-code")
if err == nil {
t.Error("expected error from invalid access_token type")
}
@@ -344,7 +344,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- tok, err := conf.PasswordCredentialsToken(NoContext, "user1", "password1")
+ tok, err :=
conf.PasswordCredentialsToken(context.Background(), "user1", "password1")
if err != nil {
t.Error(err)
}
@@ -380,7 +380,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- c := conf.Client(NoContext, &Token{RefreshToken: "REFRESH_TOKEN"})
+ c := conf.Client(context.Background(),
&Token{RefreshToken: "REFRESH_TOKEN"})
c.Get(ts.URL + "/somethingelse")
}

@@ -403,7 +403,7 @@
}))
defer ts.Close()
conf := newConf(ts.URL)
- c := conf.Client(NoContext, nil)
+ c := conf.Client(context.Background(), nil)
_, err := c.Get(ts.URL + "/somethingelse")
if err == nil {
t.Errorf("Fetch should return an error if no refresh token is set")
@@ -420,7 +420,7 @@
conf := newConf(ts.URL)
tkr := tokenRefresher{
conf: conf,
- ctx: NoContext,
+ ctx: context.Background(),
refreshToken: "OLD REFRESH TOKEN",
}
tk, err := tkr.Token()
@@ -446,7 +446,7 @@
defer ts.Close()
conf := newConf(ts.URL)

- c := conf.Client(NoContext, tok)
+ c := conf.Client(context.Background(), tok)
req, err := http.NewRequest("GET", ts.URL, nil)
if err != nil {
t.Error(err)

--
https://go-review.googlesource.com/27690
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Chris Broadfoot <cb...@golang.org>

[Jaana Burcu Dogan (Gerrit)]

Reviewers: Brad Fitzpatrick, Chris Broadfoot

Jaana Burcu Dogan uploaded a new patch set:

https://go-review.googlesource.com/27690

all: deprecate NoContext

There is no good reason why we suggest NoContext rather than
context.Background(). When the oauth2 library first came around, the
community was not familiar with the x/net/context package. for
documentation reasons, we decided to add NoContext to the oauth2
package. It was not a good idea even back then, and given that context
package is fairly popular, there is no good reason why we are
depending on this.

Updating all the references of NoContext with context.Background
and documenting it as deprecated.

Change-Id: I18e390f1351023a29b567777a3f963dd550cf657
---
M clientcredentials/clientcredentials_test.go
M example_test.go
M jwt/example_test.go
M jwt/jwt_test.go
M oauth2.go
M oauth2_test.go
6 files changed, 25 insertions(+), 23 deletions(-)

[Jaana Burcu Dogan (Gerrit)]

Reviewers: Brad Fitzpatrick, Chris Broadfoot

Jaana Burcu Dogan uploaded a new patch set:

https://go-review.googlesource.com/27690

all: deprecate NoContext

There is no good reason why we suggest NoContext rather than
context.Background(). When the oauth2 library first came around, the

community was not familiar with the x/net/context package. For

documentation reasons, we decided to add NoContext to the oauth2

package. It was not a good idea even back then. And given that context

package is fairly popular, there is no good reason why we are
depending on this.

Updating all the references of NoContext with context.Background
and documenting it as deprecated.

Change-Id: I18e390f1351023a29b567777a3f963dd550cf657
---
M clientcredentials/clientcredentials_test.go
M example_test.go
M jwt/example_test.go
M jwt/jwt_test.go
M oauth2.go
M oauth2_test.go
6 files changed, 25 insertions(+), 23 deletions(-)

[Chris Broadfoot (Gerrit)]

Chris Broadfoot has posted comments on this change.

all: deprecate NoContext

Patch Set 3: Code-Review+1

(3 comments)

https://go-review.googlesource.com/#/c/27690/1/example_test.go
File example_test.go:

Line 39: ctx := context.Background()
Move to top of function?


https://go-review.googlesource.com/#/c/27690/1/jwt/example_test.go
File jwt/example_test.go:

PS1, Line 30: client := conf.Client(context.Background())
ctx := context.Background() at top


https://go-review.googlesource.com/#/c/27690/1/oauth2.go
File oauth2.go:

Line 24: // Deprecated: use context.Background() or context.TODO()
I think you need a newline above it to turn it into a paragraph in godoc.

--
https://go-review.googlesource.com/27690
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Chris Broadfoot <cb...@golang.org>

Gerrit-HasComments: Yes

[Jaana Burcu Dogan (Gerrit)]

Reviewers: Brad Fitzpatrick, Chris Broadfoot

Jaana Burcu Dogan uploaded a new patch set:

https://go-review.googlesource.com/27690

all: deprecate NoContext

There is no good reason why we suggest NoContext rather than
context.Background(). When the oauth2 library first came around, the

community was not familiar with the x/net/context package. For

documentation reasons, we decided to add NoContext to the oauth2

package. It was not a good idea even back then. And given that context

package is fairly popular, there is no good reason why we are
depending on this.

Updating all the references of NoContext with context.Background
and documenting it as deprecated.

Change-Id: I18e390f1351023a29b567777a3f963dd550cf657
---
M clientcredentials/clientcredentials_test.go
M example_test.go
M jwt/example_test.go
M jwt/jwt_test.go
M oauth2.go
M oauth2_test.go

6 files changed, 27 insertions(+), 23 deletions(-)

[Jaana Burcu Dogan (Gerrit)]

Jaana Burcu Dogan has posted comments on this change.

all: deprecate NoContext

Patch Set 4:

(3 comments)

https://go-review.googlesource.com/#/c/27690/1/example_test.go
File example_test.go:

Line 39: }

> Move to top of function?

Done


https://go-review.googlesource.com/#/c/27690/1/jwt/example_test.go
File jwt/example_test.go:

PS1, Line 30: // authorized and authenticated on the beha

> ctx := context.Background() at top

Done

https://go-review.googlesource.com/#/c/27690/1/oauth2.go
File oauth2.go:

Line 24: //

> I think you need a newline above it to turn it into a paragraph in godoc.

Done.

Also slightly changed the format to match the standard library style.

--
https://go-review.googlesource.com/27690
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Chris Broadfoot <cb...@golang.org>

Gerrit-Reviewer: Jaana Burcu Dogan <j...@google.com>
Gerrit-HasComments: Yes

[Chris Broadfoot (Gerrit)]

Chris Broadfoot has posted comments on this change.

all: deprecate NoContext

Patch Set 4: Code-Review+2

--
https://go-review.googlesource.com/27690
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Chris Broadfoot <cb...@golang.org>

Gerrit-Reviewer: Jaana Burcu Dogan <j...@google.com>

Gerrit-HasComments: No

[Jaana Burcu Dogan (Gerrit)]

Jaana Burcu Dogan has submitted this change and it was merged.

all: deprecate NoContext

There is no good reason why we suggest NoContext rather than
context.Background(). When the oauth2 library first came around, the

community was not familiar with the x/net/context package. For

documentation reasons, we decided to add NoContext to the oauth2

package. It was not a good idea even back then. And given that context

package is fairly popular, there is no good reason why we are
depending on this.

Updating all the references of NoContext with context.Background
and documenting it as deprecated.

Change-Id: I18e390f1351023a29b567777a3f963dd550cf657

Reviewed-on: https://go-review.googlesource.com/27690
Reviewed-by: Chris Broadfoot <cb...@golang.org>

---
M clientcredentials/clientcredentials_test.go
M example_test.go
M jwt/example_test.go
M jwt/jwt_test.go
M oauth2.go
M oauth2_test.go

6 files changed, 27 insertions(+), 23 deletions(-)

Approvals:
Chris Broadfoot: Looks good to me, approved

--
https://go-review.googlesource.com/27690
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Chris Broadfoot <cb...@golang.org>