Merge pull request #170 from A-Hilaly/config-file

Michael Muré created 6 years ago

[bridge] Change bridge name format in the config

Change summary

bridge/bridges.go          | 11 ++---
bridge/core/bridge.go      | 80 ++++++++++++++++++++--------------------
bridge/github/config.go    | 10 ++++
bridge/launchpad/config.go |  7 +++
commands/bridge_pull.go    |  2 
commands/bridge_rm.go      |  2 
6 files changed, 63 insertions(+), 49 deletions(-)

Detailed changes

bridge/bridges.go

@@ -19,10 +19,9 @@ func NewBridge(repo *cache.RepoCache, target string, name string) (*core.Bridge,
 	return core.NewBridge(repo, target, name)
 }
 
-// Instantiate a new bridge for a repo, from the combined target and name contained
-// in the full name
-func NewBridgeFromFullName(repo *cache.RepoCache, fullName string) (*core.Bridge, error) {
-	return core.NewBridgeFromFullName(repo, fullName)
+// LoadBridge instantiate a new bridge from a repo configuration
+func LoadBridge(repo *cache.RepoCache, name string) (*core.Bridge, error) {
+	return core.LoadBridge(repo, name)
 }
 
 // Attempt to retrieve a default bridge for the given repo. If zero or multiple
@@ -38,6 +37,6 @@ func ConfiguredBridges(repo repository.RepoCommon) ([]string, error) {
 }
 
 // Remove a configured bridge
-func RemoveBridges(repo repository.RepoCommon, fullName string) error {
-	return core.RemoveBridge(repo, fullName)
+func RemoveBridge(repo repository.RepoCommon, name string) error {
+	return core.RemoveBridge(repo, name)
 }

bridge/core/bridge.go

@@ -9,15 +9,19 @@ import (
 	"strings"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/MichaelMure/git-bug/cache"
 	"github.com/MichaelMure/git-bug/repository"
-	"github.com/pkg/errors"
 )
 
 var ErrImportNotSupported = errors.New("import is not supported")
 var ErrExportNotSupported = errors.New("export is not supported")
 
-const bridgeConfigKeyPrefix = "git-bug.bridge"
+const (
+	keyTarget             = "target"
+	bridgeConfigKeyPrefix = "git-bug.bridge"
+)
 
 var bridgeImpl map[string]reflect.Type
 
@@ -81,15 +85,27 @@ func NewBridge(repo *cache.RepoCache, target string, name string) (*Bridge, erro
 	return bridge, nil
 }
 
-// Instantiate a new bridge for a repo, from the combined target and name contained
-// in the full name
-func NewBridgeFromFullName(repo *cache.RepoCache, fullName string) (*Bridge, error) {
-	target, name, err := splitFullName(fullName)
+// LoadBridge instantiate a new bridge from a repo configuration
+func LoadBridge(repo *cache.RepoCache, name string) (*Bridge, error) {
+	conf, err := loadConfig(repo, name)
+	if err != nil {
+		return nil, err
+	}
+
+	target := conf[keyTarget]
+	bridge, err := NewBridge(repo, target, name)
 	if err != nil {
 		return nil, err
 	}
 
-	return NewBridge(repo, target, name)
+	err = bridge.impl.ValidateConfig(conf)
+	if err != nil {
+		return nil, errors.Wrap(err, "invalid configuration")
+	}
+
+	// will avoid reloading configuration before an export or import call
+	bridge.conf = conf
+	return bridge, nil
 }
 
 // Attempt to retrieve a default bridge for the given repo. If zero or multiple
@@ -108,22 +124,7 @@ func DefaultBridge(repo *cache.RepoCache) (*Bridge, error) {
 		return nil, fmt.Errorf("multiple bridge are configured, you need to select one explicitely")
 	}
 
-	target, name, err := splitFullName(bridges[0])
-	if err != nil {
-		return nil, err
-	}
-
-	return NewBridge(repo, target, name)
-}
-
-func splitFullName(fullName string) (string, string, error) {
-	split := strings.Split(fullName, ".")
-
-	if len(split) != 2 {
-		return "", "", fmt.Errorf("bad bridge fullname: %s", fullName)
-	}
-
-	return split[0], split[1], nil
+	return LoadBridge(repo, bridges[0])
 }
 
 // ConfiguredBridges return the list of bridge that are configured for the given
@@ -134,7 +135,7 @@ func ConfiguredBridges(repo repository.RepoCommon) ([]string, error) {
 		return nil, errors.Wrap(err, "can't read configured bridges")
 	}
 
-	re, err := regexp.Compile(bridgeConfigKeyPrefix + `.([^.]+\.[^.]+)`)
+	re, err := regexp.Compile(bridgeConfigKeyPrefix + `.([^.]+)`)
 	if err != nil {
 		panic(err)
 	}
@@ -163,17 +164,17 @@ func ConfiguredBridges(repo repository.RepoCommon) ([]string, error) {
 }
 
 // Remove a configured bridge
-func RemoveBridge(repo repository.RepoCommon, fullName string) error {
-	re, err := regexp.Compile(`^[^.]+\.[^.]+$`)
+func RemoveBridge(repo repository.RepoCommon, name string) error {
+	re, err := regexp.Compile(`^[a-zA-Z0-9]+`)
 	if err != nil {
 		panic(err)
 	}
 
-	if !re.MatchString(fullName) {
-		return fmt.Errorf("bad bridge fullname: %s", fullName)
+	if !re.MatchString(name) {
+		return fmt.Errorf("bad bridge fullname: %s", name)
 	}
 
-	keyPrefix := fmt.Sprintf("git-bug.bridge.%s", fullName)
+	keyPrefix := fmt.Sprintf("git-bug.bridge.%s", name)
 	return repo.RmConfigs(keyPrefix)
 }
 
@@ -184,14 +185,18 @@ func (b *Bridge) Configure(params BridgeParams) error {
 		return err
 	}
 
-	b.conf = conf
+	err = b.impl.ValidateConfig(conf)
+	if err != nil {
+		return fmt.Errorf("invalid configuration: %v", err)
+	}
 
+	b.conf = conf
 	return b.storeConfig(conf)
 }
 
 func (b *Bridge) storeConfig(conf Configuration) error {
 	for key, val := range conf {
-		storeKey := fmt.Sprintf("git-bug.bridge.%s.%s.%s", b.impl.Target(), b.Name, key)
+		storeKey := fmt.Sprintf("git-bug.bridge.%s.%s", b.Name, key)
 
 		err := b.repo.StoreConfig(storeKey, val)
 		if err != nil {
@@ -204,7 +209,7 @@ func (b *Bridge) storeConfig(conf Configuration) error {
 
 func (b *Bridge) ensureConfig() error {
 	if b.conf == nil {
-		conf, err := b.loadConfig()
+		conf, err := loadConfig(b.repo, b.Name)
 		if err != nil {
 			return err
 		}
@@ -214,10 +219,10 @@ func (b *Bridge) ensureConfig() error {
 	return nil
 }
 
-func (b *Bridge) loadConfig() (Configuration, error) {
-	keyPrefix := fmt.Sprintf("git-bug.bridge.%s.%s.", b.impl.Target(), b.Name)
+func loadConfig(repo *cache.RepoCache, name string) (Configuration, error) {
+	keyPrefix := fmt.Sprintf("git-bug.bridge.%s.", name)
 
-	pairs, err := b.repo.ReadConfigs(keyPrefix)
+	pairs, err := repo.ReadConfigs(keyPrefix)
 	if err != nil {
 		return nil, errors.Wrap(err, "error while reading bridge configuration")
 	}
@@ -228,11 +233,6 @@ func (b *Bridge) loadConfig() (Configuration, error) {
 		result[key] = value
 	}
 
-	err = b.impl.ValidateConfig(result)
-	if err != nil {
-		return nil, errors.Wrap(err, "invalid configuration")
-	}
-
 	return result, nil
 }
 

bridge/github/config.go

@@ -17,7 +17,6 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-
 	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/MichaelMure/git-bug/bridge/core"
@@ -25,7 +24,9 @@ import (
 )
 
 const (
+	target      = "github"
 	githubV3Url = "https://api.github.com"
+	keyTarget   = "target"
 	keyOwner    = "owner"
 	keyProject  = "project"
 	keyToken    = "token"
@@ -101,6 +102,7 @@ func (*Github) Configure(repo repository.RepoCommon, params core.BridgeParams) (
 		return nil, fmt.Errorf("project doesn't exist or authentication token has an incorrect scope")
 	}
 
+	conf[keyTarget] = target
 	conf[keyToken] = token
 	conf[keyOwner] = owner
 	conf[keyProject] = project
@@ -109,6 +111,12 @@ func (*Github) Configure(repo repository.RepoCommon, params core.BridgeParams) (
 }
 
 func (*Github) ValidateConfig(conf core.Configuration) error {
+	if v, ok := conf[keyTarget]; !ok {
+		return fmt.Errorf("missing %s key", keyTarget)
+	} else if v != target {
+		return fmt.Errorf("unexpected target name: %v", v)
+	}
+
 	if _, ok := conf[keyToken]; !ok {
 		return fmt.Errorf("missing %s key", keyToken)
 	}

bridge/launchpad/config.go

@@ -17,7 +17,9 @@ import (
 var ErrBadProjectURL = errors.New("bad Launchpad project URL")
 
 const (
+	target         = "launchpad-preview"
 	keyProject     = "project"
+	keyTarget      = "target"
 	defaultTimeout = 60 * time.Second
 )
 
@@ -61,6 +63,7 @@ func (*Launchpad) Configure(repo repository.RepoCommon, params core.BridgeParams
 	}
 
 	conf[keyProject] = project
+	conf[keyTarget] = target
 	return conf, nil
 }
 
@@ -69,6 +72,10 @@ func (*Launchpad) ValidateConfig(conf core.Configuration) error {
 		return fmt.Errorf("missing %s key", keyProject)
 	}
 
+	if _, ok := conf[keyTarget]; !ok {
+		return fmt.Errorf("missing %s key", keyTarget)
+	}
+
 	return nil
 }
 

commands/bridge_pull.go

@@ -23,7 +23,7 @@ func runBridgePull(cmd *cobra.Command, args []string) error {
 	if len(args) == 0 {
 		b, err = bridge.DefaultBridge(backend)
 	} else {
-		b, err = bridge.NewBridgeFromFullName(backend, args[0])
+		b, err = bridge.LoadBridge(backend, args[0])
 	}
 
 	if err != nil {

commands/bridge_rm.go

@@ -15,7 +15,7 @@ func runBridgeRm(cmd *cobra.Command, args []string) error {
 	defer backend.Close()
 	interrupt.RegisterCleaner(backend.Close)
 
-	err = bridge.RemoveBridges(backend, args[0])
+	err = bridge.RemoveBridge(backend, args[0])
 	if err != nil {
 		return err
 	}