From ccbc1a8f6ef46a91858046150ee42ea494a619e5 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Sun, 29 Oct 2023 08:42:40 -0700
Subject: [PATCH 1/9] Reduce SQL calls when incrementing run counters

SQL logs before changes (using `#increment!`):

```sql
Bulkrax::ImporterRun Load (8.9ms)  SELECT  "bulkrax_importer_runs".* FROM "bulkrax_importer_runs" WHERE "bulkrax_importer_runs"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
Bulkrax::ImporterRun Update All (2.5ms)  UPDATE "bulkrax_importer_runs" SET "processed_records" = COALESCE("processed_records", 0) + 1 WHERE "bulkrax_importer_runs"."id" = $1  [["id", 1]]
```

SQL logs after changes (using `#increment_counter`):

```sql
Bulkrax::ImporterRun Update All (6.5ms)  UPDATE "bulkrax_importer_runs" SET "processed_records" = COALESCE("processed_records", 0) + 1 WHERE "bulkrax_importer_runs"."id" = $1  [["id", 1]]
```

The `SELECT` statement serves no purpose since the `UPDATE` statement is
atomic
---
 app/jobs/bulkrax/create_relationships_job.rb | 16 ++++++++++------
 app/jobs/bulkrax/delete_job.rb               |  2 +-
 app/jobs/bulkrax/export_work_job.rb          |  6 +++---
 app/jobs/bulkrax/import_collection_job.rb    |  8 ++++----
 app/jobs/bulkrax/import_file_set_job.rb      | 10 ++++------
 app/jobs/bulkrax/import_work_job.rb          |  8 ++++----
 app/parsers/bulkrax/application_parser.rb    |  2 +-
 7 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb
index 6e9eb77b8..ebbffd900 100644
--- a/app/jobs/bulkrax/create_relationships_job.rb
+++ b/app/jobs/bulkrax/create_relationships_job.rb
@@ -97,9 +97,11 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
       end
 
       if errors.present?
-        # rubocop:disable Rails/SkipsModelValidations
-        importer_run.increment!(:failed_relationships, number_of_failures)
-        # rubocop:enable Rails/SkipsModelValidations
+        ImporterRun.connection.execute(<<-SQL)
+          UPDATE bulkrax_importer_runs
+          SET failed_relationships = COALESCE(failed_relationships, 0) + #{number_of_failures}
+          WHERE id = #{importer_run_id}
+        SQL
 
         parent_entry&.set_status_info(errors.last, importer_run)
 
@@ -107,9 +109,11 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
         reschedule({ parent_identifier: parent_identifier, importer_run_id: importer_run_id })
         return false # stop current job from continuing to run after rescheduling
       else
-        # rubocop:disable Rails/SkipsModelValidations
-        Bulkrax::ImporterRun.find(importer_run_id).increment!(:processed_relationships, number_of_successes)
-        # rubocop:enable Rails/SkipsModelValidations
+        ImporterRun.connection.execute(<<-SQL)
+          UPDATE bulkrax_importer_runs
+          SET processed_relationships = COALESCE(processed_relationships, 0) + #{number_of_successes}
+          WHERE id = #{importer_run_id}
+        SQL
       end
     end
     # rubocop:enable Metrics/MethodLength
diff --git a/app/jobs/bulkrax/delete_job.rb b/app/jobs/bulkrax/delete_job.rb
index 764cd8a72..b326d2565 100644
--- a/app/jobs/bulkrax/delete_job.rb
+++ b/app/jobs/bulkrax/delete_job.rb
@@ -8,7 +8,7 @@ class DeleteJob < ApplicationJob
     def perform(entry, importer_run)
       obj = entry.factory.find
       obj&.delete
-      ImporterRun.find(importer_run.id).increment!(:deleted_records)
+      ImporterRun.increment_counter(:deleted_records, importer_run.id)
       ImporterRun.find(importer_run.id).decrement!(:enqueued_records)
       entry.save!
       entry.importer.current_run = ImporterRun.find(importer_run.id)
diff --git a/app/jobs/bulkrax/export_work_job.rb b/app/jobs/bulkrax/export_work_job.rb
index bc307f3d7..329a9a85b 100644
--- a/app/jobs/bulkrax/export_work_job.rb
+++ b/app/jobs/bulkrax/export_work_job.rb
@@ -12,16 +12,16 @@ def perform(*args)
         entry.save
       rescue StandardError
         # rubocop:disable Rails/SkipsModelValidations
-        exporter_run.increment!(:failed_records)
+        ExporterRun.increment_counter(:failed_records, args[1])
         exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
         raise
       else
         if entry.failed?
-          exporter_run.increment!(:failed_records)
+          ExporterRun.increment_counter(:failed_records, args[1])
           exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
           raise entry.reload.current_status.error_class.constantize
         else
-          exporter_run.increment!(:processed_records)
+          ExporterRun.increment_counter(:processed_records, args[1])
           exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
         end
         # rubocop:enable Rails/SkipsModelValidations
diff --git a/app/jobs/bulkrax/import_collection_job.rb b/app/jobs/bulkrax/import_collection_job.rb
index 163ecd6f9..a2269d560 100644
--- a/app/jobs/bulkrax/import_collection_job.rb
+++ b/app/jobs/bulkrax/import_collection_job.rb
@@ -10,12 +10,12 @@ def perform(*args)
       begin
         entry.build
         entry.save!
-        ImporterRun.find(args[1]).increment!(:processed_records)
-        ImporterRun.find(args[1]).increment!(:processed_collections)
+        ImporterRun.increment_counter(:processed_records, args[1])
+        ImporterRun.increment_counter(:processed_collections, args[1])
         ImporterRun.find(args[1]).decrement!(:enqueued_records) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
       rescue => e
-        ImporterRun.find(args[1]).increment!(:failed_records)
-        ImporterRun.find(args[1]).increment!(:failed_collections)
+        ImporterRun.increment_counter(:failed_records, args[1])
+        ImporterRun.increment_counter(:failed_collections, args[1])
         ImporterRun.find(args[1]).decrement!(:enqueued_records) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
         raise e
       end
diff --git a/app/jobs/bulkrax/import_file_set_job.rb b/app/jobs/bulkrax/import_file_set_job.rb
index c74ab45c0..387abdc1d 100644
--- a/app/jobs/bulkrax/import_file_set_job.rb
+++ b/app/jobs/bulkrax/import_file_set_job.rb
@@ -20,13 +20,11 @@ def perform(entry_id, importer_run_id)
 
       entry.build
       if entry.succeeded?
