Veraticus / Dynamoid

Ruby ORM for Amazon's DynamoDB
http://joshsymonds.com/Dynamoid/
247 stars 83 forks source link

Yield should test if there's a block to yield to in persistence/update #160

Open lisad opened 11 years ago

lisad commented 11 years ago

In lib / dynamoid / persistence.rb, line 185, there's a "yield t". There's no check to see if there's a block to yield to. If there's no block passed in, this fails with

LocalJumpError: no block given (yield)

    def update!(conditions = {}, &block)
      run_callbacks(:update) do
        options = range_key ? {:range_key => dump_field(self.read_attribute(range_key), self.class.attributes[range_key])} : {}
        new_attrs = Dynamoid::Adapter.update_item(self.class.table_name, self.hash_key, options.merge(:conditions => conditions)) do |t|
          if(self.class.attributes[:lock_version])
            raise "Optimistic locking cannot be used with Partitioning" if(Dynamoid::Config.partitioning)
            t.add(lock_version: 1)
          end

          yield t
        end
        load(new_attrs)
      end
    end

The way this code got run as "@event.update(event_params)" without a block was in some auto-generated Rails code in my project's EventController. Here's the controller method that was autogenerated.

  def update
    respond_to do |format|
      if @event.update(event_params)
        format.html { redirect_to @event, notice: 'Event was successfully updated.' }
        format.json { head :no_content }
      else
        format.html { render action: 'edit' }
        format.json { render json: @event.errors, status: :unprocessable_entity }
      end
    end
  end

It's easy enough for me to do update differently, I don't want to do it the boilerplate way anyway. However, I thought I'd point out what's going on here, and that others might run into the same issue pretty fast if they're following Rails standard practices.

tbtalbottjr commented 10 years ago

I'm assuming the #update in Dynamoid is a different beast than the #update in Rails 4.0.x ActiveRecord and it is a name collision. Dynamoid still expects #update_attributes.

This should be corrected for better Rails 4.0.x support.