From 9faa01f42700bc71ca4ec25e686669d1bf158135 Mon Sep 17 00:00:00 2001 From: sonalkr132 Date: Thu, 26 May 2016 20:27:40 +0530 Subject: [PATCH 1/2] Use raw sql in fetch_dependency_from_db and add tests raw sql insures ordering consistent with bundler-api, ie versions are ordered by created_at, number(version) and platform, and deps are ordered by name. Add tests for filtering of indexed versions and listing of dependency. --- app/models/gem_dependent.rb | 56 ++++++++++--- .../api/v1/dependencies_controller_test.rb | 14 ++-- test/unit/gem_dependent_test.rb | 79 ++++++++++++++++--- 3 files changed, 119 insertions(+), 30 deletions(-) diff --git a/app/models/gem_dependent.rb b/app/models/gem_dependent.rb index 86348639f5b..db785899bdd 100644 --- a/app/models/gem_dependent.rb +++ b/app/models/gem_dependent.rb @@ -1,5 +1,6 @@ class GemDependent extend StatsD::Instrument + DepKey = Struct.new(:name, :number, :platform, :required_ruby_version, :required_rubygems_version, :sha256, :created_at) attr_reader :gem_names @@ -35,25 +36,56 @@ def fetch_dependencies private def fetch_dependency_from_db(gem_name) - gem_record = Rubygem.includes(:versions).find_by_name(gem_name) - return [] unless gem_record - gem_record.versions.includes(:dependencies).sort_by(&:number).reverse_each.map do |version| - version_deps = version.dependencies.select { |d| d.scope == 'runtime' } + sanitize_sql = ActiveRecord::Base.send(:sanitize_sql_array, sql_query(gem_name)) + dataset = ActiveRecord::Base.connection.execute(sanitize_sql) + deps = {} + dataset.each do |row| + key = DepKey.new( + row['name'], + row['number'], + row['platform'], + row['required_ruby_version'], + row['required_rubygems_version'], + row['sha256'], + row['created_at'] + ) + deps[key] = [] unless deps[key] + deps[key] << [row['dep_name'], row['requirements']] if row['dep_name'] + end + + deps.map do |dep_key, gem_deps| { - name: gem_name, - number: version.number, - platform: version.platform, - rubygems_version: version.required_rubygems_version, - ruby_version: version.required_ruby_version, - checksum: version.sha256, - created_at: version.created_at, - dependencies: version_deps.map { |d| [d.name, d.requirements] } + name: dep_key.name, + number: dep_key.number, + platform: dep_key.platform, + rubygems_version: dep_key.required_rubygems_version, + ruby_version: dep_key.required_ruby_version, + checksum: dep_key.sha256, + created_at: dep_key.created_at, + dependencies: gem_deps } end end statsd_measure :fetch_dependency_from_db, 'gem_dependent.fetch_dependency_from_db' + def sql_query(gem_name) + ["SELECT rv.name, rv.number, rv.platform, rv.required_ruby_version, rv.sha256, + rv.required_rubygems_version, d.requirements, rv.created_at, for_dep_name.name dep_name + FROM + (SELECT r.name, v.number, v.platform,v.required_rubygems_version, v.sha256, + v.required_ruby_version, v.created_at, v.id AS version_id + FROM rubygems AS r, versions AS v + WHERE v.rubygem_id = r.id + AND v.indexed is true AND r.name = ?) AS rv + LEFT JOIN dependencies AS d ON + d.version_id = rv.version_id + LEFT JOIN rubygems AS for_dep_name ON + d.rubygem_id = for_dep_name.id + AND d.scope = 'runtime' + ORDER BY rv.created_at, rv.number, rv.platform, for_dep_name.name", gem_name] + end + # Returns a Hash of the gem's cache key, and its cached dependencies def memcached_gem_info @memcached_gem_info ||= Rails.cache.read_multi(*@gem_information.values) diff --git a/test/functional/api/v1/dependencies_controller_test.rb b/test/functional/api/v1/dependencies_controller_test.rb index 4797e2120df..197388be879 100644 --- a/test/functional/api/v1/dependencies_controller_test.rb +++ b/test/functional/api/v1/dependencies_controller_test.rb @@ -51,7 +51,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24T00:00:00.000Z', + 'created_at' => '2016-05-24 00:00:00', 'dependencies' => [] }] @@ -78,23 +78,23 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase result = [ { 'name' => 'myrails', - 'number' => '3.0.0', + 'number' => '1.0.0', 'platform' => 'ruby', 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24T00:00:00.000Z', + 'created_at' => '2016-05-24 00:00:00', 'dependencies' => [] }, { 'name' => 'myrails', - 'number' => '1.0.0', + 'number' => '3.0.0', 'platform' => 'ruby', 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24T00:00:00.000Z', + 'created_at' => '2016-05-24 00:00:00', 'dependencies' => [] }, @@ -105,7 +105,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24T00:00:00.000Z', + 'created_at' => '2016-05-24 00:00:00', 'dependencies' => [] } ] @@ -173,7 +173,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase rubygems_version: '>= 2.6.3', ruby_version: '>= 2.0.0', checksum: 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - created_at: Date.new(2016, 05, 24), + created_at: "2016-05-24 00:00:00", dependencies: [] }] diff --git a/test/unit/gem_dependent_test.rb b/test/unit/gem_dependent_test.rb index 0cf98019770..b36241aaad4 100644 --- a/test/unit/gem_dependent_test.rb +++ b/test/unit/gem_dependent_test.rb @@ -22,12 +22,8 @@ class GemDependentTest < ActiveSupport::TestCase context "with gem_names" do setup do - @gem = create(:rubygem, name: "rack") - create(:version, number: "0.0.1", rubygem_id: @gem.id) - create(:version, number: "0.0.2", rubygem_id: @gem.id) - - @gem2 = create(:rubygem, name: "rack2") - create(:version, number: "0.0.1", created_at: Date.new(2016, 05, 24), rubygem_id: @gem2.id) + rack2 = create(:rubygem, name: "rack2") + create(:version, number: "0.0.1", created_at: Date.new(2016, 05, 24), rubygem: rack2) end should "return rack2" do @@ -38,7 +34,7 @@ class GemDependentTest < ActiveSupport::TestCase rubygems_version: ">= 2.6.3", ruby_version: ">= 2.0.0", checksum: "tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=", - created_at: Date.new(2016, 05, 24), + created_at: "2016-05-24 00:00:00", dependencies: [] } @@ -48,11 +44,72 @@ class GemDependentTest < ActiveSupport::TestCase end end - should "return all versions for a gem" do - result = %w(0.0.2 0.0.1) + context "multiple versions" do + setup do + rack = create(:rubygem, name: "rack") + create(:version, number: "0.2.2", created_at: Date.new(2016, 05, 24), rubygem: rack) + create(:version, number: "0.1.2", created_at: Date.new(2016, 05, 24), rubygem: rack) + create(:version, number: "0.1.2", created_at: Date.new(2016, 05, 24), platform: 'jruby', rubygem: rack) + create(:version, number: "0.1.3", created_at: Date.new(2016, 05, 25), rubygem: rack) + end + + should "orders versions by created_at, versions and platform" do + result = [["0.1.2", "jruby"], ["0.1.2", "ruby"], ["0.2.2", "ruby"], ["0.1.3", "ruby"]] + + deps = GemDependent.new(["rack"]).to_a + assert_equal result, deps.map { |x| [x[:number], x[:platform]] } + end + end + + context "has dependencies" do + setup do + devise = create(:rubygem, name: "devise") + version = create(:version, number: "1.0.0", rubygem: devise) + + %w(foo bar).map do |gem_name| + create(:rubygem, name: gem_name).tap do |rubygem| + gem_dependency = Gem::Dependency.new(rubygem.name, ['>= 0.0.0']) + create(:dependency, rubygem: rubygem, version: version, gem_dependency: gem_dependency) + end + end + end - deps = GemDependent.new(["rack"]).to_a - assert_equal result, deps.map { |x| x[:number] } + should "return dependencies ordered by name" do + result = { + name: 'devise', + number: '1.0.0', + dependencies: [['bar', '>= 0.0.0'], ['foo', '>= 0.0.0']] + } + + dep = GemDependent.new(["devise"]).to_a.first + result.each_pair do |k, v| + assert_equal v, dep[k] + end + end + end + + context "non indexed versions" do + setup do + nokogiri = create(:rubygem, name: "nokogiri") + create(:version, number: "0.0.1", created_at: Date.new(2016, 05, 24), rubygem: nokogiri) + create(:version, number: "0.1.1", rubygem: nokogiri, indexed: false) + end + + should "filter non indexed version" do + result = { + name: "nokogiri", + number: "0.0.1", + platform: "ruby", + rubygems_version: ">= 2.6.3", + ruby_version: ">= 2.0.0", + checksum: "tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=", + created_at: "2016-05-24 00:00:00", + dependencies: [] + } + + deps = GemDependent.new(["nokogiri"]).to_a + assert_equal [result], deps + end end end From 08687e9fabcc000734b2d903185b5f597b890e29 Mon Sep 17 00:00:00 2001 From: sonalkr132 Date: Tue, 31 May 2016 20:37:42 +0530 Subject: [PATCH 2/2] Correct format of versions' created_at --- app/models/gem_dependent.rb | 2 +- test/functional/api/v1/dependencies_controller_test.rb | 10 +++++----- test/unit/gem_dependent_test.rb | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/gem_dependent.rb b/app/models/gem_dependent.rb index db785899bdd..432e168071b 100644 --- a/app/models/gem_dependent.rb +++ b/app/models/gem_dependent.rb @@ -62,7 +62,7 @@ def fetch_dependency_from_db(gem_name) rubygems_version: dep_key.required_rubygems_version, ruby_version: dep_key.required_ruby_version, checksum: dep_key.sha256, - created_at: dep_key.created_at, + created_at: Time.zone.parse(dep_key.created_at).strftime('%Y-%m-%d %H:%M:%S %z'), dependencies: gem_deps } end diff --git a/test/functional/api/v1/dependencies_controller_test.rb b/test/functional/api/v1/dependencies_controller_test.rb index 197388be879..447633f3db7 100644 --- a/test/functional/api/v1/dependencies_controller_test.rb +++ b/test/functional/api/v1/dependencies_controller_test.rb @@ -51,7 +51,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24 00:00:00', + 'created_at' => '2016-05-24 00:00:00 +0000', 'dependencies' => [] }] @@ -83,7 +83,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24 00:00:00', + 'created_at' => '2016-05-24 00:00:00 +0000', 'dependencies' => [] }, @@ -94,7 +94,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24 00:00:00', + 'created_at' => '2016-05-24 00:00:00 +0000', 'dependencies' => [] }, @@ -105,7 +105,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase 'rubygems_version' => '>= 2.6.3', 'ruby_version' => '>= 2.0.0', 'checksum' => 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - 'created_at' => '2016-05-24 00:00:00', + 'created_at' => '2016-05-24 00:00:00 +0000', 'dependencies' => [] } ] @@ -173,7 +173,7 @@ class Api::V1::DependenciesControllerTest < ActionController::TestCase rubygems_version: '>= 2.6.3', ruby_version: '>= 2.0.0', checksum: 'tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=', - created_at: "2016-05-24 00:00:00", + created_at: "2016-05-24 00:00:00 +0000", dependencies: [] }] diff --git a/test/unit/gem_dependent_test.rb b/test/unit/gem_dependent_test.rb index b36241aaad4..da490d803c6 100644 --- a/test/unit/gem_dependent_test.rb +++ b/test/unit/gem_dependent_test.rb @@ -34,7 +34,7 @@ class GemDependentTest < ActiveSupport::TestCase rubygems_version: ">= 2.6.3", ruby_version: ">= 2.0.0", checksum: "tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=", - created_at: "2016-05-24 00:00:00", + created_at: "2016-05-24 00:00:00 +0000", dependencies: [] } @@ -103,7 +103,7 @@ class GemDependentTest < ActiveSupport::TestCase rubygems_version: ">= 2.6.3", ruby_version: ">= 2.0.0", checksum: "tdQEXD9Gb6kf4sxqvnkjKhpXzfEE96JucW4KHieJ33g=", - created_at: "2016-05-24 00:00:00", + created_at: "2016-05-24 00:00:00 +0000", dependencies: [] }