-        # rubocop:disable Rails/SkipsModelValidations
-        ImporterRun.find(importer_run_id).increment!(:processed_records)
-        ImporterRun.find(importer_run_id).increment!(:processed_file_sets)
+        ImporterRun.increment_counter(:processed_records, importer_run_id)
+        ImporterRun.increment_counter(:processed_file_sets, importer_run_id)
       else
-        ImporterRun.find(importer_run_id).increment!(:failed_records)
-        ImporterRun.find(importer_run_id).increment!(:failed_file_sets)
-        # rubocop:enable Rails/SkipsModelValidations
+        ImporterRun.increment_counter(:failed_records, importer_run_id)
+        ImporterRun.increment_counter(:failed_file_sets, importer_run_id)
       end
       ImporterRun.find(importer_run_id).decrement!(:enqueued_records) unless ImporterRun.find(importer_run_id).enqueued_records <= 0 # rubocop:disable Rails/SkipsModelValidations
       entry.save!
diff --git a/app/jobs/bulkrax/import_work_job.rb b/app/jobs/bulkrax/import_work_job.rb
index 95258049f..da84e2a8a 100644
--- a/app/jobs/bulkrax/import_work_job.rb
+++ b/app/jobs/bulkrax/import_work_job.rb
@@ -23,13 +23,13 @@ def perform(entry_id, run_id, time_to_live = 3, *)
       entry = Entry.find(entry_id)
       entry.build
       if entry.status == "Complete"
-        ImporterRun.find(run_id).increment!(:processed_records)
-        ImporterRun.find(run_id).increment!(:processed_works)
+        ImporterRun.increment_counter(:processed_records, run_id)
+        ImporterRun.increment_counter(:processed_works, run_id)
       else
         # do not retry here because whatever parse error kept you from creating a work will likely
         # keep preventing you from doing so.
-        ImporterRun.find(run_id).increment!(:failed_records)
-        ImporterRun.find(run_id).increment!(:failed_works)
+        ImporterRun.increment_counter(:failed_records, run_id)
+        ImporterRun.increment_counter(:failed_works, run_id)
       end
       # Regardless of completion or not, we want to decrement the enqueued records.
       ImporterRun.find(run_id).decrement!(:enqueued_records) unless ImporterRun.find(run_id).enqueued_records <= 0
diff --git a/app/parsers/bulkrax/application_parser.rb b/app/parsers/bulkrax/application_parser.rb
index 97c5c1ed3..df7664c51 100644
--- a/app/parsers/bulkrax/application_parser.rb
+++ b/app/parsers/bulkrax/application_parser.rb
@@ -270,7 +270,7 @@ def invalid_record(message)
       current_run.invalid_records ||= ""
       current_run.invalid_records += message
       current_run.save
-      ImporterRun.find(current_run.id).increment!(:failed_records)
+      ImporterRun.increment_counter(:failed_records, current_run.id)
       ImporterRun.find(current_run.id).decrement!(:enqueued_records) unless ImporterRun.find(current_run.id).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
     end
     # rubocop:enable Rails/SkipsModelValidations

From e359ada62d39f73d76d0461388f9bd41a8c6c38d Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Mon, 30 Oct 2023 11:36:40 -0700
Subject: [PATCH 2/9] replace #decrement! with #decrement_counter

---
 app/jobs/bulkrax/delete_job.rb            | 2 +-
 app/jobs/bulkrax/export_work_job.rb       | 6 +++---
 app/jobs/bulkrax/import_collection_job.rb | 4 ++--
 app/jobs/bulkrax/import_file_set_job.rb   | 4 ++--
 app/jobs/bulkrax/import_work_job.rb       | 2 +-
 app/parsers/bulkrax/application_parser.rb | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/jobs/bulkrax/delete_job.rb b/app/jobs/bulkrax/delete_job.rb
index b326d2565..a34e73269 100644
--- a/app/jobs/bulkrax/delete_job.rb
+++ b/app/jobs/bulkrax/delete_job.rb
@@ -9,7 +9,7 @@ def perform(entry, importer_run)
       obj = entry.factory.find
       obj&.delete
       ImporterRun.increment_counter(:deleted_records, importer_run.id)
-      ImporterRun.find(importer_run.id).decrement!(:enqueued_records)
+      ImporterRun.decrement_counter(:enqueued_records, importer_run.id)
       entry.save!
       entry.importer.current_run = ImporterRun.find(importer_run.id)
       entry.importer.record_status
diff --git a/app/jobs/bulkrax/export_work_job.rb b/app/jobs/bulkrax/export_work_job.rb
index 329a9a85b..842a35c25 100644
--- a/app/jobs/bulkrax/export_work_job.rb
+++ b/app/jobs/bulkrax/export_work_job.rb
@@ -13,16 +13,16 @@ def perform(*args)
       rescue StandardError
         # rubocop:disable Rails/SkipsModelValidations
         ExporterRun.increment_counter(:failed_records, args[1])
-        exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
+        ExporterRun.decrement_counter(:enqueued_records, args[1]) unless exporter_run.reload.enqueued_records <= 0
         raise
       else
         if entry.failed?
           ExporterRun.increment_counter(:failed_records, args[1])
-          exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
+          ExporterRun.decrement_counter(:enqueued_records, args[1]) unless exporter_run.reload.enqueued_records <= 0
           raise entry.reload.current_status.error_class.constantize
         else
           ExporterRun.increment_counter(:processed_records, args[1])
-          exporter_run.decrement!(:enqueued_records) unless exporter_run.enqueued_records <= 0
+          ExporterRun.decrement_counter(:enqueued_records, args[1]) unless exporter_run.reload.enqueued_records <= 0
         end
         # rubocop:enable Rails/SkipsModelValidations
       end
diff --git a/app/jobs/bulkrax/import_collection_job.rb b/app/jobs/bulkrax/import_collection_job.rb
index a2269d560..03405180c 100644
--- a/app/jobs/bulkrax/import_collection_job.rb
+++ b/app/jobs/bulkrax/import_collection_job.rb
@@ -12,11 +12,11 @@ def perform(*args)
         entry.save!
         ImporterRun.increment_counter(:processed_records, args[1])
         ImporterRun.increment_counter(:processed_collections, args[1])
