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

Render links with compressed ids #15659

Merged
merged 2 commits into from
Aug 4, 2017
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
2 changes: 1 addition & 1 deletion app/controllers/api/base_controller/action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def api_action(type, id, options = {})

result = yield(klass) if block_given?

add_href_to_result(result, type, id) unless options[:skip_href]
add_href_to_result(result, type, compress_if_numeric_id(id)) unless options[:skip_href]
log_result(result)
result
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/base_controller/generic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def delete_resource_action(klass, type, id)
rescue => err
action_result(false, "#{err} - #{resource.errors.full_messages.join(', ')}")
end
add_href_to_result(result, type, id)
add_href_to_result(result, type, ApplicationRecord.compress_id(id))
log_result(result)
result
end
Expand All @@ -204,7 +204,7 @@ def invoke_custom_action(type, resource, action, data)
rescue => err
action_result(false, err.to_s)
end
add_href_to_result(result, type, resource.id)
add_href_to_result(result, type, resource.compressed_id)
log_result(result)
result
end
Expand All @@ -218,7 +218,7 @@ def invoke_custom_action_with_dialog(type, resource, action, data, custom_button
rescue => err
action_result(false, err.to_s)
end
add_href_to_result(result, type, resource.id)
add_href_to_result(result, type, resource.compressed_id)
log_result(result)
result
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/base_controller/normalizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def normalize_hash(type, obj, opts = {})
attrs = normalize_select_attributes(obj, opts)
result = {}

href = new_href(type, obj["id"], obj["href"])
href = new_href(compress_path(type), compress_if_numeric_id(obj["id"]), obj["href"])
if href.present?
result["href"] = href
attrs -= ["href"]
Expand Down Expand Up @@ -78,7 +78,7 @@ def normalize_href(type, value)
end

def subcollection_href(type, value)
normalize_url("#{@req.collection}/#{@req.c_id}/#{type}/#{value}")
normalize_url("#{@req.collection}/#{ApplicationRecord.compress_id(@req.c_id)}/#{type}/#{value}")
end

def collection_href(type, value)
Expand Down
20 changes: 18 additions & 2 deletions app/controllers/api/base_controller/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def collection_to_jbuilder(type, reftype, resources, opts = {})
if opts[:expand_resources]
add_hash json, resource_to_jbuilder(type, reftype, resource, opts).attributes!
else
json.href normalize_href(reftype, resource["id"])
json.href normalize_href(compress_path(reftype), ApplicationRecord.compress_id(resource["id"]))
end
end
end
Expand Down Expand Up @@ -398,7 +398,7 @@ def expand_subcollection(json, sc, sctype, subresources)
if @req.expand?(sc) || scr["id"].nil?
add_child js, normalize_hash(sctype, scr)
else
js.child! { |jsc| jsc.href normalize_href(sctype, scr["id"]) }
js.child! { |jsc| jsc.href normalize_href(sctype, ApplicationRecord.compress_id(scr["id"])) }
end
end
end
Expand Down Expand Up @@ -475,6 +475,22 @@ def render_options(resource, data = {})
klass = collection_class(resource)
render :json => OptionsSerializer.new(klass, data).serialize
end

def compress_path(path)
if path.to_s.split("/").size > 1
path.to_s.split("/").tap { |e| e[1] = ApplicationRecord.compress_id(e[1]) if e[1] =~ /\A[0-9]+\z/ }.join("/")
else
path
end
end

def compress_if_numeric_id(id)
if id.to_s =~ /\A[0-9]+\z/
ApplicationRecord.compress_id(id)
else
id
end
end
end
end
end
12 changes: 6 additions & 6 deletions app/controllers/api/base_controller/results.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def action_result(success, message = nil, options = {})
res[:result] = options[:result] unless options[:result].nil?
add_task_to_result(res, options[:task_id]) if options[:task_id].present?
add_tasks_to_result(res, options[:task_ids]) if options[:task_ids].present?
add_parent_href_to_result(res, options[:parent_id]) if options[:parent_id].present?
add_parent_href_to_result(res, ApplicationRecord.compress_id(options[:parent_id])) if options[:parent_id].present?
res
end

Expand All @@ -20,27 +20,27 @@ def add_href_to_result(hash, type, id)

def add_parent_href_to_result(hash, parent_id = nil)
return if hash[:href].present?
hash[:href] = "#{@req.api_prefix}/#{@req.collection}/#{parent_id ? parent_id : @req.c_id}"
hash[:href] = "#{@req.api_prefix}/#{@req.collection}/#{parent_id ? parent_id : ApplicationRecord.compress_id(@req.c_id)}"
hash
end

def add_task_to_result(hash, task_id)
hash[:task_id] = ApplicationRecord.compress_id(task_id)
hash[:task_href] = task_href(task_id)
hash[:task_href] = task_href(ApplicationRecord.compress_id(task_id))
hash
end

def add_tasks_to_result(hash, task_ids)
add_task_to_result(hash, task_ids.first)
hash[:tasks] = task_ids.collect do |task_id|
{ :id => ApplicationRecord.compress_id(task_id), :href => task_href(task_id) }
{ :id => ApplicationRecord.compress_id(task_id), :href => task_href(ApplicationRecord.compress_id(task_id)) }
end
end

def add_tag_to_result(hash, tag_spec)
hash[:tag_category] = tag_spec[:category] if tag_spec[:category].present?
hash[:tag_name] = tag_spec[:name] if tag_spec[:name].present?
hash[:tag_href] = "#{@req.api_prefix}/tags/#{tag_spec[:id]}" if tag_spec[:id].present?
hash[:tag_href] = "#{@req.api_prefix}/tags/#{ApplicationRecord.compress_id(tag_spec[:id])}" if tag_spec[:id].present?
hash
end

Expand All @@ -52,7 +52,7 @@ def add_subcollection_resource_to_result(hash, ctype, object)
return hash if object.blank?
ctype_pref = ctype.to_s.singularize
hash["#{ctype_pref}_id".to_sym] = object.id
hash["#{ctype_pref}_href".to_sym] = "#{@req.api_prefix}/#{ctype}/#{object.id}"
hash["#{ctype_pref}_href".to_sym] = "#{@req.api_prefix}/#{ctype}/#{object.compressed_id}"
hash
end

Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def run_report_result(success, message = nil, options = {})
res = {:success => success}
res[:message] = message if message.present?
add_parent_href_to_result(res)
add_report_result_to_result(res, options[:report_result_id]) if options[:report_result_id].present?
add_report_result_to_result(res, ApplicationRecord.compress_id(options[:report_result_id])) if options[:report_result_id].present?
add_task_to_result(res, options[:task_id]) if options[:task_id].present?
res
end
Expand Down Expand Up @@ -60,8 +60,8 @@ def schedule_reports(report, type, id, data)
desc = "scheduling of report #{report.id}"
schedule = report.add_schedule fetch_schedule_data(data)
res = action_result(true, desc)
add_report_schedule_to_result(res, schedule.id, report.id)
add_href_to_result(res, type, id)
add_report_schedule_to_result(res, schedule.compressed_id, report.compressed_id)
add_href_to_result(res, type, ApplicationRecord.compress_id(id))
res
rescue => err
action_result(false, err.to_s)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/services_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def invoke_reconfigure_dialog(type, svc, data = {})
rescue => err
action_result(false, err.to_s)
end
add_href_to_result(result, type, svc.id)
add_href_to_result(result, type, svc.compressed_id)
log_result(result)
result
end
Expand Down
4 changes: 2 additions & 2 deletions lib/api/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Utils
def self.build_href_slug(klass, id)
return unless id
collection = Api::CollectionConfig.new.name_for_subclass(klass)
"#{collection}/#{id}" if collection
"#{collection}/#{ApplicationRecord.compress_id(id)}" if collection
end

def self.resource_search_by_href_slug(href_slug, user = User.current_user)
Expand All @@ -16,7 +16,7 @@ def self.resource_search_by_href_slug(href_slug, user = User.current_user)
raise _("User must be defined") unless user

klass = collection_config.klass(collection)
Rbac.filtered_object(klass.find(id), :user => user, :class => klass)
Rbac.filtered_object(klass.find(ApplicationRecord.uncompress_id(id)), :user => user, :class => klass)
end
end
end
11 changes: 11 additions & 0 deletions spec/lib/api/utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,16 @@

expect(actual).to eq(nil)
end

it "can interpret slugs with compressed ids" do
owner_tenant = FactoryGirl.create(:tenant)
owner_group = FactoryGirl.create(:miq_group, :tenant => owner_tenant)
owner = FactoryGirl.create(:user, :miq_groups => [owner_group])
vm = FactoryGirl.create(:vm_vmware, :tenant => owner_tenant)

actual = described_class.resource_search_by_href_slug("vms/#{vm.compressed_id}", owner)

expect(actual).to eq(vm)
end
end
end
24 changes: 13 additions & 11 deletions spec/requests/api/alert_definitions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@
"subcount" => 2,
"resources" => a_collection_containing_exactly(
{
"href" => a_string_matching(alert_definitions_url(alert_definitions[0].id))
"href" => a_string_matching(alert_definitions_url(alert_definitions[0].compressed_id))
},
{
"href" => a_string_matching(alert_definitions_url(alert_definitions[1].id))
"href" => a_string_matching(alert_definitions_url(alert_definitions[1].compressed_id))
}
)
)
Expand All @@ -46,7 +46,7 @@
run_get(alert_definitions_url(alert_definition.id))
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching(alert_definitions_url(alert_definition.id)),
"href" => a_string_matching(alert_definitions_url(alert_definition.compressed_id)),
"id" => alert_definition.compressed_id,
"description" => alert_definition.description,
"guid" => alert_definition.guid
Expand Down Expand Up @@ -94,7 +94,7 @@
expect(response).to have_http_status(:ok)
expect_single_action_result(:success => true,
:message => "alert_definitions id: #{alert_definition.id} deleting",
:href => alert_definitions_url(alert_definition.id))
:href => alert_definitions_url(alert_definition.compressed_id))
end

