From 766358e52b86ca0dc6cf9bb091565e97af00b19a Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Thu, 14 Nov 2024 09:03:57 -0500
Subject: [PATCH] Add coverage for malformed version cleanup in
 `SoftwareUpdateCheckService`, add helper query methods (#32876)

---
 app/models/software_update.rb                 | 16 +++++-
 app/services/software_update_check_service.rb |  2 +-
 spec/models/software_update_spec.rb           | 54 +++++++++++++++++++
 .../software_update_check_service_spec.rb     |  5 +-
 4 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/app/models/software_update.rb b/app/models/software_update.rb
index 2cb935130a..7e2b15656e 100644
--- a/app/models/software_update.rb
+++ b/app/models/software_update.rb
@@ -22,6 +22,14 @@ class SoftwareUpdate < ApplicationRecord
     Gem::Version.new(version)
   end
 
+  def outdated?
+    runtime_version >= gem_version
+  end
+
+  def pending?
+    gem_version > runtime_version
+  end
+
   class << self
     def check_enabled?
       Rails.configuration.x.mastodon.software_update_url.present?
@@ -30,11 +38,17 @@ class SoftwareUpdate < ApplicationRecord
     def pending_to_a
       return [] unless check_enabled?
 
-      all.to_a.filter { |update| update.gem_version > Mastodon::Version.gem_version }
+      all.to_a.filter(&:pending?)
     end
 
     def urgent_pending?
       pending_to_a.any?(&:urgent?)
     end
   end
+
+  private
+
+  def runtime_version
+    Mastodon::Version.gem_version
+  end
 end
diff --git a/app/services/software_update_check_service.rb b/app/services/software_update_check_service.rb
index 148f269289..45ec1f50db 100644
--- a/app/services/software_update_check_service.rb
+++ b/app/services/software_update_check_service.rb
@@ -12,7 +12,7 @@ class SoftwareUpdateCheckService < BaseService
 
   def clean_outdated_updates!
     SoftwareUpdate.find_each do |software_update|
-      software_update.delete if Mastodon::Version.gem_version >= software_update.gem_version
+      software_update.delete if software_update.outdated?
     rescue ArgumentError
       software_update.delete
     end
diff --git a/spec/models/software_update_spec.rb b/spec/models/software_update_spec.rb
index 0a494b0c4c..43e9cd058f 100644
--- a/spec/models/software_update_spec.rb
+++ b/spec/models/software_update_spec.rb
@@ -3,6 +3,60 @@
 require 'rails_helper'
 
 RSpec.describe SoftwareUpdate do
+  describe '#pending?' do
+    subject { described_class.new(version: update_version) }
+
+    before { allow(Mastodon::Version).to receive(:gem_version).and_return(Gem::Version.new(mastodon_version)) }
+
+    context 'when the runtime version is older than the update' do
+      let(:mastodon_version) { '4.0.0' }
+      let(:update_version) { '5.0.0' }
+
+      it { is_expected.to be_pending }
+    end
+
+    context 'when the runtime version is newer than the update' do
+      let(:mastodon_version) { '6.0.0' }
+      let(:update_version) { '5.0.0' }
+
+      it { is_expected.to_not be_pending }
+    end
+
+    context 'when the runtime version is same as the update' do
+      let(:mastodon_version) { '4.0.0' }
+      let(:update_version) { '4.0.0' }
+
+      it { is_expected.to_not be_pending }
+    end
+  end
+
+  describe '#outdated?' do
+    subject { described_class.new(version: update_version) }
+
+    before { allow(Mastodon::Version).to receive(:gem_version).and_return(Gem::Version.new(mastodon_version)) }
+
+    context 'when the runtime version is older than the update' do
+      let(:mastodon_version) { '4.0.0' }
+      let(:update_version) { '5.0.0' }
+
+      it { is_expected.to_not be_outdated }
+    end
+
+    context 'when the runtime version is newer than the update' do
+      let(:mastodon_version) { '6.0.0' }
+      let(:update_version) { '5.0.0' }
+
+      it { is_expected.to be_outdated }
+    end
+
+    context 'when the runtime version is same as the update' do
+      let(:mastodon_version) { '4.0.0' }
+      let(:update_version) { '4.0.0' }
+
+      it { is_expected.to be_outdated }
+    end
+  end
+
   describe '.pending_to_a' do
     before do
       allow(Mastodon::Version).to receive(:gem_version).and_return(Gem::Version.new(mastodon_version))
diff --git a/spec/services/software_update_check_service_spec.rb b/spec/services/software_update_check_service_spec.rb
index 4098bd470a..637e1e26c5 100644
--- a/spec/services/software_update_check_service_spec.rb
+++ b/spec/services/software_update_check_service_spec.rb
@@ -27,6 +27,7 @@ RSpec.describe SoftwareUpdateCheckService do
     before do
       Fabricate(:software_update, version: '3.5.0', type: 'major', urgent: false)
       Fabricate(:software_update, version: '42.13.12', type: 'major', urgent: false)
+      Fabricate(:software_update, version: 'Malformed', type: 'major', urgent: false)
 
       owner_user.settings.update('notification_emails.software_updates': 'all')
       owner_user.save!
@@ -50,7 +51,7 @@ RSpec.describe SoftwareUpdateCheckService do
       end
 
       it 'deletes outdated update records but keeps valid update records' do
-        expect { subject.call }.to change { SoftwareUpdate.pluck(:version).sort }.from(['3.5.0', '42.13.12']).to(['42.13.12'])
+        expect { subject.call }.to change { SoftwareUpdate.pluck(:version).sort }.from(['3.5.0', '42.13.12', 'Malformed']).to(['42.13.12'])
       end
     end
 
@@ -85,7 +86,7 @@ RSpec.describe SoftwareUpdateCheckService do
       end
 
       it 'updates the list of known updates' do
-        expect { subject.call }.to change { SoftwareUpdate.pluck(:version).sort }.from(['3.5.0', '42.13.12']).to(['4.2.1', '4.3.0', '5.0.0'])
+        expect { subject.call }.to change { SoftwareUpdate.pluck(:version).sort }.from(['3.5.0', '42.13.12', 'Malformed']).to(['4.2.1', '4.3.0', '5.0.0'])
       end
 
       context 'when no update is urgent' do