15133: Remove 'commits' table, move functions to CommitsHelper
authorPeter Amstutz <pamstutz@veritasgenetics.com>
Fri, 9 Aug 2019 14:42:34 +0000 (10:42 -0400)
committerPeter Amstutz <pamstutz@veritasgenetics.com>
Fri, 9 Aug 2019 15:02:44 +0000 (11:02 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

services/api/app/helpers/commits_helper.rb
services/api/app/models/commit.rb [deleted file]
services/api/app/models/job.rb
services/api/db/migrate/20190809135453_remove_commits_table.rb [new file with mode: 0644]
services/api/db/structure.sql
services/api/test/helpers/git_test_helper.rb
services/api/test/unit/commit_test.rb
services/api/test/unit/job_test.rb

index d44719f924aa31b27225d24eb5b5f534b5de4914..b1c02b2a76a1597f7caf0837cf609b2deffa1eac 100644 (file)
@@ -3,4 +3,267 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 module CommitsHelper
+
+  class GitError < RequestError
+    def http_status
+      422
+    end
+  end
+
+  def self.git_check_ref_format(e)
+    if !e or e.empty? or e[0] == '-' or e[0] == '$'
+      # definitely not valid
+      false
+    else
+      `git check-ref-format --allow-onelevel #{e.shellescape}`
+      $?.success?
+    end
+  end
+
+  # Return an array of commits (each a 40-char sha1) satisfying the
+  # given criteria.
+  #
+  # Return [] if the revisions given in minimum/maximum are invalid or
+  # don't exist in the given repository.
+  #
+  # Raise ArgumentError if the given repository is invalid, does not
+  # exist, or cannot be read for any reason. (Any transient error that
+  # prevents commit ranges from resolving must raise rather than
+  # returning an empty array.)
+  #
+  # repository can be the name of a locally hosted repository or a git
+  # URL (see git-fetch(1)). Currently http, https, and git schemes are
+  # supported.
+  def self.find_commit_range repository, minimum, maximum, exclude
+    if minimum and minimum.empty?
+      minimum = nil
+    end
+
+    if minimum and !git_check_ref_format(minimum)
+      Rails.logger.warn "find_commit_range called with invalid minimum revision: '#{minimum}'"
+      return []
+    end
+
+    if maximum and !git_check_ref_format(maximum)
+      Rails.logger.warn "find_commit_range called with invalid maximum revision: '#{maximum}'"
+      return []
+    end
+
+    if !maximum
+      maximum = "HEAD"
+    end
+
+    gitdir, is_remote = git_dir_for repository
+    fetch_remote_repository gitdir, repository if is_remote
+    ENV['GIT_DIR'] = gitdir
+
+    commits = []
+
+    # Get the commit hash for the upper bound
+    max_hash = nil
+    git_max_hash_cmd = "git rev-list --max-count=1 #{maximum.shellescape} --"
+    IO.foreach("|#{git_max_hash_cmd}") do |line|
+      max_hash = line.strip
+    end
+
+    # If not found, nothing else to do
+    if !max_hash
+      Rails.logger.warn "no refs found looking for max_hash: `GIT_DIR=#{gitdir} #{git_max_hash_cmd}` returned no output"
+      return []
+    end
+
+    # If string is invalid, nothing else to do
+    if !git_check_ref_format(max_hash)
+      Rails.logger.warn "ref returned by `GIT_DIR=#{gitdir} #{git_max_hash_cmd}` was invalid for max_hash: #{max_hash}"
+      return []
+    end
+
+    resolved_exclude = nil
+    if exclude
+      resolved_exclude = []
+      exclude.each do |e|
+        if git_check_ref_format(e)
+          IO.foreach("|git rev-list --max-count=1 #{e.shellescape} --") do |line|
+            resolved_exclude.push(line.strip)
+          end
+        else
+          Rails.logger.warn "find_commit_range called with invalid exclude invalid characters: '#{exclude}'"
+          return []
+        end
+      end
+    end
+
+    if minimum
+      # Get the commit hash for the lower bound
+      min_hash = nil
+      git_min_hash_cmd = "git rev-list --max-count=1 #{minimum.shellescape} --"
+      IO.foreach("|#{git_min_hash_cmd}") do |line|
+        min_hash = line.strip
+      end
+
+      # If not found, nothing else to do
+      if !min_hash
+        Rails.logger.warn "no refs found looking for min_hash: `GIT_DIR=#{gitdir} #{git_min_hash_cmd}` returned no output"
+        return []
+      end
+
+      # If string is invalid, nothing else to do
+      if !git_check_ref_format(min_hash)
+        Rails.logger.warn "ref returned by `GIT_DIR=#{gitdir} #{git_min_hash_cmd}` was invalid for min_hash: #{min_hash}"
+        return []
+      end
+
+      # Now find all commits between them
+      IO.foreach("|git rev-list #{min_hash.shellescape}..#{max_hash.shellescape} --") do |line|
+        hash = line.strip
+        commits.push(hash) if !resolved_exclude or !resolved_exclude.include? hash
+      end
+
+      commits.push(min_hash) if !resolved_exclude or !resolved_exclude.include? min_hash
+    else
+      commits.push(max_hash) if !resolved_exclude or !resolved_exclude.include? max_hash
+    end
+
+    commits
+  end
+
+  # Given a repository (url, or name of hosted repo) and commit sha1,
+  # copy the commit into the internal git repo (if necessary), and tag
+  # it with the given tag (typically a job UUID).
+  #
+  # The repo can be a remote url, but in this case sha1 must already
+  # be present in our local cache for that repo: e.g., sha1 was just
+  # returned by find_commit_range.
+  def self.tag_in_internal_repository repo_name, sha1, tag
+    unless git_check_ref_format tag
+      raise ArgumentError.new "invalid tag #{tag}"
+    end
+    unless /^[0-9a-f]{40}$/ =~ sha1
+      raise ArgumentError.new "invalid sha1 #{sha1}"
+    end
+    src_gitdir, _ = git_dir_for repo_name
+    unless src_gitdir
+      raise ArgumentError.new "no local repository for #{repo_name}"
+    end
+    dst_gitdir = Rails.configuration.Containers.JobsAPI.GitInternalDir
+
+    begin
+      commit_in_dst = must_git(dst_gitdir, "log -n1 --format=%H #{sha1.shellescape}^{commit}").strip
+    rescue GitError
+      commit_in_dst = false
+    end
+
+    tag_cmd = "tag --force #{tag.shellescape} #{sha1.shellescape}^{commit}"
+    if commit_in_dst == sha1
+      must_git(dst_gitdir, tag_cmd)
+    else
+      # git-fetch is faster than pack-objects|unpack-objects, but
+      # git-fetch can't fetch by sha1. So we first try to fetch a
+      # branch that has the desired commit, and if that fails (there
+      # is no such branch, or the branch we choose changes under us in
+      # race), we fall back to pack|unpack.
+      begin
+        branches = must_git(src_gitdir,
+                            "branch --contains #{sha1.shellescape}")
+        m = branches.match(/^. (\w+)\n/)
+        if !m
+          raise GitError.new "commit is not on any branch"
+        end
+        branch = m[1]
+        must_git(dst_gitdir,
+                 "fetch file://#{src_gitdir.shellescape} #{branch.shellescape}")
+        # Even if all of the above steps succeeded, we might still not
+        # have the right commit due to a race, in which case tag_cmd
+        # will fail, and we'll need to fall back to pack|unpack. So
+        # don't be tempted to condense this tag_cmd and the one in the
+        # rescue block into a single attempt.
+        must_git(dst_gitdir, tag_cmd)
+      rescue GitError
+        must_pipe("echo #{sha1.shellescape}",
+                  "git --git-dir #{src_gitdir.shellescape} pack-objects -q --revs --stdout",
+                  "git --git-dir #{dst_gitdir.shellescape} unpack-objects -q")
+        must_git(dst_gitdir, tag_cmd)
+      end
+    end
+  end
+
+  protected
+
+  def self.remote_url? repo_name
+    /^(https?|git):\/\// =~ repo_name
+  end
+
+  # Return [local_git_dir, is_remote]. If is_remote, caller must use
+  # fetch_remote_repository to ensure content is up-to-date.
+  #
+  # Raises an exception if the latest content could not be fetched for
+  # any reason.
+  def self.git_dir_for repo_name
+    if remote_url? repo_name
+      return [cache_dir_for(repo_name), true]
+    end
+    repos = Repository.readable_by(current_user).where(name: repo_name)
+    if repos.count == 0
+      raise ArgumentError.new "Repository not found: '#{repo_name}'"
+    elsif repos.count > 1
+      Rails.logger.error "Multiple repositories with name=='#{repo_name}'!"
+      raise ArgumentError.new "Name conflict"
+    else
+      return [repos.first.server_path, false]
+    end
+  end
+
+  def self.cache_dir_for git_url
+    File.join(cache_dir_base, Digest::SHA1.hexdigest(git_url) + ".git").to_s
+  end
+
+  def self.cache_dir_base
+    Rails.root.join 'tmp', 'git-cache'
+  end
+
+  def self.fetch_remote_repository gitdir, git_url
+    # Caller decides which protocols are worth using. This is just a
+    # safety check to ensure we never use urls like "--flag" or wander
+    # into git's hardlink features by using bare "/path/foo" instead
+    # of "file:///path/foo".
+    unless /^[a-z]+:\/\// =~ git_url
+      raise ArgumentError.new "invalid git url #{git_url}"
+    end
+    begin
+      must_git gitdir, "branch"
+    rescue GitError => e
+      raise unless /Not a git repository/i =~ e.to_s
+      # OK, this just means we need to create a blank cache repository
+      # before fetching.
+      FileUtils.mkdir_p gitdir
+      must_git gitdir, "init"
+    end
+    must_git(gitdir,
+             "fetch --no-progress --tags --prune --force --update-head-ok #{git_url.shellescape} 'refs/heads/*:refs/heads/*'")
+  end
+
+  def self.must_git gitdir, *cmds
+    # Clear token in case a git helper tries to use it as a password.
+    orig_token = ENV['ARVADOS_API_TOKEN']
+    ENV['ARVADOS_API_TOKEN'] = ''
+    last_output = ''
+    begin
+      git = "git --git-dir #{gitdir.shellescape}"
+      cmds.each do |cmd|
+        last_output = must_pipe git+" "+cmd
+      end
+    ensure
+      ENV['ARVADOS_API_TOKEN'] = orig_token
+    end
+    return last_output
+  end
+
+  def self.must_pipe *cmds
+    cmd = cmds.join(" 2>&1 |") + " 2>&1"
+    out = IO.read("| </dev/null #{cmd}")
+    if not $?.success?
+      raise GitError.new "#{cmd}: #{$?}: #{out}"
+    end
+    return out
+  end
 end
diff --git a/services/api/app/models/commit.rb b/services/api/app/models/commit.rb
deleted file mode 100644 (file)
index 2f7e9cd..0000000
+++ /dev/null
@@ -1,272 +0,0 @@
-# Copyright (C) The Arvados Authors. All rights reserved.
-#
-# SPDX-License-Identifier: AGPL-3.0
-
-require 'request_error'
-
-class Commit < ActiveRecord::Base
-  extend CurrentApiClient
-
-  class GitError < RequestError
-    def http_status
-      422
-    end
-  end
-
-  def self.git_check_ref_format(e)
-    if !e or e.empty? or e[0] == '-' or e[0] == '$'
-      # definitely not valid
-      false
-    else
-      `git check-ref-format --allow-onelevel #{e.shellescape}`
-      $?.success?
-    end
-  end
-
-  # Return an array of commits (each a 40-char sha1) satisfying the
-  # given criteria.
-  #
-  # Return [] if the revisions given in minimum/maximum are invalid or
-  # don't exist in the given repository.
-  #
-  # Raise ArgumentError if the given repository is invalid, does not
-  # exist, or cannot be read for any reason. (Any transient error that
-  # prevents commit ranges from resolving must raise rather than
-  # returning an empty array.)
-  #
-  # repository can be the name of a locally hosted repository or a git
-  # URL (see git-fetch(1)). Currently http, https, and git schemes are
-  # supported.
-  def self.find_commit_range repository, minimum, maximum, exclude
-    if minimum and minimum.empty?
-      minimum = nil
-    end
-
-    if minimum and !git_check_ref_format(minimum)
-      logger.warn "find_commit_range called with invalid minimum revision: '#{minimum}'"
-      return []
-    end
-
-    if maximum and !git_check_ref_format(maximum)
-      logger.warn "find_commit_range called with invalid maximum revision: '#{maximum}'"
-      return []
-    end
-
-    if !maximum
-      maximum = "HEAD"
-    end
-
-    gitdir, is_remote = git_dir_for repository
-    fetch_remote_repository gitdir, repository if is_remote
-    ENV['GIT_DIR'] = gitdir
-
-    commits = []
-
-    # Get the commit hash for the upper bound
-    max_hash = nil
-    git_max_hash_cmd = "git rev-list --max-count=1 #{maximum.shellescape} --"
-    IO.foreach("|#{git_max_hash_cmd}") do |line|
-      max_hash = line.strip
-    end
-
-    # If not found, nothing else to do
-    if !max_hash
-      logger.warn "no refs found looking for max_hash: `GIT_DIR=#{gitdir} #{git_max_hash_cmd}` returned no output"
-      return []
-    end
-
-    # If string is invalid, nothing else to do
-    if !git_check_ref_format(max_hash)
-      logger.warn "ref returned by `GIT_DIR=#{gitdir} #{git_max_hash_cmd}` was invalid for max_hash: #{max_hash}"
-      return []
-    end
-
-    resolved_exclude = nil
-    if exclude
-      resolved_exclude = []
-      exclude.each do |e|
-        if git_check_ref_format(e)
-          IO.foreach("|git rev-list --max-count=1 #{e.shellescape} --") do |line|
-            resolved_exclude.push(line.strip)
-          end
-        else
-          logger.warn "find_commit_range called with invalid exclude invalid characters: '#{exclude}'"
-          return []
-        end
-      end
-    end
-
-    if minimum
-      # Get the commit hash for the lower bound
-      min_hash = nil
-      git_min_hash_cmd = "git rev-list --max-count=1 #{minimum.shellescape} --"
-      IO.foreach("|#{git_min_hash_cmd}") do |line|
-        min_hash = line.strip
-      end
-
-      # If not found, nothing else to do
-      if !min_hash
-        logger.warn "no refs found looking for min_hash: `GIT_DIR=#{gitdir} #{git_min_hash_cmd}` returned no output"
-        return []
-      end
-
-      # If string is invalid, nothing else to do
-      if !git_check_ref_format(min_hash)
-        logger.warn "ref returned by `GIT_DIR=#{gitdir} #{git_min_hash_cmd}` was invalid for min_hash: #{min_hash}"
-        return []
-      end
-
-      # Now find all commits between them
-      IO.foreach("|git rev-list #{min_hash.shellescape}..#{max_hash.shellescape} --") do |line|
-        hash = line.strip
-        commits.push(hash) if !resolved_exclude or !resolved_exclude.include? hash
-      end
-
-      commits.push(min_hash) if !resolved_exclude or !resolved_exclude.include? min_hash
-    else
-      commits.push(max_hash) if !resolved_exclude or !resolved_exclude.include? max_hash
-    end
-
-    commits
-  end
-
-  # Given a repository (url, or name of hosted repo) and commit sha1,
-  # copy the commit into the internal git repo (if necessary), and tag
-  # it with the given tag (typically a job UUID).
-  #
-  # The repo can be a remote url, but in this case sha1 must already
-  # be present in our local cache for that repo: e.g., sha1 was just
-  # returned by find_commit_range.
-  def self.tag_in_internal_repository repo_name, sha1, tag
-    unless git_check_ref_format tag
-      raise ArgumentError.new "invalid tag #{tag}"
-    end
-    unless /^[0-9a-f]{40}$/ =~ sha1
-      raise ArgumentError.new "invalid sha1 #{sha1}"
-    end
-    src_gitdir, _ = git_dir_for repo_name
-    unless src_gitdir
-      raise ArgumentError.new "no local repository for #{repo_name}"
-    end
-    dst_gitdir = Rails.configuration.Containers.JobsAPI.GitInternalDir
-
-    begin
-      commit_in_dst = must_git(dst_gitdir, "log -n1 --format=%H #{sha1.shellescape}^{commit}").strip
-    rescue GitError
-      commit_in_dst = false
-    end
-
-    tag_cmd = "tag --force #{tag.shellescape} #{sha1.shellescape}^{commit}"
-    if commit_in_dst == sha1
-      must_git(dst_gitdir, tag_cmd)
-    else
-      # git-fetch is faster than pack-objects|unpack-objects, but
-      # git-fetch can't fetch by sha1. So we first try to fetch a
-      # branch that has the desired commit, and if that fails (there
-      # is no such branch, or the branch we choose changes under us in
-      # race), we fall back to pack|unpack.
-      begin
-        branches = must_git(src_gitdir,
-                            "branch --contains #{sha1.shellescape}")
-        m = branches.match(/^. (\w+)\n/)
-        if !m
-          raise GitError.new "commit is not on any branch"
-        end
-        branch = m[1]
-        must_git(dst_gitdir,
-                 "fetch file://#{src_gitdir.shellescape} #{branch.shellescape}")
-        # Even if all of the above steps succeeded, we might still not
-        # have the right commit due to a race, in which case tag_cmd
-        # will fail, and we'll need to fall back to pack|unpack. So
-        # don't be tempted to condense this tag_cmd and the one in the
-        # rescue block into a single attempt.
-        must_git(dst_gitdir, tag_cmd)
-      rescue GitError
-        must_pipe("echo #{sha1.shellescape}",
-                  "git --git-dir #{src_gitdir.shellescape} pack-objects -q --revs --stdout",
-                  "git --git-dir #{dst_gitdir.shellescape} unpack-objects -q")
-        must_git(dst_gitdir, tag_cmd)
-      end
-    end
-  end
-
-  protected
-
-  def self.remote_url? repo_name
-    /^(https?|git):\/\// =~ repo_name
-  end
-
-  # Return [local_git_dir, is_remote]. If is_remote, caller must use
-  # fetch_remote_repository to ensure content is up-to-date.
-  #
-  # Raises an exception if the latest content could not be fetched for
-  # any reason.
-  def self.git_dir_for repo_name
-    if remote_url? repo_name
-      return [cache_dir_for(repo_name), true]
-    end
-    repos = Repository.readable_by(current_user).where(name: repo_name)
-    if repos.count == 0
-      raise ArgumentError.new "Repository not found: '#{repo_name}'"
-    elsif repos.count > 1
-      logger.error "Multiple repositories with name=='#{repo_name}'!"
-      raise ArgumentError.new "Name conflict"
-    else
-      return [repos.first.server_path, false]
-    end
-  end
-
-  def self.cache_dir_for git_url
-    File.join(cache_dir_base, Digest::SHA1.hexdigest(git_url) + ".git").to_s
-  end
-
-  def self.cache_dir_base
-    Rails.root.join 'tmp', 'git-cache'
-  end
-
-  def self.fetch_remote_repository gitdir, git_url
-    # Caller decides which protocols are worth using. This is just a
-    # safety check to ensure we never use urls like "--flag" or wander
-    # into git's hardlink features by using bare "/path/foo" instead
-    # of "file:///path/foo".
-    unless /^[a-z]+:\/\// =~ git_url
-      raise ArgumentError.new "invalid git url #{git_url}"
-    end
-    begin
-      must_git gitdir, "branch"
-    rescue GitError => e
-      raise unless /Not a git repository/i =~ e.to_s
-      # OK, this just means we need to create a blank cache repository
-      # before fetching.
-      FileUtils.mkdir_p gitdir
-      must_git gitdir, "init"
-    end
-    must_git(gitdir,
-             "fetch --no-progress --tags --prune --force --update-head-ok #{git_url.shellescape} 'refs/heads/*:refs/heads/*'")
-  end
-
-  def self.must_git gitdir, *cmds
-    # Clear token in case a git helper tries to use it as a password.
-    orig_token = ENV['ARVADOS_API_TOKEN']
-    ENV['ARVADOS_API_TOKEN'] = ''
-    last_output = ''
-    begin
-      git = "git --git-dir #{gitdir.shellescape}"
-      cmds.each do |cmd|
-        last_output = must_pipe git+" "+cmd
-      end
-    ensure
-      ENV['ARVADOS_API_TOKEN'] = orig_token
-    end
-    return last_output
-  end
-
-  def self.must_pipe *cmds
-    cmd = cmds.join(" 2>&1 |") + " 2>&1"
-    out = IO.read("| </dev/null #{cmd}")
-    if not $?.success?
-      raise GitError.new "#{cmd}: #{$?}: #{out}"
-    end
-    return out
-  end
-end
index c13d48bd860f6f1393aee02be848d715b06b8192..30a0294a3483f44b5f97961c53dd75482828cde5 100644 (file)
@@ -1,6 +1,26 @@
 # Copyright (C) The Arvados Authors. All rights reserved.
 #
 # SPDX-License-Identifier: AGPL-3.0
+#
+#
+# Legacy jobs API aka crunch v1
+#
+# This is superceded by containers / container_requests (aka crunch v2)
+#
+# Arvados installations since the beginning of 2018 should have never
+# used jobs, and are unaffected by this change.
+#
+# So that older Arvados sites don't lose access to legacy records, the
+# API has been converted to read-only.  Creating and updating jobs
+# (and related types job_task, pipeline_template and
+# pipeline_instance) is disabled and much of the business logic
+# related has been removed, along with the crunch-dispatch.rb and
+# various other code specific to the jobs API.
+#
+# If you need to resurrect any of this code, here is the last commit
+# on master before the branch removing jobs API support:
+#
+# Wed Aug 7 14:49:38 2019 -0400 07d92519438a592d531f2c7558cd51788da262ca
 
 require 'log_reuse_info'
 require 'safe_json'
@@ -189,7 +209,7 @@ class Job < ArvadosModel
       else
         raise ArgumentError.new("unknown attribute for git filter: #{attr}")
       end
-      revisions = Commit.find_commit_range(filter["repository"],
+      revisions = CommitsHelper::find_commit_range(filter["repository"],
                                            filter["min_version"],
                                            filter["max_version"],
                                            filter["exclude_versions"])
@@ -209,7 +229,7 @@ class Job < ArvadosModel
     # Add a filter to @filters for `attr_name` = the latest commit available
     # in `repo_name` at `refspec`.  No filter is added if refspec can't be
     # resolved.
-    commits = Commit.find_commit_range(repo_name, nil, refspec, nil)
+    commits = CommitsHelper::find_commit_range(repo_name, nil, refspec, nil)
     if commit_hash = commits.first
       [[attr_name, "=", commit_hash]]
     else
@@ -218,36 +238,7 @@ class Job < ArvadosModel
   end
 
   def cancel(cascade: false, need_transaction: true)
-    if need_transaction
-      ActiveRecord::Base.transaction do
-        cancel(cascade: cascade, need_transaction: false)
-      end
-      return
-    end
-
-    if self.state.in?([Queued, Running])
-      self.state = Cancelled
-      self.save!
-    elsif self.state != Cancelled
-      raise InvalidStateTransitionError
-    end
-
-    return if !cascade
-
-    # cancel all children; they could be jobs or pipeline instances
-    children = self.components.andand.collect{|_, u| u}.compact
-
-    return if children.empty?
-
-    # cancel any child jobs
-    Job.where(uuid: children, state: [Queued, Running]).each do |job|
-      job.cancel(cascade: cascade, need_transaction: false)
-    end
-
-    # cancel any child pipelines
-    PipelineInstance.where(uuid: children, state: [PipelineInstance::RunningOnServer, PipelineInstance::RunningOnClient]).each do |pi|
-      pi.cancel(cascade: cascade, need_transaction: false)
-    end
+    raise "No longer supported"
   end
 
   protected
@@ -283,7 +274,7 @@ class Job < ArvadosModel
       return true
     end
     if new_record? or repository_changed? or script_version_changed?
-      sha1 = Commit.find_commit_range(repository,
+      sha1 = CommitsHelper::find_commit_range(repository,
                                       nil, script_version, nil).first
       if not sha1
         errors.add :script_version, "#{script_version} does not resolve to a commit"
@@ -308,7 +299,7 @@ class Job < ArvadosModel
       uuid_was = uuid
       begin
         assign_uuid
-        Commit.tag_in_internal_repository repository, script_version, uuid
+        CommitsHelper::tag_in_internal_repository repository, script_version, uuid
       rescue
         self.uuid = uuid_was
         raise
@@ -343,7 +334,7 @@ class Job < ArvadosModel
   def find_arvados_sdk_version
     resolve_runtime_constraint("arvados_sdk_version",
                                :arvados_sdk_version) do |git_search|
-      commits = Commit.find_commit_range("arvados",
+      commits = CommitsHelper::find_commit_range("arvados",
                                          nil, git_search, nil)
       if commits.empty?
         [false, "#{git_search} does not resolve to a commit"]
diff --git a/services/api/db/migrate/20190809135453_remove_commits_table.rb b/services/api/db/migrate/20190809135453_remove_commits_table.rb
new file mode 100644 (file)
index 0000000..a3f450e
--- /dev/null
@@ -0,0 +1,9 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class RemoveCommitsTable < ActiveRecord::Migration[5.0]
+  def change
+        drop_table :commits
+  end
+end
index 080990a35a5fd33f05dea62da7a3bc5068e520bc..889ffa7486f96fda469545178ad51c4df702a117 100644 (file)
@@ -227,39 +227,6 @@ CREATE SEQUENCE public.collections_id_seq
 ALTER SEQUENCE public.collections_id_seq OWNED BY public.collections.id;
 
 
---
--- Name: commits; Type: TABLE; Schema: public; Owner: -
---
-
-CREATE TABLE public.commits (
-    id integer NOT NULL,
-    repository_name character varying(255),
-    sha1 character varying(255),
-    message character varying(255),
-    created_at timestamp without time zone NOT NULL,
-    updated_at timestamp without time zone NOT NULL
-);
-
-
---
--- Name: commits_id_seq; Type: SEQUENCE; Schema: public; Owner: -
---
-
-CREATE SEQUENCE public.commits_id_seq
-    START WITH 1
-    INCREMENT BY 1
-    NO MINVALUE
-    NO MAXVALUE
-    CACHE 1;
-
-
---
--- Name: commits_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
---
-
-ALTER SEQUENCE public.commits_id_seq OWNED BY public.commits.id;
-
-
 --
 -- Name: container_requests; Type: TABLE; Schema: public; Owner: -
 --
@@ -1233,13 +1200,6 @@ ALTER TABLE ONLY public.authorized_keys ALTER COLUMN id SET DEFAULT nextval('pub
 ALTER TABLE ONLY public.collections ALTER COLUMN id SET DEFAULT nextval('public.collections_id_seq'::regclass);
 
 
---
--- Name: commits id; Type: DEFAULT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public.commits ALTER COLUMN id SET DEFAULT nextval('public.commits_id_seq'::regclass);
-
-
 --
 -- Name: container_requests id; Type: DEFAULT; Schema: public; Owner: -
 --
@@ -1420,14 +1380,6 @@ ALTER TABLE ONLY public.collections
     ADD CONSTRAINT collections_pkey PRIMARY KEY (id);
 
 
---
--- Name: commits commits_pkey; Type: CONSTRAINT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public.commits
-    ADD CONSTRAINT commits_pkey PRIMARY KEY (id);
-
-
 --
 -- Name: container_requests container_requests_pkey; Type: CONSTRAINT; Schema: public; Owner: -
 --
@@ -1868,13 +1820,6 @@ CREATE INDEX index_collections_on_trash_at ON public.collections USING btree (tr
 CREATE UNIQUE INDEX index_collections_on_uuid ON public.collections USING btree (uuid);
 
 
---
--- Name: index_commits_on_repository_name_and_sha1; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE UNIQUE INDEX index_commits_on_repository_name_and_sha1 ON public.commits USING btree (repository_name, sha1);
-
-
 --
 -- Name: index_container_requests_on_container_uuid; Type: INDEX; Schema: public; Owner: -
 --
@@ -3070,6 +3015,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20190322174136'),
 ('20190422144631'),
 ('20190523180148'),
-('20190808145904');
+('20190808145904'),
+('20190809135453');
 
 
index 03189bdfeae50c808a48069338824772513ae541..59debc57605c397435c48c49b70255a5359f873f 100644 (file)
@@ -30,7 +30,7 @@ module GitTestHelper
     end
 
     base.teardown do
-      FileUtils.remove_entry Commit.cache_dir_base, true
+      FileUtils.remove_entry CommitsHelper.cache_dir_base, true
       FileUtils.mkdir_p @tmpdir
       system("tar", "-xC", @tmpdir.to_s, "-f", "test/test.git.tar")
     end
@@ -48,10 +48,10 @@ module GitTestHelper
     if fakeurl.is_a? Symbol
       fakeurl = 'file://' + repositories(fakeurl).server_path
     end
-    Commit.expects(:fetch_remote_repository).once.with do |gitdir, giturl|
+    CommitsHelper.expects(:fetch_remote_repository).once.with do |gitdir, giturl|
       if giturl == url
-        Commit.unstub(:fetch_remote_repository)
-        Commit.fetch_remote_repository gitdir, fakeurl
+        CommitsHelper.unstub(:fetch_remote_repository)
+        CommitsHelper.fetch_remote_repository gitdir, fakeurl
         true
       end
     end
index c5d72c3bfea7ef21cc93a5a8d88db4f564008601..1c772de0470771f8768b7b4c24f2df9c598361a4 100644 (file)
@@ -22,7 +22,7 @@ class CommitTest < ActiveSupport::TestCase
   test 'find_commit_range does not bypass permissions' do
     authorize_with :inactive
     assert_raises ArgumentError do
-      Commit.find_commit_range 'foo', nil, 'master', []
+      CommitsHelper::find_commit_range 'foo', nil, 'master', []
     end
   end
 
@@ -41,9 +41,9 @@ class CommitTest < ActiveSupport::TestCase
   ].each do |url|
     test "find_commit_range uses fetch_remote_repository to get #{url}" do
       fake_gitdir = repositories(:foo).server_path
-      Commit.expects(:cache_dir_for).once.with(url).returns fake_gitdir
-      Commit.expects(:fetch_remote_repository).once.with(fake_gitdir, url).returns true
-      c = Commit.find_commit_range url, nil, 'master', []
+      CommitsHelper::expects(:cache_dir_for).once.with(url).returns fake_gitdir
+      CommitsHelper::expects(:fetch_remote_repository).once.with(fake_gitdir, url).returns true
+      c = CommitsHelper::find_commit_range url, nil, 'master', []
       refute_empty c
     end
   end
@@ -57,9 +57,9 @@ class CommitTest < ActiveSupport::TestCase
    'github.com/curoverse/arvados.git',
   ].each do |url|
     test "find_commit_range skips fetch_remote_repository for #{url}" do
-      Commit.expects(:fetch_remote_repository).never
+      CommitsHelper::expects(:fetch_remote_repository).never
       assert_raises ArgumentError do
-        Commit.find_commit_range url, nil, 'master', []
+        CommitsHelper::find_commit_range url, nil, 'master', []
       end
     end
   end
@@ -67,12 +67,12 @@ class CommitTest < ActiveSupport::TestCase
   test 'fetch_remote_repository does not leak commits across repositories' do
     url = "http://localhost:1/fake/fake.git"
     fetch_remote_from_local_repo url, :foo
-    c = Commit.find_commit_range url, nil, 'master', []
+    c = CommitsHelper::find_commit_range url, nil, 'master', []
     assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57'], c
 
     url = "http://localhost:2/fake/fake.git"
     fetch_remote_from_local_repo url, 'file://' + File.expand_path('../../.git', Rails.root)
-    c = Commit.find_commit_range url, nil, '077ba2ad3ea24a929091a9e6ce545c93199b8e57', []
+    c = CommitsHelper::find_commit_range url, nil, '077ba2ad3ea24a929091a9e6ce545c93199b8e57', []
     assert_equal [], c
   end
 
@@ -82,7 +82,7 @@ class CommitTest < ActiveSupport::TestCase
     IO.read("|#{gitint} tag -d testtag 2>/dev/null") # "no such tag", fine
     assert_match(/^fatal: /, IO.read("|#{gitint} show testtag 2>&1"))
     refute $?.success?
-    Commit.tag_in_internal_repository 'active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', 'testtag'
+    CommitsHelper::tag_in_internal_repository 'active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', 'testtag'
     assert_match(/^commit 31ce37f/, IO.read("|#{gitint} show testtag"))
     assert $?.success?
   end
@@ -106,7 +106,7 @@ class CommitTest < ActiveSupport::TestCase
       must_pipe("git rm bar")
       must_pipe("git -c user.email=x@x -c user.name=X commit -m -")
     end
-    Commit.tag_in_internal_repository 'active/foo', sha1, tag
+    CommitsHelper::tag_in_internal_repository 'active/foo', sha1, tag
     gitint = "git --git-dir #{Rails.configuration.Containers.JobsAPI.GitInternalDir.shellescape}"
     assert_match(/^commit /, IO.read("|#{gitint} show #{tag.shellescape}"))
     assert $?.success?
@@ -122,7 +122,7 @@ class CommitTest < ActiveSupport::TestCase
       sha1 = must_pipe("git log -n1 --format=%H").strip
       must_pipe("git reset --hard HEAD^")
     end
-    Commit.tag_in_internal_repository 'active/foo', sha1, tag
+    CommitsHelper::tag_in_internal_repository 'active/foo', sha1, tag
     gitint = "git --git-dir #{Rails.configuration.Containers.JobsAPI.GitInternalDir.shellescape}"
     assert_match(/^commit /, IO.read("|#{gitint} show #{tag.shellescape}"))
     assert $?.success?
@@ -141,50 +141,50 @@ class CommitTest < ActiveSupport::TestCase
 
   test "find_commit_range min_version prefers commits over branch names" do
     assert_equal([COMMIT_BRANCH_NAME],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           COMMIT_BRANCH_NAME, nil, nil))
   end
 
   test "find_commit_range max_version prefers commits over branch names" do
     assert_equal([COMMIT_BRANCH_NAME],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           nil, COMMIT_BRANCH_NAME, nil))
   end
 
   test "find_commit_range min_version with short branch name" do
     assert_equal([SHORT_BRANCH_COMMIT_2],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           SHORT_COMMIT_BRANCH_NAME, nil, nil))
   end
 
   test "find_commit_range max_version with short branch name" do
     assert_equal([SHORT_BRANCH_COMMIT_2],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           nil, SHORT_COMMIT_BRANCH_NAME, nil))
   end
 
   test "find_commit_range min_version with disambiguated branch name" do
     assert_equal([COMMIT_BRANCH_COMMIT_2],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           "heads/#{COMMIT_BRANCH_NAME}",
                                           nil, nil))
   end
 
   test "find_commit_range max_version with disambiguated branch name" do
     assert_equal([COMMIT_BRANCH_COMMIT_2],
-                 Commit.find_commit_range("active/shabranchnames", nil,
+                 CommitsHelper::find_commit_range("active/shabranchnames", nil,
                                           "heads/#{COMMIT_BRANCH_NAME}", nil))
   end
 
   test "find_commit_range min_version with unambiguous short name" do
     assert_equal([COMMIT_BRANCH_NAME],
-                 Commit.find_commit_range("active/shabranchnames",
+                 CommitsHelper::find_commit_range("active/shabranchnames",
                                           COMMIT_BRANCH_NAME[0..-2], nil, nil))
   end
 
   test "find_commit_range max_version with unambiguous short name" do
     assert_equal([COMMIT_BRANCH_NAME],
-                 Commit.find_commit_range("active/shabranchnames", nil,
+                 CommitsHelper::find_commit_range("active/shabranchnames", nil,
                                           COMMIT_BRANCH_NAME[0..-2], nil))
   end
 
@@ -192,77 +192,77 @@ class CommitTest < ActiveSupport::TestCase
     authorize_with :active
 
     # single
-    a = Commit.find_commit_range('active/foo', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
+    a = CommitsHelper::find_commit_range('active/foo', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
     assert_equal ['31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
 
     #test "test_branch1" do
-    a = Commit.find_commit_range('active/foo', nil, 'master', nil)
+    a = CommitsHelper::find_commit_range('active/foo', nil, 'master', nil)
     assert_includes(a, '077ba2ad3ea24a929091a9e6ce545c93199b8e57')
 
     #test "test_branch2" do
-    a = Commit.find_commit_range('active/foo', nil, 'b1', nil)
+    a = CommitsHelper::find_commit_range('active/foo', nil, 'b1', nil)
     assert_equal ['1de84a854e2b440dc53bf42f8548afa4c17da332'], a
 
     #test "test_branch3" do
-    a = Commit.find_commit_range('active/foo', nil, 'HEAD', nil)
+    a = CommitsHelper::find_commit_range('active/foo', nil, 'HEAD', nil)
     assert_equal ['1de84a854e2b440dc53bf42f8548afa4c17da332'], a
 
     #test "test_single_revision_repo" do
-    a = Commit.find_commit_range('active/foo', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
+    a = CommitsHelper::find_commit_range('active/foo', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
     assert_equal ['31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
-    a = Commit.find_commit_range('arvados', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
+    a = CommitsHelper::find_commit_range('arvados', nil, '31ce37fe365b3dc204300a3e4c396ad333ed0556', nil)
     assert_equal [], a
 
     #test "test_multi_revision" do
     # complains "fatal: bad object 077ba2ad3ea24a929091a9e6ce545c93199b8e57"
-    a = Commit.find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', nil)
+    a = CommitsHelper::find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', nil)
     assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '4fe459abe02d9b365932b8f5dc419439ab4e2577', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
 
     #test "test_tag" do
     # complains "fatal: ambiguous argument 'tag1': unknown revision or path
     # not in the working tree."
-    a = Commit.find_commit_range('active/foo', 'tag1', 'master', nil)
+    a = CommitsHelper::find_commit_range('active/foo', 'tag1', 'master', nil)
     assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '4fe459abe02d9b365932b8f5dc419439ab4e2577'], a
 
     #test "test_multi_revision_exclude" do
-    a = Commit.find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['4fe459abe02d9b365932b8f5dc419439ab4e2577'])
+    a = CommitsHelper::find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['4fe459abe02d9b365932b8f5dc419439ab4e2577'])
     assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
 
     #test "test_multi_revision_tagged_exclude" do
     # complains "fatal: bad object 077ba2ad3ea24a929091a9e6ce545c93199b8e57"
-    a = Commit.find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['tag1'])
+    a = CommitsHelper::find_commit_range('active/foo', '31ce37fe365b3dc204300a3e4c396ad333ed0556', '077ba2ad3ea24a929091a9e6ce545c93199b8e57', ['tag1'])
     assert_equal ['077ba2ad3ea24a929091a9e6ce545c93199b8e57', '31ce37fe365b3dc204300a3e4c396ad333ed0556'], a
 
     Dir.mktmpdir do |touchdir|
       # invalid input to maximum
-      a = Commit.find_commit_range('active/foo', nil, "31ce37fe365b3dc204300a3e4c396ad333ed0556 ; touch #{touchdir}/uh_oh", nil)
+      a = CommitsHelper::find_commit_range('active/foo', nil, "31ce37fe365b3dc204300a3e4c396ad333ed0556 ; touch #{touchdir}/uh_oh", nil)
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'maximum' parameter of find_commit_range is exploitable"
       assert_equal [], a
 
       # invalid input to maximum
-      a = Commit.find_commit_range('active/foo', nil, "$(uname>#{touchdir}/uh_oh)", nil)
+      a = CommitsHelper::find_commit_range('active/foo', nil, "$(uname>#{touchdir}/uh_oh)", nil)
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'maximum' parameter of find_commit_range is exploitable"
       assert_equal [], a
 
       # invalid input to minimum
-      a = Commit.find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556 ; touch #{touchdir}/uh_oh", "31ce37fe365b3dc204300a3e4c396ad333ed0556", nil)
+      a = CommitsHelper::find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556 ; touch #{touchdir}/uh_oh", "31ce37fe365b3dc204300a3e4c396ad333ed0556", nil)
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'minimum' parameter of find_commit_range is exploitable"
       assert_equal [], a
 
       # invalid input to minimum
-      a = Commit.find_commit_range('active/foo', "$(uname>#{touchdir}/uh_oh)", "31ce37fe365b3dc204300a3e4c396ad333ed0556", nil)
+      a = CommitsHelper::find_commit_range('active/foo', "$(uname>#{touchdir}/uh_oh)", "31ce37fe365b3dc204300a3e4c396ad333ed0556", nil)
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'minimum' parameter of find_commit_range is exploitable"
       assert_equal [], a
 
       # invalid input to 'excludes'
       # complains "fatal: bad object 077ba2ad3ea24a929091a9e6ce545c93199b8e57"
-      a = Commit.find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556", "077ba2ad3ea24a929091a9e6ce545c93199b8e57", ["4fe459abe02d9b365932b8f5dc419439ab4e2577 ; touch #{touchdir}/uh_oh"])
+      a = CommitsHelper::find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556", "077ba2ad3ea24a929091a9e6ce545c93199b8e57", ["4fe459abe02d9b365932b8f5dc419439ab4e2577 ; touch #{touchdir}/uh_oh"])
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'excludes' parameter of find_commit_range is exploitable"
       assert_equal [], a
 
       # invalid input to 'excludes'
       # complains "fatal: bad object 077ba2ad3ea24a929091a9e6ce545c93199b8e57"
-      a = Commit.find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556", "077ba2ad3ea24a929091a9e6ce545c93199b8e57", ["$(uname>#{touchdir}/uh_oh)"])
+      a = CommitsHelper::find_commit_range('active/foo', "31ce37fe365b3dc204300a3e4c396ad333ed0556", "077ba2ad3ea24a929091a9e6ce545c93199b8e57", ["$(uname>#{touchdir}/uh_oh)"])
       assert !File.exist?("#{touchdir}/uh_oh"), "#{touchdir}/uh_oh should not exist, 'excludes' parameter of find_commit_range is exploitable"
       assert_equal [], a
     end
index 5ece9ab499a342a2e95466241c9b885ade8a8c71..b20120635ea528219ff6c4e3e1a0b3771d54db6e 100644 (file)
@@ -589,53 +589,6 @@ class JobTest < ActiveSupport::TestCase
     assert_equal foobar.uuid, j.uuid
   end
 
-  [
-    true,
-    false,
-  ].each do |cascade|
-    test "cancel job with cascade #{cascade}" do
-      job = Job.find_by_uuid jobs(:running_job_with_components_at_level_1).uuid
-      job.cancel cascade: cascade
-      assert_equal Job::Cancelled, job.state
-
-      descendents = ['zzzzz-8i9sb-jobcomponentsl2',
-                     'zzzzz-d1hrv-picomponentsl02',
-                     'zzzzz-8i9sb-job1atlevel3noc',
-                     'zzzzz-8i9sb-job2atlevel3noc']
-
-      jobs = Job.where(uuid: descendents)
-      jobs.each do |j|
-        assert_equal ('Cancelled' == j.state), cascade
-      end
-
-      pipelines = PipelineInstance.where(uuid: descendents)
-      pipelines.each do |pi|
-        assert_equal ('Paused' == pi.state), cascade
-      end
-    end
-  end
-
-  test 'cancelling a completed job raises error' do
-    job = Job.find_by_uuid jobs(:job_with_latest_version).uuid
-    assert job
-    assert_equal 'Complete', job.state
-
-    assert_raises(ArvadosModel::InvalidStateTransitionError) do
-      job.cancel
-    end
-  end
-
-  test 'cancelling a job with circular relationship with another does not result in an infinite loop' do
-    job = Job.find_by_uuid jobs(:running_job_2_with_circular_component_relationship).uuid
-
-    job.cancel cascade: true
-
-    assert_equal Job::Cancelled, job.state
-
-    child = Job.find_by_uuid job.components.collect{|_, uuid| uuid}[0]
-    assert_equal Job::Cancelled, child.state
-  end
-
   test 'enable legacy api configuration option = true' do
     Rails.configuration.Containers.JobsAPI.Enable = "true"
     check_enable_legacy_jobs_api