Move module loading to its own class

lib/compliance_engine/data.rb

@@ -13,6 +13,11 @@
 require 'compliance_engine/controls'
 require 'compliance_engine/profiles'
 
+require 'compliance_engine/data_loader'
+require 'compliance_engine/data_loader/json'
+require 'compliance_engine/data_loader/yaml'
+require 'compliance_engine/module_loader'
+
 require 'deep_merge'
 require 'json'
 
@@ -123,53 +128,35 @@ def open_environment(*paths)
   # @return [NilClass]
   def open(*paths, fileclass: File, dirclass: Dir)
     modules = {}
+
     paths.each do |path|
       if path.is_a?(ComplianceEngine::DataLoader)
         update(path, key: path.key, fileclass: fileclass)
         next
       end
 
-      if fileclass.directory?(path)
-        # Read the Puppet module's metadata.json
-        metadata_json = File.join(path.to_s, 'metadata.json')
-        if fileclass.exist?(metadata_json)
-          begin
-            metadata = JSON.parse(fileclass.read(metadata_json))
-            modules[metadata['name']] = metadata['version']
-          rescue => e
-            warn "Could not parse #{path}/metadata.json: #{e.message}"
-          end
-        end
-        # In this directory, we want to look for all yaml and json files
-        # under SIMP/compliance_profiles and simp/compliance_profiles.
-        globs = ['SIMP/compliance_profiles', 'simp/compliance_profiles']
-                .select { |dir| fileclass.directory?("#{path}/#{dir}") }
-                .map { |dir|
-          ['yaml', 'json'].map { |type| "#{path}/#{dir}/**/*.#{type}" }
-        }.flatten
-        # debug "Globs: #{globs}"
-        # Using .each here to make mocking with rspec easier.
-        globs.each do |glob|
-          dirclass.glob(glob).each do |file|
-            key = if Object.const_defined?(:Zip) && file.is_a?(Zip::Entry)
-                    File.join(file.zipfile.to_s, '.', file.to_s)
-                  else
-                    file.to_s
-                  end
-            update(file.to_s, key: key, fileclass: fileclass)
-          end
-        end
-      elsif fileclass.file?(path)
+      if fileclass.file?(path)
         key = if Object.const_defined?(:Zip) && path.is_a?(Zip::Entry)
                 File.join(path.zipfile.to_s, '.', path.to_s)
               else
                 path.to_s
               end
         update(path, key: key, fileclass: fileclass)
-      else
-        raise ComplianceEngine::Error, "Could not find path '#{path}'"
+        next
+      end
+
+      if fileclass.directory?(path)
+        loader = ComplianceEngine::ModuleLoader.new(path, fileclass: fileclass, dirclass: dirclass)
+        modules[loader.name] = loader.version unless loader.name.nil?
+        loader.files.each do |file_loader|
+          update(file_loader)
+        end
+        next
       end
+
+      raise ComplianceEngine::Error, "Invalid path or object '#{path}'"
     end
+
     self.environment_data ||= {}
     self.environment_data = self.environment_data.merge(modules)
 
@@ -189,19 +176,17 @@ def update(
     key: filename.to_s,
     fileclass: File
   )
-    data[key] ||= {}
-
     if filename.is_a?(String)
+      data[key] ||= {}
+
       if data[key]&.key?(:loader) && data[key][:loader]
         data[key][:loader].refresh if data[key][:loader].respond_to?(:refresh)
         return
       end
 
       loader = if File.extname(filename) == '.json'
-                 require 'compliance_engine/data_loader/json'
                  ComplianceEngine::DataLoader::Json.new(filename, fileclass: fileclass, key: key)
                else
-                 require 'compliance_engine/data_loader/yaml'
                  ComplianceEngine::DataLoader::Yaml.new(filename, fileclass: fileclass, key: key)
                end
 
@@ -212,6 +197,8 @@ def update(
         content: loader.data,
       }
     else
+      data[filename.key] ||= {}
+
       # Assume filename is a loader object
       unless data[filename.key]&.key?(:loader)
         data[filename.key][:loader] = filename

