[ARVADOS] updated: 1.3.0-2547-g6788fa439

[Git user]

Summary of changes:
 services/api/app/models/link.rb             |  6 ++--
 services/api/app/models/user.rb             | 14 +++++---
 services/api/lib/refresh_permission_view.rb | 53 ++++++++++++++++-------------
 3 files changed, 42 insertions(+), 31 deletions(-)

       via  6788fa439143ef41e32f7dcc1373d1ade3d17e01 (commit)
      from  f559492b6bc79ded8e1ffcfd384cd23709f2b0e3 (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 6788fa439143ef41e32f7dcc1373d1ade3d17e01
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu May 14 12:20:42 2020 -0400

    16007: Re-enable permission update checking for testing
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index f6210fbb5..33e18b296 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -17,7 +17,7 @@ class Link < ArvadosModel
   after_update :call_update_permissions
   after_create :call_update_permissions
   before_destroy :clear_permissions
-  after_destroy :clear_permissions
+  after_destroy :check_permissions
 
   api_accessible :user, extend: :common do |t|
     t.add :tail_uuid
@@ -80,13 +80,13 @@ class Link < ArvadosModel
 
   def clear_permissions
     if self.link_class == 'permission'
-      update_permissions tail_uuid, head_uuid, 0, false
+      update_permissions tail_uuid, head_uuid, 0
     end
   end
 
   def check_permissions
     if self.link_class == 'permission'
-      update_permissions tail_uuid, head_uuid, 0, true
+      #check_permissions_against_full_refresh
     end
   end
 
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index c27b633ac..d8b18054b 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -50,7 +50,8 @@ class User < ArvadosModel
     user.username_changed? and
     (not user.username_was.nil?)
   }
-  after_destroy :clear_permissions
+  before_destroy :clear_permissions
+  after_destroy :check_permissions
 
   has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
   has_many :repositories, foreign_key: :owner_uuid, primary_key: :uuid
@@ -143,7 +144,7 @@ SELECT 1 FROM #{PERMISSION_VIEW}
   def before_ownership_change
     if owner_uuid_changed? and !self.owner_uuid_was.nil?
       MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
-      update_permissions self.owner_uuid_was, self.uuid, 0, false
+      update_permissions self.owner_uuid_was, self.uuid, 0
     end
   end
 
@@ -154,9 +155,14 @@ SELECT 1 FROM #{PERMISSION_VIEW}
   end
 
   def clear_permissions
+    update_permissions self.owner_uuid, self.uuid, 0
     MaterializedPermission.where("user_uuid = ? or target_uuid = ?", uuid, uuid).delete_all
   end
 
+  def check_permissions
+    check_permissions_against_full_refresh
+  end
+
   # Return a hash of {user_uuid: group_perms}
   def self.all_group_permissions
     all_perms = {}
@@ -348,7 +354,7 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
       raise "user does not exist" if !new_user
       raise "cannot merge to an already merged user" if new_user.redirect_to_user_uuid
 
-      update_permissions self.owner_uuid, self.uuid, 0
+      self.clear_permissions
 
       # If 'self' is a remote user, don't transfer authorizations
       # (i.e. ability to access the account) to the new user, because
@@ -424,7 +430,7 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
       if redirect_to_new_user
         update_attributes!(redirect_to_user_uuid: new_user.uuid, username: nil)
       end
-      update_permissions self.owner_uuid, self.uuid, 3
+      update_permissions self.owner_uuid, self.uuid, 3, false
       update_permissions new_user.owner_uuid, new_user.uuid, 3
     end
   end
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index e89978d7b..77235c000 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -47,7 +47,7 @@ def refresh_permission_view(async=false)
 end
 
 
-def update_permissions perm_origin_uuid, starting_uuid, perm_level, check=false
+def update_permissions perm_origin_uuid, starting_uuid, perm_level, check=true
   # Update a subset of the permission graph
   # perm_level is the inherited permission
   # perm_level is a number from 0-3
@@ -93,41 +93,46 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
 },
                                            "update_permissions.insert"
 
-  if check
-    #
-    # For validation/debugging, this checks contents of the
-    # incrementally-updated 'materialized_permission' against a
-    # from-scratch permission refresh.
-    #
+  if check and perm_level>0
+    check_permissions_against_full_refresh
+  end
+end
+
+
+def check_permissions_against_full_refresh
+  #
+  # For debugging, this checks contents of the
+  # incrementally-updated 'materialized_permission' against a
+  # from-scratch permission refresh.
+  #
 
-    q1 = ActiveRecord::Base.connection.exec_query %{
+  q1 = ActiveRecord::Base.connection.exec_query %{
 select user_uuid, target_uuid, perm_level, traverse_owned from #{PERMISSION_VIEW}
 order by user_uuid, target_uuid
 }
 
-    q2 = ActiveRecord::Base.connection.exec_query %{
+  q2 = ActiveRecord::Base.connection.exec_query %{
 select users.uuid as user_uuid, g.target_uuid, g.val as perm_level, g.traverse_owned
 from users, lateral search_permission_graph(users.uuid, 3) as g where g.val > 0
 order by users.uuid, target_uuid
 }
 
-    if q1.count != q2.count
-      puts "Didn't match incremental+: #{q1.count} != full refresh-: #{q2.count}"
-    end
+  if q1.count != q2.count
+    puts "Didn't match incremental+: #{q1.count} != full refresh-: #{q2.count}"
+  end
 
-    if q1.count > q2.count
-      q1.each_with_index do |r, i|
-        if r != q2[i]
-          puts "+#{r}\n-#{q2[i]}"
-          raise "Didn't match"
-        end
+  if q1.count > q2.count
+    q1.each_with_index do |r, i|
+      if r != q2[i]
+        puts "+#{r}\n-#{q2[i]}"
+        raise "Didn't match"
       end
-    else
-      q2.each_with_index do |r, i|
-        if r != q1[i]
-          puts "+#{q1[i]}\n-#{r}"
-          raise "Didn't match"
-        end
+    end
+  else
+    q2.each_with_index do |r, i|
+      if r != q1[i]
+        puts "+#{q1[i]}\n-#{r}"
+        raise "Didn't match"
       end
     end
   end

-----------------------------------------------------------------------


hooks/post-receive
--