diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb index f19d474..3175e67 100644 --- a/apps/workbench/app/models/arvados_base.rb +++ b/apps/workbench/app/models/arvados_base.rb @@ -62,8 +62,7 @@ class ArvadosBase < ActiveRecord::Base schema = arvados_api_client.discovery[:schemas][self.to_s.to_sym] return @columns if schema.nil? schema[:properties].each do |k, coldef| - case k - when :etag, :kind + if coldef[:readonly] or (k == :etag) or (k == :kind) attr_reader k else if coldef[:type] == coldef[:type].downcase @@ -329,7 +328,7 @@ class ArvadosBase < ActiveRecord::Base (current_user.is_admin or current_user.uuid == self.owner_uuid or new_record? or - (respond_to?(:writable_by) ? + ((respond_to?(:writable_by) and !writable_by.nil?) ? writable_by.include?(current_user.uuid) : (ArvadosBase.find(owner_uuid).writable_by.include? current_user.uuid rescue false)))) or false end diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb index dcc9c63..3d8e9ff 100644 --- a/services/api/app/controllers/arvados/v1/schema_controller.rb +++ b/services/api/app/controllers/arvados/v1/schema_controller.rb @@ -95,18 +95,20 @@ class Arvados::V1::SchemaController < ApplicationController next end object_properties = {} - k.columns. - select { |col| col.name != 'id' }. - collect do |col| - if k.serialized_attributes.has_key? col.name - object_properties[col.name] = { - type: k.serialized_attributes[col.name].object_class.to_s - } + attr_types = k.attributes_types + k.all_api_accessible_attributes.each_key.map(&:to_s).each do |attr_name| + attr_schema = {} + if attr_types.include?(attr_name) + attr_schema[:type] = attr_types[attr_name] + elsif k.serialized_attributes.include?(attr_name) + attr_schema[:type] = k.serialized_attributes[attr_name].object_class.to_s else - object_properties[col.name] = { - type: col.type - } + attr_schema[:type] = k.columns_hash[attr_name].type + end + unless k.columns_hash.include?(attr_name) + attr_schema[:readonly] = true end + object_properties[attr_name] = attr_schema end discovery[:schemas][k.to_s + 'List'] = { id: k.to_s + 'List', diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index b9442d6..65c67bd 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -22,6 +22,15 @@ class ApiClientAuthorization < ArvadosModel UNLOGGED_CHANGES = ['last_used_at', 'last_used_by_ip_address', 'updated_at'] + def self.attributes_types + super.merge({ "uuid" => columns_hash["api_token"].type, + "owner_uuid" => "string", + "modified_by_client_uuid" => "string", + "modified_by_user_uuid" => "string", + "modified_at" => "timestamp", + }) + end + def assign_random_api_token self.api_token ||= rand(2**256).to_s(36) end diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 1fe5808..c4e3bea 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -81,6 +81,30 @@ class ArvadosModel < ActiveRecord::Base self.columns.select { |col| col.name == attr.to_s }.first end + def self.all_api_accessible_attributes + # Return a hash that maps attributes accessible through the API to the + # methods that generate them (all symbols). + # The result includes attributes accessible through any API template. + # If the same attribute is generated by different methods in different + # templates, the value for that attribute key is undefined. + result = {} + methods.grep(/^api_accessible_\w+$/).each do |method_name| + next if method_name == :api_accessible_attributes + result.merge!(send(method_name)) + end + result + end + + def self.attributes_types + # Return a hash that maps attribute name strings to type strings. + # Subclasses should override this to provide type information for the + # discovery document for any attributes that aren't database-backed. + { "etag" => "string", + "href" => "string", + "kind" => "string", + } + end + def self.attributes_required_columns # This method returns a hash. Each key is the name of an API attribute, # and it's mapped to a list of database columns that must be fetched @@ -91,13 +115,10 @@ class ArvadosModel < ActiveRecord::Base # specific columns from the database. all_columns = columns.map(&:name) api_column_map = Hash.new { |hash, key| hash[key] = [] } - methods.grep(/^api_accessible_\w+$/).each do |method_name| - next if method_name == :api_accessible_attributes - send(method_name).each_pair do |api_attr_name, col_name| - col_name = col_name.to_s - if all_columns.include?(col_name) - api_column_map[api_attr_name.to_s] |= [col_name] - end + all_api_accessible_attributes.each_pair do |api_attr_name, source_name| + source_name = source_name.to_s + if all_columns.include?(source_name) + api_column_map[api_attr_name.to_s] |= [source_name] end end api_column_map diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb index 0e857ad..99961c0 100644 --- a/services/api/app/models/group.rb +++ b/services/api/app/models/group.rb @@ -16,6 +16,10 @@ class Group < ArvadosModel t.add :writable_by end + def self.attributes_types + super.merge({"writable_by" => "Array"}) + end + def maybe_invalidate_permissions_cache if uuid_changed? or owner_uuid_changed? # This can change users' permissions on other groups as well as diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb index 01df069..8b9d275 100644 --- a/services/api/app/models/job.rb +++ b/services/api/app/models/job.rb @@ -62,6 +62,12 @@ class Job < ArvadosModel (Complete = 'Complete'), ] + def self.attributes_types + super.merge({ "queue_position" => "integer", + "node_uuids" => "Array", + }) + end + def assert_finished update_attributes(finished_at: finished_at || db_current_time, success: success.nil? ? false : success, diff --git a/services/api/app/models/keep_disk.rb b/services/api/app/models/keep_disk.rb index 4623393..ef14df3 100644 --- a/services/api/app/models/keep_disk.rb +++ b/services/api/app/models/keep_disk.rb @@ -23,6 +23,14 @@ class KeepDisk < ArvadosModel t.add :ping_secret end + def self.attributes_types + proxy_attrs = {} + %w(service_host service_port service_ssl_flag).each do |attr_name| + proxy_attrs[attr_name] = KeepService.columns_hash[attr_name].type + end + super.merge(proxy_attrs) + end + def foreign_key_attributes super.reject { |a| a == "filesystem_uuid" } end diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb index d9b8f6f..79ec2be 100644 --- a/services/api/app/models/link.rb +++ b/services/api/app/models/link.rb @@ -21,6 +21,12 @@ class Link < ArvadosModel t.add :properties end + def self.attributes_types + result = super + kind_type = result["kind"] + result.merge({"head_kind" => kind_type, "tail_kind" => kind_type}) + end + def properties @properties ||= Hash.new super diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb index b10a491..375c930 100644 --- a/services/api/app/models/log.rb +++ b/services/api/app/models/log.rb @@ -18,6 +18,10 @@ class Log < ArvadosModel t.add :properties end + def self.attributes_types + super.merge({"object_kind" => "string"}) + end + def object_kind if k = ArvadosModel::resource_class_for_uuid(object_uuid) k.kind diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb index bf27f6f..bb5ab46 100644 --- a/services/api/app/models/node.rb +++ b/services/api/app/models/node.rb @@ -39,6 +39,13 @@ class Node < ArvadosModel t.add lambda { |x| @@nameservers }, :as => :nameservers end + def self.attributes_types + super.merge({ "crunch_worker_state" => "string", + "nameservers" => "Array", + "status" => "string", + }) + end + def domain super || @@domain end diff --git a/services/api/app/models/repository.rb b/services/api/app/models/repository.rb index 1f14f59..ab0e574 100644 --- a/services/api/app/models/repository.rb +++ b/services/api/app/models/repository.rb @@ -19,6 +19,10 @@ class Repository < ArvadosModel super.merge({"push_url" => ["name"], "fetch_url" => ["name"]}) end + def self.attributes_types + super.merge({"fetch_url" => "string", "push_url" => "string"}) + end + def push_url "git@git.%s.arvadosapi.com:%s.git" % [Rails.configuration.uuid_prefix, name] end diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index fe5e07b..bca179e 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -57,6 +57,13 @@ class User < ArvadosModel ALL_PERMISSIONS = {read: true, write: true, manage: true} + def self.attributes_types + super.merge({ "full_name" => "string", + "is_invited" => "boolean", + "writable_by" => "Array", + }) + end + def full_name "#{first_name} #{last_name}".strip end diff --git a/services/api/test/functional/arvados/v1/schema_controller_test.rb b/services/api/test/functional/arvados/v1/schema_controller_test.rb index 520e36e..cbf7b43 100644 --- a/services/api/test/functional/arvados/v1/schema_controller_test.rb +++ b/services/api/test/functional/arvados/v1/schema_controller_test.rb @@ -20,4 +20,37 @@ class Arvados::V1::SchemaControllerTest < ActionController::TestCase assert_includes discovery_doc, 'defaultTrashLifetime' assert_equal discovery_doc['defaultTrashLifetime'], Rails.application.config.default_trash_lifetime end + + test "discovery document includes types for all API-accessible attributes" do + get :index + assert_response :success + schemas = json_response["schemas"] + ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |klass| + klass_name = klass.to_s + next if klass_name.start_with?("Commit") + klass_schema = schemas[klass_name].andand["properties"] + refute_nil(klass_schema, "no schema for #{klass}") + attr_names = klass.methods. + grep(/^api_accessible_\w+$/). + flat_map do |method_name| + if method_name == :api_accessible_attributes + [] + else + klass.send(method_name).keys + end + end.uniq.map(&:to_s).sort + schema_keys = klass_schema.keys.sort + assert_equal(attr_names, schema_keys, + "#{klass_name} has wrong attributes list") + assert_empty(klass_schema.keys. + select { |key| klass_schema[key]["type"].nil? }, + "#{klass_name} attributes missing type information") + writable_attrs = attr_names.reject { |n| klass.columns_hash[n].nil? } + assert_empty(writable_attrs.select { |k| klass_schema[k]["readonly"] }, + "writable attribute(s) marked readonly") + readonly_attrs = attr_names - writable_attrs + assert_empty(readonly_attrs.reject { |k| klass_schema[k]["readonly"] }, + "readonly attribute(s) not marked readonly") + end + end end