-        ImporterRun.find(args[1]).decrement!(:enqueued_records) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
+        ImporterRun.decrement_counter(:enqueued_records, args[1]) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
       rescue => e
         ImporterRun.increment_counter(:failed_records, args[1])
         ImporterRun.increment_counter(:failed_collections, args[1])
-        ImporterRun.find(args[1]).decrement!(:enqueued_records) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
+        ImporterRun.decrement_counter(:enqueued_records, args[1]) unless ImporterRun.find(args[1]).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
         raise e
       end
       entry.importer.current_run = ImporterRun.find(args[1])
diff --git a/app/jobs/bulkrax/import_file_set_job.rb b/app/jobs/bulkrax/import_file_set_job.rb
index 387abdc1d..fd84ad7c2 100644
--- a/app/jobs/bulkrax/import_file_set_job.rb
+++ b/app/jobs/bulkrax/import_file_set_job.rb
@@ -26,7 +26,7 @@ def perform(entry_id, importer_run_id)
         ImporterRun.increment_counter(:failed_records, importer_run_id)
         ImporterRun.increment_counter(:failed_file_sets, importer_run_id)
       end
-      ImporterRun.find(importer_run_id).decrement!(:enqueued_records) unless ImporterRun.find(importer_run_id).enqueued_records <= 0 # rubocop:disable Rails/SkipsModelValidations
+      ImporterRun.decrement_counter(:enqueued_records, importer_run_id) unless ImporterRun.find(importer_run_id).enqueued_records <= 0 # rubocop:disable Rails/SkipsModelValidations
       entry.save!
       entry.importer.current_run = ImporterRun.find(importer_run_id)
       entry.importer.record_status
@@ -38,7 +38,7 @@ def perform(entry_id, importer_run_id)
       if entry.import_attempts < 5
         ImportFileSetJob.set(wait: (entry.import_attempts + 1).minutes).perform_later(entry_id, importer_run_id)
       else
-        ImporterRun.find(importer_run_id).decrement!(:enqueued_records) # rubocop:disable Rails/SkipsModelValidations
+        ImporterRun.decrement_counter(:enqueued_records, importer_run_id) # rubocop:disable Rails/SkipsModelValidations
         entry.set_status_info(e)
       end
     end
diff --git a/app/jobs/bulkrax/import_work_job.rb b/app/jobs/bulkrax/import_work_job.rb
index da84e2a8a..a3a22ea86 100644
--- a/app/jobs/bulkrax/import_work_job.rb
+++ b/app/jobs/bulkrax/import_work_job.rb
@@ -32,7 +32,7 @@ def perform(entry_id, run_id, time_to_live = 3, *)
         ImporterRun.increment_counter(:failed_works, run_id)
       end
       # Regardless of completion or not, we want to decrement the enqueued records.
-      ImporterRun.find(run_id).decrement!(:enqueued_records) unless ImporterRun.find(run_id).enqueued_records <= 0
+      ImporterRun.decrement_counter(:enqueued_records, run_id) unless ImporterRun.find(run_id).enqueued_records <= 0
 
       entry.save!
       entry.importer.current_run = ImporterRun.find(run_id)
diff --git a/app/parsers/bulkrax/application_parser.rb b/app/parsers/bulkrax/application_parser.rb
index df7664c51..9c5a1280f 100644
--- a/app/parsers/bulkrax/application_parser.rb
+++ b/app/parsers/bulkrax/application_parser.rb
@@ -271,7 +271,7 @@ def invalid_record(message)
       current_run.invalid_records += message
       current_run.save
       ImporterRun.increment_counter(:failed_records, current_run.id)
-      ImporterRun.find(current_run.id).decrement!(:enqueued_records) unless ImporterRun.find(current_run.id).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
+      ImporterRun.decrement_counter(:enqueued_records, current_run.id) unless ImporterRun.find(current_run.id).enqueued_records <= 0 # rubocop:disable Style/IdenticalConditionalBranches
     end
     # rubocop:enable Rails/SkipsModelValidations
 

From ae65cb757d7fd4300946e847cf4786da76f861c8 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Mon, 30 Oct 2023 11:39:52 -0700
Subject: [PATCH 3/9] put rubocop disable comments back

---
 app/jobs/bulkrax/import_file_set_job.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/jobs/bulkrax/import_file_set_job.rb b/app/jobs/bulkrax/import_file_set_job.rb
index fd84ad7c2..07fc6a388 100644
--- a/app/jobs/bulkrax/import_file_set_job.rb
+++ b/app/jobs/bulkrax/import_file_set_job.rb
@@ -20,11 +20,13 @@ def perform(entry_id, importer_run_id)
 
       entry.build
       if entry.succeeded?
+        # rubocop:disable Rails/SkipsModelValidations
         ImporterRun.increment_counter(:processed_records, importer_run_id)
         ImporterRun.increment_counter(:processed_file_sets, importer_run_id)
       else
         ImporterRun.increment_counter(:failed_records, importer_run_id)
         ImporterRun.increment_counter(:failed_file_sets, importer_run_id)
+        # rubocop:enable Rails/SkipsModelValidations
       end
       ImporterRun.decrement_counter(:enqueued_records, importer_run_id) unless ImporterRun.find(importer_run_id).enqueued_records <= 0 # rubocop:disable Rails/SkipsModelValidations
       entry.save!

From 3edd18ce0af802b7247338fdee42453b2774a021 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Mon, 30 Oct 2023 17:09:27 -0700
Subject: [PATCH 4/9] WIP: update counter-related specs

---
 spec/factories/bulkrax_exporter_runs.rb       |  8 +--
 spec/factories/bulkrax_importer_runs.rb       |  6 +-
 spec/jobs/bulkrax/export_work_job_spec.rb     | 22 ++++---
 spec/jobs/bulkrax/import_file_set_job_spec.rb | 52 +++++++++++------
 spec/jobs/bulkrax/import_work_job_spec.rb     | 57 +++++++++++++++++--
 5 files changed, 108 insertions(+), 37 deletions(-)

diff --git a/spec/factories/bulkrax_exporter_runs.rb b/spec/factories/bulkrax_exporter_runs.rb
index 92a748c2d..e7088a4c4 100644
--- a/spec/factories/bulkrax_exporter_runs.rb
+++ b/spec/factories/bulkrax_exporter_runs.rb
@@ -2,11 +2,11 @@
 
 FactoryBot.define do
   factory :bulkrax_exporter_run, class: 'Bulkrax::ExporterRun' do
