Skip to content

Commit

Permalink
Merge pull request #15430 from imtayadeway/api/render-compressed-ids
Browse files Browse the repository at this point in the history
Render ids in compressed form in API responses
  • Loading branch information
abellotti authored Jun 27, 2017
2 parents 65ae59e + c696161 commit 5dc1f11
Show file tree
Hide file tree
Showing 40 changed files with 275 additions and 263 deletions.
2 changes: 2 additions & 0 deletions app/controllers/api/base_controller/normalizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ def normalize_attr(attr, value)
normalize_array(value)
elsif value.respond_to?(:attributes) || value.respond_to?(:keys)
normalize_hash(attr, value)
elsif attr == "id" || attr.to_s.ends_with?("_id")
ApplicationRecord.compress_id(value)
elsif Api.time_attribute?(attr)
normalize_time(value)
elsif Api.url_attribute?(attr)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/base_controller/results.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ def add_parent_href_to_result(hash, parent_id = nil)
end

def add_task_to_result(hash, task_id)
hash[:task_id] = task_id
hash[:task_id] = ApplicationRecord.compress_id(task_id)
hash[:task_href] = task_href(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 => task_id, :href => task_href(task_id) }
{ :id => ApplicationRecord.compress_id(task_id), :href => task_href(task_id) }
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/requests/api/actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

expect(response).to have_http_status(:ok)

action_id = response.parsed_body["results"].first["id"]
action_id = ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"])

