From f70e775dbd7c53ebb6e3cd2ec1af8d73d13449b0 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sat, 15 Jun 2019 02:33:06 +0200 Subject: [PATCH 1/4] Store bridge type alongside the other params in the config --- bridge/bridges.go | 11 +++--- bridge/core/bridge.go | 81 ++++++++++++++++++++++------------------- bridge/github/config.go | 6 +++ commands/bridge_pull.go | 2 +- 4 files changed, 56 insertions(+), 44 deletions(-) diff --git a/bridge/bridges.go b/bridge/bridges.go index 6cbd13febf2c730f9458786564263d62a213e382..0aa0007fd15cd8b15864d38c3258b0c74a857d35 100644 --- a/bridge/bridges.go +++ b/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 RemoveBridges(repo repository.RepoCommon, name string) error { + return core.RemoveBridge(repo, name) } diff --git a/bridge/core/bridge.go b/bridge/core/bridge.go index 3513b7907d887f96a33e46ebac43b201ba875503..2b2db0a28a91f32e39ad694eef8bfb15206f955c 100644 --- a/bridge/core/bridge.go +++ b/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,33 @@ 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) { + bridge := &Bridge{ + Name: name, + repo: repo, + } + + conf, err := bridge.loadConfig() if err != nil { return nil, err } + bridge.conf = conf + + target := bridge.conf[keyTarget] + implType, ok := bridgeImpl[target] + if !ok { + return nil, fmt.Errorf("unknown bridge target %v", target) + } + + bridge.impl = reflect.New(implType).Elem().Interface().(BridgeImpl) + + err = bridge.impl.ValidateConfig(bridge.conf) + if err != nil { + return nil, errors.Wrap(err, "invalid configuration") + } - return NewBridge(repo, target, name) + return bridge, nil } // Attempt to retrieve a default bridge for the given repo. If zero or multiple @@ -108,22 +130,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 +141,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 +170,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 +191,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 { @@ -215,7 +226,7 @@ func (b *Bridge) ensureConfig() error { } func (b *Bridge) loadConfig() (Configuration, error) { - keyPrefix := fmt.Sprintf("git-bug.bridge.%s.%s.", b.impl.Target(), b.Name) + keyPrefix := fmt.Sprintf("git-bug.bridge.%s.", b.Name) pairs, err := b.repo.ReadConfigs(keyPrefix) if err != nil { @@ -225,14 +236,10 @@ func (b *Bridge) loadConfig() (Configuration, error) { result := make(Configuration, len(pairs)) for key, value := range pairs { key := strings.TrimPrefix(key, keyPrefix) + fmt.Println(key, value) result[key] = value } - err = b.impl.ValidateConfig(result) - if err != nil { - return nil, errors.Wrap(err, "invalid configuration") - } - return result, nil } diff --git a/bridge/github/config.go b/bridge/github/config.go index 707b3e2fdb5a3d2b5b4bb2c428a41c56f7184ef2..9c1bbcbb284daad5dbf0143200ee8b568aae5d1b 100644 --- a/bridge/github/config.go +++ b/bridge/github/config.go @@ -26,6 +26,7 @@ import ( const ( 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] = "github" conf[keyToken] = token conf[keyOwner] = owner conf[keyProject] = project @@ -109,6 +111,10 @@ func (*Github) Configure(repo repository.RepoCommon, params core.BridgeParams) ( } func (*Github) ValidateConfig(conf core.Configuration) error { + if _, ok := conf[keyTarget]; !ok { + return fmt.Errorf("missing %s key", keyTarget) + } + if _, ok := conf[keyToken]; !ok { return fmt.Errorf("missing %s key", keyToken) } diff --git a/commands/bridge_pull.go b/commands/bridge_pull.go index f995888258d84f48f14e67af6862eb986a8a4ad7..c7a22d6da712956a24d87875f49f9fa06cb09f54 100644 --- a/commands/bridge_pull.go +++ b/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 { From 319b648d0f2d30c457c56df860dcecf89c80157a Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 16 Jun 2019 23:02:59 +0200 Subject: [PATCH 2/4] Naming fixes --- bridge/bridges.go | 2 +- bridge/core/bridge.go | 1 - bridge/github/config.go | 8 +++++--- commands/bridge_rm.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/bridge/bridges.go b/bridge/bridges.go index 0aa0007fd15cd8b15864d38c3258b0c74a857d35..ce6013e37a068de3d443b775c46818cf238676a0 100644 --- a/bridge/bridges.go +++ b/bridge/bridges.go @@ -37,6 +37,6 @@ func ConfiguredBridges(repo repository.RepoCommon) ([]string, error) { } // Remove a configured bridge -func RemoveBridges(repo repository.RepoCommon, name string) error { +func RemoveBridge(repo repository.RepoCommon, name string) error { return core.RemoveBridge(repo, name) } diff --git a/bridge/core/bridge.go b/bridge/core/bridge.go index 2b2db0a28a91f32e39ad694eef8bfb15206f955c..4b93be7f6d8f82ee03d0f5adcd81410848b8174b 100644 --- a/bridge/core/bridge.go +++ b/bridge/core/bridge.go @@ -236,7 +236,6 @@ func (b *Bridge) loadConfig() (Configuration, error) { result := make(Configuration, len(pairs)) for key, value := range pairs { key := strings.TrimPrefix(key, keyPrefix) - fmt.Println(key, value) result[key] = value } diff --git a/bridge/github/config.go b/bridge/github/config.go index 9c1bbcbb284daad5dbf0143200ee8b568aae5d1b..1971abbc0352384f6798761cdf616dc6fe9fd9a4 100644 --- a/bridge/github/config.go +++ b/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,6 +24,7 @@ import ( ) const ( + target = "github" githubV3Url = "https://api.github.com" keyTarget = "target" keyOwner = "owner" @@ -102,7 +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] = "github" + conf[keyTarget] = target conf[keyToken] = token conf[keyOwner] = owner conf[keyProject] = project @@ -111,8 +111,10 @@ func (*Github) Configure(repo repository.RepoCommon, params core.BridgeParams) ( } func (*Github) ValidateConfig(conf core.Configuration) error { - if _, ok := conf[keyTarget]; !ok { + 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 { diff --git a/commands/bridge_rm.go b/commands/bridge_rm.go index 80a831ff3e626479345625f472adb4626333154d..5d38083cf473480e3271f61c408564497d709b3d 100644 --- a/commands/bridge_rm.go +++ b/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 } From 041dc08d4306fe28770be172f057819f325af9a5 Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Mon, 17 Jun 2019 00:08:02 +0200 Subject: [PATCH 3/4] add target to launchpad configuration --- bridge/launchpad/config.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bridge/launchpad/config.go b/bridge/launchpad/config.go index d8efea46662ddf9afbb32dea1122d4b1f3af52e9..c02fd16d7db6dfbb0269aaa0cc0b5397c8152848 100644 --- a/bridge/launchpad/config.go +++ b/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 } From 316b4ea6c241960c7506fc5b4d76a4e31484465c Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Mon, 17 Jun 2019 00:10:21 +0200 Subject: [PATCH 4/4] LoadBridge create a bridge using loadConfig update loadConfig signature --- bridge/core/bridge.go | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/bridge/core/bridge.go b/bridge/core/bridge.go index 4b93be7f6d8f82ee03d0f5adcd81410848b8174b..1b960e0e1fea5f4d94d82999ce9b7bf109155e65 100644 --- a/bridge/core/bridge.go +++ b/bridge/core/bridge.go @@ -87,30 +87,24 @@ func NewBridge(repo *cache.RepoCache, target string, name string) (*Bridge, erro // LoadBridge instantiate a new bridge from a repo configuration func LoadBridge(repo *cache.RepoCache, name string) (*Bridge, error) { - bridge := &Bridge{ - Name: name, - repo: repo, - } - - conf, err := bridge.loadConfig() + conf, err := loadConfig(repo, name) if err != nil { return nil, err } - bridge.conf = conf - target := bridge.conf[keyTarget] - implType, ok := bridgeImpl[target] - if !ok { - return nil, fmt.Errorf("unknown bridge target %v", target) + target := conf[keyTarget] + bridge, err := NewBridge(repo, target, name) + if err != nil { + return nil, err } - bridge.impl = reflect.New(implType).Elem().Interface().(BridgeImpl) - - err = bridge.impl.ValidateConfig(bridge.conf) + 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 } @@ -215,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 } @@ -225,10 +219,10 @@ func (b *Bridge) ensureConfig() error { return nil } -func (b *Bridge) loadConfig() (Configuration, error) { - keyPrefix := fmt.Sprintf("git-bug.bridge.%s.", 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") }