Skip to content

Commit

Permalink
Update linkset homepage from gemspec only after version gets indexed
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
segiddins committed Nov 18, 2024
1 parent a07d05c commit f22029d
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 8 deletions.
1 change: 1 addition & 0 deletions app/jobs/after_version_write_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions app/jobs/set_linkset_home_job.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 1 addition & 5 deletions app/models/linkset.rb
Original file line number Diff line number Diff line change
@@ -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?

Expand All @@ -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
Expand Down
5 changes: 4 additions & 1 deletion app/models/pusher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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

Expand Down
14 changes: 13 additions & 1 deletion app/models/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -296,7 +308,7 @@ def <=>(other)
end

def slug
full_name.remove(/^#{rubygem.name}-/)
full_name.delete_prefix("#{rubygem.name}-")
end

def downloads_count
Expand Down
2 changes: 1 addition & 1 deletion test/models/rubygem_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f22029d

Please sign in to comment.