importer: rename foursquare to swarm, add Properties to Importer inte…

…rface

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

@@ -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}", ""]
 				}

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"
 )

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 (

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) != "" {

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

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")

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,15 +810,15 @@ 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 {
 	return !strings.HasPrefix(im.host.ImporterBaseURL(), "https://")
 }
 
 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

pkg/importer/noop.go → 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")

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 {

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")

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) != ""