From b4b3ec9ac07dad1aab6da1d3ba9c26c4b7959dc2 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Fri, 18 Nov 2011 22:45:58 +0000
Subject: [PATCH] admin: add numeric conversions for consistency

Ruby isn't Perl, Ruby won't automatically coerce between
Strings and numeric types (Integer and Floats)
---
 lib/mogilefs/admin.rb             | 58 +++++++++++++++++++++++----------------
 test/test_fresh.rb                |  4 ++-
 test/test_mogilefs_integration.rb | 25 +++++++++++++++++
 3 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/lib/mogilefs/admin.rb b/lib/mogilefs/admin.rb
index eeb58ba..926b6cf 100644
--- a/lib/mogilefs/admin.rb
+++ b/lib/mogilefs/admin.rb
@@ -28,18 +28,18 @@ class MogileFS::Admin < MogileFS::Client
   # Returns:
   #
   #   [{"status"=>"alive",
-  #     "http_get_port"=>"",
+  #     "http_get_port"=>nil,
   #     "http_port"=>"",
   #     "hostid"=>"1",
   #     "hostip"=>"",
   #     "hostname"=>"rur-1",
-  #     "remoteroot"=>"/mnt/mogilefs/rur-1",
   #     "altip"=>"",
   #     "altmask"=>""}]
 
   def get_hosts(hostid = nil)
+    to_i = { "hostid" => true, "http_port" => true, "http_get_port" => true }
     clean('hosts', 'host',
-          @backend.get_hosts(hostid ? { :hostid => hostid } : {}))
+          @backend.get_hosts(hostid ? { :hostid => hostid } : {}), true, to_i)
   end
 
   ##
@@ -51,16 +51,25 @@ class MogileFS::Admin < MogileFS::Client
   # Returns:
   #
   #   [{"status"=>"alive",
-  #     "mb_asof"=>"",
-  #     "mb_free"=>"0",
-  #     "devid"=>"1",
-  #     "hostid"=>"1",
-  #     "mb_used"=>"",
-  #     "mb_total"=>""}]
+  #     "mb_asof"=>nil,
+  #     "mb_free"=>666000,
+  #     "devid"=>1,
+  #     "hostid"=>1,
+  #     "mb_used"=>666,
+  #     "mb_total"=>666666}]
 
   def get_devices(devid = nil)
-    clean('devices', 'dev',
-          @backend.get_devices(devid ? { :devid => devid } : {}))
+    to_i = {
+      "mb_asof" => true, "mb_free" => true,
+      "mb_used" => true, "mb_total" => true ,
+      "devid" => true, "weight" => true, "hostid" => true
+    }
+    rv = @backend.get_devices(devid ? { :devid => devid } : {})
+    rv = clean('devices', 'dev', rv, true, to_i)
+    rv.each do |row|
+      u = row["utilization"] and
+        row["utilization"] = nil == u ? nil : u.to_f
+    end
   end
 
   ##
@@ -70,17 +79,17 @@ class MogileFS::Admin < MogileFS::Client
   #
   # Returns:
   #
-  #   [{"fid"=>"99",
+  #   [{"fid"=>99,
   #     "class"=>"normal",
   #     "domain"=>"test",
-  #     "devcount"=>"2",
-  #     "length"=>"4",
+  #     "devcount"=>2,
+  #     "length"=>4,
   #     "key"=>"file_key"},
-  #    {"fid"=>"82",
+  #    {"fid"=>82,
   #     "class"=>"normal",
-  #     "devcount"=>"2",
+  #     "devcount"=>2,
   #     "domain"=>"test",
-  #     "length"=>"9",
+  #     "length"=>9,
   #     "key"=>"new_new_key"}]
 
   def list_fids(from_fid, to_fid)
@@ -153,13 +162,13 @@ class MogileFS::Admin < MogileFS::Client
     have_replpolicy = false
 
     domains = {}
+    to_i = { "mindevcount" => true }
     (1..res['domains'].to_i).each do |i|
-      domain = clean "domain#{i}classes", "domain#{i}class", res, false
+      domain = clean "domain#{i}classes", "domain#{i}class", res, false, to_i
 
-      tmp = domains[res["domain#{i}"]] = {}
+      tmp = domains[res["domain#{i}"].freeze] = {}
       domain.each do |d|
-        tmp[d.delete("name")] = d
-        d["mindevcount"] = d["mindevcount"].to_i
+        tmp[d.delete("name").freeze] = d
         have_replpolicy ||= d.include?("replpolicy")
       end
     end
@@ -329,15 +338,16 @@ class MogileFS::Admin < MogileFS::Client
   #     "altmask"=>""}]
 
   def clean(count, prefix, res, underscore = true, to_i = [])
-    underscore = underscore ? '_' : ''
+    empty = ""
+    underscore = underscore ? '_' : empty
     keys = res.keys
     (1..res[count].to_i).map do |i|
       re = /^#{prefix}#{i}#{underscore}/
       row = {}
       keys.grep(re).each do |k|
         v = res[k]
-        k = k.sub(re, "").freeze
-        row[k] = to_i.include?(k) ? v.to_i : v
+        k = k.sub(re, empty).freeze
+        row[k] = to_i.include?(k) ? (empty == v ? nil : v.to_i) : v
       end
       row
     end
diff --git a/test/test_fresh.rb b/test/test_fresh.rb
index 4afed06..8c2ef53 100644
--- a/test/test_fresh.rb
+++ b/test/test_fresh.rb
@@ -88,8 +88,10 @@ EOF
     assert_equal 1, hosts.size
     host = @admin.get_hosts[0]
     assert_equal "me", host["hostname"]
-    assert_equal @mogstored_http_port, host["http_port"].to_i
+    assert_equal @mogstored_http_port, host["http_port"]
+    assert_nil host["http_get_port"]
     assert_equal @test_host, host["hostip"]
+    assert_kind_of Integer, host["hostid"]
     assert_equal hosts, @admin.get_hosts(host["hostid"])
 
     assert_equal [], @admin.get_devices
diff --git a/test/test_mogilefs_integration.rb b/test/test_mogilefs_integration.rb
index e42185a..610b899 100644
--- a/test/test_mogilefs_integration.rb
+++ b/test/test_mogilefs_integration.rb
@@ -121,4 +121,29 @@ class TestMogileFSIntegration < TestMogIntegration
     end
     assert_equal 4, rv
   end
+
+  def test_admin_get_devices
+    admin = MogileFS::Admin.new(:hosts => @trackers)
+    devices = admin.get_devices
+    if any_device = devices[0]
+      %w(mb_asof mb_free mb_used mb_total devid weight hostid).each do |field|
+        case value = any_device[field]
+        when nil
+        when Integer
+          assert value >= 0, "#{field}=#{value.inspect} is negative"
+        else
+          assert false, "#{field}=#{value.inspect} is #{value.class}"
+        end
+      end
+
+      field = "utilization"
+      case value = any_device[field]
+      when nil
+      when Float
+        assert value >= 0.0, "#{field}=#{value.inspect} is negative"
+      else
+        assert false, "#{field}=#{value.inspect} is #{value.class}"
+      end
+    end
+  end
 end
-- 
2.11.4.GIT