kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Creates specs for destroy service and improves namespace container image query performance

This commit is contained in:
Andre Guedes 2017-02-22 11:19:23 -03:00
parent 8294756fc1
commit db5b4b8b1a
8 changed files with 45 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/models/namespace.rb
View File

@ -155,7 +155,7 @@ class Namespace < ActiveRecord::Base
end
def any_project_has_container_registry_images?
projects.any? { |project| project.container_images.present? }
projects.joins(:container_images).any?
end
def send_update_instructions

26
app/services/container_images/destroy_service.rb
View File

@ -1,31 +1,9 @@
module ContainerImages
class DestroyService < BaseService
class DestroyError < StandardError; end
def execute(container_image)
@container_image = container_image
return false unless can?(current_user, :update_container_image, project)
return false unless can?(current_user, :remove_project, project)
ContainerImage.transaction do
container_image.destroy!
unless remove_container_image_tags
raise_error('Failed to remove container image tags. Please try again or contact administrator')
end
end
true
end
private
def raise_error(message)
raise DestroyError.new(message)
end
def remove_container_image_tags
container_image.delete_tags
container_image.destroy!
end
end
end

2
app/views/admin/container_registry/show.html.haml
View File

@ -17,7 +17,7 @@
X-Registry-Token: [#{@access_token}]
%br
Access token is
%code{ id: 'registry-token' } #{@access_token}
%code{ id: 'registry-token' }= @access_token
.bs-callout.clearfix
.pull-left

16
db/migrate/20161031013926_create_container_image.rb
View File

@ -7,22 +7,6 @@ class CreateContainerImage < ActiveRecord::Migration
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
create_table :container_images do |t|
t.integer :project_id

16
db/migrate/20161213212947_add_container_registry_access_token_to_application_settings.rb
View File

@ -7,22 +7,6 @@ class AddContainerRegistryAccessTokenToApplicationSettings < ActiveRecord::Migra
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
add_column :application_settings, :container_registry_access_token, :string
end

10
db/schema.rb
View File

@ -61,6 +61,7 @@ ActiveRecord::Schema.define(version: 20170215200045) do
t.boolean "shared_runners_enabled", default: true, null: false
t.integer "max_artifacts_size", default: 100, null: false
t.string "runners_registration_token"
t.integer "max_pages_size", default: 100, null: false
t.boolean "require_two_factor_authentication", default: false
t.integer "two_factor_grace_period", default: 48
t.boolean "metrics_enabled", default: false
@ -107,10 +108,9 @@ ActiveRecord::Schema.define(version: 20170215200045) do
t.string "sidekiq_throttling_queues"
t.decimal "sidekiq_throttling_factor"
t.boolean "html_emails_enabled", default: true
t.string "container_registry_access_token"
t.string "plantuml_url"
t.boolean "plantuml_enabled"
t.string "container_registry_access_token"
t.integer "max_pages_size", default: 100, null: false
t.integer "terminal_max_session_time", default: 0, null: false
end
@ -586,9 +586,9 @@ ActiveRecord::Schema.define(version: 20170215200045) do
end
add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree
add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
create_table "lfs_objects", force: :cascade do |t|
t.string "oid", null: false
@ -761,8 +761,8 @@ ActiveRecord::Schema.define(version: 20170215200045) do
t.integer "visibility_level", default: 20, null: false
t.boolean "request_access_enabled", default: false, null: false
t.datetime "deleted_at"
t.boolean "lfs_enabled"
t.text "description_html"
t.boolean "lfs_enabled"
t.integer "parent_id"
end
@ -1283,8 +1283,8 @@ ActiveRecord::Schema.define(version: 20170215200045) do
t.datetime "otp_grace_period_started_at"
t.boolean "ldap_email", default: false, null: false
t.boolean "external", default: false
t.string "organization"
t.string "incoming_email_token"
t.string "organization"
t.boolean "authorized_projects_populated"
t.boolean "notified_of_own_activity", default: false, null: false
end

4
lib/api/registry_events.rb
View File

@ -39,9 +39,9 @@ module API
params['events'].each do |event|
repository = event['target']['repository']
if event['action'] == 'push' and !!event['target']['tag']
if event['action'] == 'push' && !!event['target']['tag']
namespace, container_image_name = ContainerImage::split_namespace(repository)
project = Project::find_with_namespace(namespace)
project = Project::find_by_full_path(namespace)
if project
container_image = project.container_images.find_or_create_by(name: container_image_name)

34
spec/services/container_images/destroy_service_spec.rb Normal file
View File

@ -0,0 +1,34 @@
require 'spec_helper'
describe ContainerImages::DestroyService, services: true do
describe '#execute' do
let(:user) { create(:user) }
let(:container_image) { create(:container_image, name: '') }
let(:project) { create(:project, path: 'test', namespace: user.namespace, container_images: [container_image]) }
let(:example_host) { 'example.com' }
let(:registry_url) { 'http://' + example_host }
it { expect(container_image).to be_valid }
it { expect(project.container_images).not_to be_empty }
context 'when container image has tags' do
before do
project.team << [user, :master]
end
it 'removes all tags before destroy' do
service = described_class.new(project, user)
expect(container_image).to receive(:delete_tags).and_return(true)
expect { service.execute(container_image) }.to change(project.container_images, :count).by(-1)
end
it 'fails when tags are not removed' do
service = described_class.new(project, user)
expect(container_image).to receive(:delete_tags).and_return(false)
expect { service.execute(container_image) }.to raise_error(ActiveRecord::RecordNotDestroyed)
end
end
end
end