Merge "importers: some refactoring around oauthContext"

pkg/importer/flickr/flickr.go

@@ -101,11 +101,6 @@ type run struct {
 	primaryPhoto map[string]string
 }
 
-// TODO(mpl): same as in twitter. refactor.
-func (r *run) oauthContext() oauthContext {
-	return oauthContext{r.Context, r.oauthClient, r.accessCreds}
-}
-
 func (imp) Run(ctx *importer.RunContext) error {
 	clientID, secret, err := ctx.Credentials()
 	if err != nil {
@@ -177,7 +172,7 @@ func (r *run) importPhotosets() error {
 	resp := struct {
 		Photosets photosetList
 	}{}
-	if err := r.oauthContext().flickrAPIRequest(&resp,
+	if err := r.flickrAPIRequest(&resp,
 		photosetsAPIPath, "user_id", r.userID); err != nil {
 		return err
 	}
@@ -223,7 +218,7 @@ func (r *run) importPhotoset(parent *importer.Object, photoset *photosetInfo, pa
 	resp := struct {
 		Photoset photosetItems
 	}{}
-	if err := r.oauthContext().flickrAPIRequest(&resp, photosetAPIPath, "user_id", r.userID,
+	if err := r.flickrAPIRequest(&resp, photosetAPIPath, "user_id", r.userID,
 		"page", fmt.Sprintf("%d", page), "photoset_id", photoset.Id, "extras", "original_format"); err != nil {
 		return 0, err
 	}
@@ -305,7 +300,7 @@ func (r *run) importPhotos() error {
 
 func (r *run) importPhotosPage(page int) (int, error) {
 	resp := photosSearch{}
-	if err := r.oauthContext().flickrAPIRequest(&resp, photosAPIPath, "user_id", r.userID, "page", fmt.Sprintf("%d", page),
+	if err := r.flickrAPIRequest(&resp, photosAPIPath, "user_id", r.userID, "page", fmt.Sprintf("%d", page),
 		"extras", "description,date_upload,date_taken,original_format,last_update,geo,tags,machine_tags,views,media,url_o"); err != nil {
 		return 0, err
 	}
@@ -391,7 +386,7 @@ func (r *run) importPhoto(parent *importer.Object, photo *photosSearchItem) erro
 	}
 	form := url.Values{}
 	form.Set("user_id", r.userID)
-	res, err := r.oauthContext().flickrRequest(photo.URL, form)
+	res, err := r.fetch(photo.URL, form)
 	if err != nil {
 		log.Printf("Flickr importer: Could not fetch %s: %s", photo.URL, err)
 		return err
@@ -448,7 +443,6 @@ func (r *run) getPhotosNode() (*importer.Object, error) {
 	return r.getTopLevelNode("photos", "Photos")
 }
 
-// TODO(mpl): same in twitter. refactor.
 func (r *run) getTopLevelNode(path string, title string) (*importer.Object, error) {
 	photos, err := r.RootNode().ChildPathObject(path)
 	if err != nil {
@@ -461,54 +455,19 @@ func (r *run) getTopLevelNode(path string, title string) (*importer.Object, erro
 	return photos, nil
 }
 
-// TODO(mpl): same as in twitter. refactor.
-// oauthContext is used as a value type, wrapping a context and oauth information.
-type oauthContext struct {
-	*context.Context
-	client *oauth.Client
-	creds  *oauth.Credentials
+func (r *run) flickrAPIRequest(result interface{}, method string, keyval ...string) error {
+	keyval = append([]string{"method", method, "format", "json", "nojsoncallback", "1"}, keyval...)
+	return importer.OAuthContext{
+		r.Context,
+		r.oauthClient,
+		r.accessCreds}.PopulateJSONFromURL(result, apiURL, keyval...)
 }
 
-func (ctx oauthContext) flickrAPIRequest(result interface{}, method string, keyval ...string) error {
-	if len(keyval)%2 == 1 {
-		panic("Incorrect number of keyval arguments. must be even.")
-	}
-
-	form := url.Values{}
-	form.Set("method", method)
-	form.Set("format", "json")
-	form.Set("nojsoncallback", "1")
-	for i := 0; i < len(keyval); i += 2 {
-		form.Set(keyval[i], keyval[i+1])
-	}
-
-	res, err := ctx.flickrRequest(apiURL, form)
-	if err != nil {
-		return err
-	}
-	err = httputil.DecodeJSON(res, result)
-	if err != nil {
-		return fmt.Errorf("could not parse response for %s: %v", apiURL, err)
-	}
-	return err
-}
-
-// TODO(mpl): same in twitter. refactor.
-func (ctx oauthContext) flickrRequest(url string, form url.Values) (*http.Response, error) {
-	if ctx.creds == nil {
-		return nil, errors.New("No OAuth credentials. Not logged in?")
-	}
-	if ctx.client == nil {
-		return nil, errors.New("No OAuth client.")
-	}
-	res, err := ctx.client.Get(ctx.HTTPClient(), ctx.creds, url, form)
-	if err != nil {
-		return nil, fmt.Errorf("Error fetching %s: %v", url, err)
-	}
-	if res.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("Get request on %s failed with: %s", url, res.Status)
-	}
-	return res, nil
+func (r *run) fetch(url string, form url.Values) (*http.Response, error) {
+	return importer.OAuthContext{
+		r.Context,
+		r.oauthClient,
+		r.accessCreds}.Get(url, form)
 }
 
 // TODO(mpl): same in twitter. refactor.

pkg/importer/oauth.go

@@ -25,6 +25,9 @@ import (
 	"strings"
 
 	"camlistore.org/pkg/blob"
+	"camlistore.org/pkg/context"
+	"camlistore.org/pkg/httputil"
+	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
 )
 
 const (
@@ -143,3 +146,53 @@ func (im OAuth2) SummarizeAccount(acct *Object) string {
 		acct.Attr(AcctAttrGivenName),
 		acct.Attr(AcctAttrFamilyName))
 }
+
+// OAuthContext wraps the OAuth1 state needed to perform API calls.
+//
+// It is used as a value type.
+type OAuthContext struct {
+	Ctx    *context.Context
+	Client *oauth.Client
+	Creds  *oauth.Credentials
+}
+
+// Get fetches through octx the resource defined by url and the values in form.
+func (octx OAuthContext) Get(url string, form url.Values) (*http.Response, error) {
+	if octx.Creds == nil {
+		return nil, errors.New("No OAuth credentials. Not logged in?")
+	}
+	if octx.Client == nil {
+		return nil, errors.New("No OAuth client.")
+	}
+	res, err := octx.Client.Get(octx.Ctx.HTTPClient(), octx.Creds, url, form)
+	if err != nil {
+		return nil, fmt.Errorf("Error fetching %s: %v", url, err)
+	}
+	if res.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("Get request on %s failed with: %s", url, res.Status)
+	}
+	return res, nil
+}
+
+// PopulateJSONFromURL makes a GET call at apiURL, using keyval as parameters of
+// the associated form. The JSON response is decoded into result.
+func (ctx OAuthContext) PopulateJSONFromURL(result interface{}, apiURL string, keyval ...string) error {
+	if len(keyval)%2 == 1 {
+		return errors.New("Incorrect number of keyval arguments. must be even.")
+	}
+
+	form := url.Values{}
+	for i := 0; i < len(keyval); i += 2 {
+		form.Set(keyval[i], keyval[i+1])
+	}
+
+	hres, err := ctx.Get(apiURL, form)
+	if err != nil {
+		return err
+	}
+	err = httputil.DecodeJSON(hres, result)
+	if err != nil {
+		return fmt.Errorf("could not parse response for %s: %v", apiURL, err)
+	}
+	return err
+}

pkg/importer/twitter/twitter.go

@@ -26,7 +26,6 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
-	"net/url"
 	"os"
 	"path"
 	"regexp"
@@ -151,10 +150,6 @@ type run struct {
 	anyErr bool
 }
 
-func (r *run) oauthContext() oauthContext {
-	return oauthContext{r.Context, r.oauthClient, r.accessCreds}
-}
-
 var forceFullImport, _ = strconv.ParseBool(os.Getenv("CAMLI_TWITTER_FULL_IMPORT"))
 
 func (im *imp) Run(ctx *importer.RunContext) error {
@@ -244,6 +239,13 @@ func (r *run) errorf(format string, args ...interface{}) {
 	r.anyErr = true
 }
 
+func (r *run) doAPI(result interface{}, apiPath string, keyval ...string) error {
+	return importer.OAuthContext{
+		r.Context,
+		r.oauthClient,
+		r.accessCreds}.PopulateJSONFromURL(result, apiURL+apiPath, keyval...)
+}
+
 func (r *run) importTweets(userID string) error {
 	maxId := ""
 	continueRequests := true
@@ -272,10 +274,10 @@ func (r *run) importTweets(userID string) error {
 		var err error
 		if maxId == "" {
 			log.Printf("Fetching tweets for userid %s", userID)
-			err = r.oauthContext().doAPI(&resp, userTimeLineAPIPath, attrs...)
+			err = r.doAPI(&resp, userTimeLineAPIPath, attrs...)
 		} else {
 			log.Printf("Fetching tweets for userid %s with max ID %s", userID, maxId)
-			err = r.oauthContext().doAPI(&resp, userTimeLineAPIPath,
+			err = r.doAPI(&resp, userTimeLineAPIPath,
 				append(attrs, "max_id", maxId)...)
 		}
 		if err != nil {
@@ -524,17 +526,15 @@ func (r *run) getTopLevelNode(path string) (*importer.Object, error) {
 	return obj, obj.SetAttr(nodeattr.Title, title)
 }
 
-// TODO(mpl): move to an api.go when we it gets bigger.
-
 type userInfo struct {
 	ID         string `json:"id_str"`
 	ScreenName string `json:"screen_name"`
 	Name       string `json:"name,omitempty"`
 }
 
-func getUserInfo(ctx oauthContext) (userInfo, error) {
+func getUserInfo(ctx importer.OAuthContext) (userInfo, error) {
 	var ui userInfo
-	if err := ctx.doAPI(&ui, userInfoAPIPath); err != nil {
+	if err := ctx.PopulateJSONFromURL(&ui, apiURL+userInfoAPIPath); err != nil {
 		return ui, err
 	}
 	if ui.ID == "" {
@@ -630,7 +630,7 @@ func (im *imp) ServeCallback(w http.ResponseWriter, r *http.Request, ctx *import
 		return
 	}
 
-	u, err := getUserInfo(oauthContext{ctx.Context, oauthClient, tokenCred})
+	u, err := getUserInfo(importer.OAuthContext{ctx.Context, oauthClient, tokenCred})
 	if err != nil {
 		httputil.ServeError(w, r, fmt.Errorf("Couldn't get user info: %v", err))
 		return
@@ -647,54 +647,6 @@ func (im *imp) ServeCallback(w http.ResponseWriter, r *http.Request, ctx *import
 	http.Redirect(w, r, ctx.AccountURL(), http.StatusFound)
 }
 
-// oauthContext is used as a value type, wrapping a context and oauth information.
-//
-// TODO: move this up to pkg/importer?
-type oauthContext struct {
-	*context.Context
-	client *oauth.Client
-	creds  *oauth.Credentials
-}
-
-func (ctx oauthContext) doAPI(result interface{}, apiPath string, keyval ...string) error {
-	if len(keyval)%2 == 1 {
-		panic("Incorrect number of keyval arguments. must be even.")
-	}
-	form := url.Values{}
-	for i := 0; i < len(keyval); i += 2 {
-		if keyval[i+1] != "" {
-			form.Set(keyval[i], keyval[i+1])
-		}
-	}
-	fullURL := apiURL + apiPath
-	res, err := ctx.doGet(fullURL, form)
-	if err != nil {
-		return err
-	}
-	err = httputil.DecodeJSON(res, result)
-	if err != nil {
-		return fmt.Errorf("could not parse response for %s: %v", fullURL, err)
-	}
-	return nil
-}
-
-func (ctx oauthContext) doGet(url string, form url.Values) (*http.Response, error) {
-	if ctx.creds == nil {
-		return nil, errors.New("No OAuth credentials. Not logged in?")
-	}
-	if ctx.client == nil {
-		return nil, errors.New("No OAuth client.")
-	}
-	res, err := ctx.client.Get(ctx.HTTPClient(), ctx.creds, url, form)
-	if err != nil {
-		return nil, fmt.Errorf("Error fetching %s: %v", url, err)
-	}
-	if res.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("Get request on %s failed with: %s", url, res.Status)
-	}
-	return res, nil
-}
-
 type tweetItem interface {
 	ID() string
 	LatLong() (lat, long float64, ok bool)

pkg/importer/twitter/twitter_test.go

@@ -23,6 +23,7 @@ import (
 
 	"camlistore.org/pkg/context"
 	"camlistore.org/pkg/httputil"
+	"camlistore.org/pkg/importer"
 
 	"camlistore.org/third_party/github.com/garyburd/go-oauth/oauth"
 )
@@ -34,7 +35,7 @@ func TestGetUserID(t *testing.T) {
 		}),
 	}))
 	defer ctx.Cancel()
-	inf, err := getUserInfo(oauthContext{ctx, &oauth.Client{}, &oauth.Credentials{}})
+	inf, err := getUserInfo(importer.OAuthContext{ctx, &oauth.Client{}, &oauth.Credentials{}})
 	if err != nil {
 		t.Fatal(err)
 	}