Exercise 3 arich

.gitignore

@@ -30,3 +30,5 @@
 
 # Ignore master key for decrypting credentials and more.
 /config/master.key
+
+*.DS_Store

.tool-versions

@@ -0,0 +1 @@
+ruby 3.3.9

app/controllers/certificate_quantities_controller.rb

@@ -60,7 +60,7 @@ def transfer
       @certificate_quantity.update(account: account)
     elsif organization_id
       organization = Organization.find(organization_id)
-      @certificate_quantity.update(status: "intransit", to_organization: organization)
+      @certificate_quantity.update(status: "intransit", to_organization: organization, transfer_initiated_at: Time.zone.now)
     end
 
     render "show"
@@ -74,7 +74,7 @@ def cancel_transfer
       return head :unprocessable_entity
     end
 
-    @certificate_quantity.update(status: "active", to_organization: nil)
+    @certificate_quantity.update(status: "active", to_organization: nil, transfer_initiated_at: nil)
 
     render "show"
   end
@@ -87,7 +87,7 @@ def accept_transfer
       return head :unprocessable_entity
     end
 
-    @certificate_quantity.update(status: "active", to_organization: nil, account: current_user.organization.default_account)
+    @certificate_quantity.update(status: "active", to_organization: nil, transfer_initiated_at: nil, account: current_user.organization.default_account)
 
     render "show"
   end

app/jobs/cancel_stale_transfers_job.rb

@@ -0,0 +1,22 @@
+class CancelStaleTransfersJob < ApplicationJob
+  queue_as :default
+
+  # Prevents multiple instances from running simultaneously, allow if a job is still running after 60 minutes
+  limits_concurrency to: 1, key: -> { "cancel_stale_transfers" }, duration: 60.minutes
+
+  def perform
+    stale_certificate_quantities.find_each do |certificate_quantity|
+      certificate_quantity.cancel_transfer!
+    end
+  end
+
+  private
+
+  def stale_certificate_quantities
+    CertificateQuantity.where(status: "intransit").where("transfer_initiated_at < ?", stale_cutoff)
+  end
+
+  def stale_cutoff
+    24.hours.ago
+  end
+end

app/models/certificate_quantity.rb

@@ -11,4 +11,31 @@ def split(quantity)
   def retire
     update(status: "retired")
   end
+
+  def cancel_transfer!
+    log_transfer_cancellation
+    update!(status: "active", to_organization: nil, transfer_initiated_at: nil)
+  end
+
+  private
+
+  def log_transfer_cancellation
+    Rails.logger.info { {
+      msg: "Transfer timeout, cancelling transfer for certificate quantity ID: #{id}.",
+      id: id,
+      to_organization: to_organization
+    } }
+
+    # Not implemented, log to Transactions table.
+    # The above log might be in a service object that both logs and creates the Transaction object
+    # so it's not this model's responsibility to know how to do this.
+    #
+    # Transaction.create!(
+    #   :auto_cancel_stale_transfer,
+    #   certificate_quantity: certificate_quantity.id,
+    #   from_status: "intransit",
+    #   to_status: "active",
+    #   to_organization: certificate_quantity.to_organization
+    # )
+  end
 end

config/recurring.yml

@@ -13,3 +13,8 @@ production:
   clear_solid_queue_finished_jobs:
     command: "SolidQueue::Job.clear_finished_in_batches(sleep_between_batches: 0.3)"
     schedule: every hour at minute 12
+
+  cancel_stale_transfers:
+    class: CancelStaleTransfersJob
+    schedule: every 5 minutes
+

db/migrate/20250914010837_add_transfer_initiated_at_to_certificate_quantities.rb

@@ -0,0 +1,6 @@
+class AddTransferInitiatedAtToCertificateQuantities < ActiveRecord::Migration[8.0]
+  def change
+    add_column :certificate_quantities, :transfer_initiated_at, :datetime
+    add_index :certificate_quantities, :transfer_initiated_at
+  end
+end

exercise_3_discussion.md

