Merge branch '4759-timestamp-precision-TC' closes #4759
authorTom Clegg <tom@curoverse.com>
Tue, 17 Feb 2015 17:07:50 +0000 (12:07 -0500)
committerTom Clegg <tom@curoverse.com>
Tue, 17 Feb 2015 17:07:50 +0000 (12:07 -0500)
21 files changed:
COPYING
apps/workbench/app/controllers/collections_controller.rb
apps/workbench/app/controllers/users_controller.rb
apps/workbench/app/views/collections/show.html.erb
apps/workbench/test/controllers/collections_controller_test.rb
apps/workbench/test/controllers/users_controller_test.rb
apps/workbench/test/integration/anonymous_access_test.rb
doc/api/schema/Collection.html.textile.liquid
sdk/python/arvados/commands/put.py
sdk/python/tests/test_arv_put.py
services/api/app/controllers/arvados/v1/schema_controller.rb
services/api/app/models/collection.rb
services/api/config/application.default.yml
services/api/db/migrate/20150206230342_rename_replication_attributes.rb [new file with mode: 0644]
services/api/db/structure.sql
services/api/test/fixtures/collections.yml
services/api/test/functional/arvados/v1/collections_controller_test.rb
services/api/test/integration/collections_api_test.rb
services/api/test/unit/arvados_model_test.rb
services/api/test/unit/collection_test.rb
services/fuse/arvados_fuse/__init__.py

diff --git a/COPYING b/COPYING
index 4006e686dad73c4053af9b11d312501de55b10b3..acbd7523ed49f01217874965aa3180cccec89d61 100644 (file)
--- a/COPYING
+++ b/COPYING
@@ -1,5 +1,5 @@
 Server-side components of Arvados contained in the apps/ and services/
-directories, including the API Server, Workbench, and Crunch, are licenced
+directories, including the API Server, Workbench, and Crunch, are licensed
 under the GNU Affero General Public License version 3 (see agpl-3.0.txt)
 
 The Arvados client Software Development Kits contained in the sdk/ directory,
index 3ef46887d82c24a2edc2e5e6778b1a1fd20769e9..e883017070d20ccdc7613e8f6c88ea9acaf1930e 100644 (file)
@@ -266,6 +266,15 @@ class CollectionsController < ApplicationController
     sharing_popup
   end
 
+  def update
+    @updates ||= params[@object.resource_param_name.to_sym]
+    if @updates && (@updates.keys - ["name", "description"]).empty?
+      # exclude manifest_text since only name or description is being updated
+      @object.manifest_text = nil
+    end
+    super
+  end
+
   protected
 
   def find_usable_token(token_list)
index 7dcd4cc08d3ea91802fbec54f424c44d3e8c5966..0ca5a85f018af48187865efe030195bbdeeebdbf 100644 (file)
@@ -240,7 +240,14 @@ class UsersController < ApplicationController
               ['tail_uuid', '=', current_user.uuid],
               ['link_class', '=', 'permission'],
              ])
-    @my_repositories = Repository.where uuid: repo_links.collect(&:head_uuid)
+
+    owned_repositories = Repository.where(owner_uuid: current_user.uuid)
+
+    @my_repositories = (Repository.where(uuid: repo_links.collect(&:head_uuid)) |
+                        owned_repositories).
+                       uniq { |repo| repo.uuid }
+
+
     @repo_writable = {}
     repo_links.each do |link|
       if link.name.in? ['can_write', 'can_manage']
@@ -248,6 +255,10 @@ class UsersController < ApplicationController
       end
     end
 
