Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid constantize #1503

Merged
merged 4 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ def error(notice, message, status=400)
# handles finding an asset, and responding when it cannot be found. If it can be found the item instance is set (e.g. @project for projects_controller)
def find_requested_item
name = controller_name.singularize
object = name.camelize.constantize.find_by_id(params[:id])
object = controller_model.find_by_id(params[:id])
if object.nil?
respond_to do |format|
flash[:error] = "The #{name.humanize} does not exist!"
Expand Down Expand Up @@ -573,7 +573,7 @@ def get_parent_resource
parent_id_param = request.path_parameters.keys.detect { |k| k.to_s.end_with?('_id') }
if parent_id_param
parent_type = parent_id_param.to_s.chomp('_id')
parent_class = parent_type.camelize.constantize
parent_class = safe_class_lookup(parent_type.camelize)
if parent_class
@parent_resource = parent_class.find(params[parent_id_param])
end
Expand Down Expand Up @@ -670,5 +670,9 @@ def creator_related_params
]
end

def safe_class_lookup(class_name, raise: true)
Seek::Util.lookup_class(class_name, raise: raise)
end

helper_method :safe_class_lookup
end
2 changes: 1 addition & 1 deletion app/controllers/content_blobs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def asset_object
params.each do |param, value|
if param.end_with?('_id')
begin
c = param.chomp('_id').classify.constantize
c = safe_class_lookup(param.chomp('_id').classify)
rescue NameError
else
if c.method_defined?(:content_blob) || c.method_defined?(:content_blobs)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/custom_metadata_types_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def form_fields
format.html { render html: '' }
else
cm = CustomMetadataType.find(id)
resource = cm.supported_type.constantize.new
resource = safe_class_lookup(cm.supported_type).new
resource.custom_metadata = CustomMetadata.new(custom_metadata_type: cm)
format.html do
render partial: 'custom_metadata/custom_metadata_fields',
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/favourites_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def add

resource = saved_search if saved_search.save
else
resource = params[:resource_type].constantize.find_by_id(params[:resource_id])
resource = safe_class_lookup(params[:resource_type]).find_by_id(params[:resource_id])
end

favourite = Favourite.new(user: current_user, resource: resource)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/folders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def project_folders
end

def get_asset
@asset = params[:asset_type].constantize.find(params[:asset_id])
@asset = safe_class_lookup(params[:asset_type]).find(params[:asset_id])
unless @asset.can_view?
error("You cannot view the asset", "is invalid or not authorized")
end
Expand Down
24 changes: 12 additions & 12 deletions app/controllers/policies_controller.rb
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
class PoliciesController < ApplicationController
before_action :login_required

def send_policy_data
request_type = sanitized_text(params[:policy_type])
entity_type = sanitized_text(params[:entity_type])
entity_id = sanitized_text(params[:entity_id])

# NB! default policies now are only suppoted by Projects (but not Institutions / WorkGroups) -
# so supplying any other type apart from Project will cause the return error message
if request_type.downcase == "default" && entity_type == "Project"
if request_type.downcase == "default" && entity_type == "Project"
supported = true

# check that current user (the one sending AJAX request to get data from this handler)
# is a member of the project for which they try to get the default policy
authorized = current_person.projects.include? Project.find(entity_id)
else
supported = false
end

# only fetch all the policy/permissions settings if authorized to do so & only for request types that are supported
if supported && authorized
begin
entity = entity_type.constantize.find entity_id
entity = safe_class_lookup(entity_type).find entity_id
found_entity = true
policy = nil

if entity.default_policy
# associated default policy exists
found_exact_match = true
Expand All @@ -34,19 +34,19 @@ def send_policy_data
found_exact_match = false
policy = Policy.default()
end

rescue ActiveRecord::RecordNotFound
found_entity = false
end
end

