How To Move Logic From Controller Into Model?

2019-07-21 11:42发布

问题:

A pool has many addresses. Want to create multiple address records based on a submitted range.

I have this logic in my addresses_controller:

  def create
    @pool = Pool.find(params[:pool_id])

    unless address_params[:ipv4_range_start].blank? || address_params[:ipv4_range_stop].blank?
      (address_params[:ipv4_range_start]..address_params[:ipv4_range_stop]).each do |octet|
        params[:address][:ipv4_octet3] = octet
        @address =  @pool.addresses.build(address_params)
        if !@address.save
           render 'new'
        end
      end
      redirect_to pool_path(@pool), notice: "Address range created."

    else #something was missing
      @address = @pool.addresses.build(address_params)
      @address.errors.add_on_blank(:ipv4_range_start)
      @address.errors.add_on_blank(:ipv4_range_stop)
      render 'new'
    end
  end

Wondering how I might move this into the Address model? Seems like too much for the controller but I can't figure out how to iterate through the submitted range and build and save each address from the Address model itself.

Thanks for any suggestions!

Jim

回答1:

I think your intuition is correct that we can move a lot of this into the model.

Disclaimer: None of this code is tested; copying it and pasting it directly into your code will probably end in tears.

So there are two parts of your logic we need to deal with. The first is making sure :ipv4_range_start and _stop are provided. For that we can use a validation. Since it seems like you don't want those attributes to be required for all Addresses, we can use the :on option to supply a validation context. (More info on contexts here.):

# Address model
validates_presence_of :ipv4_range_start, :ipv4_range_stop,
                      on: :require_range

That on: :require_range part means this validation won't usually run—it'll only run when we tell ActiveRecord to use the :require_range context.

Now we can do this in the controller:

# AddressesController
def create
  @pool = Pool.find(params[:pool_id])

  @address = @pool.addresses.build(address_params)
  if @address.invalid?(:require_range)
    render 'new' and return
  end

  # ...
end

This accomplishes the same thing as the code in your else block, but the real logic is in the model, and Rails populates the errors object for us.

Now that we have that out of the way, we can deal with creating the objects. For that we can write a class method in Address. The great thing about class methods in Rails models is that they're automatically available on association collections, so for example if we define an Address.foo class method, we get @pool.addresses.foo for free. Here's a class method that will create an array of Addresses:

# Address model
def self.create_from_range!(attrs)
  start = attrs.fetch(:ipv4_range_start)
  stop = attrs.fetch(:ipv4_range_stop)

  self.transaction do
    (start..stop).map do |octet|
      self.create!(attrs.merge ipv4_octet3: octet)
    end
  end
end

This is pretty much the same as your else block, just a little cleaner. We do self.create! in a transaction, so that if any of the create!s fails they'll all be rolled back. We do that inside a map block instead of each so that, assuming no error occurs, the method will return an array of the created objects.

Now we just have to use it in our controller:

def create
  # Our code from before
  @pool = Pool.find(params[:pool_id])

  @address = @pool.addresses.build(address_params)
  if @address.invalid?(:require_range)
    render 'new' and return
  end

  # The new code
  begin
    @pool.addresses.create_from_range!(address_params)
  rescue ActiveRecord::ActiveRecordError
    flash[:error] = "Address range not created!"
    render 'new' and return
  end

  redirect_to @pool, notice: "Address range created."
end

As you can see, we used @pool.addresses.create_from_range!, so the association (Address#pool_id) will be filled in for us. If we wanted we could assign the returned array to an instance variable and show the created records in the view.

That should be about all you need.

P.S. One thing worth noting is that Ruby has a great built-in IPAddr class, and because an IP address is just a number (e.g. 3338456716 is the decimal form of 198.252.206.140) it may serve you to store each IP address as a single 4-byte integer instead of four separate columns. Most databases have useful built-in functions for dealing with IP addresses (PostgreSQL actually has a built-in inet column type for this) as well. It really depends on your use case, however, and at this point such an optimization may very well be premature. Cheers!