Update linkset homepage from gemspec only after version gets indexed

In current implementation, version.reload.indexed? is always false, so gems that only use spec.homepage to specify a homepage will never have a linkset created/updated

Signed-off-by: Samuel Giddins <[email protected]>

app/jobs/after_version_write_job.rb

@@ -16,6 +16,7 @@ def perform(version:)
       version.update!(indexed: true)
       checksum = GemInfo.new(rubygem.name, cached: false).info_checksum
       version.update_attribute :info_checksum, checksum
+      SetLinksetHomeJob.perform_later(version:)
     end
   end
 

app/jobs/set_linkset_home_job.rb

@@ -0,0 +1,14 @@
+class SetLinksetHomeJob < ApplicationJob
+  queue_as :default
+
+  def perform(version:)
+    return unless version.latest? && version.indexed?
+
+    gem = RubygemFs.instance.get("gems/#{version.gem_file_name}")
+    package = Gem::Package.new(StringIO.new(gem))
+    homepage = package.spec.homepage
+
+    version.rubygem.linkset ||= Linkset.new
+    version.rubygem.linkset.update!(home: homepage)
+  end
+end

app/models/linkset.rb

@@ -1,5 +1,5 @@
 class Linkset < ApplicationRecord
-  belongs_to :rubygem
+  belongs_to :rubygem, inverse_of: :linkset
 
   before_save :create_homepage_link_verification, if: :home_changed?
 
@@ -19,10 +19,6 @@ def empty?
     LINKS.map { |link| attributes[link] }.all?(&:blank?)
   end
 
-  def update_attributes_from_gem_specification!(spec)
-    update!(home: spec.homepage)
-  end
-
   def create_homepage_link_verification
     return if home.blank?
     rubygem.link_verifications.find_or_create_by!(uri: home).retry_if_needed

app/models/pusher.rb

@@ -246,7 +246,8 @@ def update
     end
 
     true
-  rescue ActiveRecord::RecordInvalid, ActiveRecord::Rollback, ActiveRecord::RecordNotUnique
+  rescue ActiveRecord::RecordInvalid, ActiveRecord::Rollback, ActiveRecord::RecordNotUnique => e
+    logger.info { { message: "Error updating rubygem", exception: e } }
     false
   end
 

app/models/rubygem.rb

@@ -14,7 +14,7 @@ class Rubygem < ApplicationRecord
   has_many :versions, dependent: :destroy, validate: false
   has_one :latest_version, -> { latest.order(:position) }, class_name: "Version", inverse_of: :rubygem
   has_many :web_hooks, dependent: :destroy
-  has_one :linkset, dependent: :destroy
+  has_one :linkset, dependent: :destroy, inverse_of: :rubygem
   has_one :gem_download, -> { where(version_id: 0) }, inverse_of: :rubygem
   has_many :ownership_calls, -> { opened }, dependent: :destroy, inverse_of: :rubygem
   has_many :ownership_requests, -> { opened }, dependent: :destroy, inverse_of: :rubygem
@@ -273,34 +273,11 @@ def ownership_call
     ownership_calls.find_by(status: "opened")
   end
 
-  def update_versions!(version, spec)
-    version.update_attributes_from_gem_specification!(spec)
-  end
-
-  def update_dependencies!(version, spec)
-    spec.dependencies.each do |dependency|
-      version.dependencies.create!(gem_dependency: dependency)
-    rescue ActiveRecord::RecordInvalid => e
-      # ActiveRecord can't chain a nested error here, so we have to add and reraise
-      e.record.errors.errors.each do |error|
-        errors.import(error, attribute: "dependency.#{error.attribute}")
-      end
-      raise
-    end
-  end
-
-  def update_linkset!(spec)
-    self.linkset ||= Linkset.new
-    self.linkset.update_attributes_from_gem_specification!(spec)
-    self.linkset.save!
-  end
-
   def update_attributes_from_gem_specification!(version, spec)
     Rubygem.transaction do
       save!
-      update_versions! version, spec
-      update_dependencies! version, spec
-      update_linkset! spec if version.reload.latest?
+      version.update_attributes_from_gem_specification!(spec)
+      version.update_dependencies!(spec)
     end
   end
 

app/models/version.rb

@@ -276,6 +276,18 @@ def update_attributes_from_gem_specification!(spec)
     )
   end
 
+  def update_dependencies!(spec)
+    spec.dependencies.each do |dependency|
+      dependencies.create!(gem_dependency: dependency)
+    rescue ActiveRecord::RecordInvalid => e
+      # ActiveRecord can't chain a nested error here, so we have to add and reraise
+      e.record.errors.errors.each do |error|
+        errors.import(error, attribute: "dependency.#{error.attribute}")
+      end
+      raise
+    end
+  end
+
   def platform_as_number
     if platformed?
       0
@@ -296,7 +308,7 @@ def <=>(other)
   end
 
   def slug
-    full_name.remove(/^#{rubygem.name}-/)
+    full_name.delete_prefix("#{rubygem.name}-")
   end
 
   def downloads_count

test/integration/push_test.rb

@@ -76,13 +76,15 @@ class PushTest < ActionDispatch::IntegrationTest
     push_gem "sandworm-1.0.0.gem"
 
     assert_response :success
+    perform_enqueued_jobs
 
     get rubygem_path("sandworm")
 
     assert_response :success
     assert page.has_content?("sandworm")
     assert page.has_content?("1.0.0")
     assert page.has_content?("Pushed by")
+    assert page.has_link? "Homepage", href: "http://example.com/sandworm"
 
     css = %(div.gem__users a[alt=#{@user.handle}])
 

test/models/rubygem_test.rb

@@ -376,7 +376,7 @@ class RubygemTest < ActiveSupport::TestCase
       @specification.authors = [3]
 
       assert_raise ActiveRecord::RecordInvalid do
-        @rubygem.update_versions!(@version, @specification)
+        @version.update_attributes_from_gem_specification!(@specification)
       end
 
       assert_equal "Authors must be an Array of Strings", @rubygem.all_errors(@version)