Use redis-objects gem for caching Branch.srpms.count

REDIS-STORE

@@ -1,8 +1,8 @@
 Internal structure of redis cache used in prometheus2.0:
 
-"#{branch.name}:srpms:counter" integer. Srpms counter for branch.
+"branch:#{ branch.id }:counter" integer. Srpms counter for branch.
 
-e.g. "Sisyphus:srpms:counter" => 12837
+e.g. "branch:#{ branch.id }:counter" => 12837
 
 "#{branch.name}:#{srpm.filename}" 1 or nil. 1 if srpm imported.
 

app/helpers/application_helper.rb

@@ -3,10 +3,6 @@ def title(page_title)
     content_for(:title) { page_title.html_safe }
   end
 
-  def srpm_count(srpm_count)
-    content_for(:srpms_counter) { srpm_count }
-  end
-
   def keywords(string)
     # http://en.wikipedia.org/wiki/Most_common_words_in_English
     skiplist = %w(the be to of and a in i it for not on he as you do at this but his by from or an)

app/models/branch.rb

@@ -1,4 +1,8 @@
+# Class for handing Branches.
+
 class Branch < ActiveRecord::Base
+  include Redis::Objects
+
   validates :name, presence: true
   validates :vendor, presence: true
 
@@ -13,7 +17,25 @@ class Branch < ActiveRecord::Base
   has_many :repocops
   has_many :repocop_patches
 
+  counter :counter
+
+  after_create :set_default_counter_value
+  after_destroy :destroy_counter
+
   def to_param
     name
   end
+
+  def set_default_counter_value
+    Redis.current.set("branch:#{ id }:counter", 0)
+  end
+
+  def destroy_counter
+    Redis.current.del("branch:#{ id }:counter")
+  end
+
+  def recount!
+    Redis.current.set("branch:#{ id }:counter", srpms.count)
+    self
+  end
 end

app/models/srpm.rb

@@ -19,6 +19,9 @@ class Srpm < ActiveRecord::Base
   has_one :builder, class_name: 'Maintainer', foreign_key: 'id',
                     primary_key: 'builder_id'
 
+  after_create :increment_branch_counter
+  after_destroy :decrement_branch_counter
+
   def to_param
     name
   end
@@ -31,15 +34,6 @@ def acls
     end
   end
 
-  def self.count_srpms(branch)
-    counter = $redis.get("#{branch.name}:srpms:counter")
-    unless counter
-      $redis.set("#{branch.name}:srpms:counter", branch.srpms.count)
-      counter = $redis.get("#{branch.name}:srpms:counter")
-    end
-    counter
-  end
-
   def self.import(branch, file)
     srpm = Srpm.new
     srpm.name = `export LANG=C && rpm -qp --queryformat='%{NAME}' #{file}`
@@ -96,7 +90,6 @@ def self.import(branch, file)
       Specfile.import(branch, file, srpm)
       Patch.import(branch, file, srpm)
       Source.import(branch, file, srpm)
-      $redis.incr("#{branch.name}:srpms:counter")
     else
       puts "#{Time.now.to_s}: failed to update '#{srpm.filename}'"
     end
@@ -126,7 +119,6 @@ def self.remove_old(branch, path)
         $redis.del("#{branch.name}:#{srpm.name}:acls")
         $redis.del("#{branch.name}:#{srpm.name}:leader")
         srpm.destroy
-        $redis.decr("#{branch.name}:srpms:counter")
       end
     end
   end
@@ -145,4 +137,14 @@ def self.contributors(branch, srpm)
     end
     Maintainer.where(login: logins.sort.uniq).order(:name)
   end
+
+  private
+
+  def increment_branch_counter
+    branch.counter.increment
+  end
+
+  def decrement_branch_counter
+    branch.counter.decrement
+  end
 end

app/views/home/maintainers_list.html.erb

@@ -17,7 +17,7 @@
   <td style="text-align: left;">
     <%= link_to branch.name, maintainers_path(branch), class: 'news' %>
   </td>
-  <td style="text-align: right;"><%= Srpm.count_srpms(branch) %></td>
+  <td style="text-align: right;"><%= branch.counter.value %></td>
 </tr>
 <% end %>
 

app/views/layouts/application.html.erb

@@ -89,7 +89,7 @@
 
   <div id='top-line'>
     <div id='vinfo'>
-      <b><%= _('Last update:') %> </b> <%= $redis.get("#{@branch.name}:updated_at") %> | <b><%= _('SRPMs:') %> </b> <%= Srpm.count_srpms(@branch) %> | 
+      <b><%= _('Last update:') %> </b> <%= $redis.get("#{@branch.name}:updated_at") %> | <b><%= _('SRPMs:') %> </b> <%= @branch.counter.value %> | 
       <% if user_signed_in? %>
         <b><%= "Welcome, #{current_user.email}! | " %></b>
         <%= link_to _('Sign out'), destroy_user_session_path, method: :delete %>

config/initializers/redis.rb

@@ -1,2 +1,5 @@
+# TODO: drop $redis
 REDIS_CONFIG = YAML.load(File.read(File.dirname(__FILE__) + "/../redis.yml")).symbolize_keys
 $redis = Redis.new(REDIS_CONFIG)
