Minor gems update. Fix string length, refactor git helper and module, add branches_and_tags method, refactor build_list filter, cleanup and refactor templates. Take back test. Filter incorrect symbols during git encoding fix. Fix git http level bug. Refs #263

Gemfile

@@ -13,7 +13,7 @@ gem 'omniauth-openid', '~> 1.0.1'
 gem 'cancan', '1.6.7' # 1.6.8 fail specs with strange error
 
 gem 'ancestry', '~> 1.3.0'
-gem 'paperclip', '~> 3.1.3'
+gem 'paperclip', '~> 3.1.4'
 gem 'resque', '~> 1.21.0'
 gem 'resque-status', '~> 0.3.3'
 gem 'resque_mailer', '~> 2.1.0'
@@ -27,7 +27,7 @@ gem 'grack', :git => 'git://github.com/rdblue/grack.git', :require => 'git_http'
 gem "grit", :git => 'git://github.com/warpc/grit.git' #, :path => '~/Sites/code/grit'
 gem 'charlock_holmes', '~> 0.6.8' #, :git => 'git://github.com/brianmario/charlock_holmes.git', :branch => 'bundle-icu'
 # gem 'ruby-filemagic', '~> 0.4.2', :require => 'filemagic/ext'
-gem 'github-linguist', '~> 2.0.1', :require => 'linguist'
+gem 'github-linguist', '~> 2.1.2', :require => 'linguist'
 gem 'diff-display', '~> 0.0.1'
 
 # Wiki

Gemfile.lock

@@ -117,12 +117,12 @@ GEM
       railties (>= 3.0.0)
     ffi (1.0.11)
     fssm (0.2.9)
-    github-linguist (2.0.1)
+    github-linguist (2.1.2)
       charlock_holmes (~> 0.6.6)
       escape_utils (~> 0.2.3)
       mime-types (~> 1.18)
-      pygments.rb (~> 0.2.13)
+      pygments.rb (>= 0.2.13)
-    github-markup (0.7.2)
+    github-markup (0.7.4)
     gollum (1.3.1)
       albino (~> 1.3.2)
       github-markup (>= 0.4.0, < 1.0.0)
@@ -150,7 +150,7 @@ GEM
     jquery-rails (2.0.2)
       railties (>= 3.2.0, < 5.0)
       thor (~> 0.14)
-    json (1.7.3)
+    json (1.7.4)
     kgio (2.7.4)
     libv8 (3.3.10.4)
     macaddr (1.6.1)
@@ -159,7 +159,7 @@ GEM
       i18n (>= 0.4.0)
       mime-types (~> 1.16)
       treetop (~> 1.4.8)
-    mailcatcher (0.5.7.2)
+    mailcatcher (0.5.8)
       activesupport (~> 3.0)
       eventmachine (~> 0.12)
       haml (~> 3.1)
@@ -189,7 +189,7 @@ GEM
       omniauth (~> 1.0)
       rack-openid (~> 1.3.1)
     orm_adapter (0.4.0)
-    paperclip (3.1.3)
+    paperclip (3.1.4)
       activemodel (>= 3.0.0)
       activerecord (>= 3.0.0)
       activesupport (>= 3.0.0)
@@ -267,8 +267,8 @@ GEM
       rspec-core (~> 2.11.0)
       rspec-expectations (~> 2.11.0)
       rspec-mocks (~> 2.11.0)
-    rspec-core (2.11.0)
+    rspec-core (2.11.1)
-    rspec-expectations (2.11.1)
+    rspec-expectations (2.11.2)
       diff-lcs (~> 1.1.3)
     rspec-mocks (2.11.1)
     rspec-rails (2.11.0)
@@ -285,7 +285,7 @@ GEM
       ffi (~> 1.0.7)
     russian (0.6.0)
       i18n (>= 0.5.0)
-    rvm-capistrano (1.2.3)
+    rvm-capistrano (1.2.5)
       capistrano (>= 2.0.0)
     sanitize (2.0.3)
       nokogiri (>= 1.4.4, < 1.6)
@@ -296,7 +296,7 @@ GEM
       tilt (~> 1.3)
     shotgun (0.9)
       rack (>= 1.0)
-    shoulda (3.1.0)
+    shoulda (3.1.1)
       shoulda-context (~> 1.0)
       shoulda-matchers (~> 1.2)
     shoulda-context (1.0.0)
