Merge branch '11908-properties-column-json'

refs #11908

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 0828d016874d3304167c8da8c3f40d8bb964258c..08d7e9345dbb8178d6891821277e6a01958aeacd 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -357,13 +357,14 @@ class ArvadosModel < ActiveRecord::Base
 
   def self.full_text_searchable_columns
     self.columns.select do |col|
-      col.type == :string or col.type == :text
+      [:string, :text, :jsonb].include?(col.type)
     end.map(&:name)
   end
 
   def self.full_text_tsvector
     parts = full_text_searchable_columns.collect do |column|
-      "coalesce(#{column},'')"
+      cast = serialized_attributes[column] ? '::text' : ''
+      "coalesce(#{column}#{cast},'')"
     end
     "to_tsvector('english', #{parts.join(" || ' ' || ")})"
   end

diff --git a/services/api/db/migrate/20161213172944_full_text_search_indexes.rb b/services/api/db/migrate/20161213172944_full_text_search_indexes.rb
index 3f484b4c31db48076b3385bc885d077cdcb84733..0ec52124deca9eba478da8f4799d1f2e62c34e59 100644 (file)
--- a/services/api/db/migrate/20161213172944_full_text_search_indexes.rb
+++ b/services/api/db/migrate/20161213172944_full_text_search_indexes.rb
@@ -15,17 +15,18 @@ class FullTextSearchIndexes < ActiveRecord::Migration
     }
   end
 
+  def replace_index(t)
+    i = fts_indexes[t]
+    t.classify.constantize.reset_column_information
+    execute "DROP INDEX IF EXISTS #{i}"
+    execute "CREATE INDEX #{i} ON #{t} USING gin(#{t.classify.constantize.full_text_tsvector})"
+  end
+
   def up
-    # remove existing fts indexes and create up to date ones with no leading space
-    fts_indexes.each do |t, i|
-      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
-      execute "CREATE INDEX #{i} ON #{t} USING gin(#{t.classify.constantize.full_text_tsvector});"
+    # remove existing fts indexes and create up to date ones with no
+    # leading space
+    fts_indexes.keys.each do |t|
+      replace_index(t)
     end
   end
 

diff --git a/services/api/db/migrate/20170704160233_yaml_to_json.rb b/services/api/db/migrate/20170704160233_yaml_to_json.rb
new file mode 100644 (file)
index 0000000..dfa08db
--- /dev/null
+++ b/services/api/db/migrate/20170704160233_yaml_to_json.rb
@@ -0,0 +1,37 @@
+require 'migrate_yaml_to_json'
+
+class YamlToJson < ActiveRecord::Migration
+  def up
+    [
+      ['collections', 'properties'],
+      ['containers', 'environment'],
+      ['containers', 'mounts'],
+      ['containers', 'runtime_constraints'],
+      ['containers', 'command'],
+      ['containers', 'scheduling_parameters'],
+      ['container_requests', 'properties'],
+      ['container_requests', 'environment'],
+      ['container_requests', 'mounts'],
+      ['container_requests', 'runtime_constraints'],
+      ['container_requests', 'command'],
+      ['container_requests', 'scheduling_parameters'],
+      ['humans', 'properties'],
+      ['job_tasks', 'parameters'],
+      ['links', 'properties'],
+      ['nodes', 'info'],
+      ['nodes', 'properties'],
+      ['pipeline_instances', 'components'],
+      ['pipeline_instances', 'properties'],
+      ['pipeline_instances', 'components_summary'],
+      ['pipeline_templates', 'components'],
+      ['specimens', 'properties'],
+      ['traits', 'properties'],
+      ['users', 'prefs'],
+    ].each do |table, column|
+      MigrateYAMLToJSON.migrate(table, column)
+    end
+  end
+
+  def down
+  end
+end

