Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy change: Permadelete on yank #923

Merged
merged 10 commits into from
Apr 20, 2015
12 changes: 12 additions & 0 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
class Api::BaseController < ApplicationController
skip_before_action :require_ssl

private

def find_rubygem_by_name
@url = params[:url]
@gem_name = params[:gem_name]
@rubygem = Rubygem.find_by_name(@gem_name)
if @rubygem.nil? && @gem_name != WebHook::GLOBAL_PATTERN
render :text => "This gem could not be found",
:status => :not_found
end
end
end
40 changes: 40 additions & 0 deletions app/controllers/api/v1/deletions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
class Api::V1::DeletionsController < Api::BaseController
skip_before_action :verify_authenticity_token, only: [:create, :destroy]

before_action :authenticate_with_api_key, only: [:create, :destroy]
before_action :verify_authenticated_user, only: [:create, :destroy]
before_action :find_rubygem_by_name, only: [:create, :destroy]
before_action :validate_gem_and_version, only: [:create]

def create
@deletion = current_user.deletions.build(version: @version)
if @deletion.save
StatsD.increment 'yank.success'
render text: "Successfully deleted gem: #{@version.to_title}"
else
StatsD.increment 'yank.failure'
render text: "The version #{params[:version]} has already been deleted.", status: :unprocessable_entity
end
end

def destroy
render text: "Unyanking of gems is no longer supported.", status: :gone
end

private

def validate_gem_and_version
if [email protected]?
render text: "This gem does not exist.", status: :not_found
elsif [email protected]_by?(current_user)
render text: "You do not have permission to delete this gem.", status: :forbidden
else
begin
slug = params[:platform].blank? ? params[:version] : "#{params[:version]}-#{params[:platform]}"
@version = Version.find_from_slug!(@rubygem, slug)
rescue ActiveRecord::RecordNotFound
render text: "The version #{params[:version]} does not exist.", status: :not_found
end
end
end
end
45 changes: 3 additions & 42 deletions app/controllers/api/v1/rubygems_controller.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
class Api::V1::RubygemsController < Api::BaseController
skip_before_action :verify_authenticity_token, :only => [:create, :yank, :unyank]
skip_before_action :verify_authenticity_token, :only => [:create]

before_action :authenticate_with_api_key, :only => [:index, :create, :yank, :unyank]
before_action :verify_authenticated_user, :only => [:index, :create, :yank, :unyank]
before_action :authenticate_with_api_key, :only => [:index, :create]
before_action :verify_authenticated_user, :only => [:index, :create]
before_action :find_rubygem, :only => [:show]
before_action :find_rubygem_by_name, :only => [:yank, :unyank]
before_action :validate_gem_and_version, :only => [:yank, :unyank]

def index
@rubygems = current_user.rubygems.with_versions
Expand All @@ -32,26 +30,6 @@ def create
render :text => gemcutter.message, :status => gemcutter.code
end

def yank
if @version.indexed?
@version.yank!
StatsD.increment 'yank.success'
render :text => "Successfully yanked gem: #{@version.to_title}"
else
StatsD.increment 'yank.failure'
render :text => "The version #{params[:version]} has already been yanked.", :status => :unprocessable_entity
end
end

def unyank
if [email protected]?
@version.unyank!
render :text => "Successfully unyanked gem: #{@version.to_title}"
else
render :text => "The version #{params[:version]} is already indexed.", :status => :unprocessable_entity
end
end

def reverse_dependencies
names = Rubygem.reverse_dependencies(params[:id]).pluck(:name)

Expand All @@ -60,21 +38,4 @@ def reverse_dependencies
format.yaml { render yaml: names, yamlish: true }
end
end

private

def validate_gem_and_version
if [email protected]?
render :text => "This gem does not exist.", :status => :not_found
elsif [email protected]_by?(current_user)
render :text => "You do not have permission to yank this gem.", :status => :forbidden
else
begin
slug = params[:platform].blank? ? params[:version] : "#{params[:version]}-#{params[:platform]}"
@version = Version.find_from_slug!(@rubygem, slug)
rescue ActiveRecord::RecordNotFound
render :text => "The version #{params[:version]} does not exist.", :status => :not_found
end
end
end
end
10 changes: 0 additions & 10 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,6 @@ def find_rubygem
end
end

def find_rubygem_by_name
@url = params[:url]
@gem_name = params[:gem_name]
@rubygem = Rubygem.find_by_name(@gem_name)
if @rubygem.nil? && @gem_name != WebHook::GLOBAL_PATTERN
render :text => "This gem could not be found",
:status => :not_found
end
end

def set_page
@page = [1, params[:page].to_i].max
end
Expand Down
39 changes: 39 additions & 0 deletions app/models/deletion.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
class Deletion < ActiveRecord::Base
belongs_to :user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log the username instead of the user_id. In case we delete the user, we still want to keep the Deletion entry.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Username can change. There's not a dependent relationship on has_many :deletions so these records will stay in the DB.


