From f22029ddac74659cbb437c1be6fce54736951372 Mon Sep 17 00:00:00 2001 From: Samuel Giddins Date: Mon, 18 Nov 2024 13:10:07 -0800 Subject: [PATCH] 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 --- app/jobs/after_version_write_job.rb | 1 + app/jobs/set_linkset_home_job.rb | 14 ++++++++++++++ app/models/linkset.rb | 6 +----- app/models/pusher.rb | 5 ++++- app/models/version.rb | 14 +++++++++++++- test/models/rubygem_test.rb | 2 +- 6 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 app/jobs/set_linkset_home_job.rb diff --git a/app/jobs/after_version_write_job.rb b/app/jobs/after_version_write_job.rb index 651af4f5bdf..801168cac86 100644 --- a/app/jobs/after_version_write_job.rb +++ b/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 diff --git a/app/jobs/set_linkset_home_job.rb b/app/jobs/set_linkset_home_job.rb new file mode 100644 index 00000000000..0c60dff0acd --- /dev/null +++ b/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 diff --git a/app/models/linkset.rb b/app/models/linkset.rb index ccea8016728..559293ae1f7 100644 --- a/app/models/linkset.rb +++ b/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 diff --git a/app/models/pusher.rb b/app/models/pusher.rb index 0f0dab75908..6031cee7251 100644 --- a/app/models/pusher.rb +++ b/app/models/pusher.rb @@ -226,7 +226,9 @@ def notify(message, code) def update rubygem.disown if rubygem.versions.indexed.count.zero? + logger.info { { message: "Updating rubygem attributes from gem specification", rubygem: rubygem.name, spec: spec.to_ruby } } rubygem.update_attributes_from_gem_specification!(version, spec) + logger.info { { message: "Updated rubygem attributes from gem specification", rubygem: rubygem.name, linkset: rubygem.linkset.inspect } } if rubygem.unowned? if api_key.user? @@ -246,7 +248,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 diff --git a/app/models/version.rb b/app/models/version.rb index f3aa3e75413..9dc57a83c55 100644 --- a/app/models/version.rb +++ b/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 diff --git a/test/models/rubygem_test.rb b/test/models/rubygem_test.rb index 7b23a1573de..0cb70fec45b 100644 --- a/test/models/rubygem_test.rb +++ b/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)