Skip to content

Commit

Permalink
Load rails 7.0 defaults (#4388)
Browse files Browse the repository at this point in the history
* Load rails 7.0 defaults

* Update GemDownload.increment to use update_all instead of find_by_sql with UPDATE sql.

- find_by_sql calls could be cached and are not meant to be used for data modifications
- this changes returned value, but it seems it is used only in testing

* Update LogTicket.pop to use update_all instead of find_by_sql with UPDATE sql.

- find_by_sql calls could be cached and are not meant to be used for data modifications

---------

Co-authored-by: Josef Šimánek <[email protected]>
  • Loading branch information
segiddins and simi authored Mar 12, 2024
1 parent b8e92f5 commit 4ce2e86
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 117 deletions.
5 changes: 1 addition & 4 deletions app/models/gem_download.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,11 @@ def increment(count, rubygem_id:, version_id: 0)
scope = GemDownload.where(rubygem_id: rubygem_id).select("id")
scope = scope.where(version_id: version_id)
return scope.first if count.zero?
sql = scope.to_sql

update = "UPDATE #{quoted_table_name} SET count = count + ? WHERE id = (#{sql}) RETURNING *"

# TODO: Remove this comments, once we move to GemDownload only.
# insert = "INSERT INTO #{quoted_table_name} (rubygem_id, version_id, count) SELECT ?, ?, ?"
# find_by_sql(["WITH upsert AS (#{update}) #{insert} WHERE NOT EXISTS (SELECT * FROM upsert)", count, rubygem_id, version_id, count]).first
find_by_sql([update, count]).first
scope.update_all(["count = count + ?", count])
end

# Takes an array where members have the form
Expand Down
10 changes: 6 additions & 4 deletions app/models/log_ticket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ class LogTicket < ApplicationRecord
enum status: %i[pending processing failed processed].index_with(&:to_s)

def self.pop(key: nil, directory: nil)
scope = pending.limit(1).lock(true).select("id").order("id ASC")
scope = pending.limit(1).lock(true).order("id ASC")
scope = scope.where(key: key) if key
scope = scope.where(directory: directory) if directory
sql = scope.to_sql

find_by_sql(["UPDATE #{quoted_table_name} SET status = ? WHERE id IN (#{sql}) RETURNING *", "processing"]).first
scope.sole.tap do |ticket|
ticket.update_column(:status, "processing")
end
rescue ActiveRecord::RecordNotFound
nil # no ticket in queue found by `sole` call
end

def fs
Expand Down
2 changes: 1 addition & 1 deletion config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
module Gemcutter
class Application < Rails::Application
# Initialize configuration defaults for originally generated Rails version.
config.load_defaults 6.1
config.load_defaults 7.0

# Please, add to the `ignore` list any other `lib` subdirectories that do
# not contain `.rb` files, or that should not be reloaded or eager loaded.
Expand Down
105 changes: 0 additions & 105 deletions config/initializers/new_framework_defaults_7_0.rb

This file was deleted.

7 changes: 4 additions & 3 deletions test/models/gem_download_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ class GemDownloadTest < ActiveSupport::TestCase

context ".increment" do
should "not update if download doesnt exist" do
assert_nil GemDownload.increment(1, rubygem_id: 1)
assert_equal 0, GemDownload.increment(1, rubygem_id: 1)
end

should "not update if download count is nil" do
create(:gem_download, rubygem_id: 1, version_id: 0, count: nil)
download = GemDownload.increment(1, rubygem_id: 1)
download = create(:gem_download, rubygem_id: 1, version_id: 0, count: nil)
GemDownload.increment(1, rubygem_id: 1)
download.reload

assert_nil download.count
end
Expand Down

0 comments on commit 4ce2e86

Please sign in to comment.