+    owned_repositories.each do |repo|
+      @repo_writable[repo.uuid] = 'can_manage'
+    end
+
     # virtual machines the current user can login into
     @my_vm_logins = {}
     Link.where(tail_uuid: current_user.uuid,
index c3e0b7cb2eb1aa64fc210dc90daca92b1443d107..75a70868caec34917db04f316885a4ff6f4694e2 100644 (file)
@@ -2,25 +2,25 @@
   <div class="col-md-6">
     <div class="panel panel-info">
       <div class="panel-heading">
-       <h3 class="panel-title">
+        <h3 class="panel-title">
           <% if @name_link.nil? and @object.uuid.match /[0-9a-f]{32}/ %>
             Content hash <%= @object.portable_data_hash %>
           <% else %>
-           <%= if @object.respond_to? :name
+            <%= if @object.respond_to? :name
                   render_editable_attribute @object, :name
                 else
                   @name_link.andand.name || @object.uuid
                 end %>
             <% end %>
-       </h3>
+        </h3>
       </div>
       <div class="panel-body">
         <div class="arv-description-as-subtitle">
           <%= render_editable_attribute @object, 'description', nil, { 'data-emptytext' => "(No description provided)", 'data-toggle' => 'manual' } %>
         </div>
         <img src="/favicon.ico" class="pull-right" alt="" style="opacity: 0.3"/>
-       <p><i>Content hash:</i><br />
-         <span class="arvados-uuid"><%= link_to @object.portable_data_hash, collection_path(@object.portable_data_hash) %></span>
+        <p><i>Content hash:</i><br />
+          <span class="arvados-uuid"><%= link_to @object.portable_data_hash, collection_path(@object.portable_data_hash) %></span>
         </p>
         <%= render partial: "show_source_summary" %>
       </div>
   <div class="col-md-3">
     <div class="panel panel-default">
       <div class="panel-heading">
-       <h3 class="panel-title">
-         Activity
-       </h3>
+        <h3 class="panel-title">
+          Activity
+        </h3>
       </div>
       <div class="panel-body smaller-text">
         <!--
-       <input type="text" class="form-control" placeholder="Search"/>
+        <input type="text" class="form-control" placeholder="Search"/>
         -->
-       <div style="height:0.5em;"></div>
+        <div style="height:0.5em;"></div>
         <% name_or_object = @name_link.andand.uuid ? @name_link : @object %>
         <% if name_or_object.created_at and not @logs.andand.any? %>
           <p>
       </div>
     </div>
   </div>
+  <% if current_user %>
   <div class="col-md-3">
     <div class="panel panel-default">
       <div class="panel-heading">
-       <h3 class="panel-title">
-         Sharing and permissions
-       </h3>
+        <h3 class="panel-title">
+          Sharing and permissions
+        </h3>
       </div>
       <div class="panel-body">
         <!--
-       <input type="text" class="form-control" placeholder="Search"/>
+        <input type="text" class="form-control" placeholder="Search"/>
         -->
 
         <div id="sharing-button">
           <%= render partial: 'sharing_button' %>
         </div>
 
-       <div style="height:0.5em;"></div>
+        <div style="height:0.5em;"></div>
         <% if @projects.andand.any? %>
           <p>Included in projects:<br />
           <%= render_arvados_object_list_start(@projects, 'Show all projects',
       </div>
     </div>
   </div>
+  <% else %>
+  <div class="col-md-3">
+    <div class="panel panel-default">
+      <div class="panel-heading">
+        <h3 class="panel-title">
+          Welcome to Arvados
+        </h3>
+      </div>
+      <div class="panel-body">
+        You are accessing public data.
+      </div>
+    </div>
+  </div>
+  <% end %>
 </div>
 
 <%= render file: 'application/show.html.erb', locals: local_assigns %>
index 45124f7a9e3ff8cdf3fbff29ef0c488caa1dcf5d..dfbe69f987f25c75cf03f7fe5b5a3a7398fd919d 100644 (file)
@@ -355,4 +355,52 @@ class CollectionsControllerTest < ActionController::TestCase
     assert /#{stage3_id}&#45;&gt;#{stage3_out}/.match(used_by_svg)
 
   end
+
+  test "view collection with empty properties" do
+    fixture_name = :collection_with_empty_properties
+    show_collection(fixture_name, :active)
+    assert_equal(api_fixture('collections')[fixture_name.to_s]['name'], assigns(:object).name)
+    assert_not_nil(assigns(:object).properties)
+    assert_empty(assigns(:object).properties)
+  end
+
+  test "view collection with one property" do
+    fixture_name = :collection_with_one_property
+    show_collection(fixture_name, :active)
+    fixture = api_fixture('collections')[fixture_name.to_s]
+    assert_equal(fixture['name'], assigns(:object).name)
+    assert_equal(fixture['properties'][0], assigns(:object).properties[0])
+  end
+
+  test "create collection with properties" do
+    post :create, {
+      collection: {
+        name: 'collection created with properties',
+        manifest_text: '',
+        properties: {
+          property_1: 'value_1'
+        },
+      },
+      format: :json
+    }, session_for(:active)
+    assert_response :success
+    assert_not_nil assigns(:object).uuid
+    assert_equal 'collection created with properties', assigns(:object).name
+    assert_equal 'value_1', assigns(:object).properties[:property_1]
+  end
+
+  test "update description and check manifest_text is not lost" do
+    collection = api_fixture("collections")["multilevel_collection_1"]
+    post :update, {
+      id: collection["uuid"],
+      collection: {
+        description: 'test description update'
+      },
+      format: :json
+    }, session_for(:active)
+    assert_response :success
+    assert_not_nil assigns(:object)
+    assert_equal 'test description update', assigns(:object).description
+    assert_equal collection['manifest_text'], assigns(:object).manifest_text
+  end
 end
index ebec4a3d226821a785a7604b2a4c42171e3885fd..c1436da4545e93197c95d2b850614cf55c95cafc 100644 (file)
@@ -41,6 +41,17 @@ class UsersControllerTest < ActionController::TestCase
     assert_includes editables, false, "should have a readonly repository"
   end
 
+  test "show repositories lists linked as well as owned repositories" do
+    get :manage_account, {}, session_for(:active)
+    assert_response :success
+    repos = assigns(:my_repositories)
+    assert repos
+    repo_writables = assigns(:repo_writable)
+    assert_not_empty repo_writables, "repo_writables should not be empty"
+    assert_includes repo_writables, api_fixture('repositories')['repository4']['uuid']  # writable by active
+    assert_includes repo_writables, api_fixture('repositories')['repository2']['uuid']  # owned by active
+  end
+
   test "request shell access" do
     user = api_fixture('users')['spectator']
 
index 2e04dcaecdba5f744f2a1cf0c0779848d3958392..6508879099d8252c5ca2d9561cb961661174acb4 100644 (file)
@@ -87,6 +87,7 @@ class AnonymousAccessTest < ActionDispatch::IntegrationTest
 
     # in collection page
     assert_no_selector 'input', text: 'Create sharing link'
+    assert_no_text 'Sharing and permissions'
     assert_no_selector 'a', text: 'Upload'
     assert_no_selector 'button', 'Selection'
 
index 69a8dc3366b658e81069b3805c06141513621960..9aa783cbf5f6b10d3e93c72523d67301eccfb410 100644 (file)
@@ -30,11 +30,10 @@ Each collection has, in addition to the usual "attributes of Arvados resources":
 
 table(table table-bordered table-condensed).
 |_. Attribute|_. Type|_. Description|_. Example|
-|locator|string|||
-|portable_data_hash|string|||
 |name|string|||
-|redundancy|number|||
-|redundancy_confirmed_by_client_uuid|string|API client||
-|redundancy_confirmed_at|datetime|||
-|redundancy_confirmed_as|number|||
+|description|text|||
+|portable_data_hash|string|||
 |manifest_text|text|||
+|replication_desired|number|Minimum storage replication level desired for each data block referenced by this collection. A value of @null@ signifies that the site default replication level (typically 2) is desired.|@2@|
+|replication_confirmed|number|Replication level most recently confirmed by the storage system. This field is null when a collection is first created, and is reset to null when the manifest_text changes in a way that introduces a new data block. An integer value indicates the replication level of the _least replicated_ data block in the collection.|@2@, null|
+|replication_confirmed_at|datetime|When replication_confirmed was confirmed. If replication_confirmed is null, this field is also null.||
index 4383514df5f42eb34c400a24e58d5652330cce3a..f556e7ecb598eb0381400709d7edd272c76c7be3 100644 (file)
@@ -408,10 +408,18 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
         print >>stderr, error
         sys.exit(1)
 
-    # Apply default replication, if none specified. TODO (#3410): Use
-    # default replication given by discovery document.
-    if args.replication <= 0:
-        args.replication = 2
+    # write_copies diverges from args.replication here.
+    # args.replication is how many copies we will instruct Arvados to
+    # maintain (by passing it in collections().create()) after all
+    # data is written -- and if None was given, we'll use None there.
+    # Meanwhile, write_copies is how many copies of each data block we
+    # write to Keep, which has to be a number.
+    #
+    # If we simply changed args.replication from None to a default
+    # here, we'd end up erroneously passing the default replication
+    # level (instead of None) to collections().create().
+    write_copies = (args.replication or
+                    api_client._rootDesc.get('defaultCollectionReplication', 2))
 
     if args.progress:
         reporter = progress_writer(human_progress)
@@ -437,12 +445,12 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
         writer = ArvPutCollectionWriter(
             resume_cache, reporter, bytes_expected,
             num_retries=args.retries,
-            replication=args.replication)
+            replication=write_copies)
     else:
         writer = ArvPutCollectionWriter.from_cache(
             resume_cache, reporter, bytes_expected,
             num_retries=args.retries,
-            replication=args.replication)
+            replication=write_copies)
 
     # Install our signal handler for each code in CAUGHT_SIGNALS, and save
     # the originals.
