Add UserAvatarJob for Inkbunny avatar management and refactor user model

app/jobs/domain/inkbunny/job/user_avatar_job.rb

@@ -0,0 +1,63 @@
+module Domain::Inkbunny::Job
+  class UserAvatarJob < Base
+    queue_as :inkbunny_user_avatar
+    ignore_signature_args :caused_by_entry
+
+    def perform(args)
+      @user = args[:user] || raise("user must exist")
+      @caused_by_entry = args[:caused_by_entry]
+
+      logger.prefix =
+        proc do
+          "[user #{@user.name.to_s.bold} / #{@user.ib_user_id.to_s.bold}]"
+        end
+
+      if @user.avatar_url_str.blank?
+        logger.warn("user has no avatar_url_str")
+        return
+      end
+
+      response =
+        http_client.get(@user.avatar_url_str, caused_by_entry: @caused_by_entry)
+
+      @user.avatar_state_detail ||= {}
+      @user.avatar_state_detail["log_entries"] ||= [
+        @user.avatar_file_log_entry_id,
+      ].compact
+      @user.avatar_state_detail["log_entries"] << response.log_entry.id
+      @user.avatar_file_log_entry_id = response.log_entry.id
+
+      case response.status_code
+      when 200
+        @user.avatar_state = :ok
+        @user.avatar_downloaded_at = response.log_entry.created_at
+        @user.avatar_file_sha256 = response.log_entry.response_sha256
+        logger.info("downloaded avatar")
+      when 404
+        @user.avatar_state = :not_found
+        if @user.avatar_file_sha256.blank?
+          @user.avatar_downloaded_at = response.log_entry.created_at
+          logger.info("avatar 404, and no previous file")
+        else
+          logger.info("avatar 404, keeping previous file")
+        end
+      else
+        @user.avatar_state = :error
+        @user.avatar_state_detail[
+          "download_error"
+        ] = "http status #{response.status_code}"
+        if @user.avatar_file_sha256.blank?
+          @user.avatar_downloaded_at = response.log_entry.created_at
+          logger.info("avatar error, and no previous file")
+        else
+          logger.info("avatar error, keeping previous file")
+        end
+        fatal_error(
+          "http #{response.status_code}, log entry #{response.log_entry.id}",
+        )
+      end
+    ensure
+      @user.save! if @user
+    end
+  end
+end

app/models/domain/inkbunny/avatar.rb

@@ -1,3 +0,0 @@
-class Domain::Inkbunny::Avatar < ReduxApplicationRecord
-  self.table_name = "domain_inkbunny_avatars"
-end

app/models/domain/inkbunny/user.rb

@@ -6,11 +6,24 @@ class Domain::Inkbunny::User < ReduxApplicationRecord
            inverse_of: :creator,
            foreign_key: :creator_id
 
+  belongs_to :avatar,
+             class_name: "::BlobEntry",
+             foreign_key: :avatar_file_sha256,
+             primary_key: :sha256,
+             optional: true
+
+  belongs_to :avatar_log_entry,
+             class_name: "::HttpLogEntry",
+             foreign_key: :avatar_file_log_entry_id,
+             optional: true
+
   validates_presence_of :ib_user_id, :name
   enum :state, %i[ok error]
+  enum :avatar_state, %i[ok not_found error], prefix: :avatar
   after_initialize do
     self.state ||= :ok
     self.state_detail ||= {}
+    self.avatar_state_detail ||= {}
   end
 
   def to_param

db/migrate/20241230220636_remove_inkbunny_user_avatars_and_add_avatar_columns_to_users.rb

@@ -0,0 +1,16 @@
+class RemoveInkbunnyUserAvatarsAndAddAvatarColumnsToUsers < ActiveRecord::Migration[
+  7.2
+]
+  def change
+    drop_table :domain_inkbunny_user_avatars
+
+    add_column :domain_inkbunny_users, :avatar_downloaded_at, :datetime
+    add_column :domain_inkbunny_users, :avatar_state, :integer
+    add_column :domain_inkbunny_users,
+               :avatar_state_detail,
+               :jsonb,
+               default: {
+               },
+               null: false
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_12_30_060212) do
+ActiveRecord::Schema[7.2].define(version: 2024_12_30_220636) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_prewarm"
   enable_extension "pg_stat_statements"
