[Ovirt-devel] [PATCH server 2/2] * app/controllers/host_controller.rb: factor biz logic out
David Lutterkort
lutter at redhat.com
Tue May 5 19:03:46 UTC 2009
The business logic is now in HostService.
The main functional difference is that svc_list filters the host list by
permission.
---
src/app/controllers/host_controller.rb | 125 +++++++++++++-------------------
src/app/services/host_service.rb | 119 ++++++++++++++++++++++++++++++
2 files changed, 168 insertions(+), 76 deletions(-)
create mode 100644 src/app/services/host_service.rb
diff --git a/src/app/controllers/host_controller.rb b/src/app/controllers/host_controller.rb
index 994b5e2..9b683fa 100644
--- a/src/app/controllers/host_controller.rb
+++ b/src/app/controllers/host_controller.rb
@@ -19,8 +19,13 @@
class HostController < ApplicationController
- EQ_ATTRIBUTES = [ :state, :arch, :hostname, :uuid,
- :hardware_pool_id ]
+ include HostService
+
+ # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
+ verify :method => [:post, :put], :only => [ :create, :update ],
+ :redirect_to => { :action => :list }
+ verify :method => [:post, :delete], :only => :destroy,
+ :redirect_to => { :action => :list }
def index
list
@@ -30,42 +35,21 @@ class HostController < ApplicationController
end
end
- before_filter :pre_action, :only => [:host_action, :enable, :disable, :clear_vms, :edit_network]
- before_filter :pre_addhost, :only => [:addhost]
-
- # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
- verify :method => [:post, :put], :only => [ :create, :update ],
- :redirect_to => { :action => :list }
- verify :method => [:post, :delete], :only => :destroy,
- :redirect_to => { :action => :list }
-
- def list
- conditions = []
- EQ_ATTRIBUTES.each do |attr|
- if params[attr]
- conditions << "#{attr} = :#{attr}"
- end
- end
- @hosts = Host.find(:all,
- :conditions => [conditions.join(" and "), params],
- :order => "id")
- end
-
+ def list
+ svc_list(params)
+ end
def show
- if authorize_view
- respond_to do |format|
- format.html { render :layout => 'selection' }
- format.xml { render :xml => @host.to_xml(:include => [ :cpus ] ) }
- end
+ svc_show(params[:id])
+ respond_to do |format|
+ format.html { render :layout => 'selection' }
+ format.xml { render :xml => @host.to_xml(:include => [ :cpus ] ) }
end
end
def quick_summary
- pre_show
- if authorize_view
- render :layout => false
- end
+ svc_show(id)
+ render :layout => false
end
# retrieves data used by snapshot graphs
@@ -73,15 +57,17 @@ class HostController < ApplicationController
end
def addhost
- @hardware_pool = Pool.find(params[:hardware_pool_id])
- render :layout => 'popup'
- end
+ # FIXME: This probably should go into PoolService.svc_modify,
+ # so that we have permission checks in only one place
- def pre_addhost
+ # Old pre_addhost
@pool = Pool.find(params[:hardware_pool_id])
@parent = @pool.parent
set_perms(@pool)
authorize_admin
+ # Old addhost
+ @hardware_pool = Pool.find(params[:hardware_pool_id])
+ render :layout => 'popup'
end
def add_to_smart_pool
@@ -89,6 +75,11 @@ class HostController < ApplicationController
render :layout => 'popup'
end
+ # FIXME: We implement the standard controller actions, but catch
+ # them in filters and kick out friendly warnings that you can't
+ # perform them on hosts. Tat's overkill - the only way for a user
+ # to get to these actions is with URL surgery or from a bug in the
+ # application, both of which deserve a noisy error
def new
end
@@ -116,52 +107,40 @@ class HostController < ApplicationController
end
def disable
- set_disabled(1)
+ svc_enable(params[:id], "disabled")
+ render :json => {
+ :object => :host,
+ :alert => "Host was successfully disabled",
+ :success => true
+ }
end
+
def enable
- set_disabled(0)
- end
-
- def set_disabled(value)
- operation = value == 1 ? "disabled" : "enabled"
- begin
- @host.is_disabled = value
- @host.save!
- @json_hash[:alert]="Host was successfully #{operation}"
- @json_hash[:success]=true
- rescue
- @json_hash[:alert]="Error setting host to #{operation}"
- @json_hash[:success]=false
- end
- render :json => @json_hash
+ svc_enable(params[:id], "enabled")
+ render :json => {
+ :object => :host,
+ :alert => "Host was successfully enabled",
+ :success => true
+ }
end
def clear_vms
- begin
- Host.transaction do
- task = HostTask.new({ :user => get_login_user,
- :task_target => @host,
- :action => HostTask::ACTION_CLEAR_VMS,
- :state => Task::STATE_QUEUED})
- task.save!
- @host.is_disabled = true
- @host.save!
- end
- @json_hash[:alert]="Clear VMs action was successfully queued."
- @json_hash[:success]=true
- rescue
- @json_hash[:alert]="Error in queueing Clear VMs action."
- @json_hash[:success]=false
- end
- render :json => @json_hash
+ svc_clear_vms(params[:id])
+ render :json => {
+ :object => :host,
+ :alert => "Clear VMs action was successfully queued.",
+ :success => true
+ }
end
def edit_network
+ svc_modify(params[:id])
render :layout => 'popup'
end
def bondings_json
- bondings = Host.find(params[:id]).bondings
+ svc_show(params[:id])
+ bondings = @host.bondings
render :json => bondings.collect{ |x| {:id => x.id, :name => x.name} }
end
@@ -180,12 +159,6 @@ class HostController < ApplicationController
flash[:notice] = 'Hosts may not be edited via the web UI'
redirect_to :action=> 'show', :id => @host
end
- def pre_action
- @host = Host.find(params[:id])
- set_perms(@host.hardware_pool)
- @json_hash = { :object => :host }
- authorize_admin
- end
def pre_show
@host = Host.find(params[:id])
set_perms(@host.hardware_pool)
diff --git a/src/app/services/host_service.rb b/src/app/services/host_service.rb
new file mode 100644
index 0000000..074d830
--- /dev/null
+++ b/src/app/services/host_service.rb
@@ -0,0 +1,119 @@
+#--
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by David Lutterkort <lutter at redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301, USA. A copy of the GNU General Public License is
+# also available at http://www.gnu.org/copyleft/gpl.html.
+#++
+# Business logic around hosts aka nodes
+module HostService
+
+ include ApplicationService
+
+ # Host attributes on which we filter with '='
+ EQ_ATTRIBUTES = [ :state, :arch, :hostname, :uuid,
+ :hardware_pool_id ]
+
+ # List hosts matching criteria described by +params+; only the entries
+ # for keys in +EQ_ATTRIBUTES+ are used
+ #
+ # === Instance variables
+ # [<tt>@hosts</tt>] stores list of hosts matching criteria
+ # === Required permissions
+ # [<tt>Privilege::VIEW</tt>] no exception raised, <tt>@hosts</tt>
+ # is filtered by privilege
+ def svc_list(params)
+ conditions = []
+ EQ_ATTRIBUTES.each do |attr|
+ if params[attr]
+ conditions << "hosts.#{attr} = :#{attr}"
+ end
+ end
+ # Add permission check
+ params = params.dup
+ params[:user] = get_login_user
+ params[:priv] = Privilege::VIEW
+ conditions << "privileges.name=:priv"
+ conditions << "permissions.uid=:user"
+ incl = [{ :hardware_pool => { :permissions => { :role => :privileges}}}]
+ @hosts = Host.find(:all,
+ :include => incl,
+ :conditions => [conditions.join(" and "), params],
+ :order => "hosts.id")
+ end
+
+ # Load the Host with +id+ for viewing
+ #
+ # === Instance variables
+ # [<tt>@host</tt>] stores the Host with +id+
+ # === Required permissions
+ # [<tt>Privilege::VIEW</tt>] on host's HardwarePool
+ def svc_show(id)
+ lookup(id, Privilege::VIEW)
+ end
+
+ # Load the Host with +id+ for editing
+ #
+ # === Instance variables
+ # [<tt>@host</tt>] stores the Host with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+ def svc_modify(id)
+ lookup(id, Privilege::MODIFY)
+ end
+
+ # Set the disabled state of the Host with +id+ to <tt>:enabled</tt>
+ # or <tt>:disabled</tt>
+ #
+ # === Instance variables
+ # [<tt>@host</tt>] stores the Host with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+ def svc_enable(id, state)
+ ind = ["enabled", "disabled"].index(state.to_s)
+ if ind.nil?
+ raise ArgumentError, "STATE must be 'enabled' or 'disabled'"
+ end
+ svc_modify(id)
+ @host.is_disabled = ind
+ @host.save!
+ end
+
+ # Queue task to migrate all VM's off the Host with +id+, and mark the
+ # host as disabled
+ #
+ # === Instance variables
+ # [<tt>@host</tt>] stores the Host with +id+
+ # === Required permissions
+ # [<tt>Privilege::MODIFY</tt>] on host's HardwarePool
+ def svc_clear_vms(id)
+ svc_modify(id)
+ Host.transaction do
+ task = HostTask.new({ :user => get_login_user,
+ :task_target => @host,
+ :action => HostTask::ACTION_CLEAR_VMS,
+ :state => Task::STATE_QUEUED})
+ task.save!
+ @host.is_disabled = true
+ @host.save!
+ end
+ end
+
+ private
+ def lookup(id, priv)
+ @host = Host.find(id)
+ authorized!(priv, @host.hardware_pool)
+ end
+end
--
1.6.0.6
More information about the ovirt-devel
mailing list