@@ -481,7 +489,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
                 manifest_text = CollectionReader(manifest_text).manifest_text(normalize=True)
             replication_attr = 'replication_desired'
             if api_client._schema.schemas['Collection']['properties'].get(replication_attr, None) is None:
-                # API calls it 'redundancy' until #3410.
+                # API called it 'redundancy' before #3410.
                 replication_attr = 'redundancy'
             # Register the resulting collection in Arvados.
             collection = api_client.collections().create(
index 41fa8cfb66df03fc478d7f96642d6283c0ba5734..eaefd790b0741585804ac0f6e503ee2cad31fe0f 100644 (file)
@@ -543,7 +543,7 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
 
     def test_put_collection_with_default_redundancy(self):
         collection = self.run_and_find_collection("")
-        self.assertEqual(2, collection['replication_desired'])
+        self.assertEqual(None, collection['replication_desired'])
 
     def test_put_collection_with_unnamed_project_link(self):
         link = self.run_and_find_collection(
index 09664dadd1aa0857ebdfe5a6df2b8ff9c3bf44dc..c108fb898dfc43300f511fe84851dfe72ad47dd9 100644 (file)
@@ -25,6 +25,7 @@ class Arvados::V1::SchemaController < ApplicationController
         title: "Arvados API",
         description: "The API to interact with Arvados.",
         documentationLink: "http://doc.arvados.org/api/index.html",
+        defaultCollectionReplication: Rails.configuration.default_collection_replication,
         protocol: "rest",
         baseUrl: root_url + "arvados/v1/",
         basePath: "/arvados/v1/",
index 334f3c699bb089d99b79d81b1adb6429927cb1cd..89ad874cd7d211aae90f841927a620150af1ab9b 100644 (file)
@@ -5,10 +5,13 @@ class Collection < ArvadosModel
   include KindAndEtag
   include CommonApiTemplate
 
+  serialize :properties, Hash
+
   before_validation :check_encoding
   before_validation :check_signatures
   before_validation :strip_manifest_text
   before_validation :set_portable_data_hash
+  before_validation :maybe_clear_replication_confirmed
   validate :ensure_hash_matches_manifest_text
   before_save :set_file_names
 
@@ -22,6 +25,8 @@ class Collection < ArvadosModel
     t.add :portable_data_hash
     t.add :signed_manifest_text, as: :manifest_text
     t.add :replication_desired
+    t.add :replication_confirmed
+    t.add :replication_confirmed_at
   end
 
   def self.attributes_required_columns
@@ -32,10 +37,6 @@ class Collection < ArvadosModel
                 # API response, and never let clients select the
                 # manifest_text column.
                 'manifest_text' => ['manifest_text'],
-
-                # This is a shim until the database column gets
-                # renamed to replication_desired in #3410.
-                'replication_desired' => ['redundancy'],
                 )
   end
 
@@ -183,27 +184,6 @@ class Collection < ArvadosModel
     end
   end
 
-  def replication_desired
-    # Shim until database columns get fixed up in #3410.
-    redundancy or 2
-  end
-
-  def redundancy_status
-    if redundancy_confirmed_as.nil?
-      'unconfirmed'
-    elsif redundancy_confirmed_as < redundancy
-      'degraded'
-    else
-      if redundancy_confirmed_at.nil?
-        'unconfirmed'
-      elsif Time.now - redundancy_confirmed_at < 7.days
-        'OK'
-      else
-        'stale'
-      end
-    end
-  end
-
   def signed_manifest_text
     if has_attribute? :manifest_text
       token = current_api_client_authorization.andand.api_token
@@ -227,7 +207,7 @@ class Collection < ArvadosModel
   def self.munge_manifest_locators! manifest
     # Given a manifest text and a block, yield each locator,
     # and replace it with whatever the block returns.
-    manifest.andand.gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) do |word|
+    manifest.andand.gsub!(/ [[:xdigit:]]{32}(\+\S+)?/) do |word|
       if loc = Keep::Locator.parse(word.strip)
         " " + yield(loc)
       else