@@ -1531,13 +1531,6 @@ ActiveRecord::Schema[7.2].define(version: 2024_12_30_060212) do
     t.datetime "updated_at", null: false
   end
 
-  create_table "domain_inkbunny_user_avatars", force: :cascade do |t|
-    t.bigint "user_id", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.index ["user_id"], name: "index_domain_inkbunny_user_avatars_on_user_id"
-  end
-
   create_table "domain_inkbunny_users", force: :cascade do |t|
     t.integer "state", null: false
     t.json "state_detail"
@@ -1548,6 +1541,9 @@ ActiveRecord::Schema[7.2].define(version: 2024_12_30_060212) do
     t.bigint "avatar_file_log_entry_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "avatar_downloaded_at"
+    t.integer "avatar_state"
+    t.jsonb "avatar_state_detail", default: {}, null: false
     t.index ["ib_user_id"], name: "index_domain_inkbunny_users_on_ib_user_id", unique: true
   end
 

spec/jobs/domain/inkbunny/job/user_avatar_job_spec.rb

@@ -0,0 +1,212 @@
+require "rails_helper"
+
+describe Domain::Inkbunny::Job::UserAvatarJob do
+  let(:http_client_mock) { instance_double("::Scraper::HttpClient") }
+  before { Scraper::ClientFactory.http_client_mock = http_client_mock }
+
+  let(:avatar_url_str) { "https://example.com/avatar.jpg" }
+  let(:avatar_state) { nil }
+  let(:existing_avatar_sha256) { nil }
+  let(:existing_downloaded_at) { nil }
+  let(:existing_log_entry_id) { nil }
+  let!(:user) do
+    create(
+      :domain_inkbunny_user,
+      avatar_url_str: avatar_url_str,
+      avatar_state: avatar_state,
+      avatar_file_sha256: existing_avatar_sha256,
+      avatar_downloaded_at: existing_downloaded_at,
+      avatar_file_log_entry_id: existing_log_entry_id,
+    )
+  end
+
+  def perform_job
+    perform_now({ user: user })
+  end
+
+  context "when avatar_url_str is blank" do
+    let(:avatar_url_str) { nil }
+
+    it "logs a warning and returns" do
+      perform_job
+      user.reload
+      expect(user.avatar_state).to be_nil
+      expect(user.avatar).to be_nil
+      expect(user.avatar_log_entry).to be_nil
+    end
+  end
+
+  context "when avatar download succeeds" do
+    let! :log_entries do
+      SpecUtil.init_http_client_mock(
+        http_client_mock,
+        [
+          {
+            uri: avatar_url_str,
+            status_code: 200,
+            content_type: "image/jpeg",
+            contents: "test",
+            caused_by_entry_idx: nil,
+          },
+        ],
+      )
+    end
+
+    it "sets ok state and updates file info" do
+      perform_job
+
+      user.reload
+      expect(user.avatar_state).to eq("ok")
+      expect(user.avatar_file_sha256).to eq(log_entries[0].response_sha256)
+      expect(user.avatar).to be_present
+      expect(user.avatar.sha256).to eq(log_entries[0].response_sha256)
+      expect(user.avatar_log_entry).to eq(log_entries[0])
+      expect(user.avatar_downloaded_at).to be_within(1.second).of(
+        log_entries[0].created_at,
+      )
+    end
+
+    context "when previous file exists" do
+      let(:existing_log_entry) { create(:http_log_entry) }
+      let(:existing_log_entry_id) { existing_log_entry.id }
+      let(:existing_blob_entry) do
+        create(:blob_entry, content: "previous", content_type: "image/jpeg")
+      end
+      let(:existing_avatar_sha256) { existing_blob_entry.sha256 }
+      let(:avatar_state) { :ok }
+      let(:existing_downloaded_at) { 1.day.ago }
+
+      it "updates file info" do
+        perform_job
+
+        user.reload
+        expect(user.avatar_state).to eq("ok")
+        expect(user.avatar_file_sha256).to eq(log_entries[0].response_sha256)
+        expect(user.avatar).to be_present
+        expect(user.avatar.sha256).to eq(log_entries[0].response_sha256)
+        expect(user.avatar_log_entry).to eq(log_entries[0])
+        expect(user.avatar_state_detail["log_entries"]).to eq(
+          [existing_log_entry.id, log_entries[0].id],
+        )
+        expect(user.avatar_downloaded_at).to be_within(1.second).of(
+          log_entries[0].created_at,
+        )
+      end
+    end
+  end
+
+  context "when avatar download returns 404" do
+    let! :log_entries do
+      SpecUtil.init_http_client_mock(
+        http_client_mock,
+        [
+          {
+            uri: avatar_url_str,
+            status_code: 404,
+            content_type: "text/html",
+            contents: "not found",
+            caused_by_entry_idx: nil,
+          },
+        ],
+      )
+    end
+
+    it "sets not_found state" do
+      perform_job
+
+      user.reload
+      expect(user.avatar_state).to eq("not_found")
+      expect(user.avatar_file_sha256).to be_nil
+      expect(user.avatar).to be_nil
+      expect(user.avatar_log_entry).to eq(log_entries[0])
+      expect(user.avatar_state_detail["log_entries"]).to eq([log_entries[0].id])
+      expect(user.avatar_downloaded_at).to be_within(1.second).of(
+        log_entries[0].created_at,
+      )
+    end
+
+    context "when previous file exists" do
+      let(:existing_log_entry) { create(:http_log_entry) }
+      let(:existing_log_entry_id) { existing_log_entry.id }
+      let(:existing_blob_entry) do
+        create(:blob_entry, content: "previous", content_type: "image/jpeg")
+      end
+      let(:existing_avatar_sha256) { existing_blob_entry.sha256 }
+      let(:avatar_state) { :ok }
+      let(:existing_downloaded_at) { 1.day.ago }
+
+      it "keeps previous file but updates state to not_found" do
+        perform_job
+
+        user.reload
+        expect(user.avatar_state).to eq("not_found")
+        expect(user.avatar_file_sha256).to eq(existing_blob_entry.sha256)
+        expect(user.avatar).to be_present
+        expect(user.avatar.sha256).to eq(existing_blob_entry.sha256)
+        expect(user.avatar_log_entry).to eq(log_entries[0])
+        expect(user.avatar_state_detail["log_entries"]).to eq(
+          [existing_log_entry.id, log_entries[0].id],
+        )
+        expect(user.avatar_downloaded_at).to be_within(1.second).of(
+          existing_downloaded_at,
+        )
+      end
+    end
+  end
+
+  context "when avatar download fails with error" do
+    let! :log_entries do
+      SpecUtil.init_http_client_mock(
+        http_client_mock,
+        [
+          {
+            uri: avatar_url_str,
+            status_code: 500,
+            content_type: "text/html",
+            contents: "server error",
+            caused_by_entry_idx: nil,
+          },
+        ],
+      )
+    end
+
+    it "sets error state and raises fatal error" do
+      perform_now({ user: user }, should_raise: Scraper::JobBase::JobError)
+
+      user.reload
+      expect(user.avatar_state).to eq("error")
+      expect(user.avatar_state_detail["download_error"]).to include("500")
+      expect(user.avatar).to be_nil
+      expect(user.avatar_log_entry).to eq(log_entries[0])
+      expect(user.avatar_state_detail["log_entries"]).to eq([log_entries[0].id])
+    end
+
+    context "when previous file exists" do
+      let(:existing_log_entry) { create(:http_log_entry) }
+      let(:existing_log_entry_id) { existing_log_entry.id }
+      let(:existing_blob_entry) do
+        create(:blob_entry, content: "previous", content_type: "image/jpeg")
+      end
+      let(:existing_avatar_sha256) { existing_blob_entry.sha256 }
+      let(:avatar_state) { :ok }
+      let(:existing_downloaded_at) { 1.day.ago }
+
+      it "keeps previous file but updates state to error" do
+        perform_now({ user: user }, should_raise: Scraper::JobBase::JobError)
+
+        user.reload
+        expect(user.avatar_state).to eq("error")
+        expect(user.avatar_file_sha256).to eq(existing_blob_entry.sha256)
+        expect(user.avatar).to be_present
+        expect(user.avatar.sha256).to eq(existing_blob_entry.sha256)
+        expect(user.avatar_log_entry).to eq(log_entries[0])
+        expect(user.avatar_state_detail["log_entries"]).to eq(
+          [existing_log_entry.id, log_entries[0].id],
+        )
+        expect(user.avatar_downloaded_at).to be_within(1.second).of(
+          existing_downloaded_at,
+        )
+      end
+    end
+  end
+end