fix YAML::Store

* lib/yaml/store.rb (YAML::Store#initialize): Fix arguments.
  [ruby-dev:49821] [Bug #12800]

* test/yaml/test_store.rb: Add tests from test/test_pstore.rb.

* test/yaml/test_store.rb (YAMLStoreTest#test_with_options): Add options test.

* lib/yaml/store.rb (YAML::Store#dump): Revert to to_yaml.

* lib/yaml/store.rb (YAML::Store#empty_marshal_data): Use to_yaml with options.

* lib/yaml/store.rb (YAML::Store#empty_marshal_checksum): Use CHECKSUM_ALGO.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56291 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,18 @@
+Thu Sep 29 22:22:22 2016  Kazuhiro NISHIYAMA  <zn@mbf.nifty.com>
+
+	* lib/yaml/store.rb (YAML::Store#initialize): Fix arguments.
+	  [ruby-dev:49821] [Bug #12800]
+
+	* test/yaml/test_store.rb: Add tests from test/test_pstore.rb.
+
+	* test/yaml/test_store.rb (YAMLStoreTest#test_with_options): Add options test.
+
+	* lib/yaml/store.rb (YAML::Store#dump): Revert to to_yaml.
+
+	* lib/yaml/store.rb (YAML::Store#empty_marshal_data): Use to_yaml with options.
+
+	* lib/yaml/store.rb (YAML::Store#empty_marshal_checksum): Use CHECKSUM_ALGO.
+
 Thu Sep 29 19:34:23 2016  Pete Higgins  <pete@peterhiggins.org>
 
 	* thread_sync.c (rb_queue_pop, rb_szqueue_push, rb_szqueue_pop):

lib/yaml/store.rb

@@ -39,23 +39,28 @@ require 'pstore'
 class YAML::Store < PStore
 
   # :call-seq:
-  #   initialize( file_name, yaml_opts = {} )
+  #   initialize( file_name, thread_safe = false, yaml_opts = {} )
   #
   # Creates a new YAML::Store object, which will store data in +file_name+.
   # If the file does not already exist, it will be created.
   #
+  # YAML::Store objects are always reentrant. But if _thread_safe_ is set to true,
+  # then it will become thread-safe at the cost of a minor performance hit.
   #
   # Options passed in through +yaml_opts+ will be used when converting the
   # store to YAML via Hash#to_yaml().
-  def initialize file_name, yaml_opts = {}
-    @opt = yaml_opts
-    super
+  def initialize( *o )
+    @opt = {}
+    if o.last.is_a? Hash
+      @opt.update(o.pop)
+    end
+    super(*o)
   end
 
   # :stopdoc:
 
   def dump(table)
-    YAML.dump @table
+    @table.to_yaml(@opt)
   end
 
   def load(content)
@@ -71,12 +76,10 @@ class YAML::Store < PStore
     false
   end
 
-  EMPTY_MARSHAL_DATA = YAML.dump({})
-  EMPTY_MARSHAL_CHECKSUM = Digest::MD5.digest(EMPTY_MARSHAL_DATA)
   def empty_marshal_data
-    EMPTY_MARSHAL_DATA
+    {}.to_yaml(@opt)
   end
   def empty_marshal_checksum
-    EMPTY_MARSHAL_CHECKSUM
+    CHECKSUM_ALGO.digest(empty_marshal_data)
   end
 end

test/yaml/test_store.rb

@@ -0,0 +1,180 @@
+# frozen_string_literal: false
+require 'test/unit'
+require 'yaml/store'
+require 'tmpdir'
+
+class YAMLStoreTest < Test::Unit::TestCase
+  def setup
+    @yaml_store_file = File.join(Dir.tmpdir, "yaml_store.tmp.#{Process.pid}")
+    @yaml_store = YAML::Store.new(@yaml_store_file)
+  end
+
+  def teardown
+    File.unlink(@yaml_store_file) rescue nil
+  end
+
+  def test_opening_new_file_in_readonly_mode_should_result_in_empty_values
+    @yaml_store.transaction(true) do
+      assert_nil @yaml_store[:foo]
+      assert_nil @yaml_store[:bar]
+    end
+  end
+
+  def test_opening_new_file_in_readwrite_mode_should_result_in_empty_values
+    @yaml_store.transaction do
+      assert_nil @yaml_store[:foo]
+      assert_nil @yaml_store[:bar]
+    end
+  end
+
+  def test_data_should_be_loaded_correctly_when_in_readonly_mode
+    @yaml_store.transaction do
+      @yaml_store[:foo] = "bar"
+    end
+    @yaml_store.transaction(true) do
+      assert_equal "bar", @yaml_store[:foo]
+    end
+  end
+
+  def test_data_should_be_loaded_correctly_when_in_readwrite_mode
+    @yaml_store.transaction do
+      @yaml_store[:foo] = "bar"
+    end
+    @yaml_store.transaction do
+      assert_equal "bar", @yaml_store[:foo]
+    end
+  end
+
+  def test_changes_after_commit_are_discarded
+    @yaml_store.transaction do
+      @yaml_store[:foo] = "bar"
+      @yaml_store.commit
+      @yaml_store[:foo] = "baz"
+    end
+    @yaml_store.transaction(true) do
+      assert_equal "bar", @yaml_store[:foo]
+    end
+  end
+
+  def test_changes_are_not_written_on_abort
+    @yaml_store.transaction do
+      @yaml_store[:foo] = "bar"
+      @yaml_store.abort
+    end
+    @yaml_store.transaction(true) do
+      assert_nil @yaml_store[:foo]
+    end
+  end
+
+  def test_writing_inside_readonly_transaction_raises_error
+    assert_raise(PStore::Error) do
+      @yaml_store.transaction(true) do
+        @yaml_store[:foo] = "bar"
+      end
+    end
+  end
+
+  def test_thread_safe
+    q1 = Queue.new
+    assert_raise(PStore::Error) do
+      th = Thread.new do
+        @yaml_store.transaction do
+          @yaml_store[:foo] = "bar"
+          q1.push true
+          sleep
+        end
+      end
+      begin
+        q1.pop
+        @yaml_store.transaction {}
+      ensure
+        th.kill
+        th.join
+      end
+    end
+    q2 = Queue.new
+    begin
+      yaml_store = YAML::Store.new(second_file, true)
+      cur = Thread.current
+      th = Thread.new do
+        yaml_store.transaction do
+          yaml_store[:foo] = "bar"
+          q1.push true
+          q2.pop
+          # wait for cur to enter a transaction
+          sleep 0.1 until cur.stop?
+        end
+      end
+      begin
+        q1.pop
+        q2.push true
+        assert_equal("bar", yaml_store.transaction { yaml_store[:foo] })
+      ensure
+        th.join
+      end
+    end
+  ensure
+    File.unlink(second_file) rescue nil
+  end
+
+  def test_nested_transaction_raises_error
+    assert_raise(PStore::Error) do
+      @yaml_store.transaction { @yaml_store.transaction { } }
+    end
+    yaml_store = YAML::Store.new(second_file, true)
+    assert_raise(PStore::Error) do
+      yaml_store.transaction { yaml_store.transaction { } }
+    end
+  ensure
+    File.unlink(second_file) rescue nil
+  end
+
+  # Test that PStore's file operations do not blow up when default encodings are set
+  def test_yaml_store_files_are_accessed_as_binary_files
+    bug5311 = '[ruby-core:39503]'
+    n = 128
+    assert_in_out_err(["-Eutf-8:utf-8", "-ryaml/store", "-", @yaml_store_file], <<-SRC, [bug5311], [], bug5311, timeout: 15)
+      @yaml_store = YAML::Store.new(ARGV[0])
+      (1..#{n}).each do |i|
+        @yaml_store.transaction {@yaml_store["Key\#{i}"] = "value \#{i}"}
+      end
+      @yaml_store.transaction {@yaml_store["Bug5311"] = '#{bug5311}'}
+      puts @yaml_store.transaction {@yaml_store["Bug5311"]}
+    SRC
+    assert_equal(bug5311, @yaml_store.transaction {@yaml_store["Bug5311"]}, bug5311)
+  end
+
+  def second_file
+    File.join(Dir.tmpdir, "yaml_store.tmp2.#{Process.pid}")
+  end
+
+  def test_with_options
+    bug12800 = '[ruby-dev:49821]'
+    default_yaml = "---\na:\n- - b\n"
+    indentation_3_yaml = "---\na:\n-  - b\n"
+
+    @yaml_store = YAML::Store.new(@yaml_store_file)
+    @yaml_store.transaction do
+      @yaml_store['a'] = [['b']]
+    end
+    assert_equal(default_yaml, File.read(@yaml_store_file), bug12800)
+
+    @yaml_store = YAML::Store.new(@yaml_store_file, true)
+    @yaml_store.transaction do
+      @yaml_store['a'] = [['b']]
+    end
+    assert_equal(default_yaml, File.read(@yaml_store_file), bug12800)
+
+    @yaml_store = YAML::Store.new(@yaml_store_file, indentation: 3)
+    @yaml_store.transaction do
+      @yaml_store['a'] = [['b']]
+    end
+    assert_equal(indentation_3_yaml, File.read(@yaml_store_file), bug12800)
+
+    @yaml_store = YAML::Store.new(@yaml_store_file, true, indentation: 3)
+    @yaml_store.transaction do
+      @yaml_store['a'] = [['b']]
+    end
+    assert_equal(indentation_3_yaml, File.read(@yaml_store_file), bug12800)
+  end
+end