expect(MiqAction.exists?(action_id)).to be_truthy
end
Expand Down
7 changes: 4 additions & 3 deletions spec/requests/api/alert_definitions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching(alert_definitions_url(alert_definition.id)),
"id" => alert_definition.id,
"id" => alert_definition.compressed_id,
"description" => alert_definition.description,
"guid" => alert_definition.guid
)
Expand All @@ -74,7 +74,7 @@
api_basic_authorize collection_action_identifier(:alert_definitions, :create)
run_post(alert_definitions_url, sample_alert_definition)
expect(response).to have_http_status(:ok)
alert_definition = MiqAlert.find(response.parsed_body["results"].first["id"])
alert_definition = MiqAlert.find(ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"]))
expect(alert_definition).to be_truthy
expect(alert_definition.expression.class).to eq(MiqExpression)
expect(alert_definition.expression.exp).to eq(sample_alert_definition["expression"])
Expand Down Expand Up @@ -227,7 +227,8 @@
run_post(alert_definition_profiles_url, sample_alert_definition_profile)

expect(response).to have_http_status(:ok)
alert_definition_profile = MiqAlertSet.find(response.parsed_body["results"].first["id"])
id = ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"])
alert_definition_profile = MiqAlertSet.find(id)
expect(alert_definition_profile).to be_truthy
expect(response.parsed_body["results"].first).to include(
"description" => sample_alert_definition_profile["description"],
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 @@ -39,7 +39,7 @@
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(
"href" => a_string_matching(alerts_url(alert_status.id)),
"id" => alert_status.id
"id" => alert_status.compressed_id
)
end

Expand All @@ -50,7 +50,7 @@
let(:expected_assignee) do
{
'results' => a_collection_containing_exactly(
a_hash_including("assignee_id" => assignee.id)
a_hash_including("assignee_id" => assignee.compressed_id)
)
}
end
Expand Down Expand Up @@ -125,7 +125,7 @@
expect(response).to have_http_status(:ok)
expected = {
"results" => [
a_hash_including(attributes.merge("user_id" => User.current_user.id))
a_hash_including(attributes.merge("user_id" => User.current_user.compressed_id))
]
}
expect(response.parsed_body).to include(expected)
Expand All @@ -135,7 +135,7 @@
it "create an assignment alert action reference by id" do
attributes = {
"action_type" => "assign",
"assignee" => { "id" => assignee.id }
"assignee" => { "id" => assignee.compressed_id }
}
api_basic_authorize subcollection_action_identifier(:alerts, :alert_actions, :create, :post)
run_post(actions_subcollection_url, attributes)
Expand Down Expand Up @@ -178,9 +178,9 @@
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}"),
"id" => alert_action.id,
"id" => alert_action.compressed_id,
"action_type" => alert_action.action_type,
"user_id" => user.id,
"user_id" => user.compressed_id,
"comment" => alert_action.comment,
)
end
Expand Down
38 changes: 19 additions & 19 deletions spec/requests/api/authentications_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)
]
}
Expand Down Expand Up @@ -105,12 +105,12 @@
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
),
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)
]
}
Expand Down Expand Up @@ -138,7 +138,7 @@
a_hash_including(
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)
]
}
Expand All @@ -157,12 +157,12 @@
a_hash_including(
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
),
a_hash_including(
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)
]
}
Expand Down Expand Up @@ -224,7 +224,7 @@
'results' => [a_hash_including(
'success' => true,
'message' => 'Creating Authentication',
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)]
}
run_post(authentications_url, create_params)
Expand All @@ -241,12 +241,12 @@
a_hash_including(
'success' => true,
'message' => 'Creating Authentication',
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
),
a_hash_including(
'success' => true,
'message' => 'Creating Authentication',
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
)
]
}
Expand Down Expand Up @@ -292,16 +292,16 @@
a_hash_including(
'success' => true,
'message' => a_string_including("Refreshing Authentication id:#{auth.id}"),
'task_id' => a_kind_of(Numeric),
'task_id' => a_kind_of(String),
'task_href' => /task/,
'tasks' => [a_hash_including('id' => a_kind_of(Numeric), 'href' => /task/)]
'tasks' => [a_hash_including('id' => a_kind_of(String), 'href' => /task/)]
),
a_hash_including(
'success' => true,
'message' => a_string_including("Refreshing Authentication id:#{auth_2.id}"),
'task_id' => a_kind_of(Numeric),
'task_id' => a_kind_of(String),
'task_href' => /task/,
'tasks' => [a_hash_including('id' => a_kind_of(Numeric), 'href' => /task/)]
'tasks' => [a_hash_including('id' => a_kind_of(String), 'href' => /task/)]
)
]
}
Expand All @@ -326,7 +326,7 @@
expected = {
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
Expand All @@ -350,7 +350,7 @@
expected = {
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
Expand All @@ -373,7 +373,7 @@
expected = {
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
}
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
Expand All @@ -395,7 +395,7 @@
expected = {
'success' => true,
'message' => a_string_including('Updating Authentication'),
'task_id' => a_kind_of(Numeric)
'task_id' => a_kind_of(String)
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
Expand Down Expand Up @@ -439,9 +439,9 @@
expected = {
'success' => true,
'message' => /Refreshing Authentication/,
'task_id' => a_kind_of(Numeric),
'task_id' => a_kind_of(String),
'task_href' => /task/,
'tasks' => [a_hash_including('id' => a_kind_of(Numeric), 'href' => /tasks/)]
'tasks' => [a_hash_including('id' => a_kind_of(String), 'href' => /tasks/)]
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
Expand Down
8 changes: 4 additions & 4 deletions spec/requests/api/automation_requests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
expect_result_resources_to_include_keys("results", %w(id approval_state type request_type status options))
expect_results_to_match_hash("results", [expected_hash])

task_id = response.parsed_body["results"].first["id"]
task_id = ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"])
expect(AutomationRequest.exists?(task_id)).to be_truthy
end

Expand All @@ -48,7 +48,7 @@
expect_result_resources_to_include_keys("results", %w(id approval_state type request_type status options))
expect_results_to_match_hash("results", [expected_hash])

task_id = response.parsed_body["results"].first["id"]
task_id = ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"])
expect(AutomationRequest.exists?(task_id)).to be_truthy
end

Expand All @@ -61,7 +61,7 @@
expect_result_resources_to_include_keys("results", %w(id approval_state type request_type status options))
expect_results_to_match_hash("results", [expected_hash, expected_hash])

task_id1, task_id2 = response.parsed_body["results"].collect { |r| r["id"] }
task_id1, task_id2 = response.parsed_body["results"].collect { |r| ApplicationRecord.uncompress_id(r["id"]) }
expect(AutomationRequest.exists?(task_id1)).to be_truthy
expect(AutomationRequest.exists?(task_id2)).to be_truthy
end
Expand All @@ -84,7 +84,7 @@
run_post(automation_requests_url(automation_request.id), :action => "edit", :options => {:baz => "qux"})

expected = {
"id" => automation_request.id,
"id" => automation_request.compressed_id,
"options" => a_hash_including("foo" => "bar", "baz" => "qux")
}
expect(response).to have_http_status(:ok)
Expand Down
10 changes: 5 additions & 5 deletions spec/requests/api/blueprints_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@
expected = {
"results" => a_collection_containing_exactly(
a_hash_including(
"id" => blueprint1.id,
"id" => blueprint1.compressed_id,
"name" => "baz"
),
a_hash_including(
"id" => blueprint2.id,
"id" => blueprint2.compressed_id,
"name" => "qux"
)
)
Expand Down Expand Up @@ -222,8 +222,8 @@

expected = {
"results" => a_collection_containing_exactly(
a_hash_including("id" => blueprint1.id, "status" => "published"),
a_hash_including("id" => blueprint2.id, "status" => "published")
a_hash_including("id" => blueprint1.compressed_id, "status" => "published"),
a_hash_including("id" => blueprint2.compressed_id, "status" => "published")
)
}

Expand Down Expand Up @@ -281,7 +281,7 @@
run_post(blueprints_url(blueprint.id), :action => "publish")

expected = {
"id" => blueprint.id,
"id" => blueprint.compressed_id,
"status" => "published"
}
expect(response.parsed_body).to include(expected)
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/api/categories_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
response.parsed_body,
"description" => category.description,
"href" => categories_url(category.id),
"id" => category.id
"id" => category.compressed_id
)
expect(response).to have_http_status(:ok)
end
Expand Down Expand Up @@ -97,7 +97,7 @@
api_basic_authorize collection_action_identifier(:categories, :create)

run_post categories_url, :name => "test", :description => "Test"
category = Category.find(response.parsed_body["results"].first["id"])
category = Category.find(ApplicationRecord.uncompress_id(response.parsed_body["results"].first["id"]))

expect(category.tag.name).to eq("/managed/test")
end
Expand Down
10 changes: 5 additions & 5 deletions spec/requests/api/chargebacks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
response.parsed_body,
"description" => chargeback_rate.description,
"guid" => chargeback_rate.guid,
"id" => chargeback_rate.id,
"id" => chargeback_rate.compressed_id,
"href" => chargebacks_url(chargeback_rate.id)
)
expect(response).to have_http_status(:ok)
Expand Down Expand Up @@ -63,9 +63,9 @@

expect_result_to_match_hash(
response.parsed_body,
"chargeback_rate_id" => chargeback_rate.id,
"chargeback_rate_id" => chargeback_rate.compressed_id,
"href" => "#{chargebacks_url(chargeback_rate.id)}/rates/#{chargeback_rate_detail.to_param}",
"id" => chargeback_rate_detail.id,
"id" => chargeback_rate_detail.compressed_id,
"description" => "rate_1"
)
expect(response).to have_http_status(:ok)
Expand Down Expand Up @@ -93,7 +93,7 @@
expect_result_to_match_hash(
response.parsed_body,
"name" => currency.name,
"id" => currency.id,
"id" => currency.compressed_id,
"href" => "/api/currencies/#{currency.id}"
)
expect(response).to have_http_status(:ok)
Expand Down Expand Up @@ -121,7 +121,7 @@
expect_result_to_match_hash(
response.parsed_body,
"name" => measure.name,
"id" => measure.id,
"id" => measure.compressed_id,
"href" => "/api/measures/#{measure.id}",
)
expect(response).to have_http_status(:ok)
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/api/cloud_networks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
let(:providers_cloud_networks_url) { "#{provider_url}/cloud_networks" }

it 'queries Providers cloud_networks' do
cloud_network_ids = provider.cloud_networks.pluck(:id)
cloud_network_ids = provider.cloud_networks.select(:id).collect(&:compressed_id)
api_basic_authorize subcollection_action_identifier(:providers, :cloud_networks, :read, :get)

run_get providers_cloud_networks_url, :expand => 'resources'
Expand All @@ -49,7 +49,7 @@

run_get cloud_network_url

expect_single_resource_query('name' => network.name, 'id' => network.id, 'ems_ref' => network.ems_ref)
expect_single_resource_query('name' => network.name, 'id' => network.compressed_id, 'ems_ref' => network.ems_ref)
end

it "will not show the cloud network of a provider without the appropriate role" do
Expand Down
Loading

0 comments on commit 5dc1f11

Please sign in to comment.