Code review feedback

--HG--
extra : convert_revision : svn%3A5ff7c347-ad56-4c35-b696-ccb81de16e03/trunk%4040237

src/Orchard.Tests/UI/Navigation/MenuItemComparerTests.cs

@@ -54,7 +54,7 @@ namespace Orchard.Tests.UI.Navigation {
         public void PositionAndChildrenDontMatter() {
             var item1 = new MenuItem { Text = "hello", RouteValues = new RouteValueDictionary(new { one = "1", two = "2" }) };
             var item2 = new MenuItem { Text = "hello", RouteValues = new RouteValueDictionary(new { one = "1", two = "2" }), Position = "4.0" };
-            var item3 = new MenuItem { Text = "hello", RouteValues = new RouteValueDictionary(new { one = "1", two = "2" }), Contained = new[] { new MenuItem() } };
+            var item3 = new MenuItem { Text = "hello", RouteValues = new RouteValueDictionary(new { one = "1", two = "2" }), Items = new[] { new MenuItem() } };
             AssertSameSameSame(item1, item2, item3);
         }
 

src/Orchard.Tests/UI/Navigation/NavigationManagerTests.cs

@@ -24,8 +24,8 @@ namespace Orchard.Tests.UI.Navigation {
             Assert.That(menuItems.Count(), Is.EqualTo(2));
             Assert.That(menuItems.First(), Has.Property("Text").EqualTo("Foo"));
             Assert.That(menuItems.Last(), Has.Property("Text").EqualTo("Bar"));
-            Assert.That(menuItems.Last().Contained.Count(), Is.EqualTo(1));
+            Assert.That(menuItems.Last().Items.Count(), Is.EqualTo(1));
-            Assert.That(menuItems.Last().Contained.Single().Text, Is.EqualTo("Frap"));
+            Assert.That(menuItems.Last().Items.Single().Text, Is.EqualTo("Frap"));
         }
 
         [Test]
@@ -46,9 +46,9 @@ namespace Orchard.Tests.UI.Navigation {
             Assert.That(item3.Text, Is.EqualTo("Frap"));
             Assert.That(item3.Position, Is.EqualTo("3.0"));
 
-            Assert.That(item2.Contained.Count(), Is.EqualTo(2));
+            Assert.That(item2.Items.Count(), Is.EqualTo(2));
-            var subitem1 = item2.Contained.First();
+            var subitem1 = item2.Items.First();
-            var subitem2 = item2.Contained.Last();
+            var subitem2 = item2.Items.Last();
             Assert.That(subitem1.Text, Is.EqualTo("Quad"));
             Assert.That(subitem1.Position, Is.EqualTo("1.a"));
             Assert.That(subitem2.Text, Is.EqualTo("Frap"));

src/Orchard.Web/Packages/Orchard.Users/Services/MembershipService.cs

@@ -41,7 +41,7 @@ namespace Orchard.Users.Services {
             return _modelManager.Get(userRecord.Id).As<IUser>();
         }
 
-        public IUser Identify(string username, string password) {
+        public IUser ValidateUser(string username, string password) {
             return GetUser(username);
         }
     }

src/Orchard.Web/Views/Admin/Footer.aspx

@@ -13,7 +13,7 @@
             
             <%if (Model.AdminMenu != null) {
                   foreach (var menuSection in Model.AdminMenu) {%>
-                  <div class="leftNavMod"><h4><%=Html.Encode(menuSection.Text)%></h4><ul><%foreach (var menuItem in menuSection.Contained) { %>
+                  <div class="leftNavMod"><h4><%=Html.Encode(menuSection.Text)%></h4><ul><%foreach (var menuItem in menuSection.Items) { %>
                   <li><%=Html.ActionLink(menuItem.Text, (string)menuItem.RouteValues["action"], menuItem.RouteValues)%></li>
                   <%} %></ul></div>
             <%

src/Orchard/Controllers/AccountController.cs

@@ -154,7 +154,7 @@ namespace Orchard.Controllers {
             if (String.IsNullOrEmpty(password)) {
                 ModelState.AddModelError("password", "You must specify a password.");
             }
-            var user = _membershipService.Identify(userName, password);
+            var user = _membershipService.ValidateUser(userName, password);
             if (user == null) {
                 ModelState.AddModelError("_FORM", "The username or password provided is incorrect.");
             }

src/Orchard/Orchard.csproj

@@ -163,7 +163,7 @@
     <Compile Include="Security\SecurityModule.cs" />
     <Compile Include="UI\Menus\AdminMenuFilter.cs" />
     <Compile Include="UI\Models\ModelEditor.cs" />
-    <Compile Include="UI\Navigation\INavigationBuilder.cs" />
+    <Compile Include="UI\Navigation\NavigationBuilder.cs" />
     <Compile Include="UI\Navigation\INavigationProvider.cs" />
     <Compile Include="UI\Navigation\MenuItem.cs" />
     <Compile Include="UI\Navigation\MenuItemComparer.cs" />

src/Orchard/Security/CreateUserParams.cs

@@ -1,4 +1,5 @@
 namespace Orchard.Security {
+    //TEMP: Add setters, provide default constructor and remove parameterized constructor
     public class CreateUserParams {
         private readonly string _username;
         private readonly string _password;

src/Orchard/Security/IMembershipService.cs

@@ -11,7 +11,7 @@ namespace Orchard.Security {
         IUser CreateUser(CreateUserParams createUserParams);
         IUser GetUser(string username);
 
-        IUser Identify(string username, string password);
+        IUser ValidateUser(string username, string password);
     }
 
     public class MembershipSettings {

src/Orchard/Security/Providers/OrchardMembershipProvider.cs

@@ -76,7 +76,7 @@ namespace Orchard.Security.Providers {
         }
 
         public override bool ValidateUser(string username, string password) {
-            return true;
+            return (GetService().ValidateUser(username, password) != null);
         }
 
         public override bool UnlockUser(string userName) {

src/Orchard/Storage/FileSystemStorageProvider.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 
 namespace Orchard.Storage {
     public class FileSystemStorageProvider : IStorageProvider {
@@ -10,7 +11,7 @@ namespace Orchard.Storage {
             if (!File.Exists(path)) {
                 throw new ArgumentException("File " + path + " does not exist");
             }
-            return new FileSystemStorageFile(path);
+            return new FileSystemStorageFile(new FileInfo(path));
         }
 
         public IEnumerable<IStorageFile> ListFiles(string path) {
@@ -18,12 +19,11 @@ namespace Orchard.Storage {
                 throw new ArgumentException("Directory " + path + " does not exist");
             }
 
-            List<IStorageFile> files = new List<IStorageFile>();
+            return new DirectoryInfo(path)
-            foreach (string file in Directory.GetFiles(path)) {
+                .GetFiles()
-                files.Add(new FileSystemStorageFile(file));
+                .Where(fi => !IsHidden(fi))
-            }
+                .Select<FileInfo, IStorageFile>(fi => new FileSystemStorageFile(fi))
-
+                .ToList();
-            return files;
         }
 
         public IEnumerable<IStorageFolder> ListFolders(string path) {
@@ -31,13 +31,15 @@ namespace Orchard.Storage {
                 throw new ArgumentException("Directory " + path + " does not exist");
             }
 
-            List<IStorageFolder> folders = new List<IStorageFolder>();
+            return new DirectoryInfo(path)
+                .GetDirectories()
+                .Where(di => !IsHidden(di))
+                .Select<DirectoryInfo, IStorageFolder>(di => new FileSystemStorageFolder(di))
+                .ToList();
+        }
 
-            foreach (string folder in Directory.GetDirectories(path)) {
+        private static bool IsHidden(FileSystemInfo di) {
-                folders.Add(new FileSystemStorageFolder(folder));
+            return (di.Attributes & FileAttributes.Hidden) != 0;
-            }
-
-            return folders;
         }
 
         public void CreateFolder(string path) {
@@ -69,12 +71,13 @@ namespace Orchard.Storage {
         }
 
         public IStorageFile CreateFile(string path) {
-            if (!File.Exists(path)) {
+            if (File.Exists(path)) {
                 throw new ArgumentException("File " + path + " already exists");
             }
 
-            File.Create(path);
+            var fileInfo = new FileInfo(path);
-            return new FileSystemStorageFile(path);
+            fileInfo.Create();
+            return new FileSystemStorageFile(fileInfo);
         }
 
         public void DeleteFile(string path) {
@@ -102,8 +105,8 @@ namespace Orchard.Storage {
         private class FileSystemStorageFile : IStorageFile {
             private readonly FileInfo _fileInfo;
 
-            public FileSystemStorageFile(string path) {
+            public FileSystemStorageFile(FileInfo fileInfo) {
-                _fileInfo = new FileInfo(path);
+                _fileInfo = fileInfo;
             }
 
             #region Implementation of IStorageFile
@@ -138,8 +141,8 @@ namespace Orchard.Storage {
         private class FileSystemStorageFolder : IStorageFolder {
             private readonly DirectoryInfo _directoryInfo;
 
-            public FileSystemStorageFolder(string path) {
+            public FileSystemStorageFolder(DirectoryInfo directoryInfo) {
-                _directoryInfo = new DirectoryInfo(path);
+                _directoryInfo = directoryInfo;
             }
 
             #region Implementation of IStorageFolder
@@ -158,9 +161,9 @@ namespace Orchard.Storage {
 
             public IStorageFolder GetParent() {
                 if (_directoryInfo.Parent != null) {
-                    return new FileSystemStorageFolder(_directoryInfo.Parent.FullName);
+                    return new FileSystemStorageFolder(_directoryInfo.Parent);
                 }
-                throw new ArgumentException("Directory " + _directoryInfo.Name + "does not have a parent directory");
+                throw new ArgumentException("Directory " + _directoryInfo.Name + " does not have a parent directory");
             }
 
             #endregion

src/Orchard/UI/Navigation/MenuItem.cs

@@ -6,6 +6,6 @@ namespace Orchard.UI.Navigation {
         public string Text { get; set; }
         public string Position { get; set; }
         public RouteValueDictionary RouteValues { get; set; }
-        public IEnumerable<MenuItem> Contained { get; set; }
+        public IEnumerable<MenuItem> Items { get; set; }
     }
 }

src/Orchard/UI/Navigation/NavigationBuilder.cs

@@ -0,0 +1,85 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Web.Routing;
+
+namespace Orchard.UI.Navigation {
+    public class NavigationBuilder {
+        IEnumerable<MenuItem> Contained { get; set; }
+
+        public NavigationBuilder Add(string caption, string position, Action<NavigationItemBuilder> itemBuilder) {
+            var childBuilder = new NavigationItemBuilder();
+
+            if (!string.IsNullOrEmpty(caption))
+                childBuilder.Caption(caption);
+
+            if (!string.IsNullOrEmpty(position))
+                childBuilder.Position(position);
+
+            itemBuilder(childBuilder);
+            Contained = (Contained ?? Enumerable.Empty<MenuItem>()).Concat(childBuilder.Build());
+            return this;
+        }
+
+        public NavigationBuilder Add(string caption, Action<NavigationItemBuilder> itemBuilder) {
+            return Add(caption, null, itemBuilder);
+        }
+        public NavigationBuilder Add(Action<NavigationItemBuilder> itemBuilder) {
+            return Add(null, null, itemBuilder);
+        }
+        public NavigationBuilder Add(string caption, string position) {
+            return Add(caption, position, x=> { });
+        }
+        public NavigationBuilder Add(string caption) {
+            return Add(caption, null, x => { });
+        }
+
+        public IEnumerable<MenuItem> Build() {
+            return (Contained ?? Enumerable.Empty<MenuItem>()).ToList();
+        }
+    }
+
+    public class NavigationItemBuilder : NavigationBuilder {
+        private readonly MenuItem _item;
+
+        public NavigationItemBuilder() {
+            _item = new MenuItem();
+        }
+
+        public NavigationItemBuilder Caption(string caption) {
+            _item.Text = caption;
+            return this;
+        }
+
+        public NavigationItemBuilder Position(string position) {
+            _item.Position = position;
+            return this;
+        }
+
+        public new IEnumerable<MenuItem> Build() {
+            _item.Items = base.Build();
+            return new[] { _item };
+        }
+
+        public NavigationItemBuilder Action(string actionName) {
+            return Action(actionName, null, new RouteValueDictionary());
+        }
+
+        public NavigationItemBuilder Action(string actionName, string controllerName) {
+            return Action(actionName, controllerName, new RouteValueDictionary());
+        }
+
+        public NavigationItemBuilder Action(string actionName, string controllerName, object values) {
+            return Action(actionName, controllerName, new RouteValueDictionary(values));
+        }
+
+        public NavigationItemBuilder Action(string actionName, string controllerName, RouteValueDictionary values) {            
+            _item.RouteValues = new RouteValueDictionary(values);
+            if (!string.IsNullOrEmpty(actionName))
+                _item.RouteValues["action"] = actionName;
+            if (!string.IsNullOrEmpty(controllerName))
+                _item.RouteValues["controller"] = controllerName;
+            return this;
+        }
+    }
+}

src/Orchard/UI/Navigation/NavigationManager.cs

@@ -45,7 +45,7 @@ namespace Orchard.UI.Navigation {
             var joined = new MenuItem {
                 Text = items.First().Text,
                 RouteValues = items.First().RouteValues,
-                Contained = Merge(items.Select(x => x.Contained)).ToArray(),
+                Items = Merge(items.Select(x => x.Items)).ToArray(),
                 Position = SelectBestPositionValue(items.Select(x => x.Position))
             };
             return joined;