Skip to content

Commit

Permalink
Merge pull request #2810 from varshavaradarajan/config-api-create
Browse files Browse the repository at this point in the history 
Pipeline Config API: Check for existing pipeline  name while creating.
  • Loading branch information
@ketan
ketan committed Oct 26, 2016
2 parents fddc9fe + d159f5b commit 6c7200e
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 104 deletions.
4 changes: 2 additions & 2 deletions server/webapp/WEB-INF/rails.new/app/controllers/api_v1/admin/pipelines_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ def load_pipeline(pipeline_name = params[:pipeline_name])
end

def check_if_pipeline_by_same_name_already_exists
if (!pipeline_config_service.getPipelineConfig(params[:pipeline_name]).nil?)
if (!pipeline_config_service.getPipelineConfig(params[:pipeline][:name]).nil?)
result = HttpLocalizedOperationResult.new
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline_name]))
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline][:name]))
render_http_operation_result(result)
end
end
Expand Down
4 changes: 2 additions & 2 deletions server/webapp/WEB-INF/rails.new/app/controllers/api_v2/admin/pipelines_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ def load_pipeline(pipeline_name = params[:pipeline_name])
end

def check_if_pipeline_by_same_name_already_exists
if (!pipeline_config_service.getPipelineConfig(params[:pipeline_name]).nil?)
if (!pipeline_config_service.getPipelineConfig(params[:pipeline][:name]).nil?)
result = HttpLocalizedOperationResult.new
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline_name]))
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline][:name]))
render_http_operation_result(result)
end
end
Expand Down
4 changes: 2 additions & 2 deletions server/webapp/WEB-INF/rails.new/app/controllers/api_v3/admin/pipelines_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ def load_pipeline(pipeline_name = params[:pipeline_name])
end

def check_if_pipeline_by_same_name_already_exists
if (!pipeline_config_service.getPipelineConfig(params[:pipeline_name]).nil?)
if (!pipeline_config_service.getPipelineConfig(params[:pipeline][:name]).nil?)
result = HttpLocalizedOperationResult.new
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline_name]))
result.unprocessableEntity(LocalizedMessage::string("CANNOT_CREATE_PIPELINE_ALREADY_EXISTS", params[:pipeline][:name]))
render_http_operation_result(result)
end
end
Expand Down
60 changes: 27 additions & 33 deletions server/webapp/WEB-INF/rails.new/spec/controllers/api_v1/admin/pipelines_controller_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,22 @@
end

describe :create do
before :each do
controller.stub(:check_if_pipeline_by_same_name_already_exists).and_return(nil)
end
it 'should allow anyone, with security disabled' do
disable_security
@pipeline_config_service.should_receive(:getPipelineConfig).with(anything()).and_return(nil)
expect(controller).to allow_action(:post, :create, :group => @group)
end

it 'should disallow non-admin user, with security enabled' do
enable_security
login_as_pipeline_group_Non_Admin_user
expect(controller).to disallow_action(:post, :create, :pipeline => {:pipeline_name => "pipeline1"}, :group => @group).with(401, "You are not authorized to perform this action.")
expect(controller).to disallow_action(:post, :create, :pipeline => {:name => "pipeline1"}, :group => @group).with(401, "You are not authorized to perform this action.")
end

it 'should allow admin users, with security enabled' do
login_as_pipeline_group_admin_user(@group)
@pipeline_config_service.should_receive(:getPipelineConfig).with(anything()).and_return(nil)
expect(controller).to allow_action(:post, :create, :group => @group)
end
end
Expand Down Expand Up @@ -149,16 +150,16 @@
before :each do
enable_security
@security_service.stub(:hasViewPermissionForPipeline).and_return(true)
@pipeline_name = 'pipeline1'
end

describe :show do

it "should not show pipeline config for Non Admin users" do
login_as_pipeline_group_Non_Admin_user
pipeline_name = "pipeline1"
pipeline = PipelineConfigMother.pipelineConfig(pipeline_name)
pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)

get_with_api_header :show, :pipeline_name => pipeline_name
get_with_api_header :show, :pipeline_name => @pipeline_name

expect(response.code).to eq("401")
json = JSON.parse(response.body).deep_symbolize_keys
Expand All @@ -167,14 +168,13 @@