@@ -236,6 +216,15 @@ class Collection < ArvadosModel
     end
   end
 
+  def self.each_manifest_locator manifest
+    # Given a manifest text and a block, yield each locator.
+    manifest.andand.scan(/ ([[:xdigit:]]{32}(\+\S+)?)/) do |word, _|
+      if loc = Keep::Locator.parse(word)
+        yield loc
+      end
+    end
+  end
+
   def self.normalize_uuid uuid
     hash_part = nil
     size_part = nil
@@ -321,7 +310,11 @@ class Collection < ArvadosModel
   def portable_manifest_text
     portable_manifest = self[:manifest_text].dup
     self.class.munge_manifest_locators!(portable_manifest) do |loc|
-      loc.hash + '+' + loc.size.to_s
+      if loc.size
+        loc.hash + '+' + loc.size.to_s
+      else
+        loc.hash
+      end
     end
     portable_manifest
   end
@@ -332,4 +325,32 @@ class Collection < ArvadosModel
      '+' +
      portable_manifest.bytesize.to_s)
   end
+
+  def maybe_clear_replication_confirmed
+    if manifest_text_changed?
+      # If the new manifest_text contains locators whose hashes
+      # weren't in the old manifest_text, storage replication is no
+      # longer confirmed.
+      in_old_manifest = {}
+      self.class.each_manifest_locator(manifest_text_was) do |loc|
+        in_old_manifest[loc.hash] = true
+      end
+      self.class.each_manifest_locator(manifest_text) do |loc|
+        if not in_old_manifest[loc.hash]
+          self.replication_confirmed_at = nil
+          self.replication_confirmed = nil
+          break
+        end
+      end
+    end
+  end
+
+  def ensure_permission_to_save
+    if (not current_user.andand.is_admin and
+        (replication_confirmed_at_changed? or replication_confirmed_changed?) and
+        not (replication_confirmed_at.nil? and replication_confirmed.nil?))
+      raise ArvadosModel::PermissionDeniedError.new("replication_confirmed and replication_confirmed_at attributes cannot be changed, except by setting both to nil")
+    end
+    super
+  end
 end
