refs #4926 Merge branch '4926-getting-started'

[arvados.git] / services / api / app / models / job.rb

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 6e01de9213b1d8329451602ff3ea8fdc996e5679..0444528b6bda671a4327ef39fff1918fabebf9d8 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -15,6 +15,7 @@ class Job < ArvadosModel
   validate :find_docker_image_locator
   validate :validate_status
   validate :validate_state_change
+  validate :ensure_no_collection_uuids_in_script_params
   before_save :update_timestamps_when_state_changes
 
   has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
@@ -171,10 +172,12 @@ class Job < ArvadosModel
                                :arvados_sdk_version) do |git_search|
       commits = Commit.find_commit_range(current_user, "arvados",
                                          nil, git_search, nil)
-      if commits.andand.any?
-        [true, commits.first]
-      else
+      if commits.nil? or commits.empty?
         [false, "#{git_search} does not resolve to a commit"]
+      elsif not runtime_constraints["docker_image"]
+        [false, "cannot be specified without a Docker image constraint"]
+      else
+        [true, commits.first]
       end
     end
   end
@@ -308,6 +311,8 @@ class Job < ArvadosModel
     end
     self.running ||= false # Default to false instead of nil.
 
+    @need_crunch_dispatch_trigger = true
+
     true
   end
 
@@ -372,4 +377,34 @@ class Job < ArvadosModel
     end
     ok
   end
+
+  def ensure_no_collection_uuids_in_script_params
+    # recursive_hash_search searches recursively through hashes and
+    # arrays in 'thing' for string fields matching regular expression
+    # 'pattern'.  Returns true if pattern is found, false otherwise.
+    def recursive_hash_search thing, pattern
+      if thing.is_a? Hash
+        thing.each do |k, v|
+          return true if recursive_hash_search v, pattern
+        end
+      elsif thing.is_a? Array
+        thing.each do |k|
+          return true if recursive_hash_search k, pattern
+        end
+      elsif thing.is_a? String
+        return true if thing.match pattern
+      end
+      false
+    end
+
+    # Fail validation if any script_parameters field includes a string containing a
+    # collection uuid pattern.
+    if self.script_parameters_changed?
+      if recursive_hash_search(self.script_parameters, Collection.uuid_regex)
+        self.errors.add :script_parameters, "must use portable_data_hash instead of collection uuid"
+        return false
+      end
+    end
+    true
+  end
 end