Merge branch '9820-cwl-poll-jobs' closes #9820
authorPeter Amstutz <peter.amstutz@curoverse.com>
Fri, 19 Aug 2016 15:13:51 +0000 (11:13 -0400)
committerPeter Amstutz <peter.amstutz@curoverse.com>
Fri, 19 Aug 2016 15:13:51 +0000 (11:13 -0400)
15 files changed:
apps/workbench/app/controllers/workflows_controller.rb [new file with mode: 0644]
apps/workbench/app/models/workflow.rb [new file with mode: 0644]
apps/workbench/config/routes.rb
apps/workbench/test/controllers/workflows_controller_test.rb [new file with mode: 0644]
services/api/Gemfile
services/api/Gemfile.lock
services/api/app/controllers/arvados/v1/workflows_controller.rb [new file with mode: 0644]
services/api/app/models/arvados_model.rb
services/api/app/models/collection.rb
services/api/app/models/workflow.rb [new file with mode: 0644]
services/api/config/routes.rb
services/api/db/migrate/20160808151559_create_workflows.rb [new file with mode: 0644]
services/api/db/structure.sql
services/api/test/fixtures/workflows.yml [new file with mode: 0644]
services/api/test/unit/workflow_test.rb [new file with mode: 0644]

diff --git a/apps/workbench/app/controllers/workflows_controller.rb b/apps/workbench/app/controllers/workflows_controller.rb
new file mode 100644 (file)
index 0000000..94ae8a9
--- /dev/null
@@ -0,0 +1,2 @@
+class WorkflowsController < ApplicationController
+end
diff --git a/apps/workbench/app/models/workflow.rb b/apps/workbench/app/models/workflow.rb
new file mode 100644 (file)
index 0000000..553f141
--- /dev/null
@@ -0,0 +1,5 @@
+class Workflow < ArvadosBase
+  def self.goes_in_projects?
+    true
+  end
+end
index 3a756e9939014cfc78d0053537641636e52d89cb..5d02e57ac74c42d72b7e7ee5ebf3ed8dcee8588e 100644 (file)
@@ -99,6 +99,8 @@ ArvadosWorkbench::Application.routes.draw do
     get 'choose', :on => :collection
   end
 
+  resources :workflows
+
   post 'actions' => 'actions#post'
   get 'actions' => 'actions#show'
   get 'websockets' => 'websocket#index'
diff --git a/apps/workbench/test/controllers/workflows_controller_test.rb b/apps/workbench/test/controllers/workflows_controller_test.rb
new file mode 100644 (file)
index 0000000..654f317
--- /dev/null
@@ -0,0 +1,9 @@
+require 'test_helper'
+
+class WorkflowsControllerTest < ActionController::TestCase
+  test "index" do
+    get :index, {}, session_for(:active)
+    assert_response :success
+    assert_includes @response.body, 'Valid workflow with no workflow yaml'
+  end
+end
index 1e25467e7485ed05337e2b212f4cf4fc4bc44d1e..4d03da31b2275ab89e4c041867d01cf56cc851e0 100644 (file)
@@ -80,3 +80,4 @@ gem 'pg_power'
 
 gem 'puma'
 gem 'sshkey'
+gem 'safe_yaml'
index 3715718717b7c85495717739bce5b2d8e58261d7..77e876e926f0991193960aa646cef805b133d556 100644 (file)
@@ -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/controllers/arvados/v1/workflows_controller.rb b/services/api/app/controllers/arvados/v1/workflows_controller.rb
new file mode 100644 (file)
index 0000000..5177d0a
--- /dev/null
@@ -0,0 +1,2 @@
+class Arvados::V1::WorkflowsController < ApplicationController
+end
index 5533143ac9aae52bb19147af390d93b7bdb1b97e..2908e40e335ee87fc4bf78c3280ae823e6d9a759 100644 (file)
@@ -104,6 +104,10 @@ class ArvadosModel < ActiveRecord::Base
     api_column_map
   end
 
+  def self.ignored_select_attributes
+    ["href", "kind", "etag"]
+  end
+
   def self.columns_for_attributes(select_attributes)
     if select_attributes.empty?
       raise ArgumentError.new("Attribute selection list cannot be empty")
@@ -111,7 +115,7 @@ class ArvadosModel < ActiveRecord::Base
     api_column_map = attributes_required_columns
     invalid_attrs = []
     select_attributes.each do |s|
-      next if ["href", "kind", "etag"].include? s
+      next if ignored_select_attributes.include? s
       if not s.is_a? String or not api_column_map.include? s
         invalid_attrs << s
       end
@@ -404,15 +408,16 @@ class ArvadosModel < ActiveRecord::Base
       x.each do |k,v|
         return true if has_symbols?(k) or has_symbols?(v)
       end
-      false
     elsif x.is_a? Array
       x.each do |k|
         return true if has_symbols?(k)
       end
-      false
-    else
-      (x.class == Symbol)
+    elsif x.is_a? Symbol
+      return true
+    elsif x.is_a? String
+      return true if x.start_with?(':') && !x.start_with?('::')
     end
+    false
   end
 
   def self.recursive_stringify x