@@ -368,7 +368,7 @@ DEPENDENCIES
   devise (~> 2.1.2)
   diff-display (~> 0.0.1)
   factory_girl_rails (~> 3.5.0)
-  github-linguist (~> 2.0.1)
+  github-linguist (~> 2.1.2)
   gollum (= 1.3.1)
   grack!
   grit!
@@ -382,7 +382,7 @@ DEPENDENCIES
   newrelic_rpm (~> 3.4.0.1)
   omniauth (~> 1.1.0)
   omniauth-openid (~> 1.0.1)
-  paperclip (~> 3.1.3)
+  paperclip (~> 3.1.4)
   perform_later (~> 1.3.0)
   pg (~> 0.14.0)
   rails (= 3.2.6)

app/controllers/projects/build_lists_controller.rb

@@ -46,7 +46,8 @@ class Projects::BuildListsController < Projects::BaseController
     Arch.where(:id => params[:arches]).each do |arch|
       Platform.main.where(:id => params[: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.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/helpers/git_helper.rb

@@ -47,7 +47,6 @@ module GitHelper
     end
   end
 
-  # TODO This is very dirty hack. Maybe need to be changed.
   def branch_selector_options(project)
     p = params.dup
     p.delete(:path) if p[:path].present? # to root path
@@ -55,18 +54,17 @@ module GitHelper
     current = url_for(p).split('?', 2).first
 
     res = []
-    res << [I18n.t('layout.git.repositories.commits'), [truncate(params[:treeish], :length => 20)]] unless (project.repo.branches + project.repo.tags).map(&:name).include?(params[:treeish] || project.default_branch)
+    res << [I18n.t('layout.git.repositories.commits'), [params[:treeish].truncate(20)]] unless project.repo.branches_and_tags.map(&:name).include?(params[:treeish] || project.default_branch)
-    res << [I18n.t('layout.git.repositories.branches'), project.repo.branches.map{|b| [truncate(b.name, :length => 20), url_for(p.merge :treeish => b.name).split('?', 2).first]}]
+    linking = Proc.new {|t| [t.name.truncate(20), url_for(p.merge :treeish => t.name).split('?', 2).first]}
-    res << [I18n.t('layout.git.repositories.tags'), project.repo.tags.map{|t| [truncate(t.name, :length => 20), url_for(p.merge :treeish => t.name).split('?', 2).first]}]
+    res << [I18n.t('layout.git.repositories.branches'), project.repo.branches.map(&linking)]
+    res << [I18n.t('layout.git.repositories.tags'), project.repo.tags.map(&linking)]
 
     grouped_options_for_select(res, current)
   end
 
   def versions_for_group_select(project)
-    [
+    [ ['Branches', project.repo.branches.map{|b| "latest_#{b.name}"}],
-      ['Branches', project.repo.branches.map{|b| "latest_#{b.name}"}],
+      ['Tags', project.repo.tags.map(&:name)] ]
-      ['Tags', project.repo.tags.map(&:name)]
-    ]
   end
   
   def split_commits_by_date(commits)

app/models/build_list/filter.rb

@@ -53,12 +53,12 @@ class BuildList::Filter
     }))
 
     @options[:ownership] = @options[:ownership].presence || (@project || !@user ? 'index' : 'owned')
-    @options[:status] = @options[:status].present? ? @options[:status].to_i : nil
+    @options[:status] = @options[:status].try(:to_i)
     @options[:updated_at_start] = build_date_from_params(:updated_at_start, @options)
     @options[:updated_at_end] = build_date_from_params(:updated_at_end, @options)
     @options[:project_version] = @options[:project_version].presence
-    @options[:arch_id] = @options[:arch_id].present? ? @options[:arch_id].to_i : nil
+    @options[:arch_id] = @options[:arch_id].try(:to_i)
-    @options[:platform_id] = @options[:platform_id].present? ? @options[:platform_id].to_i : nil
+    @options[:platform_id] = @options[:platform_id].try(:to_i)
     @options[:is_circle] = @options[:is_circle].present? ? @options[:is_circle] == "1" : nil
     @options[:bs_id] = @options[:bs_id].presence
     @options[:project_name] = @options[:project_name].presence

