user123
user123

Reputation: 163

How to end each loop after a particular condition in ruby

I have included given code

  def check_ip
    start_ip = IPAddr.new(myip).to_i
    end_ip = IPAddr.new(endip).to_i
    ip_pool = IpTab.all
    p '!!!!!!!!!!!!!!!!!!!!!!!'
    p ip_pool
    ip_pool.each do |ip|
     low = IPAddr.new(ip.start_ip).to_i
     high = IPAddr.new(ip.end_ip).to_i
     p '-------------------------------'
     p ((low..high)===start_ip)
     p ((low..high)===start_ip)
     p '******************************'
     break  if (low..high)===start_ip
     break  if (low..high)===end_ip
     p '*******************************'
     self.errors.add(:start_ip, I18n.t('errors.start_ip'))

    end
  end  

I get given output:

"!!!!!!!!!!!!!!!!!!!!!!!"
  IpPool Load (0.1ms)  SELECT `ip_pools`.* FROM `ip_pools`
#<ActiveRecord::Relation [#<IpPool id: 1, start_ip: "10.10.10.10", end_ip: "10.10.10.20", user_id: 1, created_at: "2015-09-08 05:12:34", updated_at: "2015-09-08 05:12:34">, #<IpPool id: 4, start_ip: "11.12.12.13", end_ip: "11.12.12.16", user_id: 1, created_at: "2015-09-08 06:08:38", updated_at: "2015-09-08 06:08:38">]>
"-------------------------------"
true
true
"******************************"

but its not working I want if my start_ip or end_ip lies between the given ips in database then it does not allow to save ip. i.e if (low..high)===start_ip or if (low..high)===end_ip is true it would not allow to save.
Guide me how to solve this as my code is not working guide me how to write this code.

Upvotes: 1

Views: 204

Answers (4)

Aleksey Shein
Aleksey Shein

Reputation: 7482

If I were you, I'd add a custom validation check to Ip model:

validate :validate_pool_existence!

def validate_pool_existence
  # DRY by taste
  errors.add(:start_ip, I18n.t('errors.start_ip')) if IpPool.contains_ip?(start_ip)
  errors.add(:end_ip, I18n.t('errors.end_ip')) if IpPool.contains_ip?(end_ip)
end

Now you need to add a couple of useful methods to IpPool to support ip range query:

# Check if any IpPool in database contains passed ip
# `all` usage maybe slow if you have lots of records in IpPool table
def self.contains_ip?(ip)
  all.any? { |pool| pool.contains?(ip) }
end

# Check if current IpPool contains passed ip
def contains_ip?(ip)
  to_addr.include? ip
end

# This returns an IPAddr range like this:
# #<IPAddr: IPv4:10.10.10.10/255.255.255.255>..#<IPAddr: IPv4:10.10.10.20/255.255.255.255>
def to_addr
  AddrIP.new(start_ip)..AddrIP.new(end_ip)
end

And that should be it. This works because IPAddr supports ranges and you can query if an ip is inside a range with simple include? check:

>> range = IPAddr.new('10.10.10.10')..IPAddr.new('10.10.10.20')
=> #<IPAddr: IPv4:10.10.10.10/255.255.255.255>..#<IPAddr: IPv4:10.10.10.20/255.255.255.255>
>> range.include? '10.10.10.15'
=> true
>> range.include? '10.10.10.25'
=> false

Enjoy! :)

Upvotes: 1

Stefan
Stefan

Reputation: 114248

if (low..high)===start_ip or if (low..high)===end_ip is true it would not allow to save

Your loop doesn't work that way. Let's assume (low..high)===start_ip is true. Your loop becomes:

ip_pool.each do |ip|
  low = IPAddr.new(ip.start_ip).to_i
  high = IPAddr.new(ip.end_ip).to_i
  break if true                                         # loop exits here
  break if (low..high)===end_ip                         # not called
  self.errors.add(:start_ip, I18n.t('errors.start_ip')) # not called either
end

If if (low..high)===end_ip is true, it becomes:

ip_pool.each do |ip|
  low = IPAddr.new(ip.start_ip).to_i
  high = IPAddr.new(ip.end_ip).to_i
  break if (low..high)===start_ip                       # nothing happens                
  break if true                                         # loop exits here
  self.errors.add(:start_ip, I18n.t('errors.start_ip')) # not called
end

Either way, self.errors.add is not called. It is however called if both conditions are false, probably not what you want.

To solve your problem, you could write:

ip_pool.each do |ip|
  low = IPAddr.new(ip.start_ip).to_i
  high = IPAddr.new(ip.end_ip).to_i

  if (low..high).include?(start_ip) || (low..high).include?(end_ip)
    errors.add(:start_ip, I18n.t('errors.start_ip'))
    break
  end
end

Or with separate errors:

ip_pool.each do |ip|
  low = IPAddr.new(ip.start_ip).to_i
  high = IPAddr.new(ip.end_ip).to_i

  if (low..high).include?(start_ip)
    errors.add(:start_ip, I18n.t('errors.start_ip'))
    break
  elsif (low..high).include?(end_ip)
    errors.add(:end_ip, I18n.t('errors.end_ip'))
    break
  end
end

Note that I've replaced rng===obj with rng.include?(obj).

Furthermore, I think your before_save :check_ip should be a validate :check_ip as shown in Performing Custom Validations. The documentation also shows how to implement an EachValidator that can be applied to multiple attributes.

Upvotes: 3

Alex Antonov
Alex Antonov

Reputation: 15226

There is a validates gem, which has IpValidator

You can just

validates :start_ip, ip: true

Upvotes: 1

Amit Badheka
Amit Badheka

Reputation: 2672

What you really need to do, is return false when the validation fails. I guess you should try this

    def check_ip
        start_ip = IPAddr.new(self.start_ip).to_i
        end_ip = IPAddr.new(self.end_ip).to_i
        ip_pool = IpPool.all
        p '!!!!!!!!!!!!!!!!!!!!!!!'
        p ip_pool
        begin
            ip_pool.each do |ip|
                low = IPAddr.new(ip.start_ip).to_i
                high = IPAddr.new(ip.end_ip).to_i
                p '-------------------------------'
                p ((low..high)===start_ip)
                p ((low..high)===start_ip)
                p '******************************'
                raise ArgumentError, I18n.t('errors.start_ip')  if ((low..high)===start_ip or (low..high)===end_ip
                p '*******************************'
            end
        rescue ArgumentError => msg
            p msg
            self.errors.add(:start_ip, msg)
            return false
        end
        return true
    end

Upvotes: 1

Related Questions