Merge pull request #594 from StanleyGoldman/fixes/408-avatar-disappear-pr-list-attempt-2

Fixing Account's CopyFrom

Author: shana

src/GitHub.App/Models/Account.cs

@@ -14,6 +14,8 @@ namespace GitHub.Models
     public class Account : ReactiveObject, IAccount
     {
         BitmapSource avatar;
+        IObservable<BitmapSource> bitmapSource;
+        IDisposable bitmapSourceSubscription;
 
         public Account(
             string login,
@@ -30,8 +32,10 @@ public Account(
             PrivateReposInPlan = privateRepositoryInPlanCount;
             IsOnFreePlan = privateRepositoryInPlanCount == 0;
             HasMaximumPrivateRepositories = OwnedPrivateRepos >= PrivateReposInPlan;
+            this.bitmapSource = bitmapSource;
 
-            bitmapSource.ObserveOn(RxApp.MainThreadScheduler)
+            bitmapSourceSubscription = bitmapSource
+                .ObserveOn(RxApp.MainThreadScheduler)
                 .Subscribe(x => Avatar = x);
         }
 
@@ -80,6 +84,16 @@ public void CopyFrom(IAccount other)
             IsOnFreePlan = other.IsOnFreePlan;
             HasMaximumPrivateRepositories = other.HasMaximumPrivateRepositories;
             Avatar = other.Avatar;
+
+            var otherAccount = other as Account;
+            if (otherAccount != null)
+            {
+                bitmapSourceSubscription.Dispose();
+
+                bitmapSourceSubscription = otherAccount.bitmapSource
+                    .ObserveOn(RxApp.MainThreadScheduler)
+                    .Subscribe(x => Avatar = x);
+            }
         }
 
         public override bool Equals([AllowNull]object obj)

src/UnitTests/GitHub.App/Models/AccountModelTests.cs

@@ -0,0 +1,144 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Reactive.Linq;
+using System.Reactive.Subjects;
+using System.Threading;
+using System.Windows.Media.Imaging;
+using GitHub.Collections;
+using GitHub.Models;
+using GitHub.Services;
+using ReactiveUI;
+using Xunit;
+
+namespace UnitTests.GitHub.App.Models
+{
+    public class AccountModelTests : TestBaseClass
+    {
+        [Fact]
+        public void CopyFromDoesNotLoseAvatar()
+        {
+            var userImage = AvatarProvider.CreateBitmapImage("pack://application:,,,/GitHub.App;component/Images/default_user_avatar.png");
+            var orgImage = AvatarProvider.CreateBitmapImage("pack://application:,,,/GitHub.App;component/Images/default_org_avatar.png");
+
+            var initialBitmapImageSubject = new Subject<BitmapImage>();
+
+            var collectionEvent = new ManualResetEvent(false);
+            var avatarPropertyEvent = new ManualResetEvent(false);
+
+            //Creating an initial account with an observable that returns immediately
+            const string login = "foo";
+            const int initialOwnedPrivateRepositoryCount = 1;
+
+            var initialAccount = new Account(login, true, false, initialOwnedPrivateRepositoryCount, 0, initialBitmapImageSubject);
+
+            //Creating the test collection
+            var col = new TrackingCollection<IAccount>(Observable.Empty<IAccount>(), OrderedComparer<IAccount>.OrderByDescending(x => x.Login).Compare);
+            col.Subscribe(account =>
+            {
+                collectionEvent.Set();
+            }, () => { });
+
+            //Adding that account to the TrackingCollection
+            col.AddItem(initialAccount);
+
+            //Waiting for the collection add the item
+            collectionEvent.WaitOne();
+            collectionEvent.Reset();
+
+            //Checking some initial properties
+            Assert.Equal(login, col[0].Login);
+            Assert.Equal(initialOwnedPrivateRepositoryCount, col[0].OwnedPrivateRepos);
+
+            //Demonstrating that the avatar is not yet present
+            Assert.Null(col[0].Avatar);
+
+            //Adding a listener to check for the changing of the Avatar property
+            initialAccount.Changed.Subscribe(args =>
+            {
+                if (args.PropertyName == "Avatar")
+                {
+                    avatarPropertyEvent.Set();
+                }
+            });
+
+            //Providing the first avatar
+            initialBitmapImageSubject.OnNext(userImage);
+            initialBitmapImageSubject.OnCompleted();
+
+            //Waiting for the avatar to be added
+            avatarPropertyEvent.WaitOne();
+            avatarPropertyEvent.Reset();
+
+            //Demonstrating that the avatar is present
+            Assert.NotNull(col[0].Avatar);
+            Assert.True(BitmapSourcesAreEqual(col[0].Avatar, userImage));
+            Assert.False(BitmapSourcesAreEqual(col[0].Avatar, orgImage));
+
+            //Creating an account update
+            const int updatedOwnedPrivateRepositoryCount = 2;
+            var updatedBitmapImageSubject = new Subject<BitmapImage>();
+            var updatedAccount = new Account(login, true, false, updatedOwnedPrivateRepositoryCount, 0, updatedBitmapImageSubject);
+
+            //Updating the account in the collection
+            col.AddItem(updatedAccount);
+
+            //Waiting for the collection to process the update
+            collectionEvent.WaitOne();
+            collectionEvent.Reset();
+
+            //Providing the second avatar
+            updatedBitmapImageSubject.OnNext(orgImage);
+            updatedBitmapImageSubject.OnCompleted();
+
+            //Waiting for the delayed bitmap image observable
+            avatarPropertyEvent.WaitOne();
+            avatarPropertyEvent.Reset();
+
+            //Login is the id, so that should be the same
+            Assert.Equal(login, col[0].Login);
+
+            //CopyFrom() should have updated this field
+            Assert.Equal(updatedOwnedPrivateRepositoryCount, col[0].OwnedPrivateRepos);
+
+            //CopyFrom() should not cause a race condition here
+            Assert.NotNull(col[0].Avatar);
+            Assert.True(BitmapSourcesAreEqual(col[0].Avatar, orgImage));
+            Assert.False(BitmapSourcesAreEqual(col[0].Avatar, userImage));
+        }
+
+        public static bool BitmapSourcesAreEqual(BitmapSource image1, BitmapSource image2)
+        {
+            if (image1 == null || image2 == null)
+            {
+                return false;
+            }
+
+            return BitmapSourceToBytes(image1).SequenceEqual(BitmapSourceToBytes(image2));
+        }
+
+        public static byte[] BitmapSourceToBytes(BitmapSource image)
+        {
+            byte[] data = new byte[] { };
+            if (image != null)
+            {
+                try
+                {
+                    var encoder = new BmpBitmapEncoder();
+                    encoder.Frames.Add(BitmapFrame.Create(image));
+                    using (MemoryStream ms = new MemoryStream())
+                    {
+                        encoder.Save(ms);
+                        data = ms.ToArray();
+                    }
+                    return data;
+                }
+                catch (Exception ex)
+                {
+                }
+            }
+
+            return data;
+        }
+    }
+}

src/UnitTests/UnitTests.csproj

@@ -160,6 +160,7 @@
     <Compile Include="GitHub.App\Caches\CredentialCacheTests.cs" />
     <Compile Include="GitHub.App\Caches\ImageCacheTests.cs" />
     <Compile Include="GitHub.App\Controllers\UIProviderTests.cs" />
+    <Compile Include="GitHub.App\Models\AccountModelTests.cs" />
     <Compile Include="GitHub.App\Models\ModelServiceTests.cs" />
     <Compile Include="GitHub.App\Controllers\UIControllerTests.cs" />
     <Compile Include="GitHub.App\Models\PullRequestModelTests.cs" />