app/models/comment.rb

@@ -15,7 +15,9 @@ class Comment < ActiveRecord::Base
   attr_accessible :body
 
   def commentable
-    commit_comment? ? project.repo.commit(commentable_id.to_s(16)) : super
+    # raise commentable_id.inspect
+    # raise commentable_id.to_s(16).inspect
+    commit_comment? ? project.repo.commit(commentable_id.to_s(16)) : super # TODO leading zero problem
   end
 
   def commentable=(c)

app/views/projects/git/blobs/_show.html.haml

@@ -1,4 +1,3 @@
-/ - raise @blob.file_mime_type.inspect
 %h3= t("layout.projects.files_in_project")
 .files
   .l= render 'whereami'

app/views/projects/git/blobs/_top.html.haml

@@ -6,15 +6,15 @@
     \|
   = number_to_human_size @blob.size
 .r
-  - if @blob.render_as == :text and can? :write, @project and @branch.present?
+  - if @blob.render_as == :text && can?(:write, @project) && @branch.present?
     = link_to "Edit", edit_blob_path(@project, @treeish, @path)
     \|
-  - if @blob.render_as == :text and params[:action] != 'show'
+  - if @blob.render_as == :text && params[:action] != 'show'
     = link_to "Normal", blob_path(@project, @treeish, @path)
     \|
   = link_to "Raw", raw_path(@project, @treeish, @path)
   \|
-  - if @blob.render_as == :text and params[:action] != 'blame'
+  - if @blob.render_as == :text && params[:action] != 'blame'
     = link_to "Blame", blame_path(@project, @treeish, @path)
     \|
   = link_to "History", commits_path(@project, @treeish, @path)

app/views/projects/git/commits/_show.html.haml

@@ -13,7 +13,6 @@
 
 -begin
   = render_commit_stats(stats)
-
   = render :partial => 'commit_diff', :collection => @commit.diffs
 - rescue Grit::Git::GitTimeout
   %p= t 'layout.git.repositories.commit_diff_too_big'

app/views/projects/git/trees/_show.html.haml

@@ -31,7 +31,7 @@
             .pic= image_tag 'folder.png'
             .name= link_to(entry.name, tree_path(@project, @treeish, entry_path), :class => 'files-see')
         %td
-          %span{:style => "display: none;"}= commit.committed_date || commit.authored_date
+          %span{:style => "display: none;"}= date = commit.committed_date || commit.authored_date
-          = l(commit.committed_date || commit.authored_date, :format => :short)
+          = l(date, :format => :short)
         %td= commit.short_message
         %td= (commit.committer || commit.author).name

lib/ext/core/string.rb

@@ -1,27 +1,24 @@
 # -*- encoding : utf-8 -*-
 require 'charlock_holmes/string'
-# require 'iconv'
 
 class String
   def default_encoding!
-    if ascii_only?
+    default_encoding = Encoding.default_internal || Encoding::UTF_8
-      force_encoding(Encoding.default_internal || Encoding::UTF_8)
+    if ascii_only? # no need to encode if ascii
-    else
+      force_encoding(default_encoding)
-      force_encoding((detected = detect_encoding and detected[:encoding]) || Encoding.default_internal || Encoding::UTF_8).encode!
+    else # should encode
+      if (detected = detect_encoding) && detected[:encoding]
+        force_encoding(detected[:encoding])
+        encode!(default_encoding, detected[:encoding], :invalid => :replace, :undef => :replace, :replace => '')
       end
+      # re-encode through UTF-16 to filter incorrect symbols
+      encode!(Encoding::UTF_16, default_encoding, :invalid => :replace, :undef => :replace, :replace => '')
+      encode!(default_encoding, Encoding::UTF_16)
+      raise unless valid_encoding? # check result
+    end
+  rescue
+    replace "--broken encoding: #{detect_encoding[:encoding] || 'unknown'}"
+  ensure
+    self
   end
-
-  # def enforce_utf8(from = nil)
-  #   begin
-  #     is_utf8? ? self : ::Iconv.iconv('utf8', from, self).first
-  #   rescue
-  #     converter = ::Iconv.new('UTF-8//IGNORE//TRANSLIT', 'ASCII//IGNORE//TRANSLIT')
-  #     # If Ruby 1.9, else another RubyEngine (ree, Ruby 1.8)
-  #     begin 
-  #       converter.iconv(self).unpack('U*').select{|cp| cp < 127}.pack('U*').force_encoding('utf-8')
-  #     rescue
-  #       converter.iconv(self).unpack('U*').select{|cp| cp < 127}.pack('U*')
-  #     end
-  #   end
-  # end  
 end

