ManageIQ/manageiq — pull request #81 by Fryguy (2014-06-24 18:27:15+00:00)
Title: Fix UI vm summary (part 1)

@dclarizio @h-kataria Please review. The first commit is the meat of the PR...the rest is cleanup and style.

https://bugzilla.redhat.com/show_bug.cgi?id=1086015

In reviewing the code, I found that the method `rbac_filtered_objects` filters
the object it is given against RBAC. Then, in order to remove "empty"
folders, it does a follow up pass.

However, the follow up pass was done by calling `process_show_list` which
calls into `get_view` to get the data for rendering the right side of the UI.
So basically, we are going through A LOT of code paths, particularly A LOT of
UI specific checks just to ask a simple RBAC question. It's almost the same
as someone clicking every folder in the explorer just to see if anything
appears on the right hand side! This PR completely removes that entire path
and just goes straight to RBAC. I did not fix the tree-walking nor the
expensive "check RBAC at every node".

For testing, the setup is very difficult. I was able to reproduce this in a
local test, but had to go against a real database which cannot be made public.
Eventually, we can make a real test case out of this with fake data, but I
want to get this in as it is a critical issue. Note that since this is a
spec, and not a real UI transaction, it doesn't get the benefit of Rails Query
Caching, so it may actually be worse that through the UI.

Performance wise, the spec went from 33.2s to 26.4s. This is about a 20%
improvement.

---

The test I used is as follows:

```ruby
describe VmInfraController do
it "#x_build_dynatree" do
expected =
[{:key=>"e-1r1",
:expand=>true,
:title=>"EMS A",
:tooltip=>"EMS A",
:icon=>"vendor-vmwarews.png",
:children=>
[{:key=>"d-1r226",
:expand=>true,
:title=>"Datacenter 1",
:tooltip=>"Datacenter 1",
:icon=>"datacenter.png",
:children=>
[{:key=>"f-1r166",
:expand=>true,
:title=>"Folder A",
:tooltip=>"Folder A",
:icon=>"blue_folder.png",
:children=>
[{:key=>"v-1r154",
:expand=>true,
:title=>"vm1",
:icon=>"off.png"},
...
}]

controller.instance_variable_set(:@sb, :trees => {:vandt_tree => {:active_node => nil, :open_nodes => []}})
# The following have to be enabled for "before" testing, because of all the UI code it passes through.
# controller.instance_variable_set(:@settings, :views => {:vmortemplate => "list"}, :perpage => {:list => 5})
# controller.instance_variable_set(:@request, :controller => "vm_infra")
# controller.stub(:request => double(:parameters => {:controller => "vm_infra"}))

session[:userid] = User.current_userid = "whomever"

actual = controller.send(:x_build_dynatree, :type => :vandt, :tree => :vandt_tree, :open_all => true)

pp actual
expected.should == actual
end
end
```