13561: Expand index API to include past versions.

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

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 05b39c1aed5f71a62b3526cc6df52e12e49022b8..a6718dd4d110f270f26e230af9fbf90e4d5ae376 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -189,7 +189,10 @@ class ApplicationController < ActionController::Base
   end
 
   def find_objects_for_index
-    @objects ||= model_class.readable_by(*@read_users, {:include_trash => (params[:include_trash] || 'untrash' == action_name)})
+    @objects ||= model_class.readable_by(*@read_users, {
+      :include_trash => (params[:include_trash] || 'untrash' == action_name),
+      :include_old_versions => params[:include_old_versions]
+    })
     apply_where_limit_order_params
   end
 

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 6e77c12a1d6f37a88b45c4875ee43e7c912b94a9..cfd1e9dc20253646d11c94e7d026b510ffa831bb 100644 (file)
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -27,9 +27,14 @@ class Arvados::V1::CollectionsController < ApplicationController
   end
 
   def find_objects_for_index
+    opts = {}
     if params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name)
-      @objects = Collection.readable_by(*@read_users, {include_trash: true})
+      opts.update({include_trash: true})
     end
+    if params[:include_old_versions]
+      opts.update({include_old_versions: true})
+    end
+    @objects = Collection.readable_by(*@read_users, opts) if !opts.empty?
     super
   end
 
@@ -223,4 +228,17 @@ class Arvados::V1::CollectionsController < ApplicationController
       @select ||= model_class.selectable_attributes - ["manifest_text", "unsigned_manifest_text"]
     end
   end
+
+  def load_filters_param
+    super
+    return if !params[:include_old_versions]
+    @filters = @filters.map do |col, operator, operand|
+      # Replace uuid filters when including past versions
+      if col == 'uuid'
+        ['current_version_uuid', operator, operand]
+      else
+        [col, operator, operand]
+      end
+    end
+  end
 end

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index c983372ad87e6a4b75d3c135039da0d49407ccc6..801da17dbee5455e33991a64d542d4ff9eaad1da 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -369,7 +369,13 @@ class ArvadosModel < ActiveRecord::Base
         end
 
         self[:name] = new_name
-        self[:uuid] = nil if uuid_was.nil? && !uuid.nil?
+        if uuid_was.nil? && !uuid.nil?
+          self[:uuid] = nil
+          if self.is_a? Collection
+            # Reset so that is assigned to the new UUID
+            self[:current_version_uuid] = nil
+          end
+        end
         conn.exec_query 'SAVEPOINT save_with_unique_name'
         retry
       ensure

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 349c002ef4e548fe168c12b4f6f2bdc718e19da1..2e79d7460348499b775d52574e8668f883dd5910 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -29,7 +29,6 @@ class Collection < ArvadosModel
   validate :ensure_storage_classes_contain_non_empty_strings
   validate :old_versions_cannot_be_updated, on: :update
   before_save :set_file_names
-  before_create :set_current_version_uuid
 
   api_accessible :user, extend: :common do |t|
     t.add :name
@@ -219,10 +218,6 @@ class Collection < ArvadosModel
     ['current_version_uuid']
   end
 
-  def set_current_version_uuid
-    self.current_version_uuid ||= self.uuid
-  end
-
   def save! *args
     # Skip if feature is disabled or saving a new record
     if !Rails.configuration.collection_versioning || new_record?
@@ -547,7 +542,7 @@ class Collection < ArvadosModel
   end
 
   def self.searchable_columns operator
-    super - ["manifest_text", "current_version_uuid"]
+    super - ["manifest_text"]
   end
 
   def self.full_text_searchable_columns
@@ -632,4 +627,10 @@ class Collection < ArvadosModel
       raise ArvadosModel::PermissionDeniedError.new("previous versions cannot be updated")
     end
   end
+
+  def assign_uuid
+    super
+    self.current_version_uuid ||= self.uuid
+    true
+  end
 end

diff --git a/services/api/db/migrate/20181004131141_add_current_version_uuid_to_collection_search_index.rb b/services/api/db/migrate/20181004131141_add_current_version_uuid_to_collection_search_index.rb
new file mode 100644 (file)
index 0000000..63e9919
--- /dev/null
+++ b/services/api/db/migrate/20181004131141_add_current_version_uuid_to_collection_search_index.rb
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddCurrentVersionUuidToCollectionSearchIndex < ActiveRecord::Migration
+  disable_ddl_transaction!
+
+  def up
+    remove_index :collections, :name => 'collections_search_index'
+    add_index :collections, ["owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "portable_data_hash", "uuid", "name", "current_version_uuid"], name: 'collections_search_index', algorithm: :concurrently
+  end
+
+  def down
+    remove_index :collections, :name => 'collections_search_index'
+    add_index :collections, ["owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "portable_data_hash", "uuid", "name"], name: 'collections_search_index', algorithm: :concurrently
+  end
+end

diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 67bc6856db0791bf8c6b9d7d15b9f5d9b2dc2643..71d630f6e81139e88800550e3fbe78bd7ab10d92 100644 (file)
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -1634,7 +1634,7 @@ CREATE INDEX collections_full_text_search_idx ON public.collections USING gin (t
 -- Name: collections_search_index; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE INDEX collections_search_index ON public.collections USING btree (owner_uuid, modified_by_client_uuid, modified_by_user_uuid, portable_data_hash, uuid, name);
+CREATE INDEX collections_search_index ON public.collections USING btree (owner_uuid, modified_by_client_uuid, modified_by_user_uuid, portable_data_hash, uuid, name, current_version_uuid);
 
 
 --
@@ -3187,3 +3187,5 @@ INSERT INTO schema_migrations (version) VALUES ('20180919001158');
 
 INSERT INTO schema_migrations (version) VALUES ('20181001175023');
 
+INSERT INTO schema_migrations (version) VALUES ('20181004131141');
+

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 970368479c5b0cda8acb75ec29a552d229bc9e8b..24a1a09cfd8628ce157d43b0fa2cb4a53268e65b 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -45,6 +45,17 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
            "basic Collections index included past version")
   end
 
+  test "get index with include_old_versions" do
+    authorize_with :active
+    get :index, {
+      include_old_versions: true
+    }
+    assert_response :success
+    assert(assigns(:objects).andand.any?, "no Collections returned in index")
+    assert(json_response["items"].any? { |c| c["uuid"] == collections(:collection_owned_by_active_past_version_1).uuid },
+           "past version not included on index")
+  end
+
   test "collections.get returns signed locators, and no unsigned_manifest_text" do
     permit_unsigned_manifests
     authorize_with :active
@@ -1145,4 +1156,21 @@ EOS
     end
     assert_includes(item_uuids, collections(:collection_in_trashed_subproject).uuid)
   end
+
+  test 'can get collection with past versions' do
+    authorize_with :active
+    get :index, {
+      filters: [['uuid','=',collections(:collection_owned_by_active).uuid]],
+      include_old_versions: true
+    }
+    assert_response :success
+    assert_equal 2, assigns(:objects).length
+    assert_equal 2, json_response['items_available']
+    assert_equal 2, json_response['items'].count
+    json_response['items'].each do |c|
+      assert_equal collections(:collection_owned_by_active).uuid,
+                   c['current_version_uuid'],
+                   'response includes a version from a different collection'
+    end
+  end
 end