Merge branch 'master' into fixes/make-ncrunch-happy

Author: Meaghan Lewis

CONTRIBUTING.md

@@ -13,16 +13,16 @@ This project adheres to the [Open Code of Conduct][code-of-conduct]. By particip
 
 ## Submitting a pull request
 
-0. [Fork][] and clone the repository (see Build Instructions in the [README][readme])
-0. Create a new branch: `git checkout -b my-branch-name`
-0. Make your change, add tests, and make sure the tests still pass
-0. Push to your fork and [submit a pull request][pr]
-0. Pat your self on the back and wait for your pull request to be reviewed and merged.
+1. [Fork][] and clone the repository (see Build Instructions in the [README][readme])
+2. Create a new branch: `git checkout -b my-branch-name`
+3. Make your change, add tests, and make sure the tests still pass
+4. Push to your fork and [submit a pull request][pr]
+5. Pat your self on the back and wait for your pull request to be reviewed and merged.
 
 Here are a few things you can do that will increase the likelihood of your pull request being accepted:
 
-- Follow the existing code's style.
-- Write tests.
+- Follow the style/format of the existing code.
+- Write tests for your changes.
 - Keep your change as focused as possible. If there are multiple changes you would like to make that are not dependent upon each other, consider submitting them as separate pull requests.
 - Write a [good commit message](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html).
 
@@ -38,7 +38,7 @@ There are certain areas of the extension that are restricted in what they can do
 ### Bug Reporting
 
 Here are a few helpful tips when reporting a bug:
-- Verify the bug resides in the GitHub for Visual Studio extension
+- Verify that the bug resides in the GitHub for Visual Studio extension
   - A lot of functionality provided by this extension resides in the Team Explorer pane, alongside other non-GitHub tools to manage and collaborate on source code, including Visual Studio's Git support, which is owned by Microsoft.
   - If this bug not is related to the GitHub extension, visit the [Visual Studio support page](https://www.visualstudio.com/support/support-overview-vs) for help
 - Screenshots are very helpful in diagnosing bugs and understanding the state of the extension when it's experiencing problems. Please include them whenever possible.

appveyor.yml

@@ -1,5 +1,5 @@
 os: Visual Studio 2017
-version: '2.5.4.{build}'
+version: '2.5.5.{build}'
 skip_tags: true
 install:
 - ps: |

docs/developer/how-viewmodels-are-turned-into-views.md

@@ -65,8 +65,8 @@ This is the basis for converting view models to views.
 
 There are currently two top-level controls for our UI:
 
-- [GitHubDialogWindow](../src/GitHub.VisualStudio/Views/Dialog/GitHubDialogWindow.xaml) for the dialog which shows the login, clone, etc views
-- [GitHubPaneView](../src/GitHub.VisualStudio/Views/GitHubPane/GitHubPaneView.xaml) for the GitHub pane
+- [GitHubDialogWindow](../../src/GitHub.VisualStudio/Views/Dialog/GitHubDialogWindow.xaml) for the dialog which shows the login, clone, etc views
+- [GitHubPaneView](../../src/GitHub.VisualStudio/Views/GitHubPane/GitHubPaneView.xaml) for the GitHub pane
 
 In the resources for each of these top-level controls we define a `DataTemplate` like so:
 
@@ -77,10 +77,10 @@ In the resources for each of these top-level controls we define a `DataTemplate`
 </DataTemplate>
 ```
 
-The `DataTemplate.DataType` here applies the template to all classes inherited from [`GitHub.ViewModels.ViewModelBase`](../src/GitHub.Exports.Reactive/ViewModels/ViewModelBase.cs) [1]. The template defines a single `ContentControl` whose contents are created by a `ViewLocator`.
+The `DataTemplate.DataType` here applies the template to all classes inherited from [`GitHub.ViewModels.ViewModelBase`](../../src/GitHub.Exports.Reactive/ViewModels/ViewModelBase.cs) [1]. The template defines a single `ContentControl` whose contents are created by a `ViewLocator`.
 
-The [`ViewLocator`](../src/GitHub.VisualStudio/Views/ViewLocator.cs) class is an `IValueConverter` which then creates an instance of the appropriate view for the view model using MEF.
+The [`ViewLocator`](../../src/GitHub.VisualStudio/Views/ViewLocator.cs) class is an `IValueConverter` which then creates an instance of the appropriate view for the view model using MEF.
 
 And thus a view model becomes a view.
 
-[1]: it would be nice to make it apply to all classes that inherit `IViewModel` but unfortunately WPF's `DataTemplate`s don't work with interfaces.
+[1]: it would be nice to make it apply to all classes that inherit `IViewModel` but unfortunately WPF's `DataTemplate`s don't work with interfaces.

lib/Octokit.GraphQL.0.0.4-alpha.nupkg

@@ -47,17 +47,14 @@
   </PropertyGroup>
   <Import Project="$(SolutionDir)\src\common\signing.props" />
   <ItemGroup>
-    <Reference Include="Newtonsoft.Json, Version=10.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
-      <HintPath>..\..\packages\Newtonsoft.Json.10.0.3\lib\net45\Newtonsoft.Json.dll</HintPath>
-      <Private>True</Private>
+    <Reference Include="Newtonsoft.Json, Version=9.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Newtonsoft.Json.9.0.1\lib\net45\Newtonsoft.Json.dll</HintPath>
     </Reference>
-    <Reference Include="Octokit.GraphQL, Version=0.0.4.0, Culture=neutral, PublicKeyToken=0be8860aee462442, processorArchitecture=MSIL">
-      <HintPath>..\..\packages\Octokit.GraphQL.0.0.4-alpha\lib\netstandard1.1\Octokit.GraphQL.dll</HintPath>
-      <Private>True</Private>
+    <Reference Include="Octokit.GraphQL, Version=0.1.0.0, Culture=neutral, PublicKeyToken=0be8860aee462442, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Octokit.GraphQL.0.1.0-beta\lib\netstandard1.1\Octokit.GraphQL.dll</HintPath>
     </Reference>
-    <Reference Include="Octokit.GraphQL.Core, Version=0.0.4.0, Culture=neutral, PublicKeyToken=0be8860aee462442, processorArchitecture=MSIL">
-      <HintPath>..\..\packages\Octokit.GraphQL.0.0.4-alpha\lib\netstandard1.1\Octokit.GraphQL.Core.dll</HintPath>
-      <Private>True</Private>
+    <Reference Include="Octokit.GraphQL.Core, Version=0.1.0.0, Culture=neutral, PublicKeyToken=0be8860aee462442, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Octokit.GraphQL.0.1.0-beta\lib\netstandard1.1\Octokit.GraphQL.Core.dll</HintPath>
     </Reference>
     <Reference Include="Serilog, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Serilog.2.5.0\lib\net46\Serilog.dll</HintPath>

src/GitHub.Api/GitHub.Api.csproj

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
-  <package id="Newtonsoft.Json" version="10.0.3" targetFramework="net461" />
-  <package id="Octokit.GraphQL" version="0.0.4-alpha" targetFramework="net461" />
+  <package id="Newtonsoft.Json" version="9.0.1" targetFramework="net461" />
+  <package id="Octokit.GraphQL" version="0.1.0-beta" targetFramework="net461" />
   <package id="Serilog" version="2.5.0" targetFramework="net461" />
 </packages>

src/GitHub.Api/packages.config

@@ -0,0 +1,40 @@
+using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.Threading.Tasks;
+
+namespace GitHub.Collections
+{
+    /// <summary>
+    /// A loader for a virtualizing list.
+    /// </summary>
+    /// <typeparam name="T">The item type.</typeparam>
+    /// <remarks>
+    /// This interface is used by the <see cref="VirtualizingList{T}"/> class to load pages of data.
+    /// </remarks>
+    public interface IVirtualizingListSource<T> : IDisposable, INotifyPropertyChanged
+    {
+        /// <summary>
+        /// Gets a value that indicates where loading is in progress.
+        /// </summary>
+        bool IsLoading { get; }
+
+        /// <summary>
+        /// Gets the page size of the list source.
+        /// </summary>
+        int PageSize { get; }
+
+        /// <summary>
+        /// Gets the total number of items in the list.
+        /// </summary>
+        /// <returns>A task returning the count.</returns>
+        Task<int> GetCount();
+
+        /// <summary>
+        /// Gets the numbered page of items.
+        /// </summary>
+        /// <param name="pageNumber">The page number.</param>
+        /// <returns>A task returning the page contents.</returns>
+        Task<IReadOnlyList<T>> GetPage(int pageNumber);
+    }
+}

src/GitHub.App/Collections/IVirtualizingListSource.cs

@@ -0,0 +1,210 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using System.Windows;
+using System.Windows.Threading;
+using GitHub.Logging;
+using GitHub.Models;
+using ReactiveUI;
+using Serilog;
+
+namespace GitHub.Collections
+{
+    /// <summary>
+    /// An <see cref="IVirtualizingListSource{T}"/> that loads GraphQL pages sequentially, and
+    /// transforms items into a view model after reading.
+    /// </summary>
+    /// <typeparam name="TModel">The type of the model read from the remote data source.</typeparam>
+    /// <typeparam name="TViewModel">The type of the transformed view model.</typeparam>
+    /// <remarks>
+    /// GraphQL can only read pages of data sequentally, so in order to read item 450 (assuming a
+    /// page size of 100), the list source must read pages 1, 2, 3 and 4 in that order. Classes
+    /// deriving from this class only need to implement <see cref="LoadPage(string)"/> to load a
+    /// single page and this class will handle the rest.
+    /// 
+    /// In addition, items will usually need to be transformed into a view model after reading. The
+    /// implementing class overrides <see cref="CreateViewModel(TModel)"/> to carry out that
+    /// transformation.
+    /// </remarks>
+    public abstract class SequentialListSource<TModel, TViewModel> : ReactiveObject, IVirtualizingListSource<TViewModel>
+    {
+        static readonly ILogger log = LogManager.ForContext<SequentialListSource<TModel, TViewModel>>();
+
+        readonly Dispatcher dispatcher;
+        readonly object loadLock = new object();
+        Dictionary<int, Page<TModel>> pages = new Dictionary<int, Page<TModel>>();
+        Task loading = Task.CompletedTask;
+        bool disposed;
+        bool isLoading;
+        int? count;
+        int nextPage;
+        int loadTo;
+        string after;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="SequentialListSource{TModel, TViewModel}"/> class.
+        /// </summary>
+        public SequentialListSource()
+        {
+            dispatcher = Application.Current?.Dispatcher;
+        }
+
+        /// <inheritdoc/>
+        public bool IsLoading
+        {
+            get { return isLoading; }
+            private set { this.RaiseAndSetIfChanged(ref isLoading, value); }
+        }
+
+        /// <inheritdoc/>
+        public virtual int PageSize => 100;
+
+        event EventHandler PageLoaded;
+
+        public void Dispose() => disposed = true;
+
+        /// <inheritdoc/>
+        public async Task<int> GetCount()
+        {
+            dispatcher?.VerifyAccess();
+
+            if (!count.HasValue)
+            {
+                count = (await EnsureLoaded(0).ConfigureAwait(false)).TotalCount;
+            }
+
+            return count.Value;
+        }
+
+        /// <inheritdoc/>
+        public async Task<IReadOnlyList<TViewModel>> GetPage(int pageNumber)
+        {
+            dispatcher?.VerifyAccess();
+
+            var page = await EnsureLoaded(pageNumber);
+
+            if (page == null)
+            {
+                return null;
+            }
+
+            var result = page.Items
+                .Select(CreateViewModel)
+                .ToList();
+            pages.Remove(pageNumber);
+            return result;
+        }
+
+        /// <summary>
+        /// When overridden in a derived class, transforms a model into a view model after loading.
+        /// </summary>
+        /// <param name="model">The model.</param>
+        /// <returns>The view model.</returns>
+        protected abstract TViewModel CreateViewModel(TModel model);
+
+        /// <summary>
+        /// When overridden in a derived class reads a page of results from GraphQL.
+        /// </summary>
+        /// <param name="after">The GraphQL after cursor.</param>
+        /// <returns>A task which returns the page of results.</returns>
+        protected abstract Task<Page<TModel>> LoadPage(string after);
+
+        /// <summary>
+        /// Called when the source begins loading pages.
+        /// </summary>
+        protected virtual void OnBeginLoading()
+        {
+            IsLoading = true;
+        }
+
+        /// <summary>
+        /// Called when the source finishes loading pages.
+        /// </summary>
+        protected virtual void OnEndLoading()
+        {
+            IsLoading = false;
+        }
+
+        async Task<Page<TModel>> EnsureLoaded(int pageNumber)
+        {
+            if (pageNumber < nextPage)
+            {
+                return pages[pageNumber];
+            }
+
+            var pageLoaded = WaitPageLoaded(pageNumber);
+            loadTo = Math.Max(loadTo, pageNumber);
+
+            while (!disposed)
+            {
+                lock (loadLock)
+                {
+                    if (loading.IsCompleted)
+                    {
+                        loading = Load();
+                    }
+                }
+
+                var completed = await Task.WhenAny(loading, pageLoaded).ConfigureAwait(false);
+
+                if (completed.IsFaulted)
+                {
+                    throw completed.Exception;
+                }
+
+                if (pageLoaded.IsCompleted)
+                {
+                    // A previous waiting task may have already returned the page. If so, return null.
+                    pages.TryGetValue(pageNumber, out var result);
+                    return result;
+                }
+            }
+
+            return null;
+        }
+
+        Task WaitPageLoaded(int page)
+        {
+            var tcs = new TaskCompletionSource<bool>();
+            EventHandler handler = null;
+            handler = (s, e) =>
+            {
+                if (nextPage > page)
+                {
+                    tcs.SetResult(true);
+                    PageLoaded -= handler;
+                }
+            };
+            PageLoaded += handler;
+            return tcs.Task;
+        }
+
+        async Task Load()
+        {
+            OnBeginLoading();
+
+            try
+            {
+                while (nextPage <= loadTo && !disposed)
+                {
+                    await LoadNextPage().ConfigureAwait(false);
+                    PageLoaded?.Invoke(this, EventArgs.Empty);
+                }
+            }
+            finally
+            {
+                OnEndLoading();
+            }
+        }
+
+        async Task LoadNextPage()
+        {
+            log.Debug("Loading page {Number} of {ModelType}", nextPage, typeof(TModel));
+
+            var page = await LoadPage(after).ConfigureAwait(false);
+            pages[nextPage++] = page;
+            after = page.EndCursor;
+        }
+    }
+}