fix code coverage

Author: krbhavith

app/services/bulk_zombie_url_uploader.rb

@@ -8,65 +8,85 @@ class Error < StandardError; end
   def initialize(filename, filepath)
     @file_name = filename
     @file_path = filepath
+    @results = nil
   end
 
   def upload
-    @results = BulkZombieUrls::Results.new(@file_name)
-    raise 'Results object not initialized' if @results.nil?
-
-    begin
-      upload_urls
-    rescue => e
-      error_message = 'Your document could not be processed. Please check the format and try again.'
-      Rails.logger.error "Problem processing bulk zombie URL document: #{error_message} | #{e.message}"
-    end
-    @results
+    initialize_results
+    process_upload
+  rescue StandardError => e
+    log_upload_error(e)
+  ensure
+    @results ||= BulkZombieUrls::Results.new(@file_name) 
   end
 
   private
 
-  def upload_urls
-    parse_csv.each do |row|
-      process_row(row)
-    end
+  def initialize_results
+    @results = BulkZombieUrls::Results.new(@file_name)
+    raise Error, 'Results object not initialized' unless @results
+  end
+
+  def process_upload
+    parse_csv.each { |row| process_row(row) }
   rescue CSV::MalformedCSVError => e
     handle_csv_error(e)
   end
 
   def parse_csv
-    CSV.parse(File.read(@file_path), headers: true)
+    csv = CSV.parse(File.read(@file_path), headers: true)
+    raise CSV::MalformedCSVError, "Missing required headers" unless %w[URL DOC_ID].all? { |col| csv.headers.include?(col) }
+    csv
+  rescue CSV::MalformedCSVError, ArgumentError => e
+    raise CSV::MalformedCSVError.new('CSV', "Malformed or invalid CSV: #{e.message}")
   end
 
   def process_row(row)
+    raise Error, 'Results object not initialized' unless @results
+
     url = row['URL']&.strip
     document_id = row['DOC_ID']&.strip
 
-    if document_id.blank?
-      @results.add_error('Document ID is missing', url || 'Unknown')
-      Rails.logger.error("Skipping row: #{row.inspect}. Document ID is mandatory.")
-      return
-    end
+    return log_missing_document_id(row, url) if document_id.blank?
 
-    process_url_with_rescue(url, document_id, row)
+    handle_url_processing(url, document_id, row)
   end
 
-  def process_url_with_rescue(url, document_id, row)
-    process_url(url, document_id)
-    @results.delete_ok
-    @results.increment_updated
+  def handle_url_processing(url, document_id, row)
+    process_url_with_rescue(url, document_id)
+    update_results
   rescue StandardError => e
     handle_processing_error(e, url, document_id, row)
   end
 
+  def update_results
+    @results.delete_ok
+    @results.increment_updated
+  end
+
+  def log_missing_document_id(row, url)
+    @results.add_error('Document ID is missing', url || 'Unknown')
+    Rails.logger.error("Skipping row: #{row.inspect}. Document ID is mandatory.")
+  end
+
   def handle_csv_error(error)
     @results.add_error('Invalid CSV format', 'Entire file')
-    Rails.logger.error "Error parsing CSV: #{error.message}"
+    Rails.logger.error("Error parsing CSV: #{error.message}")
+  end
+
+  def log_upload_error(error)
+    error_message = "Failed to process bulk zombie URL document (file: #{@file_name})."
+    Rails.logger.error("#{error_message} Error: #{error.message}\n#{error.backtrace.join("\n")}")
   end
 
   def handle_processing_error(error, url, document_id, row)
     key = url.presence || document_id
-    @results.add_error(error.message, key)
-    Rails.logger.error "Failure to process bulk upload zombie URL row: #{row.inspect}\n#{error.message}\n#{error.backtrace.join("\n")}"
+    @results&.add_error(error.message, key)
+    Rails.logger.error("Failure to process bulk upload zombie URL row: #{row.inspect}\n#{error.message}\n#{error.backtrace.join("\n")}")
+  end
+
+  def process_url_with_rescue(url, document_id)
+    process_url(url, document_id)
   end
 
   def process_url(url, document_id)
@@ -79,11 +99,7 @@ def process_url(url, document_id)
 
   def process_url_with_searchgov(url, document_id)
     searchgov_url = SearchgovUrl.find_by(url:)
-    if searchgov_url
-      searchgov_url.destroy
-    else
-      delete_document(document_id)
-    end
+    searchgov_url ? searchgov_url.destroy : delete_document(document_id)
   end
 
   def delete_document(document_id)

