Tracking downloads properly for platform gems, required refactoring f…

…or how versions are looked up and handled

app/controllers/versions_controller.rb

@@ -6,7 +6,7 @@ def index
   end
 
   def show
-    @latest_version = @rubygem.versions.find_by_number(params[:id])
+    @latest_version = Version.find_from_slug!(@rubygem.id, params[:id])
     render "rubygems/show"
   end
 

app/models/download.rb

@@ -4,10 +4,10 @@ class Download < ActiveRecord::Base
   belongs_to :version, :counter_cache => true
 
   def perform
-    rubygem_name, version_number = self.raw.split('-')
+    rubygem_name, version_number, platform = self.raw.split('-')
 
     rubygem = Rubygem.find_by_name(rubygem_name)
-    version = rubygem.versions.find_by_number(version_number)
+    version = rubygem.versions.find_by_number_and_platform(version_number, platform || "ruby")
 
     Download.transaction do
       self.version = version

app/models/version.rb

@@ -43,6 +43,13 @@ def self.published(limit=5)
     created_at_before(DateTime.now.utc).by_created_at(:desc).limited(limit)
   end
 
+  def self.find_from_slug!(rubygem_id, slug)
+    number, *raw_platform = slug.split('-')
+    platform = raw_platform.blank? ? "ruby" : raw_platform.join('-')
+
+    find_by_rubygem_id_and_number_and_platform!(rubygem_id, number, platform)
+  end
+
   def to_s
     number
   end
@@ -51,6 +58,16 @@ def to_title
     "#{rubygem.name} (#{to_s})"
   end
 
+  def to_slug
+    param = number.dup
+    param << "-#{platform}" if platformed?
+    param
+  end
+
+  def platformed?
+    platform != "ruby"
+  end
+
   def update_prerelease
     self[:prerelease] = to_gem_version.prerelease?
     true

app/views/rubygems/show.html.erb

@@ -1,5 +1,5 @@
 <% @title = @rubygem.name %>
-<% @subtitle = @latest_version %>
+<% @subtitle = @latest_version.try(:to_slug) %>
 
 <% if @rubygem.versions_count.zero? %>
   <p>

app/views/versions/_version.html.erb

@@ -1,5 +1,5 @@
 <li>
-  <%= link_to version.number, rubygem_version_url(version.rubygem, version.number) %>
+  <%= link_to version.number, rubygem_version_url(version.rubygem, version.to_slug) %>
   <small><%= version.built_at_date %></small>
   <% if version.platform != 'ruby' %>
     <span class="platform"><%= version.platform %></span>

config/routes.rb

@@ -27,7 +27,7 @@
 
     rubygems.resources :versions,
       :only         => [:index, :show],