it 'should show pipeline config for an admin' do
login_as_pipeline_group_admin_user(@group)
pipeline_name = 'pipeline1'
pipeline = PipelineConfigMother.pipelineConfig(pipeline_name)
pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
pipeline_md5 = 'md5_for_pipeline_config'

@pipeline_config_service.should_receive(:getPipelineConfig).with(pipeline_name).and_return(pipeline)
@pipeline_config_service.should_receive(:getPipelineConfig).with(@pipeline_name).and_return(pipeline)
@entity_hashing_service.should_receive(:md5ForEntity).with(pipeline).and_return(pipeline_md5)

get_with_api_header :show, :pipeline_name => pipeline_name
get_with_api_header :show, :pipeline_name => @pipeline_name

expect(response).to be_ok
expected_response = expected_response(pipeline, ApiV1::Config::PipelineConfigRepresenter)
Expand All @@ -184,27 +184,25 @@

it "should return 304 for show pipeline config if etag sent in request is fresh" do
login_as_pipeline_group_admin_user(@group)
pipeline_name = "pipeline1"
pipeline = PipelineConfigMother.pipelineConfig(pipeline_name)
pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
pipeline_md5 = 'md5_for_pipeline_config'

@pipeline_config_service.should_receive(:getPipelineConfig).with(pipeline_name).and_return(pipeline)
@pipeline_config_service.should_receive(:getPipelineConfig).with(@pipeline_name).and_return(pipeline)
@entity_hashing_service.should_receive(:md5ForEntity).with(pipeline).and_return(pipeline_md5)

controller.request.env['HTTP_IF_NONE_MATCH'] = Digest::MD5.hexdigest(pipeline_md5)

get_with_api_header :show, { :pipeline_name => pipeline_name }
get_with_api_header :show, { :pipeline_name => @pipeline_name }

expect(response.code).to eq('304')
expect(response.body).to be_empty
end

it "should return 404 for show pipeline config if pipeline is not found" do
login_as_pipeline_group_admin_user(@group)
pipeline_name = "pipeline1"
@pipeline_config_service.should_receive(:getPipelineConfig).with(pipeline_name).and_return(nil)
@pipeline_config_service.should_receive(:getPipelineConfig).with(@pipeline_name).and_return(nil)

get_with_api_header :show, :pipeline_name => pipeline_name
get_with_api_header :show, :pipeline_name => @pipeline_name

expect(response.code).to eq("404")
json = JSON.parse(response.body).deep_symbolize_keys
Expand All @@ -213,16 +211,15 @@

it "should show pipeline config if etag sent in request is stale" do
login_as_pipeline_group_admin_user(@group)
pipeline_name = "pipeline1"
pipeline = PipelineConfigMother.pipelineConfig(pipeline_name)
pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
pipeline_md5 = 'md5_for_pipeline_config'

@pipeline_config_service.should_receive(:getPipelineConfig).with(pipeline_name).and_return(pipeline)
@pipeline_config_service.should_receive(:getPipelineConfig).with(@pipeline_name).and_return(pipeline)
@entity_hashing_service.should_receive(:md5ForEntity).with(pipeline).and_return(pipeline_md5)

controller.request.env['HTTP_IF_NONE_MATCH'] = 'stale-etag'

get_with_api_header :show, { pipeline_name: pipeline_name }
get_with_api_header :show, { pipeline_name: @pipeline_name }
expect(response).to be_ok
expect(response.body).to_not be_empty
end
Expand Down Expand Up @@ -267,7 +264,6 @@
describe :update do
before(:each) do
login_as_pipeline_group_admin_user(@group)
@pipeline_name = "pipeline1"
@pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
controller.send(:go_cache).put("GO_PIPELINE_CONFIGS_ETAGS_CACHE", @pipeline_name, "latest-etag")
end
Expand Down Expand Up @@ -419,13 +415,12 @@

describe :create do
before(:each) do
@pipeline_name = "pipeline1"
@pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
end

it "should not allow non admin users to create a new pipeline config" do
login_as_pipeline_group_Non_Admin_user
post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => "new_grp"
post_with_api_header :create, :pipeline => pipeline, :group => "new_grp"

expect(response.code).to eq("401")

