Changes following PR comments, fixed tests

mansellan · mansellan · commit d4211732a5ff · 2019-07-23T20:35:40.000+01:00
diff --git a/Rubberduck.Core/UI/CodeExplorer/Commands/DeleteCommand.cs b/Rubberduck.Core/UI/CodeExplorer/Commands/DeleteCommand.cs
@@ -13,14 +13,14 @@ namespace Rubberduck.UI.CodeExplorer.Commands
     public class DeleteCommand : CodeExplorerCommandBase
     {
         private readonly RemoveCommand _removeCommand;
-        private readonly IProjectsRepository _projectsRepository;
+        private readonly IProjectsProvider _projectsProvider;
         private readonly IMessageBox _messageBox;
         private readonly IVBE _vbe;
 
-        public DeleteCommand(RemoveCommand removeCommand, IProjectsRepository projectsRepository, IMessageBox messageBox, IVBE vbe)
+        public DeleteCommand(RemoveCommand removeCommand, IProjectsProvider projectsProvider, IMessageBox messageBox, IVBE vbe)
         {
             _removeCommand = removeCommand;
-            _projectsRepository = projectsRepository;
+            _projectsProvider = projectsProvider;
             _messageBox = messageBox;
             _vbe = vbe;
 
@@ -41,7 +41,7 @@ protected override void OnExecute(object parameter)
             }
 
             var qualifiedModuleName = node.Declaration.QualifiedName.QualifiedModuleName;
-            var component = _projectsRepository.Component(qualifiedModuleName);
+            var component = _projectsProvider.Component(qualifiedModuleName);
             if (component is null)
             {
                 return;
diff --git a/Rubberduck.VBEEditor/ComManagement/IProjectsProvider.cs b/Rubberduck.VBEEditor/ComManagement/IProjectsProvider.cs
@@ -14,6 +14,5 @@ public interface IProjectsProvider : IDisposable
         IEnumerable<(QualifiedModuleName QualifiedModuleName, IVBComponent Component)> Components();
         IEnumerable<(QualifiedModuleName QualifiedModuleName, IVBComponent Component)> Components(string projectId);
         IVBComponent Component(QualifiedModuleName qualifiedModuleName);
-        void RemoveComponent(QualifiedModuleName qualifiedModuleName);
     }
 }
diff --git a/Rubberduck.VBEEditor/ComManagement/IProjectsRepository.cs b/Rubberduck.VBEEditor/ComManagement/IProjectsRepository.cs
@@ -4,5 +4,6 @@ public interface IProjectsRepository : IProjectsProvider
     {
         void Refresh();
         void Refresh(string projectId);
+        void RemoveComponent(QualifiedModuleName qualifiedModuleName);
     }
 }
diff --git a/Rubberduck.VBEEditor/ComManagement/ProjectsRepository.cs b/Rubberduck.VBEEditor/ComManagement/ProjectsRepository.cs
@@ -223,15 +223,15 @@ private T EvaluateWithinReadLock<T>(Func<T> function) where T: class
             var readLockTaken = false;
             try
             {
-                _refreshProtectionLock.EnterUpgradeableReadLock();
+                _refreshProtectionLock.EnterReadLock();
                 readLockTaken = true;
                 return function.Invoke();
             }
             finally
             {
                 if (readLockTaken)
                 {
-                    _refreshProtectionLock.ExitUpgradeableReadLock();
+                    _refreshProtectionLock.ExitReadLock();
                 }
             }
         }