-    exporter { nil }
+    exporter { FactoryBot.build(:bulkrax_exporter) }
     total_work_entries { 1 }
     enqueued_records { 1 }
-    processed_records { 1 }
-    deleted_records { 1 }
-    failed_records { 1 }
+    processed_records { 0 }
+    deleted_records { 0 }
+    failed_records { 0 }
   end
 end
diff --git a/spec/factories/bulkrax_importer_runs.rb b/spec/factories/bulkrax_importer_runs.rb
index f51f2d93f..f3cf45140 100644
--- a/spec/factories/bulkrax_importer_runs.rb
+++ b/spec/factories/bulkrax_importer_runs.rb
@@ -5,8 +5,8 @@
     importer { FactoryBot.build(:bulkrax_importer) }
     total_work_entries { 1 }
     enqueued_records { 1 }
-    processed_records { 1 }
-    deleted_records { 1 }
-    failed_records { 1 }
+    processed_records { 0 }
+    deleted_records { 0 }
+    failed_records { 0 }
   end
 end
diff --git a/spec/jobs/bulkrax/export_work_job_spec.rb b/spec/jobs/bulkrax/export_work_job_spec.rb
index 183bda396..3883fbbd9 100644
--- a/spec/jobs/bulkrax/export_work_job_spec.rb
+++ b/spec/jobs/bulkrax/export_work_job_spec.rb
@@ -5,23 +5,27 @@
 module Bulkrax
   RSpec.describe ExportWorkJob, type: :job do
     subject(:export_work_job) { described_class.new }
-    let(:entry) { FactoryBot.build(:bulkrax_entry) }
-    let(:exporter_run) { FactoryBot.build(:bulkrax_exporter_run) }
+    let(:exporter) { create(:bulkrax_exporter, :all) }
+    let(:entry) { create(:bulkrax_entry, importerexporter: exporter) }
+    let(:exporter_run) { create(:bulkrax_exporter_run, exporter: exporter) }
 
     before do
       allow(Bulkrax::Entry).to receive(:find).with(1).and_return(entry)
-      allow(Bulkrax::ExporterRun).to receive(:find).with(1).and_return(exporter_run)
       allow(entry).to receive(:build)
     end
 
     describe 'successful job' do
-      before do
-        allow(entry).to receive(:save).and_return(true)
-      end
+      # TODO: split into specific changes
+      # TODO: cover all counters
       it 'increments :processed_records and decrements enqueued record' do
-        expect(exporter_run).to receive(:increment!).with(:processed_records)
-        expect(exporter_run).to receive(:decrement!).with(:enqueued_records)
-        export_work_job.perform(1, 1)
+        expect(exporter_run.processed_records).to eq(0)
+        expect(exporter_run.enqueued_records).to eq(1)
+
+        export_work_job.perform(entry.id, exporter_run.id)
+        exporter_run.reload
+
+        expect(exporter_run.processed_records).to eq(1)
+        expect(exporter_run.enqueued_records).to eq(0)
       end
     end
   end
diff --git a/spec/jobs/bulkrax/import_file_set_job_spec.rb b/spec/jobs/bulkrax/import_file_set_job_spec.rb
index 092baa81d..968e63aae 100644
--- a/spec/jobs/bulkrax/import_file_set_job_spec.rb
+++ b/spec/jobs/bulkrax/import_file_set_job_spec.rb
@@ -16,7 +16,6 @@ module Bulkrax
 
     before do
       allow(Entry).to receive(:find).with(entry.id).and_return(entry)
-      allow(ImporterRun).to receive(:find).with(importer_run.id).and_return(importer_run)
       allow(::Hyrax.config).to receive(:curation_concerns).and_return([Work])
       allow(::Work).to receive(:where).and_return([])
       allow(importer.parser).to receive(:path_to_files).with(filename: 'removed.png').and_return('spec/fixtures/removed.png')
@@ -51,15 +50,23 @@ module Bulkrax
             import_file_set_job.perform(entry.id, importer_run.id)
           end
 
+          # TODO: split into specific changes
+          # TODO: cover all counters
           it "updates the importer run's counters" do
-            expect(importer_run).to receive(:increment!).with(:processed_records).once
-            expect(importer_run).to receive(:increment!).with(:processed_file_sets).once
-
-            expect(importer_run).not_to receive(:decrement!).with(:enqueued_records)
-            expect(importer_run).not_to receive(:increment!).with(:failed_records)
-            expect(importer_run).not_to receive(:increment!).with(:failed_file_sets)
+            expect(importer_run.processed_records).to eq(0)
+            expect(importer_run.processed_file_sets).to eq(0)
+            expect(importer_run.enqueued_records).to eq(0)
+            expect(importer_run.failed_records).to eq(0)
+            expect(importer_run.failed_file_sets).to eq(0)
 
             import_file_set_job.perform(entry.id, importer_run.id)
+            importer_run.reload
+
+            expect(importer_run.processed_records).to eq(1)
+            expect(importer_run.processed_file_sets).to eq(1)
+            expect(importer_run.enqueued_records).to eq(0)
+            expect(importer_run.failed_records).to eq(0)
+            expect(importer_run.failed_file_sets).to eq(0)
           end
         end
 
@@ -79,8 +86,8 @@ module Bulkrax
             end
 
             it "does not update any of importer run's counters" do
-              expect(importer_run).not_to receive(:increment!)
-              expect(importer_run).not_to receive(:decrement!)
+              expect(ImporterRun).not_to receive(:increment_counter)
+              expect(ImporterRun).not_to receive(:decrement_counter)
             end
 
             it 'reschedules the job to try again after a couple minutes' do
@@ -111,8 +118,11 @@ module Bulkrax
             end
 
             it "only decrements the importer run's :enqueued_records counter" do
-              expect(importer_run).not_to receive(:increment!)
-              expect(importer_run).to receive(:decrement!).with(:enqueued_records).once
+              expect(ImporterRun).not_to receive(:increment_counter)
+              expect(ImporterRun)
+                .to receive(:decrement_counter)
+                .with(:enqueued_records, importer_run.id)
+                .once
 
               import_file_set_job.perform(entry.id, importer_run.id)
             end
@@ -165,15 +175,23 @@ module Bulkrax
           import_file_set_job.perform(entry.id, importer_run.id)
         end
 
+        # TODO: split into specific changes
+        # TODO: cover all counters
         it "updates the importer run's counters" do
