Merge branch '21666-provision-test-improvement'
[arvados.git] / services / api / app / models / arvados_model.rb
index 3ddbafcdb234fc9afddb8392c0860135711b6ff3..9ee2cca410effaba81fbb4ce9d207354c5f1b3f8 100644 (file)
@@ -24,6 +24,7 @@ class ArvadosModel < ApplicationRecord
   before_destroy :ensure_owner_uuid_is_permitted
   before_destroy :ensure_permission_to_destroy
   before_create :update_modified_by_fields
+  before_create :add_uuid_to_name, :if => Proc.new { @_add_uuid_to_name }
   before_update :maybe_update_modified_by_fields
   after_create :log_create
   after_update :log_update
@@ -37,9 +38,9 @@ class ArvadosModel < ApplicationRecord
   # user.uuid==object.owner_uuid.
   has_many(:permissions,
            ->{where(link_class: 'permission')},
-           foreign_key: :head_uuid,
+           foreign_key: 'head_uuid',
            class_name: 'Link',
-           primary_key: :uuid)
+           primary_key: 'uuid')
 
   # If async is true at create or update, permission graph
   # update is deferred allowing making multiple calls without the performance
@@ -145,7 +146,7 @@ class ArvadosModel < ApplicationRecord
     super(permit_attribute_params(raw_params), *args)
   end
 
-  def update_attributes raw_params={}, *args
+  def update raw_params={}, *args
     super(self.class.permit_attribute_params(raw_params), *args)
   end
 
@@ -156,7 +157,7 @@ class ArvadosModel < ApplicationRecord
   end
 
   def self.searchable_columns operator
-    textonly_operator = !operator.match(/[<=>]/)
+    textonly_operator = !operator.match(/[<=>]/) && !operator.in?(['in', 'not in'])
     self.columns.select do |col|
       case col.type
       when :string, :text
@@ -220,7 +221,7 @@ class ArvadosModel < ApplicationRecord
   end
 
   def self.default_orders
-    ["#{table_name}.modified_at desc", "#{table_name}.uuid"]
+    ["#{table_name}.modified_at desc", "#{table_name}.uuid desc"]
   end
 
   def self.unique_columns
@@ -249,9 +250,9 @@ class ArvadosModel < ApplicationRecord
     # Return [] if this is a frozen project and the current user can't
     # unfreeze
     return [] if respond_to?(:frozen_by_uuid) && frozen_by_uuid &&
-                 !(Rails.configuration.API.UnfreezeProjectRequiresAdmin ?
-                     current_user.andand.is_admin :
-                     current_user.can?(manage: uuid))
+                 (Rails.configuration.API.UnfreezeProjectRequiresAdmin ?
+                    !current_user.andand.is_admin :
+                    !current_user.can?(manage: uuid))
     # Return [] if nobody can write because this object is inside a
     # frozen project
     return [] if FrozenGroup.where(uuid: owner_uuid).any?
@@ -273,6 +274,26 @@ class ArvadosModel < ApplicationRecord
     end.compact.uniq
   end
 
+  def can_write
+    if respond_to?(:frozen_by_uuid) && frozen_by_uuid
+      # This special case is needed to return the correct value from a
+      # "freeze project" API, during which writable status changes
+      # from true to false.
+      #
+      # current_user.can?(write: self) returns true (which is correct
+      # in the context of permission-checking hooks) but the can_write
+      # value we're returning to the caller here represents the state
+      # _after_ the update, i.e., false.
+      return false
+    else
+      return current_user.can?(write: self)
+    end
+  end
+
+  def can_manage
+    return current_user.can?(manage: self)
+  end
+
   # Return a query with read permissions restricted to the union of the
   # permissions of the members of users_list, i.e. if something is readable by
   # any user in users_list, it will be readable in the query returned by this
@@ -302,6 +323,15 @@ class ArvadosModel < ApplicationRecord
     # For details on how the trashed_groups table is constructed, see
     # see db/migrate/20200501150153_permission_table.rb
 
+    # excluded_trash is a SQL expression that determines whether a row
+    # should be excluded from the results due to being trashed.
+    # Trashed items inside frozen projects are invisible to regular
+    # (non-admin) users even when using include_trash, so we have:
+    #
+    # (item_trashed || item_inside_trashed_project)
+    # &&
+    # (!caller_requests_include_trash ||
+    #  (item_inside_frozen_project && caller_is_not_admin))
     if (admin && include_trash) || sql_table == "api_client_authorizations"
       excluded_trash = "false"
     else
@@ -322,7 +352,7 @@ class ArvadosModel < ApplicationRecord
       # on trashed items.
       if !include_trash && sql_table != "api_client_authorizations"
         # Only include records where the owner is not trashed
-        sql_conds = "NOT #{excluded_trash}"
+        sql_conds = "NOT (#{excluded_trash})"
       end
     else
       # The core of the permission check is a join against the
@@ -422,7 +452,7 @@ class ArvadosModel < ApplicationRecord
                      "    WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 3))) "
       end
 
