diff --git a/apps/dashboard/app/models/launcher.rb b/apps/dashboard/app/models/launcher.rb index b8286cf57d..97ca5bc72a 100644 --- a/apps/dashboard/app/models/launcher.rb +++ b/apps/dashboard/app/models/launcher.rb @@ -60,14 +60,13 @@ def scripts?(project_dir) ID_REX = /\A\w{8}\Z/.freeze - validates(:id, format: { with: ID_REX, allow_blank: true, message: :format }, on: [:save]) - validates(:id, format: { with: ID_REX, message: :format }, on: [:update]) + validates(:id, format: { with: ID_REX, message: "ID does not match #{Launcher::ID_REX.inspect}" }, on: [:save]) def initialize(opts = {}) opts = opts.to_h.with_indifferent_access @project_dir = opts[:project_dir] || raise(StandardError, 'You must set the project directory') - @id = opts[:id] if opts[:id].to_s.empty? || opts[:id].to_s.match?(ID_REX) + @id = opts[:id].to_s.match?(ID_REX) ? opts[:id].to_s : Launcher.next_id @title = opts[:title].to_s @created_at = opts[:created_at] sm_opts = { @@ -149,9 +148,11 @@ def []=(_id, value) end def save - @id = Launcher.next_id if @id.nil? || !@id.to_s.match?(ID_REX) + return false unless valid?(:save) + @created_at = Time.now.to_i if @created_at.nil? script_path = Launcher.script_path(project_dir, id) + script_path.mkpath unless script_path.exist? File.write(Launcher.script_form_file(script_path), to_yaml) @@ -223,6 +224,10 @@ def create_default_script private def self.script_path(root_dir, script_id) + unless script_id.to_s.match?(ID_REX) + raise(StandardError, "#{script_id} is invalid. Does not match #{ID_REX.inspect}") + end + Pathname.new(File.join(Launcher.scripts_dir(root_dir), script_id.to_s)) end diff --git a/apps/dashboard/test/models/launcher_test.rb b/apps/dashboard/test/models/launcher_test.rb index 7432eb7127..39c6dc4284 100644 --- a/apps/dashboard/test/models/launcher_test.rb +++ b/apps/dashboard/test/models/launcher_test.rb @@ -91,12 +91,16 @@ class LauncherTest < ActiveSupport::TestCase end end - test 'launchers will not assign wrong id' do + test 'launchers will re-assign wrong id' do Dir.mktmpdir do |tmp| projects_path = Pathname.new(tmp) OodAppkit.stubs(:dataroot).returns(projects_path) - launcher = Launcher.new({ project_dir: projects_path.to_s, id: '1234', title: 'Test Script' }) - assert_nil(launcher.id) + bad_id = '1234' + launcher = Launcher.new({ project_dir: projects_path.to_s, id: bad_id, title: 'Test Script' }) + + assert(launcher.id.to_s.match?(Launcher::ID_REX)) + refute(bad_id.match?(Launcher::ID_REX)) + refute(bad_id.to_s == launcher.id) end end @@ -130,4 +134,23 @@ class LauncherTest < ActiveSupport::TestCase assert_equal false, Pathname(File.join(projects_path, 'hello_world.sh')).exist? end end + + test 'will not save even if id is resest' do + Dir.mktmpdir do |tmp| + bad_id = '1234' + launcher = Launcher.new({ project_dir: tmp.to_s, id: bad_id, title: 'Default Script' }) + + # initializer reset the id, but we can reset it + refute(launcher.id.to_s == bad_id.to_s) + launcher.instance_variable_set('@id', bad_id) + assert_equal(launcher.id, bad_id) + assert(launcher.errors.size, 0) + + # now try to save it, and it fails + refute(launcher.save) + assert(launcher.errors.size, 1) + assert_equal(launcher.errors.full_messages[0], "Id ID does not match #{Launcher::ID_REX.inspect}") + assert(Dir.empty?(Launcher.scripts_dir(tmp).to_s)) + end + end end diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index 10937f27bd..a858e6e2a6 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -727,18 +727,18 @@ def add_auto_environment_variable(project_id, script_id, save: true) test 'cant show invalid script' do Dir.mktmpdir do |dir| project_id = setup_project(dir) - visit project_launcher_path(project_id, '1') + visit project_launcher_path(project_id, '12345678') assert_current_path("/projects/#{project_id}") - assert_selector('.alert-danger', text: "Close\nCannot find script 1") + assert_selector('.alert-danger', text: "Close\nCannot find script 12345678") end end test 'cant edit invalid script' do Dir.mktmpdir do |dir| project_id = setup_project(dir) - visit edit_project_launcher_path(project_id, '1') + visit edit_project_launcher_path(project_id, '12345678') assert_current_path("/projects/#{project_id}") - assert_selector('.alert-danger', text: "Close\nCannot find script 1") + assert_selector('.alert-danger', text: "Close\nCannot find script 12345678") end end