it "deletes an alert definition via DELETE" do
Expand Down Expand Up @@ -168,9 +168,11 @@

expect(response).to have_http_status(:ok)
expect_query_result(:alert_definition_profiles, 2, 2)
expect_result_resources_to_include_hrefs("resources",
[alert_definition_profiles_url(alert_definition_profiles.first.id),
alert_definition_profiles_url(alert_definition_profiles.second.id)])
expect_result_resources_to_include_hrefs(
"resources",
[alert_definition_profiles_url(alert_definition_profiles.first.compressed_id),
alert_definition_profiles_url(alert_definition_profiles.second.compressed_id)]
)
end

it "reads an alert definition profile as a resource" do
Expand All @@ -180,7 +182,7 @@

expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching(alert_definition_profiles_url(alert_definition_profile.id)),
"href" => a_string_matching(alert_definition_profiles_url(alert_definition_profile.compressed_id)),
"description" => alert_definition_profile.description,
"guid" => alert_definition_profile.guid
)
Expand All @@ -196,8 +198,8 @@
expect(response).to have_http_status(:ok)
expect_result_resources_to_include_hrefs(
"resources",
["#{alert_definition_profiles_url}/#{alert_definition_profile.id}/alert_definitions/#{alert_definitions.first.id}",
"#{alert_definition_profiles_url}/#{alert_definition_profile.id}/alert_definitions/#{alert_definitions.first.id}"]
["#{alert_definition_profiles_url}/#{alert_definition_profile.compressed_id}/alert_definitions/#{alert_definitions.first.compressed_id}",
"#{alert_definition_profiles_url}/#{alert_definition_profile.compressed_id}/alert_definitions/#{alert_definitions.first.compressed_id}"]
)
end

