Merge branch '13006-api-is_a-filter' into 13006-sync-groups-is_a-filter

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/services/api/app/controllers/arvados/v1/links_controller.rb b/services/api/app/controllers/arvados/v1/links_controller.rb
index 862582aa9ce65fcbcf4a73d8b309b19cf3749556..f54c4a9a519c563ca8fc08e9bb480b254e616608 100644 (file)
--- a/services/api/app/controllers/arvados/v1/links_controller.rb
+++ b/services/api/app/controllers/arvados/v1/links_controller.rb
@@ -66,7 +66,7 @@ class Arvados::V1::LinksController < ApplicationController
     super
 
     # head_kind and tail_kind columns are now virtual,
-    # equivilent functionality is now provided by
+    # equivalent functionality is now provided by
     # 'is_a', so fix up any old-style 'where' clauses.
     if @where
       @filters ||= []
@@ -86,7 +86,7 @@ class Arvados::V1::LinksController < ApplicationController
     super
 
     # head_kind and tail_kind columns are now virtual,
-    # equivilent functionality is now provided by
+    # equivalent functionality is now provided by
     # 'is_a', so fix up any old-style 'filter' clauses.
     @filters = @filters.map do |k|
       if k[0] == 'head_kind' and k[1] == '='

diff --git a/services/api/db/migrate/20181213183234_add_expression_index_to_links.rb b/services/api/db/migrate/20181213183234_add_expression_index_to_links.rb
new file mode 100644 (file)
index 0000000..2fdf830
--- /dev/null
+++ b/services/api/db/migrate/20181213183234_add_expression_index_to_links.rb
@@ -0,0 +1,11 @@
+class AddExpressionIndexToLinks < ActiveRecord::Migration
+  def up
+    ActiveRecord::Base.connection.execute 'CREATE INDEX index_links_on_substring_head_uuid on links (substring(head_uuid, 7, 5))'
+    ActiveRecord::Base.connection.execute 'CREATE INDEX index_links_on_substring_tail_uuid on links (substring(tail_uuid, 7, 5))'
+  end
+
+  def down
+    ActiveRecord::Base.connection.execute 'DROP INDEX index_links_on_substring_head_uuid'
+    ActiveRecord::Base.connection.execute 'DROP INDEX index_links_on_substring_tail_uuid'
+  end
+end

diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index aa29a1cbb409d59542d0d037cbdf703f9c407ea5..211fa5043fda2aedc33646f8c98dff863bec8d7a 100644 (file)
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -2278,6 +2278,20 @@ CREATE INDEX index_links_on_modified_at_uuid ON public.links USING btree (modifi
 CREATE INDEX index_links_on_owner_uuid ON public.links USING btree (owner_uuid);
 
 
+--
+-- Name: index_links_on_substring_head_uuid; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_links_on_substring_head_uuid ON public.links USING btree ("substring"((head_uuid)::text, 7, 5));
+
+
+--
+-- Name: index_links_on_substring_tail_uuid; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_links_on_substring_tail_uuid ON public.links USING btree ("substring"((tail_uuid)::text, 7, 5));
+
+
 --
 -- Name: index_links_on_tail_uuid; Type: INDEX; Schema: public; Owner: -
 --
@@ -3201,3 +3215,5 @@ INSERT INTO schema_migrations (version) VALUES ('20181005192222');
 
 INSERT INTO schema_migrations (version) VALUES ('20181011184200');
 
+INSERT INTO schema_migrations (version) VALUES ('20181213183234');
+

diff --git a/services/api/lib/record_filters.rb b/services/api/lib/record_filters.rb
index dc427c12c1f82cfc76d8b53a13ad1d7b8a88c032..831e357b4235b6b11217de47ee2c5960a4ef3563 100644 (file)
--- a/services/api/lib/record_filters.rb
+++ b/services/api/lib/record_filters.rb
@@ -74,7 +74,7 @@ module RecordFilters
             subproperty[1] = subproperty[1][1..-2]
           end
 
-        # jsonb search
+          # jsonb search
           case operator.downcase
           when '=', '!='
             not_in = if operator.downcase == "!=" then "NOT " else "" end
@@ -109,14 +109,14 @@ module RecordFilters
                                       "for '#{operator}' operator in filters")
             end
           when 'exists'