@@ -426,6 +431,8 @@ class ArvadosModel < ActiveRecord::Base
       end
     elsif x.is_a? Symbol
       x.to_s
+    elsif x.is_a? String and x.start_with?(':') and !x.start_with?('::')
+      x[1..-1]
     else
       x
     end
index 248c8b98245bd5628096fa60ba0f3ed2b7ea66e9..d0001fdc32bd5826ff607d3a6a193cc74724071a 100644 (file)
@@ -46,6 +46,10 @@ class Collection < ArvadosModel
                 )
   end
 
+  def self.ignored_select_attributes
+    super + ["updated_at", "file_names"]
+  end
+
   FILE_TOKEN = /^[[:digit:]]+:[[:digit:]]+:/
   def check_signatures
     return false if self.manifest_text.nil?
diff --git a/services/api/app/models/workflow.rb b/services/api/app/models/workflow.rb
new file mode 100644 (file)
index 0000000..ea3e985
--- /dev/null
@@ -0,0 +1,42 @@
+class Workflow < ArvadosModel
+  include HasUuid
+  include KindAndEtag
+  include CommonApiTemplate
+
+  validate :validate_workflow
+  before_save :set_name_and_description
+
+  api_accessible :user, extend: :common do |t|
+    t.add :name
+    t.add :description
+    t.add :workflow
+  end
+
+  def validate_workflow
+    begin
+      @workflow_yaml = YAML.load self.workflow if !workflow.nil?
+    rescue => e
+      errors.add :workflow, "is not valid yaml: #{e.message}"
+    end
+  end
+
+  def set_name_and_description
+    old_wf = {}
+    begin
+      old_wf = YAML.load self.workflow_was if !self.workflow_was.nil?
+    rescue => e
+      logger.warn "set_name_and_description error: #{e.message}"
+      return
+    end
+
+    ['name', 'description'].each do |a|
+      if !self.changes.include?(a)
+        v = self.read_attribute(a)
+        if !v.present? or v == old_wf[a]
+          val = @workflow_yaml[a] if self.workflow and @workflow_yaml
+          self[a] = val
+        end
+      end
+    end
+  end
+end
index ed8f8d89af9c2d429dfe661633df2ec36a0484e9..7bf75800b4af909e9537e845a1d40a43187fea9b 100644 (file)
@@ -52,6 +52,7 @@ Server::Application.routes.draw do
       end
       resources :pipeline_instances
       resources :pipeline_templates
+      resources :workflows
       resources :repositories do
         get 'get_all_permissions', on: :collection
       end
