refactor to rename pull_request to hosting_service and apply SRP

2025-11-29 22:48:24 +02:00 · 2021-12-28 13:58:09 +11:00
parent f89747451a
commit 9ef65574db
13 changed files with 569 additions and 867 deletions
--- a/pkg/commands/hosting_service/definitions.go
+++ b/pkg/commands/hosting_service/definitions.go
@@ -0,0 +1,54 @@
+package hosting_service
+
+// if you want to make a custom regex for a given service feel free to test it out
+// at regoio.herokuapp.com
+var defaultUrlRegexStrings = []string{
+	`^(?:https?|ssh)://.*/(?P<owner>.*)/(?P<repo>.*?)(?:\.git)?$`,
+	`^git@.*:(?P<owner>.*)/(?P<repo>.*?)(?:\.git)?$`,
+}
+
+// we've got less type safety using go templates but this lends itself better to
+// users adding custom service definitions in their config
+var githubServiceDef = ServiceDefinition{
+	provider:                        "github",
+	pullRequestURLIntoDefaultBranch: "/compare/{{.From}}?expand=1",
+	pullRequestURLIntoTargetBranch:  "/compare/{{.To}}...{{.From}}?expand=1",
+	commitURL:                       "/commit/{{.CommitSha}}",
+	regexStrings:                    defaultUrlRegexStrings,
+}
+
+var bitbucketServiceDef = ServiceDefinition{
+	provider:                        "bitbucket",
+	pullRequestURLIntoDefaultBranch: "/pull-requests/new?source={{.From}}&t=1",
+	pullRequestURLIntoTargetBranch:  "/pull-requests/new?source={{.From}}&dest={{.To}}&t=1",
+	commitURL:                       "/commits/{{.CommitSha}}",
+	regexStrings:                    defaultUrlRegexStrings,
+}
+
+var gitLabServiceDef = ServiceDefinition{
+	provider:                        "gitlab",
+	pullRequestURLIntoDefaultBranch: "/merge_requests/new?merge_request[source_branch]={{.From}}",
+	pullRequestURLIntoTargetBranch:  "/merge_requests/new?merge_request[source_branch]={{.From}}&merge_request[target_branch]={{.To}}",
+	commitURL:                       "/commit/{{.CommitSha}}",
+	regexStrings:                    defaultUrlRegexStrings,
+}
+
+var serviceDefinitions = []ServiceDefinition{githubServiceDef, bitbucketServiceDef, gitLabServiceDef}
+
+var defaultServiceDomains = []ServiceDomain{
+	{
+		serviceDefinition: githubServiceDef,
+		gitDomain:         "github.com",
+		webDomain:         "github.com",
+	},
+	{
+		serviceDefinition: bitbucketServiceDef,
+		gitDomain:         "bitbucket.org",
+		webDomain:         "bitbucket.org",
+	},
+	{
+		serviceDefinition: gitLabServiceDef,
+		gitDomain:         "gitlab.com",
+		webDomain:         "gitlab.com",
+	},
+}
--- a/pkg/commands/hosting_service/hosting_service.go
+++ b/pkg/commands/hosting_service/hosting_service.go
@@ -0,0 +1,201 @@
+package hosting_service
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/go-errors/errors"
+	"github.com/jesseduffield/lazygit/pkg/i18n"
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/sirupsen/logrus"
+)
+
+// This package is for handling logic specific to a git hosting service like github, gitlab, bitbucket, etc.
+// Different git hosting services have different URL formats for when you want to open a PR or view a commit,
+// and this package's responsibility is to determine which service you're using based on the remote URL,
+// and then which URL you need for whatever use case you have.
+
+type HostingServiceMgr struct {
+	log       logrus.FieldLogger
+	tr        *i18n.TranslationSet
+	remoteURL string // e.g. https://github.com/jesseduffield/lazygit
+
+	// see https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#custom-pull-request-urls
+	configServiceDomains map[string]string
+}
+
+// NewHostingServiceMgr creates new instance of PullRequest
+func NewHostingServiceMgr(log logrus.FieldLogger, tr *i18n.TranslationSet, remoteURL string, configServiceDomains map[string]string) *HostingServiceMgr {
+	return &HostingServiceMgr{
+		log:                  log,
+		tr:                   tr,
+		remoteURL:            remoteURL,
+		configServiceDomains: configServiceDomains,
+	}
+}
+
+func (self *HostingServiceMgr) GetPullRequestURL(from string, to string) (string, error) {
+	gitService, err := self.getService()
+	if err != nil {
+		return "", err
+	}
+
+	if to == "" {
+		return gitService.getPullRequestURLIntoDefaultBranch(from), nil
+	} else {
+		return gitService.getPullRequestURLIntoTargetBranch(from, to), nil
+	}
+}
+
+func (self *HostingServiceMgr) GetCommitURL(commitSha string) (string, error) {
+	gitService, err := self.getService()
+	if err != nil {
+		return "", err
+	}
+
+	pullRequestURL := gitService.getCommitURL(commitSha)
+
+	return pullRequestURL, nil
+}
+
+func (self *HostingServiceMgr) getService() (*Service, error) {
+	serviceDomain, err := self.getServiceDomain(self.remoteURL)
+	if err != nil {
+		return nil, err
+	}
+
+	root, err := serviceDomain.getRootFromRemoteURL(self.remoteURL)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Service{
+		root:              root,
+		ServiceDefinition: serviceDomain.serviceDefinition,
+	}, nil
+}
+
+func (self *HostingServiceMgr) getServiceDomain(repoURL string) (*ServiceDomain, error) {
+	candidateServiceDomains := self.getCandidateServiceDomains()
+
+	for _, serviceDomain := range candidateServiceDomains {
+		// I feel like it makes more sense to see if the repo url contains the service domain's git domain,
+		// but I don't want to break anything by changing that right now.
+		if strings.Contains(repoURL, serviceDomain.serviceDefinition.provider) {
+			return &serviceDomain, nil
+		}
+	}
+
+	return nil, errors.New(self.tr.UnsupportedGitService)
+}
+
+func (self *HostingServiceMgr) getCandidateServiceDomains() []ServiceDomain {
+	serviceDefinitionByProvider := map[string]ServiceDefinition{}
+	for _, serviceDefinition := range serviceDefinitions {
+		serviceDefinitionByProvider[serviceDefinition.provider] = serviceDefinition
+	}
+
+	var serviceDomains = make([]ServiceDomain, len(defaultServiceDomains))
+	copy(serviceDomains, defaultServiceDomains)
+
+	if len(self.configServiceDomains) > 0 {
+		for gitDomain, typeAndDomain := range self.configServiceDomains {
+			splitData := strings.Split(typeAndDomain, ":")
+			if len(splitData) != 2 {
+				self.log.Errorf("Unexpected format for git service: '%s'. Expected something like 'github.com:github.com'", typeAndDomain)
+				continue
+			}
+
+			provider := splitData[0]
+			webDomain := splitData[1]
+
+			serviceDefinition, ok := serviceDefinitionByProvider[provider]
+			if !ok {
+				providerNames := []string{}
+				for _, serviceDefinition := range serviceDefinitions {
+					providerNames = append(providerNames, serviceDefinition.provider)
+				}
+				self.log.Errorf("Unknown git service type: '%s'. Expected one of %s", provider, strings.Join(providerNames, ", "))
+				continue
+			}
+
+			serviceDomains = append(serviceDomains, ServiceDomain{
+				gitDomain:         gitDomain,
+				webDomain:         webDomain,
+				serviceDefinition: serviceDefinition,
+			})
+		}
+	}
+
+	return serviceDomains
+}
+
+// a service domains pairs a service definition with the actual domain it's being served from.
+// Sometimes the git service is hosted in a custom domains so although it'll use say
+// the github service definition, it'll actually be served from e.g. my-custom-github.com
+type ServiceDomain struct {
+	gitDomain         string // the one that appears in the git remote url
+	webDomain         string // the one that appears in the web url
+	serviceDefinition ServiceDefinition
+}
+
+func (self ServiceDomain) getRootFromRemoteURL(repoURL string) (string, error) {
+	// we may want to make this more specific to the service in future e.g. if
+	// some new service comes along which has a different root url structure.
+	repoInfo, err := self.serviceDefinition.getRepoInfoFromURL(repoURL)
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("https://%s/%s/%s", self.webDomain, repoInfo.Owner, repoInfo.Repository), nil
+}
+
+// RepoInformation holds some basic information about the repo
+type RepoInformation struct {
+	Owner      string
+	Repository string
+}
+
+type ServiceDefinition struct {
+	provider                        string
+	pullRequestURLIntoDefaultBranch string
+	pullRequestURLIntoTargetBranch  string
+	commitURL                       string
+	regexStrings                    []string
+}
+
+func (self ServiceDefinition) getRepoInfoFromURL(url string) (*RepoInformation, error) {
+	for _, regexStr := range self.regexStrings {
+		re := regexp.MustCompile(regexStr)
+		matches := utils.FindNamedMatches(re, url)
+		if matches != nil {
+			return &RepoInformation{
+				Owner:      matches["owner"],
+				Repository: matches["repo"],
+			}, nil
+		}
+	}
+
+	return nil, errors.New("Failed to parse repo information from url")
+}
+
+type Service struct {
+	root string
+	ServiceDefinition
+}
+
+func (self *Service) getPullRequestURLIntoDefaultBranch(from string) string {
+	return self.resolveUrl(self.pullRequestURLIntoDefaultBranch, map[string]string{"From": from})
+}
+
+func (self *Service) getPullRequestURLIntoTargetBranch(from string, to string) string {
+	return self.resolveUrl(self.pullRequestURLIntoTargetBranch, map[string]string{"From": from, "To": to})
+}
+
+func (self *Service) getCommitURL(commitSha string) string {
+	return self.resolveUrl(self.commitURL, map[string]string{"CommitSha": commitSha})
+}
+
+func (self *Service) resolveUrl(templateString string, args map[string]string) string {
+	return self.root + utils.ResolvePlaceholderString(templateString, args)
+}
--- a/pkg/commands/hosting_service/hosting_service_test.go
+++ b/pkg/commands/hosting_service/hosting_service_test.go
@@ -0,0 +1,233 @@
+package hosting_service
+
+import (
+	"testing"
+
+	"github.com/jesseduffield/lazygit/pkg/i18n"
+	"github.com/jesseduffield/lazygit/pkg/test"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetRepoInfoFromURL(t *testing.T) {
+	type scenario struct {
+		serviceDefinition ServiceDefinition
+		testName          string
+		repoURL           string
+		test              func(*RepoInformation)
+	}
+
+	scenarios := []scenario{
+		{
+			githubServiceDef,
+			"Returns repository information for git remote url",
+			"git@github.com:petersmith/super_calculator",
+			func(repoInfo *RepoInformation) {
+				assert.EqualValues(t, repoInfo.Owner, "petersmith")
+				assert.EqualValues(t, repoInfo.Repository, "super_calculator")
+			},
+		},
+		{
+			githubServiceDef,
+			"Returns repository information for git remote url, trimming trailing '.git'",
+			"git@github.com:petersmith/super_calculator.git",
+			func(repoInfo *RepoInformation) {
+				assert.EqualValues(t, repoInfo.Owner, "petersmith")
+				assert.EqualValues(t, repoInfo.Repository, "super_calculator")
+			},
+		},
+		{
+			githubServiceDef,
+			"Returns repository information for ssh remote url",
+			"ssh://git@github.com/petersmith/super_calculator",
+			func(repoInfo *RepoInformation) {
+				assert.EqualValues(t, repoInfo.Owner, "petersmith")
+				assert.EqualValues(t, repoInfo.Repository, "super_calculator")
+			},
+		},
+		{
+			githubServiceDef,
+			"Returns repository information for http remote url",
+			"https://my_username@bitbucket.org/johndoe/social_network.git",
+			func(repoInfo *RepoInformation) {
+				assert.EqualValues(t, repoInfo.Owner, "johndoe")
+				assert.EqualValues(t, repoInfo.Repository, "social_network")
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.testName, func(t *testing.T) {
+			result, err := s.serviceDefinition.getRepoInfoFromURL(s.repoURL)
+			assert.NoError(t, err)
+			s.test(result)
+		})
+	}
+}
+
+func TestGetPullRequestURL(t *testing.T) {
+	type scenario struct {
+		testName             string
+		from                 string
+		to                   string
+		remoteUrl            string
+		configServiceDomains map[string]string
+		test                 func(url string, err error)
+		expectedLoggedErrors []string
+	}
+
+	scenarios := []scenario{
+		{
+			testName:  "Opens a link to new pull request on bitbucket",
+			from:      "feature/profile-page",
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on bitbucket with http remote url",
+			from:      "feature/events",
+			remoteUrl: "https://my_username@bitbucket.org/johndoe/social_network.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/events&t=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on github",
+			from:      "feature/sum-operation",
+			remoteUrl: "git@github.com:peter/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://github.com/peter/calculator/compare/feature/sum-operation?expand=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on bitbucket with specific target branch",
+			from:      "feature/profile-page/avatar",
+			to:        "feature/profile-page",
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page/avatar&dest=feature/profile-page&t=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on bitbucket with http remote url with specified target branch",
+			from:      "feature/remote-events",
+			to:        "feature/events",
+			remoteUrl: "https://my_username@bitbucket.org/johndoe/social_network.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/remote-events&dest=feature/events&t=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on github with specific target branch",
+			from:      "feature/sum-operation",
+			to:        "feature/operations",
+			remoteUrl: "git@github.com:peter/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://github.com/peter/calculator/compare/feature/operations...feature/sum-operation?expand=1", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on gitlab",
+			from:      "feature/ui",
+			remoteUrl: "git@gitlab.com:peter/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/ui", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on gitlab in nested groups",
+			from:      "feature/ui",
+			remoteUrl: "git@gitlab.com:peter/public/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/ui", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on gitlab with specific target branch",
+			from:      "feature/commit-ui",
+			to:        "epic/ui",
+			remoteUrl: "git@gitlab.com:peter/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/commit-ui&merge_request[target_branch]=epic/ui", url)
+			},
+		},
+		{
+			testName:  "Opens a link to new pull request on gitlab with specific target branch in nested groups",
+			from:      "feature/commit-ui",
+			to:        "epic/ui",
+			remoteUrl: "git@gitlab.com:peter/public/calculator.git",
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://gitlab.com/peter/public/calculator/merge_requests/new?merge_request[source_branch]=feature/commit-ui&merge_request[target_branch]=epic/ui", url)
+			},
+		},
+		{
+			testName:  "Throws an error if git service is unsupported",
+			from:      "feature/divide-operation",
+			remoteUrl: "git@something.com:peter/calculator.git",
+			test: func(url string, err error) {
+				assert.EqualError(t, err, "Unsupported git service")
+			},
+		},
+		{
+			testName:  "Does not log error when config service domains are valid",
+			from:      "feature/profile-page",
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			configServiceDomains: map[string]string{
+				// valid configuration for a custom service URL
+				"git.work.com": "gitlab:code.work.com",
+			},
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1", url)
+			},
+			expectedLoggedErrors: nil,
+		},
+		{
+			testName:  "Logs error when config service domain is malformed",
+			from:      "feature/profile-page",
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			configServiceDomains: map[string]string{
+				"noservice.work.com": "noservice.work.com",
+			},
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1", url)
+			},
+			expectedLoggedErrors: []string{"Unexpected format for git service: 'noservice.work.com'. Expected something like 'github.com:github.com'"},
+		},
+		{
+			testName:  "Logs error when config service domain uses unknown provider",
+			from:      "feature/profile-page",
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			configServiceDomains: map[string]string{
+				"invalid.work.com": "noservice:invalid.work.com",
+			},
+			test: func(url string, err error) {
+				assert.NoError(t, err)
+				assert.Equal(t, "https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1", url)
+			},
+			expectedLoggedErrors: []string{"Unknown git service type: 'noservice'. Expected one of github, bitbucket, gitlab"},
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.testName, func(t *testing.T) {
+			tr := i18n.EnglishTranslationSet()
+			log := &test.FakeFieldLogger{}
+			hostingServiceMgr := NewHostingServiceMgr(log, &tr, s.remoteUrl, s.configServiceDomains)
+			s.test(hostingServiceMgr.GetPullRequestURL(s.from, s.to))
+			log.AssertErrors(t, s.expectedLoggedErrors)
+		})
+	}
+}