diff --git a/src/GitHub.App/GlobalSuppressions.cs b/src/GitHub.App/GlobalSuppressions.cs index f635f8ea1d..d3dcbf45d6 100644 --- a/src/GitHub.App/GlobalSuppressions.cs +++ b/src/GitHub.App/GlobalSuppressions.cs @@ -6,6 +6,8 @@ [assembly: SuppressMessage("Microsoft.Naming", "CA1703:ResourceStringsShouldBeSpelledCorrectly", MessageId = "Git", Scope = "resource", Target = "GitHub.App.Resources.resources")] [assembly: SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object,System.Object)", Scope = "member", Target = "GitHub.Services.PullRequestService.#CreateTempFile(System.String,System.String,System.String)")] [assembly: SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object,System.Object)", Scope = "member", Target = "GitHub.Services.PullRequestService.#CreateTempFile(System.String,System.String,System.String,System.Text.Encoding)")] +[assembly: SuppressMessage("Reliability", "CA2007:Do not directly await a Task", Justification = "We always need to think about what thread we're running on")] + // This file is used by Code Analysis to maintain SuppressMessage // attributes that are applied to this project. // Project-level suppressions either have no target or are given diff --git a/src/GitHub.App/SampleData/GitServiceDesigner.cs b/src/GitHub.App/SampleData/GitServiceDesigner.cs index de81803439..014b79e3fa 100644 --- a/src/GitHub.App/SampleData/GitServiceDesigner.cs +++ b/src/GitHub.App/SampleData/GitServiceDesigner.cs @@ -1,4 +1,5 @@ using System.Threading.Tasks; +using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using LibGit2Sharp; @@ -7,6 +8,9 @@ namespace GitHub.SampleData { class GitServiceDesigner : IGitService { + public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath) => null; + public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model) => null; + public void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel) { } public Task GetLatestPushedSha(string path, string remote = "origin") => Task.FromResult(null); public UriString GetRemoteUri(IRepository repo, string remote = "origin") => null; public IRepository GetRepository(string path) => null; diff --git a/src/GitHub.App/SampleData/PullRequestCreationViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestCreationViewModelDesigner.cs index 99f05b86f9..816e814b10 100644 --- a/src/GitHub.App/SampleData/PullRequestCreationViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestCreationViewModelDesigner.cs @@ -15,15 +15,21 @@ public class PullRequestCreationViewModelDesigner : PanePageViewModelBase, IPull { public PullRequestCreationViewModelDesigner() { + var repositoryModel = new LocalRepositoryModel + { + Name = "repo", + CloneUrl = "http://github.com/user/repo" + }; + Branches = new List { - new BranchModel("master", new LocalRepositoryModel("http://github.com/user/repo", new GitServiceDesigner())), - new BranchModel("don/stub-ui", new LocalRepositoryModel("http://github.com/user/repo", new GitServiceDesigner())), - new BranchModel("feature/pr/views", new LocalRepositoryModel("http://github.com/user/repo", new GitServiceDesigner())), - new BranchModel("release-1.0.17.0", new LocalRepositoryModel("http://github.com/user/repo", new GitServiceDesigner())), + new BranchModel("master", repositoryModel), + new BranchModel("don/stub-ui", repositoryModel), + new BranchModel("feature/pr/views", repositoryModel), + new BranchModel("release-1.0.17.0", repositoryModel), }.AsReadOnly(); - TargetBranch = new BranchModel("master", new LocalRepositoryModel("http://github.com/user/repo", new GitServiceDesigner())); + TargetBranch = new BranchModel("master", repositoryModel); SourceBranch = Branches[2]; SelectedAssignee = "Haacked (Phil Haack)"; diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 9b8015cb43..3a0f16c2bd 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -231,8 +231,8 @@ public async Task> ReadPullRequests( if (hasCheckRuns) { checksHasFailure = checkRuns - .Any(model => model.Conclusion.HasValue - && (model.Conclusion.Value == CheckConclusionState.Failure + .Any(model => model.Conclusion.HasValue + && (model.Conclusion.Value == CheckConclusionState.Failure || model.Conclusion.Value == CheckConclusionState.ActionRequired)); if (!checksHasFailure) @@ -718,7 +718,7 @@ public IObservable SwitchToBranch(ILocalRepositoryModel repository, PullRe }); } - public IObservable> GetPullRequestForCurrentBranch(ILocalRepositoryModel repository) + public IObservable<(string owner, int number)> GetPullRequestForCurrentBranch(ILocalRepositoryModel repository) { return Observable.Defer(async () => { @@ -912,7 +912,7 @@ async Task IsBranchMarkedAsPullRequest(IRepository repo, string branchName { var prConfigKey = $"branch.{branchName}.{SettingGHfVSPullRequest}"; var value = ParseGHfVSConfigKeyValue(await gitClient.GetConfig(repo, prConfigKey)); - return value != null && + return value != default && value.Item1 == pullRequest.BaseRepositoryOwner && value.Item2 == pullRequest.Number; } @@ -983,7 +983,7 @@ static string BuildGHfVSConfigKeyValue(string owner, int number) return owner + '#' + number.ToString(CultureInfo.InvariantCulture); } - static Tuple ParseGHfVSConfigKeyValue(string value) + static (string owner, int number) ParseGHfVSConfigKeyValue(string value) { if (value != null) { @@ -996,12 +996,12 @@ static Tuple ParseGHfVSConfigKeyValue(string value) if (int.TryParse(value.Substring(separator + 1), NumberStyles.None, CultureInfo.InvariantCulture, out number)) { - return Tuple.Create(owner, number); + return (owner, number); } } } - return null; + return default; } class ListItemAdapter : PullRequestListItemModel diff --git a/src/GitHub.App/Services/TeamExplorerContext.cs b/src/GitHub.App/Services/TeamExplorerContext.cs index 6d443e4dfc..ecbaddf742 100644 --- a/src/GitHub.App/Services/TeamExplorerContext.cs +++ b/src/GitHub.App/Services/TeamExplorerContext.cs @@ -40,10 +40,7 @@ public class TeamExplorerContext : ITeamExplorerContext string solutionPath; string repositoryPath; UriString cloneUrl; - string branchName; - string headSha; - string trackedSha; - Tuple pullRequest; + (string owner, int number) pullRequest; ILocalRepositoryModel repositoryModel; JoinableTask refreshJoinableTask; @@ -113,10 +110,7 @@ async Task RefreshAsync() { var newRepositoryPath = repo?.LocalPath; var newCloneUrl = repo?.CloneUrl; - var newBranchName = repo?.CurrentBranch?.Name; - var newHeadSha = repo?.CurrentBranch?.Sha; - var newTrackedSha = repo?.CurrentBranch?.TrackedSha; - var newPullRequest = repo != null ? await pullRequestService.GetPullRequestForCurrentBranch(repo) : null; + var newPullRequest = repo != null ? await pullRequestService.GetPullRequestForCurrentBranch(repo) : default; if (newRepositoryPath != repositoryPath) { @@ -128,33 +122,20 @@ async Task RefreshAsync() log.Debug("ActiveRepository changed to {CloneUrl} @ {Path}", repo?.CloneUrl, newRepositoryPath); ActiveRepository = repo; } - else if (newBranchName != branchName) - { - log.Debug("Fire StatusChanged event when BranchName changes for ActiveRepository"); - StatusChanged?.Invoke(this, EventArgs.Empty); - } - else if (newHeadSha != headSha) - { - log.Debug("Fire StatusChanged event when HeadSha changes for ActiveRepository"); - StatusChanged?.Invoke(this, EventArgs.Empty); - } - else if (newTrackedSha != trackedSha) + else if (newPullRequest != pullRequest) { - log.Debug("Fire StatusChanged event when TrackedSha changes for ActiveRepository"); + log.Debug("Fire StatusChanged event when PullRequest changes for ActiveRepository"); StatusChanged?.Invoke(this, EventArgs.Empty); } - else if (newPullRequest != pullRequest) + else if (newRepositoryPath != null) { - log.Debug("Fire StatusChanged event when PullRequest changes for ActiveRepository"); + log.Debug("Fire StatusChanged event when on a repository and anything changes"); StatusChanged?.Invoke(this, EventArgs.Empty); } repositoryPath = newRepositoryPath; cloneUrl = newCloneUrl; - branchName = newBranchName; - headSha = newHeadSha; solutionPath = newSolutionPath; - trackedSha = newTrackedSha; pullRequest = newPullRequest; } } diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs index f72bc605fe..764fc1ec10 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs @@ -38,6 +38,7 @@ public class PullRequestCreationViewModel : PanePageViewModelBase, IPullRequestC readonly IPullRequestService service; readonly IModelServiceFactory modelServiceFactory; readonly IMessageDraftStore draftStore; + readonly IGitService gitService; readonly IScheduler timerScheduler; readonly CompositeDisposable disposables = new CompositeDisposable(); ILocalRepositoryModel activeLocalRepo; @@ -49,8 +50,9 @@ public PullRequestCreationViewModel( IModelServiceFactory modelServiceFactory, IPullRequestService service, INotificationService notifications, - IMessageDraftStore draftStore) - : this(modelServiceFactory, service, notifications, draftStore, DefaultScheduler.Instance) + IMessageDraftStore draftStore, + IGitService gitService) + : this(modelServiceFactory, service, notifications, draftStore, gitService, DefaultScheduler.Instance) { } @@ -59,17 +61,20 @@ public PullRequestCreationViewModel( IPullRequestService service, INotificationService notifications, IMessageDraftStore draftStore, + IGitService gitService, IScheduler timerScheduler) { Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); Guard.ArgumentNotNull(service, nameof(service)); Guard.ArgumentNotNull(notifications, nameof(notifications)); Guard.ArgumentNotNull(draftStore, nameof(draftStore)); + Guard.ArgumentNotNull(gitService, nameof(gitService)); Guard.ArgumentNotNull(timerScheduler, nameof(timerScheduler)); this.service = service; this.modelServiceFactory = modelServiceFactory; this.draftStore = draftStore; + this.gitService = gitService; this.timerScheduler = timerScheduler; this.WhenAnyValue(x => x.Branches) @@ -137,7 +142,7 @@ public async Task InitializeAsync(ILocalRepositoryModel repository, IConnection { modelService = await modelServiceFactory.CreateAsync(connection); activeLocalRepo = repository; - SourceBranch = repository.CurrentBranch; + SourceBranch = gitService.CreateCurrentBranchModel(repository); var obs = modelService.ApiClient.GetRepository(repository.Owner, repository.Name) .Select(r => new RemoteRepositoryModel(r)) @@ -207,7 +212,7 @@ async Task LoadInitialState(string draftKey) void LoadDescriptionFromCommits() { - SourceBranch = activeLocalRepo.CurrentBranch; + SourceBranch = gitService.CreateCurrentBranchModel(activeLocalRepo); var uniqueCommits = this.WhenAnyValue( x => x.SourceBranch, diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs index 32d4dbe2d7..fc1f6dd703 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs @@ -39,6 +39,7 @@ public sealed class PullRequestDetailViewModel : PanePageViewModelBase, IPullReq readonly ITeamExplorerContext teamExplorerContext; readonly ISyncSubmodulesCommand syncSubmodulesCommand; readonly IViewViewModelFactory viewViewModelFactory; + readonly IGitService gitService; IModelService modelService; PullRequestDetailModel model; IActorViewModel author; @@ -77,7 +78,8 @@ public PullRequestDetailViewModel( ITeamExplorerContext teamExplorerContext, IPullRequestFilesViewModel files, ISyncSubmodulesCommand syncSubmodulesCommand, - IViewViewModelFactory viewViewModelFactory) + IViewViewModelFactory viewViewModelFactory, + IGitService gitService) { Guard.ArgumentNotNull(pullRequestsService, nameof(pullRequestsService)); Guard.ArgumentNotNull(sessionManager, nameof(sessionManager)); @@ -86,6 +88,7 @@ public PullRequestDetailViewModel( Guard.ArgumentNotNull(teamExplorerContext, nameof(teamExplorerContext)); Guard.ArgumentNotNull(syncSubmodulesCommand, nameof(syncSubmodulesCommand)); Guard.ArgumentNotNull(viewViewModelFactory, nameof(viewViewModelFactory)); + Guard.ArgumentNotNull(gitService, nameof(gitService)); this.pullRequestsService = pullRequestsService; this.sessionManager = sessionManager; @@ -94,6 +97,7 @@ public PullRequestDetailViewModel( this.teamExplorerContext = teamExplorerContext; this.syncSubmodulesCommand = syncSubmodulesCommand; this.viewViewModelFactory = viewViewModelFactory; + this.gitService = gitService; Files = files; Checkout = ReactiveCommand.CreateFromObservable( @@ -401,7 +405,8 @@ public async Task Load(PullRequestDetailModel pullRequest) var localBranches = await pullRequestsService.GetLocalBranches(LocalRepository, pullRequest).ToList(); - IsCheckedOut = localBranches.Contains(LocalRepository.CurrentBranch); + var currentBranch = gitService.CreateCurrentBranchModel(LocalRepository); + IsCheckedOut = localBranches.Contains(currentBranch); if (IsCheckedOut) { diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index 48837ce2d6..0d34471afb 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -173,7 +173,7 @@ IObservable CreatePullRequest(IModelService modelService, /// This method does not do an API request - it simply checks the mark left in the git /// config by . /// - IObservable> GetPullRequestForCurrentBranch(ILocalRepositoryModel repository); + IObservable<(string owner, int number)> GetPullRequestForCurrentBranch(ILocalRepositoryModel repository); /// /// Gets the encoding for the specified local file. diff --git a/src/GitHub.Exports/Models/BranchModel.cs b/src/GitHub.Exports/Models/BranchModel.cs index 9a02091871..b61bc61853 100644 --- a/src/GitHub.Exports/Models/BranchModel.cs +++ b/src/GitHub.Exports/Models/BranchModel.cs @@ -32,7 +32,8 @@ public BranchModel(LibGit2Sharp.Branch branch, IRepositoryModel repo, IGitServic Extensions.Guard.ArgumentNotNull(repo, nameof(repo)); Name = DisplayName = branch.FriendlyName; #pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. - Repository = branch.IsRemote ? new LocalRepositoryModel(branch.Remote.Url, gitService) : repo; + // NOTE: This method expects a localPath not a URL! + Repository = branch.IsRemote ? gitService.CreateLocalRepositoryModel(branch.Remote.Url) : repo; #pragma warning restore 0618 IsTracking = branch.IsTracking; Sha = branch.Tip?.Sha; diff --git a/src/GitHub.Exports/Models/ILocalRepositoryModel.cs b/src/GitHub.Exports/Models/ILocalRepositoryModel.cs index 1295599b6a..781180f4f2 100644 --- a/src/GitHub.Exports/Models/ILocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/ILocalRepositoryModel.cs @@ -1,7 +1,4 @@ -using GitHub.Exports; -using GitHub.Primitives; -using System.ComponentModel; -using System.Threading.Tasks; +using System.ComponentModel; namespace GitHub.Models { @@ -14,26 +11,5 @@ public interface ILocalRepositoryModel : IRepositoryModel, INotifyPropertyChange /// Gets the path to the repository on the filesystem. /// string LocalPath { get; } - - /// - /// Gets the current branch. - /// - IBranch CurrentBranch { get; } - - /// - /// Updates the url information based on the local path - /// - void Refresh(); - - /// - /// Generates a http(s) url to the repository in the remote server, optionally - /// pointing to a specific file and specific line range in it. - /// - /// The type of repository link to create - /// The file to generate an url to. Optional. - /// A specific line, or (if specifying the as well) the start of a range - /// The end of a line range on the specified file. - /// An UriString with the generated url, or null if the repository has no remote server configured or if it can't be found locally - Task GenerateUrl(LinkType linkType, string path = null, int startLine = -1, int endLine = -1); } } diff --git a/src/GitHub.Exports/Models/ILocalRepositoryModelFactory.cs b/src/GitHub.Exports/Models/ILocalRepositoryModelFactory.cs deleted file mode 100644 index 3c8dfed0ce..0000000000 --- a/src/GitHub.Exports/Models/ILocalRepositoryModelFactory.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace GitHub.Models -{ - /// - /// A factory for objects. - /// - public interface ILocalRepositoryModelFactory - { - /// - /// Construct a new . - /// - /// The local path for the repository. - /// A new repository model. - ILocalRepositoryModel Create(string localPath); - } -} diff --git a/src/GitHub.Exports/Models/IRepositoryModel.cs b/src/GitHub.Exports/Models/IRepositoryModel.cs index c3c1d577ae..084bf96fe4 100644 --- a/src/GitHub.Exports/Models/IRepositoryModel.cs +++ b/src/GitHub.Exports/Models/IRepositoryModel.cs @@ -17,7 +17,7 @@ public interface IRepositoryModel /// /// Gets the repository clone URL. /// - UriString CloneUrl { get; } + UriString CloneUrl { get; set; } /// /// Gets the name of the owner of the repository, taken from the clone URL. diff --git a/src/GitHub.Exports/Models/LocalRepositoryModel.cs b/src/GitHub.Exports/Models/LocalRepositoryModel.cs index fe07ef5ab8..6e9e1e521e 100644 --- a/src/GitHub.Exports/Models/LocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/LocalRepositoryModel.cs @@ -1,14 +1,6 @@ using System; using System.Diagnostics; using System.Globalization; -using System.IO; -using System.Linq; -using GitHub.Primitives; -using GitHub.UI; -using GitHub.Exports; -using GitHub.Services; -using GitHub.Extensions; -using System.Threading.Tasks; namespace GitHub.Models { @@ -18,174 +10,12 @@ namespace GitHub.Models [DebuggerDisplay("{DebuggerDisplay,nq}")] public class LocalRepositoryModel : RepositoryModel, ILocalRepositoryModel, IEquatable { - readonly IGitService gitService; - - /// - /// Initializes a new instance of the class. - /// - /// The repository name. - /// The repository's clone URL. - /// The repository's local path. - /// The service used to refresh the repository's URL. - public LocalRepositoryModel(string name, UriString cloneUrl, string localPath, IGitService gitService) - : base(name, cloneUrl) - { - Guard.ArgumentNotEmptyString(localPath, nameof(localPath)); - Guard.ArgumentNotNull(gitService, nameof(gitService)); - - this.gitService = gitService; - LocalPath = localPath; - Icon = Octicon.repo; - } - - /// - /// Initializes a new instance of the class. - /// - /// The repository's local path. - /// The service used to find the repository's URL. - public LocalRepositoryModel(string path, IGitService gitService) - : base(path, gitService) - { - Guard.ArgumentNotNull(gitService, nameof(gitService)); - - this.gitService = gitService; - LocalPath = path; - Icon = Octicon.repo; - } - - /// - /// Updates the clone URL from the local repository. - /// - public void Refresh() - { - if (LocalPath == null) - return; - CloneUrl = gitService.GetUri(LocalPath); - } - - /// - /// Generates a http(s) url to the repository in the remote server, optionally - /// pointing to a specific file and specific line range in it. - /// - /// Type of link to generate - /// The file to generate an url to. Optional. - /// A specific line, or (if specifying the as well) the start of a range - /// The end of a line range on the specified file. - /// An UriString with the generated url, or null if the repository has no remote server configured or if it can't be found locally - public async Task GenerateUrl(LinkType linkType, string path = null, int startLine = -1, int endLine = -1) - { - if (CloneUrl == null) - return null; - - var sha = await gitService.GetLatestPushedSha(path ?? LocalPath); - // this also incidentally checks whether the repo has a valid LocalPath - if (String.IsNullOrEmpty(sha)) - return CloneUrl.ToRepositoryUrl().AbsoluteUri; - - if (path != null && Path.IsPathRooted(path)) - { - // if the path root doesn't match the repository local path, then ignore it - if (!path.StartsWith(LocalPath, StringComparison.OrdinalIgnoreCase)) - { - Debug.Assert(false, String.Format(CultureInfo.CurrentCulture, "GenerateUrl: path {0} doesn't match repository {1}", path, LocalPath)); - path = null; - } - else - path = path.Substring(LocalPath.Length + 1); - } - - if (startLine > 0 && endLine > 0 && startLine > endLine) - { - // if startLine is greater than endLine and both are set, swap them - var temp = startLine; - startLine = endLine; - endLine = temp; - } - - if (startLine == endLine) - { - // if startLine is the same as endLine don't generate a range link - endLine = -1; - } - - return new UriString(GenerateUrl(linkType, CloneUrl.ToRepositoryUrl().AbsoluteUri, sha, path, startLine, endLine)); - } - - const string CommitFormat = "{0}/commit/{1}"; - const string BlobFormat = "{0}/blob/{1}/{2}"; - const string BlameFormat = "{0}/blame/{1}/{2}"; - const string StartLineFormat = "{0}#L{1}"; - const string EndLineFormat = "{0}-L{1}"; - static string GenerateUrl(LinkType linkType, string basePath, string sha, string path, int startLine = -1, int endLine = -1) - { - if (sha == null) - return basePath; - - if (String.IsNullOrEmpty(path)) - return String.Format(CultureInfo.InvariantCulture, CommitFormat, basePath, sha); - - var ret = String.Format(CultureInfo.InvariantCulture, GetLinkFormat(linkType), basePath, sha, path.Replace(@"\", "/")); - - if (startLine < 0) - return ret; - ret = String.Format(CultureInfo.InvariantCulture, StartLineFormat, ret, startLine); - if (endLine < 0) - return ret; - return String.Format(CultureInfo.InvariantCulture, EndLineFormat, ret, endLine); - } - - /// - /// Selects the proper format for the link type, defaults to the blob url when link type is not selected. - /// - /// Type of link to generate - /// The string format of the selected link type - static string GetLinkFormat(LinkType linkType) - { - switch (linkType) - { - case LinkType.Blame: - return BlameFormat; - - case LinkType.Blob: - return BlobFormat; - - default: - return BlobFormat; - } - } - /// /// Gets the local path of the repository. /// - public string LocalPath { get; } - - /// - /// Gets the head SHA of the repository. - /// - public string HeadSha - { - get - { - using (var repo = gitService.GetRepository(LocalPath)) - { - return repo?.Commits.FirstOrDefault()?.Sha ?? string.Empty; - } - } - } - - /// - /// Gets the current branch of the repository. - /// - public IBranch CurrentBranch + public string LocalPath { - get - { - // BranchModel doesn't keep a reference to Repository - using (var repo = gitService.GetRepository(LocalPath)) - { - return new BranchModel(repo?.Head, this, gitService); - } - } + get; set; } /// @@ -211,13 +41,13 @@ public bool Equals(LocalRepositoryModel other) if (ReferenceEquals(this, other)) return true; return other != null && - String.Equals(Name, other.Name) && - String.Equals(Owner, other.Owner) && - String.Equals(CloneUrl, other.CloneUrl) && - String.Equals(LocalPath?.TrimEnd('\\'), other.LocalPath?.TrimEnd('\\'), StringComparison.CurrentCultureIgnoreCase); + string.Equals(Name, other.Name) && + string.Equals(Owner, other.Owner) && + string.Equals(CloneUrl, other.CloneUrl) && + string.Equals(LocalPath?.TrimEnd('\\'), other.LocalPath?.TrimEnd('\\'), StringComparison.CurrentCultureIgnoreCase); } - internal string DebuggerDisplay => String.Format( + internal string DebuggerDisplay => string.Format( CultureInfo.InvariantCulture, "{4}\tOwner: {0} Name: {1} CloneUrl: {2} LocalPath: {3}", Owner, diff --git a/src/GitHub.Exports/Models/RepositoryModel.cs b/src/GitHub.Exports/Models/RepositoryModel.cs index 3ec99829b5..fe3976de98 100644 --- a/src/GitHub.Exports/Models/RepositoryModel.cs +++ b/src/GitHub.Exports/Models/RepositoryModel.cs @@ -1,9 +1,7 @@ using System; using System.Diagnostics; -using System.IO; using GitHub.Extensions; using GitHub.Primitives; -using GitHub.Services; using GitHub.UI; namespace GitHub.Models @@ -33,30 +31,14 @@ public RepositoryModel( CloneUrl = cloneUrl; } - /// - /// Initializes a new instance of the class. - /// - /// - /// The path to the local repository from which repository name and clone URL will be - /// extracted. - /// - /// The service used to find the repository's and . - protected RepositoryModel(string path, IGitService gitService) + protected RepositoryModel() { - Guard.ArgumentNotNull(path, nameof(path)); - - var dir = new DirectoryInfo(path); - if (!dir.Exists) - throw new ArgumentException("Path does not exist", nameof(path)); - var uri = gitService.GetUri(path); - Name = uri?.RepositoryName ?? dir.Name; - CloneUrl = gitService.GetUri(path); } /// /// Gets the name of the repository. /// - public string Name { get; } + public string Name { get; set; } /// /// Gets the repository clone URL. @@ -64,7 +46,7 @@ protected RepositoryModel(string path, IGitService gitService) public UriString CloneUrl { get { return cloneUrl; } - protected set + set { if (cloneUrl != value) { @@ -85,7 +67,7 @@ protected set public Octicon Icon { get { return icon; } - protected set + set { if (icon != value) { diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs index a02700f972..053821dfce 100644 --- a/src/GitHub.Exports/Services/GitService.cs +++ b/src/GitHub.Exports/Services/GitService.cs @@ -1,11 +1,13 @@ -using System.ComponentModel.Composition; -using GitHub.Primitives; -using LibGit2Sharp; -using System; +using System; +using System.IO; +using System.Linq; using System.Threading.Tasks; +using System.ComponentModel.Composition; +using GitHub.UI; using GitHub.Models; -using System.Linq; +using GitHub.Primitives; using GitHub.Extensions; +using LibGit2Sharp; namespace GitHub.Services { @@ -21,6 +23,63 @@ public GitService(IRepositoryFacade repositoryFacade) this.repositoryFacade = repositoryFacade; } + /// + /// Initializes a new instance of the class. + /// + /// The repository's local path. + /// A repository model. + public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath) + { + Guard.ArgumentNotNull(localPath, nameof(localPath)); + + var dir = new DirectoryInfo(localPath); + if (!dir.Exists) + { + throw new ArgumentException("Path does not exist", nameof(localPath)); + } + + var cloneUrl = GetUri(localPath); + var name = cloneUrl?.RepositoryName ?? dir.Name; + + var model = new LocalRepositoryModel + { + LocalPath = localPath, + CloneUrl = cloneUrl, + Name = name, + Icon = Octicon.repo + }; + + return model; + } + + public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model) + { + var localPath = model.LocalPath; + using (var repo = GetRepository(localPath)) + { + var headBranch = repo?.Head; + if (headBranch == null) + { + return null; + } + + // BranchModel doesn't keep a reference to Repository + return new BranchModel(headBranch, model, this); + } + } + + /// + /// Updates the CloneUrl from the "origin" remote. + /// + public void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel) + { + var localPath = localRepositoryModel.LocalPath; + if (localPath == null) + return; + + localRepositoryModel.CloneUrl = GetUri(localPath); + } + /// /// Returns the URL of the remote for the specified . If the repository /// is null or no remote named origin exists, this method returns null diff --git a/src/GitHub.Exports/Services/IGitService.cs b/src/GitHub.Exports/Services/IGitService.cs index 91453ad792..85209e34dc 100644 --- a/src/GitHub.Exports/Services/IGitService.cs +++ b/src/GitHub.Exports/Services/IGitService.cs @@ -7,6 +7,26 @@ namespace GitHub.Services { public interface IGitService { + /// + /// Initializes a new instance of the class. + /// + /// The repository's local path. + /// A repository model. + ILocalRepositoryModel CreateLocalRepositoryModel(string localPath); + + /// + /// Creates a new branch model for the current branch. + /// + /// The to create a current branch model for. + /// A new branch model. + IBranch CreateCurrentBranchModel(ILocalRepositoryModel model); + + /// + /// Updates the CloneUrl information based on the "origin" remote. + /// + /// The repository model to refresh. + void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel); + /// /// Returns the URL of the remote for the specified . If the repository /// is null or no remote exists, this method returns null diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index 0c62775f9a..b8e10ccf8e 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -203,7 +203,7 @@ async Task StatusChanged() var session = CurrentSession; var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); - if (pr != null) + if (pr != default) { var changePR = pr.Item1 != (session?.PullRequest.BaseRepositoryOwner) || diff --git a/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs b/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs index 20879c8fbe..d4f7b0ba6d 100644 --- a/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs +++ b/src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs @@ -25,14 +25,15 @@ public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder bool activeRepoNotified = false; IServiceProvider serviceProvider; - readonly IVSGitExt gitService; + readonly IVSGitExt gitExt; + readonly IGitService gitService; /// /// This class relies on IVSGitExt that provides information when VS switches repositories. /// - /// Used for monitoring the active repository. + /// Used for monitoring the active repository. [ImportingConstructor] - TeamExplorerServiceHolder(IVSGitExt gitService) : this(gitService, ThreadHelper.JoinableTaskContext) + TeamExplorerServiceHolder(IVSGitExt gitExt, IGitService gitService) : this(gitExt, gitService, ThreadHelper.JoinableTaskContext) { } @@ -41,19 +42,23 @@ public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder /// /// Used for monitoring the active repository. /// Used for switching to the Main thread. - public TeamExplorerServiceHolder(IVSGitExt gitService, JoinableTaskContext joinableTaskContext) + public TeamExplorerServiceHolder(IVSGitExt gitExt, IGitService gitService, JoinableTaskContext joinableTaskContext) { JoinableTaskCollection = joinableTaskContext.CreateCollection(); JoinableTaskCollection.DisplayName = nameof(TeamExplorerServiceHolder); JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection); - // This might be null in Blend or SafeMode - if (gitService != null) + // HACK: This might be null in SafeMode. + // We should be throwing rather than returning a null MEF service. + if (gitExt == null) { - this.gitService = gitService; - UpdateActiveRepo(); - gitService.ActiveRepositoriesChanged += UpdateActiveRepo; + return; } + + this.gitExt = gitExt; + this.gitService = gitService; + UpdateActiveRepo(); + gitExt.ActiveRepositoriesChanged += UpdateActiveRepo; } // set by the sections when they get initialized @@ -106,7 +111,10 @@ public void Subscribe(object who, Action handler) // the repo url might have changed and we don't get notifications // for that, so this is a good place to refresh it in case that happened - repo?.Refresh(); + if (repo != null) + { + gitService.RefreshCloneUrl(repo); + } // if the active repo hasn't changed and there's notifications queued up, // notify the subscriber. If the repo has changed, the set above will trigger @@ -152,7 +160,7 @@ void NotifyActiveRepo() void UpdateActiveRepo() { - var repo = gitService.ActiveRepositories.FirstOrDefault(); + var repo = gitExt.ActiveRepositories.FirstOrDefault(); if (!Equals(repo, ActiveRepo)) { diff --git a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj index a27bc63e3a..d4b56fc610 100644 --- a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj +++ b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj @@ -163,7 +163,6 @@ - diff --git a/src/GitHub.TeamFoundation.14/RegistryHelper.cs b/src/GitHub.TeamFoundation.14/RegistryHelper.cs index f4fa44eb31..dafba2e15f 100644 --- a/src/GitHub.TeamFoundation.14/RegistryHelper.cs +++ b/src/GitHub.TeamFoundation.14/RegistryHelper.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel.Composition; using System.Diagnostics; using System.Globalization; using System.IO; @@ -40,7 +41,7 @@ internal static IEnumerable PokeTheRegistryForRepositoryL { var path = subkey?.GetValue("Path") as string; if (path != null && Directory.Exists(path)) - return new LocalRepositoryModel(path, GitService.GitServiceHelper); + return GitService.GitServiceHelper.CreateLocalRepositoryModel(path); } catch (Exception) { diff --git a/src/GitHub.TeamFoundation.14/Services/LocalRepositoryModelFactory.cs b/src/GitHub.TeamFoundation.14/Services/LocalRepositoryModelFactory.cs deleted file mode 100644 index f6d7dc4a54..0000000000 --- a/src/GitHub.TeamFoundation.14/Services/LocalRepositoryModelFactory.cs +++ /dev/null @@ -1,13 +0,0 @@ -using GitHub.Models; -using GitHub.Services; - -namespace GitHub.TeamFoundation.Services -{ - class LocalRepositoryModelFactory : ILocalRepositoryModelFactory - { - public ILocalRepositoryModel Create(string localPath) - { - return new LocalRepositoryModel(localPath, GitService.GitServiceHelper); - } - } -} diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index d996b9e170..10167940e0 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -27,18 +27,18 @@ public class VSGitExt : IVSGitExt static readonly ILogger log = LogManager.ForContext(); readonly IServiceProvider serviceProvider; - readonly ILocalRepositoryModelFactory repositoryFactory; + readonly IGitService gitService; readonly object refreshLock = new object(); - IGitExt gitService; + IGitExt gitExt; IReadOnlyList activeRepositories; - public VSGitExt(IServiceProvider serviceProvider) - : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory(), ThreadHelper.JoinableTaskContext) + public VSGitExt(IServiceProvider serviceProvider, IGitService gitService) + : this(serviceProvider, new VSUIContextFactory(), gitService, ThreadHelper.JoinableTaskContext) { } - public VSGitExt(IServiceProvider serviceProvider, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory, + public VSGitExt(IServiceProvider serviceProvider, IVSUIContextFactory factory, IGitService gitService, JoinableTaskContext joinableTaskContext) { JoinableTaskCollection = joinableTaskContext.CreateCollection(); @@ -46,7 +46,7 @@ public VSGitExt(IServiceProvider serviceProvider, IVSUIContextFactory factory, I JoinableTaskFactory = joinableTaskContext.CreateFactory(JoinableTaskCollection); this.serviceProvider = serviceProvider; - this.repositoryFactory = repositoryFactory; + this.gitService = gitService; // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); @@ -64,7 +64,7 @@ public VSGitExt(IServiceProvider serviceProvider, IVSUIContextFactory factory, I async Task InitializeAsync() { - gitService = await GetServiceAsync(); + gitExt = await GetServiceAsync(); if (gitService == null) { log.Error("Couldn't find IGitExt service"); @@ -74,9 +74,9 @@ async Task InitializeAsync() // Refresh on background thread await Task.Run(() => RefreshActiveRepositories()); - gitService.PropertyChanged += (s, e) => + gitExt.PropertyChanged += (s, e) => { - if (e.PropertyName == nameof(gitService.ActiveRepositories)) + if (e.PropertyName == nameof(gitExt.ActiveRepositories)) { RefreshActiveRepositories(); } @@ -91,10 +91,10 @@ public void RefreshActiveRepositories() { log.Debug( "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", - gitService.GetHashCode(), - gitService.ActiveRepositories.Select(x => x.RepositoryPath)); + gitExt.GetHashCode(), + gitExt.ActiveRepositories.Select(x => x.RepositoryPath)); - ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); + ActiveRepositories = gitExt?.ActiveRepositories.Select(x => gitService.CreateLocalRepositoryModel(x.RepositoryPath)).ToList(); } } catch (Exception e) diff --git a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj index 0f872555d1..b8a8b29374 100644 --- a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj +++ b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj @@ -180,9 +180,6 @@ RegistryHelper.cs - - Services\LocalRepositoryModelFactory.cs - Services\VSGitExt.cs diff --git a/src/GitHub.TeamFoundation.16/GitHub.TeamFoundation.16.csproj b/src/GitHub.TeamFoundation.16/GitHub.TeamFoundation.16.csproj index 5e727e3fed..5f7b3d4c9a 100644 --- a/src/GitHub.TeamFoundation.16/GitHub.TeamFoundation.16.csproj +++ b/src/GitHub.TeamFoundation.16/GitHub.TeamFoundation.16.csproj @@ -180,9 +180,6 @@ RegistryHelper.cs - - Services\LocalRepositoryModelFactory.cs - Services\VSGitExt.cs diff --git a/src/GitHub.VisualStudio/Commands/BlameLinkCommand.cs b/src/GitHub.VisualStudio/Commands/BlameLinkCommand.cs index 0d55d951be..40d0352dab 100644 --- a/src/GitHub.VisualStudio/Commands/BlameLinkCommand.cs +++ b/src/GitHub.VisualStudio/Commands/BlameLinkCommand.cs @@ -15,8 +15,8 @@ namespace GitHub.VisualStudio.Commands public class BlameLinkCommand : LinkCommandBase, IBlameLinkCommand { [ImportingConstructor] - protected BlameLinkCommand(IGitHubServiceProvider serviceProvider) - : base(CommandSet, CommandId, serviceProvider) + protected BlameLinkCommand(IGitHubServiceProvider serviceProvider, Lazy gitService) + : base(CommandSet, CommandId, serviceProvider, gitService) { } diff --git a/src/GitHub.VisualStudio/Commands/CopyLinkCommand.cs b/src/GitHub.VisualStudio/Commands/CopyLinkCommand.cs index c01aedfdfc..b195d110cf 100644 --- a/src/GitHub.VisualStudio/Commands/CopyLinkCommand.cs +++ b/src/GitHub.VisualStudio/Commands/CopyLinkCommand.cs @@ -17,8 +17,8 @@ namespace GitHub.VisualStudio.Commands public class CopyLinkCommand : LinkCommandBase, ICopyLinkCommand { [ImportingConstructor] - protected CopyLinkCommand(IGitHubServiceProvider serviceProvider) - : base(CommandSet, CommandId, serviceProvider) + protected CopyLinkCommand(IGitHubServiceProvider serviceProvider, Lazy gitService) + : base(CommandSet, CommandId, serviceProvider, gitService) { } diff --git a/src/GitHub.VisualStudio/Commands/LinkCommandBase.cs b/src/GitHub.VisualStudio/Commands/LinkCommandBase.cs index 7326e306b8..d1abe4f1d4 100644 --- a/src/GitHub.VisualStudio/Commands/LinkCommandBase.cs +++ b/src/GitHub.VisualStudio/Commands/LinkCommandBase.cs @@ -1,4 +1,5 @@ using System; +using System.Globalization; using System.IO; using System.Threading.Tasks; using GitHub.Api; @@ -20,16 +21,19 @@ public abstract class LinkCommandBase : VsCommand static readonly ILogger log = LogManager.ForContext(); readonly Lazy apiFactory; readonly Lazy usageTracker; + readonly Lazy lazyGitService; protected LinkCommandBase( Guid commandSet, int commandId, - IGitHubServiceProvider serviceProvider) + IGitHubServiceProvider serviceProvider, + Lazy gitService) : base(commandSet, commandId) { ServiceProvider = serviceProvider; apiFactory = new Lazy(() => ServiceProvider.TryGetService()); usageTracker = new Lazy(() => serviceProvider.TryGetService()); + lazyGitService = gitService; } protected ILocalRepositoryModel ActiveRepo { get; private set; } @@ -46,7 +50,7 @@ protected ILocalRepositoryModel GetRepositoryByPath(string path) if (!string.IsNullOrEmpty(path)) { var repo = ServiceProvider.TryGetService().GetRepository(path); - return new LocalRepositoryModel(repo.Info.WorkingDirectory.TrimEnd('\\'), GitService.GitServiceHelper); + return GitService.GitServiceHelper.CreateLocalRepositoryModel(repo.Info.WorkingDirectory.TrimEnd('\\')); } } catch (Exception ex) @@ -66,7 +70,7 @@ protected ILocalRepositoryModel GetActiveRepo() var path = ServiceProvider.TryGetService()?.GetActiveRepoPath() ?? String.Empty; try { - activeRepo = !string.IsNullOrEmpty(path) ? new LocalRepositoryModel(path, GitService.GitServiceHelper) : null; + activeRepo = !string.IsNullOrEmpty(path) ? GitService.GitServiceHelper.CreateLocalRepositoryModel(path) : null; } catch (Exception ex) { @@ -86,7 +90,7 @@ void RefreshRepo() string path = vsGitServices?.GetActiveRepoPath() ?? String.Empty; try { - ActiveRepo = !String.IsNullOrEmpty(path) ? new LocalRepositoryModel(path, GitService.GitServiceHelper) : null; + ActiveRepo = !String.IsNullOrEmpty(path) ? GitService.GitServiceHelper.CreateLocalRepositoryModel(path) : null; } catch (Exception ex) { @@ -134,7 +138,7 @@ protected Task GenerateLink(LinkType linkType) var repo = GetRepositoryByPath(activeDocument.Name); - return repo.GenerateUrl(linkType, activeDocument.Name, activeDocument.StartLine, activeDocument.EndLine); + return GenerateUrl(lazyGitService.Value, repo, linkType, activeDocument.Name, activeDocument.StartLine, activeDocument.EndLine); } protected override void QueryStatus() @@ -160,5 +164,97 @@ public static bool IsFileDescendantOfDirectory(string file, string directory) } return false; } + + /// + /// Generates a http(s) url to the repository in the remote server, optionally + /// pointing to a specific file and specific line range in it. + /// + /// Type of link to generate + /// The file to generate an url to. Optional. + /// A specific line, or (if specifying the as well) the start of a range + /// The end of a line range on the specified file. + /// An UriString with the generated url, or null if the repository has no remote server configured or if it can't be found locally + public static async Task GenerateUrl(IGitService gitService, + ILocalRepositoryModel repo, LinkType linkType, string path = null, int startLine = -1, int endLine = -1) + { + if (repo.CloneUrl == null) + return null; + + var sha = await gitService.GetLatestPushedSha(path ?? repo.LocalPath); + // this also incidentally checks whether the repo has a valid LocalPath + if (String.IsNullOrEmpty(sha)) + return repo.CloneUrl.ToRepositoryUrl().AbsoluteUri; + + if (path != null && Path.IsPathRooted(path)) + { + // if the path root doesn't match the repository local path, then ignore it + if (!path.StartsWith(repo.LocalPath, StringComparison.OrdinalIgnoreCase)) + { + System.Diagnostics.Debug.Assert(false, String.Format(CultureInfo.CurrentCulture, "GenerateUrl: path {0} doesn't match repository {1}", path, repo.LocalPath)); + path = null; + } + else + path = path.Substring(repo.LocalPath.Length + 1); + } + + if (startLine > 0 && endLine > 0 && startLine > endLine) + { + // if startLine is greater than endLine and both are set, swap them + var temp = startLine; + startLine = endLine; + endLine = temp; + } + + if (startLine == endLine) + { + // if startLine is the same as endLine don't generate a range link + endLine = -1; + } + + return new UriString(GenerateUrl(linkType, repo.CloneUrl.ToRepositoryUrl().AbsoluteUri, sha, path, startLine, endLine)); + } + + const string CommitFormat = "{0}/commit/{1}"; + const string BlobFormat = "{0}/blob/{1}/{2}"; + const string BlameFormat = "{0}/blame/{1}/{2}"; + const string StartLineFormat = "{0}#L{1}"; + const string EndLineFormat = "{0}-L{1}"; + static string GenerateUrl(LinkType linkType, string basePath, string sha, string path, int startLine = -1, int endLine = -1) + { + if (sha == null) + return basePath; + + if (String.IsNullOrEmpty(path)) + return String.Format(CultureInfo.InvariantCulture, CommitFormat, basePath, sha); + + var ret = String.Format(CultureInfo.InvariantCulture, GetLinkFormat(linkType), basePath, sha, path.Replace(@"\", "/")); + + if (startLine < 0) + return ret; + ret = String.Format(CultureInfo.InvariantCulture, StartLineFormat, ret, startLine); + if (endLine < 0) + return ret; + return String.Format(CultureInfo.InvariantCulture, EndLineFormat, ret, endLine); + } + + /// + /// Selects the proper format for the link type, defaults to the blob url when link type is not selected. + /// + /// Type of link to generate + /// The string format of the selected link type + static string GetLinkFormat(LinkType linkType) + { + switch (linkType) + { + case LinkType.Blame: + return BlameFormat; + + case LinkType.Blob: + return BlobFormat; + + default: + return BlobFormat; + } + } } } diff --git a/src/GitHub.VisualStudio/Commands/OpenFromClipboardCommand.cs b/src/GitHub.VisualStudio/Commands/OpenFromClipboardCommand.cs index 7476a6eb58..73e77cea96 100644 --- a/src/GitHub.VisualStudio/Commands/OpenFromClipboardCommand.cs +++ b/src/GitHub.VisualStudio/Commands/OpenFromClipboardCommand.cs @@ -4,7 +4,6 @@ using GitHub.Exports; using GitHub.Services; using GitHub.Services.Vssdk.Commands; -using GitHub.VisualStudio.UI; using Microsoft.VisualStudio.Shell; using Task = System.Threading.Tasks.Task; @@ -13,11 +12,12 @@ namespace GitHub.VisualStudio.Commands [Export(typeof(IOpenFromClipboardCommand))] public class OpenFromClipboardCommand : VsCommand, IOpenFromClipboardCommand { - + readonly Lazy gitHubContextService; readonly Lazy teamExplorerContext; readonly Lazy vsServices; + readonly Lazy gitService; readonly Lazy uiContext; /// @@ -34,12 +34,14 @@ public class OpenFromClipboardCommand : VsCommand, IOpenFromClipboardCom public OpenFromClipboardCommand( Lazy gitHubContextService, Lazy teamExplorerContext, - Lazy vsServices) + Lazy vsServices, + Lazy gitService) : base(CommandSet, CommandId) { this.gitHubContextService = gitHubContextService; this.teamExplorerContext = teamExplorerContext; this.vsServices = vsServices; + this.gitService = gitService; // See https://code.msdn.microsoft.com/windowsdesktop/AllowParams-2005-9442298f ParametersDescription = "u"; // accept a single url @@ -96,9 +98,11 @@ public override async Task Execute(string url) var hasChanges = gitHubContextService.Value.HasChangesInWorkingDirectory(repositoryDir, commitish, path); if (hasChanges) { - // AnnotateFile expects a branch name so we use the current branch - var branchName = activeRepository.CurrentBranch.Name; + // TODO: What if this returns null because we're not on a branch? + var currentBranch = gitService.Value.CreateCurrentBranchModel(activeRepository); + var branchName = currentBranch.Name; + // AnnotateFile expects a branch name so we use the current branch if (await gitHubContextService.Value.TryAnnotateFile(repositoryDir, branchName, context)) { return; diff --git a/src/GitHub.VisualStudio/Commands/OpenLinkCommand.cs b/src/GitHub.VisualStudio/Commands/OpenLinkCommand.cs index 5d5ddc7142..3c00747ca2 100644 --- a/src/GitHub.VisualStudio/Commands/OpenLinkCommand.cs +++ b/src/GitHub.VisualStudio/Commands/OpenLinkCommand.cs @@ -14,8 +14,8 @@ namespace GitHub.VisualStudio.Commands public class OpenLinkCommand : LinkCommandBase, IOpenLinkCommand { [ImportingConstructor] - protected OpenLinkCommand(IGitHubServiceProvider serviceProvider) - : base(CommandSet, CommandId, serviceProvider) + protected OpenLinkCommand(IGitHubServiceProvider serviceProvider, Lazy gitService) + : base(CommandSet, CommandId, serviceProvider, gitService) { } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 741b9ccfe8..f8fff137dd 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -299,7 +299,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT else if (serviceType == typeof(IVSGitExt)) { var vsVersion = ApplicationInfo.GetHostVersionInfo().FileMajorPart; - return new VSGitExtFactory(vsVersion, this).Create(); + return new VSGitExtFactory(vsVersion, this, GitService.GitServiceHelper).Create(); } else if (serviceType == typeof(IGitHubToolWindowManager)) { diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 928eb96d1d..1464d32c02 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -17,11 +17,13 @@ public class VSGitExtFactory readonly int vsVersion; readonly IServiceProvider serviceProvider; + readonly IGitService gitService; - public VSGitExtFactory(int vsVersion, IServiceProvider serviceProvider) + public VSGitExtFactory(int vsVersion, IServiceProvider serviceProvider, IGitService gitService) { this.vsVersion = vsVersion; this.serviceProvider = serviceProvider; + this.gitService = gitService; } public IVSGitExt Create() @@ -29,11 +31,11 @@ public IVSGitExt Create() switch (vsVersion) { case 14: - return Create(() => new VSGitExt14(serviceProvider)); + return Create(() => new VSGitExt14(serviceProvider, gitService)); case 15: - return Create(() => new VSGitExt15(serviceProvider)); + return Create(() => new VSGitExt15(serviceProvider, gitService)); case 16: - return Create(() => new VSGitExt16(serviceProvider)); + return Create(() => new VSGitExt16(serviceProvider, gitService)); default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", vsVersion); return null; diff --git a/test/GitHub.App.UnitTests/Models/RepositoryModelTests.cs b/test/GitHub.App.UnitTests/Models/RepositoryModelTests.cs index fc048b5c95..fd2b9a574c 100644 --- a/test/GitHub.App.UnitTests/Models/RepositoryModelTests.cs +++ b/test/GitHub.App.UnitTests/Models/RepositoryModelTests.cs @@ -20,8 +20,8 @@ public class ComparisonTests : TestBaseClass public void SameContentEqualsTrue(string name1, string url1, string path1, string name2, string url2, string path2) { var gitService = Substitute.For(); - var a = new LocalRepositoryModel(name1, new UriString(url1), path1, gitService); - var b = new LocalRepositoryModel(name2, new UriString(url2), path2, gitService); + var a = new LocalRepositoryModel { Name = name1, CloneUrl = url1, LocalPath = path1 }; + var b = new LocalRepositoryModel { Name = name2, CloneUrl = url2, LocalPath = path2 }; Assert.That(a, Is.EqualTo(b)); Assert.False(a == b); Assert.That(a.GetHashCode(), Is.EqualTo(b.GetHashCode())); @@ -50,39 +50,6 @@ public void DifferentContentEqualsFalse(long id1, string name1, string url1, lon } } - //[Collection("PackageServiceProvider global data tests")] - public class PathConstructorTests : TestBaseClass - { - [Test] - public void NoRemoteUrl() - { - using (var temp = new TempDirectory()) - { - var gitService = Substitute.For(); - var repo = Substitute.For(); - var path = temp.Directory.CreateSubdirectory("repo-name"); - gitService.GetUri(path.FullName).Returns((UriString)null); - var model = new LocalRepositoryModel(path.FullName, gitService); - Assert.That("repo-name", Is.EqualTo(model.Name)); - } - } - - [Test] - public void WithRemoteUrl() - { - using (var temp = new TempDirectory()) - { - var gitService = Substitute.For(); - var repo = Substitute.For(); - var path = temp.Directory.CreateSubdirectory("repo-name"); - gitService.GetUri(path.FullName).Returns(new UriString("https://github.com/user/repo-name")); - var model = new LocalRepositoryModel(path.FullName, gitService); - Assert.That("repo-name", Is.EqualTo(model.Name)); - Assert.That("user", Is.EqualTo(model.Owner)); - } - } - } - public class HostAddressTests : TestBaseClass { [TestCase("https://github.com/owner/repo")] diff --git a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs index c8e437cb8d..2ede44920b 100644 --- a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs @@ -701,10 +701,10 @@ public void CreatePullRequestAllArgsMandatory() ms = Substitute.For(); Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); - sourceRepo = new LocalRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"), "c:\\path", gitService); + sourceRepo = new LocalRepositoryModel { Name = "name", CloneUrl = "http://github.com/github/stuff", LocalPath = "c:\\path" }; Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); - targetRepo = new LocalRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"), "c:\\path", gitService); + targetRepo = new LocalRepositoryModel { Name = "name", CloneUrl = "http://github.com/github/stuff", LocalPath = "c:\\path" }; Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); title = "a title"; diff --git a/test/GitHub.App.UnitTests/Services/TeamExplorerContextTests.cs b/test/GitHub.App.UnitTests/Services/TeamExplorerContextTests.cs index f539b589e0..6e202f0aaf 100644 --- a/test/GitHub.App.UnitTests/Services/TeamExplorerContextTests.cs +++ b/test/GitHub.App.UnitTests/Services/TeamExplorerContextTests.cs @@ -151,45 +151,21 @@ public void NoActiveRepositoryChange_SolutionChanges() public class TheStatusChangedEvent { - [TestCase(false, "name1", "sha1", "name1", "sha1", false)] - [TestCase(false, "name1", "sha1", "name2", "sha1", true)] - [TestCase(false, "name1", "sha1", "name1", "sha2", true)] - [TestCase(false, "name1", "sha1", "name2", "sha2", true)] - [TestCase(true, "name1", "sha1", "name1", "sha1", false)] - [TestCase(true, "name1", "sha1", "name2", "sha2", false)] - public void SameActiveRepository_ExpectWasRaised(bool changePath, string name1, string sha1, string name2, string sha2, bool expectWasRaised) + [TestCase("path", "path", true)] + [TestCase("path1", "path2", false)] + [TestCase(null, null, false)] + public void AlwaysFireWhenNoLocalPathChange(string path1, string path2, bool expectWasRaised) { var gitExt = CreateGitExt(); var repositoryPaths = new[] { Directory.GetCurrentDirectory(), Path.GetTempPath() }; - var path1 = Directory.GetCurrentDirectory(); - var path2 = changePath ? Path.GetTempPath() : path1; - var repoInfo1 = CreateRepositoryModel(path1, name1, sha1); - var repoInfo2 = CreateRepositoryModel(path2, name2, sha2); + var repoInfo1 = CreateRepositoryModel(path1); + var repoInfo2 = CreateRepositoryModel(path2); var target = CreateTeamExplorerContext(gitExt); - var eventWasRaised = false; - target.StatusChanged += (s, e) => eventWasRaised = true; - SetActiveRepository(gitExt, repoInfo1); - SetActiveRepository(gitExt, repoInfo2); - - Assert.That(eventWasRaised, Is.EqualTo(expectWasRaised)); - } - - [TestCase("trackedSha", "trackedSha", false)] - [TestCase("trackedSha1", "trackedSha2", true)] - public void TrackedShaChanges_CheckWasRaised(string trackedSha1, string trackedSha2, bool expectWasRaised) - { - var gitExt = CreateGitExt(); - var repositoryPaths = new[] { Directory.GetCurrentDirectory(), Path.GetTempPath() }; - var repoPath = Directory.GetCurrentDirectory(); - var repoInfo1 = CreateRepositoryModel(repoPath, "name", "sha", trackedSha1); - var repoInfo2 = CreateRepositoryModel(repoPath, "name", "sha", trackedSha2); - var target = CreateTeamExplorerContext(gitExt); SetActiveRepository(gitExt, repoInfo1); var eventWasRaised = false; target.StatusChanged += (s, e) => eventWasRaised = true; - SetActiveRepository(gitExt, repoInfo2); Assert.That(eventWasRaised, Is.EqualTo(expectWasRaised)); @@ -200,7 +176,7 @@ public void SolutionUnloadedAndReloaded_DontFireStatusChanged() { var gitExt = CreateGitExt(); var path = Directory.GetCurrentDirectory(); - var repoInfo1 = CreateRepositoryModel(path, "name", "sha"); + var repoInfo1 = CreateRepositoryModel(path); var repoInfo2 = CreateRepositoryModel(null); var target = CreateTeamExplorerContext(gitExt); SetActiveRepository(gitExt, repoInfo1); @@ -226,15 +202,10 @@ static TeamExplorerContext CreateTeamExplorerContext( return new TeamExplorerContext(gitExt, new AsyncLazy(() => Task.FromResult(dte)), pullRequestService, joinableTaskContext); } - static ILocalRepositoryModel CreateRepositoryModel(string path, string branchName = null, string headSha = null, string trackedSha = null) + static ILocalRepositoryModel CreateRepositoryModel(string path) { var repo = Substitute.For(); repo.LocalPath.Returns(path); - var currentBranch = Substitute.For(); - currentBranch.Name.Returns(branchName); - currentBranch.Sha.Returns(headSha); - currentBranch.TrackedSha.Returns(trackedSha); - repo.CurrentBranch.Returns(currentBranch); return repo; } diff --git a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs index 5b21260e0a..726566dae8 100644 --- a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs @@ -6,14 +6,12 @@ using GitHub.Models; using GitHub.Primitives; using GitHub.Services; -using GitHub.ViewModels; using GitHub.ViewModels.GitHubPane; using NSubstitute; using Octokit; using UnitTests; using NUnit.Framework; using IConnection = GitHub.Models.IConnection; -using ReactiveUI.Testing; using System.Reactive.Concurrency; using GitHub.Models.Drafts; @@ -99,7 +97,7 @@ static TestData PrepareTestData( var targetRepo = targetRepoOwner == sourceRepoOwner ? sourceRepo : sourceRepo.Parent; var targetBranch = targetBranchName != targetRepo.DefaultBranch.Name ? new BranchModel(targetBranchName, targetRepo) : targetRepo.DefaultBranch; - activeRepo.CurrentBranch.Returns(sourceBranch); + gitService.CreateCurrentBranchModel(activeRepo).Returns(sourceBranch); api.GetRepository(Args.String, Args.String).Returns(Observable.Return(githubRepo)); ms.ApiClient.Returns(api); @@ -130,7 +128,8 @@ public async Task TargetBranchDisplayNameIncludesRepoOwnerWhenForkAsync() var data = PrepareTestData("octokit.net", "shana", "master", "octokit", "master", "origin", true, true); var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), Substitute.For(), data.ServiceProvider.GetOperatingSystem(), Substitute.For()); prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Empty()); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, Substitute.For()); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, + Substitute.For(), data.GitService); await vm.InitializeAsync(data.ActiveRepo, data.Connection); Assert.That("octokit/master", Is.EqualTo(vm.TargetBranch.DisplayName)); } @@ -165,7 +164,8 @@ public async Task CreatingPRsAsync( var ms = data.ModelService; var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), Substitute.For(), data.ServiceProvider.GetOperatingSystem(), Substitute.For()); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, Substitute.For()); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, + Substitute.For(), data.GitService); await vm.InitializeAsync(data.ActiveRepo, data.Connection); // the TargetBranch property gets set to whatever the repo default is (we assume master here), @@ -208,7 +208,7 @@ public async Task TemplateIsUsedIfPresentAsync() prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Return("Test PR template")); var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, - Substitute.For()); + Substitute.For(), data.GitService); await vm.InitializeAsync(data.ActiveRepo, data.Connection); Assert.That("Test PR template", Is.EqualTo(vm.Description)); @@ -227,7 +227,8 @@ public async Task LoadsDraft() }); var prservice = Substitute.For(); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, + draftStore, data.GitService); await vm.InitializeAsync(data.ActiveRepo, data.Connection); Assert.That(vm.PRTitle, Is.EqualTo("This is a Title.")); @@ -241,7 +242,8 @@ public async Task UpdatesDraftWhenDescriptionChanges() var scheduler = new HistoricalScheduler(); var draftStore = Substitute.For(); var prservice = Substitute.For(); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, scheduler); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, + draftStore, data.GitService, scheduler); await vm.InitializeAsync(data.ActiveRepo, data.Connection); vm.Description = "Body changed."; @@ -263,7 +265,8 @@ public async Task UpdatesDraftWhenTitleChanges() var scheduler = new HistoricalScheduler(); var draftStore = Substitute.For(); var prservice = Substitute.For(); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, scheduler); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, + draftStore, data.GitService, scheduler); await vm.InitializeAsync(data.ActiveRepo, data.Connection); vm.PRTitle = "Title changed."; @@ -285,7 +288,8 @@ public async Task DeletesDraftWhenPullRequestSubmitted() var scheduler = new HistoricalScheduler(); var draftStore = Substitute.For(); var prservice = Substitute.For(); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, scheduler); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, + data.GitService, scheduler); await vm.InitializeAsync(data.ActiveRepo, data.Connection); await vm.CreatePullRequest.Execute(); @@ -300,7 +304,8 @@ public async Task DeletesDraftWhenCanceled() var scheduler = new HistoricalScheduler(); var draftStore = Substitute.For(); var prservice = Substitute.For(); - var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, scheduler); + var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, draftStore, + data.GitService, scheduler); await vm.InitializeAsync(data.ActiveRepo, data.Connection); await vm.Cancel.Execute(); diff --git a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs index ac24f57097..fae1bd9fa7 100644 --- a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs @@ -552,7 +552,8 @@ static Tuple CreateTargetAndSer { var repository = Substitute.For(); var currentBranchModel = new BranchModel(currentBranch, repository); - repository.CurrentBranch.Returns(currentBranchModel); + var gitService = Substitute.For(); + gitService.CreateCurrentBranchModel(repository).Returns(currentBranchModel); repository.CloneUrl.Returns(new UriString(Uri.ToString())); repository.LocalPath.Returns(@"C:\projects\ThisRepo"); repository.Name.Returns("repo"); @@ -607,7 +608,8 @@ static Tuple CreateTargetAndSer Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For()); + Substitute.For(), + gitService); vm.InitializeAsync(repository, Substitute.For(), "owner", "repo", 1).Wait(); return Tuple.Create(vm, pullRequestService); diff --git a/test/GitHub.Exports.UnitTests/GitServiceTests.cs b/test/GitHub.Exports.UnitTests/GitServiceTests.cs index 1b2ac321ba..c98db18f97 100644 --- a/test/GitHub.Exports.UnitTests/GitServiceTests.cs +++ b/test/GitHub.Exports.UnitTests/GitServiceTests.cs @@ -1,7 +1,5 @@ using System; -using System.Collections.Generic; using System.IO; -using System.Linq; using System.Threading.Tasks; using GitHub.Services; using LibGit2Sharp; @@ -10,6 +8,41 @@ public class GitServiceTests { + public class CreateLocalRepositoryModelTests : TestBaseClass + { + [Test] + public void NoRemoteUrl() + { + using (var temp = new TempDirectory()) + { + var repositoryFacade = Substitute.For(); + var gitService = new GitService(repositoryFacade); + var path = temp.Directory.CreateSubdirectory("repo-name"); + + var model = gitService.CreateLocalRepositoryModel(path.FullName); + + Assert.That(model.Name, Is.EqualTo("repo-name")); + } + } + + [Test] + public void WithRemoteUrl() + { + using (var temp = new TempDirectory()) + { + var path = temp.Directory.CreateSubdirectory("repo-name"); + var repository = CreateRepositoryWithOrigin("https://github.com/user/repo-name"); + var repositoryFacade = CreateRepositoryFacade(path.FullName, repository); + var gitService = new GitService(repositoryFacade); + + var model = gitService.CreateLocalRepositoryModel(path.FullName); + + Assert.That(model.Name, Is.EqualTo("repo-name")); + Assert.That(model.Owner, Is.EqualTo("user")); + } + } + } + [TestCase("asdf", null)] [TestCase("", null)] [TestCase(null, null)] @@ -47,12 +80,20 @@ static IRepositoryFacade CreateRepositoryFacade(string path, IRepository repo) return repositoryFacade; } - static IRepository CreateRepository() + static IRepository CreateRepositoryWithOrigin(string originUrl) { - var repo = Substitute.For(); + var repo = CreateRepository(); + var origin = Substitute.For(); + origin.Url.Returns(originUrl); + repo.Network.Remotes["origin"].Returns(origin); return repo; } + static IRepository CreateRepository() + { + return Substitute.For(); + } + public class TheGetLatestPushedShaMethod : TestBaseClass { [Test] diff --git a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs index 5b6dc68a57..7b49b9ff2c 100644 --- a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs +++ b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs @@ -41,7 +41,7 @@ public void ReadsPullRequestFromCorrectFork() var sessionService = CreateSessionService(); service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs( - Observable.Return(Tuple.Create("fork", CurrentBranchPullRequestNumber))); + Observable.Return(("fork", CurrentBranchPullRequestNumber))); var connectionManager = CreateConnectionManager(); var target = CreateTarget( @@ -80,7 +80,7 @@ public void CreatesSessionForCurrentBranch() public void CurrentSessionIsNullIfNoPullRequestForCurrentBranch() { var service = CreatePullRequestService(); - service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Empty>()); + service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Empty<(string, int)>()); var target = CreateTarget(service: service); @@ -98,7 +98,7 @@ public void CurrentSessionChangesWhenBranchChanges() var session = target.CurrentSession; - service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(Tuple.Create("foo", 22))); + service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(("foo", 22))); teamExplorerContext.StatusChanged += Raise.Event(); Assert.That(session, Is.Not.SameAs(target.CurrentSession)); @@ -124,7 +124,7 @@ public void CurrentSessionChangesToNullIfNoPullRequestForCurrentBranch() teamExplorerContext: teamExplorerContext); Assert.That(target.CurrentSession, Is.Not.Null); - Tuple newPullRequest = null; + (string owner, int number) newPullRequest = default; service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(newPullRequest)); teamExplorerContext.StatusChanged += Raise.Event(); @@ -813,14 +813,14 @@ public async Task GetSessionUpdatesCurrentSessionIfCurrentBranchIsPullRequestBut var model = CreatePullRequestModel(); var sessionService = CreateSessionService(model); - service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Empty>()); + service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Empty<(string, int)>()); var target = CreateTarget(service: service, sessionService: sessionService); Assert.That(target.CurrentSession, Is.Null); service.EnsureLocalBranchesAreMarkedAsPullRequests(Arg.Any(), model).Returns(Observable.Return(true)); - service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(Tuple.Create("owner", CurrentBranchPullRequestNumber))); + service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(("owner", CurrentBranchPullRequestNumber))); var session = await target.GetSession("owner", "name", CurrentBranchPullRequestNumber); @@ -881,7 +881,7 @@ PullRequestDetailModel CreatePullRequestModel( IPullRequestService CreatePullRequestService(string owner = "owner") { var result = Substitute.For(); - result.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(Tuple.Create(owner, CurrentBranchPullRequestNumber))); + result.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return((owner, CurrentBranchPullRequestNumber))); return result; } diff --git a/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs b/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs index 7b9ee71c58..afe09c6cd8 100644 --- a/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs +++ b/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs @@ -10,10 +10,12 @@ using NUnit.Framework; using NSubstitute; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Threading; using Task = System.Threading.Tasks.Task; using static Microsoft.VisualStudio.VSConstants; +using GitHub.Primitives; +using LibGit2Sharp; +using System.Threading.Tasks; public class VSGitExtTests { @@ -149,32 +151,32 @@ public void RepositoryOpenContextNotActive_IsEmpty() public void RepositoryOpenIsActive_InitializeWithActiveRepositories() { var repoPath = "repoPath"; - var repoFactory = Substitute.For(); + var gitService = Substitute.For(); var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); - var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); + var target = CreateVSGitExt(context, gitExt, gitService: gitService); target.JoinTillEmpty(); var activeRepositories = target.ActiveRepositories; Assert.That(activeRepositories.Count, Is.EqualTo(1)); - repoFactory.Received(1).Create(repoPath); + gitService.Received(1).CreateLocalRepositoryModel(repoPath); } [Test] public void ExceptionRefreshingRepositories_ReturnsEmptyList() { var repoPath = "repoPath"; - var repoFactory = Substitute.For(); - repoFactory.Create(repoPath).ReturnsForAnyArgs(x => { throw new Exception("Boom!"); }); + var gitService = Substitute.For(); + gitService.CreateLocalRepositoryModel(repoPath).ReturnsForAnyArgs(x => { throw new Exception("Boom!"); }); var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); - var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); + var target = CreateVSGitExt(context, gitExt, gitService: gitService); target.JoinTillEmpty(); var activeRepositories = target.ActiveRepositories; - repoFactory.Received(1).Create(repoPath); + gitService.Received(1).CreateLocalRepositoryModel(repoPath); Assert.That(activeRepositories.Count, Is.EqualTo(0)); } @@ -182,8 +184,8 @@ public void ExceptionRefreshingRepositories_ReturnsEmptyList() public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads() { var gitExt = new MockGitExt(); - var repoFactory = new MockRepositoryFactory(); - var target = CreateVSGitExt(gitExt: gitExt, repoFactory: repoFactory); + var gitService = new MockGitService(); + var target = CreateVSGitExt(gitExt: gitExt, gitService: gitService); var activeRepositories1 = CreateActiveRepositories("repo1"); var activeRepositories2 = CreateActiveRepositories("repo2"); var task1 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories1); @@ -211,18 +213,18 @@ static IReadOnlyList CreateActiveRepositories(params string[ } static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IServiceProvider sp = null, - ILocalRepositoryModelFactory repoFactory = null, JoinableTaskContext joinableTaskContext = null, string contextGuidString = null) + IGitService gitService = null, JoinableTaskContext joinableTaskContext = null, string contextGuidString = null) { context = context ?? CreateVSUIContext(true); gitExt = gitExt ?? CreateGitExt(); var contextGuid = new Guid(contextGuidString ?? Guids.GitSccProviderId); sp = sp ?? Substitute.For(); - repoFactory = repoFactory ?? Substitute.For(); + gitService = gitService ?? Substitute.For(); joinableTaskContext = joinableTaskContext ?? new JoinableTaskContext(); var factory = Substitute.For(); factory.GetUIContext(contextGuid).Returns(context); sp.GetService(typeof(IGitExt)).Returns(gitExt); - var vsGitExt = new VSGitExt(sp, factory, repoFactory, joinableTaskContext); + var vsGitExt = new VSGitExt(sp, factory, gitService, joinableTaskContext); vsGitExt.JoinTillEmpty(); return vsGitExt; } @@ -291,9 +293,9 @@ public IReadOnlyList ActiveRepositories public event PropertyChangedEventHandler PropertyChanged; } - class MockRepositoryFactory : ILocalRepositoryModelFactory + class MockGitService : IGitService { - public ILocalRepositoryModel Create(string localPath) + public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath) { var result = Substitute.For(); result.LocalPath.Returns(localPath); @@ -308,5 +310,13 @@ public ILocalRepositoryModel Create(string localPath) return result; } + + public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model) => throw new NotImplementedException(); + public Task GetLatestPushedSha(string path, string remote = "origin") => throw new NotImplementedException(); + public UriString GetRemoteUri(IRepository repo, string remote = "origin") => throw new NotImplementedException(); + public IRepository GetRepository(string path) => throw new NotImplementedException(); + public UriString GetUri(IRepository repository, string remote = "origin") => throw new NotImplementedException(); + public UriString GetUri(string path, string remote = "origin") => throw new NotImplementedException(); + public void RefreshCloneUrl(ILocalRepositoryModel localRepositoryModel) => throw new NotImplementedException(); } } diff --git a/test/GitHub.Exports.UnitTests/LocalRepositoryModelTests.cs b/test/GitHub.VisualStudio.UnitTests/Commands/LinkCommandBaseTests.cs similarity index 81% rename from test/GitHub.Exports.UnitTests/LocalRepositoryModelTests.cs rename to test/GitHub.VisualStudio.UnitTests/Commands/LinkCommandBaseTests.cs index cb2c163b61..8be88301a7 100644 --- a/test/GitHub.Exports.UnitTests/LocalRepositoryModelTests.cs +++ b/test/GitHub.VisualStudio.UnitTests/Commands/LinkCommandBaseTests.cs @@ -1,14 +1,13 @@ -using System.Threading.Tasks; -using System.Collections.Generic; +using System; +using System.Threading.Tasks; using GitHub.Models; using GitHub.Exports; using GitHub.Services; -using GitHub.Primitives; +using GitHub.VisualStudio.Commands; using NSubstitute; -using LibGit2Sharp; using NUnit.Framework; -public class LocalRepositoryModelTests : TestBaseClass +public class LinkCommandBaseTests : TestBaseClass { [TestCase(1, LinkType.Blob, false, "https://github.com/foo/bar", "123123", @"src\dir\file1.cs", -1, -1, "https://github.com/foo/bar/blob/123123/src/dir/file1.cs")] [TestCase(2, LinkType.Blob, false, "https://github.com/foo/bar", "123123", @"src\dir\file1.cs", 1, -1, "https://github.com/foo/bar/blob/123123/src/dir/file1.cs#L1")] @@ -48,32 +47,18 @@ public async Task GenerateUrl(int testid, LinkType linkType, bool createRootedPa if (createRootedPath && path != null) path = System.IO.Path.Combine(basePath.FullName, path); ILocalRepositoryModel model = null; - if (!string.IsNullOrEmpty(baseUrl)) - model = new LocalRepositoryModel("bar", new UriString(baseUrl), basePath.FullName, gitService); - else - model = new LocalRepositoryModel(basePath.FullName, gitService); - var result = await model.GenerateUrl(linkType, path, startLine, endLine); - Assert.That(expected, Is.EqualTo(result?.ToString())); + model = new LocalRepositoryModel { Name = "bar", CloneUrl = baseUrl, LocalPath = basePath.FullName }; + + var result = await LinkCommandBase.GenerateUrl(gitService, model, linkType, path, startLine, endLine); + + Assert.That(result?.ToString(), Is.EqualTo(expected)); } } static IGitService CreateGitService(string sha) { var gitservice = Substitute.For(); - var repo = Substitute.For(); - gitservice.GetRepository(Args.String).Returns(repo); gitservice.GetLatestPushedSha(Args.String).Returns(Task.FromResult(sha)); - if (!string.IsNullOrEmpty(sha)) - { - var refs = Substitute.For(); - var refrence = Substitute.For(); - refs.ReachableFrom(Arg.Any>(), Arg.Any>()).Returns(new Reference[] { refrence }); - repo.Refs.Returns(refs); - var commit = Substitute.For(); - commit.Sha.Returns(sha); - repo.Commits.Returns(new FakeCommitLog { commit }); - } - return gitservice; } } diff --git a/test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs b/test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs index 0235482263..b43a203c74 100644 --- a/test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs +++ b/test/GitHub.VisualStudio.UnitTests/Commands/OpenFromClipboardCommandTests.cs @@ -3,10 +3,10 @@ using System.Threading.Tasks; using GitHub; using GitHub.Exports; +using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using GitHub.VisualStudio.Commands; -using GitHub.VisualStudio.UI; using Microsoft.VisualStudio; using NSubstitute; using NUnit.Framework; @@ -146,7 +146,8 @@ public async Task HasChangesInWorkingDirectory(bool annotateFileSupported, strin var resolveBlobResult = (targetBranch, "foo.cs", ""); var vsServices = Substitute.For(); var target = CreateOpenFromClipboardCommand(gitHubContextService: gitHubContextService, vsServices: vsServices, - contextFromClipboard: context, repositoryDir: repositoryDir, repositoryName: repositoryName, currentBranch: currentBranch, resolveBlobResult: resolveBlobResult, hasChanges: true); + contextFromClipboard: context, repositoryDir: repositoryDir, repositoryName: repositoryName, + currentBranchName: currentBranch, resolveBlobResult: resolveBlobResult, hasChanges: true); await target.Execute(null); @@ -179,7 +180,7 @@ static OpenFromClipboardCommand CreateOpenFromClipboardCommand( string repositoryDir = null, string repositoryName = null, string repositoryOwner = null, - string currentBranch = null, + string currentBranchName = null, (string, string, string)? resolveBlobResult = null, bool? hasChanges = null) { @@ -189,10 +190,14 @@ static OpenFromClipboardCommand CreateOpenFromClipboardCommand( vsServices = vsServices ?? Substitute.For(); gitHubContextService.FindContextFromClipboard().Returns(contextFromClipboard); - teamExplorerContext.ActiveRepository.LocalPath.Returns(repositoryDir); - teamExplorerContext.ActiveRepository.Name.Returns(repositoryName); - teamExplorerContext.ActiveRepository.Owner.Returns(repositoryOwner); - teamExplorerContext.ActiveRepository.CurrentBranch.Name.Returns(currentBranch); + var activeRepository = teamExplorerContext.ActiveRepository; + activeRepository.LocalPath.Returns(repositoryDir); + activeRepository.Name.Returns(repositoryName); + activeRepository.Owner.Returns(repositoryOwner); + var gitService = Substitute.For(); + var currentBranch = Substitute.For(); + currentBranch.Name.Returns(currentBranchName); + gitService.CreateCurrentBranchModel(activeRepository).Returns(currentBranch); if (resolveBlobResult != null) { gitHubContextService.ResolveBlob(repositoryDir, contextFromClipboard).Returns(resolveBlobResult.Value); @@ -206,7 +211,8 @@ static OpenFromClipboardCommand CreateOpenFromClipboardCommand( return new OpenFromClipboardCommand( new Lazy(() => gitHubContextService), new Lazy(() => teamExplorerContext), - new Lazy(() => vsServices)); + new Lazy(() => vsServices), + new Lazy(() => gitService)); } static string ResolveResources(string str)