-      sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) AND NOT #{excluded_trash}"
+      sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) AND NOT (#{excluded_trash})"
 
     end
 
@@ -435,26 +465,24 @@ class ArvadosModel < ApplicationRecord
       end
     end
 
+    return self if sql_conds == nil
     self.where(sql_conds,
                user_uuids: all_user_uuids.collect{|c| c["target_uuid"]},
                permission_link_classes: ['permission'])
   end
 
   def save_with_unique_name!
-    uuid_was = uuid
-    name_was = name
     max_retries = 2
     transaction do
       conn = ActiveRecord::Base.connection
       conn.exec_query 'SAVEPOINT save_with_unique_name'
       begin
         save!
+        conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name'
       rescue ActiveRecord::RecordNotUnique => rn
         raise if max_retries == 0
         max_retries -= 1
 
-        conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name'
-
         # Dig into the error to determine if it is specifically calling out a
         # (owner_uuid, name) uniqueness violation.  In this specific case, and
         # the client requested a unique name with ensure_unique_name==true,
@@ -472,27 +500,23 @@ class ArvadosModel < ApplicationRecord
         detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL)
         raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail
 
-        new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})"
-        if new_name == name
-          # If the database is fast enough to do two attempts in the
-          # same millisecond, we need to wait to ensure we try a
-          # different timestamp on each attempt.
-          sleep 0.002
-          new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})"
-        end
+        conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name'
 
-        self[:name] = new_name
-        if uuid_was.nil? && !uuid.nil?
+        if uuid_was.nil?
+          # new record, the uuid caused a name collision (very
+          # unlikely but possible), so generate new uuid
           self[:uuid] = nil
           if self.is_a? Collection
-            # Reset so that is assigned to the new UUID
+            # Also needs to be reset
             self[:current_version_uuid] = nil
           end
+          # need to adjust the name after the uuid has been generated
+          add_uuid_to_make_unique_name
+        else
+          # existing record, just update the name directly.
+          add_uuid_to_name
         end
-        conn.exec_query 'SAVEPOINT save_with_unique_name'
         retry
-      ensure
-        conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name'
       end
     end
   end
@@ -552,6 +576,26 @@ class ArvadosModel < ApplicationRecord
                           *ft[:param_out])
   end
 
+  @_add_uuid_to_name = false
+  def add_uuid_to_make_unique_name
+    @_add_uuid_to_name = true
+  end
+
+  def add_uuid_to_name
+    # Incorporate the random part of the UUID into the name.  This
+    # lets us prevent name collision but the part we add to the name
+    # is still somewhat meaningful (instead of generating a second
+    # random meaningless string).
+    #
+    # Because ArvadosModel is an abstract class and assign_uuid is
+    # part of HasUuid (which is included by the other concrete
+    # classes) the assign_uuid hook gets added (and run) after this
+    # one.  So we need to call assign_uuid here to make sure we have a
+    # uuid.
+    assign_uuid
+    self.name = "#{self.name[0..236]} (#{self.uuid[-15..-1]})"
+  end
+
   protected
 
   def self.deep_sort_hash(x)
@@ -647,7 +691,7 @@ class ArvadosModel < ApplicationRecord
       # itself. (If we're in the act of unfreezing, we only need
       # :unfreeze permission, which means "what write permission would
       # be if target weren't frozen")
-      unless ((respond_to?(:frozen_by_uuid) && frozen_by_uuid_in_database && !frozen_by_uuid) ?
+      unless ((respond_to?(:frozen_by_uuid) && frozen_by_uuid_was && !frozen_by_uuid) ?
                 current_user.can?(unfreeze: uuid) :
                 current_user.can?(write: uuid))
         logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} without write permission"
@@ -910,8 +954,6 @@ class ArvadosModel < ApplicationRecord
   # hook.
   def fill_container_defaults_after_find
     fill_container_defaults
-    set_attribute_was('runtime_constraints', runtime_constraints)
-    set_attribute_was('scheduling_parameters', scheduling_parameters)
     clear_changes_information
   end
 
@@ -922,6 +964,10 @@ class ArvadosModel < ApplicationRecord
   # value in the database to an implicit zero/false value in an update
   # request.
   def fill_container_defaults
+    # Make sure this is correctly sorted by key, because we merge in
+    # whatever is in the database on top of it, this will be the order
+    # that gets used downstream rather than the order the keys appear
+    # in the database.
     self.runtime_constraints = {
       'API' => false,
       'cuda' => {
@@ -929,6 +975,7 @@ class ArvadosModel < ApplicationRecord
         'driver_version' => '',
         'hardware_capability' => '',
       },
+      'keep_cache_disk' => 0,
       'keep_cache_ram' => 0,
       'ram' => 0,
       'vcpus' => 0,
@@ -937,6 +984,7 @@ class ArvadosModel < ApplicationRecord
       'max_run_time' => 0,
       'partitions' => [],
       'preemptible' => false,
+      'supervisor' => false,
     }.merge(attributes['scheduling_parameters'] || {})
   end