From 92134654eae6728ceaba66b1f89cc8c60ff7cc18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=B3=A5=E4=BA=95=20=E9=9B=AA?= Date: Fri, 26 Dec 2014 14:59:50 +0900 Subject: [PATCH 01/22] Make dir app/models/fluentd/setting_archive/ and move backup_file model under it. prepare for gathering models that concernig about archive --- app/controllers/fluentd/settings/histories_controller.rb | 4 ++-- app/controllers/fluentd/settings/running_backup_controller.rb | 2 +- app/controllers/fluentd/settings_controller.rb | 4 ++-- .../fluentd/{setting => setting_archive}/backup_file.rb | 2 +- spec/features/fluentd/setting/hisotries_spec.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename app/models/fluentd/{setting => setting_archive}/backup_file.rb (97%) diff --git a/app/controllers/fluentd/settings/histories_controller.rb b/app/controllers/fluentd/settings/histories_controller.rb index e73a18f..c789a36 100644 --- a/app/controllers/fluentd/settings/histories_controller.rb +++ b/app/controllers/fluentd/settings/histories_controller.rb @@ -5,7 +5,7 @@ class Fluentd::Settings::HistoriesController < ApplicationController def index @backup_files = @fluentd.agent.backup_files_in_new_order.map do |file_path| - Fluentd::Setting::BackupFile.new(file_path) + Fluentd::SettingArchive::BackupFile.new(file_path) end end @@ -21,6 +21,6 @@ class Fluentd::Settings::HistoriesController < ApplicationController def find_backup_file #Do not use BackupFile.new(params[:id]) because params[:id] can be any path. - @backup_file = Fluentd::Setting::BackupFile.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id]) + @backup_file = Fluentd::SettingArchive::BackupFile.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id]) end end diff --git a/app/controllers/fluentd/settings/running_backup_controller.rb b/app/controllers/fluentd/settings/running_backup_controller.rb index 2cb4096..7fe33db 100644 --- a/app/controllers/fluentd/settings/running_backup_controller.rb +++ b/app/controllers/fluentd/settings/running_backup_controller.rb @@ -14,6 +14,6 @@ class Fluentd::Settings::RunningBackupController < ApplicationController private def find_backup_file - @backup_file = Fluentd::Setting::BackupFile.new(@fluentd.agent.running_config_backup_file) + @backup_file = Fluentd::SettingArchive::BackupFile.new(@fluentd.agent.running_config_backup_file) end end diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb index ec9d5a1..84f56be 100644 --- a/app/controllers/fluentd/settings_controller.rb +++ b/app/controllers/fluentd/settings_controller.rb @@ -7,10 +7,10 @@ class Fluentd::SettingsController < ApplicationController def show @backup_files = @fluentd.agent.backup_files_in_new_order.first(Settings.histories_count_in_preview).map do |file_path| - Fluentd::Setting::BackupFile.new(file_path) + Fluentd::SettingArchive::BackupFile.new(file_path) end - @running_backedup_file = Fluentd::Setting::BackupFile.new(@fluentd.agent.running_config_backup_file) + @running_backedup_file = Fluentd::SettingArchive::BackupFile.new(@fluentd.agent.running_config_backup_file) end def edit diff --git a/app/models/fluentd/setting/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb similarity index 97% rename from app/models/fluentd/setting/backup_file.rb rename to app/models/fluentd/setting_archive/backup_file.rb index 939182b..6ee72a7 100644 --- a/app/models/fluentd/setting/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -1,5 +1,5 @@ class Fluentd - module Setting + module SettingArchive class BackupFile attr_accessor :file_path diff --git a/spec/features/fluentd/setting/hisotries_spec.rb b/spec/features/fluentd/setting/hisotries_spec.rb index 9ae1db7..26cb94b 100644 --- a/spec/features/fluentd/setting/hisotries_spec.rb +++ b/spec/features/fluentd/setting/hisotries_spec.rb @@ -26,7 +26,7 @@ describe "histories", stub: :daemon do end describe 'show' do - let(:last_backup_file) { Fluentd::Setting::BackupFile.new(daemon.agent.backup_files_in_new_order.first) } + let(:last_backup_file) { Fluentd::SettingArchive::BackupFile.new(daemon.agent.backup_files_in_new_order.first) } before do visit "/daemon/setting/histories/#{last_backup_file.file_id}" From bb7786499bf9977db829c501b51140d05b0350fa Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 12:03:54 +0900 Subject: [PATCH 02/22] Extract common procedures into module. --- .../fluentd/setting_archive/archivable.rb | 36 +++++++++++++++++++ .../fluentd/setting_archive/backup_file.rb | 32 +++-------------- 2 files changed, 40 insertions(+), 28 deletions(-) create mode 100644 app/models/concerns/fluentd/setting_archive/archivable.rb diff --git a/app/models/concerns/fluentd/setting_archive/archivable.rb b/app/models/concerns/fluentd/setting_archive/archivable.rb new file mode 100644 index 0000000..db34798 --- /dev/null +++ b/app/models/concerns/fluentd/setting_archive/archivable.rb @@ -0,0 +1,36 @@ +class Fluentd + module SettingArchive + module Archivable + attr_accessor :file_path + + def file_id + @file_id ||= with_file { name.gsub(/#{self.class::FILE_EXTENSION}\Z/,'') } + end + + def name + @name ||= with_file { File.basename(file_path) } + end + + def content + @content ||= with_file { File.open(file_path, "r") { |f| f.read } } + end + + def ctime + with_file { File.ctime(file_path) } + end + + private + + def file_path_of(dir, id) + file_path = Pathname.new(dir).join("#{id}#{self.class::FILE_EXTENSION}") + raise "No such a file #{file_path}" unless File.exist?(file_path) + file_path + end + + def with_file + return nil unless file_path && File.exist?(file_path) + yield + end + end + end +end diff --git a/app/models/fluentd/setting_archive/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb index 6ee72a7..3b0e2cb 100644 --- a/app/models/fluentd/setting_archive/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -1,41 +1,17 @@ class Fluentd module SettingArchive class BackupFile - attr_accessor :file_path + include Archivable + + FILE_EXTENSION = ".conf".freeze def self.find_by_file_id(backup_dir, file_id) - file_path = Pathname.new(backup_dir).join("#{file_id}.conf") - raise "No such a file #{file_path}" unless File.exist?(file_path) - - new(file_path) + new(file_path_of(backup_dir, file_id)) end def initialize(file_path) @file_path = file_path end - - def file_id - @file_id ||= with_file { name.gsub(/.conf\Z/,'') } - end - - def name - @name ||= with_file { File.basename(file_path) } - end - - def content - @content ||= with_file { File.open(file_path, "r") { |f| f.read } } - end - - def ctime - with_file { File.ctime(file_path) } - end - - private - - def with_file - return nil unless file_path && File.exist?(file_path) - yield - end end end end From 7a95af83ce1709129c2f0db9f346822858bc60db Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 12:05:57 +0900 Subject: [PATCH 03/22] Add Note class to memo about backup files. --- app/models/fluentd/setting_archive/note.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 app/models/fluentd/setting_archive/note.rb diff --git a/app/models/fluentd/setting_archive/note.rb b/app/models/fluentd/setting_archive/note.rb new file mode 100644 index 0000000..561f8f9 --- /dev/null +++ b/app/models/fluentd/setting_archive/note.rb @@ -0,0 +1,17 @@ +class Fluentd + module SettingArchive + class Note + include Archivable + + FILE_EXTENSION = ".note".freeze + + def self.find_by_file_id(backup_dir, file_id) + new(file_path_of(backup_dir, file_id)) + end + + def initialize(file_path) + @file_path = file_path + end + end + end +end From a28b71084c00c92d3c06516d17c75d07376683df Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 12:06:44 +0900 Subject: [PATCH 04/22] Associate a note with a backup file. --- app/models/fluentd/setting_archive/backup_file.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/fluentd/setting_archive/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb index 3b0e2cb..f0c310c 100644 --- a/app/models/fluentd/setting_archive/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -2,15 +2,18 @@ class Fluentd module SettingArchive class BackupFile include Archivable + attr_reader :note FILE_EXTENSION = ".conf".freeze def self.find_by_file_id(backup_dir, file_id) - new(file_path_of(backup_dir, file_id)) + note = Note.find_by_file_id(backup_dir, file_id) rescue nil + new(file_path_of(backup_dir, file_id), note) end - def initialize(file_path) + def initialize(file_path, note = nil) @file_path = file_path + @note = note || Note.new(file_path.sub(/#{FILE_EXTENSION}$/, Note::FILE_EXTENSION)) end end end From 5c3e8952bc2cd90526bf307eb002085d6ac1552a Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 12:40:27 +0900 Subject: [PATCH 05/22] Fix to define file_path_of as class method. --- .../fluentd/setting_archive/archivable.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/models/concerns/fluentd/setting_archive/archivable.rb b/app/models/concerns/fluentd/setting_archive/archivable.rb index db34798..fa530f4 100644 --- a/app/models/concerns/fluentd/setting_archive/archivable.rb +++ b/app/models/concerns/fluentd/setting_archive/archivable.rb @@ -1,8 +1,19 @@ class Fluentd module SettingArchive module Archivable + extend ActiveSupport::Concern attr_accessor :file_path + module ClassMethods + private + + def file_path_of(dir, id) + file_path = Pathname.new(dir).join("#{id}#{self::FILE_EXTENSION}") + raise "No such a file #{file_path}" unless File.exist?(file_path) + file_path + end + end + def file_id @file_id ||= with_file { name.gsub(/#{self.class::FILE_EXTENSION}\Z/,'') } end @@ -21,12 +32,6 @@ class Fluentd private - def file_path_of(dir, id) - file_path = Pathname.new(dir).join("#{id}#{self.class::FILE_EXTENSION}") - raise "No such a file #{file_path}" unless File.exist?(file_path) - file_path - end - def with_file return nil unless file_path && File.exist?(file_path) yield From e2e884e31795f1f2dfb4096c7654a58d40f5441e Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 13:56:22 +0900 Subject: [PATCH 06/22] Add method to update content of note. --- app/models/fluentd/setting_archive/note.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/models/fluentd/setting_archive/note.rb b/app/models/fluentd/setting_archive/note.rb index 561f8f9..0cc12b2 100644 --- a/app/models/fluentd/setting_archive/note.rb +++ b/app/models/fluentd/setting_archive/note.rb @@ -12,6 +12,18 @@ class Fluentd def initialize(file_path) @file_path = file_path end + + def update(content) + begin + File.open(@file_path, "w") do |f| + f.write content + end + rescue => ex + Rails.logger.error ex.message + return false + end + true + end end end end From 7e062f860f833dfb9ccbe21dedfec05e998f8102 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:05:09 +0900 Subject: [PATCH 07/22] Add action to update a note. --- .../fluentd/settings/notes_controller.rb | 20 +++++++++++++++++++ config/locales/translation_en.yml | 2 ++ config/locales/translation_ja.yml | 2 ++ config/routes.rb | 2 ++ 4 files changed, 26 insertions(+) create mode 100644 app/controllers/fluentd/settings/notes_controller.rb diff --git a/app/controllers/fluentd/settings/notes_controller.rb b/app/controllers/fluentd/settings/notes_controller.rb new file mode 100644 index 0000000..e1271d9 --- /dev/null +++ b/app/controllers/fluentd/settings/notes_controller.rb @@ -0,0 +1,20 @@ +class Fluentd::Settings::NotesController < ApplicationController + before_action :login_required + before_action :find_fluentd + before_action :find_note, only: [:update] + + def update + if @note.update(params[:note][:content]) + flash[:success] = t('messages.note_updating_success') + else + flash[:error] = t('errors.messages.note_updating_failed') + end + redirect_to daemon_setting_path + end + + private + + def find_note + @note = Fluentd::SettingArchive::Note.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id]) + end +end diff --git a/config/locales/translation_en.yml b/config/locales/translation_en.yml index e29beaf..66fad7e 100644 --- a/config/locales/translation_en.yml +++ b/config/locales/translation_en.yml @@ -12,6 +12,7 @@ en: fluentd_status_running: Running fluentd_status_stopped: Stopped config_successfully_copied: "Config has been copied successfully. Please restart %{brand} to use the new config" + note_updating_success: "Note successfully updated." terms: &terms name: Name @@ -284,6 +285,7 @@ en: is_a_directory: is a directory. should be a file duplicated_conf: Configurations is duplicated out_forward_blank_server: server settings should be at least one + note_updating_failed: Updating the note is failed. See application logs. activemodel: &activemodel errors: diff --git a/config/locales/translation_ja.yml b/config/locales/translation_ja.yml index 3e3d63d..cb0d840 100644 --- a/config/locales/translation_ja.yml +++ b/config/locales/translation_ja.yml @@ -12,6 +12,7 @@ ja: fluentd_status_running: 稼働中 fluentd_status_stopped: 停止中 config_successfully_copied: "設定をコピーしました。反映させるには、 %{brand}を再起動してください。" + note_updating_success: "メモを更新しました。" terms: &terms name: アカウント名 @@ -289,6 +290,7 @@ ja: is_a_directory: はディレクトリです。ファイルを指定してください duplicated_conf: 同じ内容の設定がすでに存在しています out_forward_blank_server: serverの設定がありません。 + note_updating_failed: メモの更新に失敗しました。ログを確認して下さい。 activemodel: &activemodel errors: diff --git a/config/routes.rb b/config/routes.rb index 1f86f7a..42ea789 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,6 +68,8 @@ Rails.application.routes.draw do post "reuse", action: 'reuse', on: :member, as: 'reuse' end + resources :notes, only: [:update], module: :settings, controller: :notes + resource :running_backup, only: [:show], module: :settings, controller: :running_backup do post "reuse", action: 'reuse', on: :member, as: 'reuse' end From 9a0f01131d7963701f189a913a2ea4891755ce96 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:11:54 +0900 Subject: [PATCH 08/22] Add form to update a note of a history. --- app/views/fluentd/settings/histories/_list.html.haml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index 0393b28..5aae8b0 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -4,3 +4,6 @@ = link_to(file.name, daemon_setting_history_path(id: file.file_id)) %label= t('terms.backup_time') = file.ctime.strftime(I18n.t 'time.formats.default') + = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch, id: "note-#{file.note.file_id}" do |f| + = f.text_field :content, value: file.note.content + = submit_tag t('terms.save'), class: 'btn' From acb1c17fc142343a9c60546be35c505215f058ed Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:32:38 +0900 Subject: [PATCH 09/22] Show list of config histories with table layout. --- .../settings/histories/_list.html.haml | 21 ++++++++++++------- config/locales/translation_en.yml | 2 ++ config/locales/translation_ja.yml | 2 ++ .../fluentd/setting/hisotries_spec.rb | 4 ++-- spec/features/setting_spec.rb | 4 ++-- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index 5aae8b0..5782993 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -1,9 +1,14 @@ -%ul +%table.table.table-hover.table-bordered + %tbody + %tr + %th= t('terms.backup_file') + %th= t('terms.backup_time') + %th= t('terms.note') - @backup_files.each do |file| - %li - = link_to(file.name, daemon_setting_history_path(id: file.file_id)) - %label= t('terms.backup_time') - = file.ctime.strftime(I18n.t 'time.formats.default') - = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch, id: "note-#{file.note.file_id}" do |f| - = f.text_field :content, value: file.note.content - = submit_tag t('terms.save'), class: 'btn' + %tr + %td= link_to(file.name, daemon_setting_history_path(id: file.file_id)) + %td= file.ctime.strftime(I18n.t 'time.formats.default') + %td + = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f| + = f.text_field :content, value: file.note.content + = submit_tag t('terms.save'), class: 'btn' diff --git a/config/locales/translation_en.yml b/config/locales/translation_en.yml index 66fad7e..762155e 100644 --- a/config/locales/translation_en.yml +++ b/config/locales/translation_en.yml @@ -60,7 +60,9 @@ en: notice_restart_for_config_edit: "NOTICE: running %{brand} will restart after update config" lines: Lines languages: Language + backup_file: Backup File backup_time: Backed up at + note: Note reuse: reuse plugins: diff --git a/config/locales/translation_ja.yml b/config/locales/translation_ja.yml index cb0d840..c28960c 100644 --- a/config/locales/translation_ja.yml +++ b/config/locales/translation_ja.yml @@ -60,7 +60,9 @@ ja: notice_restart_for_config_edit: "※更新すると稼働中の%{brand}が再起動されます" lines: 行 languages: 言語 + backup_file: バックアップファイル backup_time: バックアップ日時 + note: メモ reuse: 再利用 plugins: diff --git a/spec/features/fluentd/setting/hisotries_spec.rb b/spec/features/fluentd/setting/hisotries_spec.rb index 26cb94b..a8de6db 100644 --- a/spec/features/fluentd/setting/hisotries_spec.rb +++ b/spec/features/fluentd/setting/hisotries_spec.rb @@ -15,11 +15,11 @@ describe "histories", stub: :daemon do it 'show histories#index' do page.should have_css('h1', text: I18n.t('fluentd.settings.histories.index.page_title')) - expect(all('.row li').count).to eq 9 #links to hisotries#show + expect(all('.row tr').count).to eq 9 + 1 # links to hisotries#show + 1 table header end it 'will go to histories#show' do - all('.row li a').first.click + all('.row tr td a').first.click page.should have_css('h1', text: I18n.t('fluentd.settings.histories.show.page_title')) end diff --git a/spec/features/setting_spec.rb b/spec/features/setting_spec.rb index 0f557af..c3e134b 100644 --- a/spec/features/setting_spec.rb +++ b/spec/features/setting_spec.rb @@ -17,7 +17,7 @@ describe 'setting', stub: :daemon do page.should have_css('h1', text: I18n.t('fluentd.settings.show.page_title')) page.should have_link(I18n.t('terms.edit')) page.should have_css('pre', text: 'GREAT CONFIG HERE') - expect(all('.row li').count).to eq Settings.histories_count_in_preview #links to hisotries#show + expect(all('.row tr').count).to eq Settings.histories_count_in_preview + 1 # links to hisotries#show + 1 table header page.should have_link(I18n.t('fluentd.settings.show.link_to_histories')) page.should have_text(I18n.t('fluentd.settings.running_backup.title')) end @@ -29,7 +29,7 @@ describe 'setting', stub: :daemon do end it 'will go to histories#show' do - all('.row li a').first.click + all('.row tr td a').first.click page.should have_css('h1', text: I18n.t('fluentd.settings.histories.show.page_title')) end From b8ca05a16c8b8e756ffdf8d1d3b7d99868f0cdbd Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:48:44 +0900 Subject: [PATCH 10/22] Touch an empty file if note doesn't exist. --- app/models/fluentd/setting_archive/backup_file.rb | 2 +- app/models/fluentd/setting_archive/note.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/fluentd/setting_archive/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb index f0c310c..b2d2f61 100644 --- a/app/models/fluentd/setting_archive/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -13,7 +13,7 @@ class Fluentd def initialize(file_path, note = nil) @file_path = file_path - @note = note || Note.new(file_path.sub(/#{FILE_EXTENSION}$/, Note::FILE_EXTENSION)) + @note = note || Note.create(file_path.sub(/#{FILE_EXTENSION}$/, Note::FILE_EXTENSION)) end end end diff --git a/app/models/fluentd/setting_archive/note.rb b/app/models/fluentd/setting_archive/note.rb index 0cc12b2..4aee755 100644 --- a/app/models/fluentd/setting_archive/note.rb +++ b/app/models/fluentd/setting_archive/note.rb @@ -9,6 +9,11 @@ class Fluentd new(file_path_of(backup_dir, file_id)) end + def self.create(file_path) + FileUtils.touch(file_path) + new(file_path) + end + def initialize(file_path) @file_path = file_path end From fada5d185cfdaa26a1754f2c93a41705ee8d5a05 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:53:56 +0900 Subject: [PATCH 11/22] Use simpler translation method. --- app/views/fluentd/settings/histories/_list.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index 5782993..fb3f5b0 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -7,7 +7,7 @@ - @backup_files.each do |file| %tr %td= link_to(file.name, daemon_setting_history_path(id: file.file_id)) - %td= file.ctime.strftime(I18n.t 'time.formats.default') + %td= file.ctime.strftime t('time.formats.default') %td = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f| = f.text_field :content, value: file.note.content From 8b22f2d892fd395147051f935d5e484084eb4a12 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 14:58:12 +0900 Subject: [PATCH 12/22] Show the note on also hisotory's detail page. --- app/views/fluentd/settings/histories/show.html.haml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/views/fluentd/settings/histories/show.html.haml b/app/views/fluentd/settings/histories/show.html.haml index 73992af..99b5144 100644 --- a/app/views/fluentd/settings/histories/show.html.haml +++ b/app/views/fluentd/settings/histories/show.html.haml @@ -3,6 +3,13 @@ = icon('fa-pencil') = t("terms.reuse") +- if @backup_file.note.content.present? + .row + .col-xs-12 + %pre + = preserve do + = @backup_file.note.content + .row .col-xs-12 %pre From 974869a6a8a7c24776cc7a0a623c5fdc02bbf755 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 15:17:29 +0900 Subject: [PATCH 13/22] Escape argument not to substitute unintended path. --- app/models/fluentd/setting_archive/backup_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/fluentd/setting_archive/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb index b2d2f61..6e53063 100644 --- a/app/models/fluentd/setting_archive/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -13,7 +13,7 @@ class Fluentd def initialize(file_path, note = nil) @file_path = file_path - @note = note || Note.create(file_path.sub(/#{FILE_EXTENSION}$/, Note::FILE_EXTENSION)) + @note = note || Note.create(file_path.sub(/#{Regexp.escape(FILE_EXTENSION)}$/, Note::FILE_EXTENSION)) end end end From 7752c7c4d7039c1dda7965de62e2f82e23c16a13 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 16:10:22 +0900 Subject: [PATCH 14/22] Add css class instead of id due to multiple forms. --- app/views/fluentd/settings/histories/_list.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index fb3f5b0..d865fe3 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -10,5 +10,5 @@ %td= file.ctime.strftime t('time.formats.default') %td = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f| - = f.text_field :content, value: file.note.content + = f.text_field :content, value: file.note.content, class: "note-content", id: nil = submit_tag t('terms.save'), class: 'btn' From 6b4cfaf3a1752f678b25dc06538472d0b4278968 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 16:13:38 +0900 Subject: [PATCH 15/22] Add feature spec for updating a note of a config history. --- spec/features/fluentd/setting/notes_spec.rb | 27 +++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 spec/features/fluentd/setting/notes_spec.rb diff --git a/spec/features/fluentd/setting/notes_spec.rb b/spec/features/fluentd/setting/notes_spec.rb new file mode 100644 index 0000000..a869416 --- /dev/null +++ b/spec/features/fluentd/setting/notes_spec.rb @@ -0,0 +1,27 @@ +require "spec_helper" + +describe "notes", stub: :daemon do + let!(:exists_user) { build(:user) } + include_context 'daemon has some config histories' + + before { login_with exists_user } + + describe 'update' do + let(:note_field) { ".note-content" } + let(:updating_content) { "This config file is for ..." } + + before do + visit '/daemon/setting/histories' + within first("form") do + first(note_field).set updating_content + click_button(I18n.t('terms.save')) + end + end + + it "update a content of a note" do + within first("form") do + first(note_field).value.should eq updating_content + end + end + end +end From 7f20ba640ca904d59f6de12546262a5dbf22101f Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 16:48:04 +0900 Subject: [PATCH 16/22] Set proper class for submit button. --- app/views/fluentd/settings/histories/_list.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index d865fe3..afd96b2 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -11,4 +11,4 @@ %td = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f| = f.text_field :content, value: file.note.content, class: "note-content", id: nil - = submit_tag t('terms.save'), class: 'btn' + = submit_tag t('terms.save'), class: 'btn btn-default btn-sm pull-right' From fdcb704bec948f12ba6f48484bf43ef6b1490cad Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 17:14:10 +0900 Subject: [PATCH 17/22] Escape argument not to substitute unintended path (fix another again). --- app/models/concerns/fluentd/setting_archive/archivable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/fluentd/setting_archive/archivable.rb b/app/models/concerns/fluentd/setting_archive/archivable.rb index fa530f4..22aa427 100644 --- a/app/models/concerns/fluentd/setting_archive/archivable.rb +++ b/app/models/concerns/fluentd/setting_archive/archivable.rb @@ -15,7 +15,7 @@ class Fluentd end def file_id - @file_id ||= with_file { name.gsub(/#{self.class::FILE_EXTENSION}\Z/,'') } + @file_id ||= with_file { name.gsub(/#{Regexp.escape(self.class::FILE_EXTENSION)}\Z/,'') } end def name From 336e22bea7d8040aa202be3639e8f8b13a6833c3 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 17:27:10 +0900 Subject: [PATCH 18/22] Change termination regular expression properly. --- app/models/fluentd/setting_archive/backup_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/fluentd/setting_archive/backup_file.rb b/app/models/fluentd/setting_archive/backup_file.rb index 6e53063..d535bb0 100644 --- a/app/models/fluentd/setting_archive/backup_file.rb +++ b/app/models/fluentd/setting_archive/backup_file.rb @@ -13,7 +13,7 @@ class Fluentd def initialize(file_path, note = nil) @file_path = file_path - @note = note || Note.create(file_path.sub(/#{Regexp.escape(FILE_EXTENSION)}$/, Note::FILE_EXTENSION)) + @note = note || Note.create(file_path.sub(/#{Regexp.escape(FILE_EXTENSION)}\z/, Note::FILE_EXTENSION)) end end end From 9cc12f7bc38bb2b2bf0d326ad4425b155555a7a9 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 17:34:02 +0900 Subject: [PATCH 19/22] Show exceptions if updating note fails. --- app/controllers/fluentd/settings/notes_controller.rb | 8 ++------ app/models/fluentd/setting_archive/note.rb | 12 +++--------- config/locales/translation_en.yml | 1 - config/locales/translation_ja.yml | 1 - 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/app/controllers/fluentd/settings/notes_controller.rb b/app/controllers/fluentd/settings/notes_controller.rb index e1271d9..3846922 100644 --- a/app/controllers/fluentd/settings/notes_controller.rb +++ b/app/controllers/fluentd/settings/notes_controller.rb @@ -4,12 +4,8 @@ class Fluentd::Settings::NotesController < ApplicationController before_action :find_note, only: [:update] def update - if @note.update(params[:note][:content]) - flash[:success] = t('messages.note_updating_success') - else - flash[:error] = t('errors.messages.note_updating_failed') - end - redirect_to daemon_setting_path + @note.update!(params[:note][:content]) + redirect_to daemon_setting_path, flash: { success: t('messages.note_updating_success') } end private diff --git a/app/models/fluentd/setting_archive/note.rb b/app/models/fluentd/setting_archive/note.rb index 4aee755..94d9c0e 100644 --- a/app/models/fluentd/setting_archive/note.rb +++ b/app/models/fluentd/setting_archive/note.rb @@ -18,16 +18,10 @@ class Fluentd @file_path = file_path end - def update(content) - begin - File.open(@file_path, "w") do |f| - f.write content - end - rescue => ex - Rails.logger.error ex.message - return false + def update!(content) + File.open(@file_path, "w") do |f| + f.write content end - true end end end diff --git a/config/locales/translation_en.yml b/config/locales/translation_en.yml index 762155e..de25bf3 100644 --- a/config/locales/translation_en.yml +++ b/config/locales/translation_en.yml @@ -287,7 +287,6 @@ en: is_a_directory: is a directory. should be a file duplicated_conf: Configurations is duplicated out_forward_blank_server: server settings should be at least one - note_updating_failed: Updating the note is failed. See application logs. activemodel: &activemodel errors: diff --git a/config/locales/translation_ja.yml b/config/locales/translation_ja.yml index c28960c..6d572be 100644 --- a/config/locales/translation_ja.yml +++ b/config/locales/translation_ja.yml @@ -292,7 +292,6 @@ ja: is_a_directory: はディレクトリです。ファイルを指定してください duplicated_conf: 同じ内容の設定がすでに存在しています out_forward_blank_server: serverの設定がありません。 - note_updating_failed: メモの更新に失敗しました。ログを確認して下さい。 activemodel: &activemodel errors: From 7ba644359469cb5ae97c640cd3e77a0944ca0d2c Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 17:39:01 +0900 Subject: [PATCH 20/22] Change pre tag for historiy's note to p tag as visible. --- app/views/fluentd/settings/histories/show.html.haml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/views/fluentd/settings/histories/show.html.haml b/app/views/fluentd/settings/histories/show.html.haml index 99b5144..ace0457 100644 --- a/app/views/fluentd/settings/histories/show.html.haml +++ b/app/views/fluentd/settings/histories/show.html.haml @@ -6,9 +6,7 @@ - if @backup_file.note.content.present? .row .col-xs-12 - %pre - = preserve do - = @backup_file.note.content + %p= @backup_file.note.content .row .col-xs-12 From 24811edec4c0afa77e289b6deda25c56463cb392 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 17:56:03 +0900 Subject: [PATCH 21/22] Adjust field with submit button. --- app/assets/stylesheets/common.css.scss | 4 ++++ app/views/fluentd/settings/histories/_list.html.haml | 8 +++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/common.css.scss b/app/assets/stylesheets/common.css.scss index 579521e..9e890e5 100644 --- a/app/assets/stylesheets/common.css.scss +++ b/app/assets/stylesheets/common.css.scss @@ -165,3 +165,7 @@ label { color: #999; } } + +.datetime { + white-space: nowrap; +} diff --git a/app/views/fluentd/settings/histories/_list.html.haml b/app/views/fluentd/settings/histories/_list.html.haml index afd96b2..c99991c 100644 --- a/app/views/fluentd/settings/histories/_list.html.haml +++ b/app/views/fluentd/settings/histories/_list.html.haml @@ -7,8 +7,10 @@ - @backup_files.each do |file| %tr %td= link_to(file.name, daemon_setting_history_path(id: file.file_id)) - %td= file.ctime.strftime t('time.formats.default') + %td.datetime= file.ctime.strftime t('time.formats.default') %td = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f| - = f.text_field :content, value: file.note.content, class: "note-content", id: nil - = submit_tag t('terms.save'), class: 'btn btn-default btn-sm pull-right' + .form-group.input-group + = f.text_field :content, value: file.note.content, class: "note-content form-control", id: nil + %span.input-group-btn + = submit_tag t('terms.save'), class: 'btn btn-default' From 5de418539fb7ad8449897bcd0f2bb09c01ed3385 Mon Sep 17 00:00:00 2001 From: hassaku Date: Wed, 7 Jan 2015 18:23:15 +0900 Subject: [PATCH 22/22] Remove also note file at the same time as removing backup config file. --- app/models/fluentd/agent/local_common.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/fluentd/agent/local_common.rb b/app/models/fluentd/agent/local_common.rb index 14823e4..231d430 100644 --- a/app/models/fluentd/agent/local_common.rb +++ b/app/models/fluentd/agent/local_common.rb @@ -89,8 +89,9 @@ class Fluentd return if over_file_count <= 0 backup_files_in_old_order.first(over_file_count).each do |file| - next unless File.exist? file - FileUtils.rm(file) + note_file_attached_backup = file.sub(/#{Regexp.escape(File.extname(file))}\z/, ::Fluentd::SettingArchive::Note::FILE_EXTENSION) + FileUtils.rm(note_file_attached_backup) if File.exist? note_file_attached_backup + FileUtils.rm(file) if File.exist? file end end