-          expect(importer_run).to receive(:increment!).with(:failed_records).once
-          expect(importer_run).to receive(:increment!).with(:failed_file_sets).once
-
-          expect(importer_run).not_to receive(:decrement!).with(:enqueued_records)
-          expect(importer_run).not_to receive(:increment!).with(:processed_records)
-          expect(importer_run).not_to receive(:increment!).with(:processed_file_sets)
+          expect(importer_run.failed_records).to eq(0)
+          expect(importer_run.failed_file_sets).to eq(0)
+          expect(importer_run.enqueued_records).to eq(0)
+          expect(importer_run.processed_records).to eq(0)
+          expect(importer_run.processed_file_sets).to eq(0)
 
           import_file_set_job.perform(entry.id, importer_run.id)
+          importer_run.reload
+
+          expect(importer_run.failed_records).to eq(1)
+          expect(importer_run.failed_file_sets).to eq(1)
+          expect(importer_run.enqueued_records).to eq(0)
+          expect(importer_run.processed_records).to eq(0)
+          expect(importer_run.processed_file_sets).to eq(0)
         end
       end
     end
diff --git a/spec/jobs/bulkrax/import_work_job_spec.rb b/spec/jobs/bulkrax/import_work_job_spec.rb
index 14652c4c6..32b7fd93a 100644
--- a/spec/jobs/bulkrax/import_work_job_spec.rb
+++ b/spec/jobs/bulkrax/import_work_job_spec.rb
@@ -11,7 +11,6 @@ module Bulkrax
 
     before do
       allow(Bulkrax::Entry).to receive(:find).with(1).and_return(entry)
-      allow(Bulkrax::ImporterRun).to receive(:find).with(importer_run_id).and_return(importer_run)
     end
 
     describe 'successful job' do
@@ -20,11 +19,61 @@ module Bulkrax
         allow(entry).to receive(:build).and_return(instance_of(Work))
         allow(entry).to receive(:status).and_return('Complete')
       end
+
       it 'increments :processed_records' do
-        expect(importer_run).to receive(:increment!).with(:processed_records)
-        expect(importer_run).to receive(:increment!).with(:processed_works)
-        expect(importer_run).to receive(:decrement!).with(:enqueued_records)
+        expect(importer_run.processed_records).to eq(0)
+
+        import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.processed_records).to eq(1)
+      end
+
+      it 'increments :processed_works' do
+        expect(importer_run.processed_works).to eq(0)
+
         import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.processed_works).to eq(1)
+      end
+
+      it 'decrements :enqueued_records' do
+        expect(importer_run.enqueued_records).to eq(1)
+
+        import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.enqueued_records).to eq(0)
+      end
+
+      it "doesn't change unrelated counters" do
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.deleted_records).to eq(0)
+        expect(importer_run.processed_collections).to eq(0)
+        expect(importer_run.failed_collections).to eq(0)
+        expect(importer_run.processed_relationships).to eq(0)
+        expect(importer_run.failed_relationships).to eq(0)
+        expect(importer_run.processed_file_sets).to eq(0)
+        expect(importer_run.failed_file_sets).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+        expect(importer_run.processed_children).to eq(0)
+        expect(importer_run.failed_children).to eq(0)
+
+        import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.deleted_records).to eq(0)
+        expect(importer_run.processed_collections).to eq(0)
+        expect(importer_run.failed_collections).to eq(0)
+        expect(importer_run.processed_relationships).to eq(0)
+        expect(importer_run.failed_relationships).to eq(0)
+        expect(importer_run.processed_file_sets).to eq(0)
+        expect(importer_run.failed_file_sets).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+        expect(importer_run.processed_children).to eq(0)
+        expect(importer_run.failed_children).to eq(0)
       end
     end
 

From e99708ece877e7f0ed011a3bc01dc5324b55c318 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 31 Oct 2023 07:38:26 -0700
Subject: [PATCH 5/9] fix more specs

---
 app/jobs/bulkrax/delete_job.rb            |  4 +-
 spec/jobs/bulkrax/delete_work_job_spec.rb | 24 +++++----
 spec/jobs/bulkrax/import_work_job_spec.rb | 60 +++++++++++++++++++----
 3 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/app/jobs/bulkrax/delete_job.rb b/app/jobs/bulkrax/delete_job.rb
index a34e73269..1fcd04cca 100644
--- a/app/jobs/bulkrax/delete_job.rb
+++ b/app/jobs/bulkrax/delete_job.rb
@@ -4,17 +4,17 @@ module Bulkrax
   class DeleteJob < ApplicationJob
     queue_as :import
 
-    # rubocop:disable Rails/SkipsModelValidations
     def perform(entry, importer_run)
       obj = entry.factory.find
       obj&.delete
+      # rubocop:disable Rails/SkipsModelValidations
       ImporterRun.increment_counter(:deleted_records, importer_run.id)
       ImporterRun.decrement_counter(:enqueued_records, importer_run.id)
+      # rubocop:enable Rails/SkipsModelValidations
       entry.save!
       entry.importer.current_run = ImporterRun.find(importer_run.id)
       entry.importer.record_status
       entry.set_status_info("Deleted", ImporterRun.find(importer_run.id))
     end
-    # rubocop:enable Rails/SkipsModelValidations
   end
 end
diff --git a/spec/jobs/bulkrax/delete_work_job_spec.rb b/spec/jobs/bulkrax/delete_work_job_spec.rb
index 73599ae16..a40c0ba47 100644
--- a/spec/jobs/bulkrax/delete_work_job_spec.rb
+++ b/spec/jobs/bulkrax/delete_work_job_spec.rb
@@ -5,8 +5,8 @@
 module Bulkrax
   RSpec.describe DeleteWorkJob, type: :job do
     subject(:delete_work_job) { described_class.new }
-    let(:entry) { FactoryBot.build(:bulkrax_entry) }
-    let(:importer_run) { FactoryBot.build(:bulkrax_importer_run) }
+    let(:entry) { create(:bulkrax_entry) }
+    let(:importer_run) { create(:bulkrax_importer_run) }
 
     describe 'successful job object removed' do
       before do
@@ -18,12 +18,14 @@ module Bulkrax
       end
 
       it 'increments :deleted_records' do
-        entry.save
-        importer_run.save
+        expect(importer_run.enqueued_records).to eq(1)
+        expect(importer_run.deleted_records).to eq(0)
+
         delete_work_job.perform(entry, importer_run)
         importer_run.reload
