From 3723f697b61ce60858455473b3a5464a2da65bfb Mon Sep 17 00:00:00 2001 From: radhika Date: Tue, 9 Aug 2016 15:51:50 -0400 Subject: [PATCH] 9684: add workflow resource to api server --- .../app/controllers/workflows_controller.rb | 2 + apps/workbench/app/models/workflow.rb | 5 ++ apps/workbench/config/routes.rb | 2 + .../controllers/workflows_controller_test.rb | 9 ++ .../arvados/v1/workflows_controller.rb | 2 + services/api/app/models/workflow.rb | 41 +++++++++ services/api/config/routes.rb | 1 + .../20160808151559_create_workflows.rb | 30 +++++++ services/api/db/structure.sql | 87 +++++++++++++++++- services/api/test/fixtures/workflows.yml | 16 ++++ services/api/test/unit/workflow_test.rb | 89 +++++++++++++++++++ 11 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 apps/workbench/app/controllers/workflows_controller.rb create mode 100644 apps/workbench/app/models/workflow.rb create mode 100644 apps/workbench/test/controllers/workflows_controller_test.rb create mode 100644 services/api/app/controllers/arvados/v1/workflows_controller.rb create mode 100644 services/api/app/models/workflow.rb create mode 100644 services/api/db/migrate/20160808151559_create_workflows.rb create mode 100644 services/api/test/fixtures/workflows.yml create mode 100644 services/api/test/unit/workflow_test.rb diff --git a/apps/workbench/app/controllers/workflows_controller.rb b/apps/workbench/app/controllers/workflows_controller.rb new file mode 100644 index 0000000000..94ae8a95ea --- /dev/null +++ b/apps/workbench/app/controllers/workflows_controller.rb @@ -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 index 0000000000..553f141031 --- /dev/null +++ b/apps/workbench/app/models/workflow.rb @@ -0,0 +1,5 @@ +class Workflow < ArvadosBase + def self.goes_in_projects? + true + end +end diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb index 3a756e9939..5d02e57ac7 100644 --- a/apps/workbench/config/routes.rb +++ b/apps/workbench/config/routes.rb @@ -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 index 0000000000..654f3175f7 --- /dev/null +++ b/apps/workbench/test/controllers/workflows_controller_test.rb @@ -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 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 index 0000000000..5177d0a508 --- /dev/null +++ b/services/api/app/controllers/arvados/v1/workflows_controller.rb @@ -0,0 +1,2 @@ +class Arvados::V1::WorkflowsController < ApplicationController +end diff --git a/services/api/app/models/workflow.rb b/services/api/app/models/workflow.rb new file mode 100644 index 0000000000..c899cc08fa --- /dev/null +++ b/services/api/app/models/workflow.rb @@ -0,0 +1,41 @@ +class Workflow < ArvadosModel + include HasUuid + include KindAndEtag + include CommonApiTemplate + + validate :validate_workflow + after_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.blank? + rescue + errors.add :validate_workflow, "#{self.workflow} is not valid yaml" + end + end + + 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 + ['name', 'description'].each do |a| + if !changes.include?(a) + v = self.read_attribute(a) + if !v.present? or v == old_wf[a] + self[a] = @workflow_yaml[a] + end + end + end + rescue => e + errors.add :set_name_and_description, "#{e.message}" + end + end +end diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb index ed8f8d89af..7bf75800b4 100644 --- a/services/api/config/routes.rb +++ b/services/api/config/routes.rb @@ -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 index 0000000000..23319b6119 --- /dev/null +++ b/services/api/db/migrate/20160808151559_create_workflows.rb @@ -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 diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql index 4bf4a173bd..1d7a2c6792 100644 --- a/services/api/db/structure.sql +++ b/services/api/db/structure.sql @@ -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 index 0000000000..5cd6c34b58 --- /dev/null +++ b/services/api/test/fixtures/workflows.yml @@ -0,0 +1,16 @@ +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: this of course is a valid yaml + +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 + +workflow_with_no_name_and_desc: + uuid: zzzzz-7fd4e-validnonamedesc + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz diff --git a/services/api/test/unit/workflow_test.rb b/services/api/test/unit/workflow_test.rb new file mode 100644 index 0000000000..7bcf2f6b54 --- /dev/null +++ b/services/api/test/unit/workflow_test.rb @@ -0,0 +1,89 @@ +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 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) + 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) + 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) + 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) + 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) + assert_equal "remains", w.name + assert_equal "remains", w.description + end +end -- 2.30.2