lib/ext/rails/constraint.rb

@@ -1,49 +0,0 @@
-# -*- encoding : utf-8 -*-
-class OwnerConstraint
-  def initialize(class_name, bang = false)
-    @class_name = class_name
-    @finder = 'find_by_insensitive_uname'
-    @finder << '!' if bang
-  end
-
-  def matches?(request)
-    @class_name.send(@finder, request.params[:uname]).present?
-  end
-end
-
-class AdminAccess
-  def self.matches?(request)
-    !!request.env['warden'].user.try(:admin?)
-  end
-end
-
-class TreeishConstraint
-  def self.matches?(request)
-    # raise request.params.inspect
-    # params = request.env['action_dispatch.request.path_parameters'] || request.params
-    params = request.path_parameters
-    if params[:treeish] # parse existing branch (tag) and path
-      branch_or_tag = begin
-        (p = Project.find_by_owner_and_name params[:owner_name], params[:project_name]) &&
-        (p.repo.branches + p.repo.tags).detect{|t| params[:treeish].start_with?(t.name)}.try(:name) ||
-        params[:treeish].split('/').first
-      end
-      if path = params[:treeish].sub(branch_or_tag, '')[1..-1] and path.present?
-        params[:path] = File.join([path, params[:path]].compact)
-      end
-      params[:treeish] = branch_or_tag
-    end
-    true
-  end
-
-  # def set_treeish_and_path
-  #   if params[:treeish] and params[:path] and # try to correct branch with slashes
-  #      treeish_with_path = File.join(params[:treeish], params[:path]) and
-  #      branch_name = @project.repo.branches.detect{|t| treeish_with_path.start_with?(t.name)}.try(:name)
-  #     params[:treeish] = branch_name
-  #     params[:path] = treeish_with_path.sub(branch_name, '')[1..-1]
-  #   end
-  #   @treeish = params[:treeish].presence || @project.default_branch
-  #   @path = params[:path]
-  # end  
-end

lib/ext/rails/middleware.rb

@@ -1,18 +0,0 @@
-# class ParamsParser
-#   DEFAULT_PARSERS = {
-#     Mime::XML => :xml_simple,
-#     Mime::JSON => :json
-#   }
-# 
-#   def initialize(app, parsers = {})
-#     @app, @parsers = app, DEFAULT_PARSERS.merge(parsers)
-#   end
-# 
-#   def call(env)
-#     if params = parse_formatted_parameters(env)
-#       env["action_dispatch.request.request_parameters"] = params
-#     end
-# 
-#     @app.call(env)
-#   end
-# end

lib/ext/rosa/constraints.rb

@@ -21,13 +21,11 @@ module Rosa
 
     class Treeish
       def self.matches?(request)
-        # raise request.params.inspect
-        # params = request.env['action_dispatch.request.path_parameters'] || request.params
         params = request.path_parameters
         if params[:treeish] # parse existing branch (tag) and path
           branch_or_tag = begin
             (p = Project.find_by_owner_and_name params[:owner_name], params[:project_name]) &&
-            (p.repo.branches + p.repo.tags).detect{|t| params[:treeish].start_with?(t.name)}.try(:name) ||
+            p.repo.branches_and_tags.detect{|t| params[:treeish].start_with?(t.name)}.try(:name) ||
             params[:treeish].split('/').first
           end
           if path = params[:treeish].sub(branch_or_tag, '')[1..-1] and path.present?

lib/modules/models/git.rb

@@ -60,11 +60,8 @@ module Modules
       end
 
       def paginate_commits(treeish, options = {})
-        options[:page] = 1 unless options[:page].present?
+        options[:page] = options[:page].try(:to_i) || 1
-        options[:page] = options[:page].to_i
+        options[:per_page] = options[:per_page].try(:to_i) || 20
-
-        options[:per_page] = 20 unless options[:per_page].present?
-        options[:per_page] = options[:per_page].to_i
 
         skip = options[:per_page] * (options[:page] - 1)
         last_page = (skip + options[:per_page]) >= repo.commit_count(treeish)
