From 7c55ab090a952b90b79324fd163feb2bc99a0724 Mon Sep 17 00:00:00 2001
From: Alex Polcyn <apolcyn@google.com>
Date: Mon, 11 Jul 2016 23:14:32 -0700
Subject: [PATCH] removed unnecessary public methods removed tests of non api
 methods

---
 src/ruby/ext/grpc/rb_compression_options.c | 140 ++----------
 src/ruby/spec/compression_options_spec.rb  | 252 ++++-----------------
 2 files changed, 65 insertions(+), 327 deletions(-)

diff --git a/src/ruby/ext/grpc/rb_compression_options.c b/src/ruby/ext/grpc/rb_compression_options.c
index 02bb3a5ddf..0a3a215b1c 100644
--- a/src/ruby/ext/grpc/rb_compression_options.c
+++ b/src/ruby/ext/grpc/rb_compression_options.c
@@ -148,18 +148,9 @@ grpc_compression_level grpc_rb_compression_options_level_name_to_value_internal(
   rb_raise(rb_eArgError,
            "Unrecognized compression level name."
            "Valid compression level names are none, low, medium, and high.");
-}
-
-/*  Wrapper over grpc_rb_compression_options_level_name_to_value available for
- * use or testing.
- * Raises an exception for unrecognized level names. */
-VALUE grpc_rb_compression_options_level_name_to_value(VALUE self,
-                                                      VALUE level_name) {
-  (void)self;
-  Check_Type(level_name, T_SYMBOL);
 
-  return INT2NUM((int)grpc_rb_compression_options_level_name_to_value_internal(
-      level_name));
+  /* Dummy return statement. */
+  return GRPC_COMPRESS_LEVEL_NONE;
 }
 
 /* Sets the default compression level, given the name of a compression level.
@@ -199,18 +190,6 @@ void grpc_rb_compression_options_algorithm_name_to_value_internal(
   }
 }
 
-/* Wrapper around algorithm_name_to_value_internal function available for use or
- * testing. */
-VALUE grpc_rb_compression_options_algorithm_name_to_value(
-    VALUE self, VALUE algorithm_name) {
-  grpc_compression_algorithm algorithm_value;
-  (void)self;
-  grpc_rb_compression_options_algorithm_name_to_value_internal(&algorithm_value,
-                                                               algorithm_name);
-
-  return INT2NUM((int)algorithm_value);
-}
-
 /* Indicates whether a given algorithm is enabled on this instance, given the
  * readable algorithm name. */
 VALUE grpc_rb_compression_options_is_algorithm_enabled(VALUE self,
@@ -307,15 +286,6 @@ VALUE grpc_rb_compression_options_level_value_to_name_internal(
   }
 }
 
