Fixed loop limit bug in Task.TryRemove for the ObservableConcurrentCollection.
authorPanagiotis Kanavos <pkanavos@gmail.com>
Thu, 1 Mar 2012 16:57:15 +0000 (18:57 +0200)
committerPanagiotis Kanavos <pkanavos@gmail.com>
Thu, 1 Mar 2012 16:57:15 +0000 (18:57 +0200)
Fixes #2131

trunk/Pithos.Client.WPF/Preferences/PreferencesViewModel.cs
trunk/Pithos.Client.WPF/Shell/ShellView.xaml
trunk/Pithos.Client.WPF/Shell/ShellViewModel.cs
trunk/Pithos.Core.Test/FileSystemWatcherAdapterTest.cs [new file with mode: 0644]
trunk/Pithos.Core.Test/Pithos.Core.Test.csproj
trunk/Pithos.Core.Test/TaskExtensionsTest.cs [new file with mode: 0644]
trunk/Pithos.Core/TaskExtensions.cs

index 1b8a400..fbe78fe 100644 (file)
@@ -42,6 +42,7 @@
 
 
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Diagnostics;
 using System.IO;
@@ -280,6 +281,12 @@ namespace Pithos.Client.WPF.Preferences
             Settings.Save();
             //SetStartupMode();            
 
+            foreach (var account in _accountsToRemove)
+            {
+                Settings.Accounts.Remove(account);
+                Shell.RemoveMonitor(account.AccountName);
+            }
+
             foreach (var account in Settings.Accounts)
             {                                
                 Shell.MonitorAccount(account);
@@ -374,20 +381,18 @@ namespace Pithos.Client.WPF.Preferences
             NotifyOfPropertyChange(()=>Settings);                       
        }
 
+
+        readonly List<AccountSettings> _accountsToRemove = new List<AccountSettings>();
         public void RemoveAccount()
         {
             var accountName = CurrentAccount.AccountName;
-            Settings.Accounts.Remove(CurrentAccount.Account);
 
             Accounts.TryRemove(CurrentAccount);
-            
-            
+            _accountsToRemove.Add(CurrentAccount.Account);
+
             CurrentAccount = null;
-            //Accounts = Settings.Accounts;
-            //Settings.Save();            
-            Shell.RemoveMonitor(accountName);
             NotifyOfPropertyChange(() => Accounts);
-            NotifyOfPropertyChange(() => Settings);                       
+
             
             //NotifyOfPropertyChange("Settings.Accounts");
         }
index 0ecc694..3ed44c7 100644 (file)
@@ -63,7 +63,8 @@
                     <MenuItem Header="Recently Changed Files" x:Name="RecentFiles" ItemsSource="{Binding RecentFiles}">
                         <MenuItem.ItemTemplate>
                             <DataTemplate>
-                                <TextBlock Text="{Binding FileName}"/>
+                                <TextBlock Text="{Binding FileName}" cal:Message.Attach="[Event MouseLeftButtonUp]=[Action GoToFile($dataContext)]" 
+                                           cal:Action.TargetWithoutContext="{Binding RelativeSource={RelativeSource FindAncestor, AncestorType=MenuItem, AncestorLevel=2}, Path=DataContext}" />
                             </DataTemplate>
                         </MenuItem.ItemTemplate>
                     </MenuItem>
index f5c1ddb..d6a1e79 100644 (file)
@@ -55,7 +55,6 @@ using Hardcodet.Wpf.TaskbarNotification;
 using Pithos.Client.WPF.Configuration;
 using Pithos.Client.WPF.FileProperties;
 using Pithos.Client.WPF.Preferences;
-using Pithos.Client.WPF.Properties;
 using Pithos.Client.WPF.SelectiveSynch;
 using Pithos.Client.WPF.Services;
 using Pithos.Client.WPF.Shell;