@@ -72,26 +69,13 @@ module Modules
         [repo.commits(treeish, options[:per_page], skip), options[:page], last_page]
       end
 
-      def last_active_branch
-        @last_active_branch ||= repo.branches.inject do |r, c|
-          r_last = r.commit.committed_date || r.commit.authored_date unless r.nil?
-          c_last = c.commit.committed_date || c.commit.authored_date
-          if r.nil? or r_last < c_last
-            r = c
-          end
-          r
-        end
-        @last_active_branch
-      end
-
       def tree_info(tree, treeish = nil, path = nil)
-        treeish = tree.id unless treeish.present?
+        treeish ||= tree.id
         # initialize result as hash of <tree_entry> => nil
         res = (tree.trees.sort + tree.blobs.sort).inject({}){|h, e| h.merge!({e => nil})}
         # fills result vith commits that describes this file
         res = res.inject(res) do |h, (entry, commit)|
-          # only if commit == nil ...
+          if commit.nil? and entry.respond_to?(:name) # only if commit == nil
-          if commit.nil? and entry.respond_to? :name
             # ... find last commit corresponds to this file ...
             c = repo.log(treeish, File.join([path, entry.name].compact), :max_count => 1).first
             # ... and add it to result.

lib/plugins/grack/base.rb

@@ -35,7 +35,7 @@ module Grack
     def project
       @project ||= begin
         uname, name = @env['PATH_INFO'].split('/')[1,2]
-        name.gsub!(/\.git$/, '').gsub!(/\.wiki$/, '')
+        name.gsub!(/(\.wiki)?\.git$/, '')
         Project.find_by_owner_and_name uname, name
       end
     end

spec/controllers/platforms/mass_builds_controller_spec.rb

@@ -104,6 +104,11 @@ describe Platforms::MassBuildsController do
       response.should redirect_to(new_user_session_path)
     end
 
+    it "should not be able to perform cancel action" do
+      post :cancel, :platform_id => @platform, :id => @mass_build
+      response.should redirect_to(new_user_session_path)
+    end
+
     it 'should not change objects count on create success' do
       lambda { post :create, @create_params }.should change{ MassBuild.count }.by(0)
     end

spec/routing/projects_routing_spec.rb.rb

@@ -13,7 +13,7 @@ describe Projects::ProjectsController do
     end
 
     it "routes to #edit" do
-      get("/import/glib2.0-mib/edit").should route_to("projects/projects#edit", :owner_name => 'import', :project_name => 'glib2.0-mib')
+      get("/import/glib2.0-mib/modify").should route_to("projects/projects#edit", :owner_name => 'import', :project_name => 'glib2.0-mib')
     end
 
     it "routes to #create" do
@@ -39,6 +39,7 @@ describe Projects::Git::TreesController do
       get("/import/glib2.0-mib/tree/lib2safe-0.03").should route_to("projects/git/trees#show", :owner_name => 'import', :project_name => 'glib2.0-mib', :treeish => 'lib2safe-0.03')
       get("/import/glib2.0-mib/tree/branch-with.dot/folder_with.dot/path-with.dot").should route_to("projects/git/trees#show", :owner_name => 'import', :project_name => 'glib2.0-mib', :treeish => 'branch-with.dot', :path => 'folder_with.dot/path-with.dot')
       # get("/import/glib2.0-mib/tree/ветка-с.точкой/папка_с.точкой/путь-с.точкой").should route_to("projects/git/trees#show", :owner_name => 'import', :project_name => 'glib2.0-mib', :treeish => 'ветка-с.точкой', :path => 'папка_с.точкой/путь-с.точкой')
+      get("/import/glib2.0-mib/tree/branch-with/slash.dot/folder_with.dot/path-with.dot").should route_to("projects/git/trees#show", :owner_name => 'import', :project_name => 'glib2.0-mib', :treeish => 'branch-with/slash.dot', :path => 'folder_with.dot/path-with.dot')
       get("/import/glib2.0-mib/tree/tag13.52-5").should route_to("projects/git/trees#show", :owner_name => 'import', :project_name => 'glib2.0-mib', :treeish => 'tag13.52-5')
     end