[refs #374] added commit validation

app/controllers/api/v1/build_lists_controller.rb

@@ -1,12 +1,12 @@
 # -*- encoding : utf-8 -*-
 class Api::V1::BuildListsController < Api::V1::BaseController
-  
+
   before_filter :authenticate_user!
   skip_before_filter :authenticate_user!, :only => [:show, :index] if APP_CONFIG['anonymous_access']
-  
+
   load_and_authorize_resource :project, :only => :index
   load_and_authorize_resource :build_list, :only => [:show, :create, :cancel, :publish, :reject_publish]
-  
+
   def index
     filter = BuildList::Filter.new(nil, current_user, params[:filter] || {})
     @build_lists = filter.find.scoped(:include => [:save_to_platform, :project, :user, :arch])
@@ -23,7 +23,6 @@ class Api::V1::BuildListsController < Api::V1::BaseController
       bl_params[:auto_publish] = false unless save_to_repository.publish_without_qa?
 
       @build_list = project.build_lists.build(bl_params)
-      @build_list.project_version = @build_list.commit_hash
 
       @build_list.user = current_user
       @build_list.priority = current_user.build_priority # User builds more priority than mass rebuild with zero priority

app/controllers/projects/build_lists_controller.rb

@@ -57,8 +57,6 @@ class Projects::BuildListsController < Projects::BaseController
     Arch.where(:id => params[:arches]).each do |arch|
       Platform.main.where(:id => build_for_platforms).each do |build_for_platform|
         @build_list = @project.build_lists.build(params[:build_list])
-        @build_list.commit_hash = @project.repo.commits(@build_list.project_version.match(/^latest_(.+)/).to_a.last ||
-                                  @build_list.project_version).first.id if @build_list.project_version
         @build_list.build_for_platform = build_for_platform; @build_list.arch = arch; @build_list.user = current_user
         @build_list.include_repos = @build_list.include_repos.select {|ir| @build_list.build_for_platform.repository_ids.include? ir.to_i}
         @build_list.priority = current_user.build_priority # User builds more priority than mass rebuild with zero priority

app/models/build_list.rb

@@ -32,8 +32,13 @@ class BuildList < ActiveRecord::Base
       errors.add(:save_to_repository, I18n.t('flash.build_list.wrong_include_repos')) unless build_for_platform.repository_ids.include? ir.to_i
     }
   }
+  validate lambda {
+    if commit_hash.blank? || project.repo.commit(commit_hash).blank?
+      errors.add :commit_hash, I18n.t('flash.build_list.wrong_commit_hash', :commit_hash => commit_hash)
+    end
+  }
 
-  LIVE_TIME = 4.week # for unpublished 
+  LIVE_TIME = 4.week # for unpublished
   MAX_LIVE_TIME = 3.month # for published
 
   # The kernel does not send these statuses directly
@@ -110,6 +115,7 @@ class BuildList < ActiveRecord::Base
 
   after_commit :place_build
   after_destroy :delete_container
+  before_validation :set_commit_and_version
 
   @queue = :clone_and_build
 
@@ -204,7 +210,7 @@ class BuildList < ActiveRecord::Base
   def set_version_and_tag
     pkg = self.packages.where(:package_type => 'source', :project_id => self.project_id).first
     # TODO: remove 'return' after deployment ABF kernel 2.0
-    return if pkg.nil? # For old client that does not sends data about packages 
+    return if pkg.nil? # For old client that does not sends data about packages
     self.package_version = "#{pkg.platform.name}-#{pkg.version}-#{pkg.release}"
     system("cd #{self.project.repo.path} && git tag #{self.package_version} #{self.commit_hash}") # TODO REDO through grit
     save
@@ -293,7 +299,7 @@ class BuildList < ActiveRecord::Base
   end
 
   def in_work?
-    status == BuildServer::BUILD_STARTED 
+    status == BuildServer::BUILD_STARTED
     #[WAITING_FOR_RESPONSE, BuildServer::BUILD_PENDING, BuildServer::BUILD_STARTED].include?(status)
   end
 
@@ -331,4 +337,13 @@ class BuildList < ActiveRecord::Base
       yield p
     end
   end
