From de78c11893d7c25f9faaf4bcb0ef10a9df1cd96e Mon Sep 17 00:00:00 2001
From: Josh Elser <elserj@apache.org>
Date: Tue, 2 May 2017 11:05:09 -0400
Subject: [PATCH] HBASE-17973 Fix incorrect method call in list_regions and
 expand tests (addendum)

---
 .../main/ruby/shell/commands/list_regions.rb  | 22 ++++++++++++++-----
 .../hbase/list_regions_test_no_cluster.rb     | 18 +++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hbase-shell/src/main/ruby/shell/commands/list_regions.rb b/hbase-shell/src/main/ruby/shell/commands/list_regions.rb
index 94b7a29fbd6..892653ee86f 100644
--- a/hbase-shell/src/main/ruby/shell/commands/list_regions.rb
+++ b/hbase-shell/src/main/ruby/shell/commands/list_regions.rb
@@ -49,13 +49,11 @@ EOF
         hregion_locator_instance = conn_instance.getRegionLocator(TableName.valueOf(table_name))
         hregion_locator_list = hregion_locator_instance.getAllRegionLocations().to_a
         results = Array.new
+        desired_server_name = options[SERVER_NAME]
 
         begin
           # Filter out region servers which we don't want, default to all RS
-          regions = hregion_locator_list.filter do |hregion|
-            server_name = options[SERVER_NAME] || '*'
-            accept_server_name? server_name, hregion.getServerName().toString()
-          end
+          regions = get_regions_for_table_and_server(table_name, conn_instance, desired_server_name)
           # A locality threshold of "1.0" would be all regions (cannot have greater than 1 locality)
           # Regions which have a `dataLocality` less-than-or-equal to this value are accepted
           locality_threshold = 1.0
@@ -101,8 +99,22 @@ EOF
         value >= 0 and value <= 1.0
       end
 
+      def get_regions_for_table_and_server(table_name, conn, server_name)
+        get_regions_for_server(get_regions_for_table(table_name, conn), server_name)
+      end
+
+      def get_regions_for_server(regions_for_table, server_name)
+        regions_for_table.select do |hregion|
+          accept_server_name? server_name, hregion.getServerName().toString()
+        end
+      end
+
+      def get_regions_for_table(table_name, conn)
+        conn.getRegionLocator(TableName.valueOf(table_name)).getAllRegionLocations().to_a
+      end
+
       def accept_server_name?(desired_server_name, actual_server_name)
-        desired_server_name.eql? '*' or actual_server_name.start_with? desired_server_name
+        desired_server_name.nil? or actual_server_name.start_with? desired_server_name
       end
 
       def accept_region_for_locality?(actual_locality, locality_threshold)
diff --git a/hbase-shell/src/test/ruby/hbase/list_regions_test_no_cluster.rb b/hbase-shell/src/test/ruby/hbase/list_regions_test_no_cluster.rb
index 657b1bd944c..b7a14b647b1 100644
--- a/hbase-shell/src/test/ruby/hbase/list_regions_test_no_cluster.rb
+++ b/hbase-shell/src/test/ruby/hbase/list_regions_test_no_cluster.rb
@@ -20,6 +20,10 @@ require 'hbase_constants'
 
 include HBaseConstants
 
+java_import 'org.apache.hadoop.hbase.HRegionLocation'
+java_import 'org.apache.hadoop.hbase.HRegionInfo'
+java_import 'org.apache.hadoop.hbase.ServerName'
+
 module Hbase
   class NoClusterListRegionsTest < Test::Unit::TestCase
     include TestHelpers
@@ -57,5 +61,19 @@ module Hbase
       assert_equal false, command.accept_region_for_locality?(1.0, 0.999)
       assert_equal false, command.accept_region_for_locality?(0.5, 0.3)
     end
+
+    define_test 'filter nondesired servers' do
+      command = ::Shell::Commands::ListRegions.new(nil)
+      server1 = create_region_location('server1,16020,1234')
+      server2 = create_region_location('server2,16020,1234')
+      server3 = create_region_location('server3,16020,1234')
+      assert_equal [server2], command.get_regions_for_server([server1, server2, server3], 'server2')
+      assert_equal [server3], command.get_regions_for_server([server1, server2, server3], 'server3')
+      assert_equal [], command.get_regions_for_server([server1, server2, server3], 'server5')
+    end
+
+    def create_region_location(server_name)
+      HRegionLocation.new(HRegionInfo.new(TableName.valueOf('t1')), ServerName.valueOf(server_name))
+    end
   end
 end