[hbase] 02/04: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace (#2232)

[bu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

busbey pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git

commit 29bbc7de45058343d55a16ed8b4da2c558992d7b
Author: Elliot <el...@apple.com>
AuthorDate: Tue Aug 18 16:14:34 2020 -0400

    HBASE-24806 Small Updates to Functionality of Shell IRB Workspace (#2232)
    
    * HBASE-24806 Small Updates to Functionality of Shell IRB Workspace
    
    - Move exception handler from Shell::Shell#eval_io to new method,
      Shell::Shell#exception_handler
    - Add unit tests for Shell::Shell#exception_handler
    - Change Shell::Shell#eval_io to no longer raise SystemExit when any error is
      seen and update unit test
    - Update ruby test runner to catch SystemExit and fail to avoid tests that
      cause the test runner to incorrectly exit successfully
    - Add Hbase::Loader module to find ruby scripts in the $LOAD_PATH and classpath
      using JRuby's loader.
    - In hbase-shell, install IRB commands before exporting HBase commands. The
      HBase commands will override the IRB commands, and no warning will be
      printed.
    
    * Remove unused variables from shell_test
    
    Signed-off-by: Nick Dimiduk <nd...@apache.org>
    Signed-off-by: stack <st...@apache.org>
    (cherry picked from commit 98e35842ebeb9a419df37a1c3166b4468b85a6bc)
---
 bin/hirb.rb                                     |  8 +++-
 hbase-shell/src/main/ruby/shell.rb              | 36 ++++++++++------
 hbase-shell/src/main/ruby/shell/hbase_loader.rb | 56 +++++++++++++++++++++++++
 hbase-shell/src/test/ruby/shell/shell_test.rb   | 24 ++++++++++-
 hbase-shell/src/test/ruby/tests_runner.rb       | 11 +++--
 5 files changed, 115 insertions(+), 20 deletions(-)

diff --git a/bin/hirb.rb b/bin/hirb.rb
index 5b502f4..7b1b8f1 100644
--- a/bin/hirb.rb
+++ b/bin/hirb.rb
@@ -174,16 +174,20 @@ def debug?
   nil
 end
 
+
 # For backwards compatibility, this will export all the HBase shell commands, constants, and
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-@shell.eval_io(File.new(script2run)) if script2run
+require 'shell/hbase_loader'
+if script2run
+  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+end
 
 # If we are not running interactively, evaluate standard input
-@shell.eval_io(STDIN) unless interactive
+::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
 
 if interactive
   # Output a banner message that tells users where to go for help
diff --git a/hbase-shell/src/main/ruby/shell.rb b/hbase-shell/src/main/ruby/shell.rb
index b6cc8b0..96b7fe2 100644
--- a/hbase-shell/src/main/ruby/shell.rb
+++ b/hbase-shell/src/main/ruby/shell.rb
@@ -297,11 +297,11 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
       return @irb_workspace unless @irb_workspace.nil?
 
       hbase_receiver = HBaseReceiver.new
-      # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
-      # must be BEFORE the irb commands are installed so that our help command is not overwritten.
-      export_all(hbase_receiver)
       # install all the IRB commands onto our receiver
       IRB::ExtendCommandBundle.extend_object(hbase_receiver)
+      # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
+      # will override names that conflict with IRB methods like "help".
+      export_all(hbase_receiver)
       ::IRB::WorkSpace.new(hbase_receiver.get_binding)
     end
 
@@ -311,7 +311,10 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
     # Unlike Ruby's require or load, this method allows us to execute code with a custom binding. In
     # this case, we are using the binding constructed with all the HBase shell constants and
     # methods.
-    def eval_io(io)
+    #
+    # @param [IO] io instance of Ruby's IO (or subclass like File) to read script from
+    # @param [String] filename to print in tracebacks
+    def eval_io(io, filename = 'stdin')
       require 'irb/ruby-lex'
       # Mixing HBaseIOExtensions into IO allows us to pass IO objects to RubyLex.
       IO.include HBaseIOExtensions
@@ -320,10 +323,20 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))
+      end
+      nil
+    end
+
+    ##
+    # Runs a block and logs exception from both Ruby and Java, optionally discarding the traceback
+    #
+    # @param [Boolean] hide_traceback if true, Exceptions will be converted to
+    #   a SystemExit so that the traceback is not printed
+    def self.exception_handler(hide_traceback)
       begin
-        scanner.each_top_level_statement do |statement, linenum|
-          puts(workspace.evaluate(nil, statement, 'stdin', linenum))
-        end
+        yield
       rescue Exception => e
         message = e.to_s
         # exception unwrapping in shell means we'll have to handle Java exceptions
@@ -335,12 +348,9 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
         # Include the 'ERROR' string to try to make transition easier for scripts that
         # may have already been relying on grepping output.
         puts "ERROR #{e.class}: #{message}"
-        if $fullBacktrace
-          # re-raising the will include a backtrace and exit.
-          raise e
-        else
-          exit 1
-        end
+        raise e unless hide_traceback
+
+        exit 1
       end
       nil
     end
diff --git a/hbase-shell/src/main/ruby/shell/hbase_loader.rb b/hbase-shell/src/main/ruby/shell/hbase_loader.rb
new file mode 100644
index 0000000..2ad2ea9
--- /dev/null
+++ b/hbase-shell/src/main/ruby/shell/hbase_loader.rb
@@ -0,0 +1,56 @@
+#
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+module Hbase
+  ##
+  # HBase::Loader serves a similar purpose to IRB::IrbLoader, but with a different separation of
+  # concerns. This loader allows us to directly get the path for a filename in ruby's load path,
+  # and then use that in combination with something like HBase::Shell#eval_io.
+  module Loader
+    ##
+    # Determine the loadable path for a given filename by searching through $LOAD_PATH
+    #
+    # This serves a similar purpose to IRB::IrbLoader#search_file_from_ruby_path, but uses JRuby's
+    # loader, which allows us to find special paths like "uri:classloader" inside of a Jar.
+    #
+    # @param [String] filename
+    # @return [String] path
+    def self.path_for_load(filename)
+      return File.absolute_path(filename) if File.exist? filename
+
+      # Get JRuby's LoadService from the global (singleton) instance of the runtime
+      # (org.jruby.Ruby), which allows us to use JRuby's tools for searching the load path.
+      runtime = org.jruby.Ruby.getGlobalRuntime
+      loader = runtime.getLoadService
+      search_state = loader.findFileForLoad filename
+      raise LoadError, "no such file to load -- #{filename}" if search_state.library.nil?
+
+      search_state.loadName
+    end
+
+    ##
+    # Return a file handle for the given file found in the load path
+    #
+    # @param [String] filename
+    # @return [File] file handle
+    def self.file_for_load(filename)
+      File.new(path_for_load(filename))
+    end
+  end
+end
diff --git a/hbase-shell/src/test/ruby/shell/shell_test.rb b/hbase-shell/src/test/ruby/shell/shell_test.rb
index c8f6807..5d3b07a 100644
--- a/hbase-shell/src/test/ruby/shell/shell_test.rb
+++ b/hbase-shell/src/test/ruby/shell/shell_test.rb
@@ -114,6 +114,7 @@ class ShellTest < Test::Unit::TestCase
   #-----------------------------------------------------------------------------
 
   define_test 'Shell::Shell#eval_io should evaluate IO' do
+    StringIO.include HBaseIOExtensions
     # check that at least one of the commands is present while evaluating
     io = StringIO.new <<~EOF
       puts (respond_to? :list)
@@ -123,7 +124,7 @@ class ShellTest < Test::Unit::TestCase
 
     # check that at least one of the HBase constants is present while evaluating
     io = StringIO.new <<~EOF
-      ROWPREFIXFILTER
+      ROWPREFIXFILTER.dump
     EOF
     output = capture_stdout { @shell.eval_io(io) }
     assert_match(/"ROWPREFIXFILTER"/, output)
@@ -131,6 +132,25 @@ class ShellTest < Test::Unit::TestCase
 
   #-----------------------------------------------------------------------------
 
+  define_test 'Shell::Shell#exception_handler should hide traceback' do
+    class TestException < RuntimeError; end
+    # When hide_traceback is true, exception_handler should replace exceptions
+    # with SystemExit so that the traceback is not printed.
+    assert_raises(SystemExit) do
+      ::Shell::Shell.exception_handler(true) { raise TestException, 'Custom Exception' }
+    end
+  end
+
+  define_test 'Shell::Shell#exception_handler should show traceback' do
+    class TestException < RuntimeError; end
+    # When hide_traceback is false, exception_handler should re-raise Exceptions
+    assert_raises(TestException) do
+      ::Shell::Shell.exception_handler(false) { raise TestException, 'Custom Exception' }
+    end
+  end
+
+  #-----------------------------------------------------------------------------
+
   define_test 'Shell::Shell#print_banner should display Reference Guide link' do
     @shell.interactive = true
     output = capture_stdout { @shell.print_banner }
@@ -141,7 +161,7 @@ class ShellTest < Test::Unit::TestCase
 
   #-----------------------------------------------------------------------------
 
-  define_test "Shell::Shell interactive mode should not throw" do
+  define_test 'Shell::Shell interactive mode should not throw' do
     # incorrect number of arguments
     @shell.command('create', 'nothrow_table')
     @shell.command('create', 'nothrow_table', 'family_1')
diff --git a/hbase-shell/src/test/ruby/tests_runner.rb b/hbase-shell/src/test/ruby/tests_runner.rb
index 73d4a6e..b0a0aaf 100644
--- a/hbase-shell/src/test/ruby/tests_runner.rb
+++ b/hbase-shell/src/test/ruby/tests_runner.rb
@@ -82,9 +82,14 @@ if java.lang.System.get_property('shell.test')
   puts "Only running tests that match #{shell_test_pattern}"
   runner_args << "--testcase=#{shell_test_pattern}"
 end
-# first couple of args are to match the defaults, so we can pass options to limit the tests run
-if !(Test::Unit::AutoRunner.run(false, nil, runner_args))
-  raise "Shell unit tests failed. Check output file for details."
+begin
+  # first couple of args are to match the defaults, so we can pass options to limit the tests run
+  unless Test::Unit::AutoRunner.run(false, nil, runner_args)
+    raise 'Shell unit tests failed. Check output file for details.'
+  end
+rescue SystemExit => e
+  # Unit tests should not raise uncaught SystemExit exceptions. This could cause tests to be ignored.
+  raise 'Caught SystemExit during unit test execution! Check output file for details.'
 end
 
 puts "Done with tests! Shutting down the cluster..."