11652: Merge branch 'master' into 11652-recursive-contents
authorTom Clegg <tom@curoverse.com>
Tue, 6 Jun 2017 17:53:36 +0000 (13:53 -0400)
committerTom Clegg <tom@curoverse.com>
Tue, 6 Jun 2017 17:53:36 +0000 (13:53 -0400)
doc/api/methods/groups.html.textile.liquid
services/api/app/controllers/arvados/v1/groups_controller.rb
services/api/app/models/user.rb
services/api/lib/can_be_an_owner.rb
services/api/lib/create_ancestor_view.sql [new file with mode: 0644]
services/api/test/functional/arvados/v1/groups_controller_test.rb

index 7a15d20d5aaf3d5b1ff63aedf9f958d6e91efc3e..a57a089eec89de73b8a888a8e489fd909327555c 100644 (file)
@@ -43,6 +43,7 @@ table(table table-bordered table-condensed).
 |limit|integer (default 100)|Maximum number of items to return.|query||
 |order|string|Order in which to return matching items.  Sort within a resource type by prefixing the attribute with the resource name and a dot.|query|@"collections.modified_at desc"@|
 |filters|array|Conditions for filtering items.|query|@[["uuid", "is_a", "arvados#job"]]@|
+|recursive|boolean (default false)|Include items owned by subprojects.|query|@true@|
 
 Note: Because adding access tokens to manifests can be computationally expensive, the @manifest_text@ field is not included in listed collections.  If you need it, request a "list of collections":{{site.baseurl}}/api/methods/collections.html with the filter @["owner_uuid", "=", GROUP_UUID]@, and @"manifest_text"@ listed in the select parameter.
 
index fc6489901a967a2dc667fc106e8f2178110420a8..4c33be666d5006949e229f11bbae2dea699be16b 100644 (file)
@@ -91,6 +91,15 @@ class Arvados::V1::GroupsController < ApplicationController
       end
     end
 
+    filter_by_owner = {}
+    if @object
+      if params['recursive']
+        filter_by_owner[:owner_uuid] = [@object.uuid] + @object.descendant_project_uuids
+      else
+        filter_by_owner[:owner_uuid] = @object.uuid
+      end
+    end
+
     seen_last_class = false
     klasses.each do |klass|
       @offset = 0 if seen_last_class  # reset offset for the new next type being processed
@@ -118,12 +127,11 @@ class Arvados::V1::GroupsController < ApplicationController
         klass.default_orders.join(", ")
 
       @select = nil
-      where_conds = {}
-      where_conds[:owner_uuid] = @object.uuid if @object
+      where_conds = filter_by_owner
       if klass == Collection
         @select = klass.selectable_attributes - ["manifest_text"]
       elsif klass == Group
-        where_conds[:group_class] = "project"
+        where_conds = where_conds.merge(group_class: "project")
       end
 
       @filters = request_filters.map do |col, op, val|
index 742db4c9b02282aecbf824bf087c7f2eb2c76dc8..d944474712708b3207f28807e10349b8f34007b0 100644 (file)
@@ -144,33 +144,20 @@ class User < ArvadosModel
   # and perm_hash[:write] are true if this user can read and write
   # objects owned by group_uuid.
   def calculate_group_permissions
-    conn = ActiveRecord::Base.connection
-    self.class.transaction do
-      # Check whether the temporary view has already been created
-      # during this connection. If not, create it.
-      conn.exec_query 'SAVEPOINT check_permission_view'
-      begin
-        conn.exec_query('SELECT 1 FROM permission_view LIMIT 0')
-      rescue
-        conn.exec_query 'ROLLBACK TO SAVEPOINT check_permission_view'
-        sql = File.read(Rails.root.join('lib', 'create_permission_view.sql'))
-        conn.exec_query(sql)
-      ensure
-        conn.exec_query 'RELEASE SAVEPOINT check_permission_view'
-      end
-    end
+    install_view('permission')
 
     group_perms = {}