+
+Redis.current = $redis

lib/tasks/migrate.rake

@@ -1,13 +1,13 @@
 namespace :migrate do
-  desc 'Update all srpms with groupname'
-  task :update_groupname => :environment do
+  desc 'Call Branch.recount! on each branch'
+  task :recount => :environment do
 
-    Srpm.where(groupname: nil).each do |srpm|
-      srpm.update_column(:groupname, srpm.group.full_name)
+    Branch.all.each do |branch|
+      branch.recount!
     end
 
-    Package.where(groupname: nil).each do |package|
-      package.update_column(:groupname, package.group.full_name)
+    Branch.all.each do |branch|
+      Redis.current.del("#{branch.name}:srpms:counter")
     end
 
   end

spec/models/branch_spec.rb

@@ -22,4 +22,22 @@
   it 'should return Branch.name on .to_param' do
     Branch.new(name: 'Sisyphus').to_param.should == 'Sisyphus'
   end
+
+  it 'should set default value in redis for counter after create' do
+    branch = FactoryGirl.create(:branch)
+    branch.counter.value.should eq(0)
+  end
+
+  it 'should remove counter in redis after destroy' do
+    branch = FactoryGirl.create(:branch)
+    branch.destroy
+    Redis.current.get("branch:#{ branch.id }:counter").should be_nil
+  end
+
+  it 'should recount Branch.srpms on recount!' do
+    branch = FactoryGirl.create(:branch)
+    Redis.current.set("branch:#{ branch.id }:counter", 42)
+    branch.counter.value.should eq(42)
+    branch.recount!.counter.value.should eq(0)
+  end
 end

spec/models/srpm_spec.rb

@@ -33,7 +33,6 @@
 
   it 'should import srpm file' do
     branch = FactoryGirl.create(:branch)
-    $redis.set("#{ branch.name }:srpms:counter", 0)
     file = 'openbox-3.4.11.1-alt1.1.1.src.rpm'
     md5 = "f87ff0eaa4e16b202539738483cd54d1  /Sisyphus/files/SRPMS/#{file}"
     maintainer = Maintainer.create!(login: 'icesik', email: 'icesik@altlinux.org', name: 'Igor Zubkov')
@@ -94,7 +93,6 @@
     srpm.filename.should eq('openbox-3.4.11.1-alt1.1.1.src.rpm')
 
     $redis.get("#{ branch.name }:#{ srpm.filename }").should eq('1')
-    $redis.get("#{ branch.name }:srpms:counter").should eq('1')
   end
 
   it 'should import all srpms from path' do
@@ -111,14 +109,11 @@
 
   it 'should remove old srpms from database' do
     branch = FactoryGirl.create(:branch)
-    $redis.set("#{ branch.name }:srpms:counter", 0)
     group = FactoryGirl.create(:group, branch_id: branch.id)
     srpm1 = FactoryGirl.create(:srpm, branch_id: branch.id, group_id: group.id)
     $redis.set("#{ branch.name }:#{ srpm1.filename }", 1)
-    $redis.incr("#{ branch.name }:srpms:counter")
     srpm2 = FactoryGirl.create(:srpm, name: 'blackbox', filename: 'blackbox-1.0-alt1.src.rpm', branch_id: branch.id, group_id: group.id)
     $redis.set("#{ branch.name }:#{ srpm2.filename }", 1)
-    $redis.incr("#{ branch.name }:srpms:counter")
     $redis.sadd("#{ branch.name }:#{ srpm2.name }:acls", "icesik")
     $redis.set("#{ branch.name }:#{ srpm2.name }:leader", "icesik")
 
@@ -133,20 +128,24 @@
 
     $redis.get("#{ branch.name }:openbox-3.4.11.1-alt1.1.1.src.rpm").should eq('1')
     $redis.get("#{ branch.name }:blackbox-1.0-alt1.src.rpm").should be_nil
-    $redis.get("#{ branch.name }:srpms:counter").should eq('1')
     $redis.get("#{ branch.name }:#{ srpm2.name }:acls").should be_nil
     $redis.get("#{ branch.name }:#{ srpm2.name }:leader").should be_nil
 
     # TODO: add checks for sub packages, set-get-delete
   end
 
-  it 'should cache srpms counter in redis' do
+  it 'should increment branch.counter on srpm.save' do
+    branch = FactoryGirl.create(:branch)
+    group = FactoryGirl.create(:group, branch_id: branch.id)
+    FactoryGirl.create(:srpm, branch_id: branch.id, group_id: group.id)
+    branch.counter.value.should eq(1)
+  end
+
+  it 'should decrement branch.counter on srpm.destroy' do
     branch = FactoryGirl.create(:branch)
-    $redis.del("#{ branch.name }:srpms:counter")
-    $redis.get("#{ branch.name }:srpms:counter").should be_nil
     group = FactoryGirl.create(:group, branch_id: branch.id)
     srpm = FactoryGirl.create(:srpm, branch_id: branch.id, group_id: group.id)
-    Srpm.count_srpms(branch)
-    $redis.get("#{ branch.name }:srpms:counter").should eq('1')
+    srpm.destroy
+    branch.counter.value.should eq(0)
   end
 end