@@ -122,9 +121,9 @@ namespace Pithos.Client.WPF {
                private static readonly log4net.ILog Log = log4net.LogManager.GetLogger("Pithos");
 
                //Lazily initialized File Version info. This is done once and lazily to avoid blocking the UI
-               private Lazy<FileVersionInfo> _fileVersion;
+               private readonly Lazy<FileVersionInfo> _fileVersion;
 
-           private PollAgent _pollAgent;
+           private readonly PollAgent _pollAgent;
 
                ///<summary>
                /// The Shell depends on MEF to provide implementations for windowManager, events, the status checker service and the settings
@@ -408,12 +407,22 @@ namespace Pithos.Client.WPF {
 
                public void GoToSite(AccountInfo account)
                {
-                       /*var site = String.Format("{0}/ui/?token={1}&user={2}",
-                               account.SiteUri,account.Token,
-                               account.UserName);*/
                        Process.Start(account.SiteUri);
                }
 
+        /// <summary>
+        /// Open an explorer window to the target path's directory
+        /// and select the file
+        /// </summary>
+        /// <param name="entry"></param>
+        public void GoToFile(FileEntry entry)
+        {
+            var fullPath = entry.FullPath;
+            if (!File.Exists(fullPath) && !Directory.Exists(fullPath))
+                return;
+            Process.Start("explorer.exe","/select, " + fullPath);
+        }
+
                public void ShowFileProperties()
                {
                        var account = Settings.Accounts.First(acc => acc.IsActive);            
@@ -623,7 +632,7 @@ namespace Pithos.Client.WPF {
                        Execute.OnUIThread(() =>
                                                                   {                                       
                                                                           var proxyAccount = IoC.Get<ProxyAccountViewModel>();
-                                                                               proxyAccount.Settings = this.Settings;
+                                                                               proxyAccount.Settings = Settings;
                                                                           if (true != _windowManager.ShowDialog(proxyAccount)) 
                                                                                   return;
                                                                           StartMonitor(monitor, retries);
@@ -645,20 +654,7 @@ namespace Pithos.Client.WPF {
                }
 
 
-
-               private static bool IsUnauthorized(WebException exc)
-               {
-                       if (exc==null)
-                               throw new ArgumentNullException("exc");
-                       Contract.EndContractBlock();
-
-                       var response = exc.Response as HttpWebResponse;
-                       if (response == null)
-                               return false;
-                       return (response.StatusCode == HttpStatusCode.Unauthorized);
-               }
-
-               private void TryLater(PithosMonitor monitor, Exception exc,int retries)
+           private void TryLater(PithosMonitor monitor, Exception exc,int retries)
                {
                        var message = String.Format("An exception occured. Can't start monitoring\nWill retry in 10 seconds");
                        Task.Factory.StartNewDelayed(10000, () => StartMonitor(monitor,retries+1));
@@ -677,11 +673,14 @@ namespace Pithos.Client.WPF {
 
                public void NotifyChangedFile(string filePath)
                {
-                       var entry = new FileEntry {FullPath=filePath};
+            if (RecentFiles.Any(e => e.FullPath == filePath))
+                return;
+            
                        IProducerConsumerCollection<FileEntry> files=RecentFiles;
                        FileEntry popped;
                        while (files.Count > 5)
                                files.TryTake(out popped);
+            var entry = new FileEntry { FullPath = filePath };
                        files.TryAdd(entry);
                }
 
@@ -703,12 +702,14 @@ namespace Pithos.Client.WPF {
                {
                        if (conflictFiles == null)
                                return;
-                       if (!conflictFiles.Any())
+                   //Convert to list to avoid multiple iterations
+            var files = conflictFiles.ToList();
+                       if (files.Count==0)
                                return;
 
                        UpdateStatus();
                        //TODO: Create a more specific message. For now, just show a warning
-                       NotifyForFiles(conflictFiles,message,TraceLevel.Warning);
+                       NotifyForFiles(files,message,TraceLevel.Warning);
 
                }
 
@@ -743,6 +744,8 @@ namespace Pithos.Client.WPF {
                        if (Monitors.TryRemove(accountName, out monitor))
                        {
                                monitor.Stop();
+                //TODO: Also remove any pending actions for this account
+                //from the network queue                
                        }
                }
 
@@ -790,7 +793,7 @@ namespace Pithos.Client.WPF {
                }
 
 
-               private bool _pollStarted = false;
+               private bool _pollStarted;
 
                //SMELL: Doing so much work for notifications in the shell is wrong
                //The notifications should be moved to their own view/viewmodel pair
diff --git a/trunk/Pithos.Core.Test/FileSystemWatcherAdapterTest.cs b/trunk/Pithos.Core.Test/FileSystemWatcherAdapterTest.cs
new file mode 100644 (file)
index 0000000..6b74cd2
--- /dev/null
@@ -0,0 +1,22 @@
+// -----------------------------------------------------------------------
+// <copyright file="FileSystemWatcherAdapter.cs" company="Microsoft">
+// TODO: Update copyright text.
+// </copyright>
+// -----------------------------------------------------------------------
+
+using NUnit.Framework;
+
+namespace Pithos.Core.Test
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+
+    [TestFixture]
+    public class FileSystemWatcherAdapterTest
+    {
+        
+
+    }
+}
index cb9bf18..55afdfe 100644 (file)
     <Compile Include="MockSettings.cs" />
     <Compile Include="MockStatusKeeper.cs" />
     <Compile Include="SnapshotDifferencerTest.cs" />
+    <Compile Include="TaskExtensionsTest.cs" />
     <Compile Include="WorkflowFileStatusTest.cs" />
   </ItemGroup>
   <ItemGroup>
diff --git a/trunk/Pithos.Core.Test/TaskExtensionsTest.cs b/trunk/Pithos.Core.Test/TaskExtensionsTest.cs
new file mode 100644 (file)
index 0000000..94592f2
--- /dev/null
@@ -0,0 +1,46 @@
+// -----------------------------------------------------------------------
+// <copyright file="TaskExtensionsTest.cs" company="Microsoft">
+// TODO: Update copyright text.
+// </copyright>
+// -----------------------------------------------------------------------
+
+using System.Collections.Concurrent;
+using NUnit.Framework;
+
+namespace Pithos.Core.Test
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Linq;
+    using System.Text;
+
+    [TestFixture]
+    public class TaskExtensionsTest
+    {
+        [Test]
+        public void when_adding_to_ObservableConcurrentCollection()
+        {
+            var collection = new ObservableConcurrentCollection<string>();
+            collection.TryAdd("1");
+            Assert.That(collection,Contains.Item("1"));
+            collection.TryAdd("2");
+            Assert.That(collection, Contains.Item("2"));
+            Assert.That(collection, Contains.Item("1"));
+            
+        }
+
+        [Test]
+        public void when_removing_from_ObservableConcurrentCollection()
+        {
+            var collection = new ObservableConcurrentCollection<string>();
+            collection.TryAdd("1");
+            collection.TryAdd("2");
+            collection.TryAdd("3");
+            collection.TryRemove("2");
+            Assert.That(collection, Contains.Item("1"));
+            Assert.That(collection.Contains("2"), Is.False);
+            Assert.That(collection, Contains.Item("3"));
+            
+        }
+    }
+}
index 30a672d..fce3827 100644 (file)
@@ -157,17 +157,25 @@ namespace Pithos.Core
 
         public static bool TryRemove<T>(this ObservableConcurrentCollection<T> collection,T item) where T:class
         {
+            var found = false;
             IProducerConsumerCollection<T> items= collection;
-            for (var i = 0; i < items.Count; i++)
+            //Store the initial count
+            var count = items.Count;
+            for (var i = 0; i < count; i++)
             {
                 T tempItem;
+                //Take an item
                 if (!items.TryTake(out tempItem)) 
                     return false;
-                if (tempItem == item) 
-                    return true;
-                items.TryAdd(item);
+                //If it isn't the one we are looking for
+                if (tempItem != item)
+                    //put it back
+                    items.TryAdd(tempItem);
+                else
+                    //otherwise skip it and flag succcess
+                    found = true;
             }
-            return false;
+            return found;
         }
 
         public static bool TryAdd<T>(this ObservableConcurrentCollection<T> collection,T item) where T:class