ref: refs/heads/master commit 0952b6928968ea7f6934c0cecfcf102e04e75cde Author: Duncan Mac-Vicar P <dmacvicar@suse.de> Date: Wed Aug 12 13:53:53 2009 +0200 refactoring (first round): - moved dbus stuff to Patch model - simplified controller - added Patch model test case - fixed Patch controller stubs - big refactor of cache status checking, moved the checks on the solv files and rpm db to Patch class as mtime method and the cache function now just checks if the expiration time is older than the Patch.mtime This allows to mock mtime easily and test. Also separates policy from mechanism. --- .../patches/app/controllers/patches_controller.rb | 165 +----------------- plugins/patches/app/models/patch.rb | 186 +++++++++++++++++++- .../test/functional/patches_controller_test.rb | 29 +++- plugins/patches/test/unit/patch_test.rb | 18 ++ 4 files changed, 227 insertions(+), 171 deletions(-) diff --git a/plugins/patches/app/controllers/patches_controller.rb b/plugins/patches/app/controllers/patches_controller.rb index b72b3e4..ae082fd 100644 --- a/plugins/patches/app/controllers/patches_controller.rb +++ b/plugins/patches/app/controllers/patches_controller.rb @@ -1,49 +1,5 @@ -require "dbus" -require 'socket' -require 'thread' require 'singleton' - -# = MainPkg event loop class. -# -# Class that takes care of handling message and signal events -# asynchronously. -class MainPkg - # Create a new main event loop. - def initialize - @buses = Hash.new - end - - # Add a _bus_ to the list of buses to watch for events. - def <<(bus) - @buses[bus.socket] = bus - end - - # Run the main loop. This is a blocking call! - def run - ok = true - finished = false - while !finished do - ready, dum, dum = IO.select(@buses.keys) - ready.each do |socket| - b = @buses[socket] - b.update_buffer - while m = b.pop_message - b.process(m) - if m.member == "Finished" || m.member == "Errorcode" - finished = true - if m.member == "Error" - ok = false - end - end - end - end - end - return ok - end -end # class MainPkg - - class PatchesController < ApplicationController before_filter :login_required @@ -56,12 +12,6 @@ class PatchesController < ApplicationController # cache 'index' method result caches_action :index -#-------------------------------------------------------------------------------- -# -#local methods -# -#-------------------------------------------------------------------------------- - private def check_read_permissions @@ -81,10 +31,7 @@ class PatchesController < ApplicationController # the cache expires after 5 minutes, repository metadata # or RPM database update invalidates the cache immeditely # (new patches might be applicable) - elsif cache_timestamp < 5.minutes.ago || - cache_timestamp < File.stat("/var/lib/rpm/Packages").mtime || - cache_timestamp < File.stat("/var/cache/zypp/solv").mtime || - Dir["/var/cache/zypp/solv/*/solv"].find{ |x| File.stat(x).mtime > cache_timestamp} + elsif cache_timestamp < 5.minutes.ago || cache_timestamp < Patch.mtime logger.debug "#### Patch cache expired" expire_action :action => :index, :format => params["format"] Rails.cache.write('patches:timestamp', Time.now) @@ -93,113 +40,11 @@ class PatchesController < ApplicationController public - def get_updateList - patch_updates = [] - system_bus = DBus::SystemBus.instance - package_kit = system_bus.service("org.freedesktop.PackageKit") - obj = package_kit.object("/org/freedesktop/PackageKit") -#logger.debug obj.inspect - obj.introspect - obj_with_iface = obj["org.freedesktop.PackageKit"] - tid = obj_with_iface.GetTid - obj_tid = package_kit.object(tid[0]) - obj_tid.introspect - obj_tid_with_iface = obj_tid["org.freedesktop.PackageKit.Transaction"] - obj_tid.default_iface = "org.freedesktop.PackageKit.Transaction" - - finished = false - obj_tid.on_signal("Package") do |line1,line2,line3| - columns = line2.split ";" - update = Patch.new(columns[1], line1, columns[0], columns[2], columns[3], line3) - patch_updates << update - finished = true - end - - obj_tid.on_signal("Error") do |u1,u2| - finished = true - end - obj_tid.on_signal("Finished") do |u1,u2| - finished = true - end - obj_tid_with_iface.GetUpdates("NONE") - - unless finished - @main = MainPkg.new - @main << system_bus - @main.run - end - - obj_with_iface.SuggestDaemonQuit - - return patch_updates - end - - def get_update (id) - if @patch_updates.nil? || @patch_updates.empty? - @patch_updates = get_updateList - end - - @patch_updates.find { |p| p.resolvable_id.to_s == id.to_s } - end - - def install_update (id) - ok = true - - updateId = "#{@patch_update.name};#{@patch_update.resolvable_id};#{@patch_update.arch};#{@patch_update.repo}" - logger.debug "Install Update: #{updateId}" - - system_bus = DBus::SystemBus.instance - package_kit = system_bus.service("org.freedesktop.PackageKit") - obj = package_kit.object("/org/freedesktop/PackageKit") - obj.introspect - obj_with_iface = obj["org.freedesktop.PackageKit"] - tid = obj_with_iface.GetTid - obj_tid = package_kit.object(tid[0]) - obj_tid.introspect - obj_tid_with_iface = obj_tid["org.freedesktop.PackageKit.Transaction"] - obj_tid.default_iface = "org.freedesktop.PackageKit.Transaction" - - finished = false - obj_tid.on_signal("Package") do |line1,line2,line3| - logger.debug " update package: #{line2}" - end - - obj_tid.on_signal("Error") do |u1,u2| - finished = true - ok = false - end - obj_tid.on_signal("Finished") do |u1,u2| - finished = true - end - obj_tid_with_iface.UpdatePackages([updateId]) - - unless finished - @main = MainPkg.new - @main << system_bus - if (!@main.run) - ok = false - end - end - obj_with_iface.SuggestDaemonQuit - - return ok - end - - -#-------------------------------------------------------------------------------- -# -# actions -# -#-------------------------------------------------------------------------------- - - - # GET /patch_updates # GET /patch_updates.xml def index # note: permission check was performed in :before_filter - - @patch_updates = get_updateList + @patch_updates = Patch.find(:available) end # GET /patch_updates/1 @@ -208,7 +53,7 @@ class PatchesController < ApplicationController unless permission_check( "org.opensuse.yast.system.patches.read") render ErrorResult.error(403, 1, "no permission") and return end - @patch_update = get_update params[:id] + @patch_update = Patch.find(params[:id]) if @patch_update.nil? logger.error "Patch: #{params[:id]} not found." render ErrorResult.error(404, 1, "Patch: #{params[:id]} not found.") and return @@ -221,12 +66,12 @@ class PatchesController < ApplicationController unless permission_check( "org.opensuse.yast.system.patches.install") render ErrorResult.error(403, 1, "no permission") and return end - @patch_update = get_update(params[:id]) + @patch_update = Patch.find(params[:id]) if @patch_update.nil? logger.error "Patch: #{params[:id]} not found." render ErrorResult.error(404, 1, "Patch: #{params[:id]} not found.") and return end - unless install_update params[:id] + unless @patch_update.install render ErrorResult.error(404, 2, "packagekit error") and return end render :show diff --git a/plugins/patches/app/models/patch.rb b/plugins/patches/app/models/patch.rb index 76088d4..93fb6bb 100644 --- a/plugins/patches/app/models/patch.rb +++ b/plugins/patches/app/models/patch.rb @@ -1,3 +1,48 @@ +require "dbus" +require 'socket' +require 'thread' + +# = MainPkg event loop class. +# +# Class that takes care of handling message and signal events +# asynchronously. +class MainPkg + # Create a new main event loop. + def initialize + @buses = Hash.new + end + + # Add a _bus_ to the list of buses to watch for events. + def <<(bus) + @buses[bus.socket] = bus + end + + # Run the main loop. This is a blocking call! + def run + ok = true + finished = false + while !finished do + ready, dum, dum = IO.select(@buses.keys) + ready.each do |socket| + b = @buses[socket] + b.update_buffer + while m = b.pop_message + b.process(m) + if m.member == "Finished" || m.member == "Errorcode" + finished = true + if m.member == "Error" + ok = false + end + end + end + end + end + return ok + end +end # class MainPkg + + +# Model for patches available via package kit class Patch attr_accessor :resolvable_id, @@ -15,14 +60,22 @@ class Patch @resolvable_id = id_val end - def initialize (resolvable_id, kind, name, - arch, repo, summary) - @resolvable_id = resolvable_id - @kind = kind - @name = name - @arch = arch - @repo = repo - @summary = summary + # returns the modification time of + # the patch status, which you can use + # for cache policy purposes + def self.mtime + # we look for the most recent (max) modification time + # of either the package database or libzypp cache files + [ File.stat("/var/lib/rpm/Packages").mtime, + File.stat("/var/cache/zypp/solv").mtime, + * Dir["/var/cache/zypp/solv/*/solv"].map{ |x| File.stat(x).mtime } ].max + end + + # default constructor + def initialize(attributes) + attributes.each do |key, value| + instance_variable_set("@#{key}", value) + end end def to_xml( options = {} ) @@ -45,4 +98,121 @@ class Patch return hash.to_json end + # find patches + # Patch.find(:available) + # Patch.find(212) + def self.find(what) + if what == :available + patch_updates = [] + system_bus = DBus::SystemBus.instance + package_kit = system_bus.service("org.freedesktop.PackageKit") + obj = package_kit.object("/org/freedesktop/PackageKit") + #logger.debug obj.inspect + obj.introspect + obj_with_iface = obj["org.freedesktop.PackageKit"] + tid = obj_with_iface.GetTid + obj_tid = package_kit.object(tid[0]) + obj_tid.introspect + obj_tid_with_iface = obj_tid["org.freedesktop.PackageKit.Transaction"] + obj_tid.default_iface = "org.freedesktop.PackageKit.Transaction" + + finished = false + obj_tid.on_signal("Package") do |line1,line2,line3| + columns = line2.split ";" + update = Patch.new(:resolvable_id => columns[1], + :kind => line1, + :name => columns[0], + :arch => columns[2], + :repo => columns[3], + :summary => line3 ) + patch_updates << update + finished = true + end + + obj_tid.on_signal("Error") do |u1,u2| + finished = true + end + obj_tid.on_signal("Finished") do |u1,u2| + finished = true + end + obj_tid_with_iface.GetUpdates("NONE") + + unless finished + @main = MainPkg.new + @main << system_bus + @main.run + end + + obj_with_iface.SuggestDaemonQuit + + return patch_updates + else + # try to find by id + self.find(:available).find { |p| p.resolvable_id.to_s == what.to_s } + end + end + + # installs this + def install + self.class.install(id) + end + + # Patch.install(patch) + # Patch.install(id) + def self.install(patch) + if patch.is_a?(Patch) + update_id = "#{patch.name};#{patch.resolvable_id};#{patch.arch};#{@patch.repo}" + Rails.logger.debug "Install Update: #{update_id}" + self.package_kit_install(update_id) + else + # if is not an object, assume it is an id + patch_id = patch + patch = Patch.find(patch_id) + raise "Can't install update #{patch_id} because it does not exist" if patch.nil? or not patch.is_a?(Patch) + self.install(patch) + end + end + + # install an update, based on the PackageKit + # id + def self.package_kit_install(pkkit_id) + ok = true + system_bus = DBus::SystemBus.instance + package_kit = system_bus.service("org.freedesktop.PackageKit") + obj = package_kit.object("/org/freedesktop/PackageKit") + obj.introspect + obj_with_iface = obj["org.freedesktop.PackageKit"] + tid = obj_with_iface.GetTid + obj_tid = package_kit.object(tid[0]) + obj_tid.introspect + obj_tid_with_iface = obj_tid["org.freedesktop.PackageKit.Transaction"] + obj_tid.default_iface = "org.freedesktop.PackageKit.Transaction" + + finished = false + obj_tid.on_signal("Package") do |line1,line2,line3| + Rails.logger.debug " update package: #{line2}" + end + + obj_tid.on_signal("Error") do |u1,u2| + finished = true + ok = false + end + obj_tid.on_signal("Finished") do |u1,u2| + finished = true + end + obj_tid_with_iface.UpdatePackages([pkkit_id]) + + unless finished + @main = MainPkg.new + @main << system_bus + if (!@main.run) + ok = false + end + end + obj_with_iface.SuggestDaemonQuit + + return ok + end + + end diff --git a/plugins/patches/test/functional/patches_controller_test.rb b/plugins/patches/test/functional/patches_controller_test.rb index 699a93a..24052ea 100644 --- a/plugins/patches/test/functional/patches_controller_test.rb +++ b/plugins/patches/test/functional/patches_controller_test.rb @@ -12,7 +12,32 @@ class PatchesControllerTest < ActionController::TestCase @request = ActionController::TestRequest.new # http://railsforum.com/viewtopic.php?id=1719 @request.session[:account_id] = 1 # defined in fixtures - PatchesController.any_instance.stubs(:get_updateList).with().returns([Patch.new("462","important","softwaremgmt","noarch","openSUSE-11.0-Updates","Various fixes for the software management stack")]) + + # we make them member variables so we can use + # in other assertions + @p1 = Patch.new( + :resolvable_id => "462", + :name => "softwaremgmt", + :kind => "important", + :summary => "Various fixes for the software management stack", + :arch => "noarch", + :repo => "openSUSE-11.1-Updates") + + @p2 = Patch.new( + :resolvable_id => "463", + :name => "yast", + :kind => "security", + :summary => "Various fixes", + :arch => "noarch", + :repo => "openSUSE-11.1-Updates") + + Patch.stubs(:mtime).returns(Time.now) + Patch.stubs(:find).with(:available).returns([@p1, @p2]) + Patch.stubs(:find).with('462').returns(@p1) + Patch.stubs(:find).with('wrong_id').returns(nil) + Patch.stubs(:find).with('not_found').returns(nil) + + @p1.stubs(:install).returns(true) end test "access index" do @@ -59,13 +84,11 @@ class PatchesControllerTest < ActionController::TestCase end test "installing a patch" do - PatchesController.any_instance.stubs(:install_update).with("462").returns(true) put :update, :id =>"462" assert_response :success end test "installing a patch with wrong ID" do - PatchesController.any_instance.stubs(:install_update).with("462").returns(true) put :update, :id =>"wrong_id" assert_response 404 end diff --git a/plugins/patches/test/unit/patch_test.rb b/plugins/patches/test/unit/patch_test.rb new file mode 100644 index 0000000..50ed227 --- /dev/null +++ b/plugins/patches/test/unit/patch_test.rb @@ -0,0 +1,18 @@ + +require 'test_helper' +require 'patch' + +class PatchTest < ActiveSupport::TestCase + + def setup + Patch.stubs(:mtime).returns(Time.now) + end + + def test_available_patches + patches = Patch.find(:available) + assert_equal(1, patches.size) + + patch = Patch.find(patches.first.id) + end + +end -- To unsubscribe, e-mail: yast-commit+unsubscribe@opensuse.org For additional commands, e-mail: yast-commit+help@opensuse.org