-        expect(importer_run.enqueued_records).to equal(0)
-        expect(importer_run.deleted_records).to equal(2)
+
+        expect(importer_run.enqueued_records).to eq(0)
+        expect(importer_run.deleted_records).to eq(1)
       end
     end
 
@@ -35,12 +37,14 @@ module Bulkrax
       end
 
       it 'increments :deleted_records' do
-        entry.save
-        importer_run.save
+        expect(importer_run.enqueued_records).to eq(1)
+        expect(importer_run.deleted_records).to eq(0)
+
         delete_work_job.perform(entry, importer_run)
         importer_run.reload
-        expect(importer_run.enqueued_records).to equal(0)
-        expect(importer_run.deleted_records).to equal(2)
+
+        expect(importer_run.enqueued_records).to eq(0)
+        expect(importer_run.deleted_records).to eq(1)
       end
     end
   end
diff --git a/spec/jobs/bulkrax/import_work_job_spec.rb b/spec/jobs/bulkrax/import_work_job_spec.rb
index 32b7fd93a..377fe485a 100644
--- a/spec/jobs/bulkrax/import_work_job_spec.rb
+++ b/spec/jobs/bulkrax/import_work_job_spec.rb
@@ -81,11 +81,22 @@ module Bulkrax
       before do
         allow(entry).to receive(:build_for_importer).and_raise(CollectionsCreatedError)
       end
-      it 'does not call increment' do
-        expect(importer_run).not_to receive(:increment!)
-        expect(importer_run).not_to receive(:decrement!)
+
+      it 'does not change counters' do
+        expect(importer_run.processed_records).to eq(0)
+        expect(importer_run.processed_works).to eq(0)
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+
         import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.processed_records).to eq(0)
+        expect(importer_run.processed_works).to eq(0)
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
       end
+
       it 'reschedules the job' do
         expect(import_work_job).to receive(:reschedule) # rubocop:disable RSpec/SubjectStub
         import_work_job.perform(1, importer_run_id)
@@ -96,11 +107,25 @@ module Bulkrax
       before do
         allow(entry).to receive(:build).and_return(nil)
       end
-      it 'increments :failed_records' do
-        expect(importer_run).to receive(:increment!).with(:failed_records)
-        expect(importer_run).to receive(:increment!).with(:failed_works)
-        expect(importer_run).to receive(:decrement!).with(:enqueued_records)
+
+      it 'increments :failed_records and :failed_works' do
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+
+        import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.failed_records).to eq(1)
+        expect(importer_run.failed_works).to eq(1)
+      end
+
+      it 'decrements :enqueued_records' do
+        expect(importer_run.enqueued_records).to eq(1)
+
         import_work_job.perform(1, importer_run_id)
+        importer_run.reload
+
+        expect(importer_run.enqueued_records).to eq(0)
       end
     end
 
@@ -108,10 +133,25 @@ module Bulkrax
       before do
         allow(entry).to receive(:build).and_raise(OAIError)
       end
-      it 'increments :failed_records' do
+
+      it 'does not increment :failed_records or :failed_works' do
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+
+        expect { import_work_job.perform(1, importer_run_id) }.to raise_error(OAIError)
+        importer_run.reload
+
+        expect(importer_run.failed_records).to eq(0)
+        expect(importer_run.failed_works).to eq(0)
+      end
+
+      it 'does not decrement :enqueued_records' do
+        expect(importer_run.enqueued_records).to eq(1)
+
         expect { import_work_job.perform(1, importer_run_id) }.to raise_error(OAIError)
-        expect(importer_run).not_to receive(:increment!).with(:failed_records)
-        expect(importer_run).not_to receive(:decrement!).with(:enqueued_records)
+        importer_run.reload
+
+        expect(importer_run.enqueued_records).to eq(1)
       end
     end
   end

From 658ff3ceaf92ec09dc02eb52ffe9e7d028508875 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 31 Oct 2023 08:11:52 -0700
Subject: [PATCH 6/9] make some counter specs more specific

---
 spec/jobs/bulkrax/export_work_job_spec.rb     | 24 ++++-
 spec/jobs/bulkrax/import_file_set_job_spec.rb | 88 ++++++++++++++++---
 2 files changed, 94 insertions(+), 18 deletions(-)

diff --git a/spec/jobs/bulkrax/export_work_job_spec.rb b/spec/jobs/bulkrax/export_work_job_spec.rb
index 3883fbbd9..c4d3176b4 100644
--- a/spec/jobs/bulkrax/export_work_job_spec.rb
+++ b/spec/jobs/bulkrax/export_work_job_spec.rb
@@ -15,18 +15,34 @@ module Bulkrax
     end
 
     describe 'successful job' do
-      # TODO: split into specific changes
-      # TODO: cover all counters
-      it 'increments :processed_records and decrements enqueued record' do
+      it 'increments :processed_records' do
         expect(exporter_run.processed_records).to eq(0)
-        expect(exporter_run.enqueued_records).to eq(1)
 
         export_work_job.perform(entry.id, exporter_run.id)
         exporter_run.reload
 
         expect(exporter_run.processed_records).to eq(1)
+      end
+
+      it 'decrements :enqueued_records' do
+        expect(exporter_run.enqueued_records).to eq(1)
+
+        export_work_job.perform(entry.id, exporter_run.id)
+        exporter_run.reload
+
         expect(exporter_run.enqueued_records).to eq(0)
       end
+
+      it "doesn't change unrelated counters" do
+        expect(exporter_run.failed_records).to eq(0)
+        expect(exporter_run.deleted_records).to eq(0)
+
+        export_work_job.perform(1, exporter_run.id)
+        exporter_run.reload
+
+        expect(exporter_run.failed_records).to eq(0)
+        expect(exporter_run.deleted_records).to eq(0)
+      end
     end
   end
 end
diff --git a/spec/jobs/bulkrax/import_file_set_job_spec.rb b/spec/jobs/bulkrax/import_file_set_job_spec.rb
index 968e63aae..b5ce005ed 100644
--- a/spec/jobs/bulkrax/import_file_set_job_spec.rb
+++ b/spec/jobs/bulkrax/import_file_set_job_spec.rb
@@ -5,8 +5,8 @@
 module Bulkrax
   RSpec.describe ImportFileSetJob, type: :job do
     subject(:import_file_set_job) { described_class.new }