-          if operand == true
-            cond_out << "jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)"
-          elsif operand == false
-            cond_out << "(NOT jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)) OR #{ar_table_name}.#{subproperty[0]} is NULL"
-          else
-            raise ArgumentError.new("Invalid operand '#{operand}' for '#{operator}' must be true or false")
-          end
-          param_out << subproperty[1]
+            if operand == true
+              cond_out << "jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)"
+            elsif operand == false
+              cond_out << "(NOT jsonb_exists(#{ar_table_name}.#{subproperty[0]}, ?)) OR #{ar_table_name}.#{subproperty[0]} is NULL"
+            else
+              raise ArgumentError.new("Invalid operand '#{operand}' for '#{operator}' must be true or false")
+            end
+            param_out << subproperty[1]
           else
             raise ArgumentError.new("Invalid operator for subproperty search '#{operator}'")
           end
@@ -197,8 +197,17 @@ module RecordFilters
             operand.each do |op|
               cl = ArvadosModel::kind_class op
               if cl
-                cond << "#{ar_table_name}.#{attr} like ?"
-                param_out << cl.uuid_like_pattern
+                if attr == 'uuid'
+                  if model_class.uuid_prefix == cl.uuid_prefix
+                    cond << "1=1"
+                  else
+                    cond << "1=0"
+                  end
+                else
+                  # Use a substring query to support remote uuids
+                  cond << "substring(#{ar_table_name}.#{attr}, 7, 5) = ?"
+                  param_out << cl.uuid_prefix
+                end
               else
                 cond << "1=0"
               end

diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 2b247a960d989e962b373b726878828bc008d105..e66baceb28d0a28b3efb5361ca2a3a06b9401d75 100644 (file)
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -156,6 +156,20 @@ foo_file_readable_by_active:
   head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
   properties: {}
 
+foo_file_readable_by_federated_active:
+  uuid: zzzzz-o0j2j-dp1d8395ldqw23r
+  owner_uuid: zzzzz-tpzed-000000000000000
+  created_at: 2014-01-24 20:42:26 -0800
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-01-24 20:42:26 -0800
+  updated_at: 2014-01-24 20:42:26 -0800
+  tail_uuid: zbbbb-tpzed-xurymjxw79nv3jz
+  link_class: permission
+  name: can_read
+  head_uuid: zzzzz-4zz18-znfnqtbbv4spc3w
+  properties: {}
+
 foo_file_readable_by_active_duplicate_permission:
   uuid: zzzzz-o0j2j-2qlmhgothiur55r
   owner_uuid: zzzzz-tpzed-000000000000000

diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb
index 4ae37455503ca79326b3c92986e7ed36be9ab1b2..47e46fe8378410f0804b21b9ca0d079788928e6c 100644 (file)
--- a/services/api/test/functional/arvados/v1/links_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/links_controller_test.rb
@@ -144,6 +144,23 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
     assert_equal found.count, (found.select { |f| f.tail_uuid.match User.uuid_regex }).count
   end
 
+  test "filter links with 'is_a' operator includes remote objects" do
+    authorize_with :admin
+    get :index, {
+      filters: [
+        ['tail_uuid', 'is_a', 'arvados#user'],
+        ['link_class', '=', 'permission'],
+        ['name', '=', 'can_read'],
+        ['head_uuid', '=', collections(:foo_file).uuid],
+      ]
+    }
+    assert_response :success
+    found = assigns(:objects)
+    assert_not_equal 0, found.count
+    assert_includes(found.map(&:tail_uuid),
+                    users(:federated_active).uuid)
+  end
+
   test "filter links with 'is_a' operator with more than one" do
     authorize_with :admin
     get :index, {