respond_to do |format|
format.json {
if supported && authorized && found_entity
policy_settings = policy.get_settings
permission_settings = policy.get_permission_settings
render :json => {:status => 200, :found_exact_match => found_exact_match, :policy => policy_settings,

render :json => {:status => 200, :found_exact_match => found_exact_match, :policy => policy_settings,
:permission_count => permission_settings.length, :permissions => permission_settings }
elsif supported && authorized && !found_entity
render :json => {:status => 404, :error => "Couldn't find #{entity_type} with ID #{entity_id}."}
Expand All @@ -60,7 +60,7 @@ def send_policy_data
end

def preview_permissions
resource_class = params[:resource_name].camelize.constantize
resource_class = safe_class_lookup(params[:resource_name].camelize)
resource = nil
resource = resource_class.find_by_id(params[:resource_id]) if params[:resource_id]
resource ||= resource_class.new
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/publications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ def preprocess_pubmed_or_doi(pubmed_id, doi)

def create_or_update_associations(asset_ids, asset_type, required_action)
asset_ids.each do |id|
asset = asset_type.constantize.find_by_id(id)
asset = safe_class_lookup(asset_type).find_by_id(id)
if asset && asset.send("can_#{required_action}?")
@publication.associate(asset)
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def determine_sources
raise InvalidSearchException, "#{type} is not a valid search type"
end

sources = [type_name.constantize]
sources = [safe_class_lookup(type_name)]
end
sources
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/snapshots_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def destroy

def find_resource # This is hacky :(
resource, id = request.path.split('/')[1, 2]
@resource = resource.singularize.classify.constantize.find(id)
@resource = safe_class_lookup(resource.singularize.classify).find(id)
end

def auth_resource
Expand Down
22 changes: 2 additions & 20 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -418,24 +418,6 @@ def unable_to_delete_text(model_item)
no_deletion_explanation_message(model_item.class).html_safe
end

# returns a new instance of the string describing a resource type, or nil if it is not applicable
def instance_of_resource_type(resource_type)
resource = nil
begin
resource_class = resource_type.classify.constantize unless resource_type.nil?
resource = resource_class.send(:new) if !resource_class.nil? && resource_class.respond_to?(:new)
rescue NameError => e
logger.error("Unable to find constant for resource type #{resource_type}")
end
resource
end

# returns the class associated with the controller, e.g. DataFile for data_files
#
def klass_from_controller(c = controller_name)
c.singularize.camelize.constantize
end

def cancel_button(path, html_options = {})
html_options[:class] ||= ''
html_options[:class] << ' btn btn-default'
Expand Down Expand Up @@ -472,7 +454,7 @@ def show_form_manage_specific_attributes?
end

def pending_project_creation_request?
return false unless logged_in_and_registered?
return false unless logged_in_and_registered?
ProjectCreationMessageLog.pending_requests.collect do |log|
log.can_respond_project_creation_request?(User.current_user)
end.any?
Expand Down Expand Up @@ -554,4 +536,4 @@ def fancy_multiselect(association, options = {})

def cookie_consent
CookieConsent.new(cookies)
end
end
8 changes: 3 additions & 5 deletions app/helpers/sharing_permissions_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,12 @@ def add_permissions_to_tree_json (parent_node)
parent_node
end


def add_asset_permission_nodes (parent_node)

def add_asset_permission_nodes(parent_node)
asset_type = parent_node["id"].split("-")[0]
asset_id = parent_node["id"].split("-")[1].to_i

# get asset instance
asset = asset_type.camelize.constantize.find(asset_id)
asset = safe_class_lookup(asset_type.camelize).find(asset_id)
parent_node["text"] = "#{h(asset.title)} #{icon_link_to("", "new_window", asset , options = {target:'blank',class:'asset-icon',:onclick => 'window.open(this.href, "_blank");'})}"

permissions_array = get_permission(asset)
Expand Down Expand Up @@ -224,4 +222,4 @@ def unique_policy_node_id(object)
end


end
end
10 changes: 1 addition & 9 deletions app/models/custom_metadata_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,7 @@ def attributes_with_linked_custom_metadata_type

def supported_type_must_be_valid_type
return if supported_type.blank? # already convered by presence validation
valid = true
begin
clz = supported_type.constantize
# TODO: in the future to check it is a supported active record type
valid = clz.ancestors.include?(ActiveRecord::Base)
rescue NameError
valid = false
end
unless valid
unless Seek::Util.lookup_class(supported_type, raise: false)
errors.add(:supported_type, 'is not a type that can supported custom metadata')
end
end
Expand Down
3 changes: 2 additions & 1 deletion app/models/sample.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def referenced_resources
sample_type.sample_attributes.select(&:seek_resource?).map do |sa|
value = get_attribute_value(sa)
type = sa.sample_attribute_type.base_type_handler.type
Array.wrap(value).map { |v| type.constantize.find_by_id(v['id']) if v && type }
return [] unless type
Array.wrap(value).map { |v| type.find_by_id(v['id']) if v }
end.flatten.compact
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/stats/_snapshot_and_doi_stats.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<td><%= snapshots.count %></td>
<td>
<%= resources.count %>
(<%= ((resources.count / type_name.constantize.count.to_f) * 100).round(4) %>%)
(<%= ((resources.count / safe_class_lookup(type_name).count.to_f) * 100).round(4) %>%)
</td>
</tr>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/assets/_resource_counts.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<% if @active_filters&.any? -%>
<%= resource_text %> matching the given criteria:
<%= link_to("(Clear all filters)", page_and_sort_params.merge(filter: nil)) %>
<% elsif @total_count && klass_from_controller.authorization_supported? %>
<% elsif @total_count && controller_model.authorization_supported? %>
<%= resource_text %> visible to you, out of a total of <strong><%= @total_count %></strong>
<% else %>
<%= resource_text %> found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
Showing <%= type[:items_count] %> out of a possible <%= pluralize(resource_type_total_visible_count(type),'item') %>
<% end %>

<% if !type[:is_external] && type[:type].constantize.available_filters.any? %>
<% if !type[:is_external] && safe_class_lookup(type[:type]).available_filters.any? %>
<% if query %>

<%= link_to "Advanced #{type[:visible_resource_type]} search with filtering ...",
Expand Down
5 changes: 2 additions & 3 deletions app/views/assets/_select_tags.html.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
<%
entity=controller_name.singularize
object=instance_variable_get("@#{entity}")
model_class=entity.camelize.constantize
#FIXME: this is required for some cases where the page is rendered from an error, but the @entity doesn't exist. Mainly when rendering from a caught Exception AssetsCommon.handle_data
#the correct fix is to fix AssetsCommon.handle_data to correctly create an instance of @entity

object ||= model_class.new
object ||= controller_model.new
type_name = text_for_resource(object)

owned_annotations = current_user.annotations_by.collect{|a| a.value}
Expand All @@ -28,4 +27,4 @@
<% end %>
</p>
<%= render :partial=>"tags/select_tags", :locals=>{:all_tags=>all_tags, :owned_tags=>owned_annotations, :item_tags=>item_tags, :name=>"tag"} -%>
<% end %>
<% end %>
8 changes: 4 additions & 4 deletions app/views/assets/publishing/published.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@
if params[:published_items]
published_items |= params[:published_items].collect do |param|
item_class,item_id = param.split(',')
item_class.constantize.find_by_id(item_id)
safe_class_lookup(item_class).find_by_id(item_id)
end.compact.select(&:can_view?)
end
if params[:waiting_for_publish_items]
waiting_for_publish_items |= params[:waiting_for_publish_items].collect do |param|
item_class,item_id = param.split(',')
item_class.constantize.find_by_id(item_id)
safe_class_lookup(item_class).find_by_id(item_id)
end.compact.select(&:can_view?)
end
if params[:notified_items]
notified_items |= params[:notified_items].collect do |param|
item_class,item_id = param.split(',')
item_class.constantize.find_by_id(item_id)
safe_class_lookup(item_class).find_by_id(item_id)
end.compact.select(&:can_view?)
end

Expand Down Expand Up @@ -61,4 +61,4 @@

<% if @asset %>
<%= link_to 'Finished', @asset, :class=>"btn btn-primary" %>
<% end %>
<% end %>
4 changes: 2 additions & 2 deletions app/views/permissions/_preview_permissions.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%
resource_name = resource.class.name.underscore
permissions, privileged_people = uniq_people_permissions_and_privileged_people(policy.permissions.to_a, privileged_people)
downloadable = resource_name.camelize.constantize.new.try(:is_downloadable?)
downloadable = resource.is_downloadable?
effective_permissions = permissions.reject { |p| p.access_type <= policy.access_type }
sorted_permissions = effective_permissions.sort_by { |p| Permission::PRECEDENCE.index(p.contributor_type) }
grouped_contributors = group_by_access_type(sorted_permissions, privileged_people, downloadable)
Expand Down Expand Up @@ -42,7 +42,7 @@
<% [Policy::MANAGING, Policy::EDITING, downloadable ? Policy::ACCESSIBLE : nil, Policy::VISIBLE].compact.each do |access_type| %>
<% if grouped_contributors[access_type].try(:any?) %>
<div class="access-type-<%= PolicyHelper::access_type_key(access_type)-%>">
<p>The following can <strong><%= Policy.get_access_type_wording(access_type, resource_name.camelize.constantize.new.try(:is_downloadable?)).downcase -%></strong></p>
<p>The following can <strong><%= Policy.get_access_type_wording(access_type, downloadable).downcase -%></strong></p>
<ul>
<% grouped_contributors[access_type].each do |contributor| %>
<li><%= permission_title(contributor, member_prefix: true, icon: true) %></li>
Expand Down
2 changes: 1 addition & 1 deletion lib/seek/publishing/gatekeeper_publish.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def resolve_items_params(param)
return if param.nil?

param.keys.each do |asset_class|
klass = asset_class.constantize
klass = safe_class_lookup(asset_class)
param[asset_class].keys.each do |id|
asset = klass.find_by_id(id)
decision = param[asset_class][id]['decision']
Expand Down
4 changes: 2 additions & 2 deletions lib/seek/publishing/publishing_common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def waiting_approval_assets
end

def cancel_publishing_request
asset = params[:asset_class].classify.constantize.find(params[:asset_id])
asset = safe_class_lookup(params[:asset_class].classify).find(params[:asset_id])
if asset.can_manage?
if asset.last_publishing_log.publish_state.in?([ResourcePublishLog::WAITING_FOR_APPROVAL, ResourcePublishLog::REJECTED])
ResourcePublishLog.add_log(ResourcePublishLog::UNPUBLISHED, asset)
Expand Down Expand Up @@ -241,7 +241,7 @@ def resolve_publish_params(param)
else
assets = []
param.keys.each do |asset_class|
klass = asset_class.constantize
klass = safe_class_lookup(asset_class)
param[asset_class].keys.each do |id|
assets << klass.find_by_id(id)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Samples
module AttributeTypeHandlers
class SeekDataFileAttributeTypeHandler < SeekResourceAttributeTypeHandler
def type
'DataFile'
DataFile
end
end
end
Expand Down
Loading
Loading