Expand All @@ -440,7 +435,7 @@
@security_service.stub(:isUserAdminOfGroup).and_return(false) # pipeline group admin
@security_service.stub(:isUserAdmin).and_return(false) # not an admin

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => "another_group"
post_with_api_header :create, :pipeline => pipeline, :group => "another_group"

expect(response.code).to eq("401")

Expand All @@ -459,7 +454,7 @@
@pipeline_config_service.should_receive(:createPipelineConfig).with(anything(), anything(), anything(), "new_grp")
expect(@pipeline_pause_service).to receive(:pause).with("pipeline1", "Under construction", @user)

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => "new_grp"
post_with_api_header :create, :pipeline => pipeline, :group => "new_grp"

expect(response).to be_ok
expect(actual_response).to eq(expected_response(@pipeline, ApiV1::Config::PipelineConfigRepresenter))
Expand All @@ -472,7 +467,7 @@
@pipeline_config_service.should_receive(:createPipelineConfig).with(anything(), anything(), anything(), "new_grp")
expect(@pipeline_pause_service).to receive(:pause).with("pipeline1", "Under construction", @user)

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => "new_grp"
post_with_api_header :create, :pipeline => pipeline, :group => "new_grp"

expect(response).to be_ok
expect(actual_response).to eq(expected_response(@pipeline, ApiV1::Config::PipelineConfigRepresenter))
Expand All @@ -495,7 +490,7 @@
@pipeline_config_service.should_receive(:createPipelineConfig).with(anything(), anything(), result, "group")
controller.request.env['HTTP_IF_MATCH'] = "\"#{Digest::MD5.hexdigest("latest-etag")}\""

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => invalid_pipeline, :group => "group"
post_with_api_header :create, :pipeline => invalid_pipeline, :group => "group"

expect(response.code).to eq("406")
json = JSON.parse(response.body).deep_symbolize_keys
Expand All @@ -512,7 +507,7 @@
@pipeline_config_service.should_receive(:getPipelineConfig).with(@pipeline_name).and_return(@pipeline)
@pipeline_config_service.should_not_receive(:createPipelineConfig).with(anything(), anything(), anything(), "new_grp")

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => "new_grp"
post_with_api_header :create, :pipeline => pipeline, :group => "new_grp"

expect(response.code).to eq("422")

Expand All @@ -525,7 +520,7 @@
@security_service.stub(:isUserAdmin).and_return(true)
@pipeline_config_service.stub(:getPipelineConfig).and_return(nil)

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline, :group => ""
post_with_api_header :create, :pipeline => pipeline, :group => ""

expect(response.code).to eq("422")

Expand All @@ -544,7 +539,7 @@
pipeline_being_saved = pipeline
end

post_with_api_header :create, pipeline_name: @pipeline_name, :pipeline => pipeline_with_pluggable_material("pipeline1", "package", "package-name"), :group => "group"
post_with_api_header :create, :pipeline => pipeline_with_pluggable_material("pipeline1", "package", "package-name"), :group => "group"
expect(response).to be_ok
expect(pipeline_being_saved.materialConfigs().first().getPackageDefinition()).to eq(@repo.findPackage("package-name"))
end
Expand All @@ -560,7 +555,7 @@
pipeline_being_saved = pipeline
end

post_with_api_header :create, :pipeline => pipeline_with_pluggable_material("pipeline1", "plugin", "scm-id"), :group => "group", :pipeline_name => @pipeline_name
post_with_api_header :create, :pipeline => pipeline_with_pluggable_material("pipeline1", "plugin", "scm-id"), :group => "group"
expect(response).to be_ok
expect(pipeline_being_saved.materialConfigs().first().getSCMConfig()).to eq(@scm)
end
Expand Down Expand Up @@ -589,7 +584,6 @@
describe :destroy do
before(:each) do
login_as_admin
@pipeline_name = "pipeline1"
@pipeline = PipelineConfigMother.pipelineConfig(@pipeline_name)
@pipeline_config_service.stub(:getPipelineConfig).with(@pipeline_name).and_return(@pipeline)
@security_service.stub(:isUserAdminOfGroup).and_return(true)
Expand Down

0 comments on commit 6c7200e

Please sign in to comment.