Fixed a bug with AutoScaling scheduled actions.

The `AWS::AutoScaling::ScheduledAction` class incorrectly assumed that their
group names were unique and suffient to identify the action.  In order to correct
this bug, it was required to add a new positional required argument to the
`ScheduledAction` constructor.  This affects two code paths.

First, the scheduled action collection must now be scoped by a group:

    # now raises an error, must scope to a group first
    auto_scaling.scheduled_actions['name']

    # this works
    auto_scaling.groups['group-name'].scheduled_actions['name']

    # this also works
    auto_scaling.scheduled_actions.filter(:group => 'group-name')['name']

Also the constructor now requires a group:

    # no longer works, raises error
    AWS::AutoScaling::ScheduledAction.new('name')

    # this works
    group = AWS::AutoScaling::Group.new('group-name')
    AWS::AutoScaling::ScheduledAction.new(group, 'name')

lib/aws/auto_scaling/scheduled_action.rb

@@ -16,8 +16,6 @@
 module AWS
   class AutoScaling
 
-    # @attr_reader [String] auto_scaling_group_name
-    #
     # @attr_reader [Integer] desired_capacity
     #
     # @attr_reader [String] recurrence
@@ -35,17 +33,23 @@ class AutoScaling
     class ScheduledAction < Core::Resource
 
       # @api private
-      def initialize name, options = {}
+      def initialize group, name, options = {}
+        @group = group
         @name = name
         super
       end
 
       # @return [String]
       attr_reader :name
 
-      attribute :arn, :from => :scheduled_action_arn
+      attr_reader :group
 
-      attribute :auto_scaling_group_name, :static => true
+      # @return [String]
+      def auto_scaling_group_name
+        group.name
+      end
+
+      attribute :arn, :from => :scheduled_action_arn
 
       attribute :desired_capacity
 
@@ -65,11 +69,6 @@ def initialize name, options = {}
         end
       end
 
-      # @return [Group]
-      def group
-        Group.new(auto_scaling_group_name, :config => config)
-      end
-
       # Updates the scheduled action.  If you omit an option,
       # the corresponding value remains unchanged in the Auto
       # Scaling group.
@@ -91,52 +90,49 @@ def group
       # @return [nil]
       #
       def update options = {}
-
-        client_opts = options.dup
-        client_opts[:scheduled_action_name] = name
-        client_opts[:auto_scaling_group_name] = auto_scaling_group_name
-
-        # convert these options to timestamps
+        options.update(resource_options)
+        # convert times to formatted strings
         [:start_time, :end_time].each do |opt|
-          if client_opts[opt].is_a?(Time)
-            client_opts[opt] = client_opts[opt].iso8601
+          if options[opt].is_a?(Time)
+            options[opt] = options[opt].iso8601
           end
         end
-
-        client.put_scheduled_update_group_action(client_opts)
-
+        client.put_scheduled_update_group_action(options)
         nil
-
       end
       alias_method :put, :update
 
       # @return [Boolean]
       def exists?
         client_opts = {}
         client_opts[:scheduled_action_names] = [name]
+        client_opts[:auto_scaling_group_name] = auto_scaling_group_name
         resp = client.describe_scheduled_actions(client_opts)
         !resp.scheduled_update_group_actions.empty?
+      rescue Errors::ValidationError
+        false
       end
 
       # Deletes the current scheduled action.
       # @return [nil]
       def delete
-        client_opts = {}
-        client_opts[:scheduled_action_name] = name
-        client_opts[:auto_scaling_group_name] = auto_scaling_group_name
-        client.delete_scheduled_action(client_opts)
+        client.delete_scheduled_action(resource_options)
         nil
       end
 
       protected
 
       def resource_identifiers
-        [[:name, name]]
+        [
+          [:auto_scaling_group_name, auto_scaling_group_name],
+          [:scheduled_action_name, name],
+        ]
       end
 
       def get_resource attr_name = nil
         client_opts = {}
         client_opts[:scheduled_action_names] = [name]
+        client_opts[:auto_scaling_group_name] = auto_scaling_group_name
         client.describe_scheduled_actions(client_opts)
       end
 

lib/aws/auto_scaling/scheduled_action_collection.rb

@@ -62,8 +62,10 @@ def initialize options = {}
       #
       def create name, options = {}
 