-    let(:importer) { FactoryBot.create(:bulkrax_importer_csv_complex) }
-    let(:importer_run) { importer.current_run }
+    let(:importer) { create(:bulkrax_importer_csv_complex) }
+    let(:importer_run) { create(:bulkrax_importer_run, importer: importer) }
     let(:entry) { create(:bulkrax_csv_entry_file_set, :with_file_set_metadata, importerexporter: importer) }
     let(:factory) { instance_double(ObjectFactory) }
 
@@ -50,23 +50,53 @@ module Bulkrax
             import_file_set_job.perform(entry.id, importer_run.id)
           end
 
-          # TODO: split into specific changes
-          # TODO: cover all counters
-          it "updates the importer run's counters" do
+          it 'increments :processed_records and :processed_file_sets' do
             expect(importer_run.processed_records).to eq(0)
             expect(importer_run.processed_file_sets).to eq(0)
-            expect(importer_run.enqueued_records).to eq(0)
-            expect(importer_run.failed_records).to eq(0)
-            expect(importer_run.failed_file_sets).to eq(0)
 
             import_file_set_job.perform(entry.id, importer_run.id)
             importer_run.reload
 
             expect(importer_run.processed_records).to eq(1)
             expect(importer_run.processed_file_sets).to eq(1)
+          end
+
+          it 'decrements :enqueued_records' do
+            expect(importer_run.enqueued_records).to eq(1)
+
+            import_file_set_job.perform(entry.id, importer_run.id)
+            importer_run.reload
+
             expect(importer_run.enqueued_records).to eq(0)
+          end
+
+          it "doesn't change unrelated counters" do
+            expect(importer_run.failed_records).to eq(0)
+            expect(importer_run.deleted_records).to eq(0)
+            expect(importer_run.processed_collections).to eq(0)
+            expect(importer_run.failed_collections).to eq(0)
+            expect(importer_run.processed_relationships).to eq(0)
+            expect(importer_run.failed_relationships).to eq(0)
+            expect(importer_run.failed_file_sets).to eq(0)
+            expect(importer_run.processed_works).to eq(0)
+            expect(importer_run.failed_works).to eq(0)
+            expect(importer_run.processed_children).to eq(0)
+            expect(importer_run.failed_children).to eq(0)
+
+            import_file_set_job.perform(entry.id, importer_run.id)
+            importer_run.reload
+
             expect(importer_run.failed_records).to eq(0)
+            expect(importer_run.deleted_records).to eq(0)
+            expect(importer_run.processed_collections).to eq(0)
+            expect(importer_run.failed_collections).to eq(0)
+            expect(importer_run.processed_relationships).to eq(0)
+            expect(importer_run.failed_relationships).to eq(0)
             expect(importer_run.failed_file_sets).to eq(0)
+            expect(importer_run.processed_works).to eq(0)
+            expect(importer_run.failed_works).to eq(0)
+            expect(importer_run.processed_children).to eq(0)
+            expect(importer_run.failed_children).to eq(0)
           end
         end
 
@@ -175,23 +205,53 @@ module Bulkrax
           import_file_set_job.perform(entry.id, importer_run.id)
         end
 
-        # TODO: split into specific changes
-        # TODO: cover all counters
-        it "updates the importer run's counters" do
+        it 'increments :failed_records and :failed_file_sets' do
           expect(importer_run.failed_records).to eq(0)
           expect(importer_run.failed_file_sets).to eq(0)
-          expect(importer_run.enqueued_records).to eq(0)
-          expect(importer_run.processed_records).to eq(0)
-          expect(importer_run.processed_file_sets).to eq(0)
 
           import_file_set_job.perform(entry.id, importer_run.id)
           importer_run.reload
 
           expect(importer_run.failed_records).to eq(1)
           expect(importer_run.failed_file_sets).to eq(1)
+        end
+
+        it 'decrements :enqueued_records' do
+          expect(importer_run.enqueued_records).to eq(1)
+
+          import_file_set_job.perform(entry.id, importer_run.id)
+          importer_run.reload
+
           expect(importer_run.enqueued_records).to eq(0)
+        end
+
+        it "doesn't change unrelated counters" do
+          expect(importer_run.processed_records).to eq(0)
+          expect(importer_run.deleted_records).to eq(0)
+          expect(importer_run.processed_collections).to eq(0)
+          expect(importer_run.failed_collections).to eq(0)
+          expect(importer_run.processed_relationships).to eq(0)
+          expect(importer_run.failed_relationships).to eq(0)
+          expect(importer_run.processed_file_sets).to eq(0)
+          expect(importer_run.processed_works).to eq(0)
+          expect(importer_run.failed_works).to eq(0)
+          expect(importer_run.processed_children).to eq(0)
+          expect(importer_run.failed_children).to eq(0)
+
+          import_file_set_job.perform(entry.id, importer_run.id)
+          importer_run.reload
+
           expect(importer_run.processed_records).to eq(0)
+          expect(importer_run.deleted_records).to eq(0)
+          expect(importer_run.processed_collections).to eq(0)
+          expect(importer_run.failed_collections).to eq(0)
+          expect(importer_run.processed_relationships).to eq(0)
+          expect(importer_run.failed_relationships).to eq(0)
           expect(importer_run.processed_file_sets).to eq(0)
+          expect(importer_run.processed_works).to eq(0)
+          expect(importer_run.failed_works).to eq(0)
+          expect(importer_run.processed_children).to eq(0)
+          expect(importer_run.failed_children).to eq(0)
         end
       end
     end

From d2d46d71071b6001f79af4b0ece2a86481c8161a Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 31 Oct 2023 08:23:19 -0700
Subject: [PATCH 7/9] remove references to "children" ImporterRun columns

These columns (`processed_children` and `failed_children`) were renamed
in the RenameChildrenCountersToRelationships migration; they no longer
exist
---
 spec/jobs/bulkrax/import_file_set_job_spec.rb | 8 --------
 spec/jobs/bulkrax/import_work_job_spec.rb     | 4 ----
 spec/test_app/db/schema.rb                    | 2 --
 3 files changed, 14 deletions(-)

diff --git a/spec/jobs/bulkrax/import_file_set_job_spec.rb b/spec/jobs/bulkrax/import_file_set_job_spec.rb
index b5ce005ed..962c7c0af 100644
--- a/spec/jobs/bulkrax/import_file_set_job_spec.rb
+++ b/spec/jobs/bulkrax/import_file_set_job_spec.rb
@@ -80,8 +80,6 @@ module Bulkrax
             expect(importer_run.failed_file_sets).to eq(0)
             expect(importer_run.processed_works).to eq(0)
             expect(importer_run.failed_works).to eq(0)
