Added Filter to fix Rails API resource id usage

app/controllers/tim/base_images_controller.rb

@@ -2,7 +2,8 @@
 
 module Tim
   class BaseImagesController < Tim::ApplicationController
-    append_before_filter :set_template_xml, :only => [:create, :update]
+    prepend_before_filter ResourceLinkFilter.new({ :base_image => :template }),
+                :only => [:create]
 
     def index
       @base_images = Tim::BaseImage.all unless defined? @base_images
@@ -46,14 +47,5 @@ def destroy
       respond_with(@base_image)
     end
 
-    private
-    # Handles the cases when the template xml is supplied within request
-    def set_template_xml
-      doc = ::Nokogiri::XML request.body.read
-      if !doc.xpath("//base_image/template").empty?
-        params[:base_image][:template] = { :xml => doc.xpath("//base_image/template").children.to_s}
-      end
-    end
-
   end
 end

app/controllers/tim/image_versions_controller.rb

@@ -1,5 +1,7 @@
 module Tim
   class ImageVersionsController < Tim::ApplicationController
+    prepend_before_filter ResourceLinkFilter.new({ :image_version => :base_image }),
+                :only => [:create]
 
     def index
       @image_versions = Tim::ImageVersion.all unless defined? @image_versions

app/controllers/tim/provider_images_controller.rb

@@ -2,6 +2,8 @@ module Tim
   class ProviderImagesController < Tim::ApplicationController
     respond_to :json, :only => :update
 
+    prepend_before_filter ResourceLinkFilter.new({ :provider_image => :target_image }),
+                :only => [:create]
     before_filter :factory_keys, :only => :update
 
     def index

app/controllers/tim/target_images_controller.rb

@@ -2,6 +2,8 @@ module Tim
   class TargetImagesController < Tim::ApplicationController
     respond_to :json, :only => :update
 
+    prepend_before_filter ResourceLinkFilter.new({ :target_image => :image_version }),
+                :only => [:create]
     before_filter :factory_keys, :only => :update
 
     def index

app/filters/tim/resource_link_filter.rb

@@ -0,0 +1,56 @@
+# Before filter for controllers that handle REST API requests. Conductor uses
+# linking to resources like <pool id='1'/> (resulting in :pool => { :id => 1 }
+# in request params hash) but ActiveRecord expects linking to resources like
+# <pool_id>1</pool_id> (resluting in :pool_id => 1 in request params hash).
+#
+# This before filter operates on request params hash to rewrite it from our
+# format to ActiveRecord-friendly format.
+#
+# The constructor of the filter accepts specification of what should be
+# transformed. E.g.:
+#
+# before_filter ResourceLinkFilter.new({ :catalog => :pool }),
+# :only => [:create, :update]
+#
+# would transform { :catalog => { :pool => { :id => 1 }}}
+# into { :catalog => { :pool_id => 1 }}
+#
+# See the specs in spec/util/resource_link_filter_spec.rb for more examples.
+#
+
+# NOTE This filter was taken from the aeolus conductor project: https://github.com/aeolusproject/conductor/
+
+class Tim::ResourceLinkFilter
+  def initialize(resource_links)
+    @resource_links = resource_links
+  end
+
+  def before(controller)
+    return unless controller.request.format == :xml
+
+    transform_resource_links_recursively(controller.params, @resource_links)
+  end
+
+
+  private
+
+  def transform_resource_links_recursively(subparams, sublinks)
+    return if subparams == nil
+
+    case sublinks
+    when Symbol # then transform the link (last level of recursion)
+      return if subparams[sublinks] == nil || subparams[sublinks][:id] == nil
+
+      subparams[:"#{sublinks}_id"] = subparams[sublinks][:id]
+      subparams.delete(sublinks)
+    when Array # then process each item
+      sublinks.each do |item|
+        transform_resource_links_recursively(subparams, item)
+      end
+    when Hash # then descend into each entry
+      sublinks.each_key do |key|
+        transform_resource_links_recursively(subparams[key], sublinks[key])
+      end
+    end
+  end
+end

spec/controllers/base_images_controller_spec.rb

@@ -35,6 +35,19 @@ module Tim
           end
         end
 
+          it "should return a new base image with an existing template" do
+            template = FactoryGirl.create(:template)
+            post :create, { :base_image => { :name => "Name",
+                                             :description => "Description",
+                                             :template => {:id =>  template.id}}}
+            response.code.should == "201"
+
+            body = Hash.from_xml(response.body)
+            body.keys.should  == ["base_image"]
+            body["base_image"].keys.should =~ ["template", "id", "href", "name", "description", "image_versions"]
+            body["base_image"]["template"].keys.should =~ ["id", "href"]
+          end
+
         context "failure" do
           it "should return a unprocessable entity error when the client sends invalid content" do
             post :create, { :invalid_image => FactoryGirl.build(:base_image).attributes }

spec/controllers/image_versions_controller_spec.rb

@@ -31,6 +31,17 @@ module Tim
             body["image_version"].keys.should =~ ["base_image", "id", "href", "target_images"]
             body["image_version"]["base_image"].keys.should =~ ["id", "href"]
           end
+
+          it "should return a new image version with existing base image" do
+            base_image = FactoryGirl.create(:base_image)
+            post :create, { :image_version => {:base_image => {:id => base_image.id}}}
+            response.code.should == "201"
+
+            body = Hash.from_xml(response.body)
+            body.keys.should  == ["image_version"]
+            body["image_version"].keys.should =~ ["base_image", "id", "href", "target_images"]
+            body["image_version"]["base_image"].keys.should =~ ["id", "href"]
+          end
         end
 
         context "failure" do

