Skip to content

Commit

Permalink
[api][webui] Fix architecture validation
Browse files Browse the repository at this point in the history
DownloadRepository architecture has to be within the valid architectures
and the architectures that the associated repositories have. Otherwise the
backend validation would fail.
  • Loading branch information
bgeuken committed Apr 13, 2016
1 parent ee58bcf commit cdeee3d
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ def create

begin
ActiveRecord::Base.transaction do
RepositoryArchitecture.first_or_create!(
repository: @download_on_demand.repository,
architecture: Architecture.find_by_name(permitted_params[:arch])
)
@download_on_demand.save!
@project.store
end
Expand All @@ -24,6 +28,7 @@ def update

begin
ActiveRecord::Base.transaction do
# FIXME: Handle changing architecture or permit it
@download_on_demand.update_attributes!(permitted_params)
@project.store
end
Expand All @@ -41,6 +46,7 @@ def destroy

begin
ActiveRecord::Base.transaction do
# FIXME: should remove repo arch?
@download_on_demand.destroy!
@project.store
end
Expand Down
10 changes: 9 additions & 1 deletion src/api/app/models/download_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@ class DownloadRepository < ActiveRecord::Base

validates :repository_id, presence: true
validates :arch, uniqueness: { scope: :repository_id }, presence: true
validates :arch, inclusion: { in: Architecture.all.pluck(:name) }
validate :architecture_inclusion
validates :url, presence: true
validates :repotype, presence: true
validates :repotype, inclusion: { in: REPOTYPES }

delegate :to_s, to: :id

def architecture_inclusion
# Workaround for rspec validation test (validate_presence_of(:repository_id))
if self.repository
unless self.repository.architectures.pluck(:name).include?(self.arch)
errors.add(:base, "Architecture has to be available via repository association.")
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
end

it { is_expected.to redirect_to(root_path) }
it { expect(flash[:error]).to eq("Download on Demand can't be created: Validation failed: Arch can't be blank, Arch is not included in the list") }
it { expect(flash[:error]).to eq("Download on Demand can't be created: Validation failed: Architecture can't be blank") }
it { expect(assigns(:download_on_demand)).to be_kind_of(DownloadRepository) }
it { expect(DownloadRepository.where(dod_parameters[:download_repository])).not_to exist }
end
Expand Down Expand Up @@ -112,15 +112,11 @@
describe "POST update" do
let(:dod_repository) { create(:download_repository) }

before do
repository.download_repositories << dod_repository
end

context "for non-admin users" do
before do
login(create(:confirmed_user))
dod_parameters[:id] = dod_repository.id
dod_parameters[:download_repository][:arch] = "s390x"
dod_parameters[:download_repository][:url] = "http://opensuse.org"

post :update, dod_parameters
end
Expand All @@ -129,25 +125,23 @@
it { expect(flash[:error]).to eq("Sorry, you are not authorized to update this DownloadRepository.") }

it "updates the DownloadRepository" do
expect(dod_repository.arch).to eq("x86_64")
expect(dod_repository.url).to eq("http://suse.com")
end
end

context "valid requests" do
before do
login(admin_user)
dod_parameters[:id] = dod_repository.id
dod_parameters[:download_repository][:arch] = "s390x"
dod_parameters[:download_repository][:url] = "http://opensuse.org"
dod_parameters[:download_repository][:repository_id] = dod_repository.repository.id

post :update, dod_parameters
end

it { is_expected.to redirect_to(project_repositories_path(project)) }
it { expect(flash[:notice]).to eq("Successfully updated Download on Demand") }

it "updates the DownloadRepository" do
expect(dod_repository.reload.arch).to eq("s390x")
end
it { expect(dod_repository.reload.url).to eq("http://opensuse.org") }
end

context "invalid requests" do
Expand Down
7 changes: 7 additions & 0 deletions src/api/spec/factories/download_repository_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,12 @@
url "http://suse.com"
repotype "rpmmd"
repository

before(:create) do |download_repository|
RepositoryArchitecture.first_or_create!(
repository: download_repository.repository,
architecture: Architecture.find_by_name("x86_64")
)
end
end
end
13 changes: 8 additions & 5 deletions src/api/spec/models/download_repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

RSpec.describe DownloadRepository do
describe "validations" do
subject(:download_repository) { create(:download_repository) }
it { is_expected.to validate_presence_of(:url) }
it { is_expected.to validate_presence_of(:arch) }
it { is_expected.to validate_presence_of(:repotype) }
it { is_expected.to validate_presence_of(:repository_id) }
it { expect(create(:download_repository)).to validate_uniqueness_of(:arch).scoped_to(:repository_id) }
it { is_expected.to validate_uniqueness_of(:arch).scoped_to(:repository_id) }
it { is_expected.to validate_inclusion_of(:repotype).in_array(["rpmmd", "susetags", "deb", "arch", "mdk"]) }

it "validates that architecture is supported by scheduler" do
# FIXME: This is required because of seeding our test DB
architectures = Architecture.all.pluck(:name)
is_expected.to validate_inclusion_of(:arch).in_array(architectures)
describe "architecture_inclusion validation" do
subject(:download_repository) { create(:download_repository) }
it {
expect { download_repository.update_attributes!(arch: "s390x") }.to raise_error(
ActiveRecord::RecordInvalid, "Validation failed: Architecture has to be available via repository association.")
}
end
end
end
1 change: 1 addition & 0 deletions src/api/spec/models/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@
<master url='http://master.opensuse.org' sslfingerprint='my_fingerprint'/>
<pubkey>my_pubkey</pubkey>
</download>
<arch>i586</arch>
</repository>
</project>
EOF
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def test_adding_download_on_demand
select('rpmmd', from: 'Type')
fill_in('Url', with: '')
click_button('Add Download on Demand')
find(:id, 'flash-messages').must_have_text("Download on Demand can't be created: Url can't be blank")
find(:id, 'flash-messages').must_have_text("Download on Demand can't be created: Validation failed: Url can't be blank")
end

def test_editing_download_on_demand
Expand Down Expand Up @@ -132,7 +132,7 @@ def test_editing_download_on_demand
# Fill in the form and send a not working dod data
fill_in('Url', with: '')
click_button('Update Download on Demand')
find(:id, 'flash-messages').must_have_text("Download on Demand can't be updated: Url can't be blank")
find(:id, 'flash-messages').must_have_text("Download on Demand can't be updated: Validation failed: Url can't be blank")
page.must_have_link 'http://somerandomurl_2.es'
end

Expand Down

0 comments on commit cdeee3d

Please sign in to comment.