Merge pull request #2680 from ganmacs/remove-unnecessary-condition

Delete unnecessary condition

Author: ganmacs

lib/fluent/plugin/buf_file.rb

@@ -175,12 +175,8 @@ def resume
 
       def generate_chunk(metadata)
         # FileChunk generates real path with unique_id
-        if @file_permission
-          chunk = Fluent::Plugin::Buffer::FileChunk.new(metadata, @path, :create, perm: @file_permission, compress: @compress)
-        else
-          chunk = Fluent::Plugin::Buffer::FileChunk.new(metadata, @path, :create, compress: @compress)
-        end
-
+        perm = @file_permission || system_config.file_permission
+        chunk = Fluent::Plugin::Buffer::FileChunk.new(metadata, @path, :create, perm: perm, compress: @compress)
         log.debug "Created new chunk", chunk_id: dump_unique_id_hex(chunk.unique_id), metadata: metadata
 
         return chunk

lib/fluent/plugin/buf_file_single.rb

@@ -195,11 +195,8 @@ def resume
 
       def generate_chunk(metadata)
         # FileChunk generates real path with unique_id
-        if @file_permission
-          chunk = Fluent::Plugin::Buffer::FileSingleChunk.new(metadata, @path, :create, @key_in_path, perm: @file_permission, compress: @compress)
-        else
-          chunk = Fluent::Plugin::Buffer::FileSingleChunk.new(metadata, @path, :create, @key_in_path, compress: @compress)
-        end
+        perm = @file_permission || system_config.file_permission
+        chunk = Fluent::Plugin::Buffer::FileSingleChunk.new(metadata, @path, :create, @key_in_path, perm: perm, compress: @compress)
 
         log.debug "Created new chunk", chunk_id: dump_unique_id_hex(chunk.unique_id), metadata: metadata
 

lib/fluent/plugin/buffer/file_chunk.rb

@@ -37,14 +37,13 @@ class FileChunkError < StandardError; end
         # path_prefix: path prefix string, ended with '.'
         # path_suffix: path suffix string, like '.log' (or any other user specified)
 
-        include SystemConfig::Mixin
-
         FILE_PERMISSION = 0644
 
         attr_reader :path, :permission
 
-        def initialize(metadata, path, mode, perm: system_config.file_permission || FILE_PERMISSION, compress: :text)
+        def initialize(metadata, path, mode, perm: nil, compress: :text)
           super(metadata, compress: compress)
+          perm ||= FILE_PERMISSION
           @permission = perm.is_a?(String) ? perm.to_i(8) : perm
           @bytesize = @size = @adding_bytes = @adding_size = 0
           @meta = nil

lib/fluent/plugin/buffer/file_single_chunk.rb

@@ -29,18 +29,17 @@ class FileChunkError < StandardError; end
         ## buffer chunk path          : /path/to/directory/fsb.key.b513b61c9791029c2513b61c9791029c2.buf
         ## state: b/q - 'b'(on stage), 'q'(enqueued)
 
-        include SystemConfig::Mixin
-
         PATH_EXT = 'buf'
         PATH_SUFFIX = ".#{PATH_EXT}"
         PATH_REGEXP = /\.(b|q)([0-9a-f]+)\.#{PATH_EXT}*\Z/n  # //n switch means explicit 'ASCII-8BIT' pattern
         FILE_PERMISSION = 0644
 
         attr_reader :path, :permission
 
-        def initialize(metadata, path, mode, key, perm: system_config.file_permission || FILE_PERMISSION, compress: :text)
+        def initialize(metadata, path, mode, key, perm: FILE_PERMISSION, compress: :text)
           super(metadata, compress: compress)
           @key = key
+          perm ||= FILE_PERMISSION
           @permission = perm.is_a?(String) ? perm.to_i(8) : perm
           @bytesize = @size = @adding_bytes = @adding_size = 0
 

test/plugin/test_buf_file.rb

@@ -327,6 +327,46 @@ def write_metadata(path, chunk_id, metadata, size, ctime, mtime)
       plugin.stop; plugin.before_shutdown; plugin.shutdown; plugin.after_shutdown; plugin.close; plugin.terminate
       FileUtils.rm_r bufdir
     end