@@ -0,0 +1,27 @@
+# Exercise 3 Implementation
+
+## Assumptions
+1. A stale transfer is considered any transfer that is still status: `intransit` for > 24 hours. 
+2. The number of stale transfers in a given day is < 1M
+3. There is not a particular time of day we want to cancel `intransit` stale transfers en masse, we want to cancel ASAP after 24 hours of time in transit. 
+4. There are not going to be model level validations added to the currently sparse validations on `CertificateQuantity` that would cause an automated update to fail.
+
+## Discussion
+
+I added a new column `certificate_quantities.transfer_initiated_at`. If exercise 1 were implemented, 
+this might be a simpler column name like `status_updated_at` but for this exercise I figured let's be precise. 
+This column must be updated when a transfer is initiated, accepted, or cancelled. 
+
+We're kicking off a recurring SolidQueue job `CancelStaleTransfersJob` every 5 minutes to catch any `CertificateQuantities` in transit that have 
+gone stale in the past 5 minutes. We've limited the concurrency to 1 so even if this job runs long, it shouldn't overlap
+with another recurring job run. It will allow another job to run if one job is stuck for > 1 hr. 
+
+## Extensions
+
+### We want this to cancel intransit transfers immediately at our 24h threshold
+We could enqueue a job to cancel the transfer at 24h exactly after the transfer was initiated, and if the CertificateQuantity is no longer `intransit`, the job is a noop. 
+
+### We have millions of rows of transfers to cancel and this is always taking > 5 min
+I would first try implementing a fanout strategy, where the periodic job `FindStaleTransfersJob` enqueues n other jobs `CancelStaleTransferJob`. `CancelStaleTransferJob` would take an argument for a `certificate_quantity_id`. Each `CancelStaleTransferJob` job would be responsible for 
+canceling that stale transfer. If the state has changed and the certificate_quantity is no longer `intransit`, it would noop and return early. 
+We would enqueue the jobs using bulk enqueuing to reduce roundtrips to the database: https://guides.rubyonrails.org/active_job_basics.html#bulk-enqueuing

spec/jobs/cancel_stale_transfers_job_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+
+RSpec.describe "CancelStaleTransfersJob", type: :job do
+  def create_certificate_quantity
+    generation = create(:generation)
+    certificate = generation.certificate
+    certificate_quantity = certificate.certificate_quantities.first
+    certificate_quantity.update!(
+      status: "intransit",
+      to_organization: create(:organization),
+      transfer_initiated_at: 25.hours.ago
+    )
+    certificate_quantity
+  end
+
+  it "resets intransit certificate quantities in transit for over 24 hours" do
+    certificate_quantity = create_certificate_quantity
+
+    CancelStaleTransfersJob.perform_now
+
+    certificate_quantity.reload
+    expect(certificate_quantity.status).to eq("active")
+    expect(certificate_quantity.transfer_initiated_at).to be_nil
+    expect(certificate_quantity.to_organization).to be_nil
+  end
+
+  it "does not reset intransit certificate quantities in transit for under 24 hours" do
+    certificate_quantity = create_certificate_quantity
+    certificate_quantity.update!(transfer_initiated_at: 23.hours.ago)
+
+    CancelStaleTransfersJob.perform_now
+
+    certificate_quantity.reload
+    expect(certificate_quantity.status).to eq("intransit")
+    expect(certificate_quantity.transfer_initiated_at).to be_present
+    expect(certificate_quantity.to_organization).to be_present
+  end
+
+  it "does not reset active certificate quantities" do
+    certificate_quantity = create_certificate_quantity
+    certificate_quantity.update!(transfer_initiated_at: nil, status: "active", to_organization: nil)
+
+    CancelStaleTransfersJob.perform_now
+
+    certificate_quantity.reload
+    expect(certificate_quantity.status).to eq("active")
+    expect(certificate_quantity.transfer_initiated_at).to be_nil
+    expect(certificate_quantity.to_organization).to be_nil
+  end
+end

spec/models/certificate_quantity_spec.rb

@@ -0,0 +1,36 @@
+require 'rails_helper'
+
+describe "CertificateQuantity" do
+  describe "#cancel_transfer!" do
+    let(:other_organization) { create(:organization) }
+    let(:generation) { create(:generation, quantity: 5, generator: create(:generator)) }
+    let(:certificate_quantity) { generation.certificate.certificate_quantities.first }
+
+    before do
+      certificate_quantity.update!(
+        status: "intransit",
+        to_organization: other_organization,
+        transfer_initiated_at: 1.hour.ago
+      )
+      certificate_quantity.cancel_transfer!
+    end
+
+    it "resets the status to active" do
+      expect(certificate_quantity.reload.status).to eq("active")
+    end
+
+    it "clears the to_organization" do
+      expect(certificate_quantity.reload.to_organization).to be_nil
+    end
+
+    it "clears the transfer_initiated_at timestamp" do
+      expect(certificate_quantity.reload.transfer_initiated_at).to be_nil
+    end
+
+    it "logs the transfer cancellation" do
+      expect(Rails.logger).to receive(:info)
+
+      certificate_quantity.cancel_transfer!
+    end
+  end
+end