-    conn.exec_query('SELECT target_owner_uuid, max(perm_level)
-                    FROM permission_view
-                    WHERE user_uuid = $1
-                    AND target_owner_uuid IS NOT NULL
-                    GROUP BY target_owner_uuid',
-                    # "name" arg is a query label that appears in logs:
-                    "group_permissions for #{uuid}",
-                    # "binds" arg is an array of [col_id, value] for '$1' vars:
-                    [[nil, uuid]],
-                    ).rows.each do |group_uuid, max_p_val|
+    ActiveRecord::Base.connection.
+      exec_query('SELECT target_owner_uuid, max(perm_level)
+                  FROM permission_view
+                  WHERE user_uuid = $1
+                  AND target_owner_uuid IS NOT NULL
+                  GROUP BY target_owner_uuid',
+                  # "name" arg is a query label that appears in logs:
+                  "group_permissions for #{uuid}",
+                  # "binds" arg is an array of [col_id, value] for '$1' vars:
+                  [[nil, uuid]],
+                  ).rows.each do |group_uuid, max_p_val|
       group_perms[group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
     end
     Rails.cache.write "groups_for_user_#{self.uuid}", group_perms
index 75a63509c206ac86fa64e39afa990073350a87cb..e9f016dc051a06ec09f9aa071ab98fc0a15aa236 100644 (file)
@@ -22,6 +22,22 @@ module CanBeAnOwner
     base.validate :restrict_uuid_change_breaking_associations
   end
 
+  def descendant_project_uuids
+    install_view('ancestor')
+    ActiveRecord::Base.connection.
+      exec_query('SELECT ancestor_view.uuid
+                  FROM ancestor_view
+                  LEFT JOIN groups ON groups.uuid=ancestor_view.uuid
+                  WHERE ancestor_uuid = $1 AND groups.group_class = $2',
+                  # "name" arg is a query label that appears in logs:
+                  "descendant_project_uuids for #{self.uuid}",
+                  # "binds" arg is an array of [col_id, value] for '$1' vars:
+                  [[nil, self.uuid], [nil, 'project']],
+                  ).rows.map do |project_uuid,|
+      project_uuid
+    end
+  end
+
   protected
 
   def restrict_uuid_change_breaking_associations
@@ -44,4 +60,21 @@ module CanBeAnOwner
     end
   end
 
+  def install_view(type)
+    conn = ActiveRecord::Base.connection
+    self.class.transaction do
+      # Check whether the temporary view has already been created
+      # during this connection. If not, create it.
+      conn.exec_query "SAVEPOINT check_#{type}_view"
+      begin
+        conn.exec_query("SELECT 1 FROM #{type}_view LIMIT 0")
+      rescue
+        conn.exec_query "ROLLBACK TO SAVEPOINT check_#{type}_view"
+        sql = File.read(Rails.root.join("lib", "create_#{type}_view.sql"))
+        conn.exec_query(sql)
+      ensure
+        conn.exec_query "RELEASE SAVEPOINT check_#{type}_view"
+      end
+    end
+  end
 end
diff --git a/services/api/lib/create_ancestor_view.sql b/services/api/lib/create_ancestor_view.sql
new file mode 100644 (file)
index 0000000..105fd04
--- /dev/null
@@ -0,0 +1,14 @@
+CREATE TEMPORARY VIEW ancestor_view AS
+WITH RECURSIVE
+ancestor (uuid, ancestor_uuid) AS (
+     SELECT groups.uuid::varchar(32)       AS uuid,
+            groups.owner_uuid::varchar(32) AS ancestor_uuid
+            FROM groups
+     UNION
+     SELECT ancestor.uuid::varchar(32)     AS uuid,
+            groups.owner_uuid::varchar(32) AS ancestor_uuid
+            FROM ancestor
+            INNER JOIN groups
+            ON groups.uuid = ancestor.ancestor_uuid
+)
+SELECT * FROM ancestor;
index 02d8c153a8abf6157d099415a6f6553ac1816b71..3beec35958b45dc63611e9fcf393e98dff2575ef 100644 (file)
@@ -477,4 +477,51 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
     assert_operator(json_response['items'].count,
                     :<, json_response['items_available'])
   end
+
+  test 'get contents, recursive=true' do
+    authorize_with :active
+    params = {
+      id: groups(:aproject).uuid,
+      recursive: true,
+      format: :json,
+    }
+    get :contents, params
+    owners = json_response['items'].map do |item|
+      item['owner_uuid']
+    end
+    assert_includes(owners, groups(:aproject).uuid)
+    assert_includes(owners, groups(:asubproject).uuid)
+  end
+
+  [false, nil].each do |recursive|
+    test "get contents, recursive=#{recursive.inspect}" do
+      authorize_with :active
+      params = {
+        id: groups(:aproject).uuid,
+        format: :json,
+      }
+      params[:recursive] = false if recursive == false
+      get :contents, params
+      owners = json_response['items'].map do |item|
+        item['owner_uuid']
+      end
+      assert_includes(owners, groups(:aproject).uuid)
+      refute_includes(owners, groups(:asubproject).uuid)
+    end
+  end
+
+  test 'get home project contents, recursive=true' do
+    authorize_with :active
+    get :contents, {
+          id: users(:active).uuid,
+          recursive: true,
+          format: :json,
+        }
+    owners = json_response['items'].map do |item|
+      item['owner_uuid']
+    end
+    assert_includes(owners, users(:active).uuid)
+    assert_includes(owners, groups(:aproject).uuid)
+    assert_includes(owners, groups(:asubproject).uuid)
+  end
 end