Refactor "children" to use make_child_entry everywhere, simplifyjk/fs_children

17 files changed, 60 insertions, 87 deletions

diff --git a/lib/chef/chef_fs/file_system/acl_dir.rb b/lib/chef/chef_fs/file_system/acl_dir.rb
index a05f3ca534..9f68d7cda7 100644
--- a/lib/chef/chef_fs/file_system/acl_dir.rb
+++ b/lib/chef/chef_fs/file_system/acl_dir.rb
@@ -28,9 +28,9 @@ class Chef
           parent.parent.child(name).api_path
         end
 
-        def child(name)
+        def make_child_entry(name, exists = nil)
           result = @children.select { |child| child.name == name }.first if @children
-          result || super
+          result || AclEntry.new(name, self, exists)
         end
 
         def can_have_child?(name, is_dir)
@@ -41,7 +41,7 @@ class Chef
           if @children.nil?
             # Grab the ACTUAL children (/nodes, /containers, etc.) and get their names
             names = parent.parent.child(name).children.map { |child| child.dir? ? "#{child.name}.json" : child.name }
-            @children = names.map { |name| AclEntry.new(name, self, true) }
+            @children = names.map { |name| make_child_entry(name, true) }
           end
           @children
         end
diff --git a/lib/chef/chef_fs/file_system/acls_dir.rb b/lib/chef/chef_fs/file_system/acls_dir.rb
index 938bf73fb2..a8c63726b7 100644
--- a/lib/chef/chef_fs/file_system/acls_dir.rb
+++ b/lib/chef/chef_fs/file_system/acls_dir.rb
@@ -40,8 +40,12 @@ class Chef
           parent.api_path
         end
 
+        def make_child_entry(name)
+          children.select { |child| child.name == name }.first
+        end
+
         def can_have_child?(name, is_dir)
-          is_dir ? ENTITY_TYPES.include(name) : name == 'organization.json'
+          is_dir ? ENTITY_TYPES.include?(name) : name == 'organization.json'
         end
 
         def children
diff --git a/lib/chef/chef_fs/file_system/base_fs_object.rb b/lib/chef/chef_fs/file_system/base_fs_object.rb
index 6f05a0406e..916ab8297d 100644
--- a/lib/chef/chef_fs/file_system/base_fs_object.rb
+++ b/lib/chef/chef_fs/file_system/base_fs_object.rb
@@ -95,9 +95,10 @@ class Chef
         # directly perform a network request to retrieve the y.json data bag.  No
         # network request was necessary to retrieve
         def child(name)
-          (can_have_child?(name, true) || can_have_child?(name, false)) ?
-            make_child_entry(name) :
-            NonexistentFSObject.new(name, self)
+          if can_have_child?(name, true) || can_have_child?(name, false)
+            result = make_child_entry(name)
+          end
+          result || NonexistentFSObject.new(name, self)
         end
 
         # Override children to report your *actual* list of children as an array.
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_dir.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_dir.rb
index 4d1af9ad95..4391bdbfcd 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_dir.rb
@@ -58,14 +58,7 @@ class Chef
         end
 
         def children
-          begin
-            Dir.entries(file_path).sort.
-                select { |child_name| can_have_child?(child_name, File.directory?(File.join(file_path, child_name))) }.
-                map { |child_name| make_child_entry(child_name) }.
-                select { |entry| !(entry.dir? && entry.children.size == 0) }
-          rescue Errno::ENOENT
-            raise Chef::ChefFS::FileSystem::NotFoundError.new(self, $!)
-          end
+          super.select { |entry| !(entry.dir? && entry.children.size == 0 ) }
         end
 
         def can_have_child?(name, is_dir)
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_entry.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_entry.rb
index dd7f67492c..914412f839 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_entry.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbook_entry.rb
@@ -34,14 +34,7 @@ class Chef
         attr_reader :recursive
 
         def children