-      :requirements => { :rubygem_id => RUBYGEM_NAME_MATCHER, :id => /#{Gem::Version::VERSION_PATTERN}/ }
+      :requirements => { :rubygem_id => RUBYGEM_NAME_MATCHER, :id => RUBYGEM_NAME_MATCHER }
   end
 
   map.search "/search", :controller => "searches", :action => "new"

features/download.feature

@@ -17,7 +17,6 @@ Feature: Download Gems
 
     When I download the rubygem "sandworm" version "2.0.0" 3 times
     And the system processes jobs
-    And I go to the homepage
     And I visit the gem page for "sandworm"
     Then I should see "3 total downloads"
     And I should see "3 version downloads"
@@ -30,3 +29,28 @@ Feature: Download Gems
     When I follow "1.0.0"
     Then I should see "5 total downloads"
     And I should see "2 version downloads"
+
+  Scenario: Download a platform gem
+    Given I am signed up and confirmed as "email@person.com/password"
+    And I have a gem "crysknife" with version "1.0.0"
+    And I have a gem "crysknife" with version "1.0.0" and platform "java"
+    And I have an api key for "email@person.com/password"
+    And I push the gem "crysknife-1.0.0.gem" with my api key
+    And I push the gem "crysknife-1.0.0-java.gem" with my api key
+    And the system processes jobs
+
+    When I visit the gem page for "crysknife" version "1.0.0"
+    Then I should see "0 total downloads"
+
+    When I download the rubygem "crysknife" version "1.0.0" 3 times
+    And the system processes jobs
+    And I visit the gem page for "crysknife" version "1.0.0"
+    And I save and open the page
+    Then I should see "3 total downloads"
+    And I should see "3 version downloads"
+
+    When I download the rubygem "crysknife" version "1.0.0-java" 2 times
+    And the system processes jobs
+    And I visit the gem page for "crysknife" version "1.0.0-java"
+    Then I should see "5 total downloads"
+    And I should see "2 version downloads"

features/step_definitions/gem_steps.rb

@@ -6,20 +6,25 @@
   build_gem(name, version, summary)
 end
 
+Given /^I have a gem "([^\"]*)" with version "([^\"]*)" and platform "([^\"]*)"$/ do |name, version, platform|
+  build_gem(name, version, "Gemcutter", platform)
+end
+
 Given /^a rubygem exists with name "([^\"]*)" and rubyforge project "([^\"]*)"$/ do |name, rubyforge_project|
   rubygem = Factory(:rubygem, :name => name)
   Factory(:version, :rubygem => rubygem, :rubyforge_project => rubyforge_project)
 end
 
-def build_gem(name, version, summary = "Gemcutter")
-  builder = Gem::Builder.new(build_gemspec(name, version, summary))
+def build_gem(name, version, summary = "Gemcutter", platform = "ruby")
+  builder = Gem::Builder.new(build_gemspec(name, version, summary, platform))
   builder.ui = Gem::SilentUI.new
   builder.build
 end
 
-def build_gemspec(name, version, summary)
+def build_gemspec(name, version, summary, platform)
   Gem::Specification.new do |s|
     s.name = "#{name}"
+    s.platform = "#{platform}"
     s.version = "#{version}"
     s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
     s.authors = ["John Doe"]

features/step_definitions/view_steps.rb

@@ -3,6 +3,11 @@
   visit rubygem_path(rubygem)
 end
 
+When /^I visit the gem page for "([^\"]*)" version "([^\"]*)"$/ do |gem_name, version_number|
+  rubygem = Rubygem.find_by_name!(gem_name)
+  visit rubygem_version_path(rubygem, version_number)
+end
+
 And /^I save and open the page$/ do
   save_and_open_page
 end

test/factories/version.rb

@@ -1,10 +1,11 @@
 Factory.define :version do |version|
-  version.authors     { 'Joe User' }
-  version.description { 'Some awesome gem' }
-  version.number      { Factory.next(:version_number) }
-  version.built_at    { 1.day.ago }
+  version.authors           { 'Joe User' }
+  version.description       { 'Some awesome gem' }
+  version.number            { Factory.next(:version_number) }
+  version.built_at          { 1.day.ago }
+  version.platform          { "ruby" }
   version.rubyforge_project { 'awesome' }
-  version.association :rubygem
+  version.association       :rubygem
 end
 
 Factory.sequence :version_number do |n|

test/unit/download_test.rb

@@ -11,10 +11,27 @@ class DownloadTest < ActiveSupport::TestCase
     rubygem = Factory(:rubygem)
     version = Factory(:version, :rubygem => rubygem)
 
-    raw_download = Download.new(:raw => "#{rubygem.name}-#{version.number}")
+    3.times do
+      raw_download = Download.new(:raw => "#{rubygem.name}-#{version.number}")
+      raw_download.perform
+      assert_equal raw_download.reload.version, version
+    end
+
+    assert_equal 3, version.reload.downloads_count
+    assert_equal 3, rubygem.reload.downloads
+  end
+
+  should "track platform gem downloads correctly" do
+    rubygem = Factory(:rubygem)
+    version = Factory(:version, :rubygem => rubygem, :platform => "java")
+    other_platform_version = Factory(:version, :rubygem => rubygem, :platform => "mswin32")
+
+    raw_download = Download.new(:raw => "#{rubygem.name}-#{version.number}-java")
     raw_download.perform
 
     assert_equal raw_download.reload.version, version
     assert_equal 1, version.reload.downloads_count
+    assert_equal 1, rubygem.reload.downloads
+    assert_equal 0, other_platform_version.reload.downloads_count
   end
 end

test/unit/version_test.rb

@@ -39,6 +39,29 @@ class VersionTest < ActiveSupport::TestCase
       assert_equal @version.number, @version.to_s
     end
 
+    should "return just number for to_slug if platform is ruby" do
+      assert ! @version.platformed?
+      assert_equal @version.number, @version.to_slug
+      assert_equal @version, Version.find_from_slug!(@version.rubygem_id, @version.number)
+    end
+
+    should "raise an ActiveRecord::RecordNotFound if an invalid slug is given" do
+      assert_raise ActiveRecord::RecordNotFound do
+        Version.find_from_slug!(@version.rubygem_id, "some stupid version 399")
+      end
+    end
+
+    %w[x86_64-linux java mswin x86-mswin32-60].each do |platform|
+      should "be able to deal with platform of #{platform}" do
+        @version.update_attribute(:platform, platform)
+        slug = "#{@version.number}-#{platform}"
+
+        assert @version.platformed?
+        assert_equal slug, @version.to_slug
+        assert_equal @version, Version.find_from_slug!(@version.rubygem_id, slug)
+      end
+    end
+
     should "have a default download count" do
       assert @version.downloads_count.zero?
     end