diff --git a/services/api/db/migrate/20170706141334_json_collection_properties.rb b/services/api/db/migrate/20170706141334_json_collection_properties.rb
new file mode 100644 (file)
index 0000000..003e5fb
--- /dev/null
+++ b/services/api/db/migrate/20170706141334_json_collection_properties.rb
@@ -0,0 +1,16 @@
+require './db/migrate/20161213172944_full_text_search_indexes'
+
+class JsonCollectionProperties < ActiveRecord::Migration
+  def up
+    # Drop the FT index before changing column type to avoid
+    # "PG::DatatypeMismatch: ERROR: COALESCE types jsonb and text
+    # cannot be matched".
+    ActiveRecord::Base.connection.execute 'DROP INDEX IF EXISTS collections_full_text_search_idx'
+    ActiveRecord::Base.connection.execute 'ALTER TABLE collections ALTER COLUMN properties TYPE jsonb USING properties::jsonb'
+    FullTextSearchIndexes.new.replace_index('collections')
+  end
+
+  def down
+    ActiveRecord::Base.connection.execute 'ALTER TABLE collections ALTER COLUMN properties TYPE text'
+  end
+end

diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index e062d43e4b30518d5f00d9846394f481a9306bab..d2da430102282f2ab28aa0d7d4848c228450f972 100644 (file)
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -166,7 +166,7 @@ CREATE TABLE collections (
     manifest_text text,
     name character varying(255),
     description character varying(524288),
-    properties text,
+    properties jsonb,
     delete_at timestamp without time zone,
     file_names character varying(8192),
     trash_at timestamp without time zone,
@@ -1607,7 +1607,7 @@ CREATE INDEX authorized_keys_search_index ON authorized_keys USING btree (uuid,
 -- Name: collections_full_text_search_idx; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE INDEX collections_full_text_search_idx ON collections USING gin (to_tsvector('english'::regconfig, (((((((((((((((((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(portable_data_hash, ''::character varying))::text) || ' '::text) || (COALESCE(uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || (COALESCE(description, ''::character varying))::text) || ' '::text) || COALESCE(properties, ''::text)) || ' '::text) || (COALESCE(file_names, ''::character varying))::text)));
+CREATE INDEX collections_full_text_search_idx ON collections USING gin (to_tsvector('english'::regconfig, (((((((((((((((((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(portable_data_hash, ''::character varying))::text) || ' '::text) || (COALESCE(uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || (COALESCE(description, ''::character varying))::text) || ' '::text) || COALESCE((properties)::text, ''::text)) || ' '::text) || (COALESCE(file_names, ''::character varying))::text)));
 
 
 --
@@ -3026,9 +3026,12 @@ INSERT INTO schema_migrations (version) VALUES ('20170419175801');
 
 INSERT INTO schema_migrations (version) VALUES ('20170628185847');
 
+INSERT INTO schema_migrations (version) VALUES ('20170704160233');
+
+INSERT INTO schema_migrations (version) VALUES ('20170706141334');
+
 INSERT INTO schema_migrations (version) VALUES ('20170824202826');
 
 INSERT INTO schema_migrations (version) VALUES ('20170906224040');
 
 INSERT INTO schema_migrations (version) VALUES ('20171027183824');
-

diff --git a/services/api/lib/serializers.rb b/services/api/lib/serializers.rb
index ec6fe199729cdd663eda72497290a5e269b120cf..37734e0bb41dce88500b143fab0a71102b1b8b33 100644 (file)
--- a/services/api/lib/serializers.rb
+++ b/services/api/lib/serializers.rb
@@ -32,7 +32,10 @@ class Serializer
   end
 
   def self.load(s)
-    if s.nil?
+    if s.is_a?(object_class)
+      # Rails already deserialized for us
+      s
+    elsif s.nil?
       object_class.new()
     elsif s[0] == first_json_char
       SafeJSON.load(s)

diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index c6b76c4c7374664f7a43eea57fc92cefc41f8a7b..923083832c658627f02b6001b8f71ccfd47f6a59 100644 (file)
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -258,4 +258,24 @@ class ArvadosModelTest < ActiveSupport::TestCase
 
     assert_equal true, (updated_at_2 > updated_at_1), "Expected updated time 2 to be newer than 1"
   end
+
+  test 'jsonb column' do
+    set_user_from_auth :active
+
+    c = Collection.create!(properties: {})
+    assert_equal({}, c.properties)
+
+    c.update_attributes(properties: {'foo' => 'foo'})
+    c.reload
+    assert_equal({'foo' => 'foo'}, c.properties)
+
+    c.update_attributes(properties: nil)
+    c.reload
+    assert_equal({}, c.properties)
+
+    c.update_attributes(properties: {foo: 'bar'})
+    assert_equal({'foo' => 'bar'}, c.properties)
+    c.reload
+    assert_equal({'foo' => 'bar'}, c.properties)
+  end
 end