From ab797685bf65e50bfee8ef28054b95c22eb9f63d Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 22 Apr 2018 10:41:45 -0700 Subject: [PATCH] importer: rename foursquare to swarm, add Properties to Importer interface Foursquare was rebranded Swarm (for check-ins) some time ago, so rename our implementation to match the upstream branding. But still store "foursquare" as the permanode importer type, to not break old users. That override is now added to the new Properties struct, and all importers now return their properties. More stuff will be added to Properties later: WIP flag, bug link, proper title, icon? Change-Id: I31cbe2feec3dbf9c6bdb0c866056d9c6966313e3 --- config/dev-server-config.json | 6 +- pkg/importer/allimporters/importers.go | 2 +- pkg/importer/dummy/dummy.go | 31 +++--- pkg/importer/feed/feed.go | 9 +- pkg/importer/flickr/flickr.go | 9 +- pkg/importer/gphotos/gphotos.go | 7 +- pkg/importer/importer.go | 98 ++++++++++++++----- pkg/importer/{noop.go => noop_test.go} | 9 +- pkg/importer/oauth.go | 3 - pkg/importer/picasa/picasa.go | 7 +- pkg/importer/pinboard/pinboard.go | 9 +- pkg/importer/plaid/plaid.go | 11 +-- pkg/importer/{foursquare => swarm}/README | 5 +- pkg/importer/{foursquare => swarm}/api.go | 4 +- .../foursquare.go => swarm/swarm.go} | 25 ++--- .../swarm_test.go} | 2 +- .../{foursquare => swarm}/testdata.go | 2 +- .../testdata/users-me-res.json | 0 pkg/importer/twitter/twitter.go | 8 +- 19 files changed, 158 insertions(+), 89 deletions(-) rename pkg/importer/{noop.go => noop_test.go} (92%) rename pkg/importer/{foursquare => swarm}/README (85%) rename pkg/importer/{foursquare => swarm}/api.go (97%) rename pkg/importer/{foursquare/foursquare.go => swarm/swarm.go} (96%) rename pkg/importer/{foursquare/foursquare_test.go => swarm/swarm_test.go} (98%) rename pkg/importer/{foursquare => swarm}/testdata.go (99%) rename pkg/importer/{foursquare => swarm}/testdata/users-me-res.json (100%) diff --git a/config/dev-server-config.json b/config/dev-server-config.json index f98ea559c..ee3a6a205 100644 --- a/config/dev-server-config.json +++ b/config/dev-server-config.json @@ -361,15 +361,15 @@ "flickr": { "clientSecret": ["_env", "${CAMLI_FLICKR_API_KEY}", ""] }, - "foursquare": { - "clientSecret": ["_env", "${CAMLI_FOURSQUARE_API_KEY}", ""] - }, "picasa": { "clientSecret": ["_env", "${CAMLI_PICASA_API_KEY}", ""] }, "plaid": { "clientSecret": ["_env", "${CAMLI_PLAID_API_KEY}", ""] }, + "swarm": { + "clientSecret": ["_env", "${CAMLI_SWARM_API_KEY}", ""] + }, "twitter": { "clientSecret": ["_env", "${CAMLI_TWITTER_API_KEY}", ""] } diff --git a/pkg/importer/allimporters/importers.go b/pkg/importer/allimporters/importers.go index 7f07bf358..7a417467a 100644 --- a/pkg/importer/allimporters/importers.go +++ b/pkg/importer/allimporters/importers.go @@ -21,10 +21,10 @@ import ( _ "perkeep.org/pkg/importer/dummy" _ "perkeep.org/pkg/importer/feed" _ "perkeep.org/pkg/importer/flickr" - _ "perkeep.org/pkg/importer/foursquare" _ "perkeep.org/pkg/importer/gphotos" _ "perkeep.org/pkg/importer/picasa" _ "perkeep.org/pkg/importer/pinboard" _ "perkeep.org/pkg/importer/plaid" + _ "perkeep.org/pkg/importer/swarm" _ "perkeep.org/pkg/importer/twitter" ) diff --git a/pkg/importer/dummy/dummy.go b/pkg/importer/dummy/dummy.go index 87694a68f..0ca77f8f5 100644 --- a/pkg/importer/dummy/dummy.go +++ b/pkg/importer/dummy/dummy.go @@ -63,21 +63,22 @@ type imp struct { } -func (*imp) SupportsIncremental() bool { - // SupportsIncremental signals to the importer host that this - // importer has been optimized to be run regularly (e.g. every 5 - // minutes or half hour). If it returns false, the user must - // manually start imports. - return false -} - -func (*imp) NeedsAPIKey() bool { - // This tells the importer framework that we our importer will - // be calling the {RunContext,SetupContext}.Credentials method - // to get the OAuth client ID & client secret, which may be - // either configured on the importer permanode, or statically - // in the server's config file. - return true +func (*imp) Properties() importer.Properties { + return importer.Properties{ + // NeedsAPIKey tells the importer framework that this + // importer will be calling the + // {RunContext,SetupContext}.Credentials method to get + // the OAuth client ID & client secret, which may be + // either configured on the importer permanode, or + // statically in the server's config file. + NeedsAPIKey: true, + + // SupportsIncremental signals to the importer host that this + // importer has been optimized to be run regularly (e.g. every 5 + // minutes or half hour). If it returns false, the user must + // manually start imports. + SupportsIncremental: false, + } } const ( diff --git a/pkg/importer/feed/feed.go b/pkg/importer/feed/feed.go index 7f0c5fe77..3803cab0b 100644 --- a/pkg/importer/feed/feed.go +++ b/pkg/importer/feed/feed.go @@ -55,9 +55,12 @@ type imp struct { importer.OAuth1 // for CallbackRequestAccount and CallbackURLParameters } -func (im *imp) NeedsAPIKey() bool { return false } - -func (im *imp) SupportsIncremental() bool { return true } +func (*imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, + NeedsAPIKey: false, + } +} func (im *imp) IsAccountReady(acctNode *importer.Object) (ok bool, err error) { if acctNode.Attr(acctAttrFeedURL) != "" { diff --git a/pkg/importer/flickr/flickr.go b/pkg/importer/flickr/flickr.go index b27d34966..1a56ccd2c 100644 --- a/pkg/importer/flickr/flickr.go +++ b/pkg/importer/flickr/flickr.go @@ -67,9 +67,12 @@ type imp struct { importer.OAuth1 // for CallbackRequestAccount and CallbackURLParameters } -func (imp) NeedsAPIKey() bool { return true } - -func (imp) SupportsIncremental() bool { return false } +func (imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: false, + NeedsAPIKey: true, + } +} func (imp) IsAccountReady(acctNode *importer.Object) (ok bool, err error) { return acctNode.Attr(importer.AcctAttrUserName) != "" && acctNode.Attr(importer.AcctAttrAccessToken) != "", nil diff --git a/pkg/importer/gphotos/gphotos.go b/pkg/importer/gphotos/gphotos.go index 34ef70fab..081fcb2f9 100644 --- a/pkg/importer/gphotos/gphotos.go +++ b/pkg/importer/gphotos/gphotos.go @@ -85,7 +85,12 @@ type imp struct { importer.OAuth2 } -func (imp) SupportsIncremental() bool { return true } +func (imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, // TODO: but not well + NeedsAPIKey: true, + } +} type userInfo struct { ID string // numeric gphotos user ID ("11583474931002155675") diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go index 377afb26c..815cc1192 100644 --- a/pkg/importer/importer.go +++ b/pkg/importer/importer.go @@ -68,19 +68,8 @@ type Importer interface { // importer exits for that reason. Run(*RunContext) error - // NeedsAPIKey reports whether this importer requires an API key - // (OAuth2 client_id & client_secret, or equivalent). - // If the API only requires a username & password, or a flow to get - // an auth token per-account without an overall API key, importers - // can return false here. - NeedsAPIKey() bool - - // SupportsIncremental reports whether this importer has been optimized - // to run efficiently in regular incremental runs. (e.g. every 5 minutes - // or half hour). Eventually all importers might support this and we'll - // make it required, in which case we might delete this option. - // For now, some importers (e.g. Flickr) don't yet support this. - SupportsIncremental() bool + // Properties returns properties of this importer type. + Properties() Properties // IsAccountReady reports whether the provided account node // is configured. @@ -101,6 +90,34 @@ type Importer interface { CallbackURLParameters(acctRef blob.Ref) url.Values } +// Properties contains the properties of an importer type. +type Properties struct { + // NeedsAPIKey reports whether this importer requires an API key + // (OAuth2 client_id & client_secret, or equivalent). + // If the API only requires a username & password, or a flow to get + // an auth token per-account without an overall API key, importers + // can return false here. + NeedsAPIKey bool + + // SupportsIncremental reports whether this importer has been optimized + // to run efficiently in regular incremental runs. (e.g. every 5 minutes + // or half hour). Eventually all importers might support this and we'll + // make it required, in which case we might delete this option. + // For now, some importers (e.g. Flickr) don't yet support this. + SupportsIncremental bool + + // PermanodeImporterType optionally specifies the "importerType" + // permanode attribute value that should be stored for + // accounts of this type. By default, it is the string that it + // was registered with. This should only be specified for + // products that have been rebranded, and then this should be + // the old branding, to not break people who have been + // importing the account since before the rebranding. + // For example, this is "foursquare" for "swarm", so "swarm" shows + // in the UI and URLs, but it's "foursquare" in permanodes. + PermanodeImporterType string +} + // LongPoller is optionally implemented by importers which can long // poll efficiently to wait for new content. // For example, Twitter uses this to subscribe to the user's stream. @@ -131,7 +148,10 @@ type ImporterSetupHTMLer interface { AccountSetupHTML(*Host) string } -var importers = make(map[string]Importer) +var ( + importers = map[string]Importer{} + reservedImporterKey = map[string]bool{} +) // All returns the map of importer implementation name to implementation. This // map should not be mutated. @@ -145,6 +165,18 @@ func Register(name string, im Importer) { if _, dup := importers[name]; dup { panic("Dup registration of importer " + name) } + if _, dup := reservedImporterKey[name]; dup { + panic("Dup registration of importer " + name) + } + if pt := im.Properties().PermanodeImporterType; pt != "" { + if _, dup := importers[pt]; dup { + panic("Dup registration of importer " + pt) + } + if _, dup := reservedImporterKey[pt]; dup { + panic("Dup registration of importer " + pt) + } + reservedImporterKey[pt] = true + } importers[name] = im } @@ -195,10 +227,12 @@ func NewHost(hc HostConfig) (*Host, error) { if clientSecret != "" && clientId == "" { return nil, fmt.Errorf("Invalid static configuration for importer %q: clientSecret specified without clientId", k) } + props := impl.Properties() imp := &importer{ host: h, name: k, impl: impl, + props: &props, clientID: clientId, clientSecret: clientSecret, } @@ -734,9 +768,10 @@ func (h *Host) Searcher() search.QueryDescriber { return h.search } // importer is an importer for a certain site, but not a specific account on that site. type importer struct { - host *Host - name string // importer name e.g. "twitter" - impl Importer + host *Host + name string // importer name e.g. "twitter" + impl Importer + props *Properties // impl.Properties; pointer so we crash on & find uninitialized callers // If statically configured in config file, else // they come from the importer node's attributes. @@ -753,6 +788,17 @@ type importer struct { func (im *importer) Name() string { return im.name } +// ImporterType returns the account permanode's attrImporterType +// value. This is almost always the same as the Name, except in cases +// where a product gets rebranded. (e.g. "foursquare" to "swarm", in +// which case the importer type remains the old branding) +func (im *importer) ImporterType() string { + if im.props.PermanodeImporterType != "" { + return im.props.PermanodeImporterType + } + return im.name +} + func (im *importer) StaticConfig() bool { return im.clientSecret != "" } // URL returns the importer's URL without trailing slash. @@ -764,7 +810,7 @@ func (im *importer) ShowClientAuthEditForm() bool { // to the user. (e.g. a hosted multi-user configuation) return false } - return im.impl.NeedsAPIKey() + return im.props.NeedsAPIKey } func (im *importer) InsecureForm() bool { @@ -772,7 +818,7 @@ func (im *importer) InsecureForm() bool { } func (im *importer) CanAddNewAccount() bool { - if !im.impl.NeedsAPIKey() { + if !im.props.NeedsAPIKey { return true } id, sec, err := im.credentials() @@ -790,7 +836,7 @@ func (im *importer) ClientSecret() (v string, err error) { } func (im *importer) Status() (status string, err error) { - if !im.impl.NeedsAPIKey() { + if !im.props.NeedsAPIKey { return "no configuration required", nil } if im.StaticConfig() { @@ -838,7 +884,7 @@ func (im *importer) account(nodeRef blob.Ref) (*importerAcct, error) { if acct.Attr(attrNodeType) != nodeTypeImporterAccount { return nil, errors.New("account has wrong node type") } - if acct.Attr(attrImporterType) != im.name { + if acct.Attr(attrImporterType) != im.ImporterType() { return nil, errors.New("account has wrong importer type") } var root *Object @@ -884,7 +930,7 @@ func (im *importer) newAccount() (*importerAcct, error) { if err := acct.SetAttrs( "title", fmt.Sprintf("%s account", im.name), attrNodeType, nodeTypeImporterAccount, - attrImporterType, im.name, + attrImporterType, im.ImporterType(), attrImportRoot, root.PermanodeRef().String(), ); err != nil { return nil, err @@ -917,7 +963,7 @@ func (im *importer) Accounts() ([]*importerAcct, error) { res, err := im.host.search.Query(context.TODO(), &search.SearchQuery{ Expression: fmt.Sprintf("attr:%s:%s attr:%s:%s", attrNodeType, nodeTypeImporterAccount, - attrImporterType, im.name, + attrImporterType, im.ImporterType(), ), }) if err != nil { @@ -961,7 +1007,7 @@ func (im *importer) Node() (*Object, error) { expr := fmt.Sprintf("attr:%s:%s attr:%s:%s", attrNodeType, nodeTypeImporter, - attrImporterType, im.name, + attrImporterType, im.ImporterType(), ) res, err := im.host.search.Query(context.TODO(), &search.SearchQuery{ Limit: 10, // might be more than one because of multiple blob hash types @@ -985,7 +1031,7 @@ func (im *importer) Node() (*Object, error) { if err := o.SetAttrs( "title", fmt.Sprintf("%s importer", im.name), attrNodeType, nodeTypeImporter, - attrImporterType, im.name, + attrImporterType, im.ImporterType(), ); err != nil { return nil, err } @@ -1065,7 +1111,7 @@ func (ia *importerAcct) delete() error { func (ia *importerAcct) toggleAuto() error { old := ia.acct.Attr(attrImportAuto) - if old == "" && !ia.im.impl.SupportsIncremental() { + if old == "" && !ia.im.props.SupportsIncremental { return fmt.Errorf("Importer %q doesn't support automatic mode", ia.im.name) } var new string diff --git a/pkg/importer/noop.go b/pkg/importer/noop_test.go similarity index 92% rename from pkg/importer/noop.go rename to pkg/importer/noop_test.go index d577905a1..0748fa721 100644 --- a/pkg/importer/noop.go +++ b/pkg/importer/noop_test.go @@ -28,9 +28,12 @@ type todoImp struct { OAuth1 // for CallbackRequestAccount and CallbackURLParameters } -func (todoImp) NeedsAPIKey() bool { return false } - -func (todoImp) SupportsIncremental() bool { return false } +func (todoImp) Properties() Properties { + return Properties{ + NeedsAPIKey: false, + SupportsIncremental: false, + } +} func (todoImp) Run(*RunContext) error { return errors.New("fake error from todo importer") diff --git a/pkg/importer/oauth.go b/pkg/importer/oauth.go index 19503793e..a2c3e1969 100644 --- a/pkg/importer/oauth.go +++ b/pkg/importer/oauth.go @@ -126,9 +126,6 @@ func (OAuth2) IsAccountReady(acctNode *Object) (ok bool, err error) { return false, nil } -// NeedsAPIKey returns whether the importer needs an API key - returns constant true. -func (OAuth2) NeedsAPIKey() bool { return true } - // SummarizeAccount returns a summary for the account if it is configured, // or an error string otherwise. func (im OAuth2) SummarizeAccount(acct *Object) string { diff --git a/pkg/importer/picasa/picasa.go b/pkg/importer/picasa/picasa.go index cc9955f9d..b5d396ebe 100644 --- a/pkg/importer/picasa/picasa.go +++ b/pkg/importer/picasa/picasa.go @@ -82,7 +82,12 @@ type imp struct { importer.OAuth2 } -func (imp) SupportsIncremental() bool { return true } +func (imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, + NeedsAPIKey: true, + } +} type userInfo struct { ID string // numeric picasa user ID ("11583474931002155675") diff --git a/pkg/importer/pinboard/pinboard.go b/pkg/importer/pinboard/pinboard.go index b7b43f45a..9f41b1adf 100644 --- a/pkg/importer/pinboard/pinboard.go +++ b/pkg/importer/pinboard/pinboard.go @@ -101,9 +101,12 @@ type imp struct { importer.OAuth1 // for CallbackRequestAccount and CallbackURLParameters } -func (imp) SupportsIncremental() bool { return false } - -func (imp) NeedsAPIKey() bool { return false } +func (imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: false, + NeedsAPIKey: false, + } +} func (imp) IsAccountReady(acct *importer.Object) (ready bool, err error) { ready = acct.Attr(attrAuthToken) != "" diff --git a/pkg/importer/plaid/plaid.go b/pkg/importer/plaid/plaid.go index 34e371dd2..1140f999c 100644 --- a/pkg/importer/plaid/plaid.go +++ b/pkg/importer/plaid/plaid.go @@ -42,12 +42,11 @@ func init() { type imp struct{} -func (*imp) SupportsIncremental() bool { - return true -} - -func (*imp) NeedsAPIKey() bool { - return true +func (*imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, + NeedsAPIKey: true, + } } const ( diff --git a/pkg/importer/foursquare/README b/pkg/importer/swarm/README similarity index 85% rename from pkg/importer/foursquare/README rename to pkg/importer/swarm/README index f2e5d9e77..811f56422 100644 --- a/pkg/importer/foursquare/README +++ b/pkg/importer/swarm/README @@ -1,7 +1,7 @@ -Foursquare Importer +Swarm Importer =================== -This is an incomplete Perkeep importer for Foursquare. +This is a Perkeep importer for Swarm (by Foursquare). To use: @@ -16,4 +16,3 @@ To use: TODO: https://github.com/perkeep/perkeep/issues?q=is%3Aopen+is%3Aissue+foursquare - diff --git a/pkg/importer/foursquare/api.go b/pkg/importer/swarm/api.go similarity index 97% rename from pkg/importer/foursquare/api.go rename to pkg/importer/swarm/api.go index a5c0543ff..f5d2203f2 100644 --- a/pkg/importer/foursquare/api.go +++ b/pkg/importer/swarm/api.go @@ -14,9 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Types for Foursquare's JSON API. +// Types for Foursquare Swarm's JSON API. -package foursquare +package swarm type user struct { Id string diff --git a/pkg/importer/foursquare/foursquare.go b/pkg/importer/swarm/swarm.go similarity index 96% rename from pkg/importer/foursquare/foursquare.go rename to pkg/importer/swarm/swarm.go index c3ee41000..73aba8d1f 100644 --- a/pkg/importer/foursquare/foursquare.go +++ b/pkg/importer/swarm/swarm.go @@ -14,8 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package foursquare implements an importer for foursquare.com accounts. -package foursquare // import "perkeep.org/pkg/importer/foursquare" +// Package swarm implements an importer for Foursquare Swarm check-ins. +package swarm // import "perkeep.org/pkg/importer/swarm" import ( "context" @@ -72,7 +72,7 @@ const ( ) func init() { - importer.Register("foursquare", &imp{ + importer.Register("swarm", &imp{ imageFileRef: make(map[string]blob.Ref), }) } @@ -86,11 +86,12 @@ type imp struct { importer.OAuth2 // for CallbackRequestAccount and CallbackURLParameters } -func (im *imp) NeedsAPIKey() bool { - return true -} -func (im *imp) SupportsIncremental() bool { - return true +func (*imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, + NeedsAPIKey: true, + PermanodeImporterType: "foursquare", // old brand name + } } func (im *imp) IsAccountReady(acctNode *importer.Object) (ok bool, err error) { @@ -191,7 +192,7 @@ func (r *run) urlFileRef(urlstr, filename string) string { } res, err := ctxutil.Client(r.Context()).Get(urlstr) if err != nil { - log.Printf("foursquare: couldn't fetch image %q: %v", urlstr, err) + log.Printf("swarm: couldn't fetch image %q: %v", urlstr, err) return "" } defer res.Body.Close() @@ -232,7 +233,7 @@ func (r *run) importCheckins() error { } itemcount := len(resp.Response.Checkins.Items) - log.Printf("foursquare: importing %d checkins (offset %d)", itemcount, offset) + log.Printf("swarm: importing %d checkins (offset %d)", itemcount, offset) if itemcount < limit { continueRequests = false } else { @@ -341,14 +342,14 @@ func (r *run) importPhotos(placeNode *importer.Object, checkinWasDup bool) error if len(need) > 0 { venueTitle := placeNode.Attr(nodeattr.Title) - log.Printf("foursquare: importing %d photos for venue %s", len(need), venueTitle) + log.Printf("swarm: importing %d photos for venue %s", len(need), venueTitle) for _, photo := range need { attr := "camliPath:" + photo.Id + filepath.Ext(photo.Suffix) if photosNode.Attr(attr) != "" { continue } url := photo.Prefix + "original" + photo.Suffix - log.Printf("foursquare: importing photo for venue %s: %s", venueTitle, url) + log.Printf("swarm: importing photo for venue %s: %s", venueTitle, url) ref := r.urlFileRef(url, "") if ref == "" { r.errorf("Error slurping photo: %s", url) diff --git a/pkg/importer/foursquare/foursquare_test.go b/pkg/importer/swarm/swarm_test.go similarity index 98% rename from pkg/importer/foursquare/foursquare_test.go rename to pkg/importer/swarm/swarm_test.go index 99eccb2f6..444829fb9 100644 --- a/pkg/importer/foursquare/foursquare_test.go +++ b/pkg/importer/swarm/swarm_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package foursquare +package swarm import ( "context" diff --git a/pkg/importer/foursquare/testdata.go b/pkg/importer/swarm/testdata.go similarity index 99% rename from pkg/importer/foursquare/testdata.go rename to pkg/importer/swarm/testdata.go index 9af899e0b..9c725e328 100644 --- a/pkg/importer/foursquare/testdata.go +++ b/pkg/importer/swarm/testdata.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package foursquare +package swarm import ( "encoding/json" diff --git a/pkg/importer/foursquare/testdata/users-me-res.json b/pkg/importer/swarm/testdata/users-me-res.json similarity index 100% rename from pkg/importer/foursquare/testdata/users-me-res.json rename to pkg/importer/swarm/testdata/users-me-res.json diff --git a/pkg/importer/twitter/twitter.go b/pkg/importer/twitter/twitter.go index b6b9b72da..5a5a96152 100644 --- a/pkg/importer/twitter/twitter.go +++ b/pkg/importer/twitter/twitter.go @@ -102,8 +102,12 @@ type imp struct { importer.OAuth1 // for CallbackRequestAccount and CallbackURLParameters } -func (im *imp) NeedsAPIKey() bool { return true } -func (im *imp) SupportsIncremental() bool { return true } +func (*imp) Properties() importer.Properties { + return importer.Properties{ + SupportsIncremental: true, + NeedsAPIKey: true, + } +} func (im *imp) IsAccountReady(acctNode *importer.Object) (ok bool, err error) { if acctNode.Attr(importer.AcctAttrUserID) != "" && acctNode.Attr(importer.AcctAttrAccessToken) != "" {