+
+  def set_commit_and_version
+    if project_version.present? && commit_hash.blank?
+      self.commit_hash = project.repo.commits(project_version.match(/^latest_(.+)/).to_a.last ||
+                    project_version).try(:first).try(:id)
+    elsif project_version.blank? && commit_hash.present?
+      self.project_version = commit_hash
+    end
+  end
 end

config/locales/models/build_list.en.yml

@@ -29,6 +29,7 @@ en:
         started_at: Build started at
         duration: Build duration in seconds
         mass_build_id: Mass build
+        commit_hash: Commit hash
 
       build_list/item:
         name: Name
@@ -123,7 +124,7 @@ en:
         build_log: Build Log
         not_available: Log not available yet.
         download: Download log
-        autoreload: Update log every 
+        autoreload: Update log every
         load_lines: Load last %{count} lines
 
         reload_times:
@@ -147,3 +148,4 @@ en:
       can_not_published: Build can only be published with status "Build complete"
       frozen_platform: In case of a repository for package storage with frozen platform allowed only bugfix and security updates
       wrong_include_repos: Include repos have to belongs to build for platform
+      wrong_commit_hash: Unable find commit '%{commit_hash}' in project

config/locales/models/build_list.ru.yml

@@ -28,6 +28,7 @@ ru:
         preferences: Настройки
         duration: Длительность билда в секундах
         mass_build_id: Массовая сборка
+        commit_hash: Хэш коммита
 
       build_list/item:
         name: Название
@@ -120,9 +121,9 @@ ru:
         build_log: Лог сборки
         not_available: В настоящий момент лог недоступен.
         download: Загрузить лог
-        autoreload: Обновлять лог каждые 
+        autoreload: Обновлять лог каждые
         load_lines: Загружать последние %{count} строк
-        
+
         reload_times:
           10000:  "10 сек"
           30000:  "30 сек"
@@ -144,3 +145,4 @@ ru:
       can_not_published: Опубликовать сборку можно только со статусом "Собран"
       frozen_platform: В случае выбора репозитория для сохранения пакетов из замороженнной платформы разрешены только bugfix и security обновления
       wrong_include_repos: Включаемые репозитории должны принадлежать платформе для сборки
+      wrong_commit_hash: Невозможно найти коммит '%{commit_hash}' в проекте

spec/controllers/api/v1/build_lists_controller_spec.rb

@@ -47,6 +47,10 @@ shared_examples_for 'create build list via api' do
     #@project.build_lists.last.commit_hash.should == @project.repo.commits('4.7.5.3').last.id
     @project.build_lists.last.commit_hash.should == @params[:commit_hash]
   end
+
+  it 'should not create without existing commit hash in project' do
+    lambda{ post :create, @create_params.deep_merge(:build_list => {:commit_hash => 'wrong'})}.should change{@project.build_lists.count}.by(0)
+  end
 end
 
 shared_examples_for 'not create build list via api' do
@@ -383,13 +387,13 @@ describe Api::V1::BuildListsController do
 
       # Build Lists:
       @build_list1 = FactoryGirl.create(:build_list_core)
-      
+
       @build_list2 = FactoryGirl.create(:build_list_core)
       @build_list2.project.update_column(:visibility, 'hidden')
-      
+
       project = FactoryGirl.create(:project, :visibility => 'hidden', :owner => @user)
       @build_list3 = FactoryGirl.create(:build_list_core, :project => project)
-      
+
       @build_list4 = FactoryGirl.create(:build_list_core)
       @build_list4.project.update_column(:visibility, 'hidden')
       @build_list4.project.relations.create! :role => 'reader', :actor_id => @user.id, :actor_type => 'User'

spec/controllers/projects/build_lists_controller_spec.rb

@@ -30,7 +30,6 @@ describe Projects::BuildListsController do
   shared_examples_for 'create build list' do
     before {
       @project.update_attribute(:repositories, @platform.repositories)
-      test_git_commit(@project)
     }
 
     it 'should be able to perform new action' do
@@ -53,12 +52,19 @@ describe Projects::BuildListsController do
       post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params).deep_merge(:build_list => {:project_version => "4.7.5.3"})
       @project.build_lists.last.commit_hash.should == @project.repo.commits('4.7.5.3').last.id
     end
