[ARVADOS] updated: a263160f6243ba5485d8cc2dd10c15822f1252d6
Git user
git at public.curoverse.com
Thu Aug 24 16:23:03 EDT 2017
Summary of changes:
services/api/app/models/arvados_model.rb | 73 ++++++++++++-----------
services/api/app/models/collection.rb | 84 +-------------------------
services/api/app/models/user.rb | 10 ++--
services/api/lib/create_permission_view.sql | 7 ++-
services/api/lib/trashable.rb | 91 +++++++++++++++++++++++++++++
5 files changed, 137 insertions(+), 128 deletions(-)
create mode 100644 services/api/lib/trashable.rb
via a263160f6243ba5485d8cc2dd10c15822f1252d6 (commit)
from 59d6af850acadece8512e701188f111d82e03497 (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 a263160f6243ba5485d8cc2dd10c15822f1252d6
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 24 16:22:12 2017 -0400
12032: Additional refactoring of readable_by. Refactor "trashable" into module.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 457b1bf..5a9d43f 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -254,7 +254,7 @@ class ArvadosModel < ActiveRecord::Base
# Collect the UUIDs of the authorized users.
sql_table = kwargs.fetch(:table_name, table_name)
- include_trashed = kwargs.fetch(:include_trashed, 0)
+ include_trashed = kwargs.fetch(:include_trashed, false)
sql_conds = []
user_uuids = users_list.map { |u| u.uuid }
@@ -263,57 +263,56 @@ class ArvadosModel < ActiveRecord::Base
# Check if any of the users are admin.
if users_list.select { |u| u.is_admin }.any?
- # For admins, only filter on "trashed"
- # sql_conds += ["#{sql_table}.uuid in (SELECT target_uuid
- # FROM permission_view
- # WHERE trashed in (:include_trashed)
- # GROUP BY user_uuid, target_uuid)"]
-
- # if self.column_names.include? 'owner_uuid'
- # sql_conds[0] += "AND #{sql_table}.owner_uuid in (SELECT target_uuid
- # FROM permission_view
- # WHERE trashed in (:include_trashed)
- # GROUP BY user_uuid, target_uuid)"
- # end
- return where({})
+ if !include_trashed
+ # exclude rows that are trashed.
+ if self.column_names.include? "owner_uuid"
+ sql_conds += ["NOT EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE trashed = 1 AND
+ (#{sql_table}.uuid = pv.target_uuid OR #{sql_table}.owner_uuid = pv.target_uuid))"]
+ else
+ sql_conds += ["NOT EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE trashed = 1 AND
+ (#{sql_table}.uuid = pv.target_uuid))"]
+ end
+ end
else
+ # Match objects which appear in the permission view
+ trash_clause = if !include_trashed then "trashed = 0 AND" else "" end
+
# Match any object (evidently a group or user) whose UUID is
# listed explicitly in user_uuids.
- sql_conds += ["#{sql_table}.uuid in (:user_uuids)"]
-
- # Match any object whose owner is listed explicitly in
- # user_uuids.
- sql_conds += ["#{sql_table}.owner_uuid IN (:user_uuids)"]
-
- # At least read permission from user_uuid to target_uuid of object
- sql_conds += ["#{sql_table}.uuid in (SELECT target_uuid
- FROM permission_view
- WHERE user_uuid in (:user_uuids) and perm_level >= 1 and trashed = (:include_trashed)
- GROUP BY user_uuid, target_uuid)"]
-
- if self.column_names.include? 'owner_uuid'
- # At least read permission from user_uuid to target_uuid that owns object
- sql_conds += ["#{sql_table}.owner_uuid in (SELECT target_uuid
- FROM permission_view
- WHERE user_uuid in (:user_uuids) and
- target_owner_uuid IS NOT NULL and
- perm_level >= 1 and trashed = (:include_trashed)
- GROUP BY user_uuid, target_uuid)"]
+ sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
+
+ if self.column_names.include? "owner_uuid"
+ sql_conds += ["EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+ (#{sql_table}.uuid = pv.target_uuid OR
+ (#{sql_table}.owner_uuid = pv.target_uuid AND pv.target_owner_uuid is NOT NULL)))"]
+ # Match any object whose owner is listed explicitly in
+ # user_uuids.
+ sql_conds += ["#{sql_table}.owner_uuid IN (:user_uuids)"]
+ else
+ sql_conds += ["EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+ (#{sql_table}.uuid = pv.target_uuid))"]
end
if sql_table == "links"
# Match any permission link that gives one of the authorized
# users some permission _or_ gives anyone else permission to
# view one of the authorized users.
- sql_conds += ["(#{sql_table}.link_class in (:permission_link_classes) AND "+
+ sql_conds += ["(#{sql_table}.link_class IN (:permission_link_classes) AND "+
"(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"]
end
end
where(sql_conds.join(' OR '),
user_uuids: user_uuids,
- permission_link_classes: ['permission', 'resources'],
- include_trashed: include_trashed)
+ permission_link_classes: ['permission', 'resources'])
end
def save_with_unique_name!
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 6f13a63..ce7e9fe 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -4,6 +4,7 @@
require 'arvados/keep'
require 'sweep_trashed_collections'
+require 'trashable'
class Collection < ArvadosModel
extend CurrentApiClient
@@ -11,20 +12,16 @@ class Collection < ArvadosModel
include HasUuid
include KindAndEtag
include CommonApiTemplate
+ include Trashable
serialize :properties, Hash
- before_validation :set_validation_timestamp
before_validation :default_empty_manifest
before_validation :check_encoding
before_validation :check_manifest_validity
before_validation :check_signatures
before_validation :strip_signatures_and_update_replication_confirmed
- before_validation :ensure_trash_at_not_in_past
- before_validation :sync_trash_state
- before_validation :default_trash_interval
validate :ensure_pdh_matches_manifest_text
- validate :validate_trash_and_delete_timing
before_save :set_file_names
# Query only untrashed collections by default.
@@ -486,81 +483,4 @@ class Collection < ArvadosModel
super
end
- # Use a single timestamp for all validations, even though each
- # validation runs at a different time.
- def set_validation_timestamp
- @validation_timestamp = db_current_time
- end
-
- # If trash_at is being changed to a time in the past, change it to
- # now. This allows clients to say "expires {client-current-time}"
- # without failing due to clock skew, while avoiding odd log entries
- # like "expiry date changed to {1 year ago}".
- def ensure_trash_at_not_in_past
- if trash_at_changed? && trash_at
- self.trash_at = [@validation_timestamp, trash_at].max
- end
- end
-
- # Caller can move into/out of trash by setting/clearing is_trashed
- # -- however, if the caller also changes trash_at, then any changes
- # to is_trashed are ignored.
- def sync_trash_state
- if is_trashed_changed? && !trash_at_changed?
- if is_trashed
- self.trash_at = @validation_timestamp
- else
- self.trash_at = nil
- self.delete_at = nil
- end
- end
- self.is_trashed = trash_at && trash_at <= @validation_timestamp || false
- true
- end
-
- def default_trash_interval
- if trash_at_changed? && !delete_at_changed?
- # If trash_at is updated without touching delete_at,
- # automatically update delete_at to a sensible value.
- if trash_at.nil?
- self.delete_at = nil
- else
- self.delete_at = trash_at + Rails.configuration.default_trash_lifetime.seconds
- end
- elsif !trash_at || !delete_at || trash_at > delete_at
- # Not trash, or bogus arguments? Just validate in
- # validate_trash_and_delete_timing.
- elsif delete_at_changed? && delete_at >= trash_at
- # Fix delete_at if needed, so it's not earlier than the expiry
- # time on any permission tokens that might have been given out.
-
- # In any case there are no signatures expiring after now+TTL.
- # Also, if the existing trash_at time has already passed, we
- # know we haven't given out any signatures since then.
- earliest_delete = [
- @validation_timestamp,
- trash_at_was,
- ].compact.min + Rails.configuration.blob_signature_ttl.seconds
-
- # The previous value of delete_at is also an upper bound on the
- # longest-lived permission token. For example, if TTL=14,
- # trash_at_was=now-7, delete_at_was=now+7, then it is safe to
- # set trash_at=now+6, delete_at=now+8.
- earliest_delete = [earliest_delete, delete_at_was].compact.min
-
- # If delete_at is too soon, use the earliest possible time.
- if delete_at < earliest_delete
- self.delete_at = earliest_delete
- end
- end
- end
-
- def validate_trash_and_delete_timing
- if trash_at.nil? != delete_at.nil?
- errors.add :delete_at, "must be set if trash_at is set, and must be nil otherwise"
- elsif delete_at && delete_at < trash_at
- errors.add :delete_at, "must not be earlier than trash_at"
- end
- true
- end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index c8c9bfb..0e2db76 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -155,10 +155,9 @@ class User < ArvadosModel
install_view('permission')
all_perms = {}
ActiveRecord::Base.connection.
- exec_query('SELECT user_uuid, target_owner_uuid, max(perm_level), max(trashed)
+ exec_query('SELECT user_uuid, target_owner_uuid, perm_level, trashed
FROM permission_view
- WHERE target_owner_uuid IS NOT NULL
- GROUP BY user_uuid, target_owner_uuid',
+ WHERE target_owner_uuid IS NOT NULL',
# "name" arg is a query label that appears in logs:
"all_group_permissions",
).rows.each do |user_uuid, group_uuid, max_p_val, trashed|
@@ -176,11 +175,10 @@ class User < ArvadosModel
group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
ActiveRecord::Base.connection.
- exec_query('SELECT target_owner_uuid, max(perm_level), max(trashed)
+ exec_query('SELECT target_owner_uuid, perm_level, trashed
FROM permission_view
WHERE user_uuid = $1
- AND target_owner_uuid IS NOT NULL
- GROUP BY target_owner_uuid',
+ AND target_owner_uuid IS NOT NULL',
# "name" arg is a query label that appears in logs:
"group_permissions for #{uuid}",
# "binds" arg is an array of [col_id, value] for '$1' vars:
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 10c4b27..82839e8 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -60,7 +60,8 @@ perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
)
SELECT user_uuid,
target_uuid,
- val AS perm_level,
+ MAX(val) AS perm_level,
CASE follow WHEN true THEN target_uuid ELSE NULL END AS target_owner_uuid,
- trashed
- FROM perm;
+ MAX(trashed) AS trashed
+ FROM perm
+ GROUP BY user_uuid, target_uuid, target_owner_uuid;
diff --git a/services/api/lib/trashable.rb b/services/api/lib/trashable.rb
new file mode 100644
index 0000000..1e2f466
--- /dev/null
+++ b/services/api/lib/trashable.rb
@@ -0,0 +1,91 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+module Trashable
+ def self.included(base)
+ base.before_validation :set_validation_timestamp
+ base.before_validation :ensure_trash_at_not_in_past
+ base.before_validation :sync_trash_state
+ base.before_validation :default_trash_interval
+ base.validate :validate_trash_and_delete_timing
+ end
+
+ # Use a single timestamp for all validations, even though each
+ # validation runs at a different time.
+ def set_validation_timestamp
+ @validation_timestamp = db_current_time
+ end
+
+ # If trash_at is being changed to a time in the past, change it to
+ # now. This allows clients to say "expires {client-current-time}"
+ # without failing due to clock skew, while avoiding odd log entries
+ # like "expiry date changed to {1 year ago}".
+ def ensure_trash_at_not_in_past
+ if trash_at_changed? && trash_at
+ self.trash_at = [@validation_timestamp, trash_at].max
+ end
+ end
+
+ # Caller can move into/out of trash by setting/clearing is_trashed
+ # -- however, if the caller also changes trash_at, then any changes
+ # to is_trashed are ignored.
+ def sync_trash_state
+ if is_trashed_changed? && !trash_at_changed?
+ if is_trashed
+ self.trash_at = @validation_timestamp
+ else
+ self.trash_at = nil
+ self.delete_at = nil
+ end
+ end
+ self.is_trashed = trash_at && trash_at <= @validation_timestamp || false
+ true
+ end
+
+ def default_trash_interval
+ if trash_at_changed? && !delete_at_changed?
+ # If trash_at is updated without touching delete_at,
+ # automatically update delete_at to a sensible value.
+ if trash_at.nil?
+ self.delete_at = nil
+ else
+ self.delete_at = trash_at + Rails.configuration.default_trash_lifetime.seconds
+ end
+ elsif !trash_at || !delete_at || trash_at > delete_at
+ # Not trash, or bogus arguments? Just validate in
+ # validate_trash_and_delete_timing.
+ elsif delete_at_changed? && delete_at >= trash_at
+ # Fix delete_at if needed, so it's not earlier than the expiry
+ # time on any permission tokens that might have been given out.
+
+ # In any case there are no signatures expiring after now+TTL.
+ # Also, if the existing trash_at time has already passed, we
+ # know we haven't given out any signatures since then.
+ earliest_delete = [
+ @validation_timestamp,
+ trash_at_was,
+ ].compact.min + Rails.configuration.blob_signature_ttl.seconds
+
+ # The previous value of delete_at is also an upper bound on the
+ # longest-lived permission token. For example, if TTL=14,
+ # trash_at_was=now-7, delete_at_was=now+7, then it is safe to
+ # set trash_at=now+6, delete_at=now+8.
+ earliest_delete = [earliest_delete, delete_at_was].compact.min
+
+ # If delete_at is too soon, use the earliest possible time.
+ if delete_at < earliest_delete
+ self.delete_at = earliest_delete
+ end
+ end
+ end
+
+ def validate_trash_and_delete_timing
+ if trash_at.nil? != delete_at.nil?
+ errors.add :delete_at, "must be set if trash_at is set, and must be nil otherwise"
+ elsif delete_at && delete_at < trash_at
+ errors.add :delete_at, "must not be earlier than trash_at"
+ end
+ true
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list