spec/fixtures/csv/bulk_zombie_urls.csv

@@ -0,0 +1,3 @@
+URL,DOC_ID
+https://open.defense.gov/1,abc124
+https://open.defense.gov2/,abc125

spec/services/bulk_zombie_url_uploader_spec.rb

@@ -2,99 +2,199 @@
 
 describe BulkZombieUrlUploader do
   let(:filename) { 'test_file.csv' }
-  let(:filepath) { '/path/to/test_file.csv' }
+  let(:filepath) { Rails.root.join('spec', 'fixtures', 'files', filename) }
   let(:uploader) { described_class.new(filename, filepath) }
-  let(:results) { instance_double(BulkZombieUrls::Results) }
-  let(:csv_content) do
-    <<~CSV
-      URL,DOC_ID
-      http://example.com,doc1
-      ,doc2
-      http://missingdoc.com,
-    CSV
-  end
+  let(:results_double) { instance_double('BulkZombieUrls::Results') }
 
   before do
-    allow(File).to receive(:read).with(filepath).and_return(csv_content)
-    allow(BulkZombieUrls::Results).to receive(:new).and_return(results)
-    allow(results).to receive(:add_error)
-    allow(results).to receive(:delete_ok)
-    allow(results).to receive(:increment_updated)
-    uploader.instance_variable_set(:@results, results) # Ensure `@results` is initialized
+    allow(BulkZombieUrls::Results).to receive(:new).and_return(results_double)
+    allow(results_double).to receive(:add_error)
+    allow(results_double).to receive(:delete_ok)
+    allow(results_double).to receive(:increment_updated)
   end
 
   describe '#initialize' do
     it 'assigns filename and filepath' do
       expect(uploader.instance_variable_get(:@file_name)).to eq(filename)
       expect(uploader.instance_variable_get(:@file_path)).to eq(filepath)
+      expect(uploader.results).to be_nil
     end
   end
 
-  describe '#upload_urls' do
-    context 'with valid CSV content' do
-      it 'processes each row in the CSV' do
-        allow(uploader).to receive(:process_row)
-        uploader.send(:upload_urls)
-        expect(uploader).to have_received(:process_row).exactly(3).times
+  describe '#upload' do
+    subject { uploader.upload }
+
+    before do
+      allow(uploader).to receive(:initialize_results).and_call_original
+      allow(uploader).to receive(:process_upload)
+      allow(uploader).to receive(:log_upload_error)
+    end
+
+    it 'initializes results correctly' do
+      expect { subject }.not_to raise_error
+    end
+
+    context 'when no error occurs' do
+      it 'initializes results correctly' do
+        expect { subject }.not_to raise_error
       end
     end
 
-    context 'with invalid CSV content' do
-      let(:csv_error) { CSV::MalformedCSVError.new('Invalid CSV format', 'Line causing error') }
+    context 'when an error occurs' do
+      let(:error) { StandardError.new('Test Error') }
 
       before do
-        allow(CSV).to receive(:parse).and_raise(csv_error)
-        allow(Rails.logger).to receive(:error)
+        allow(uploader).to receive(:process_upload).and_raise(error)
+      end
+
+      it 'logs the upload error' do
+        expect(uploader).to receive(:log_upload_error).with(error)
+        subject
       end
+    end
+  end
+
+  describe '#initialize_results' do
+    subject { uploader.send(:initialize_results) }
+
+    it 'initializes the @results object' do
+      expect { subject }.not_to raise_error
+      expect(uploader.instance_variable_get(:@results)).to eq(results_double)
+    end
+  end
+
+  describe '#process_upload' do
+    subject { uploader.send(:process_upload) }
+
+    let(:csv_content) { "URL,DOC_ID\nhttp://example.com,123\n" }
+    let(:parsed_csv) { CSV.parse(csv_content, headers: true) }
+
+    before do
+      allow(File).to receive(:read).with(filepath).and_return(csv_content)
+      allow(CSV).to receive(:parse).and_return(parsed_csv)
+      allow(uploader).to receive(:process_row)
+    end
+
+    it 'parses the CSV and processes each row' do
+      expect(CSV).to receive(:parse).and_return(parsed_csv)
+      expect(uploader).to receive(:process_row).with(parsed_csv.first)
+      subject
+    end
+  end
+
+  describe '#parse_csv' do
+    subject { uploader.send(:parse_csv) }
+
+    let(:csv_content) { "URL,DOC_ID\nhttp://example.com,123\n" }
+
+    before do
+      allow(File).to receive(:read).with(filepath).and_return(csv_content)
+    end
+
+    context 'with valid CSV headers' do
+      it 'returns parsed CSV' do
+        expect(subject).to be_a(CSV::Table)
+      end
+    end
 