spec/requests/certificate_quantities_request_spec.rb

@@ -157,6 +157,10 @@ def certificate_quantity_json(certificate_quantity)
       it 'modifies the to_organization' do
         expect(certificate_quantity.reload.to_organization).to eq(other_organization)
       end
+
+      it 'sets the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_within(1.minute).of(Time.zone.now)
+      end
     end
 
     context "when passing invalid parameters" do
@@ -204,7 +208,7 @@ def certificate_quantity_json(certificate_quantity)
   context 'cancel_transfer' do
     context "when cancelling a transfer that is associated with the user's organization" do
       before do
-        certificate_quantity.update(status: 'intransit')
+        certificate_quantity.update(status: 'intransit', to_organization: other_organization, transfer_initiated_at: 1.hour.ago)
         put "/certificate_quantities/#{certificate_quantity.id}/cancel_transfer", headers: headers
       end
 
@@ -224,6 +228,10 @@ def certificate_quantity_json(certificate_quantity)
       it 'clears out the to_organization' do
         expect(certificate_quantity.reload.to_organization).to be_nil
       end
+
+      it 'clears out the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_nil
+      end
     end
 
     context 'when the status is not intransit' do
@@ -236,7 +244,7 @@ def certificate_quantity_json(certificate_quantity)
 
     context "when canceling a transfer of a certificate to another organization that is associated with the user's organization" do
       before do
-        certificate_quantity.update(status: 'intransit', to_organization: other_organization)
+        certificate_quantity.update(status: 'intransit', to_organization: other_organization, transfer_initiated_at: 1.hour.ago)
         put "/certificate_quantities/#{certificate_quantity.id}/cancel_transfer", headers: headers
       end
 
@@ -256,11 +264,15 @@ def certificate_quantity_json(certificate_quantity)
       it 'resets the to_organization' do
         expect(certificate_quantity.reload.to_organization).to be_nil
       end
+
+      it 'resets the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_nil
+      end
     end
 
     context "when transferring a certificate that is not associated with the user's organization" do
       before do
-        certificate_quantity.update(status: 'intransit', to_organization: other_organization)
+        certificate_quantity.update(status: 'intransit', to_organization: other_organization, transfer_initiated_at: 1.hour.ago)
         put "/certificate_quantities/#{other_certificate_quantity.id}/transfer", headers: headers
       end
 
@@ -275,6 +287,10 @@ def certificate_quantity_json(certificate_quantity)
       it 'leaves the to_organization' do
         expect(certificate_quantity.reload.to_organization).to eq(other_organization)
       end
+
+      it 'leaves the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_present
+      end
     end
   end
 
@@ -306,6 +322,10 @@ def certificate_quantity_json(certificate_quantity)
         expect(certificate_quantity.reload.to_organization).to be_nil
       end
 
+      it 'clears out the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_nil
+      end
+
       it 'sets the account to the organizations default account' do
         expect(certificate_quantity.reload.account).to eq(other_organization.default_account)
       end
@@ -326,7 +346,7 @@ def certificate_quantity_json(certificate_quantity)
 
     context "when transferring a certificate that is not associated with the user's organization" do
       before do
-        certificate_quantity.update(status: 'intransit', to_organization: other_organization)
+        certificate_quantity.update(status: 'intransit', to_organization: other_organization, transfer_initiated_at: 1.hour.ago)
         put "/certificate_quantities/#{other_certificate_quantity.id}/transfer", headers: headers
       end
 
@@ -341,6 +361,10 @@ def certificate_quantity_json(certificate_quantity)
       it 'leaves the to_organization' do
         expect(certificate_quantity.reload.to_organization).to eq(other_organization)
       end
+
+      it 'leaves the transfer_initiated_at timestamp' do
+        expect(certificate_quantity.reload.transfer_initiated_at).to be_present
+      end
     end
   end