From a6e1067a50a3245730707f7153ab3899e23448b8 Mon Sep 17 00:00:00 2001 From: jackgr Date: Mon, 11 Jan 2016 20:01:17 -0800 Subject: [PATCH] Make github registry provider and repository service mockable. --- registry/github_package_registry.go | 11 ++-- registry/github_registry.go | 27 ++++++++-- registry/github_template_registry.go | 9 ++-- registry/registryprovider.go | 77 +++++++++++++++++----------- 4 files changed, 77 insertions(+), 47 deletions(-) diff --git a/registry/github_package_registry.go b/registry/github_package_registry.go index 681eeef18..5bea6613a 100644 --- a/registry/github_package_registry.go +++ b/registry/github_package_registry.go @@ -17,7 +17,6 @@ limitations under the License. package registry import ( - "github.com/google/go-github/github" "github.com/kubernetes/deployment-manager/common" "fmt" @@ -41,9 +40,9 @@ type GithubPackageRegistry struct { } // NewGithubPackageRegistry creates a GithubPackageRegistry. -func NewGithubPackageRegistry(name, shortURL string, client *github.Client) (GithubPackageRegistry, error) { +func NewGithubPackageRegistry(name, shortURL string, service RepositoryService) (GithubPackageRegistry, error) { format := fmt.Sprintf("%s;%s", common.UnversionedRegistry, common.OneLevelRegistry) - gr, err := newGithubRegistry(name, shortURL, common.RegistryFormat(format), client) + gr, err := newGithubRegistry(name, shortURL, common.RegistryFormat(format), service) if err != nil { return GithubPackageRegistry{}, err } @@ -64,7 +63,7 @@ func (g GithubPackageRegistry) ListTypes(regex *regexp.Regexp) ([]Type, error) { var retTypes []Type for _, t := range types { // Check to see if there's a Chart.yaml file in the directory - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, t, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, t, nil) if err != nil { log.Printf("Failed to list package files at path: %s: %v", t, err) return nil, err @@ -87,7 +86,7 @@ func (g GithubPackageRegistry) GetDownloadURLs(t Type) ([]*url.URL, error) { return nil, err } - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, path, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, path, nil) if err != nil { log.Printf("Failed to list package files at path: %s: %v", path, err) return nil, err @@ -111,7 +110,7 @@ func (g GithubPackageRegistry) GetDownloadURLs(t Type) ([]*url.URL, error) { } func (g GithubPackageRegistry) getDirs(dir string) ([]string, error) { - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, dir, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, dir, nil) if err != nil { log.Printf("Failed to get contents at path: %s: %v", dir, err) return nil, err diff --git a/registry/github_registry.go b/registry/github_registry.go index 5f25aa1ae..fed57322c 100644 --- a/registry/github_registry.go +++ b/registry/github_registry.go @@ -38,17 +38,34 @@ type githubRegistry struct { repository string path string format common.RegistryFormat - client *github.Client + service RepositoryService +} + +type RepositoryService interface { + GetContents( + owner, repo, path string, + opt *github.RepositoryContentGetOptions, + ) ( + fileContent *github.RepositoryContent, + directoryContent []*github.RepositoryContent, + resp *github.Response, + err error, + ) } // newGithubRegistry creates a githubRegistry. -func newGithubRegistry(name, shortURL string, format common.RegistryFormat, client *github.Client) (githubRegistry, error) { +func newGithubRegistry(name, shortURL string, format common.RegistryFormat, service RepositoryService) (githubRegistry, error) { trimmed := util.TrimURLScheme(shortURL) owner, repository, path, err := parseGithubShortURL(trimmed) if err != nil { return githubRegistry{}, fmt.Errorf("cannot create Github template registry %s: %s", name, err) } + if service == nil { + client := github.NewClient(nil) + service = client.Repositories + } + return githubRegistry{ name: name, shortURL: trimmed, @@ -56,7 +73,7 @@ func newGithubRegistry(name, shortURL string, format common.RegistryFormat, clie repository: repository, path: path, format: format, - client: client, + service: service, }, nil } @@ -165,7 +182,7 @@ func (g githubRegistry) GetDownloadURLs(t Type) ([]*url.URL, error) { if err != nil { return nil, err } - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, path, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, path, nil) if err != nil { return nil, fmt.Errorf("cannot list versions at path %s: %v", path, err) } @@ -205,7 +222,7 @@ func (g githubRegistry) getDirs(dir string) ([]string, error) { path = g.path + "/" + dir } - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, path, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, path, nil) if err != nil { log.Printf("Failed to get contents at path: %s: %v", path, err) return nil, err diff --git a/registry/github_template_registry.go b/registry/github_template_registry.go index 586e22118..be704eae2 100644 --- a/registry/github_template_registry.go +++ b/registry/github_template_registry.go @@ -17,7 +17,6 @@ limitations under the License. package registry import ( - "github.com/google/go-github/github" "github.com/kubernetes/deployment-manager/common" "fmt" @@ -54,9 +53,9 @@ type GithubTemplateRegistry struct { } // NewGithubTemplateRegistry creates a GithubTemplateRegistry. -func NewGithubTemplateRegistry(name, shortURL string, client *github.Client) (GithubTemplateRegistry, error) { +func NewGithubTemplateRegistry(name, shortURL string, service RepositoryService) (GithubTemplateRegistry, error) { format := fmt.Sprintf("%s;%s", common.VersionedRegistry, common.CollectionRegistry) - gr, err := newGithubRegistry(name, shortURL, common.RegistryFormat(format), client) + gr, err := newGithubRegistry(name, shortURL, common.RegistryFormat(format), service) if err != nil { return GithubTemplateRegistry{}, err } @@ -113,7 +112,7 @@ func (g GithubTemplateRegistry) GetDownloadURLs(t Type) ([]*url.URL, error) { if err != nil { return nil, err } - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, path, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, path, nil) if err != nil { return nil, fmt.Errorf("cannot list versions at path %s: %v", path, err) } @@ -153,7 +152,7 @@ func (g GithubTemplateRegistry) getDirs(dir string) ([]string, error) { path = g.path + "/" + dir } - _, dc, _, err := g.client.Repositories.GetContents(g.owner, g.repository, path, nil) + _, dc, _, err := g.service.GetContents(g.owner, g.repository, path, nil) if err != nil { log.Printf("Failed to get contents at path: %s: %v", path, err) return nil, err diff --git a/registry/registryprovider.go b/registry/registryprovider.go index 3675a9401..2d499fa0e 100644 --- a/registry/registryprovider.go +++ b/registry/registryprovider.go @@ -17,7 +17,6 @@ limitations under the License. package registry import ( - "github.com/google/go-github/github" "github.com/kubernetes/deployment-manager/common" "github.com/kubernetes/deployment-manager/util" @@ -28,55 +27,71 @@ import ( "sync" ) -// RegistryProvider returns factories for creating registry clients. +// RegistryProvider is a factory for Registry instances. type RegistryProvider interface { GetRegistryByShortURL(URL string) (Registry, error) GetRegistryByName(registryName string) (Registry, error) +} + +// GithubRegistryProvider is a factory for GithubRegistry instances. +type GithubRegistryProvider interface { GetGithubRegistry(cr common.Registry) (GithubRegistry, error) } func NewDefaultRegistryProvider() RegistryProvider { + return NewRegistryProvider(nil, nil) +} + +func NewRegistryProvider(rs common.RegistryService, grp GithubRegistryProvider) RegistryProvider { + if rs == nil { + rs = NewInmemRegistryService() + } + registries := make(map[string]Registry) - rs := NewInmemRegistryService() - return &DefaultRegistryProvider{registries: registries, rs: rs} + rp := registryProvider{rs: rs, registries: registries} + if grp == nil { + grp = rp + } + + rp.grp = grp + return rp } -type DefaultRegistryProvider struct { +type registryProvider struct { sync.RWMutex - registries map[string]Registry rs common.RegistryService + grp GithubRegistryProvider + registries map[string]Registry } -// Deprecated: Use GetRegistryByShortURL instead. -func (drp DefaultRegistryProvider) GetRegistryByURL(URL string) (Registry, error) { - return drp.GetRegistryByShortURL(URL) -} - -func (drp DefaultRegistryProvider) GetRegistryByShortURL(URL string) (Registry, error) { - drp.RLock() - defer drp.RUnlock() +func (rp registryProvider) GetRegistryByShortURL(URL string) (Registry, error) { + rp.RLock() + defer rp.RUnlock() - r := drp.findRegistryByShortURL(URL) + r := rp.findRegistryByShortURL(URL) if r == nil { - cr, err := drp.rs.GetByURL(URL) + cr, err := rp.rs.GetByURL(URL) if err != nil { return nil, err } - r, err := drp.GetGithubRegistry(*cr) + r, err := rp.GetGithubRegistry(*cr) if err != nil { return nil, err } - drp.registries[r.GetRegistryName()] = r + rp.registries[r.GetRegistryName()] = r } return r, nil } -func (drp DefaultRegistryProvider) findRegistryByShortURL(URL string) Registry { - for _, r := range drp.registries { - if strings.HasPrefix(URL, r.GetRegistryShortURL()) { +// findRegistryByShortURL trims the scheme from both the supplied URL +// and the short URL returned by GetRegistryShortURL. +func (rp registryProvider) findRegistryByShortURL(URL string) Registry { + trimmed := util.TrimURLScheme(URL) + for _, r := range rp.registries { + if strings.HasPrefix(trimmed, util.TrimURLScheme(r.GetRegistryShortURL())) { return r } } @@ -84,23 +99,23 @@ func (drp DefaultRegistryProvider) findRegistryByShortURL(URL string) Registry { return nil } -func (drp DefaultRegistryProvider) GetRegistryByName(registryName string) (Registry, error) { - drp.RLock() - defer drp.RUnlock() +func (rp registryProvider) GetRegistryByName(registryName string) (Registry, error) { + rp.RLock() + defer rp.RUnlock() - r, ok := drp.registries[registryName] + r, ok := rp.registries[registryName] if !ok { - cr, err := drp.rs.Get(registryName) + cr, err := rp.rs.Get(registryName) if err != nil { return nil, err } - r, err := drp.GetGithubRegistry(*cr) + r, err := rp.GetGithubRegistry(*cr) if err != nil { return nil, err } - drp.registries[r.GetRegistryName()] = r + rp.registries[r.GetRegistryName()] = r } return r, nil @@ -116,15 +131,15 @@ func ParseRegistryFormat(rf common.RegistryFormat) map[common.RegistryFormat]boo return result } -func (drp DefaultRegistryProvider) GetGithubRegistry(cr common.Registry) (GithubRegistry, error) { +func (rp registryProvider) GetGithubRegistry(cr common.Registry) (GithubRegistry, error) { if cr.Type == common.GithubRegistryType { fMap := ParseRegistryFormat(cr.Format) if fMap[common.UnversionedRegistry] && fMap[common.OneLevelRegistry] { - return NewGithubPackageRegistry(cr.Name, cr.URL, github.NewClient(nil)) + return NewGithubPackageRegistry(cr.Name, cr.URL, nil) } if fMap[common.VersionedRegistry] && fMap[common.CollectionRegistry] { - return NewGithubTemplateRegistry(cr.Name, cr.URL, github.NewClient(nil)) + return NewGithubTemplateRegistry(cr.Name, cr.URL, nil) } return nil, fmt.Errorf("unknown registry format: %s", cr.Format)