From 718c9d8b63c48ec55a1143df2df365cd6c173bdd Mon Sep 17 00:00:00 2001
From: mpl <mathieu.lonjaret@gmail.com>
Date: Wed, 16 Dec 2015 01:22:48 +0100
Subject: [PATCH] pkg/importer: use go4.org/ctxutil

side-effect: fix an unchecked type assertion, and use a conventional
key for the HTTP client associated with the context (instead of our own
string).

Change-Id: I2701de3f41f1f1c56fe3b76bbf590c0e5b39bc72
---
 pkg/geocode/geocode.go                     | 11 +++-------
 pkg/importer/feed/feed.go                  |  7 +++++--
 pkg/importer/flickr/flickr.go              |  7 +++++--
 pkg/importer/foursquare/foursquare.go      |  7 +++++--
 pkg/importer/foursquare/foursquare_test.go |  4 +++-
 pkg/importer/importer.go                   | 20 +++++++-----------
 pkg/importer/oauth.go                      |  4 +++-
 pkg/importer/picasa/oa2_importers.go       |  8 +++++---
 pkg/importer/picasa/picasa.go              | 24 +++++++++-------------
 pkg/importer/pinboard/pinboard.go          |  3 ++-
 pkg/importer/twitter/twitter.go            |  8 +++++---
 pkg/importer/twitter/twitter_test.go       |  6 ++++--
 pkg/search/predicate_test.go               |  4 +++-
 13 files changed, 60 insertions(+), 53 deletions(-)