lib/compliance_engine/module_loader.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'compliance_engine'
+require 'compliance_engine/data_loader/json'
+require 'compliance_engine/data_loader/yaml'
+
+# Load compliance engine data from a Puppet module
+class ComplianceEngine::ModuleLoader
+  def initialize(path, fileclass: File, dirclass: Dir)
+    raise ComplianceEngine::Error, "#{path} is not a directory" unless fileclass.directory?(path)
+
+    @name = nil
+    @version = nil
+    @files = []
+
+    # Read the Puppet module's metadata.json
+    metadata_json = File.join(path.to_s, 'metadata.json')
+    if fileclass.exist?(metadata_json)
+      begin
+        metadata = ComplianceEngine::DataLoader::Json.new(metadata_json, fileclass: fileclass)
+        @name = metadata.data['name']
+        @version = metadata.data['version']
+      rescue => e
+        warn "Could not parse #{metadata_json}: #{e.message}"
+      end
+    end
+
+    # In this directory, we want to look for all yaml and json files
+    # under SIMP/compliance_profiles and simp/compliance_profiles.
+    globs = ['SIMP/compliance_profiles', 'simp/compliance_profiles']
+            .select { |dir| fileclass.directory?("#{path}/#{dir}") }
+            .map { |dir|
+      ['yaml', 'json'].map { |type| "#{path}/#{dir}/**/*.#{type}" }
+    }.flatten
+    # debug "Globs: #{globs}"
+    # Using .each here to make mocking with rspec easier.
+    globs.each do |glob|
+      dirclass.glob(glob).each do |file|
+        key = if Object.const_defined?(:Zip) && file.is_a?(Zip::Entry)
+                File.join(file.zipfile.to_s, '.', file.to_s)
+              else
+                file.to_s
+              end
+        loader = if File.extname(file.to_s) == '.json'
+                   ComplianceEngine::DataLoader::Json.new(file.to_s, fileclass: fileclass, key: key)
+                 else
+                   ComplianceEngine::DataLoader::Yaml.new(file.to_s, fileclass: fileclass, key: key)
+                 end
+        @files << loader
+      rescue => e
+        warn "Could not load #{file}: #{e.message}"
+      end
+    end
+  end
+
+  attr_reader :name, :version, :files
+end

spec/classes/compliance_engine/data_spec.rb

@@ -58,7 +58,7 @@
     end
 
     it 'fails to initialize' do
-      expect { described_class.new('non_existant') }.to raise_error(ComplianceEngine::Error, %r{Could not find path})
+      expect { described_class.new('non_existant') }.to raise_error(ComplianceEngine::Error, %r{Invalid path or object})
     end
   end
 

spec/classes/compliance_engine/module_loader_spec.rb