@@ -276,25 +276,16 @@ public IVBComponent Component(QualifiedModuleName qualifiedModuleName)
 
         public void RemoveComponent(QualifiedModuleName qualifiedModuleName)
         {
-            EvaluateWithinReadLock(() =>
+            ExecuteWithinWriteLock(() =>
             {
-                if (_components.TryGetValue(qualifiedModuleName, out var component))
+                if (!_components.TryGetValue(qualifiedModuleName, out var component) ||
+                    !_componentsCollections.TryGetValue(qualifiedModuleName.ProjectId, out var components))
                 {
-                    ExecuteWithinWriteLock(() =>
-                    {
-                        if (_projects.TryGetValue(qualifiedModuleName.ProjectId, out var project))
-                        {
-                            using (var components = project.VBComponents)
-                            {
-                                // Remove the actual component...
-                                components.Remove(component);
-                            }
-                        }
-                        // ...and our cached copy of it
-                        _components.Remove(qualifiedModuleName);
-                    });
+                    return;
                 }
-                return new { };
+
+                _components.Remove(qualifiedModuleName);
+                components.Remove(component);                
             });
         }
 
diff --git a/RubberduckTests/CodeExplorer/CodeExplorerViewModelTests.cs b/RubberduckTests/CodeExplorer/CodeExplorerViewModelTests.cs
@@ -467,7 +467,7 @@ public void RemoveCommand_RemovesModuleWhenPromptOk()
                 var component = explorer.VbComponent.Object;
 
                 explorer.ViewModel.RemoveCommand.Execute(removing);
-                explorer.VbComponents.Verify(c => c.Remove(component), Times.Once);
+                explorer.ProjectsRepository.Verify(c => c.RemoveComponent(component.QualifiedModuleName), Times.Once);
             }
         }
 
@@ -503,8 +503,9 @@ public void RemoveCommand_GivenMsgBoxNo_RemovesModuleNoExport()
                 var removing = explorer.ViewModel.SelectedItem;
                 var component = explorer.VbComponent.Object;
 
+
                 explorer.ViewModel.RemoveCommand.Execute(removing);
-                explorer.VbComponents.Verify(c => c.Remove(component), Times.Once);
+                explorer.ProjectsRepository.Verify(c => c.RemoveComponent(component.QualifiedModuleName), Times.Once);
             }
         }
 
diff --git a/RubberduckTests/CodeExplorer/MockedCodeExplorer.cs b/RubberduckTests/CodeExplorer/MockedCodeExplorer.cs
@@ -24,6 +24,8 @@
 using Rubberduck.UnitTesting;
 using Rubberduck.UnitTesting.CodeGeneration;
 using Rubberduck.UnitTesting.Settings;
+using Rubberduck.VBEditor;
+using Rubberduck.VBEditor.ComManagement;
 using Rubberduck.VBEditor.SourceCodeHandling;
 using Rubberduck.VBEditor.Utility;
 using RubberduckTests.Settings;
@@ -87,6 +89,10 @@ public MockedCodeExplorer(ProjectType projectType, ComponentType componentType =
             VbProject = project.Build();
             Vbe = builder.AddProject(VbProject).Build();
 
+            ProjectsRepository = new Mock<IProjectsRepository>();
+            ProjectsRepository.Setup(x => x.Project(It.IsAny<string>())).Returns(VbProject.Object);
+            ProjectsRepository.Setup(x => x.Component(It.IsAny<QualifiedModuleName>())).Returns(VbComponent.Object);
+
             SetupViewModelAndParse();
         }
 