diff --git a/services/api/db/migrate/20160808151559_create_workflows.rb b/services/api/db/migrate/20160808151559_create_workflows.rb
new file mode 100644 (file)
index 0000000..23319b6
--- /dev/null
@@ -0,0 +1,30 @@
+class CreateWorkflows < ActiveRecord::Migration
+  def up
+    create_table :workflows do |t|
+      t.string :uuid
+      t.string :owner_uuid
+      t.datetime :created_at
+      t.datetime :modified_at
+      t.string :modified_by_client_uuid
+      t.string :modified_by_user_uuid
+      t.string :name
+      t.text :description
+      t.text :workflow
+
+      t.timestamps
+    end
+
+    add_index :workflows, :uuid, :unique => true
+    add_index :workflows, :owner_uuid
+    add_index :workflows, ["uuid", "owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "name"], name: 'workflows_search_idx'
+    execute "CREATE INDEX workflows_full_text_search_idx ON workflows USING gin(#{Workflow.full_text_tsvector});"
+  end
+
+  def down
+    remove_index :workflows, :name => 'workflows_full_text_search_idx'
+    remove_index :workflows, :name => 'workflows_search_idx'
+    remove_index :workflows, :owner_uuid
+    remove_index :workflows, :uuid
+    drop_table :workflows
+  end
+end
index 4bf4a173bd9d1c31e04d0f7517c1927baf9f3ff2..1d7a2c67921061312e377d6e30cfc6fa4da576de 100644 (file)
@@ -1054,6 +1054,44 @@ CREATE SEQUENCE virtual_machines_id_seq
 ALTER SEQUENCE virtual_machines_id_seq OWNED BY virtual_machines.id;
 
 
+--
+-- Name: workflows; Type: TABLE; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE TABLE workflows (
+    id integer NOT NULL,
+    uuid character varying(255),
+    owner_uuid character varying(255),
+    created_at timestamp without time zone NOT NULL,
+    modified_at timestamp without time zone,
+    modified_by_client_uuid character varying(255),
+    modified_by_user_uuid character varying(255),
+    name character varying(255),
+    description text,
+    workflow text,
+    updated_at timestamp without time zone NOT NULL
+);
+
+
+--
+-- Name: workflows_id_seq; Type: SEQUENCE; Schema: public; Owner: -
+--
+
+CREATE SEQUENCE workflows_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+
+--
+-- Name: workflows_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
+--
+
+ALTER SEQUENCE workflows_id_seq OWNED BY workflows.id;
+
+
 --
 -- Name: id; Type: DEFAULT; Schema: public; Owner: -
 --
@@ -1222,6 +1260,13 @@ ALTER TABLE ONLY users ALTER COLUMN id SET DEFAULT nextval('users_id_seq'::regcl
 ALTER TABLE ONLY virtual_machines ALTER COLUMN id SET DEFAULT nextval('virtual_machines_id_seq'::regclass);
 
 
+--
+-- Name: id; Type: DEFAULT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY workflows ALTER COLUMN id SET DEFAULT nextval('workflows_id_seq'::regclass);
+
+
 --
 -- Name: api_client_authorizations_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: 
 --
@@ -1414,6 +1459,14 @@ ALTER TABLE ONLY virtual_machines
     ADD CONSTRAINT virtual_machines_pkey PRIMARY KEY (id);
 
 
+--
+-- Name: workflows_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: 
+--
+
+ALTER TABLE ONLY workflows
+    ADD CONSTRAINT workflows_pkey PRIMARY KEY (id);
+
+
 --
 -- Name: api_client_authorizations_search_index; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
@@ -2191,6 +2244,20 @@ CREATE INDEX index_virtual_machines_on_owner_uuid ON virtual_machines USING btre
 CREATE UNIQUE INDEX index_virtual_machines_on_uuid ON virtual_machines USING btree (uuid);
 
 
+--
+-- Name: index_workflows_on_owner_uuid; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX index_workflows_on_owner_uuid ON workflows USING btree (owner_uuid);
+
+
+--
+-- Name: index_workflows_on_uuid; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE UNIQUE INDEX index_workflows_on_uuid ON workflows USING btree (uuid);
+
+
 --
 -- Name: job_tasks_search_index; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
@@ -2331,6 +2398,20 @@ CREATE INDEX users_search_index ON users USING btree (uuid, owner_uuid, modified
 CREATE INDEX virtual_machines_search_index ON virtual_machines USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, hostname);
 
 
+--
+-- Name: workflows_full_text_search_idx; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX workflows_full_text_search_idx ON workflows USING gin (to_tsvector('english'::regconfig, (((((((((((((' '::text || (COALESCE(uuid, ''::character varying))::text) || ' '::text) || (COALESCE(owner_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_user_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || COALESCE(description, ''::text)) || ' '::text) || COALESCE(workflow, ''::text))));
+
+
+--
+-- Name: workflows_search_idx; Type: INDEX; Schema: public; Owner: -; Tablespace: 
+--
+
+CREATE INDEX workflows_search_idx ON workflows USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, name);
+
+
 --
 -- PostgreSQL database dump complete
 --
@@ -2589,4 +2670,8 @@ INSERT INTO schema_migrations (version) VALUES ('20160324144017');
 
 INSERT INTO schema_migrations (version) VALUES ('20160506175108');
 
-INSERT INTO schema_migrations (version) VALUES ('20160509143250');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20160509143250');
+
+INSERT INTO schema_migrations (version) VALUES ('20160808151459');
+
+INSERT INTO schema_migrations (version) VALUES ('20160808151559');
\ No newline at end of file
diff --git a/services/api/test/fixtures/workflows.yml b/services/api/test/fixtures/workflows.yml
new file mode 100644 (file)
index 0000000..d7818f2
--- /dev/null
@@ -0,0 +1,17 @@
+workflow_with_workflow_yml:
+  uuid: zzzzz-7fd4e-validworkfloyml
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  name: Valid workflow
+  description: this work has a valid workflow 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 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
new file mode 100644 (file)
index 0000000..ac8df47
--- /dev/null
@@ -0,0 +1,125 @@
+require 'test_helper'
+
+class WorkflowTest < ActiveSupport::TestCase
+  test "create workflow with no workflow yaml" do
+    set_user_from_auth :active
+
+    wf = {
+      name: "test name",
+    }
+
+    w = Workflow.create!(wf)
+    assert_not_nil w.uuid
+  end
+
+  test "create workflow with valid workflow yaml" do
+    set_user_from_auth :active
+
+    wf = {
+      name: "test name",
+      workflow: "k1:\n v1: x\n v2: y"
+    }
+
+    w = Workflow.create!(wf)
+    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
+
+    wf = {
+      name: "test name",
+      workflow: "k1:\n v1: x\n  v2: y"
+    }
+
+    assert_raises(ActiveRecord::RecordInvalid) do
+      Workflow.create! wf
+    end
+  end
+
+  test "update workflow with invalid workflow yaml" do
+    set_user_from_auth :active
+
+    w = Workflow.find_by_uuid(workflows(:workflow_with_workflow_yml).uuid)
+    wf = "k1:\n v1: x\n  v2: y"
+
+    assert_raises(ActiveRecord::RecordInvalid) do
+      w.update_attributes!(workflow: wf)
+    end
+  end
+
+  test "update workflow and verify name and description" do
+    set_user_from_auth :active
+
+    # Workflow name and desc should be set with values from workflow yaml
+    # when it does not already have custom values for these fields
+    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
+
+    # Workflow name and desc should be set with values from workflow yaml
+    # 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
+
+    # Workflow name and desc should be set with values from workflow yaml
+    # 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
+
+    # Workflow name and desc should retain provided custom values
+    # 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
+end