[ARVADOS] updated: c9199d296a74d21239da5f162e7594f6b4246ff1
Git user
git at public.curoverse.com
Thu Jan 5 16:24:17 EST 2017
Summary of changes:
services/api/app/models/container_request.rb | 5 ++++
...170105160301_add_output_name_to_cr_fts_index.rb | 22 +++++++++++++++++
services/api/db/structure.sql | 3 ++-
services/api/test/unit/arvados_model_test.rb | 22 +++++++++++++++++
services/api/test/unit/container_request_test.rb | 28 +++++++++-------------
5 files changed, 62 insertions(+), 18 deletions(-)
create mode 100644 services/api/db/migrate/20170105160301_add_output_name_to_cr_fts_index.rb
via c9199d296a74d21239da5f162e7594f6b4246ff1 (commit)
via 2eafde8ff6f3228fce34a3e25dad6b2d2f171ba9 (commit)
via e0fade6bbda39812854fdfc316e8904886d23fe2 (commit)
via 066f9bcf5a74c255cb64cf89e554cf64ec719f5a (commit)
from 6599088b45103087b4be743fd51a8330e694e57f (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 c9199d296a74d21239da5f162e7594f6b4246ff1
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Thu Jan 5 18:23:03 2017 -0300
10223: Added CR's output_name to FTS index
diff --git a/services/api/db/migrate/20170105160301_add_output_name_to_cr_fts_index.rb b/services/api/db/migrate/20170105160301_add_output_name_to_cr_fts_index.rb
new file mode 100644
index 0000000..9721ead
--- /dev/null
+++ b/services/api/db/migrate/20170105160301_add_output_name_to_cr_fts_index.rb
@@ -0,0 +1,22 @@
+class AddOutputNameToCrFtsIndex < ActiveRecord::Migration
+ def up
+ t = "container_requests"
+ i = "container_requests_full_text_search_idx"
+ t.classify.constantize.reset_column_information
+ ActiveRecord::Base.connection.indexes(t).each do |idx|
+ if idx.name == i
+ remove_index t.to_sym, :name => i
+ break
+ end
+ end
+ # By now, container_request should have the new column "output_name" so full_text_tsvector
+ # would include it on its results
+ execute "CREATE INDEX #{i} ON #{t} USING gin(#{t.classify.constantize.full_text_tsvector});"
+ end
+
+ def down
+ t = "container_requests"
+ i = "container_requests_full_text_search_idx"
+ remove_index t.to_sym, :name => i
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index d4b3713..ea406f2 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -1516,7 +1516,7 @@ CREATE INDEX collections_search_index ON collections USING btree (owner_uuid, mo
-- Name: container_requests_full_text_search_idx; Type: INDEX; Schema: public; Owner: -; Tablespace:
--
-CREATE INDEX container_requests_full_text_search_idx ON container_requests USING gin (to_tsvector('english'::regconfig, (((((((((((((((((((((((((((((((((((((((((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(properties, ''::text)) || ' '::text) || (COALESCE(state, ''::character varying))::text) || ' '::text) || (COALESCE(requesting_container_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(container_uuid, ''::character varying))::text) || ' '::text) || COALESCE(mounts, ''::text)) || ' '::text) || COALESCE(runtime_constraints, ''::text)) || ' '::text) || (COALESCE(container_image, ''::character varying))::text) || ' '::text) || COALESCE(environment, ''::text)) || ' '::text) || (COALESCE(cwd, ''::character varying))::text) || ' '::text) || COALESCE(command, ''::text)) || ' '::text) || (COALESCE(output_path, ''::character varying))::text) || ' '::text) || COALESCE(filters, ''::text)) || ' '::text) || COALESCE(scheduling_parameters, ''::text)) || ' '::text) || (COALESCE(output_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(log_uuid, ''::character varying))::text)));
+CREATE INDEX container_requests_full_text_search_idx ON container_requests USING gin (to_tsvector('english'::regconfig, (((((((((((((((((((((((((((((((((((((((((((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(properties, ''::text)) || ' '::text) || (COALESCE(state, ''::character varying))::text) || ' '::text) || (COALESCE(requesting_container_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(container_uuid, ''::character varying))::text) || ' '::text) || COALESCE(mounts, ''::text)) || ' '::text) || COALESCE(runtime_constraints, ''::text)) || ' '::text) || (COALESCE(container_image, ''::character varying))::text) || ' '::text) || COALESCE(environment, ''::text)) || ' '::text) || (COALESCE(cwd, ''::character varying))::text) || ' '::text) || COALESCE(command, ''::text)) || ' '::text) || (COALESCE(output_path, ''::character varying))::text) || ' '::text) || COALESCE(filters, ''::text)) || ' '::text) || COALESCE(scheduling_parameters, ''::text)) || ' '::text) || (COALESCE(output_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(log_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(output_name, ''::character varying))::text)));
--
@@ -2738,3 +2738,4 @@ INSERT INTO schema_migrations (version) VALUES ('20161223090712');
INSERT INTO schema_migrations (version) VALUES ('20170102153111');
+INSERT INTO schema_migrations (version) VALUES ('20170105160301');
\ No newline at end of file
commit 2eafde8ff6f3228fce34a3e25dad6b2d2f171ba9
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Thu Jan 5 17:59:28 2017 -0300
10223: Added test to check for missing full text search indexes
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 6918aa0..6765814 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -146,6 +146,28 @@ class ArvadosModelTest < ActiveSupport::TestCase
end
end
+ test "full text search index exists on models" do
+ fts_tables = ["collections", "container_requests", "groups", "jobs",
+ "pipeline_instances", "pipeline_templates", "workflows"]
+ fts_tables.each do |table|
+ table_class = table.classify.constantize
+ if table_class.respond_to?('full_text_searchable_columns')
+ fts_index_columns = table_class.full_text_searchable_columns
+ index_columns = nil
+ indexes = ActiveRecord::Base.connection.indexes(table)
+ fts_index_by_columns = indexes.select do |index|
+ if index.columns.first.match(/to_tsvector/)
+ index_columns = index.columns.first.scan(/\((?<columns>[A-Za-z_]+)\,/).flatten!
+ index_columns.sort == fts_index_columns.sort
+ else
+ false
+ end
+ end
+ assert !fts_index_by_columns.empty?, "#{table} has no FTS index with columns #{fts_index_columns}. Instead found FTS index with columns #{index_columns}"
+ end
+ end
+ end
+
test "selectable_attributes includes database attributes" do
assert_includes(Job.selectable_attributes, "success")
end
commit e0fade6bbda39812854fdfc316e8904886d23fe2
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Thu Jan 5 12:54:30 2017 -0300
10223: Added relevant explanation on the rollback command
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 4555820..a264bbf 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -107,6 +107,11 @@ class ContainerRequest < ArvadosModel
'container_request' => uuid,
})
rescue ActiveRecord::RecordNotUnique => rn
+ # In case this is executed as part of a transaction: When a Postgres exception happens,
+ # the following statements on the same transaction become invalid, so a rollback is
+ # needed. One example are Unit Tests, every test is enclosed inside a transaction so
+ # that the database can be reverted before every new test starts.
+ # See: http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Exception+handling+and+rolling+back
ActiveRecord::Base.connection.execute 'ROLLBACK'
raise unless out_type == 'output' and self.output_name
# Postgres specific unique name check. See ApplicationController#create for
commit 066f9bcf5a74c255cb64cf89e554cf64ec719f5a
Author: Lucas Di Pentima <lucas at curoverse.com>
Date: Thu Jan 5 12:43:17 2017 -0300
10223: Test simplification: better name, assertion error description added, and use of preexisting fixture.
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 78dc3e3..da4f0bb 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -512,13 +512,13 @@ class ContainerRequestTest < ActiveSupport::TestCase
assert_not_equal cr2.container_uuid, cr.container_uuid
end
- test "Set up output collection on finalized container request" do
+ test "Output collection name setting using output_name with name collision resolution" do
set_user_from_auth :active
- output_name = 'Test output collection'
+ output_name = collections(:foo_file).name
+
cr = create_minimal_req!(priority: 1,
state: ContainerRequest::Committed,
output_name: output_name)
- cr.reload
act_as_system_user do
c = Container.find_by_uuid(cr.container_uuid)
c.update_attributes!(state: Container::Locked)
@@ -528,23 +528,17 @@ class ContainerRequestTest < ActiveSupport::TestCase
output: '1f4b0bc7583c2a7f9102c395f4ffc5e3+45',
log: 'fa7aeb5140e2848d39b416daeef4ffc5+45')
end
- cr.reload
+ cr.save
+ assert_equal ContainerRequest::Final, cr.state
output_coll = Collection.find_by_uuid(cr.output_uuid)
- assert_not_nil output_coll
- assert_equal output_coll.name, output_name
- # Now ask for a second CR, equal to the first one so it will reuse the container
- cr2 = create_minimal_req!(priority: 1,
- state: ContainerRequest::Uncommitted,
- output_name: output_name)
- cr2.state = ContainerRequest::Committed
- cr2.save
- assert_equal ContainerRequest::Final, cr2.state
- output_coll_2 = Collection.find_by_uuid(cr2.output_uuid)
# Make sure the resulting output collection name include the original name
# plus the date
- assert_not_equal output_coll_2.name, output_coll.name
- assert output_coll_2.name.include? output_name
- assert_match /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z/, output_coll_2.name
+ assert_not_equal output_name, output_coll.name,
+ "It shouldn't exist more than one collection with the same owner and name '${output_name}'"
+ assert output_coll.name.include?(output_name),
+ "New name should include original name"
+ assert_match /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z/, output_coll.name,
+ "New name should include ISO8601 date"
end
test "Finalize committed request when reusing a finished container" do
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list