@@ -120,6 +126,10 @@ public MockedCodeExplorer(ProjectType projectType,
             VbProject = project.Build();
             Vbe = builder.AddProject(VbProject).Build();
 
+            ProjectsRepository = new Mock<IProjectsRepository>();
+            ProjectsRepository.Setup(x => x.Project(It.IsAny<string>())).Returns(VbProject.Object);
+            ProjectsRepository.Setup(x => x.Component(It.IsAny<QualifiedModuleName>())).Returns(VbComponent.Object);
+
             SetupViewModelAndParse();
 
             VbProject.SetupGet(m => m.VBComponents.Count).Returns(componentTypes.Count);
@@ -130,7 +140,8 @@ private void SetupViewModelAndParse()
             var parser = MockParser.Create(Vbe.Object, null, MockVbeEvents.CreateMockVbeEvents(Vbe));
             State = parser.State;
 
-            var removeCommand = new RemoveCommand(BrowserFactory.Object, MessageBox.Object, State.ProjectsProvider);
+            var exportCommand = new ExportCommand(BrowserFactory.Object, MessageBox.Object, State.ProjectsProvider, Vbe.Object);
+            var removeCommand = new RemoveCommand(exportCommand, ProjectsRepository.Object, MessageBox.Object, Vbe.Object);
 
             ViewModel = new CodeExplorerViewModel(State, removeCommand,
                 _generalSettingsProvider.Object,
@@ -157,6 +168,7 @@ private void SetupViewModelAndParse()
         public Mock<IOpenFileDialog> OpenDialog { get; }
         public Mock<IFolderBrowser> FolderBrowser { get; }
         public Mock<IMessageBox> MessageBox { get; } = new Mock<IMessageBox>();
+        public Mock<IProjectsRepository> ProjectsRepository { get; }
 
         public WindowSettings WindowSettings { get; } = new WindowSettings();
 
@@ -354,7 +366,7 @@ public void ExecuteExportCommand()
 
         public MockedCodeExplorer ImplementExportCommand()
         {
-            ViewModel.ExportCommand = new ExportCommand(BrowserFactory.Object, MessageBox.Object, State.ProjectsProvider);
+            ViewModel.ExportCommand = new ExportCommand(BrowserFactory.Object, MessageBox.Object, State.ProjectsProvider, Vbe.Object);
             return this;
         }
 

Original file line number	Diff line number	Diff line change
`@@ -13,14 +13,14 @@ namespace Rubberduck.UI.CodeExplorer.Commands`
`13`	`13`	`public class DeleteCommand : CodeExplorerCommandBase`
`14`	`14`	`{`
`15`	`15`	`private readonly RemoveCommand _removeCommand;`
`16`		`- private readonly IProjectsRepository _projectsRepository;`
	`16`	`+ private readonly IProjectsProvider _projectsProvider;`
`17`	`17`	`private readonly IMessageBox _messageBox;`
`18`	`18`	`private readonly IVBE _vbe;`
`19`	`19`
`20`		`- public DeleteCommand(RemoveCommand removeCommand, IProjectsRepository projectsRepository, IMessageBox messageBox, IVBE vbe)`
	`20`	`+ public DeleteCommand(RemoveCommand removeCommand, IProjectsProvider projectsProvider, IMessageBox messageBox, IVBE vbe)`
`21`	`21`	`{`
`22`	`22`	`_removeCommand = removeCommand;`
`23`		`- _projectsRepository = projectsRepository;`
	`23`	`+ _projectsProvider = projectsProvider;`
`24`	`24`	`_messageBox = messageBox;`
`25`	`25`	`_vbe = vbe;`
`26`	`26`
`@@ -41,7 +41,7 @@ protected override void OnExecute(object parameter)`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`var qualifiedModuleName = node.Declaration.QualifiedName.QualifiedModuleName;`
`44`		`- var component = _projectsRepository.Component(qualifiedModuleName);`
	`44`	`+ var component = _projectsProvider.Component(qualifiedModuleName);`
`45`	`45`	`if (component is null)`
`46`	`46`	`{`
`47`	`47`	`return;`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,5 @@ public interface IProjectsProvider : IDisposable`
`14`	`14`	`IEnumerable<(QualifiedModuleName QualifiedModuleName, IVBComponent Component)> Components();`
`15`	`15`	`IEnumerable<(QualifiedModuleName QualifiedModuleName, IVBComponent Component)> Components(string projectId);`
`16`	`16`	`IVBComponent Component(QualifiedModuleName qualifiedModuleName);`
`17`		`- void RemoveComponent(QualifiedModuleName qualifiedModuleName);`
`18`	`17`	`}`
`19`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,6 @@ public interface IProjectsRepository : IProjectsProvider`
`4`	`4`	`{`
`5`	`5`	`void Refresh();`
`6`	`6`	`void Refresh(string projectId);`
	`7`	`+ void RemoveComponent(QualifiedModuleName qualifiedModuleName);`
`7`	`8`	`}`
`8`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -223,15 +223,15 @@ private T EvaluateWithinReadLock<T>(Func<T> function) where T: class`
`223`	`223`	`var readLockTaken = false;`
`224`	`224`	`try`
`225`	`225`	`{`
`226`		`- _refreshProtectionLock.EnterUpgradeableReadLock();`
	`226`	`+ _refreshProtectionLock.EnterReadLock();`
`227`	`227`	`readLockTaken = true;`
`228`	`228`	`return function.Invoke();`
`229`	`229`	`}`
`230`	`230`	`finally`
`231`	`231`	`{`
`232`	`232`	`if (readLockTaken)`
`233`	`233`	`{`
`234`		`- _refreshProtectionLock.ExitUpgradeableReadLock();`
	`234`	`+ _refreshProtectionLock.ExitReadLock();`
`235`	`235`	`}`
`236`	`236`	`}`
`237`	`237`	`}`
`@@ -276,25 +276,16 @@ public IVBComponent Component(QualifiedModuleName qualifiedModuleName)`
`276`	`276`
`277`	`277`	`public void RemoveComponent(QualifiedModuleName qualifiedModuleName)`
`278`	`278`	`{`
`279`		`- EvaluateWithinReadLock(() =>`
	`279`	`+ ExecuteWithinWriteLock(() =>`
`280`	`280`	`{`
`281`		`- if (_components.TryGetValue(qualifiedModuleName, out var component))`
	`281`	`+ if (!_components.TryGetValue(qualifiedModuleName, out var component) \|\|`
	`282`	`+ !_componentsCollections.TryGetValue(qualifiedModuleName.ProjectId, out var components))`
`282`	`283`	`{`
`283`		`- ExecuteWithinWriteLock(() =>`
`284`		`- {`
`285`		`- if (_projects.TryGetValue(qualifiedModuleName.ProjectId, out var project))`
`286`		`- {`
`287`		`- using (var components = project.VBComponents)`
`288`		`- {`
`289`		`- // Remove the actual component...`
`290`		`- components.Remove(component);`
`291`		`- }`
`292`		`- }`
`293`		`- // ...and our cached copy of it`
`294`		`- _components.Remove(qualifiedModuleName);`
`295`		`- });`
	`284`	`+ return;`
`296`	`285`	`}`
`297`		`- return new { };`
	`286`	`+`
	`287`	`+ _components.Remove(qualifiedModuleName);`
	`288`	`+ components.Remove(component);`
`298`	`289`	`});`
`299`	`290`	`}`
`300`	`291`
Original file line number	Diff line number	Diff line change
`@@ -467,7 +467,7 @@ public void RemoveCommand_RemovesModuleWhenPromptOk()`
`467`	`467`	`var component = explorer.VbComponent.Object;`
`468`	`468`
`469`	`469`	`explorer.ViewModel.RemoveCommand.Execute(removing);`
`470`		`- explorer.VbComponents.Verify(c => c.Remove(component), Times.Once);`
	`470`	`+ explorer.ProjectsRepository.Verify(c => c.RemoveComponent(component.QualifiedModuleName), Times.Once);`
`471`	`471`	`}`
`472`	`472`	`}`
`473`	`473`
`@@ -503,8 +503,9 @@ public void RemoveCommand_GivenMsgBoxNo_RemovesModuleNoExport()`
`503`	`503`	`var removing = explorer.ViewModel.SelectedItem;`
`504`	`504`	`var component = explorer.VbComponent.Object;`
`505`	`505`
	`506`	`+`
`506`	`507`	`explorer.ViewModel.RemoveCommand.Execute(removing);`
`507`		`- explorer.VbComponents.Verify(c => c.Remove(component), Times.Once);`
	`508`	`+ explorer.ProjectsRepository.Verify(c => c.RemoveComponent(component.QualifiedModuleName), Times.Once);`
`508`	`509`	`}`
`509`	`510`	`}`
`510`	`511`