fix: Optimize/improve string-ification of values

Author: kgilpin

lib/appmap/event.rb

@@ -20,7 +20,9 @@ def next_id_counter
     MethodEventStruct = Struct.new(:id, :event, :thread_id)
 
     class MethodEvent < MethodEventStruct
-      LIMIT = 100
+      MAX_ARRAY_ENUMERATION = 10
+      MAX_HASH_ENUMERATION = 10
+      MAX_STRING_LENGTH = 100
 
       class << self
         def build_from_invocation(event_type, event:)
@@ -48,14 +50,14 @@ def display_string(value)
           end
 
           start = Time.now
-          value_string = custom_display_string(value) || default_display_string(value)
+          value_string, final = custom_display_string(value) || default_display_string(value)
 
           if @times
             elapsed = Time.now - start
             @times[best_class_name(value)] += elapsed
           end
 
-          encode_display_string(value_string)
+          final ? value_string : encode_display_string(value_string)
         end
 
         def object_properties(hash_like)
@@ -80,21 +82,33 @@ def best_class_name(value)
         end
 
         def encode_display_string(value)
-          (value||'')[0...LIMIT].encode('utf-8', invalid: :replace, undef: :replace, replace: '_')
+          (value||'')[0...MAX_STRING_LENGTH].encode('utf-8', invalid: :replace, undef: :replace, replace: '_')
         end
 
         def custom_display_string(value)
           case value
           when NilClass, TrueClass, FalseClass, Numeric, Time, Date
-            value.to_s
+            [ value.to_s, true ]
+          when Symbol
+            [ ":#{value}", true ]
           when String
-            value[0...LIMIT].encode('utf-8', invalid: :replace, undef: :replace, replace: '_')
+            result = value[0...MAX_STRING_LENGTH].encode('utf-8', invalid: :replace, undef: :replace, replace: '_')
+            result << " (...#{value.length - MAX_STRING_LENGTH} more characters)" if value.length > MAX_STRING_LENGTH
+            [ result, true ]
+          when Array
+            result = value[0...MAX_ARRAY_ENUMERATION].map{|v| display_string(v)}.join(', ')
+            result << " (...#{value.length - MAX_ARRAY_ENUMERATION} more items)" if value.length > MAX_ARRAY_ENUMERATION
+            [ [ '[', result, ']' ].join, true ]
+          when Hash
+            result = value.keys[0...MAX_HASH_ENUMERATION].map{|key| "#{display_string(key)}=>#{display_string(value[key])}"}.join(', ')
+            result << " (...#{value.size - MAX_HASH_ENUMERATION} more entries)" if value.size > MAX_HASH_ENUMERATION
+            [ [ '{', result, '}' ].join, true ]
           when File
-            "#{value.class}[path=#{value.path}]"
+            [ "#{value.class}[path=#{value.path}]", true ]
           when Net::HTTP
-            "#{value.class}[#{value.address}:#{value.port}]"
+            [ "#{value.class}[#{value.address}:#{value.port}]", true ]
           when Net::HTTPGenericRequest
-            "#{value.class}[#{value.method} #{value.path}]"
+            [ "#{value.class}[#{value.method} #{value.path}]", true ]
           end
         rescue StandardError
           nil

spec/display_string_spec.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'appmap/event'
+
+include AppMap
+
+describe 'display_string' do
+  def display_string(value)
+    Event::MethodEvent.display_string value
+  end
+
+  def compare_display_string(value, expected)
+    expect(display_string(value)).to eq(expected)
+  end
+
+  context 'for a' do
+    it 'String' do
+      compare_display_string 'foo', 'foo'
+    end
+    it 'long String' do
+      compare_display_string 'foo' * 100, 'foo' * 33 + 'f (...200 more characters)'
+    end
+    it 'Array' do
+      compare_display_string([ 1, 'my', :bar, [ 2, 3 ], { 4 => 5 } ], '[1, my, :bar, [2, 3], {4=>5}]')
+    end
+    it 'large Array' do
+      compare_display_string 50.times.map { |i| i }, '[0, 1, 2, 3, 4, 5, 6, 7, 8, 9 (...40 more items)]'
+    end
+    it 'large Hash' do
+      compare_display_string(50.times.map { |i| [ i*2, i*2+1] }.to_h, '{0=>1, 2=>3, 4=>5, 6=>7, 8=>9, 10=>11, 12=>13, 14=>15, 16=>17, 18=>19 (...40 more entries)}')
+    end
+    it 'Hash' do
+      compare_display_string({ 1 => 2, 'my' => 'neighbor', bar: :baz, ary: [ 1, 2 ]}, '{1=>2, my=>neighbor, :bar=>:baz, :ary=>[1, 2]}')
+    end
+    it 'big Hash' do
+      compare_display_string(50.times.map { |i| [ i*2, i*2+1] }.to_h, '{0=>1, 2=>3, 4=>5, 6=>7, 8=>9, 10=>11, 12=>13, 14=>15, 16=>17, 18=>19 (...40 more entries)}')
+    end
+  end
+end

spec/hook_spec.rb

@@ -1136,7 +1136,7 @@ def secure_compare(a, b)
       :parameters:
       - :name: :args
         :class: Array
-        :value: '["foo"]'
+        :value: "[foo]"
         :kind: :rest
       - :name: :kw1
         :class: String

spec/rails_recording_spec.rb

@@ -78,7 +78,7 @@ def run_spec(spec_name)
                 'name' => 'params',
                 'class' => 'ActiveSupport::HashWithIndifferentAccess',
                 'object_id' => Integer,
-                'value' => '{"login"=>"alice"}',
+                'value' => '{login=>alice}',
                 'kind' => 'req'
               ),
               'receiver' => anything

spec/record_net_http_spec.rb

@@ -127,7 +127,7 @@ def stop_recording
         :message:
         - :name: ary
           :class: Array
-          :value: '["1", "2"]'
+          :value: "[1, 2]"
         EVENT
       end