index 2d62e40f034a6bcbaca3697768d9ca7e48246ad4..1e8d79fcd679d1f4cb6b82ac61e55a519484314d 100644 (file)
@@ -244,4 +244,8 @@ common:
   # Permit insecure (OpenSSL::SSL::VERIFY_NONE) connections to the Single Sign
   # On (sso) server.  Should only be enabled during development when the SSO
   # server is using a self-signed cert.
-  sso_insecure: false
\ No newline at end of file
+  sso_insecure: false
+
+  # Default replication level for collections. This is used when a
+  # collection's replication_desired attribute is nil.
+  default_collection_replication: 2
diff --git a/services/api/db/migrate/20150206230342_rename_replication_attributes.rb b/services/api/db/migrate/20150206230342_rename_replication_attributes.rb
new file mode 100644 (file)
index 0000000..58572d7
--- /dev/null
@@ -0,0 +1,30 @@
+class RenameReplicationAttributes < ActiveRecord::Migration
+  RENAME = [[:redundancy, :replication_desired],
+            [:redundancy_confirmed_as, :replication_confirmed],
+            [:redundancy_confirmed_at, :replication_confirmed_at]]
+
+  def up
+    RENAME.each do |oldname, newname|
+      rename_column :collections, oldname, newname
+    end
+    remove_column :collections, :redundancy_confirmed_by_client_uuid
+    Collection.reset_column_information
+
+    # Removing that column dropped some search indexes. Let's put them back.
+    add_index :collections, ["owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "portable_data_hash", "uuid", "name", "file_names"], name: 'collections_search_index'
+    execute "CREATE INDEX collections_full_text_search_idx ON collections USING gin(#{Collection.full_text_tsvector});"
+  end
+
+  def down
+    remove_index :collections, name: 'collections_search_index'
+    add_column :collections, :redundancy_confirmed_by_client_uuid, :string
+    RENAME.reverse.each do |oldname, newname|
+      rename_column :collections, newname, oldname
+    end
+    remove_index :collections, :name => 'collections_full_text_search_idx'
+    Collection.reset_column_information
+
+    execute "CREATE INDEX collections_full_text_search_idx ON collections USING gin(#{Collection.full_text_tsvector});"
+    add_index :collections, ["owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "portable_data_hash", "uuid", "name", "file_names", "redundancy_confirmed_by_client_uuid"], name: 'collections_search_index'
+  end
+end
index e2c6b66719b97cf959e89b1c4fb0c5aa287e7ec3..756dfc90c1e22b26ff16240694e55bf6057d4da8 100644 (file)
@@ -159,10 +159,9 @@ CREATE TABLE collections (
     modified_by_user_uuid character varying(255),
     modified_at timestamp without time zone,
     portable_data_hash character varying(255),
-    redundancy integer,
-    redundancy_confirmed_by_client_uuid character varying(255),
-    redundancy_confirmed_at timestamp without time zone,
-    redundancy_confirmed_as integer,
+    replication_desired integer,
+    replication_confirmed_at timestamp without time zone,
+    replication_confirmed integer,
     updated_at timestamp without time zone NOT NULL,
     uuid character varying(255),
     manifest_text text,
@@ -1311,14 +1310,14 @@ CREATE UNIQUE INDEX collection_owner_uuid_name_unique ON collections USING btree
 -- Name: collections_full_text_search_idx; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
 
-CREATE INDEX collections_full_text_search_idx ON collections USING gin (to_tsvector('english'::regconfig, (((((((((((((((((((COALESCE(owner_uuid, ''::character varying))::text || ' '::text) || (COALESCE(modified_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_user_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(portable_data_hash, ''::character varying))::text) || ' '::text) || (COALESCE(redundancy_confirmed_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || (COALESCE(description, ''::character varying))::text) || ' '::text) || COALESCE(properties, ''::text)) || ' '::text) || (COALESCE(file_names, ''::character varying))::text)));
+CREATE INDEX collections_full_text_search_idx ON collections USING gin (to_tsvector('english'::regconfig, (((((((((((((((((COALESCE(owner_uuid, ''::character varying))::text || ' '::text) || (COALESCE(modified_by_client_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(modified_by_user_uuid, ''::character varying))::text) || ' '::text) || (COALESCE(portable_data_hash, ''::character varying))::text) || ' '::text) || (COALESCE(uuid, ''::character varying))::text) || ' '::text) || (COALESCE(name, ''::character varying))::text) || ' '::text) || (COALESCE(description, ''::character varying))::text) || ' '::text) || COALESCE(properties, ''::text)) || ' '::text) || (COALESCE(file_names, ''::character varying))::text)));
 
 
 --
 -- Name: collections_search_index; Type: INDEX; Schema: public; Owner: -; Tablespace: 
 --
 
-CREATE INDEX collections_search_index ON collections USING btree (owner_uuid, modified_by_client_uuid, modified_by_user_uuid, portable_data_hash, redundancy_confirmed_by_client_uuid, uuid, name, file_names);
+CREATE INDEX collections_search_index ON collections USING btree (owner_uuid, modified_by_client_uuid, modified_by_user_uuid, portable_data_hash, uuid, name, file_names);
 
 
 --
@@ -2357,4 +2356,6 @@ INSERT INTO schema_migrations (version) VALUES ('20150123142953');
 
 INSERT INTO schema_migrations (version) VALUES ('20150203180223');
 
-INSERT INTO schema_migrations (version) VALUES ('20150206210804');
\ No newline at end of file
+INSERT INTO schema_migrations (version) VALUES ('20150206210804');
+
+INSERT INTO schema_migrations (version) VALUES ('20150206230342');
\ No newline at end of file
index d5b78aaac211dabfcd1c9f0150f25158d600029a..220122533bea050ef23cb70cef5ba9f5454ef89b 100644 (file)
@@ -401,6 +401,75 @@ collection_with_unique_words_to_test_full_text_search:
   name: collection_with_some_unique_words
   description: The quick_brown_fox jumps over the lazy_dog
 
+replication_undesired_unconfirmed:
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2015-02-07 00:19:28.596506247 Z
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2015-02-07 00:19:28.596338465 Z
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  replication_desired: ~
+  replication_confirmed_at: ~
+  replication_confirmed: ~
+  updated_at: 2015-02-07 00:19:28.596236608 Z
+  uuid: zzzzz-4zz18-wjxq7uzx2m9jj4a
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  name: replication want=null have=null
+
+replication_desired_2_unconfirmed:
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2015-02-07 00:21:35.050333515 Z
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2015-02-07 00:21:35.050189104 Z
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  replication_desired: 2
+  replication_confirmed_at: ~
+  replication_confirmed: ~
+  updated_at: 2015-02-07 00:21:35.050126576 Z
+  uuid: zzzzz-4zz18-3t236wrz4769h7x
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  name: replication want=2 have=null
+
+replication_desired_2_confirmed_2:
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2015-02-07 00:19:28.596506247 Z
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  modified_at: 2015-02-07 00:19:28.596338465 Z
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  replication_desired: 2
+  replication_confirmed_at: 2015-02-07 00:24:52.983381227 Z
+  replication_confirmed: 2
+  updated_at: 2015-02-07 00:24:52.983381227 Z
+  uuid: zzzzz-4zz18-434zv1tnnf2rygp
+  manifest_text: ". acbd18db4cc2f85cedef654fccc4a4d8+3 37b51d194a7513e45b56f6524f2d51f2+3 0:3:foo 3:6:bar\n"
+  name: replication want=2 have=2
+
+collection_with_empty_properties:
+  uuid: zzzzz-4zz18-emptyproperties
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2015-02-13T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2015-02-13T17:22:54Z
+  updated_at: 2015-02-13T17:22:54Z
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  name: collection with empty properties
+  properties: {}
+
+collection_with_one_property:
+  uuid: zzzzz-4zz18-withoneproperty
+  portable_data_hash: fa7aeb5140e2848d39b416daeef4ffc5+45
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2015-02-13T17:22:54Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+  modified_at: 2015-02-13T17:22:54Z
+  updated_at: 2015-02-13T17:22:54Z
+  manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
+  name: collection with one property
+  properties:
+    property1: value1
+
 # Test Helper trims the rest of the file
 
 # Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper
index 61b05570850635d9878c42522b728a12f2527476..54ffe66f174baf341ae19a00a58c71b578f9b3ce 100644 (file)
@@ -698,21 +698,40 @@ EOS
   end
 
   [1, 5, nil].each do |ask|
-    test "Set replication_desired=#{ask} using redundancy attr" do
-      # The Python SDK checks the Collection schema in the discovery
-      # doc, then asks for 'redundancy' or 'replication_desired'
-      # accordingly, so it isn't necessary to maintain backward
-      # compatibility here when the attribute changes to
-      # replication_desired.
+    test "Set replication_desired=#{ask.inspect}" do
+      Rails.configuration.default_collection_replication = 2
       authorize_with :active
       put :update, {
-        id: collections(:collection_owned_by_active).uuid,
+        id: collections(:replication_undesired_unconfirmed).uuid,
         collection: {
-          redundancy: ask,
+          replication_desired: ask,
         },
       }
       assert_response :success
-      assert_equal (ask or 2), json_response['replication_desired']
+      assert_equal ask, json_response['replication_desired']
     end
   end
+
+  test "get collection with properties" do
+    authorize_with :active
+    get :show, {id: collections(:collection_with_one_property).uuid}
+    assert_response :success
+    assert_not_nil json_response['uuid']
+    assert_equal 'value1', json_response['properties']['property1']
+  end
+
+  test "create collection with properties" do
+    authorize_with :active
+    manifest_text = ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt\n"
+    post :create, {
+      collection: {
+        manifest_text: manifest_text,
+        portable_data_hash: "d30fe8ae534397864cb96c544f4cf102+47",
+        properties: {'property_1' => 'value_1'}
+      }
+    }
+    assert_response :success
+    assert_not_nil json_response['uuid']
+    assert_equal 'value_1', json_response['properties']['property_1']
+  end
 end
index 93ed1563f7341c54d53b456b651357f0102a6c69..4251047cea6b74ece4d8e4b1473d554e59daeb7e 100644 (file)
@@ -273,4 +273,25 @@ class CollectionsApiTest < ActionDispatch::IntegrationTest
       assert_equal first_item['description'], 'The quick_brown_fox jumps over the lazy_dog'
     end
   end
+
+  test "create and get collection with properties" do
+    # create collection to be searched for
+    signed_manifest = Collection.sign_manifest(". bad42fa702ae3ea7d888fef11b46f450+44 0:44:my_test_file.txt\n", api_token(:active))
+    post "/arvados/v1/collections", {
+      format: :json,
+      collection: {manifest_text: signed_manifest}.to_json,
+    }, auth(:active)
+    assert_response 200
+    assert_not_nil json_response['uuid']
+    assert_not_nil json_response['properties']
+    assert_empty json_response['properties']
+
+    # update collection's description
+    put "/arvados/v1/collections/#{json_response['uuid']}", {
+      format: :json,
+      collection: { properties: {'property_1' => 'value_1'} }
+    }, auth(:active)
+    assert_response :success
+    assert_equal 'value_1', json_response['properties']['property_1']
+  end
 end
index 750b9334dfb67fcf414a987f22e84520561d0822..09dece2660f38b1ecac09acaaa50543a4224698b 100644 (file)
@@ -136,7 +136,7 @@ class ArvadosModelTest < ActiveSupport::TestCase
 
         indexes = ActiveRecord::Base.connection.indexes(table)
         search_index_by_columns = indexes.select do |index|
-          index.columns == search_index_columns
+          index.columns.sort == search_index_columns.sort
         end
         search_index_by_name = indexes.select do |index|
           index.name == "#{table}_search_index"
index 59f9d3d41a52149e0d7f6a1532373df037b01a6d..37ab1d3172b2a72c488e5512776cec88dba3482f 100644 (file)
@@ -119,13 +119,118 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  test 'portable data hash with missing size hints' do
+    [[". d41d8cd98f00b204e9800998ecf8427e+0+Bar 0:0:x",
+      ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:x"],
+     [". d41d8cd98f00b204e9800998ecf8427e+Foo 0:0:x",
+      ". d41d8cd98f00b204e9800998ecf8427e 0:0:x"],
+     [". d41d8cd98f00b204e9800998ecf8427e 0:0:x",
+      ". d41d8cd98f00b204e9800998ecf8427e 0:0:x"],
+    ].each do |unportable, portable|
+      c = Collection.new(manifest_text: unportable)
+      assert c.valid?
+      assert_equal(Digest::MD5.hexdigest(portable)+"+#{portable.length}",
+                   c.portable_data_hash)
+    end
+  end
+
   [0, 2, 4, nil].each do |ask|
-    test "replication_desired reports #{ask or 2} if redundancy is #{ask}" do
+    test "set replication_desired to #{ask.inspect}" do
+      Rails.configuration.default_collection_replication = 2
       act_as_user users(:active) do
-        c = collections(:collection_owned_by_active)
-        c.update_attributes redundancy: ask
-        assert_equal (ask or 2), c.replication_desired
+        c = collections(:replication_undesired_unconfirmed)
+        c.update_attributes replication_desired: ask
+        assert_equal ask, c.replication_desired
+      end
+    end
+  end
+
+  test "replication_confirmed* can be set by admin user" do
+    c = collections(:replication_desired_2_unconfirmed)
+    act_as_user users(:admin) do
+      assert c.update_attributes(replication_confirmed: 2,
+                                 replication_confirmed_at: Time.now)
+    end
+  end
+
+  test "replication_confirmed* cannot be set by non-admin user" do
+    act_as_user users(:active) do
+      c = collections(:replication_desired_2_unconfirmed)
+      # Cannot set just one at a time.
+      assert_raise ArvadosModel::PermissionDeniedError do
+        c.update_attributes replication_confirmed: 1
+      end
+      assert_raise ArvadosModel::PermissionDeniedError do
+        c.update_attributes replication_confirmed_at: Time.now
+      end
+      # Cannot set both at once, either.
+      assert_raise ArvadosModel::PermissionDeniedError do
+        c.update_attributes(replication_confirmed: 1,
+                            replication_confirmed_at: Time.now)
+      end
+    end
+  end
+
+  test "replication_confirmed* can be cleared (but only together) by non-admin user" do
+    act_as_user users(:active) do
+      c = collections(:replication_desired_2_confirmed_2)
+      # Cannot clear just one at a time.
+      assert_raise ArvadosModel::PermissionDeniedError do
+        c.update_attributes replication_confirmed: nil
       end
+      c.reload
+      assert_raise ArvadosModel::PermissionDeniedError do
+        c.update_attributes replication_confirmed_at: nil
+      end
+      # Can clear both at once.
+      c.reload
+      assert c.update_attributes(replication_confirmed: nil,
+                                 replication_confirmed_at: nil)
+    end
+  end
+
+  test "clear replication_confirmed* when introducing a new block in manifest" do
+    c = collections(:replication_desired_2_confirmed_2)
+    act_as_user users(:active) do
+      assert c.update_attributes(manifest_text: collections(:user_agreement).signed_manifest_text)
+      assert_nil c.replication_confirmed
+      assert_nil c.replication_confirmed_at
+    end
+  end
+
+  test "don't clear replication_confirmed* when just renaming a file" do
+    c = collections(:replication_desired_2_confirmed_2)
+    act_as_user users(:active) do
+      new_manifest = c.signed_manifest_text.sub(':bar', ':foo')
+      assert c.update_attributes(manifest_text: new_manifest)
+      assert_equal 2, c.replication_confirmed
+      assert_not_nil c.replication_confirmed_at
+    end
+  end
+
+  test "don't clear replication_confirmed* when just deleting a data block" do
+    c = collections(:replication_desired_2_confirmed_2)
+    act_as_user users(:active) do
+      new_manifest = c.signed_manifest_text
+      new_manifest.sub!(/ \S+:bar/, '')
+      new_manifest.sub!(/ acbd\S+/, '')
+
+      # Confirm that we did just remove a block from the manifest (if
+      # not, this test would pass without testing the relevant case):
+      assert_operator new_manifest.length+40, :<, c.signed_manifest_text.length
+
+      assert c.update_attributes(manifest_text: new_manifest)
+      assert_equal 2, c.replication_confirmed
+      assert_not_nil c.replication_confirmed_at
+    end
+  end
+
+  test "create collection with properties" do
+    act_as_system_user do
+      c = Collection.create(manifest_text: ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n",
+                            properties: {'property_1' => 'value_1'})
+      assert c.valid?
+      assert_equal 'value_1', c.properties['property_1']
     end
   end
 end
index d2c95fc0b696a4b4ec8cf56c1898f8fff9c4deea..71c4ee5a2c4b713aba669a51605f01736b02bfe0 100644 (file)
@@ -65,7 +65,9 @@ class SafeApi(object):
 
 
 def convertTime(t):
-    '''Parse Arvados timestamp to unix time.'''
+    """Parse Arvados timestamp to unix time."""
+    if not t:
+        return 0
     try:
         return calendar.timegm(time.strptime(t, "%Y-%m-%dT%H:%M:%SZ"))
     except (TypeError, ValueError):
@@ -301,8 +303,10 @@ class CollectionDirectory(Directory):
         self.collection_object = None
         if isinstance(collection, dict):
             self.collection_locator = collection['uuid']
+            self._mtime = convertTime(collection.get('modified_at'))
         else:
             self.collection_locator = collection
+            self._mtime = 0
 
     def same(self, i):
         return i['uuid'] == self.collection_locator or i['portable_data_hash'] == self.collection_locator
@@ -317,6 +321,8 @@ class CollectionDirectory(Directory):
     def new_collection(self, new_collection_object, coll_reader):
         self.collection_object = new_collection_object
 
+        self._mtime = convertTime(self.collection_object.get('modified_at'))
+
         if self.collection_object_file is not None:
             self.collection_object_file.update(self.collection_object)
 
@@ -390,10 +396,6 @@ class CollectionDirectory(Directory):
         else:
             return super(CollectionDirectory, self).__contains__(k)
 
-    def mtime(self):
-        self.checkupdate()
-        return convertTime(self.collection_object["modified_at"]) if self.collection_object is not None and 'modified_at' in self.collection_object else 0
-
 
 class MagicDirectory(Directory):
     '''A special directory that logically contains the set of all extant keep