[ARVADOS] updated: bb741f18407fd0190e1b6c642c14ef9f502b004e
Git user
git at public.curoverse.com
Mon Aug 15 18:23:57 EDT 2016
Summary of changes:
services/api/Gemfile | 1 +
services/api/Gemfile.lock | 2 ++
services/api/app/models/workflow.rb | 19 ++++++++---------
services/api/test/fixtures/workflows.yml | 5 +++--
services/api/test/unit/workflow_test.rb | 36 ++++++++++++++++++++++++++++++++
5 files changed, 51 insertions(+), 12 deletions(-)
via bb741f18407fd0190e1b6c642c14ef9f502b004e (commit)
from 3723f697b61ce60858455473b3a5464a2da65bfb (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit bb741f18407fd0190e1b6c642c14ef9f502b004e
Author: radhika <radhika at curoverse.com>
Date: Mon Aug 15 18:19:00 2016 -0400
9678: use safe_yaml and other updates.
diff --git a/services/api/Gemfile b/services/api/Gemfile
index 1e25467..4d03da3 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -80,3 +80,4 @@ gem 'pg_power'
gem 'puma'
gem 'sshkey'
+gem 'safe_yaml'
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 3715718..77e876e 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -182,6 +182,7 @@ GEM
ruby-prof (0.15.2)
rvm-capistrano (1.5.1)
capistrano (~> 2.15.4)
+ safe_yaml (1.0.4)
sass (3.3.4)
sass-rails (3.2.6)
railties (~> 3.2.0)
@@ -248,6 +249,7 @@ DEPENDENCIES
rails (~> 3.2.0)
ruby-prof
rvm-capistrano
+ safe_yaml
sass-rails (>= 3.2.0)
simplecov (~> 0.7.1)
simplecov-rcov
diff --git a/services/api/app/models/workflow.rb b/services/api/app/models/workflow.rb
index c899cc0..97be6dc 100644
--- a/services/api/app/models/workflow.rb
+++ b/services/api/app/models/workflow.rb
@@ -4,7 +4,7 @@ class Workflow < ArvadosModel
include CommonApiTemplate
validate :validate_workflow
- after_save :set_name_and_description
+ before_save :set_name_and_description
api_accessible :user, extend: :common do |t|
t.add :name
@@ -14,7 +14,7 @@ class Workflow < ArvadosModel
def validate_workflow
begin
- @workflow_yaml = YAML.load self.workflow if !workflow.blank?
+ @workflow_yaml = YAML.load self.workflow if !workflow.nil?
rescue
errors.add :validate_workflow, "#{self.workflow} is not valid yaml"
end
@@ -22,20 +22,19 @@ class Workflow < ArvadosModel
def set_name_and_description
begin
- old_wf = []
- old_wf = YAML.load self.workflow_was if !self.workflow_was.blank?
- changes = self.changes
- need_save = false
+ old_wf = {}
+ old_wf = YAML.load self.workflow_was if !self.workflow_was.nil?
['name', 'description'].each do |a|
- if !changes.include?(a)
+ if !self.changes.include?(a)
v = self.read_attribute(a)
if !v.present? or v == old_wf[a]
- self[a] = @workflow_yaml[a]
+ val = @workflow_yaml[a] if self.workflow and @workflow_yaml
+ self[a] = val
end
end
end
- rescue => e
- errors.add :set_name_and_description, "#{e.message}"
+ rescue ActiveRecord::RecordInvalid
+ errors.add :set_name_and_description, "#{self.workflow_was} is not valid yaml"
end
end
end
diff --git a/services/api/test/fixtures/workflows.yml b/services/api/test/fixtures/workflows.yml
index 5cd6c34..d7818f2 100644
--- a/services/api/test/fixtures/workflows.yml
+++ b/services/api/test/fixtures/workflows.yml
@@ -3,14 +3,15 @@ workflow_with_workflow_yml:
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
name: Valid workflow
description: this work has a valid workflow yaml
- workflow: this of course is a valid yaml
+ workflow: "name: foo\ndesc: bar"
workflow_with_no_workflow_yml:
uuid: zzzzz-7fd4e-validbutnoyml00
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
name: Valid workflow with no workflow yaml
- description: this work does not have a workflow yaml
+ description: this workflow does not have a workflow yaml
workflow_with_no_name_and_desc:
uuid: zzzzz-7fd4e-validnonamedesc
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ workflow: this is valid yaml
diff --git a/services/api/test/unit/workflow_test.rb b/services/api/test/unit/workflow_test.rb
index 7bcf2f6..ac8df47 100644
--- a/services/api/test/unit/workflow_test.rb
+++ b/services/api/test/unit/workflow_test.rb
@@ -24,6 +24,18 @@ class WorkflowTest < ActiveSupport::TestCase
assert_not_nil w.uuid
end
+ test "create workflow with simple string as workflow" do
+ set_user_from_auth :active
+
+ wf = {
+ name: "test name",
+ workflow: "this is valid yaml"
+ }
+
+ w = Workflow.create!(wf)
+ assert_not_nil w.uuid
+ end
+
test "create workflow with invalid workflow yaml" do
set_user_from_auth :active
@@ -56,6 +68,7 @@ class WorkflowTest < ActiveSupport::TestCase
w = Workflow.find_by_uuid(workflows(:workflow_with_no_name_and_desc).uuid)
wf = "name: test name 1\ndescription: test desc 1\nother: some more"
w.update_attributes!(workflow: wf)
+ w.reload
assert_equal "test name 1", w.name
assert_equal "test desc 1", w.description
@@ -63,6 +76,7 @@ class WorkflowTest < ActiveSupport::TestCase
# when it does not already have custom values for these fields
wf = "name: test name 2\ndescription: test desc 2\nother: some more"
w.update_attributes!(workflow: wf)
+ w.reload
assert_equal "test name 2", w.name
assert_equal "test desc 2", w.description
@@ -70,12 +84,25 @@ class WorkflowTest < ActiveSupport::TestCase
# even if it means emptying them out
wf = "more: etc"
w.update_attributes!(workflow: wf)
+ w.reload
+ assert_equal nil, w.name
+ assert_equal nil, w.description
+
+ # Workflow name and desc set using workflow yaml should be cleared
+ # if workflow yaml is cleared
+ wf = "name: test name 2\ndescription: test desc 2\nother: some more"
+ w.update_attributes!(workflow: wf)
+ w.reload
+ wf = nil
+ w.update_attributes!(workflow: wf)
+ w.reload
assert_equal nil, w.name
assert_equal nil, w.description
# Workflow name and desc should be set to provided custom values
wf = "name: test name 3\ndescription: test desc 3\nother: some more"
w.update_attributes!(name: "remains", description: "remains", workflow: wf)
+ w.reload
assert_equal "remains", w.name
assert_equal "remains", w.description
@@ -83,6 +110,15 @@ class WorkflowTest < ActiveSupport::TestCase
# and should not be overwritten by values from yaml
wf = "name: test name 4\ndescription: test desc 4\nother: some more"
w.update_attributes!(workflow: wf)
+ w.reload
+ assert_equal "remains", w.name
+ assert_equal "remains", w.description
+
+ # Workflow name and desc should retain provided custom values
+ # and not be affected by the clearing of the workflow yaml
+ wf = nil
+ w.update_attributes!(workflow: wf)
+ w.reload
assert_equal "remains", w.name
assert_equal "remains", w.description
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list