+
+    test '#generate_chunk generates blank file chunk with specified permission with system_config' do
+      omit "NTFS doesn't support UNIX like permissions" if Fluent.windows?
+
+      begin
+        plugin = Fluent::Plugin::FileBuffer.new
+        plugin.owner = @d
+        rand_num = rand(0..100)
+        bufpath = File.join(File.expand_path("../../tmp/buffer_file_#{rand_num}", __FILE__), 'testbuf.*.log')
+        bufdir = File.dirname(bufpath)
+
+        FileUtils.rm_r bufdir if File.exist?(bufdir)
+        assert !File.exist?(bufdir)
+
+        plugin.configure(config_element('buffer', '', { 'path' => bufpath }))
+
+        assert !File.exist?(bufdir)
+        plugin.start
+
+        m = metadata()
+        c = nil
+        Fluent::SystemConfig.overwrite_system_config("file_permission" => "700") do
+           c = plugin.generate_chunk(m)
+        end
+
+        assert c.is_a? Fluent::Plugin::Buffer::FileChunk
+        assert_equal m, c.metadata
+        assert c.empty?
+        assert_equal :unstaged, c.state
+        assert_equal 0700, c.permission
+        assert_equal bufpath.gsub('.*.', ".b#{Fluent::UniqueId.hex(c.unique_id)}."), c.path
+        assert{ File.stat(c.path).mode.to_s(8).end_with?('700') }
+
+        c.purge
+
+        plugin.stop; plugin.before_shutdown; plugin.shutdown; plugin.after_shutdown; plugin.close; plugin.terminate
+      ensure
+        FileUtils.rm_r bufdir
+      end
+    end
   end
 
   sub_test_case 'configured with system root directory and plugin @id' do

test/plugin/test_buf_file_single.rb

@@ -293,6 +293,38 @@ def create_driver(conf = TAG_CONF, klass = FluentPluginFileSingleBufferTest::Dum
 
       c.purge
     end
+
+    test '#generate_chunk generates blank file chunk with specified permission with system_config' do
+      omit "NTFS doesn't support UNIX like permissions" if Fluent.windows?
+
+      @d = create_driver(%[
+        <buffer tag>
+          @type file_single
+          path #{PATH}
+        </buffer>
+      ])
+      @p = @d.instance.buffer
+
+      FileUtils.rm_r @bufdir if File.exist?(@bufdir)
+      assert !File.exist?(@bufdir)
+
+      @p.start
+
+      m = metadata()
+      c = nil
+      Fluent::SystemConfig.overwrite_system_config("file_permission" => "700") do
+        c = @p.generate_chunk(m)
+      end
+      assert c.is_a? Fluent::Plugin::Buffer::FileSingleChunk
+      assert_equal m, c.metadata
+      assert c.empty?
+      assert_equal :unstaged, c.state
+      assert_equal 0700, c.permission
+      assert_equal File.join(@bufdir, "fsb.testing.b#{Fluent::UniqueId.hex(c.unique_id)}.buf"), c.path
+      assert{ File.stat(c.path).mode.to_s(8).end_with?('700') }
+
+      c.purge
+    end
   end
 
   sub_test_case 'configured with system root directory and plugin @id' do

test/plugin/test_buffer_file_chunk.rb

@@ -416,17 +416,6 @@ def gen_chunk_path(prefix, unique_id)
       assert_equal d4.to_json + "\n", lines[3]
     end
 
-    test 'can refer system config for file permission' do
-      omit "NTFS doesn't support UNIX like permissions" if Fluent.windows?
-
-      chunk_path = File.join(@chunkdir, 'testperm.*.log')
-      Fluent::SystemConfig.overwrite_system_config("file_permission" => "600") do
-        c = Fluent::Plugin::Buffer::FileChunk.new(gen_metadata, chunk_path, :create)
-        assert{ File.stat(c.path).mode.to_s(8).end_with?('600') }
-        assert{ File.stat(c.path + '.meta').mode.to_s(8).end_with?('600') }
-      end
-    end
-
     test '#write_metadata tries to store metadata on file' do
       d1 = {"f1" => 'v1', "f2" => 'v2', "f3" => 'v3'}
       d2 = {"f1" => 'vv1', "f2" => 'vv2', "f3" => 'vv3'}

test/plugin/test_buffer_file_single_chunk.rb

@@ -353,16 +353,6 @@ def gen_chunk_path(prefix, unique_id)
       assert_equal d3.to_json + "\n", lines[2]
       assert_equal d4.to_json + "\n", lines[3]
     end
-
-    test 'can refer system config for file permission' do
-      omit "NTFS doesn't support UNIX like permissions" if Fluent.windows?
-
-      chunk_path = File.join(@chunkdir, 'fsb.*.buf')
-      Fluent::SystemConfig.overwrite_system_config("file_permission" => "600") do
-        c = Fluent::Plugin::Buffer::FileSingleChunk.new(gen_metadata, chunk_path, :create, nil)
-        assert{ File.stat(c.path).mode.to_s(8).end_with?('600') }
-      end
-    end
   end
 
   sub_test_case 'chunk with file for staged chunk' do