-/* Wrapper of internal method that makes it available for use and testing. */
-VALUE grpc_rb_compression_options_level_value_to_name(VALUE self,
-                                                      VALUE compression_value) {
-  (void)self;
-  Check_Type(compression_value, T_FIXNUM);
-  return grpc_rb_compression_options_level_value_to_name_internal(
-      (grpc_compression_level)NUM2INT(compression_value));
-}
-
 /* Converts an algorithm internal enum value to a readable name.
  * Fails if the enum value is invalid. */
 VALUE grpc_rb_compression_options_algorithm_value_to_name_internal(
@@ -329,59 +299,21 @@ VALUE grpc_rb_compression_options_algorithm_value_to_name_internal(
   return ID2SYM(rb_intern(algorithm_name));
 }
 
-/* Wrapper of algorithm_to_name internal function available for ues and testing.
- */
-VALUE grpc_rb_compression_options_algorithm_value_to_name(
-    VALUE self, VALUE algorithm_value) {
-  grpc_compression_algorithm algorithm_internal_value =
-      (grpc_compression_algorithm)NUM2INT(algorithm_value);
-  (void)self;
-
-  return grpc_rb_compression_options_algorithm_value_to_name_internal(
-      algorithm_internal_value);
-}
-
-/* Gets the internal value of the default compression level that is to be passed
- * to the GRPC core as a channel argument value.
- * A nil return value means that it hasn't been set. */
-VALUE grpc_rb_compression_options_get_default_algorithm_internal_value(
-    VALUE self) {
-  grpc_rb_compression_options *wrapper = NULL;
-
-  TypedData_Get_Struct(self, grpc_rb_compression_options,
-                       &grpc_rb_compression_options_data_type, wrapper);
-
-  if (wrapper->wrapped->default_algorithm.is_set) {
-    return INT2NUM(wrapper->wrapped->default_algorithm.algorithm);
-  }
-  return Qnil;
-}
-
 /* Gets the readable name of the default algorithm if one has been set.
  * Returns nil if no algorithm has been set. */
 VALUE grpc_rb_compression_options_get_default_algorithm(VALUE self) {
-  VALUE algorithm_value =
-      grpc_rb_compression_options_get_default_algorithm_internal_value(self);
-
-  if (RTEST(algorithm_value)) {
-    return grpc_rb_compression_options_algorithm_value_to_name(self,
-                                                               algorithm_value);
-  }
-
-  return Qnil;
-}
-
-/* Gets the internal enum value of the default algorithm if one has been set.
- * Returns nil if no default algorithm has been set. */
-VALUE grpc_rb_compression_options_get_default_level_internal_value(VALUE self) {
+  grpc_compression_algorithm internal_value;
   grpc_rb_compression_options *wrapper = NULL;
 
   TypedData_Get_Struct(self, grpc_rb_compression_options,
                        &grpc_rb_compression_options_data_type, wrapper);
 
-  if (wrapper->wrapped->default_level.is_set) {
-    return INT2NUM((int)wrapper->wrapped->default_level.level);
+  if (wrapper->wrapped->default_algorithm.is_set) {
+    internal_value = wrapper->wrapped->default_algorithm.algorithm;
+    return grpc_rb_compression_options_algorithm_value_to_name_internal(
+        internal_value);
   }
+
   return Qnil;
 }
 
@@ -390,11 +322,13 @@ VALUE grpc_rb_compression_options_get_default_level_internal_value(VALUE self) {
  * A nil return value means that it hasn't been set. */
 VALUE grpc_rb_compression_options_get_default_level(VALUE self) {
   grpc_compression_level internal_value;
-  VALUE ruby_value =
-      grpc_rb_compression_options_get_default_level_internal_value(self);
+  grpc_rb_compression_options *wrapper = NULL;
+
+  TypedData_Get_Struct(self, grpc_rb_compression_options,
+                       &grpc_rb_compression_options_data_type, wrapper);
 
-  if (RTEST(ruby_value)) {
-    internal_value = (grpc_compression_level)NUM2INT(ruby_value);
+  if (wrapper->wrapped->default_level.is_set) {
+    internal_value = wrapper->wrapped->default_level.level;
     return grpc_rb_compression_options_level_value_to_name_internal(
         internal_value);
   }
@@ -403,7 +337,7 @@ VALUE grpc_rb_compression_options_get_default_level(VALUE self) {
 }
 
 /* Gets a list of the disabled algorithms as readable names.
- * Returns an empty list of no algorithms have been disabled. */
+ * Returns an empty list if no algorithms have been disabled. */
 VALUE grpc_rb_compression_options_get_disabled_algorithms(VALUE self) {
   VALUE disabled_algorithms = rb_ary_new();
   grpc_compression_algorithm internal_value;
@@ -424,16 +358,6 @@ VALUE grpc_rb_compression_options_get_disabled_algorithms(VALUE self) {
   return disabled_algorithms;
 }
 
-/* Provides a bitset as a ruby number that is suitable to pass to
- * the GRPC core as a channel argument to enable compression algorithms. */
-VALUE grpc_rb_compression_options_get_enabled_algorithms_bitset(VALUE self) {
-  grpc_rb_compression_options *wrapper = NULL;
-
-  TypedData_Get_Struct(self, grpc_rb_compression_options,
-                       &grpc_rb_compression_options_data_type, wrapper);
-  return INT2NUM((int)wrapper->wrapped->enabled_algorithms_bitset);
-}
-
 /* Initializes the compression options wrapper.
  * Takes an optional hash parameter.
  *
@@ -511,11 +435,6 @@ void Init_grpc_compression_options() {
   rb_define_method(grpc_rb_cCompressionOptions, "initialize",
                    grpc_rb_compression_options_init, -1);
 
-  /* Gets the bitset of enabled algorithms. */
-  rb_define_method(grpc_rb_cCompressionOptions, "enabled_algorithms_bitset",
-                   grpc_rb_compression_options_get_enabled_algorithms_bitset,
-                   0);
-
   /* Methods for getting the default algorithm, default level, and disabled
    * algorithms as readable names. */
   rb_define_method(grpc_rb_cCompressionOptions, "default_algorithm",
@@ -525,38 +444,11 @@ void Init_grpc_compression_options() {
   rb_define_method(grpc_rb_cCompressionOptions, "disabled_algorithms",
                    grpc_rb_compression_options_get_disabled_algorithms, 0);
 
-  /* Methods for getting the internal enum default algorithm and level enum
-   * values of an instance. */
-  rb_define_method(
-      grpc_rb_cCompressionOptions, "default_algorithm_internal_value",
-      grpc_rb_compression_options_get_default_algorithm_internal_value, 0);
-  rb_define_method(grpc_rb_cCompressionOptions, "default_level_internal_value",
-                   grpc_rb_compression_options_get_default_level_internal_value,
-                   0);
-
   /* Determines whether or not an algorithm is enabled, given a readable
    * algorithm name.*/
-  rb_define_method(grpc_rb_cCompressionOptions, "is_algorithm_enabled",
+  rb_define_method(grpc_rb_cCompressionOptions, "algorithm_enabled?",
                    grpc_rb_compression_options_is_algorithm_enabled, 1);
 
-  /* Methods for converting to and from algorithm enum values and their readable
-   * names. */
-  rb_define_singleton_method(
-      grpc_rb_cCompressionOptions, "algorithm_name_to_value",
-      grpc_rb_compression_options_algorithm_name_to_value, 1);
-  rb_define_singleton_method(
-      grpc_rb_cCompressionOptions, "algorithm_value_to_name",
-      grpc_rb_compression_options_algorithm_value_to_name, 1);
-
-  /* Methods for converting to and from compression level enum values and their
-   * readable names. */
-  rb_define_singleton_method(grpc_rb_cCompressionOptions, "level_name_to_value",
-                             grpc_rb_compression_options_level_name_to_value,
-                             1);
-  rb_define_singleton_method(grpc_rb_cCompressionOptions, "level_value_to_name",
-                             grpc_rb_compression_options_level_value_to_name,
-                             1);
-
   /* Provides a hash of the compression settings suitable
    * for passing to server or channel args. */
   rb_define_method(grpc_rb_cCompressionOptions, "to_hash",
diff --git a/src/ruby/spec/compression_options_spec.rb b/src/ruby/spec/compression_options_spec.rb
index 7fa416ef09..dbd7e59294 100644
--- a/src/ruby/spec/compression_options_spec.rb
+++ b/src/ruby/spec/compression_options_spec.rb
@@ -30,30 +30,14 @@
 require 'grpc'
 
 describe GRPC::Core::CompressionOptions do
-  # Note these constants should be updated according to what the core lib does.
+  # Note these constants should be updated
+  # according to what the core lib provides.
 
-  # Names of supported compression algorithms and their internal enum values
-  ALGORITHMS = {
-    identity: 0,
-    deflate: 1,
-    gzip: 2
-  }
+  # Names of supported compression algorithms
+  ALGORITHMS = [:identity, :deflate, :gzip]
 
-  # Compression algorithms and their corresponding bits in the internal
-  # enabled algorithms bitset for GRPC core channel args.
-  ALGORITHM_BITS = {
-    identity: 0x1,
-    deflate: 0x2,
-    gzip: 0x4
-  }
-
-  # Names of valid supported compression levels and their internal enum values
-  COMPRESS_LEVELS = {
-    none: 0,
-    low: 1,
-    medium: 2,
-    high: 3
-  }
+  # Names of valid supported compression levels
+  COMPRESS_LEVELS = [:none, :low, :medium, :high]
 
   it 'implements to_s' do
     expect { GRPC::Core::CompressionOptions.new.to_s }.to_not raise_error
@@ -61,70 +45,73 @@ describe GRPC::Core::CompressionOptions do
 
   it '#to_channel_arg_hash gives the same result as #to_hash' do
     options = GRPC::Core::CompressionOptions.new
-    expect(options.to_channel_arg_hash).to eql(options.to_hash)
+    expect(options.to_channel_arg_hash).to eq(options.to_hash)
   end
 
   # Test the normal call sequence of creating an instance
   # and then obtaining the resulting channel-arg hash that
   # corresponds to the compression settings of the instance
-  describe 'creating and converting to channel args hash' do
-    it 'gives the correct channel args when nothing has been adjusted yet' do
-      expect(GRPC::Core::CompressionOptions.new.to_hash).to(
-        eql('grpc.compression_enabled_algorithms_bitset' => 0x7))
+  describe 'creating, reading, and converting to channel args hash' do
+    it 'works when no optional args were provided' do
+      options = GRPC::Core::CompressionOptions.new
+
+      ALGORITHMS.each do |algorithm|
+        expect(options.algorithm_enabled?(algorithm)).to be true
+      end
+
+      expect(options.disabled_algorithms).to be_empty
+      expect(options.default_algorithm).to be nil
+      expect(options.default_level).to be nil
+      expect(options.to_hash).to be_instance_of(Hash)
     end
 
-    it 'gives the correct channel args after disabling multiple algorithms' do
+    it 'works when disabling multiple algorithms' do
       options = GRPC::Core::CompressionOptions.new(
         default_algorithm: :identity,
         default_level: :none,
         disabled_algorithms: [:gzip, :deflate]
       )
 
-      channel_arg_hash = options.to_hash
-      expect(channel_arg_hash['grpc.default_compression_algorithm']).to eq(0)
-      expect(channel_arg_hash['grpc.default_compression_level']).to eq(0)
+      [:gzip, :deflate].each do |algorithm|
+        expect(options.algorithm_enabled?(algorithm)).to be false
+        expect(options.disabled_algorithms.include?(algorithm)).to be true
+      end
 
-      bitset = channel_arg_hash['grpc.compression_enabled_algorithms_bitset']
-      expect(bitset & ALGORITHM_BITS[:gzip]).to eq(0)
-      expect(bitset & ALGORITHM_BITS[:deflate]).to eq(0)
+      expect(options.default_algorithm).to be(:identity)
+      expect(options.default_level).to be(:none)
+      expect(options.to_hash).to be_instance_of(Hash)
     end
 
-    it 'gives correct channel args with all args set' do
+    it 'works when all optional args have been set' do
       options = GRPC::Core::CompressionOptions.new(
         default_algorithm: :gzip,
         default_level: :low,
         disabled_algorithms: [:deflate]
       )
 
-      channel_arg_hash = options.to_hash
-
-      actual_bitset = channel_arg_hash[
-        'grpc.compression_enabled_algorithms_bitset']
-      default_algorithm = channel_arg_hash['grpc.default_compression_algorithm']
-      default_level = channel_arg_hash['grpc.default_compression_level']
+      expect(options.algorithm_enabled?(:deflate)).to be false
+      expect(options.algorithm_enabled?(:gzip)).to be true
+      expect(options.disabled_algorithms).to eq([:deflate])
 
-      expect(actual_bitset & ALGORITHM_BITS[:deflate]).to eq(0)
-      expect(default_algorithm).to eq(ALGORITHMS[:gzip])
-      expect(default_level).to eq(COMPRESS_LEVELS[:low])
+      expect(options.default_algorithm).to be(:gzip)
+      expect(options.default_level).to be(:low)
+      expect(options.to_hash).to be_instance_of(Hash)
     end
 
-    it 'gives correct channel args when no algorithms are disabled' do
+    it 'doesnt fail when no algorithms are disabled' do
       options = GRPC::Core::CompressionOptions.new(
         default_algorithm: :identity,
         default_level: :high
       )
 
-      channel_arg_hash = options.to_hash
-
-      actual_bitset = channel_arg_hash[
-        'grpc.compression_enabled_algorithms_bitset']
-      default_algorithm = channel_arg_hash['grpc.default_compression_algorithm']
-      default_level = channel_arg_hash['grpc.default_compression_level']
+      ALGORITHMS.each do |algorithm|
+        expect(options.algorithm_enabled?(algorithm)).to be(true)
+      end
 
-      expect(actual_bitset & ALGORITHM_BITS[:deflate]).to_not eq(0)
-      expect(actual_bitset & ALGORITHM_BITS[:gzip]).to_not eq(0)
-      expect(default_algorithm).to eq(ALGORITHMS[:identity])
-      expect(default_level).to eq(COMPRESS_LEVELS[:high])
+      expect(options.disabled_algorithms).to be_empty
+      expect(options.default_algorithm).to be(:identity)
+      expect(options.default_level).to be(:high)
+      expect(options.to_hash).to be_instance_of(Hash)
     end
   end
 
@@ -140,179 +127,38 @@ describe GRPC::Core::CompressionOptions do
     end
   end
 
-  describe '#level_name_to_value' do
-    COMPRESS_LEVELS.each_pair do |name, internal_value|
-      it "should return value #{internal_value} for level #{name}" do
-        actual_value = GRPC::Core::CompressionOptions.level_name_to_value(name)
-        expect(actual_value).to eq(internal_value)
-      end
-    end
-
-    [:gzip, :deflate, :any, Object.new, 'none', 'low', 1].each do |name|
-      it "should fail for parameter #{name} of class #{name.class}" do
-        blk = proc do
-          GRPC::Core::CompressionOptions.level_name_to_value(name)
-        end
-        expect { blk.call }.to raise_error
-      end
-    end
-  end
-
-  describe '#level_value_to_name' do
-    COMPRESS_LEVELS.each_pair do |name, internal_value|
-      it "should return level name #{name} for value #{internal_value}" do
-        actual_name = GRPC::Core::CompressionOptions.level_value_to_name(
-          internal_value)
-        expect(actual_name).to eq(name)
-      end
-    end
-    it 'should give the correct internal values from compression level names' do
-      cls = GRPC::Core::CompressionOptions
-      COMPRESS_LEVELS.each_pair do |name, internal_value|
-        expect(cls.level_value_to_name(internal_value)).to eq(name)
-      end
-    end
-
-    [:gzip, :any, Object.new, '1', :low].each do |name|
-      it "should fail for parameter #{name} of class #{name.class}" do
-        blk = proc do
-          GRPC::Core::CompressionOptions.level_value_to_name(name)
-        end
-        expect { blk.call }.to raise_error
-      end
-    end
-  end
-
-  describe '#algorithm_name_to_value' do
-    it 'should give the correct internal values from algorithm names' do
-      cls = GRPC::Core::CompressionOptions
-      ALGORITHMS.each_pair do |name, internal_value|
-        expect(cls.algorithm_name_to_value(name)).to eq(internal_value)
-      end
-    end
-
-    ['gzip', 'deflate', :any, Object.new, :none, :low, 1].each do |name|
-      it "should fail for parameter #{name} of class #{name.class}" do
-        blk = proc do
-          GRPC::Core::CompressionOptions.algorithm_name_to_value(name)
-        end
-        expect { blk.call }.to raise_error
-      end
-    end
-  end
-
-  describe '#algorithm_value_to_name' do
-    it 'should give the correct internal values from algorithm names' do
-      cls = GRPC::Core::CompressionOptions
-      ALGORITHMS.each_pair do |name, internal_value|
-        expect(cls.algorithm_value_to_name(internal_value)).to eq(name)
-      end
-    end
-
-    ['gzip', :deflate, :any, Object.new, :low, '1'].each do |value|
-      it "should fail for parameter #{value} of class #{value.class}" do
-        blk = proc do
-          GRPC::Core::CompressionOptions.algorithm_value_to_name(value)
-        end
-        expect { blk.call }.to raise_error
-      end
-    end
-  end
-
-  describe '#default_algorithm and #default_algorithm_internal_value' do
-    it 'can set the default algorithm and then read it back out' do
-      ALGORITHMS.each_pair do |name, internal_value|
-        options = GRPC::Core::CompressionOptions.new(default_algorithm: name)
-        expect(options.default_algorithm).to eq(name)
-        expect(options.default_algorithm_internal_value).to eq(internal_value)
-      end
-    end
-
+  describe '#default_algorithm' do
     it 'returns nil if unset' do
       options = GRPC::Core::CompressionOptions.new
-      expect(options.default_algorithm).to be_nil
-      expect(options.default_algorithm_internal_value).to be_nil
+      expect(options.default_algorithm).to be(nil)
     end
   end
 
-  describe '#default_level and #default_level_internal_value' do
-    it 'can set the default level and read it back out' do
-      COMPRESS_LEVELS.each_pair do |name, internal_value|
-        options = GRPC::Core::CompressionOptions.new(default_level: name)
-        expect(options.default_level).to eq(name)
-        expect(options.default_level_internal_value).to eq(internal_value)
-      end
-    end
-
+  describe '#default_level' do
     it 'returns nil if unset' do
       options = GRPC::Core::CompressionOptions.new
-      expect(options.default_level).to be_nil
-      expect(options.default_level_internal_value).to be_nil
+      expect(options.default_level).to be(nil)
     end
   end
 
   describe '#disabled_algorithms' do
-    it 'can set the disabled algorithms and read them back out' do
-      options = GRPC::Core::CompressionOptions.new(
-        disabled_algorithms: [:gzip, :deflate])
-
-      [:gzip, :deflate].each do |name|
-        expect(options.disabled_algorithms.include?(name)).to eq(true)
-      end
-      expect(options.disabled_algorithms.size).to eq(2)
-    end
-
     it 'returns an empty list if no algorithms were disabled' do
       options = GRPC::Core::CompressionOptions.new
-      expect(options.disabled_algorithms).to eq([])
+      expect(options.disabled_algorithms).to be_empty
     end
   end
 
-  describe '#is_algorithm_enabled' do
-    it 'returns true if the algorithm is valid and not disabled' do
-      options = GRPC::Core::CompressionOptions.new(disabled_algorithms: [:gzip])
-      expect(options.is_algorithm_enabled(:deflate)).to eq(true)
-    end
-
-    it 'returns false if the algorithm is valid and disabled' do
-      options = GRPC::Core::CompressionOptions.new(disabled_algorithms: [:gzip])
-      expect(options.is_algorithm_enabled(:gzip)).to eq(false)
-    end
-
+  describe '#algorithm_enabled?' do
     [:none, :any, 'gzip', Object.new, 1].each do |name|
       it "should fail for parameter ${name} of class #{name.class}" do
         options = GRPC::Core::CompressionOptions.new(
           disabled_algorithms: [:gzip])
 
         blk = proc do
-          options.is_algorithm_enabled(name)
+          options.algorithm_enabled?(name)
         end
         expect { blk.call }.to raise_error
       end
     end
   end
-
-  describe '#enabled_algoritms_bitset' do
-    it 'should respond to not disabling any algorithms' do
-      options = GRPC::Core::CompressionOptions.new
-      actual_bitset = options.enabled_algorithms_bitset
-      expect(actual_bitset & ALGORITHM_BITS[:gzip]).to_not eq(0)
-      expect(actual_bitset & ALGORITHM_BITS[:deflate]).to_not eq(0)
-    end
-
-    it 'should respond to disabling one algorithm' do
-      options = GRPC::Core::CompressionOptions.new(
-        disabled_algorithms: [:gzip])
-      expect(options.enabled_algorithms_bitset & ALGORITHM_BITS[:gzip]).to eq(0)
-    end
-
-    it 'should respond to disabling multiple algorithms' do
-      options = GRPC::Core::CompressionOptions.new(
-        disabled_algorithms: [:gzip, :deflate])
-
-      actual_bitset = options.enabled_algorithms_bitset
-      expect(actual_bitset & ALGORITHM_BITS[:gzip]).to eq(0)
-      expect(actual_bitset & ALGORITHM_BITS[:deflate]).to eq(0)
-    end
-  end
 end
-- 
GitLab