validates :user, :rubygem, :number, presence: true
validates :version, presence: true
validate :version_is_indexed

before_validation :record_metadata
after_create :remove_from_index
after_commit :remove_from_storage

attr_accessor :version

private

def version_is_indexed
errors.add(:number, "is already deleted") unless @version.indexed?
end

def rubygem_name
@version.rubygem.name
end

def record_metadata
self.rubygem = rubygem_name
self.number = @version.number
self.platform = @version.platform
end

def remove_from_index
@version.update!(indexed: false)
Redis.current.lrem(Rubygem.versions_key(rubygem_name), 1, @version.full_name)
end

def remove_from_storage
RubygemFs.instance.remove("gems/#{@version.full_name}.gem")
RubygemFs.instance.remove("quick/Marshal.4.8/#{@version.full_name}.gemspec.rz")
end
end
2 changes: 1 addition & 1 deletion app/models/pusher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def find

if @rubygem.name != name and @rubygem.indexed_versions?
return notify("Unable to change case of gem name with indexed versions\n" +
"Please yank all versions first", 409)
"Please delete all versions first with `gem yank`.", 409)
end
end

Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class User < ActiveRecord::Base
:through => :subscriptions,
:source => :rubygem

has_many :deletions
has_many :ownerships
has_many :subscriptions
has_many :web_hooks
Expand Down
10 changes: 0 additions & 10 deletions app/models/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,6 @@ def reorder_versions
rubygem.reorder_versions
end

def yank!
update_attributes!(:indexed => false)
Redis.current.lrem(Rubygem.versions_key(rubygem.name), 1, full_name)
end

def unyank!
update_attributes!(:indexed => true)
push
end

def push
Redis.current.lpush(Rubygem.versions_key(rubygem.name), full_name)
end
Expand Down
5 changes: 3 additions & 2 deletions app/views/rubygems/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
</div>
<% else %>
<div class="t-body">
<p><%= t('.yanked_notice', :more_info_link => link_to(t('.yank_info_link_text'), "http://help.rubygems.org/faqs/gemcutter/removing-a-published-rubygem")).html_safe %></p>
<p><%=t '.yanked_notice' %></p>
</div>
<% end %>

Expand Down Expand Up @@ -158,9 +158,10 @@
<%= link_to_page t('.links.mail'), @rubygem.linkset.mail %>
<%= link_to_page t('.links.bugs'), @rubygem.linkset.bugs %>
<% end %>

<%= download_link(@latest_version) %>
<% end %>

<%= download_link(@latest_version) %>
<%= documentation_link(@latest_version, @rubygem.linkset) %>
<%= badge_link(@rubygem) %>
<%= subscribe_link(@rubygem) if @latest_version.indexed %>
Expand Down
1 change: 1 addition & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def config_for(name, env = Rails.env)
config.middleware.use "Redirector"

config.active_record.include_root_in_json = false
config.active_record.raise_in_transactional_callbacks = true

config.after_initialize do
RubygemFs.s3! ENV['S3_PROXY'] if ENV['S3_PROXY']
Expand Down
3 changes: 1 addition & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ en:
show:
not_hosted_notice: This gem is not currently hosted on Gemcutter.
install: install
yanked_notice: "This gem has been yanked, but it is still available for download for other gems that may have depended on it. For more information or if you need to remove it permanently, %{more_info_link}."
yank_info_link_text: please click here
yanked_notice: "This gem has been yanked, and it is not available for download directly or for other gems that may have depended on it."
authors_header: Authors
downloads_for_this_version: "For this version"
owners_header: Owners
Expand Down
2 changes: 0 additions & 2 deletions config/locales/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ nl:
authors_header: Auteurs
versions_header: Versies
not_hosted_notice: Deze gem wordt momenteel niet gehost op rubygems.org.
yanked_notice: Deze gem is verwijderd, maar is nog steeds te downloaden voor andere gems die ervan afhankelijk zijn. Voor meer informatie of als het nodig is de gem permanent te verwijderen, zie %{more_info_link}.
yank_info_link_text: gebruik deze link
owners_header: Eigenaren
links:
home: Startpagina
Expand Down
4 changes: 2 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
get :reverse_dependencies
end
collection do
delete :yank
put :unyank
delete :yank, to: "deletions#create"
put :unyank, to: "deletions#destroy"
end
constraints :rubygem_id => Patterns::ROUTE_PATTERN do
resource :owners, :only => [:show, :create, :destroy]
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20150407012331_create_deletions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreateDeletions < ActiveRecord::Migration
def change
create_table :deletions do |t|
t.belongs_to :user, index: true
t.string :rubygem
t.string :number
t.string :platform

t.timestamps null: false
end
end
end
Loading