[ARVADOS] updated: 876c8e2ada7a9b8d1baf6fc11bd579e6bfad7789

[git at public.curoverse.com]

Summary of changes:
 services/api/app/models/arvados_model.rb           |   19 ++++++++++++
 services/api/app/models/collection.rb              |   22 ++++++++++++--
 services/api/app/models/job.rb                     |    4 ++
 .../functional/arvados/v1/jobs_controller_test.rb  |   32 ++++++++++++++++++++
 4 files changed, 74 insertions(+), 3 deletions(-)

       via  876c8e2ada7a9b8d1baf6fc11bd579e6bfad7789 (commit)
      from  c41beef3142e9c3e15385b2ad76c2a0088ca6a20 (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 876c8e2ada7a9b8d1baf6fc11bd579e6bfad7789
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Feb 13 12:49:21 2014 -0800

    Normalize collection hashes provided by clients as attribute values
    before storing in the database.
    
    closes #2088
    refs #1881

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 4f2aa72..8ee14b7 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -14,6 +14,7 @@ class ArvadosModel < ActiveRecord::Base
   before_create :update_modified_by_fields
   before_update :maybe_update_modified_by_fields
   validate :ensure_serialized_attribute_type
+  validate :normalize_collection_uuids
 
   has_many :permissions, :foreign_key => :head_uuid, :class_name => 'Link', :primary_key => :uuid, :conditions => "link_class = 'permission'"
 
@@ -157,6 +158,24 @@ class ArvadosModel < ActiveRecord::Base
     end
   end
 
+  def foreign_key_attributes
+    attributes.keys.select { |a| a.match /_uuid$/ }
+  end
+
+  def normalize_collection_uuids
+    foreign_key_attributes.each do |attr|
+      attr_value = send attr
+      if attr_value.is_a? String and
+          attr_value.match /^[0-9a-f]{32,}(\+[@\w]+)*$/
+        begin
+          send "#{attr}=", Collection.normalize_uuid(attr_value)
+        rescue
+          # TODO: abort instead of silently accepting unnormalizable value?
+        end
+      end
+    end
+  end
+
   def self.resource_class_for_uuid(uuid)
     if uuid.is_a? ArvadosModel
       return uuid.class
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 03e5e4e..ef8a909 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -33,14 +33,14 @@ class Collection < ArvadosModel
         self.uuid.gsub! /$/, '+' + self.manifest_text.length.to_s
         true
       else
-        errors.add :uuid, 'uuid does not match checksum of manifest_text'
+        errors.add :uuid, 'does not match checksum of manifest_text'
         false
       end
     elsif self.manifest_text
-      errors.add :uuid, 'checksum for manifest_text not supplied in uuid'
+      errors.add :uuid, 'not supplied (must match checksum of manifest_text)'
       false
     else
-      errors.add :manifest_text, 'manifest_text not supplied'
+      errors.add :manifest_text, 'not supplied'
       false
     end
   end
@@ -100,4 +100,20 @@ class Collection < ArvadosModel
       end
     end
   end
+
+  def self.normalize_uuid uuid
+    hash_part = nil
+    size_part = nil
+    uuid.split('+').each do |token|
+      if token.match /^[0-9a-f]{32,}$/
+        raise "uuid #{uuid} has multiple hash parts" if hash_part
+        hash_part = token
+      elsif token.match /^\d+$/
+        raise "uuid #{uuid} has multiple size parts" if size_part
+        size_part = token
+      end
+    end
+    raise "uuid #{uuid} has no hash part" if !hash_part
+    [hash_part, size_part].compact.join '+'
+  end
 end
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index f32f001..9c8f724 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -58,6 +58,10 @@ class Job < ArvadosModel
 
   protected
 
+  def foreign_key_attributes
+    super + %w(output log)
+  end
+
   def ensure_script_version_is_commit
     if self.is_locked_by_uuid and self.started_at
       # Apparently client has already decided to go for it. This is
diff --git a/services/api/test/functional/arvados/v1/jobs_controller_test.rb b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
index 912a901..95bbf52 100644
--- a/services/api/test/functional/arvados/v1/jobs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/jobs_controller_test.rb
@@ -13,6 +13,38 @@ class Arvados::V1::JobsControllerTest < ActionController::TestCase
     assert_not_nil assigns(:object)
     new_job = JSON.parse(@response.body)
     assert_not_nil new_job['uuid']
+    assert_not_nil new_job['script_version'].match(/^[0-9a-f]{40}$/)
+  end
+
+  test "normalize output and log uuids when creating job" do
+    authorize_with :active
+    post :create, job: {
+      script: "hash",
+      script_version: "master",
+      script_parameters: {},
+      started_at: Time.now,
+      finished_at: Time.now,
+      running: false,
+      success: true,
+      output: 'd41d8cd98f00b204e9800998ecf8427e+0+K at xyzzy',
+      log: 'd41d8cd98f00b204e9800998ecf8427e+0+K at xyzzy'
+    }
+    assert_response :success
+    assert_not_nil assigns(:object)
+    new_job = JSON.parse(@response.body)
+    assert_equal 'd41d8cd98f00b204e9800998ecf8427e+0', new_job['log']
+    assert_equal 'd41d8cd98f00b204e9800998ecf8427e+0', new_job['output']
+    version = new_job['script_version']
+
+    # Make sure version doesn't get mangled by normalize
+    assert_not_nil version.match(/^[0-9a-f]{40}$/)
+    put :update, {
+      id: new_job['uuid'],
+      job: {
+        log: new_job['log']
+      }
+    }
+    assert_equal version, JSON.parse(@response.body)['script_version']
   end
 
   test "cancel a running job" do

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


hooks/post-receive
--