[ARVADOS] created: 484370cbfe47b04e1d4222dd4a7606171c87a324

[Git user]

        at  484370cbfe47b04e1d4222dd4a7606171c87a324 (commit)


commit 484370cbfe47b04e1d4222dd4a7606171c87a324
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Feb 17 16:17:26 2017 -0500

    11127: Delete dependent links too when emptying trash.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index fd542ca..b2e1bea 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -32,7 +32,11 @@ class ArvadosModel < ActiveRecord::Base
   # Note: This only returns permission links. It does not account for
   # permissions obtained via user.is_admin or
   # user.uuid==object.owner_uuid.
-  has_many :permissions, :foreign_key => :head_uuid, :class_name => 'Link', :primary_key => :uuid, :conditions => "link_class = 'permission'"
+  has_many(:permissions,
+           foreign_key: :head_uuid,
+           class_name: 'Link',
+           primary_key: :uuid,
+           conditions: "link_class = 'permission'")
 
   class PermissionDeniedError < StandardError
     def http_status
diff --git a/services/api/lib/has_uuid.rb b/services/api/lib/has_uuid.rb
index e0a5613..36c0879 100644
--- a/services/api/lib/has_uuid.rb
+++ b/services/api/lib/has_uuid.rb
@@ -7,8 +7,8 @@ module HasUuid
     base.validate :validate_uuid
     base.before_create :assign_uuid
     base.before_destroy :destroy_permission_links
-    base.has_many :links_via_head, class_name: 'Link', foreign_key: :head_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :restrict
-    base.has_many :links_via_tail, class_name: 'Link', foreign_key: :tail_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :restrict
+    base.has_many :links_via_head, class_name: 'Link', foreign_key: :head_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :destroy
+    base.has_many :links_via_tail, class_name: 'Link', foreign_key: :tail_uuid, primary_key: :uuid, conditions: "not (link_class = 'permission')", dependent: :destroy
   end
 
   module ClassMethods
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 4984aad..87bec21 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -493,4 +493,20 @@ class CollectionTest < ActiveSupport::TestCase
     SweepTrashedCollections.sweep_now
     assert_empty Collection.unscoped.where(uuid: uuid)
   end
+
+  test "delete referring links in SweepTrashedCollections" do
+    uuid = collections(:trashed_on_next_sweep).uuid
+    act_as_system_user do
+      Link.create!(head_uuid: uuid,
+                   tail_uuid: system_user_uuid,
+                   link_class: 'whatever',
+                   name: 'something')
+    end
+    past = db_current_time
+    Collection.unscoped.where(uuid: uuid).
+      update_all(is_trashed: true, trash_at: past, delete_at: past)
+    assert_not_empty Collection.unscoped.where(uuid: uuid)
+    SweepTrashedCollections.sweep_now
+    assert_empty Collection.unscoped.where(uuid: uuid)
+  end
 end
diff --git a/services/api/test/unit/link_test.rb b/services/api/test/unit/link_test.rb
index 16ce54b..64c9885 100644
--- a/services/api/test/unit/link_test.rb
+++ b/services/api/test/unit/link_test.rb
@@ -7,17 +7,27 @@ class LinkTest < ActiveSupport::TestCase
     set_user_from_auth :admin_trustedclient
   end
 
-  test "cannot delete an object referenced by links" do
-    ob = Specimen.create
-    link = Link.create(tail_uuid: users(:active).uuid,
-                       head_uuid: ob.uuid,
-                       link_class: 'test',
-                       name: 'test')
+  test "cannot delete an object referenced by unwritable links" do
+    ob = act_as_user users(:active) do
+      Specimen.create
+    end
+    link = act_as_user users(:admin) do
+      Link.create(tail_uuid: users(:active).uuid,
+                  head_uuid: ob.uuid,
+                  link_class: 'test',
+                  name: 'test')
+    end
     assert_equal users(:admin).uuid, link.owner_uuid
-    assert_raises(ActiveRecord::DeleteRestrictionError,
+    assert_raises(ArvadosModel::PermissionDeniedError,
                   "should not delete #{ob.uuid} with link #{link.uuid}") do
+      act_as_user users(:active) do
+        ob.destroy
+      end
+    end
+    act_as_user users(:admin) do
       ob.destroy
     end
+    assert_empty Link.where(uuid: link.uuid)
   end
 
   def new_active_link_valid?(link_attrs)

commit 85887cd7fed798345e340480062b8ffcf3cf053a
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Feb 17 14:49:13 2017 -0500

    11127: Do not crash server if SweepTrashedCollections thread has an exception.

diff --git a/services/api/lib/sweep_trashed_collections.rb b/services/api/lib/sweep_trashed_collections.rb
index ab2d27a..01b63fd 100644
--- a/services/api/lib/sweep_trashed_collections.rb
+++ b/services/api/lib/sweep_trashed_collections.rb
@@ -23,8 +23,11 @@ module SweepTrashedCollections
     end
     if need
       Thread.new do
+        Thread.current.abort_on_exception = false
         begin
           sweep_now
+        rescue => e
+          Rails.logger.error "#{e.class}: #{e}\n#{e.backtrace.join("\n\t")}"
         ensure
           ActiveRecord::Base.connection.close
         end

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


hooks/post-receive
--