Expand Down Expand Up @@ -244,7 +246,7 @@
expect(response).to have_http_status(:ok)
expect_single_action_result(:success => true,
:message => "alert_definition_profiles id: #{alert_definition_profile.id} deleting",
:href => alert_definition_profiles_url(alert_definition_profile.id))
:href => alert_definition_profiles_url(alert_definition_profile.compressed_id))
end

it "deletes an alert definition profile via DELETE" do
Expand Down
12 changes: 6 additions & 6 deletions spec/requests/api/alerts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
"subcount" => 2,
"resources" => [
{
"href" => a_string_matching(alerts_url(alert_statuses[0].id))
"href" => a_string_matching(alerts_url(alert_statuses[0].compressed_id))
},
{
"href" => a_string_matching(alerts_url(alert_statuses[1].id))
"href" => a_string_matching(alerts_url(alert_statuses[1].compressed_id))
}
]
)
Expand All @@ -38,7 +38,7 @@
run_get(alerts_url(alert_status.id))
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching(alerts_url(alert_status.id)),
"href" => a_string_matching(alerts_url(alert_status.compressed_id)),
"id" => alert_status.compressed_id
)
end
Expand Down Expand Up @@ -81,7 +81,7 @@
"subcount" => 1,
"resources" => [
{
"href" => a_string_matching("#{alerts_url(alert.id)}/alert_actions/#{alert_action.id}")
"href" => a_string_matching("#{alerts_url(alert.compressed_id)}/alert_actions/#{alert_action.compressed_id}")
}
]
)
Expand Down Expand Up @@ -146,7 +146,7 @@
it "create an assignment alert action reference by href" do
attributes = {
"action_type" => "assign",
"assignee" => { "href" => users_url(assignee.id) }
"assignee" => { "href" => users_url(assignee.compressed_id) }
}
api_basic_authorize subcollection_action_identifier(:alerts, :alert_actions, :create, :post)
run_post(actions_subcollection_url, attributes)
Expand Down Expand Up @@ -177,7 +177,7 @@
run_get("#{actions_subcollection_url}/#{alert_action.id}")
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching("#{alerts_url(alert.id)}/alert_actions/#{alert_action.id}"),
"href" => a_string_matching("#{alerts_url(alert.compressed_id)}/alert_actions/#{alert_action.compressed_id}"),
"id" => alert_action.compressed_id,
"action_type" => alert_action.action_type,
"user_id" => user.compressed_id,
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/api/authentications_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
'count' => 1,
'subcount' => 1,
'name' => 'authentications',
'resources' => [hash_including('href' => a_string_matching(authentications_url(auth.id)))]
'resources' => [hash_including('href' => a_string_matching(authentications_url(auth.compressed_id)))]
}
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
Expand All @@ -36,7 +36,7 @@
run_get(authentications_url(auth.id))

expected = {
'href' => a_string_matching(authentications_url(auth.id))
'href' => a_string_matching(authentications_url(auth.compressed_id))
}
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/api/automate_domains_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
expect_single_action_result(
:success => true,
:message => a_string_matching(/Refreshing Automate Domain .* from git repository/),
:href => automate_domains_url(git_domain.id)
:href => automate_domains_url(git_domain.compressed_id)
)
end

Expand Down
Loading