@@ -0,0 +1,150 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'compliance_engine'
+require 'compliance_engine/module_loader'
+
+RSpec.describe ComplianceEngine::ModuleLoader do
+  before(:each) do
+    allow(File).to receive(:directory?).and_call_original
+    allow(File).to receive(:exist?).and_call_original
+    allow(File).to receive(:file?).and_call_original
+    allow(File).to receive(:read).and_call_original
+    allow(File).to receive(:mtime).and_call_original
+    allow(File).to receive(:size).and_call_original
+
+    allow(Dir).to receive(:glob).and_call_original
+  end
+
+  context 'with no path' do
+    it 'does not initialize' do
+      expect { described_class.new }.to raise_error(ArgumentError)
+    end
+  end
+
+  context 'with an invalid path' do
+    let(:path) { '/path/to/module' }
+
+    before(:each) do
+      allow(File).to receive(:directory?).with(path).and_return(false)
+    end
+
+    it 'does not initialize' do
+      expect { described_class.new(path) }.to raise_error(ComplianceEngine::Error, "#{path} is not a directory")
+    end
+  end
+
+  context 'with module data' do
+    subject(:module_loader) { described_class.new(test_data.keys.first) }
+
+    let(:test_data) do
+      {
+        'test_module_00' => {
+          'a/file.yaml' => <<~A_YAML,
+            ---
+            version: '2.0.0'
+            profiles:
+              test_profile_00:
+                ces:
+                  ce_00: true
+              test_profile_01:
+                ces:
+                  ce_01: true
+            ce:
+              ce_00: {}
+              ce_01: {}
+            A_YAML
+          'b/file.yaml' => <<~B_YAML,
+            ---
+            version: '2.0.0'
+            profiles:
+              test_profile_01:
+                ces:
+                  ce_02: true
+            ce:
+              ce_02: {}
+            B_YAML
+          'c/file.yaml' => <<~C_YAML,
+            ---
+            version: '2.0.0'
+            profiles:
+              test_profile_02:
+                ces:
+                  ce_03: true
+            ce:
+              ce_03: {}
+            C_YAML
+        },
+      }
+    end
+
+    before(:each) do
+      test_data.each do |module_path, file_data|
+        allow(File).to receive(:directory?).with(module_path).and_return(true)
+        allow(File).to receive(:directory?).with("#{module_path}/SIMP/compliance_profiles").and_return(true)
+        allow(File).to receive(:directory?).with("#{module_path}/simp/compliance_profiles").and_return(false)
+        allow(Dir).to receive(:glob)
+          .with("#{module_path}/SIMP/compliance_profiles/**/*.yaml")
+          .and_return(
+            file_data.map { |name, _contents| "#{module_path}/SIMP/compliance_profiles/#{name}" },
+          )
+        allow(Dir).to receive(:glob)
+          .with("#{module_path}/SIMP/compliance_profiles/**/*.json")
+          .and_return([])
+
+        file_data.each do |name, contents|
+          filename = "#{module_path}/SIMP/compliance_profiles/#{name}"
+          allow(File).to receive(:size).with(filename).and_return(contents.length)
+          allow(File).to receive(:mtime).with(filename).and_return(Time.now)
+          allow(File).to receive(:read).with(filename).and_return(contents)
+        end
+      end
+    end
+
+    context 'with no metadata.json' do
+      before(:each) do
+        allow(File).to receive(:exist?).with("#{test_data.keys.first}/metadata.json").and_return(false)
+      end
+
+      it 'initializes' do
+        expect(module_loader).not_to be_nil
+        expect(module_loader).to be_instance_of(described_class)
+      end
+
+      it 'has no name or version' do
+        expect(module_loader.name).to be_nil
+        expect(module_loader.version).to be_nil
+      end
+
+      it 'returns a list of file loader objects' do
+        expect(module_loader.files.map { |loader| loader.key }).to eq(test_data.map { |module_path, files| files.map { |name, _| "#{module_path}/SIMP/compliance_profiles/#{name}" } }.flatten)
+      end
+    end
+
+    context 'with a metadata.json' do
+      before(:each) do
+        allow(File).to receive(:exist?).with("#{test_data.keys.first}/metadata.json").and_return(true)
+        allow(File).to receive(:read).with("#{test_data.keys.first}/metadata.json").and_return('{"name": "author-test_module_00", "version": "2.0.0"}')
+        allow(File).to receive(:size).with("#{test_data.keys.first}/metadata.json").and_return(53)
+        allow(File).to receive(:mtime).with("#{test_data.keys.first}/metadata.json").and_return(Time.now)
+      end
+
+      it 'initializes' do
+        expect(module_loader).not_to be_nil
+        expect(module_loader).to be_instance_of(described_class)
+      end
+
+      it 'has a name' do
+        expect(module_loader.name).to eq('author-test_module_00')
+      end
+
+      it 'has a version' do
+        expect(module_loader.version).to eq('2.0.0')
+      end
+
+      it 'returns a list of file loader objects' do
+        expect(module_loader.files.map { |loader| loader.key }).to eq(test_data.map { |module_path, files| files.map { |name, _| "#{module_path}/SIMP/compliance_profiles/#{name}" } }.flatten)
+      end
+    end
+  end
+end