From 667a7121e08d4fffc24cafdc3ed474374782b959 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Sat, 9 Dec 2017 13:59:23 -0500 Subject: [PATCH] 4019: Initial support for queries on jsonb subproperties. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- services/api/app/models/arvados_model.rb | 2 +- services/api/db/structure.sql | 1 + services/api/lib/record_filters.rb | 176 +++++++++++------- services/api/test/fixtures/collections.yml | 56 ++++++ .../functional/arvados/v1/filters_test.rb | 120 ++++++++++++ 5 files changed, 291 insertions(+), 64 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 08d7e9345d..72db306f3a 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -132,7 +132,7 @@ class ArvadosModel < ActiveRecord::Base textonly_operator = !operator.match(/[<=>]/) self.columns.select do |col| case col.type - when :string, :text + when :string, :text, :jsonb true when :datetime, :integer, :boolean !textonly_operator diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql index 60fd88a98b..8f405c00c2 100644 --- a/services/api/db/structure.sql +++ b/services/api/db/structure.sql @@ -3039,3 +3039,4 @@ INSERT INTO schema_migrations (version) VALUES ('20170906224040'); INSERT INTO schema_migrations (version) VALUES ('20171027183824'); INSERT INTO schema_migrations (version) VALUES ('20171208203841'); + diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb index eb8d09b74c..ce9fe6b13a 100644 --- a/services/api/lib/record_filters.rb +++ b/services/api/lib/record_filters.rb @@ -9,6 +9,9 @@ # model_class # Operates on: # @objects + +require 'safe_json' + module RecordFilters # Input: @@ -58,80 +61,127 @@ module RecordFilters param_out << operand.split.join(' & ') end attrs.each do |attr| - if !model_class.searchable_columns(operator).index attr.to_s - raise ArgumentError.new("Invalid attribute '#{attr}' in filter") + subproperty = attr.split(".", 2) + + if !model_class.searchable_columns(operator).index subproperty[0] + raise ArgumentError.new("Invalid attribute '#{subproperty[0]}' in filter") end - case operator.downcase - when '=', '<', '<=', '>', '>=', '!=', 'like', 'ilike' - attr_type = model_class.attribute_column(attr).type - operator = '<>' if operator == '!=' - if operand.is_a? String - if attr_type == :boolean - if not ['=', '<>'].include?(operator) - raise ArgumentError.new("Invalid operator '#{operator}' for " \ - "boolean attribute '#{attr}'") - end - case operand.downcase - when '1', 't', 'true', 'y', 'yes' - operand = true - when '0', 'f', 'false', 'n', 'no' - operand = false - else - raise ArgumentError("Invalid operand '#{operand}' for " \ - "boolean attribute '#{attr}'") + + if subproperty.length == 2 + # jsonb search + case operator.downcase + when '=', '!=' + not_in = if operator.downcase == "!=" then "NOT " else "" end + cond_out << "#{not_in}(#{ar_table_name}.#{subproperty[0]} @> ?::jsonb)" + param_out << SafeJSON.dump({subproperty[1] => operand}) + when 'in' + if operand.is_a? Array + operand.each do |opr| + cond_out << "#{ar_table_name}.#{subproperty[0]} @> ?::jsonb" + param_out << SafeJSON.dump({subproperty[1] => opr}) end - end - if operator == '<>' - # explicitly allow NULL - cond_out << "#{ar_table_name}.#{attr} #{operator} ? OR #{ar_table_name}.#{attr} IS NULL" else - cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ + "for '#{operator}' operator in filters") end - if (# any operator that operates on value rather than - # representation: - operator.match(/[<=>]/) and (attr_type == :datetime)) - operand = Time.parse operand - end - param_out << operand - elsif operand.nil? and operator == '=' - cond_out << "#{ar_table_name}.#{attr} is null" - elsif operand.nil? and operator == '<>' - cond_out << "#{ar_table_name}.#{attr} is not null" - elsif (attr_type == :boolean) and ['=', '<>'].include?(operator) and - [true, false].include?(operand) - cond_out << "#{ar_table_name}.#{attr} #{operator} ?" - param_out << operand - else - raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ - "for '#{operator}' operator in filters") - end - when 'in', 'not in' - if operand.is_a? Array - cond_out << "#{ar_table_name}.#{attr} #{operator} (?)" + # when '<', '<=', '>', '>=' + # cond_out << "#{ar_table_name}.#{subproperty[0]}->? #{operator} ?::jsonb" + # param_out << subproperty[1] + # param_out << SafeJSON.dump(operand) + when 'like', 'ilike' + cond_out << "#{ar_table_name}.#{subproperty[0]}->>? #{operator} ?" param_out << operand - if operator == 'not in' and not operand.include?(nil) - # explicitly allow NULL - cond_out[-1] = "(#{cond_out[-1]} OR #{ar_table_name}.#{attr} IS NULL)" + when 'not in' + if operand.is_a? Array + cond_out << "#{ar_table_name}.#{subproperty[0]}->>? NOT IN (?)" + param_out << subproperty[1] + param_out << operand + else + raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ + "for '#{operator}' operator in filters") end + when '?' + if operand + raise ArgumentError.new("Invalid operand for subproperty existence filter, should be empty or null") + end + cond_out << "jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)" + param_out << subproperty[1] else - raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ - "for '#{operator}' operator in filters") + raise ArgumentError.new("Invalid operator for subproperty search '#{operator}'") end - when 'is_a' - operand = [operand] unless operand.is_a? Array - cond = [] - operand.each do |op| - cl = ArvadosModel::kind_class op - if cl - cond << "#{ar_table_name}.#{attr} like ?" - param_out << cl.uuid_like_pattern + else + case operator.downcase + when '=', '<', '<=', '>', '>=', '!=', 'like', 'ilike' + attr_type = model_class.attribute_column(attr).type + operator = '<>' if operator == '!=' + if operand.is_a? String + if attr_type == :boolean + if not ['=', '<>'].include?(operator) + raise ArgumentError.new("Invalid operator '#{operator}' for " \ + "boolean attribute '#{attr}'") + end + case operand.downcase + when '1', 't', 'true', 'y', 'yes' + operand = true + when '0', 'f', 'false', 'n', 'no' + operand = false + else + raise ArgumentError("Invalid operand '#{operand}' for " \ + "boolean attribute '#{attr}'") + end + end + if operator == '<>' + # explicitly allow NULL + cond_out << "#{ar_table_name}.#{attr} #{operator} ? OR #{ar_table_name}.#{attr} IS NULL" + else + cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + end + if (# any operator that operates on value rather than + # representation: + operator.match(/[<=>]/) and (attr_type == :datetime)) + operand = Time.parse operand + end + param_out << operand + elsif operand.nil? and operator == '=' + cond_out << "#{ar_table_name}.#{attr} is null" + elsif operand.nil? and operator == '<>' + cond_out << "#{ar_table_name}.#{attr} is not null" + elsif (attr_type == :boolean) and ['=', '<>'].include?(operator) and + [true, false].include?(operand) + cond_out << "#{ar_table_name}.#{attr} #{operator} ?" + param_out << operand else - cond << "1=0" + raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ + "for '#{operator}' operator in filters") end + when 'in', 'not in' + if operand.is_a? Array + cond_out << "#{ar_table_name}.#{attr} #{operator} (?)" + param_out << operand + if operator == 'not in' and not operand.include?(nil) + # explicitly allow NULL + cond_out[-1] = "(#{cond_out[-1]} OR #{ar_table_name}.#{attr} IS NULL)" + end + else + raise ArgumentError.new("Invalid operand type '#{operand.class}' "\ + "for '#{operator}' operator in filters") + end + when 'is_a' + operand = [operand] unless operand.is_a? Array + cond = [] + operand.each do |op| + cl = ArvadosModel::kind_class op + if cl + cond << "#{ar_table_name}.#{attr} like ?" + param_out << cl.uuid_like_pattern + else + cond << "1=0" + end + end + cond_out << cond.join(' OR ') + else + raise ArgumentError.new("Invalid operator '#{operator}'") end - cond_out << cond.join(' OR ') - else - raise ArgumentError.new("Invalid operator '#{operator}'") end end conds_out << cond_out.join(' OR ') if cond_out.any? diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml index 8023503201..b265f242a0 100644 --- a/services/api/test/fixtures/collections.yml +++ b/services/api/test/fixtures/collections.yml @@ -715,6 +715,62 @@ collection_in_trashed_subproject: manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:file1 0:0:file2\n" name: collection in trashed subproject +collection_with_prop1_value1: + uuid: zzzzz-4zz18-withprop1value1 + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-13T17:22:54Z + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f + modified_at: 2015-02-13T17:22:54Z + updated_at: 2015-02-13T17:22:54Z + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: collection with prop1 value1 + properties: + prop1: value1 + +collection_with_prop1_value2: + uuid: zzzzz-4zz18-withprop1value2 + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-13T17:22:54Z + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f + modified_at: 2015-02-13T17:22:54Z + updated_at: 2015-02-13T17:22:54Z + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: collection with prop1 value2 + properties: + prop1: value2 + +collection_with_prop2_1: + uuid: zzzzz-4zz18-withprop2value1 + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-13T17:22:54Z + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f + modified_at: 2015-02-13T17:22:54Z + updated_at: 2015-02-13T17:22:54Z + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: collection with prop1 1 + properties: + prop2: 1 + +collection_with_prop2_5: + uuid: zzzzz-4zz18-withprop2value5 + portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45 + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2015-02-13T17:22:54Z + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f + modified_at: 2015-02-13T17:22:54Z + updated_at: 2015-02-13T17:22:54Z + manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n" + name: collection with prop1 5 + properties: + prop2: 5 + # Test Helper trims the rest of the file # Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper diff --git a/services/api/test/functional/arvados/v1/filters_test.rb b/services/api/test/functional/arvados/v1/filters_test.rb index 2c7427cba8..61b9e786b7 100644 --- a/services/api/test/functional/arvados/v1/filters_test.rb +++ b/services/api/test/functional/arvados/v1/filters_test.rb @@ -151,4 +151,124 @@ class Arvados::V1::FiltersTest < ActionController::TestCase assert_equal all_objects['arvados#pipelineInstance'], second_page['arvados#pipelineInstance']+5 assert_equal true, second_page['arvados#pipelineTemplate']>0 end + + test "jsonb '=' filter" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', '=', 'value1'] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_equal(found, [collections(:collection_with_prop1_value1).uuid]) + end + + test "jsonb '!=' filter" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', '!=', 'value1'] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_operator found.length, :>, 1 + assert_includes(found, collections(:collection_with_prop1_value2).uuid) + end + + test "jsonb '?'" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', '?', nil] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_equal found.length, 2 + assert_includes(found, collections(:collection_with_prop1_value1).uuid) + assert_includes(found, collections(:collection_with_prop1_value2).uuid) + end + + test "jsonb '?' and '!=' filter" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', '?', nil], ['properties.prop1', '!=', 'value1'] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_equal(found, [collections(:collection_with_prop1_value2).uuid]) + end + + test "jsonb 'in' filter (match all)" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', 'in', ['value1', 'value2']] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_equal found.length, 2 + assert_includes(found, collections(:collection_with_prop1_value1).uuid) + assert_includes(found, collections(:collection_with_prop1_value2).uuid) + end + + test "jsonb 'in' filter (match some)" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', 'in', ['value1', 'value3']] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_equal(found, [collections(:collection_with_prop1_value1).uuid]) + end + + test "jsonb 'not in' filter (match all)" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', 'not in', ['value1', 'value2']] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_not_includes(found, collections(:collection_with_prop1_value1).uuid) + assert_not_includes(found, collections(:collection_with_prop1_value2).uuid) + end + + test "jsonb 'not in' filter (match some)" do + @controller = Arvados::V1::CollectionsController.new + authorize_with :admin + get :index, { + filters: [ ['properties.prop1', 'not in', ['value1', 'value3']] ] + } + assert_response :success + found = assigns(:objects).collect(&:uuid) + assert_not_includes(found, collections(:collection_with_prop1_value1).uuid) + assert_includes(found, collections(:collection_with_prop1_value2).uuid) + end + + # test "jsonb '>' filter (>3)" do + # @controller = Arvados::V1::CollectionsController.new + # authorize_with :admin + # get :index, { + # filters: [ ['properties.prop2', '>', 3] ] + # } + # assert_response :success + # found = assigns(:objects).collect(&:uuid) + # assert_not_includes(found, collections(:collection_with_prop2_1).uuid) + # assert_includes(found, collections(:collection_with_prop2_5).uuid) + # end + + # test "jsonb '>' filter (>'value1')" do + # @controller = Arvados::V1::CollectionsController.new + # authorize_with :admin + # get :index, { + # filters: [ ['properties.prop1', '>', "value1"] ] + # } + # assert_response :success + # found = assigns(:objects).collect(&:uuid) + # assert_not_includes(found, collections(:collection_with_prop1_value1).uuid) + # assert_includes(found, collections(:collection_with_prop1_value2).uuid) + # end + end -- 2.39.5