-          begin
-            Dir.entries(file_path).sort.
-                select { |child_name| can_have_child?(child_name, File.directory?(File.join(file_path, child_name))) }.
-                map { |child_name| make_child_entry(child_name) }.
-                select { |entry| !(entry.dir? && entry.children.size == 0) }
-          rescue Errno::ENOENT
-            raise Chef::ChefFS::FileSystem::NotFoundError.new(self, $!)
-          end
+          super.select { |entry| !(entry.dir? && entry.children.size == 0 ) }
         end
 
         def can_have_child?(name, is_dir)
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbooks_dir.rb
index 8a268ff755..5b495666c3 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_cookbooks_dir.rb
@@ -37,21 +37,14 @@ class Chef
         attr_reader :chefignore
 
         def children
-          begin
-            Dir.entries(file_path).sort.
-                select { |child_name| can_have_child?(child_name, File.directory?(File.join(file_path, child_name))) }.
-                map { |child_name| make_child_entry(child_name) }.
-                select do |entry|
-                  # empty cookbooks and cookbook directories are ignored
-                  if !entry.can_upload?
-                    Chef::Log.warn("Cookbook '#{entry.name}' is empty or entirely chefignored at #{entry.path_for_printing}")
-                    false
-                  else
-                    true
-                  end
-                end
-          rescue Errno::ENOENT
-            raise Chef::ChefFS::FileSystem::NotFoundError.new(self, $!)
+          super.select do |entry|
+            # empty cookbooks and cookbook directories are ignored
+            if !entry.can_upload?
+              Chef::Log.warn("Cookbook '#{entry.name}' is empty or entirely chefignored at #{entry.path_for_printing}")
+              false
+            else
+              true
+            end
           end
         end
 
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
index 2e6a638585..39172e7ab9 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_entry.rb
@@ -70,17 +70,6 @@ class Chef
           Chef::JSONCompat.to_json_pretty(object)
         end
 
-        def children
-          # Except cookbooks and data bag dirs, all things must be json files
-          begin
-            Dir.entries(file_path).sort.
-                select { |child_name| can_have_child?(child_name, File.directory?(File.join(file_path, child_name))) }.
-                map { |child_name| make_child_entry(child_name) }
-          rescue Errno::ENOENT
-            raise Chef::ChefFS::FileSystem::NotFoundError.new(self, $!)
-          end
-        end
-
         protected
 
         def make_child_entry(child_name)
diff --git a/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb b/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
index 48b1d200db..267fe30456 100644
--- a/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_repository_file_system_root_dir.rb
@@ -68,13 +68,13 @@ class Chef
         attr_reader :child_paths
         attr_reader :versioned_cookbooks
 
-        CHILDREN = %w(invitations.json members.json org.json)
+        CHILDREN = %w(org.json invitations.json members.json)
 
         def children
           @children ||= begin
-            result = child_paths.keys.sort.map { |name| make_child_entry(name) }.select { |child| child.exists? }
-            result += root_dir.children.select { |c| CHILDREN.include?(c.name) } if root_dir
-            result.sort_by { |c| c.name }
+            result = child_paths.keys.sort.map { |name| make_child_entry(name) }
+            result += CHILDREN.map { |name| make_child_entry(name) }
+            result.select { |c| c && c.exists? }.sort_by { |c| c.name }
           end
         end
 
@@ -149,19 +149,23 @@ class Chef
         # cookbooks from all of them when you list or grab them).
         #
         def make_child_entry(name)
-          paths = child_paths[name].select do |path|
-            File.exists?(path)
+          if CHILDREN.include?(name)
+            return nil if !root_dir
+            return root_dir.child(name)
           end
+
+          paths = (child_paths[name] || []).select { |path| File.exists?(path) }
           if paths.size == 0
             return NonexistentFSObject.new(name, self)
           end
-          if name == 'cookbooks'
+          case name
+          when 'cookbooks'
             dirs = paths.map { |path| ChefRepositoryFileSystemCookbooksDir.new(name, self, path) }
-          elsif name == 'data_bags'
+          when 'data_bags'
             dirs = paths.map { |path| ChefRepositoryFileSystemDataBagsDir.new(name, self, path) }