-            expect(importer_run.processed_children).to eq(0)
-            expect(importer_run.failed_children).to eq(0)
 
             import_file_set_job.perform(entry.id, importer_run.id)
             importer_run.reload
@@ -95,8 +93,6 @@ module Bulkrax
             expect(importer_run.failed_file_sets).to eq(0)
             expect(importer_run.processed_works).to eq(0)
             expect(importer_run.failed_works).to eq(0)
-            expect(importer_run.processed_children).to eq(0)
-            expect(importer_run.failed_children).to eq(0)
           end
         end
 
@@ -235,8 +231,6 @@ module Bulkrax
           expect(importer_run.processed_file_sets).to eq(0)
           expect(importer_run.processed_works).to eq(0)
           expect(importer_run.failed_works).to eq(0)
-          expect(importer_run.processed_children).to eq(0)
-          expect(importer_run.failed_children).to eq(0)
 
           import_file_set_job.perform(entry.id, importer_run.id)
           importer_run.reload
@@ -250,8 +244,6 @@ module Bulkrax
           expect(importer_run.processed_file_sets).to eq(0)
           expect(importer_run.processed_works).to eq(0)
           expect(importer_run.failed_works).to eq(0)
-          expect(importer_run.processed_children).to eq(0)
-          expect(importer_run.failed_children).to eq(0)
         end
       end
     end
diff --git a/spec/jobs/bulkrax/import_work_job_spec.rb b/spec/jobs/bulkrax/import_work_job_spec.rb
index 377fe485a..f55eb50a4 100644
--- a/spec/jobs/bulkrax/import_work_job_spec.rb
+++ b/spec/jobs/bulkrax/import_work_job_spec.rb
@@ -57,8 +57,6 @@ module Bulkrax
         expect(importer_run.processed_file_sets).to eq(0)
         expect(importer_run.failed_file_sets).to eq(0)
         expect(importer_run.failed_works).to eq(0)
-        expect(importer_run.processed_children).to eq(0)
-        expect(importer_run.failed_children).to eq(0)
 
         import_work_job.perform(1, importer_run_id)
         importer_run.reload
@@ -72,8 +70,6 @@ module Bulkrax
         expect(importer_run.processed_file_sets).to eq(0)
         expect(importer_run.failed_file_sets).to eq(0)
         expect(importer_run.failed_works).to eq(0)
-        expect(importer_run.processed_children).to eq(0)
-        expect(importer_run.failed_children).to eq(0)
       end
     end
 
diff --git a/spec/test_app/db/schema.rb b/spec/test_app/db/schema.rb
index 054b476fc..70580d6ec 100644
--- a/spec/test_app/db/schema.rb
+++ b/spec/test_app/db/schema.rb
@@ -99,8 +99,6 @@
     t.integer "total_file_set_entries", default: 0
     t.integer "processed_works", default: 0
     t.integer "failed_works", default: 0
-    t.integer "processed_children", default: 0
-    t.integer "failed_children", default: 0
     t.index ["importer_id"], name: "index_bulkrax_importer_runs_on_importer_id"
   end
 

From 321ca84f00f0e9880a5b439c63dd876b053f9fb4 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 31 Oct 2023 11:18:00 -0700
Subject: [PATCH 8/9] prefer avoiding raw SQL

---
 app/jobs/bulkrax/create_relationships_job.rb | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb
index ebbffd900..0dad0d253 100644
--- a/app/jobs/bulkrax/create_relationships_job.rb
+++ b/app/jobs/bulkrax/create_relationships_job.rb
@@ -97,11 +97,7 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
       end
 
       if errors.present?
-        ImporterRun.connection.execute(<<-SQL)
-          UPDATE bulkrax_importer_runs
-          SET failed_relationships = COALESCE(failed_relationships, 0) + #{number_of_failures}
-          WHERE id = #{importer_run_id}
-        SQL
+        ImporterRun.update_counters(importer_run_id, failed_relationships: number_of_failures)
 
         parent_entry&.set_status_info(errors.last, importer_run)
 
@@ -109,11 +105,7 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
         reschedule({ parent_identifier: parent_identifier, importer_run_id: importer_run_id })
         return false # stop current job from continuing to run after rescheduling
       else
-        ImporterRun.connection.execute(<<-SQL)
-          UPDATE bulkrax_importer_runs
-          SET processed_relationships = COALESCE(processed_relationships, 0) + #{number_of_successes}
-          WHERE id = #{importer_run_id}
-        SQL
+        ImporterRun.update_counters(importer_run_id, processed_relationships: number_of_successes)
       end
     end
     # rubocop:enable Metrics/MethodLength

From de3b9b4ec6971fd46c4a128cc374fd04b683ac93 Mon Sep 17 00:00:00 2001
From: Benjamin Kiah Stroud <32469930+bkiahstroud@users.noreply.github.com>
Date: Tue, 31 Oct 2023 11:21:34 -0700
Subject: [PATCH 9/9] rubocop

---
 app/jobs/bulkrax/create_relationships_job.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/jobs/bulkrax/create_relationships_job.rb b/app/jobs/bulkrax/create_relationships_job.rb
index 0dad0d253..b6399dabd 100644
--- a/app/jobs/bulkrax/create_relationships_job.rb
+++ b/app/jobs/bulkrax/create_relationships_job.rb
@@ -97,7 +97,9 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
       end
 
       if errors.present?
+        # rubocop:disable Rails/SkipsModelValidations
         ImporterRun.update_counters(importer_run_id, failed_relationships: number_of_failures)
+        # rubocop:enable Rails/SkipsModelValidations
 
         parent_entry&.set_status_info(errors.last, importer_run)
 
@@ -105,7 +107,9 @@ def perform(parent_identifier:, importer_run_id:) # rubocop:disable Metrics/AbcS
         reschedule({ parent_identifier: parent_identifier, importer_run_id: importer_run_id })
         return false # stop current job from continuing to run after rescheduling
       else
+        # rubocop:disable Rails/SkipsModelValidations
         ImporterRun.update_counters(importer_run_id, processed_relationships: number_of_successes)
+        # rubocop:enable Rails/SkipsModelValidations
       end
     end
     # rubocop:enable Metrics/MethodLength