From a530b1506ceb54a962a08129b9e043bdfa7196c0 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Fri, 22 May 2020 15:53:44 +0200 Subject: [PATCH] Refactor URL parsing so it's not being called twice --- pkg/repo.go | 10 ++++---- pkg/repo_test.go | 5 +++- pkg/status_test.go | 9 ++++++-- pkg/url.go | 57 +++++++++++++++++++++++----------------------- pkg/url_test.go | 15 +++++++----- 5 files changed, 52 insertions(+), 44 deletions(-) diff --git a/pkg/repo.go b/pkg/repo.go index 1466488..96c33f2 100644 --- a/pkg/repo.go +++ b/pkg/repo.go @@ -1,6 +1,7 @@ package pkg import ( + urlpkg "net/url" "os" "path" @@ -14,11 +15,8 @@ type Repo struct { Status *RepoStatus } -func CloneRepo(url string, repoRoot string) (path string, err error) { - repoPath, err := URLToPath(url) - if err != nil { - return path, err - } +func CloneRepo(url *urlpkg.URL, repoRoot string) (path string, err error) { + repoPath := URLToPath(url) path, err = MakeDir(repoRoot, repoPath) if err != nil { @@ -33,7 +31,7 @@ func CloneRepo(url string, repoRoot string) (path string, err error) { RemoteCreateCallback: nil, } - _, err = git.Clone(url, path, options) + _, err = git.Clone(url.String(), path, options) if err != nil { return path, errors.Wrap(err, "Failed cloning repo") } diff --git a/pkg/repo_test.go b/pkg/repo_test.go index 0bc48da..64aafda 100644 --- a/pkg/repo_test.go +++ b/pkg/repo_test.go @@ -1,6 +1,7 @@ package pkg import ( + urlpkg "net/url" "os" "testing" ) @@ -14,7 +15,9 @@ func TestFetch(t *testing.T) { // Clone the origin repo repoRoot := newTempDir(t) - path, err := CloneRepo(origin.Path(), repoRoot) + url, err := urlpkg.Parse(origin.Path()) + checkFatal(t, err) + path, err := CloneRepo(url, repoRoot) checkFatal(t, err) // Open cloned repo and load its status diff --git a/pkg/status_test.go b/pkg/status_test.go index 0875735..508fc9a 100644 --- a/pkg/status_test.go +++ b/pkg/status_test.go @@ -1,6 +1,7 @@ package pkg import ( + urlpkg "net/url" "reflect" "testing" @@ -159,7 +160,9 @@ func TestStatusCloned(t *testing.T) { origin := newTestRepo(t) repoRoot := newTempDir(t) - path, err := CloneRepo(origin.Path(), repoRoot) + url, err := urlpkg.Parse(origin.Path()) + checkFatal(t, err) + path, err := CloneRepo(url, repoRoot) checkFatal(t, err) repo, err := OpenRepo(path) checkFatal(t, err) @@ -213,7 +216,9 @@ func TestBranchCloned(t *testing.T) { createBranch(t, origin, "branch") repoRoot := newTempDir(t) - path, err := CloneRepo(origin.Path(), repoRoot) + url, err := urlpkg.Parse(origin.Path()) + checkFatal(t, err) + path, err := CloneRepo(url, repoRoot) checkFatal(t, err) repo, err := OpenRepo(path) checkFatal(t, err) diff --git a/pkg/url.go b/pkg/url.go index ffee682..71a69fc 100644 --- a/pkg/url.go +++ b/pkg/url.go @@ -13,12 +13,35 @@ import ( // See: https://golang.org/src/cmd/go/internal/get/vcs.go var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+)@([a-zA-Z0-9._-]+):(.*)$`) -func URLToPath(rawurl string) (repoPath string, err error) { - url, err := parseURL(rawurl) - if err != nil { - return "", err +func ParseURL(rawURL string) (url *urlpkg.URL, err error) { + // If rawURL matches SCP-like syntax, convert it into a URL. + // eg, "git@github.com:user/repo" becomes "ssh://git@github.com/user/repo". + if m := scpSyntax.FindStringSubmatch(rawURL); m != nil { + url = &urlpkg.URL{ + Scheme: "ssh", + User: urlpkg.User(m[1]), + Host: m[2], + Path: m[3], + } + } else { + url, err = urlpkg.Parse(rawURL) + if err != nil { + return nil, errors.Wrap(err, "Failed parsing URL") + } } + if url.Host == "" && url.Path == "" { + return nil, errors.New("Parsed URL is empty") + } + + if url.Scheme == "" || url.Scheme == "git+ssh" { + url.Scheme = "ssh" + } + + return url, nil +} + +func URLToPath(url *urlpkg.URL) (repoPath string) { // remove port numbers from host repoHost := strings.Split(url.Host, ":")[0] @@ -29,29 +52,5 @@ func URLToPath(rawurl string) (repoPath string, err error) { // remove tilde (~) char from username repoPath = strings.ReplaceAll(repoPath, "~", "") - return repoPath, nil -} - -func parseURL(rawurl string) (url *urlpkg.URL, err error) { - // If rawurl matches SCP-like syntax, convert it into a URL. - // eg, "git@github.com:user/repo" becomes "ssh://git@github.com/user/repo". - if m := scpSyntax.FindStringSubmatch(rawurl); m != nil { - url = &urlpkg.URL{ - Scheme: "ssh", - User: urlpkg.User(m[1]), - Host: m[2], - Path: m[3], - } - } else { - url, err = urlpkg.Parse(rawurl) - if err != nil { - return nil, errors.Wrap(err, "Failed parsing URL") - } - } - - if url.Host == "" && url.Path == "" { - return nil, errors.New("Parsed URL is empty") - } - - return url, nil + return repoPath } diff --git a/pkg/url_test.go b/pkg/url_test.go index 61c53c1..adf29e4 100644 --- a/pkg/url_test.go +++ b/pkg/url_test.go @@ -1,6 +1,8 @@ package pkg -import "testing" +import ( + "testing" +) // Following URLs are considered valid according to https://git-scm.com/docs/git-clone#_git_urls: // ssh://[user@]host.xz[:port]/path/to/repo.git @@ -47,11 +49,13 @@ func TestURLParse(t *testing.T) { } for _, test := range tests { - got, err := URLToPath(test.in) + url, err := ParseURL(test.in) if err != nil { t.Errorf("Error parsing URL: %+v", err) } + got := URLToPath(url) + if got != test.want { t.Errorf("Wrong result of parsing URL: %s, got: %s; want: %s", test.in, got, test.want) } @@ -68,11 +72,10 @@ func TestInvalidURLParse(t *testing.T) { //"git@github.com:1234:grdl/git-get.git", } - for _, url := range invalidURLs { - got, err := URLToPath(url) + for _, in := range invalidURLs { + got, err := ParseURL(in) if err == nil { - t.Errorf("Wrong result of parsing invalid URL: %s, got: %s, want: error", url, got) + t.Errorf("Wrong result of parsing invalid URL: %s, got: %s, want: error", in, got) } } - }