-          elsif name == 'policies'
+          when 'policies'
             dirs = paths.map { |path| ChefRepositoryFileSystemPoliciesDir.new(name, self, path) }
-          elsif name == 'acls'
+          when 'acls'
             dirs = paths.map { |path| ChefRepositoryFileSystemAclsDir.new(name, self, path) }
           else
             data_handler = case name
diff --git a/lib/chef/chef_fs/file_system/chef_server_root_dir.rb b/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
index 824325f31b..e3ffd644ad 100644
--- a/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
@@ -110,7 +110,8 @@ class Chef
         end
 
         def can_have_child?(name, is_dir)
-          is_dir && children.any? { |child| child.name == name }
+          result = children.select { |child| child.name == name }.first
+          result && !!result.dir? == !!is_dir
         end
 
         def org
@@ -124,6 +125,10 @@ class Chef
           end
         end
 
+        def make_child_entry(name)
+          children.select { |child| child.name == name }.first
+        end
+
         def children
           @children ||= begin
             result = [
diff --git a/lib/chef/chef_fs/file_system/cookbook_dir.rb b/lib/chef/chef_fs/file_system/cookbook_dir.rb
index 555f9aef0a..c0f0390e98 100644
--- a/lib/chef/chef_fs/file_system/cookbook_dir.rb
+++ b/lib/chef/chef_fs/file_system/cookbook_dir.rb
@@ -72,16 +72,15 @@ class Chef
           "#{parent.api_path}/#{cookbook_name}/#{version || "_latest"}"
         end
 
-        def child(name)
+        def make_child_entry(name)
           # Since we're ignoring the rules and doing a network request here,
           # we need to make sure we don't rethrow the exception.  (child(name)
           # is not supposed to fail.)
           begin
-            result = children.select { |child| child.name == name }.first
-            return result if result
+            children.select { |child| child.name == name }.first
           rescue Chef::ChefFS::FileSystem::NotFoundError
+            nil
           end
-          return NonexistentFSObject.new(name, self)
         end
 
         def can_have_child?(name, is_dir)
diff --git a/lib/chef/chef_fs/file_system/cookbooks_acl_dir.rb b/lib/chef/chef_fs/file_system/cookbooks_acl_dir.rb
index d6246f1e60..560ceb4886 100644
--- a/lib/chef/chef_fs/file_system/cookbooks_acl_dir.rb
+++ b/lib/chef/chef_fs/file_system/cookbooks_acl_dir.rb
@@ -31,7 +31,7 @@ class Chef
         def children
           if @children.nil?
             names = parent.parent.child(name).children.map { |child| "#{child.cookbook_name}.json" }
-            @children = names.uniq.map { |name| AclEntry.new(name, self, true) }
+            @children = names.uniq.map { |name| make_child_entry(name, true) }
           end
           @children
         end
diff --git a/lib/chef/chef_fs/file_system/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/cookbooks_dir.rb
index cac625ccf6..6f49c28996 100644
--- a/lib/chef/chef_fs/file_system/cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/cookbooks_dir.rb
@@ -36,13 +36,9 @@ class Chef
           super("cookbooks", parent)
         end
 
-        def child(name)
-          result = @children.select { |child| child.name == name }.first if @children
-          result || super
-        end
-
         def make_child_entry(name)
-          CookbookDir.new(name, self)
+          result = @children.select { |child| child.name == name }.first if @children
+          result || CookbookDir.new(name, self)
         end
 
         def children
diff --git a/lib/chef/chef_fs/file_system/data_bags_dir.rb b/lib/chef/chef_fs/file_system/data_bags_dir.rb
index 6d9794c726..1cb61bbd1a 100644
--- a/lib/chef/chef_fs/file_system/data_bags_dir.rb
+++ b/lib/chef/chef_fs/file_system/data_bags_dir.rb
@@ -27,16 +27,14 @@ class Chef
           super("data_bags", parent, "data")
         end
 
-        def child(name)
+        def make_child_entry(name, exists = false)
           result = @children.select { |child| child.name == name }.first if @children
-          result || super
+          result || DataBagDir.new(name, self, exists)
         end
 
         def children
           begin
-            @children ||= root.get_json(api_path).keys.sort.map do |entry|
-              DataBagDir.new(entry, self, true)
-            end
+            @children ||= root.get_json(api_path).keys.sort.map { |entry| make_child_entry(entry, true) }
           rescue Timeout::Error => e
             raise Chef::ChefFS::FileSystem::OperationFailedError.new(:children, self, e), "Timeout getting children: #{e}"
           rescue Net::HTTPServerException => e
diff --git a/lib/chef/chef_fs/file_system/file_system_entry.rb b/lib/chef/chef_fs/file_system/file_system_entry.rb
index 46366a16c6..8611aa2e0f 100644
--- a/lib/chef/chef_fs/file_system/file_system_entry.rb
+++ b/lib/chef/chef_fs/file_system/file_system_entry.rb
@@ -40,8 +40,11 @@ class Chef
         end
 
         def children
+          # Except cookbooks and data bag dirs, all things must be json files
           begin
-            Dir.entries(file_path).sort.select { |entry| entry != '.' && entry != '..' }.map { |entry| make_child_entry(entry) }
+            Dir.entries(file_path).sort.
+                map { |child_name| make_child_entry(child_name) }.
+                select { |child| child && can_have_child?(child.name, child.dir?) }
           rescue Errno::ENOENT
             raise Chef::ChefFS::FileSystem::NotFoundError.new(self, $!)
           end
diff --git a/lib/chef/chef_fs/file_system/memory_dir.rb b/lib/chef/chef_fs/file_system/memory_dir.rb
index a7eda3c654..260a91693c 100644
--- a/lib/chef/chef_fs/file_system/memory_dir.rb
+++ b/lib/chef/chef_fs/file_system/memory_dir.rb
@@ -1,5 +1,4 @@
 require 'chef/chef_fs/file_system/base_fs_dir'
-require 'chef/chef_fs/file_system/nonexistent_fs_object'
 require 'chef/chef_fs/file_system/memory_file'
 
 class Chef
@@ -13,8 +12,8 @@ class Chef
 
         attr_reader :children
 
-        def child(name)
-          @children.select { |child| child.name == name }.first || Chef::ChefFS::FileSystem::NonexistentFSObject.new(name, self)
+        def make_child_entry(name)
+          @children.select { |child| child.name == name }.first
         end
 
         def add_child(child)
diff --git a/lib/chef/chef_fs/file_system/multiplexed_dir.rb b/lib/chef/chef_fs/file_system/multiplexed_dir.rb
index a79acb0022..70b827f85f 100644
--- a/lib/chef/chef_fs/file_system/multiplexed_dir.rb
+++ b/lib/chef/chef_fs/file_system/multiplexed_dir.rb
@@ -35,7 +35,7 @@ class Chef
           end
         end
 
-        def child(name)
+        def make_child_entry(name)
           result = nil
           multiplexed_dirs.each do |dir|
             child_entry = dir.child(name)
@@ -47,7 +47,7 @@ class Chef
               end
             end
           end
-          result || NonexistentFSObject.new(name, self)
+          result
         end
 
         def can_have_child?(name, is_dir)
diff --git a/lib/chef/chef_fs/file_system/rest_list_dir.rb b/lib/chef/chef_fs/file_system/rest_list_dir.rb
index 6ebfdb0ea8..0ac735a2c4 100644
--- a/lib/chef/chef_fs/file_system/rest_list_dir.rb
+++ b/lib/chef/chef_fs/file_system/rest_list_dir.rb
@@ -33,11 +33,6 @@ class Chef
         attr_reader :api_path
         attr_reader :data_handler
 
-        def child(name)
-          result = @children.select { |child| child.name == name }.first if @children
-          result || super
-        end
-
         def can_have_child?(name, is_dir)
           name =~ /\.json$/ && !is_dir
         end
@@ -106,6 +101,7 @@ class Chef
         end
 
         def make_child_entry(name, exists = nil)
+          @children.select { |child| child.name == name }.first if @children
           RestListEntry.new(name, self, exists)
         end
       end