13561: Avoid collections.index to include old versions

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

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 4ba4ac77105db67b2037199a792ab3f79f635295..c983372ad87e6a4b75d3c135039da0d49407ccc6 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -240,7 +240,7 @@ class ArvadosModel < ActiveRecord::Base
     end.compact.uniq
   end
 
-  # Return a query with read permissions restricted to the union of of the
+  # Return a query with read permissions restricted to the union of the
   # permissions of the members of users_list, i.e. if something is readable by
   # any user in users_list, it will be readable in the query returned by this
   # function.
@@ -258,6 +258,7 @@ class ArvadosModel < ActiveRecord::Base
     # Collect the UUIDs of the authorized users.
     sql_table = kwargs.fetch(:table_name, table_name)
     include_trash = kwargs.fetch(:include_trash, false)
+    include_old_versions = kwargs.fetch(:include_old_versions, false)
 
     sql_conds = nil
     user_uuids = users_list.map { |u| u.uuid }
@@ -268,6 +269,11 @@ class ArvadosModel < ActiveRecord::Base
       exclude_trashed_records = "AND #{sql_table}.is_trashed = false"
     end
 
+    exclude_old_versions = ""
+    if !include_old_versions && sql_table == "collections"
+      exclude_old_versions = "AND #{sql_table}.uuid = #{sql_table}.current_version_uuid"
+    end
+
     if users_list.select { |u| u.is_admin }.any?
       # Admin skips most permission checks, but still want to filter on trashed items.
       if !include_trash
@@ -275,7 +281,7 @@ class ArvadosModel < ActiveRecord::Base
           # Only include records where the owner is not trashed
           sql_conds = "NOT EXISTS(SELECT 1 FROM #{PERMISSION_VIEW} "+
                       "WHERE trashed = 1 AND "+
-                      "(#{sql_table}.owner_uuid = target_uuid)) #{exclude_trashed_records}"
+                      "(#{sql_table}.owner_uuid = target_uuid)) #{exclude_trashed_records} #{exclude_old_versions}"
         end
       end
     else
@@ -312,7 +318,7 @@ class ArvadosModel < ActiveRecord::Base
                        "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"
       end
 
-      sql_conds = "(#{direct_check} #{owner_check} #{links_cond}) #{exclude_trashed_records}"
+      sql_conds = "(#{direct_check} #{owner_check} #{links_cond}) #{exclude_trashed_records} #{exclude_old_versions}"
 
     end
 

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index a449b5ef6d787426819363d292398988ac6df9dc..349c002ef4e548fe168c12b4f6f2bdc718e19da1 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -47,6 +47,9 @@ class Collection < ArvadosModel
     t.add :delete_at
     t.add :trash_at
     t.add :is_trashed
+    t.add :version
+    t.add :current_version_uuid
+    t.add :preserve_version
   end
 
   after_initialize do

diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 3578651ba031833bc1ea8ab54f72aa2d937195e2..2bc362a4c88873550ac12b0bea52e3d68510521b 100644 (file)
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -27,6 +27,21 @@ collection_owned_by_active:
   updated_at: 2014-02-03T17:22:54Z
   manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
   name: owned_by_active
+  version: 2
+
+collection_owned_by_active_past_version_1:
+  uuid: zzzzz-4zz18-znfnqtbbv4spast
+  current_version_uuid: zzzzz-4zz18-bv31uwvy3neko21
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-02-03T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2014-02-03T15:22:54Z
+  updated_at: 2014-02-03T15:22:54Z
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  name: owned_by_active_version_1
+  version: 1
 
 foo_file:
   uuid: zzzzz-4zz18-znfnqtbbv4spc3w

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index e6ecea219b9da1ea4c71fee07dddf025aa78aab3..970368479c5b0cda8acb75ec29a552d229bc9e8b 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -41,6 +41,8 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
     assert(assigns(:objects).andand.any?, "no Collections returned in index")
     refute(json_response["items"].any? { |c| c.has_key?("manifest_text") },
            "basic Collections index included manifest_text")
+    refute(json_response["items"].any? { |c| c["uuid"] == collections(:collection_owned_by_active_past_version_1).uuid },
+           "basic Collections index included past version")
   end
 
   test "collections.get returns signed locators, and no unsigned_manifest_text" do