-      it 'handles the CSV error and logs it' do
-        expect(results).to receive(:add_error).with('Invalid CSV format', 'Entire file')
-        uploader.send(:upload_urls)
-        expect(Rails.logger).to have_received(:error).with(/Error parsing CSV/)
+    context 'with missing headers' do
+      let(:csv_content) { "INVALID_HEADER\nhttp://example.com\n" }
+
+      it 'raises a CSV::MalformedCSVError' do
+        expect { subject }.to raise_error(CSV::MalformedCSVError)
       end
     end
   end
 
   describe '#process_row' do
-    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => 'doc1' } }
+    subject { uploader.send(:process_row, row) }
+
+    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => '123' } }
+
+    context 'when @results is not initialized' do
+      before do
+        uploader.instance_variable_set(:@results, nil)
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(BulkZombieUrlUploader::Error, 'Results object not initialized')
+      end
+    end
 
-    context 'when DOC_ID is blank' do
-      let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => nil } }
+    context 'when @results is initialized' do
+      before do
+        uploader.send(:initialize_results)
+      end
 
-      it 'adds an error and logs it' do
-        allow(Rails.logger).to receive(:error)
-        uploader.send(:process_row, row)
-        expect(results).to have_received(:add_error).with('Document ID is missing', 'http://example.com')
-        expect(Rails.logger).to have_received(:error).with(/Document ID is mandatory/)
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
       end
     end
   end
 
-  describe '#process_url_with_rescue' do
-    let(:row) { { 'URL' => 'http://example.com', 'DOC_ID' => 'doc1' } }
+  describe '#handle_url_processing' do
+    subject { uploader.send(:handle_url_processing, url, document_id, row) }
 
-    before do
-      allow(uploader).to receive(:process_url)
+    let(:url) { 'http://example.com' }
+    let(:document_id) { '123' }
+    let(:row) { { 'URL' => url, 'DOC_ID' => document_id } }
+
+    context 'when no error occurs' do
+      before do
+        allow(uploader).to receive(:process_url_with_rescue)
+        allow(uploader).to receive(:update_results)
+      end
+
+      it 'processes the URL and updates results' do
+        expect(uploader).to receive(:process_url_with_rescue).with(url, document_id)
+        expect(uploader).to receive(:update_results)
+        subject
+      end
     end
 
-    it 'processes the URL and updates results' do
-      uploader.send(:process_url_with_rescue, 'http://example.com', 'doc1', row)
-      expect(results).to have_received(:delete_ok)
-      expect(results).to have_received(:increment_updated)
+    context 'when an error occurs' do
+      let(:error) { StandardError.new('Test Error') }
+
+      before do
+        allow(uploader).to receive(:process_url_with_rescue).and_raise(error)
+        allow(uploader).to receive(:handle_processing_error)
+      end
+
+      it 'handles processing error' do
+        expect(uploader).to receive(:handle_processing_error).with(error, url, document_id, row)
+        subject
+      end
+    end
+  end
+
+  describe '#process_url' do
+    subject { uploader.send(:process_url, url, document_id) }
+
+    let(:document_id) { '123' }
+
+    context 'when URL is present' do
+      let(:url) { 'http://example.com' }
+
+      before do
+        allow(uploader).to receive(:process_url_with_searchgov)
+      end
+
+      it 'processes URL with Searchgov' do
+        expect(uploader).to receive(:process_url_with_searchgov).with(url, document_id)
+        subject
+      end
     end
 
-    context 'when an error occurs during processing' do
-      let(:error) { StandardError.new('Processing error') }
+    context 'when URL is blank' do
+      let(:url) { nil }
 
       before do
-        allow(uploader).to receive(:process_url).and_raise(error)
-        allow(Rails.logger).to receive(:error)
+        allow(uploader).to receive(:delete_document)
       end
 
-      it 'handles the error and logs it' do
-        uploader.send(:process_url_with_rescue, 'http://example.com', 'doc1', row)
-        expect(results).to have_received(:add_error).with('Processing error', 'http://example.com')
-        expect(Rails.logger).to have_received(:error).with(/Failure to process bulk upload zombie URL row/)
+      it 'deletes the document' do
+        expect(uploader).to receive(:delete_document).with(document_id)
+        subject
       end
     end
   end