-        scheduled_action = ScheduledAction.new(name,
-          :auto_scaling_group_name => auto_scaling_group_name_opt(options),
+        group = auto_scaling_group(options)
+
+        scheduled_action = ScheduledAction.new(group, name,
+          :auto_scaling_group_name => group.name,
           :config => config)
 
         scheduled_action.update(options)
@@ -75,12 +77,14 @@ def create name, options = {}
       # @param [String] name The name of the scheduled action.
       # @return [ScheduledAction]
       def [] name
-        options = {}
-        options[:config] = config
-        if group = @filters[:auto_scaling_group_name]
-          options[:auto_scaling_group_name] = group
+        if group_name = @filters[:auto_scaling_group_name]
+          group = Group.new(group_name, :config => config)
+          ScheduledAction.new(group, name)
+        else
+          msg = 'uou must filter this collection by a group to get a ' +
+            'scheduled action by name'
+          raise msg
         end
-        ScheduledAction.new(name, options)
       end
 
       # Returns a new {ScheduledActionCollection} filtered
@@ -132,11 +136,11 @@ def filter filters = {}
 
       protected
 
-      def auto_scaling_group_name_opt options
+      def auto_scaling_group(options)
 
         group = options.delete(:group)
         group ||= @filters[:auto_scaling_group_name]
-        group = group.name if group.is_a?(Group)
+        group = Group.new(group, :config => config) if group.is_a?(String)
 
         unless group
           raise ArgumentError, 'missing required option :group'
@@ -176,9 +180,12 @@ def _each_item next_token, limit, options = {}, &block
         resp = client.describe_scheduled_actions(options.merge(@filters))
         resp.scheduled_update_group_actions.each do |details|
 
+          group = Group.new(details[:auto_scaling_group_name], :config => config)
+
           scheduled_action = ScheduledAction.new_from(
             :describe_scheduled_actions,
             details,
+            group,
             details.scheduled_action_name,
             :config => config)
 

spec/aws/auto_scaling/scheduled_action_collection_spec.rb

@@ -83,12 +83,16 @@ class AutoScaling
       context '#[]' do
 
         it 'returns a scheduled action' do
-          action = actions['name']
+          action = actions.filter(:group => 'group-name')['name']
           action.should be_a(ScheduledAction)
           action.name.should == 'name'
           action.config.should == config
         end
 
+        it 'raises an error if the collection is not filtered by a group' do
+          lambda { actions['name'] }.should raise_error
+        end
+
       end
 
       context '#filter' do
@@ -155,7 +159,10 @@ def stub_next_token(response, token)
 
         def stub_n_members response, n
           response.data[:scheduled_update_group_actions] = (1..n).map{|i|
-            { :scheduled_action_name => i.to_s }
+            {
+              :scheduled_action_name => i.to_s,
+              :auto_scaling_group_name => 'group-name',
+            }
           }
         end
 

spec/aws/auto_scaling/scheduled_action_spec.rb

@@ -21,7 +21,9 @@ class AutoScaling
 
       let(:client) { config.auto_scaling_client }
 
-      let(:action) { ScheduledAction.new('name', :config => config) }
+      let(:group) { Group.new('group', :config => config) }
+
+      let(:action) { ScheduledAction.new(group, 'name') }
 
       let(:response) { client.stub_for(:describe_scheduled_actions) }
 
@@ -47,7 +49,7 @@ class AutoScaling
       context '#name' do
 
         it 'is set in the constructor' do
-          ScheduledAction.new('name').name.should == 'name'
+          ScheduledAction.new(group, 'name').name.should == 'name'
         end
 
       end
@@ -108,9 +110,10 @@ class AutoScaling
         let(:resp) { client.stub_for(:describe_scheduled_actions) }
 
         before(:each) do
-          client.should_receive(:describe_scheduled_actions).
-            with(:scheduled_action_names => [action.name]).
-            and_return(resp)
+          client.should_receive(:describe_scheduled_actions).with(
+            :scheduled_action_names => [action.name],
+            :auto_scaling_group_name => 'group'
+          ).and_return(resp)
         end
 
         it 'returns true if it can be described' do
@@ -129,6 +132,7 @@ class AutoScaling
 
         it 'calls #delete_scheduled_action on the client' do
           client.should_receive(:delete_scheduled_action).with(
+            :auto_scaling_group_name => 'group',
             :scheduled_action_name => action.name,
             :auto_scaling_group_name => 'group')
           action.delete