+
+    it 'should not be able to create with wrong project version' do
+      lambda{ post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params).deep_merge(:build_list => {:project_version => "latest_wrong", :commit_hash => nil})}.should change{@project.build_lists.count}.by(0)
+    end
+
+    it 'should not be able to create with wrong git hash' do
+      lambda{ post :create, {:owner_name => @project.owner.uname, :project_name => @project.name}.merge(@create_params).deep_merge(:build_list => {:commit_hash => 'wrong'})}.should change{@project.build_lists.count}.by(0)
+    end
   end
 
   shared_examples_for 'not create build list' do
     before {
       @project.update_attribute(:repositories, @platform.repositories)
-      test_git_commit(@project)
     }
 
     it 'should not be able to perform new action' do
@@ -78,7 +84,7 @@ describe Projects::BuildListsController do
     before(:each) do
       @platform = FactoryGirl.create(:platform_with_repos)
       @create_params = {
-        :build_list => { 
+        :build_list => {
           :project_version => 'latest_master',
           :save_to_repository_id => @platform.repositories.first.id,
           :update_type => 'security',
@@ -119,13 +125,13 @@ describe Projects::BuildListsController do
       context 'for all build lists' do
         before(:each) do
           @build_list1 = FactoryGirl.create(:build_list_core)
-          
+
           @build_list2 = FactoryGirl.create(:build_list_core)
           @build_list2.project.update_column(:visibility, 'hidden')
-          
+
           project = FactoryGirl.create(:project, :visibility => 'hidden', :owner => @user)
           @build_list3 = FactoryGirl.create(:build_list_core, :project => project)
-          
+
           @build_list4 = FactoryGirl.create(:build_list_core)
           @build_list4.project.update_column(:visibility, 'hidden')
           @build_list4.project.relations.create! :role => 'reader', :actor_id => @user.id, :actor_type => 'User'
@@ -203,17 +209,17 @@ describe Projects::BuildListsController do
 
         @show_params = {:owner_name => @project.owner.uname, :project_name => @project.name, :id => @build_list.id}
       end
-  
+
       context 'for all build lists' do
         before(:each) do
           @build_list1 = FactoryGirl.create(:build_list_core)
-          
+
           @build_list2 = FactoryGirl.create(:build_list_core)
           @build_list2.project.update_column(:visibility, 'hidden')
-          
+
           project = FactoryGirl.create(:project, :visibility => 'hidden', :owner => @user)
           @build_list3 = FactoryGirl.create(:build_list_core, :project => project)
-          
+
           @build_list4 = FactoryGirl.create(:build_list_core)
           @build_list4.project.update_column(:visibility, 'hidden')
           @build_list4.project.relations.create! :role => 'reader', :actor_id => @user.id, :actor_type => 'User'
@@ -286,13 +292,13 @@ describe Projects::BuildListsController do
         get :index
         assigns[:build_server_status].should == {}
         response.should be_success
-      end    
+      end
     end
   end
 
   context 'filter' do
-    
-    before(:each) do 
+
+    before(:each) do
       set_session_for FactoryGirl.create(:admin)
 
       @build_list1 = FactoryGirl.create(:build_list_core)

spec/factories/build_lists.rb

@@ -8,11 +8,11 @@ FactoryGirl.define do
     association :arch
     build_for_platform {|bl| bl.save_to_platform}
     save_to_repository {|bl| bl.save_to_platform.repositories.first}
-    project_version "1.0"
     build_requires true
     update_type 'security'
     include_repos {|bl| bl.save_to_platform.repositories.map(&:id)}
-    commit_hash '1234567890abcdef1234567890abcdef12345678'
+    project_version 'latest_master'
+    after(:build) {|bl| test_git_commit bl.project }
   end
 
   factory :build_list_core, :parent => :build_list do

spec/spec_helper.rb

@@ -27,7 +27,7 @@ RSpec.configure do |config|
   config.use_transactional_fixtures = true
 
   config.filter_run_excluding :anonymous_access => !(APP_CONFIG['anonymous_access'])
-  
+
 end
 
 def set_session_for(user=nil)