On Thu, 2013-03-28 at 12:19 +1100, Koper, Dies wrote: > A bit of background on this patch: > It contains my latest work on implementing cimi systems and system > templates for mock and fgcp. > It is not complete and contains some debugging statements. Its main > purpose is to show you what I have and what issues I'm seeing. > > It applies to master (fcffad13e66175152cf8a43d615b79727902b5ee) and > requires "[PATCH] CIMI schema: tolerate nil hash_map attributes" to > solve a nil error for system templates with fgcp. > > Issues I'm having: > > (1) > With mock, even unsupported subcollections are shown when retrieving > systems. With fgcp they are also shown, and their href urls are broken:
This is a shortcoming of how we populate models right now - there's no way to suppress collections that the driver doesn't support. In fact, the code goes through great length to make sure we generate the href for empty collections (since that href is needed to add the first element to an empty collection) The easiest way to 'fix' this for now is to comment those unsupported collections out in system.rb. A better fix is to add a mechanism similar to what we do for $select to models; call that the 'exclude' mechanism. The various generate_xxx methods should set the that exclude parameter based on collections that are declared in Rabbit, but not available (Rabbit knows that, Michal: how do we get that info out of Rabbit ?) Similar to how CIMI::Model::Resource has @select_attrs it should also have @exclude_attrs that are consulted in prepare and cause the corresponding collection to be set to nil (which will remove it from the XML/JSON output) > (2) > With mock (not with fgcp), when I list a system's volumes, its id is not > generated correctly: > > d:\projects>curl --user mockuser:mockpassword > http://localhost:3001/cimi/systems/system2/volumes?format=xml > <Collection xmlns="http://schemas.dmtf.org/cimi/1"; > resourceURI="http://schemas.dmtf.org/cimi/1/SystemVolumeCollection";> > <id>http://localhost:3001/cimi/system/system2/volumes</id> > <count>1</count> > <SystemVolume> > <id>http://localhost:3001/cimi/volumes?id=sysvol1</id> > ... > > That should be http://localhost:3001/cimi/volumes/sysvol1. > Note my comment in mock_driver_cimi_methods.rb#system_volumes about the > 1st arg I pass to convert_cimi_mock_urls: > > #FIXME: with ":volumes", delete url becomes > 'http://localhost:3001/cimi/volumes?id=sysvol1' > #with ":system_volume" or ":system_volumes", undefined method > `system_volume_url' for #<CIMI::Collections::Systems:0x44fe338> in > mock_driver_cimi_methods.rb:261 > volumes.map{|vol|convert_cimi_mock_urls(:volumes, vol, > opts[:env])}.flatten This seems to be a Rabbit problem with the names of the subcollections; somehow URL helpers for the subcollections do not get generated correctly. Michal ? BTW, attached is a patch that addresses a few URL conversion problems I found in the mock driver; feel free to commit if it doesn't make things worse ;) David
>From c06a92816926fa24780721241be03ff507fa95e9 Mon Sep 17 00:00:00 2001 From: David Lutterkort <[email protected]> Date: Fri, 15 Mar 2013 15:53:41 -0700 Subject: [PATCH 1/2] Mock driver: convert all stub URL's There were cases where we didn't catch stubbed out URL's and reported invalid references with the mock driver. This fixes that and simplifies the URL conversion code. --- .../mock/data/cimi/forwarding_group/group1.json | 6 +- .../drivers/mock/mock_driver_cimi_methods.rb | 111 +++++++++++---------- 2 files changed, 60 insertions(+), 57 deletions(-) diff --git a/server/lib/deltacloud/drivers/mock/data/cimi/forwarding_group/group1.json b/server/lib/deltacloud/drivers/mock/data/cimi/forwarding_group/group1.json index e10e0bd..04f9266 100644 --- a/server/lib/deltacloud/drivers/mock/data/cimi/forwarding_group/group1.json +++ b/server/lib/deltacloud/drivers/mock/data/cimi/forwarding_group/group1.json @@ -1,4 +1,4 @@ -{ "id": "http://cimi.example.org/routing_groups/group1";, +{ "id": "http://cimi.example.org/forwarding_groups/group1";, "name": "group1", "description": "a mock routing group", "created": "Thu Jan 12 16:02:56 EET 2012", @@ -7,7 +7,7 @@ { "href": "http://cimi.example.org/networks/network2"} ], "operations": [ - { "rel": "edit", "href": "http://cimi.example.org/routing_groups/group1"; }, - { "rel": "delete", "href": "http://cimi.example.org/routing_groups/group1"; } + { "rel": "edit", "href": "http://cimi.example.org/forwarding_groups/group1"; }, + { "rel": "delete", "href": "http://cimi.example.org/forwarding_groups/group1"; } ] } diff --git a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb index 9a6daa2..b0d4f9a 100644 --- a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb +++ b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb @@ -32,7 +32,7 @@ module Deltacloud::Drivers::Mock return [] end end - systems.map{|sys|convert_cimi_mock_urls(:system, sys ,opts[:env])}.flatten + systems.map{|sys| convert_urls(sys, opts[:env])}.flatten end def create_system(credentials, opts={}) @@ -155,17 +155,17 @@ module Deltacloud::Drivers::Mock return [] end end - system_templates.map{|sys_templ|convert_cimi_mock_urls(:system_template, sys_templ, opts[:env])}.flatten + system_templates.map{|sys_templ| convert_urls(sys_templ, opts[:env])}.flatten end def networks(credentials, opts={}) check_credentials(credentials) if opts[:id].nil? networks = @client.load_all_cimi(:network).map{|net| CIMI::Model::Network.from_json(net)} - networks.map{|net|convert_cimi_mock_urls(:network, net ,opts[:env])}.flatten + networks.map{|net| convert_urls(net, opts[:env])}.flatten else network = CIMI::Model::Network.from_json(@client.load_cimi(:network, opts[:id])) - convert_cimi_mock_urls(:network, network, opts[:env]) + convert_urls(network, opts[:env]) end end @@ -215,10 +215,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? network_configs = @client.load_all_cimi(:network_configuration).map{|net_config| CIMI::Model::NetworkConfiguration.from_json(net_config)} - network_configs.map{|net_config|convert_cimi_mock_urls(:network_configuration, net_config, opts[:env])}.flatten + network_configs.map{|net_config| convert_urls(net_config, opts[:env])}.flatten else network_config = CIMI::Model::NetworkConfiguration.from_json(@client.load_cimi(:network_configuration, opts[:id])) - convert_cimi_mock_urls(:network_configuration, network_config, opts[:env]) + convert_urls(network_config, opts[:env]) end end @@ -226,10 +226,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? network_templates = @client.load_all_cimi(:network_template).map{|net_templ| CIMI::Model::NetworkTemplate.from_json(net_templ)} - network_templates.map{|net_templ|convert_cimi_mock_urls(:network_template, net_templ, opts[:env])}.flatten + network_templates.map{|net_templ| convert_urls(net_templ, opts[:env])}.flatten else network_template = CIMI::Model::NetworkTemplate.from_json(@client.load_cimi(:network_template, opts[:id])) - convert_cimi_mock_urls(:network_template, network_template, opts[:env]) + convert_urls(network_template, opts[:env]) end end @@ -237,10 +237,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? forwarding_groups = @client.load_all_cimi(:forwarding_group).map{|fg| CIMI::Model::ForwardingGroup.from_json(fg)} - forwarding_groups.map{|fg|convert_cimi_mock_urls(:forwarding_group, fg, opts[:env])}.flatten + forwarding_groups.map{|fg| convert_urls(fg, opts[:env])}.flatten else forwarding_group = CIMI::Model::ForwardingGroup.from_json(@client.load_cimi(:forwarding_group, opts[:id])) - convert_cimi_mock_urls(:forwarding_group, forwarding_group, opts[:env]) + convert_urls(forwarding_group, opts[:env]) end end @@ -248,10 +248,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? forwarding_group_templates = @client.load_all_cimi(:forwarding_group_template).map{|fg_templ| CIMI::Model::ForwardingGroupTemplate.from_json(fg_templ)} - forwarding_group_templates.map{|fg_templ|convert_cimi_mock_urls(:forwarding_group_template, fg_templ, opts[:env])}.flatten + forwarding_group_templates.map{|fg_templ| convert_urls(fg_templ, opts[:env])}.flatten else forwarding_group_template = CIMI::Model::ForwardingGroupTemplate.from_json(@client.load_cimi(:forwarding_group_template, opts[:id])) - convert_cimi_mock_urls(:forwarding_group_template, forwarding_group_template, opts[:env]) + convert_urls(forwarding_group_template, opts[:env]) end end @@ -259,10 +259,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? ports = @client.load_all_cimi(:network_port).map{|net_port| CIMI::Model::NetworkPort.from_json(net_port)} - ports.map{|net_port|convert_cimi_mock_urls(:network_port, net_port, opts[:env])}.flatten + ports.map{|net_port| convert_urls(net_port, opts[:env])}.flatten else port = CIMI::Model::NetworkPort.from_json(@client.load_cimi(:network_port, opts[:id])) - convert_cimi_mock_urls(:network_port, port, opts[:env]) + convert_urls(port, opts[:env]) end end @@ -270,10 +270,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? network_port_configurations = @client.load_all_cimi(:network_port_configuration).map{|network_port_config| CIMI::Model::NetworkPortConfiguration.from_json(network_port_config)} - network_port_configurations.map{|network_port_config|convert_cimi_mock_urls(:network_port_configuration, network_port_config, opts[:env])}.flatten + network_port_configurations.map{|network_port_config| convert_urls(network_port_config, opts[:env])}.flatten else network_port_configuration = CIMI::Model::NetworkPortConfiguration.from_json(@client.load_cimi(:network_port_configuration, opts[:id])) - convert_cimi_mock_urls(:network_port_configuration, network_port_configuration, opts[:env]) + convert_urls(network_port_configuration, opts[:env]) end end @@ -281,10 +281,10 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? network_port_templates = @client.load_all_cimi(:network_port_template).map{|net_port_templ| CIMI::Model::NetworkPortTemplate.from_json(net_port_templ)} - network_port_templates.map{|net_port_templ|convert_cimi_mock_urls(:network_port_template, net_port_templ, opts[:env])}.flatten + network_port_templates.map{|net_port_templ| convert_urls(net_port_templ, opts[:env])}.flatten else network_port_template = CIMI::Model::NetworkPortTemplate.from_json(@client.load_cimi(:network_port_template, opts[:id])) - convert_cimi_mock_urls(:network_port_template, network_port_template, opts[:env]) + convert_urls(network_port_template, opts[:env]) end end @@ -292,54 +292,57 @@ module Deltacloud::Drivers::Mock check_credentials(credentials) if opts[:id].nil? address_templates = @client.load_all_cimi(:address_template).map{|addr_templ| CIMI::Model::AddressTemplate.from_json(addr_templ)} - address_templates.map{|addr_templ|convert_cimi_mock_urls(:address_template, addr_templ, opts[:env])}.flatten + address_templates.map{|addr_templ| convert_urls(addr_templ, opts[:env])}.flatten else address_template = CIMI::Model::AddressTemplate.from_json(@client.load_cimi(:address_template, opts[:id])) - convert_cimi_mock_urls(:address_template, address_template, opts[:env]) + convert_urls(address_template, opts[:env]) end end private - def convert_cimi_mock_urls(model_name, cimi_object, context) - cimi_object.attribute_values.each do |k,v| - if ( v.is_a?(Struct) || ( v.is_a?(Array) && v.first.is_a?(Struct))) - case v - when Array - v.each do |item| - convert_struct_urls(item, k.to_s.singularize.to_sym, context) - end - else - opts = nil - if is_subcollection?(v, cimi_object.id) - opts = {:parent_model_name => model_name, :parent_item_name => cimi_object.name} - end - convert_struct_urls(v, k, context, opts) - end + # Convert all attributes that have values of the form + # http://cimi.example.org/COLL/ID + def convert_urls(val, context) + if val.nil? || val.is_a?(Fixnum) + # Nothing to do + elsif val.is_a?(Struct) + val.members.each { |m| val[m] = convert_urls(val[m], context) } + elsif val.is_a?(Hash) + val.keys.each { |m| val[m] = convert_urls(val[m], context) } + elsif val.is_a?(Array) + val.each_index { |i| val[i] = convert_urls(val[i], context) } + elsif val.is_a?(String) + val = rewrite_url(val, context) + elsif val.is_a?(CIMI::Model::Resource) + val.attribute_values.each do |k, v| + val[k] = convert_urls(val[k], context) end + else + # Need to add a branch for val.class + raise "Whoa ! #{val.inspect}" end - object_url = context.send(:"#{model_name}_url", cimi_object.name) - cimi_object.id=object_url - cimi_object.operations.each{|op| op.href=object_url } - cimi_object - end - - def is_subcollection?(struct, cimi_object_id) - return false if struct.href.nil? - struct.href.include?(cimi_object_id) + val end - def convert_struct_urls(struct, cimi_name, context, opts = nil) - return unless (struct.respond_to?(:href) && (not struct.href.nil?) && (not cimi_name == :operation )) - if opts - struct.href = context.send(:"#{opts[:parent_model_name]}_url", opts[:parent_item_name]) + "/#{cimi_name}" + # Rewrite URL assuming it points at a valid resource; if that's not + # possible, return +s+ + # + # URLs that should be rewritten need to be in the form + # http://cimi.example.org/COLLECTION/ID + def rewrite_url(s, context) + begin + u = URI.parse(s) + rescue URI::InvalidURIError + return s + end + return s unless u.scheme == 'http' && u.host == 'cimi.example.org' + _, coll, id = u.path.split("/") + method = "#{coll.singularize}_url" + if context.respond_to?(method) + context.send(method, id) else - obj_name = struct.href.split("/").last - if cimi_name.to_s.end_with?("config") - struct.href = context.send(:"#{cimi_name}uration_url", obj_name) - else - struct.href = context.send(:"#{cimi_name}_url", obj_name) - end + s end end -- 1.8.1.4