diff --git a/pkg/geocode/geocode.go b/pkg/geocode/geocode.go
index 80c4de349..a3d8c8a27 100644
--- a/pkg/geocode/geocode.go
+++ b/pkg/geocode/geocode.go
@@ -21,14 +21,13 @@ import (
 	"encoding/json"
 	"io"
 	"log"
-	"net/http"
 	"net/url"
 	"sync"
 
+	"go4.org/ctxutil"
+	"go4.org/syncutil/singleflight"
 	"golang.org/x/net/context"
 	"golang.org/x/net/context/ctxhttp"
-
-	"go4.org/syncutil/singleflight"
 )
 
 type LatLong struct {
@@ -61,11 +60,7 @@ func Lookup(ctx context.Context, address string) ([]Rect, error) {
 	rectsi, err := sf.Do(address, func() (interface{}, error) {
 		// TODO: static data files from OpenStreetMap, Wikipedia, etc?
 		urlStr := "https://maps.googleapis.com/maps/api/geocode/json?address=" + url.QueryEscape(address) + "&sensor=false"
-		cl := http.DefaultClient
-		if x := ctx.Value("HTTPClient"); x != nil {
-			cl = x.(*http.Client)
-		}
-		res, err := ctxhttp.Get(ctx, cl, urlStr)
+		res, err := ctxhttp.Get(ctx, ctxutil.Client(ctx), urlStr)
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/importer/feed/feed.go b/pkg/importer/feed/feed.go
index 14e29263a..e9363f526 100644
--- a/pkg/importer/feed/feed.go
+++ b/pkg/importer/feed/feed.go
@@ -33,8 +33,11 @@ import (
 	"camlistore.org/pkg/httputil"
 	"camlistore.org/pkg/importer"
 	"camlistore.org/pkg/schema"
+
 	"camlistore.org/third_party/code.google.com/p/go.net/html"
 	"camlistore.org/third_party/code.google.com/p/go.net/html/atom"
+
+	"go4.org/ctxutil"
 	"golang.org/x/net/context"
 )
 
@@ -195,7 +198,7 @@ func doGet(ctx context.Context, url string) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	res, err := importer.HTTPClient(ctx).Do(req)
+	res, err := ctxutil.Client(ctx).Do(req)
 	if err != nil {
 		log.Printf("Error fetching %s: %v", url, err)
 		return nil, err
@@ -221,7 +224,7 @@ func (r *run) urlFileRef(urlstr string) string {
 	}
 	im.mu.Unlock()
 
-	res, err := importer.HTTPClient(r).Get(urlstr)
+	res, err := ctxutil.Client(r).Get(urlstr)
 	if err != nil {
 		log.Printf("couldn't get file: %v", err)
 		return ""
diff --git a/pkg/importer/flickr/flickr.go b/pkg/importer/flickr/flickr.go
index afd83b2fe..970050136 100644
--- a/pkg/importer/flickr/flickr.go
+++ b/pkg/importer/flickr/flickr.go
@@ -32,7 +32,10 @@ import (
 	"camlistore.org/pkg/importer"
 	"camlistore.org/pkg/schema"
 	"camlistore.org/pkg/schema/nodeattr"
+
 	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
+
+	"go4.org/ctxutil"
 )
 
 const (
@@ -511,7 +514,7 @@ func (imp) ServeSetup(w http.ResponseWriter, r *http.Request, ctx *importer.Setu
 		httputil.ServeError(w, r, err)
 		return err
 	}
-	tempCred, err := oauthClient.RequestTemporaryCredentials(importer.HTTPClient(ctx), ctx.CallbackURL(), nil)
+	tempCred, err := oauthClient.RequestTemporaryCredentials(ctxutil.Client(ctx), ctx.CallbackURL(), nil)
 	if err != nil {
 		err = fmt.Errorf("Error getting temp cred: %v", err)
 		httputil.ServeError(w, r, err)
@@ -551,7 +554,7 @@ func (imp) ServeCallback(w http.ResponseWriter, r *http.Request, ctx *importer.S
 		return
 	}
 	tokenCred, vals, err := oauthClient.RequestToken(
-		importer.HTTPClient(ctx),
+		ctxutil.Client(ctx),
 		&oauth.Credentials{
 			Token:  tempToken,
 			Secret: tempSecret,
diff --git a/pkg/importer/foursquare/foursquare.go b/pkg/importer/foursquare/foursquare.go
index 9b7816f2b..66379355b 100644
--- a/pkg/importer/foursquare/foursquare.go
+++ b/pkg/importer/foursquare/foursquare.go
@@ -35,7 +35,10 @@ import (
 	"camlistore.org/pkg/importer"
 	"camlistore.org/pkg/schema"
 	"camlistore.org/pkg/schema/nodeattr"
+
 	"camlistore.org/third_party/code.google.com/p/goauth2/oauth"
+
+	"go4.org/ctxutil"
 	"golang.org/x/net/context"
 )
 
@@ -180,7 +183,7 @@ func (r *run) urlFileRef(urlstr, filename string) string {
 	}
 	im.mu.Unlock()
 
-	res, err := importer.HTTPClient(r).Get(urlstr)
+	res, err := ctxutil.Client(r).Get(urlstr)
 	if err != nil {
 		log.Printf("couldn't get image: %v", err)
 		return ""
@@ -435,7 +438,7 @@ func doGet(ctx context.Context, url string, form url.Values) (*http.Response, er
 	if err != nil {
 		return nil, err
 	}
-	res, err := importer.HTTPClient(ctx).Do(req)
+	res, err := ctxutil.Client(ctx).Do(req)
 	if err != nil {
 		log.Printf("Error fetching %s: %v", url, err)
 		return nil, err
diff --git a/pkg/importer/foursquare/foursquare_test.go b/pkg/importer/foursquare/foursquare_test.go
index c3a5a3005..d0c0b3761 100644
--- a/pkg/importer/foursquare/foursquare_test.go
+++ b/pkg/importer/foursquare/foursquare_test.go
@@ -21,12 +21,14 @@ import (
 	"testing"
 
 	"camlistore.org/pkg/httputil"
+
+	"go4.org/ctxutil"
 	"golang.org/x/net/context"
 )
 
 func TestGetUserId(t *testing.T) {
 	im := &imp{}
-	ctx, cancel := context.WithCancel(context.WithValue(context.TODO(), "HTTPClient", &http.Client{
+	ctx, cancel := context.WithCancel(context.WithValue(context.TODO(), ctxutil.HTTPClient, &http.Client{
 		Transport: httputil.NewFakeTransport(map[string]func() *http.Response{
 			"https://api.foursquare.com/v2/users/self?oauth_token=footoken&v=20140225": httputil.FileResponder("testdata/users-me-res.json"),
 		}),
diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go
index d48a1bf13..d2da9d90e 100644
--- a/pkg/importer/importer.go
+++ b/pkg/importer/importer.go
@@ -39,10 +39,11 @@ import (
 	"camlistore.org/pkg/search"
 	"camlistore.org/pkg/server"
 	"camlistore.org/pkg/types/camtypes"
-	"go4.org/jsonconfig"
-	"golang.org/x/net/context"
 
+	"go4.org/ctxutil"
+	"go4.org/jsonconfig"
 	"go4.org/syncutil"
+	"golang.org/x/net/context"
 )
 
 const (
@@ -274,7 +275,7 @@ func (sc *SetupContext) AccountURL() string {
 // a certain account on a certain importer.
 type RunContext struct {
 	context.Context
-	Cancel context.CancelFunc
+	cancel context.CancelFunc // for when we stop/pause the importing
 	Host   *Host
 
 	ia *importerAcct
@@ -299,7 +300,7 @@ func CreateAccount(h *Host, impl string) (*RunContext, error) {
 		Host: ia.im.host,
 		ia:   ia,
 	}
-	rc.Context, rc.Cancel = context.WithCancel(context.WithValue(context.TODO(), "HTTPClient", ia.im.host.HTTPClient()))
+	rc.Context, rc.cancel = context.WithCancel(context.WithValue(context.TODO(), ctxutil.HTTPClient, ia.im.host.HTTPClient()))
 	return rc, nil
 
 }
@@ -1072,7 +1073,7 @@ func (ia *importerAcct) start() {
 		Host: ia.im.host,
 		ia:   ia,
 	}
-	rc.Context, rc.Cancel = context.WithCancel(context.WithValue(context.TODO(), "HTTPClient", ia.im.host.HTTPClient()))
+	rc.Context, rc.cancel = context.WithCancel(context.WithValue(context.TODO(), ctxutil.HTTPClient, ia.im.host.HTTPClient()))
 	ia.current = rc
 	ia.stopped = false
 	ia.lastRunStart = time.Now()
@@ -1100,7 +1101,7 @@ func (ia *importerAcct) stop() {
 	if ia.current == nil || ia.stopped {
 		return
 	}
-	ia.current.Cancel()
+	ia.current.cancel()
 	ia.stopped = true
 }
 
@@ -1337,10 +1338,3 @@ func (h *Host) ObjectFromRef(permanodeRef blob.Ref) (*Object, error) {
 		attr: map[string][]string(db.Permanode.Attr),
 	}, nil
 }
-
-func HTTPClient(ctx context.Context) *http.Client {
-	if x := ctx.Value("HTTPClient"); x != nil {
-		return x.(*http.Client)
-	}
-	return http.DefaultClient
-}
diff --git a/pkg/importer/oauth.go b/pkg/importer/oauth.go
index 6a26f6443..aaacdc79d 100644
--- a/pkg/importer/oauth.go
+++ b/pkg/importer/oauth.go
@@ -27,6 +27,8 @@ import (
 	"camlistore.org/pkg/blob"
 	"camlistore.org/pkg/httputil"
 	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
+
+	"go4.org/ctxutil"
 	"golang.org/x/net/context"
 )
 
@@ -164,7 +166,7 @@ func (octx OAuthContext) Get(url string, form url.Values) (*http.Response, error
 	if octx.Client == nil {
 		return nil, errors.New("No OAuth client.")
 	}
-	res, err := octx.Client.Get(HTTPClient(octx.Ctx), octx.Creds, url, form)
+	res, err := octx.Client.Get(ctxutil.Client(octx.Ctx), octx.Creds, url, form)
 	if err != nil {
 		return nil, fmt.Errorf("Error fetching %s: %v", url, err)
 	}
diff --git a/pkg/importer/picasa/oa2_importers.go b/pkg/importer/picasa/oa2_importers.go
index 56d711d70..516e89e14 100644
--- a/pkg/importer/picasa/oa2_importers.go
+++ b/pkg/importer/picasa/oa2_importers.go
@@ -28,9 +28,11 @@ import (
 	"camlistore.org/pkg/blob"
 	"camlistore.org/pkg/httputil"
 	"camlistore.org/pkg/importer"
-	"golang.org/x/net/context"
 
 	"camlistore.org/third_party/code.google.com/p/goauth2/oauth"
+
+	"go4.org/ctxutil"
+	"golang.org/x/net/context"
 )
 
 const (
@@ -131,7 +133,7 @@ func (im extendedOAuth2) ServeCallback(w http.ResponseWriter, r *http.Request, c
 	// needs to have the access token that is obtained during Exchange.
 	transport := &oauth.Transport{
 		Config:    oauthConfig,
-		Transport: notOAuthTransport(importer.HTTPClient(ctx)),
+		Transport: notOAuthTransport(ctxutil.Client(ctx)),
 	}
 	token, err := transport.Exchange(code)
 	log.Printf("Token = %#v, error %v", token, err)
@@ -141,7 +143,7 @@ func (im extendedOAuth2) ServeCallback(w http.ResponseWriter, r *http.Request, c
 		return
 	}
 
-	picagoCtx, cancel := context.WithCancel(context.WithValue(ctx, "HTTPClient", transport.Client()))
+	picagoCtx, cancel := context.WithCancel(context.WithValue(ctx, ctxutil.HTTPClient, transport.Client()))
 	defer cancel()
 
 	userInfo, err := im.getUserInfo(picagoCtx)
diff --git a/pkg/importer/picasa/picasa.go b/pkg/importer/picasa/picasa.go
index 74347153e..d200e5ba9 100644
--- a/pkg/importer/picasa/picasa.go
+++ b/pkg/importer/picasa/picasa.go
@@ -37,12 +37,12 @@ import (
 	"camlistore.org/pkg/schema/nodeattr"
 	"camlistore.org/pkg/search"
 
-	"golang.org/x/net/context"
-
-	"go4.org/syncutil"
-
 	"camlistore.org/third_party/code.google.com/p/goauth2/oauth"
 	"camlistore.org/third_party/github.com/tgulacsi/picago"
+
+	"go4.org/ctxutil"
+	"go4.org/syncutil"
+	"golang.org/x/net/context"
 )
 
 const (
@@ -91,7 +91,7 @@ func init() {
 			baseOAuthConfig,
 			func(ctx context.Context) (*userInfo, error) {
 
-				u, err := picago.GetUser(importer.HTTPClient(ctx), "default")
+				u, err := picago.GetUser(ctxutil.Client(ctx), "default")
 				if err != nil {
 					return nil, err
 				}
@@ -143,10 +143,6 @@ type run struct {
 	anyErr bool
 }
 
-func (r *run) HTTPClient() *http.Client {
-	return importer.HTTPClient(context.Context(r))
-}
-
 func (r *run) errorf(format string, args ...interface{}) {
 	log.Printf(format, args...)
 	r.mu.Lock()
@@ -168,9 +164,9 @@ func (imp) Run(ctx *importer.RunContext) error {
 	transport := &oauth.Transport{
 		Config:    &ocfg,
 		Token:     &token,
-		Transport: notOAuthTransport(importer.HTTPClient(ctx)),
+		Transport: notOAuthTransport(ctxutil.Client(ctx)),
 	}
-	ctx.Context = context.WithValue(ctx.Context, "HTTPClient", transport.Client())
+	ctx.Context = context.WithValue(ctx.Context, ctxutil.HTTPClient, transport.Client())
 
 	root := ctx.RootNode()
 	if root.Attr(nodeattr.Title) == "" {
@@ -204,7 +200,7 @@ func (imp) Run(ctx *importer.RunContext) error {
 }
 
 func (r *run) importAlbums() error {
-	albums, err := picago.GetAlbums(r.HTTPClient(), "default")
+	albums, err := picago.GetAlbums(ctxutil.Client(r), "default")
 	if err != nil {
 		return fmt.Errorf("importAlbums: error listing albums: %v", err)
 	}
@@ -264,7 +260,7 @@ func (r *run) importAlbum(albumsNode *importer.Object, album picago.Album) (ret
 	// return a slice of all photos. My "InstantUpload/Auto
 	// Backup" album has 6678 photos (and growing) and this
 	// currently takes like 40 seconds. Fix.
-	photos, err := picago.GetPhotos(r.HTTPClient(), "default", album.ID)
+	photos, err := picago.GetPhotos(ctxutil.Client(r), "default", album.ID)
 	if err != nil {
 		return err
 	}
@@ -296,7 +292,7 @@ func (r *run) updatePhotoInAlbum(albumNode *importer.Object, photo picago.Photo)
 
 	getMediaBytes := func() (io.ReadCloser, error) {
 		log.Printf("Importing media from %v", photo.URL)
-		resp, err := r.HTTPClient().Get(photo.URL)
+		resp, err := ctxutil.Client(r).Get(photo.URL)
 		if err != nil {
 			return nil, fmt.Errorf("importing photo %s: %v", photo.ID, err)
 		}
diff --git a/pkg/importer/pinboard/pinboard.go b/pkg/importer/pinboard/pinboard.go
index 99a01e5a0..cee73687c 100644
--- a/pkg/importer/pinboard/pinboard.go
+++ b/pkg/importer/pinboard/pinboard.go
@@ -57,6 +57,7 @@ import (
 	"camlistore.org/pkg/schema"
 	"camlistore.org/pkg/schema/nodeattr"
 
+	"go4.org/ctxutil"
 	"go4.org/syncutil"
 )
 
@@ -254,7 +255,7 @@ func (r *run) importBatch(authToken string, parent *importer.Object) (keepTrying
 	start := time.Now()
 
 	u := fmt.Sprintf(fetchUrl, authToken, batchLimit, r.nextCursor)
-	resp, err := importer.HTTPClient(r).Get(u)
+	resp, err := ctxutil.Client(r).Get(u)
 	if err != nil {
 		return false, err
 	}
diff --git a/pkg/importer/twitter/twitter.go b/pkg/importer/twitter/twitter.go
index f23d65a11..48eab0a4a 100644
--- a/pkg/importer/twitter/twitter.go
+++ b/pkg/importer/twitter/twitter.go
@@ -39,8 +39,10 @@ import (
 	"camlistore.org/pkg/importer"
 	"camlistore.org/pkg/schema"
 	"camlistore.org/pkg/schema/nodeattr"
+
 	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
 
+	"go4.org/ctxutil"
 	"go4.org/syncutil"
 )
 
@@ -474,7 +476,7 @@ func (r *run) importTweet(parent *importer.Object, tweet tweetItem, viaAPI bool)
 		tried, gotMedia := 0, false
 		for _, mediaURL := range m.URLs() {
 			tried++
-			res, err := importer.HTTPClient(r).Get(mediaURL)
+			res, err := ctxutil.Client(r).Get(mediaURL)
 			if err != nil {
 				return false, fmt.Errorf("Error fetching %s for tweet %s : %v", mediaURL, url, err)
 			}
@@ -560,7 +562,7 @@ func (im *imp) ServeSetup(w http.ResponseWriter, r *http.Request, ctx *importer.
 		httputil.ServeError(w, r, err)
 		return err
 	}
-	tempCred, err := oauthClient.RequestTemporaryCredentials(importer.HTTPClient(ctx), ctx.CallbackURL(), nil)
+	tempCred, err := oauthClient.RequestTemporaryCredentials(ctxutil.Client(ctx), ctx.CallbackURL(), nil)
 	if err != nil {
 		err = fmt.Errorf("Error getting temp cred: %v", err)
 		httputil.ServeError(w, r, err)
@@ -600,7 +602,7 @@ func (im *imp) ServeCallback(w http.ResponseWriter, r *http.Request, ctx *import
 		return
 	}
 	tokenCred, vals, err := oauthClient.RequestToken(
-		importer.HTTPClient(ctx),
+		ctxutil.Client(ctx),
 		&oauth.Credentials{
 			Token:  tempToken,
 			Secret: tempSecret,
diff --git a/pkg/importer/twitter/twitter_test.go b/pkg/importer/twitter/twitter_test.go
index 02c6446af..965eaea0e 100644
--- a/pkg/importer/twitter/twitter_test.go
+++ b/pkg/importer/twitter/twitter_test.go
@@ -23,13 +23,15 @@ import (
 
 	"camlistore.org/pkg/httputil"
 	"camlistore.org/pkg/importer"
-	"golang.org/x/net/context"
 
 	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
+
+	"go4.org/ctxutil"
+	"golang.org/x/net/context"
 )
 
 func TestGetUserID(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.WithValue(context.TODO(), "HTTPClient", &http.Client{
+	ctx, cancel := context.WithCancel(context.WithValue(context.TODO(), ctxutil.HTTPClient, &http.Client{
 		Transport: httputil.NewFakeTransport(map[string]func() *http.Response{
 			apiURL + userInfoAPIPath: httputil.FileResponder(filepath.FromSlash("testdata/verify_credentials-res.json")),
 		}),
diff --git a/pkg/search/predicate_test.go b/pkg/search/predicate_test.go
index 7007c4de0..bc62ef682 100644
--- a/pkg/search/predicate_test.go
+++ b/pkg/search/predicate_test.go
@@ -26,6 +26,8 @@ import (
 
 	"camlistore.org/pkg/httputil"
 	"camlistore.org/pkg/types"
+
+	"go4.org/ctxutil"
 	"golang.org/x/net/context"
 )
 
@@ -62,7 +64,7 @@ var uitdamLC = &LocationConstraint{
 func newGeocodeContext() context.Context {
 	url := "https://maps.googleapis.com/maps/api/geocode/json?address=Uitdam&sensor=false"
 	transport := httputil.NewFakeTransport(map[string]func() *http.Response{url: httputil.StaticResponder(uitdamGoogle)})
-	return context.WithValue(context.TODO(), "HTTPClient", &http.Client{Transport: transport})
+	return context.WithValue(context.TODO(), ctxutil.HTTPClient, &http.Client{Transport: transport})
 }
 
 var uitdamGoogle = `HTTP/1.1 200 OK