spec/controllers/provider_images_controller_spec.rb

@@ -33,6 +33,18 @@ module Tim
               "snapshot", "status_detail", "progress", "href", "id","target_image", "status"]
             body["provider_image"]["target_image"]["id"].should == provider_image.target_image .id.to_s
           end
+
+          it "should return a new provider image awith existing target image" do
+            target_image = FactoryGirl.create(:target_image)
+            post :create, { :provider_image => { :target_image => { :id => target_image.id }}}
+            response.code.should == "201"
+
+            body = Hash.from_xml(response.body)
+            body.keys.should  == ["provider_image"]
+            body["provider_image"].keys.should  =~ ["external_image_id", "provider",
+              "snapshot", "status_detail", "progress", "href", "id","target_image", "status"]
+            body["provider_image"]["target_image"]["id"].should == target_image .id.to_s
+          end
         end
 
         context "failure" do

spec/controllers/target_images_controller_spec.rb

@@ -31,6 +31,17 @@ module Tim
             body["target_image"].keys.should  =~ ["target", "status", "status_detail", "progress", "href", "id", "provider_images", "image_version"]
             body["target_image"]["image_version"]["id"].should == target_image.image_version.id.to_s
           end
+
+          it "should create a new target image as with existing image version" do
+            image_version = FactoryGirl.create(:image_version)
+            post :create, { :target_image => {:image_version => {:id => image_version.id}, :target => "mock"}}
+            response.code.should == "201"
+
+            body = Hash.from_xml(response.body)
+            body.keys.should  == ["target_image"]
+            body["target_image"].keys.should  =~ ["target", "status", "status_detail", "progress", "href", "id", "provider_images", "image_version"]
+            body["target_image"]["image_version"]["id"].should == image_version.id.to_s
+          end
         end
 
         context "failure" do

spec/filters/resource_link_filter_spec.rb

@@ -0,0 +1,158 @@
+require 'spec_helper'
+
+module Tim
+  describe ResourceLinkFilter do
+
+    describe "#transform_resource_links" do
+
+      before do
+        @controller = stub({
+          :request => stub({
+            :format => :xml
+          })
+        })
+        @controller.stub(:params).and_return(params)
+      end
+
+      context "successful transformation" do
+
+        before do
+          ResourceLinkFilter.new(resource_links).before(@controller)
+        end
+
+        context "for a simple link" do
+          let(:params) do {
+              :catalog => {
+                :id => 123
+              }
+            }
+          end
+          let(:resource_links) { :catalog }
+
+          it "transforms the link" do
+            params[:catalog].should == nil
+            params[:catalog_id].should == 123
+          end
+        end
+
+        context "for a nested link" do
+          let(:params) do
+            {
+              :catalog => {
+                :pool => {
+                  :id => 123
+                }
+              }
+            }
+          end
+          let(:resource_links) { { :catalog => :pool } }
+
+          it "transforms the link" do
+            params[:catalog][:pool].should == nil
+            params[:catalog][:pool_id].should == 123
+          end
+        end
+
+        context "for multiple nested links" do
+          let(:params) do
+            {
+              :catalog => {
+                :pool => {
+                  :id => 123
+                },
+                :something => {
+                  :id => 456
+                }
+              }
+            }
+          end
+          let(:resource_links) { { :catalog => [:pool, :something] } }
+
+          it "transforms the links" do
+            params[:catalog][:pool].should == nil
+            params[:catalog][:pool_id].should == 123
+            params[:catalog][:something].should == nil
+            params[:catalog][:something_id].should == 456
+          end
+        end
+
+        context "for combined single- and double-nested links" do
+          let(:params) do
+            {
+              :catalog => {
+                :pool => {
+                  :id => 123
+                },
+                :something => {
+                  :double_nested_1 => {
+                    :id => 456
+                  },
+                  :double_nested_2 => {
+                    :id => 789
+                  }
+                }
+              }
+            }
+          end
+          let(:resource_links) do
+            {
+              :catalog => [
+                :pool,
+                { :something => [:double_nested_1, :double_nested_2] }
+              ]
+            }
+          end
+
+          it "transforms the links" do
+            params[:catalog][:pool].should == nil
+            params[:catalog][:pool_id].should == 123
+            params[:catalog][:something][:double_nested_1].should == nil
+            params[:catalog][:something][:double_nested_1_id].should == 456
+            params[:catalog][:something][:double_nested_2].should == nil
+            params[:catalog][:something][:double_nested_2_id].should == 789
+          end
+        end
+
+      end
+
+      context "when missing 'id' attribute in the link" do
+        let(:params) do
+          {
+            :catalog => {}
+          }
+        end
+        let(:resource_links) { :catalog }
+
+        it "does not transform anything" do
+          params[:catalog].should == {}
+          params[:catalog_id].should == nil
+        end
+      end
+
+      context "when missing the whole link" do
+        let(:params) { {} }
+        let(:resource_links) { :catalog }
+
+        it "does not transform anything" do
+          params[:catalog].should == nil
+          params[:catalog_id].should == nil
+        end
+      end
+
+      context "when not XML request" do
+        let(:params) { {} }
+
+        before do
+          @controller.stub_chain(:request, :format).and_return(:html)
+        end
+
+        it "does not touch params" do
+          @controller.should_not_receive(:params)
+          ResourceLinkFilter.new(:something).before(@controller)
+        end
+      end
+
+    end
+
+  end
+end