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.

Sign up for GitHub

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

Revert backwards-compat changes to devices API #197

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Revert backwards-compat changes to devices API #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a12fef
Revert "Modify API to maintain support for current middleware."
jamesremuscat Mar 5, 2024
3e9f5b5
Update tests to expect new API format.
jamesremuscat Mar 12, 2024
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
24 changes: 4 additions & 20 deletions app/controllers/api/v1/devices_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ def destroy
private

def device_params
permitted_params.except(
:location, :details, :public_ips, :private_ips, :ssh_key, :login_user,
:volume_details
)
permitted_params.except(:location, :details)
end

def location_params
Expand All @@ -58,25 +55,12 @@ def location_params
end

def details_params
permitted_params.fetch(:details, {}).tap do |details|
if details.empty?
legacy_params = permitted_params.slice(
:public_ips,
:private_ips,
:ssh_key,
:login_user,
:volume_details
)
return legacy_params
end
end
permitted_params.fetch(:details, nil)
end

PERMITTED_PARAMS = [
"name", "description", "status", "cost",
"public_ips", "private_ips", "ssh_key", "login_user",
"location" => %w[rack_id start_u facing]
] << {metadata: {}, details: {}, volume_details: {}}
"name", "description", "status", "cost", "location" => %w[rack_id start_u facing]
] << {metadata: {}, details: {}}
def permitted_params
params.require(:device).permit(*PERMITTED_PARAMS)
end
Expand Down
24 changes: 4 additions & 20 deletions app/controllers/api/v1/nodes_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ def find_template
end

def device_params
permitted_params.except(:location, :template_id, :details,
:public_ips, :private_ips, :ssh_key, :login_user, :volume_details
)
permitted_params.except(:location, :template_id, :details)
end

def location_params
Expand All @@ -40,26 +38,12 @@ def location_params
end

def details_params
permitted_params.fetch(:details, {}).tap do |details|
if details.empty?
legacy_params = permitted_params.slice(
:public_ips,
:private_ips,
:ssh_key,
:login_user,
:volume_details
)
legacy_params[:type] = 'Device::ComputeDetails'
return legacy_params
end
end
permitted_params.fetch(:details, {})
end

PERMITTED_PARAMS = [
"name", "description", "status", "cost",
"public_ips", "private_ips", "ssh_key", "login_user",
"location" => %w[rack_id start_u facing],
] << {metadata: {}, details: {}, volume_details: {}}
"name", "description", "status", "cost", "location" => %w[rack_id start_u facing]
] << {metadata: {}, details: {}}

def permitted_params
params.require(:device).permit(*PERMITTED_PARAMS)
Expand Down
8 changes: 4 additions & 4 deletions app/views/api/v1/devices/show.rabl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ child(:template, if: @include_full_template_details) do
extends 'api/v1/templates/show'
end

glue :details do |details|
child details: :details do |details|
extends "api/v1/devices/details/#{details.class.name.split('::').last.underscore}"
#node :type do |details|
# details.class.name
#end
node :type do |details|
details.class.name
end
end

attribute :template_id, unless: @include_full_template_details
60 changes: 3 additions & 57 deletions spec/requests/api/v1/devices_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def send_request
expect(parsed_device["cost"]).to eq "#{'%.2f' % valid_attributes[:device][:cost]}"
expect(parsed_device["template"]["id"]).to eq device_template.id

parsed_details = parsed_device #['details']
parsed_details = parsed_device['details']
expect(parsed_details["public_ips"]).to eq valid_attributes[:device][:details][:public_ips]
expect(parsed_details["private_ips"]).to eq valid_attributes[:device][:details][:private_ips]
expect(parsed_details["ssh_key"]).to eq valid_attributes[:device][:details][:ssh_key]
Expand Down Expand Up @@ -249,60 +249,6 @@ def send_request
end
end

context "with legacy valid parameters" do
let(:attributes) {
{
device: {
name: device.name + "-updated",
metadata: device.metadata.merge("kate" => "kate"),
status: "ACTIVE",
cost: 99.99,
public_ips: "1.1.1.1",
private_ips: "2.2.2.2",
ssh_key: "abc",
login_user: "Billy Bob",
volume_details: { id: "abc" }
}
}
}

def send_request
patch url_under_test,
params: attributes,
headers: headers,
as: :json
end

it "renders a successful response" do
send_request
expect(response).to have_http_status :ok
end

it "updates the device" do
expect {
send_request
}.to change{ device.reload.updated_at }
end

it "includes the device in the response" do
expect(device.metadata).not_to eq attributes[:device][:metadata]

send_request

parsed_device = JSON.parse(response.body)
expect(parsed_device["name"]).to eq attributes[:device][:name]
expect(parsed_device["metadata"]).to eq attributes[:device][:metadata]
expect(parsed_device["status"]).to eq attributes[:device][:status]
expect(parsed_device["cost"]).to eq "#{'%.2f' % attributes[:device][:cost]}"
expect(parsed_device["template"]["id"]).to eq device_template.id
expect(parsed_device["public_ips"]).to eq attributes[:device][:public_ips]
expect(parsed_device["private_ips"]).to eq attributes[:device][:private_ips]
expect(parsed_device["ssh_key"]).to eq attributes[:device][:ssh_key]
expect(parsed_device["login_user"]).to eq attributes[:device][:login_user]
expect(parsed_device["volume_details"]["id"]).to eq attributes[:device][:volume_details][:id]
end
end

context "with invalid parameters" do
def send_request
patch url_under_test,
Expand Down Expand Up @@ -388,7 +334,7 @@ def send_request
expect(parsed_device["name"]).to eq attributes[:device][:name]
expect(parsed_device["status"]).to eq attributes[:device][:status]
expect(parsed_device["cost"]).to eq "#{'%.2f' % attributes[:device][:cost]}"
parsed_details = parsed_device # ['details'] after revertion of 6b8d3e9
parsed_details = parsed_device['details']
expect(parsed_details["mtu"]).to eq attributes[:device][:details][:mtu]
end
end
Expand Down Expand Up @@ -435,7 +381,7 @@ def send_request
parsed_device = JSON.parse(response.body)
expect(parsed_device["name"]).to eq attributes[:device][:name]
expect(parsed_device["status"]).to eq attributes[:device][:status]
parsed_details = parsed_device # ['details'] after revertion of 6b8d3e9
parsed_details = parsed_device['details']
expect(parsed_details["bootable"]).to eq attributes[:device][:details][:bootable]
end
end
Expand Down
76 changes: 3 additions & 73 deletions spec/requests/api/v1/nodes_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,6 @@
}
}
}
let(:legacy_valid_attributes) {
{
template_id: device_template.id,
device: {
name: "device-1",
description: "device-1 description",
location: {
rack_id: rack.id,
start_u: 1,
facing: 'f',
},
cost: 77.77,
public_ips: "1.1.1.1, 2.2.2.2",
private_ips: "3.3.3.3, 4.4.4.4",
ssh_key: "abc",
login_user: "Billy Bob",
volume_details: {id: "abc"},
metadata: { "foo" => "bar", "baz" => "qux" },
status: 'IN_PROGRESS',
}
}
}
let(:invalid_attributes) {
{
template_id: device_template.id,
Expand Down Expand Up @@ -117,7 +95,7 @@ def send_request
expect(parsed_device["template"]["id"]).to eq valid_attributes[:template_id]
expect(parsed_device["cost"]).to eq "#{'%.2f' % valid_attributes[:device][:cost]}"

parsed_details = parsed_device #["details"]
parsed_details = parsed_device["details"]
expect(parsed_details["public_ips"]).to eq valid_attributes[:device][:details][:public_ips]
expect(parsed_details["private_ips"]).to eq valid_attributes[:device][:details][:private_ips]
expect(parsed_details["ssh_key"]).to eq valid_attributes[:device][:details][:ssh_key]
Expand All @@ -126,54 +104,6 @@ def send_request
end
end

context "with legacy valid parameters" do
# Backwards-compatible layer until middleware can be updated with
# Device::Details changes
def send_request
post url_under_test,
params: legacy_valid_attributes,
headers: headers,
as: :json
end

it "creates a new device" do
expect {
send_request
}.to change(Device, :count).by(1)
end

it "renders a successful response" do
send_request
expect(response).to have_http_status :ok
end

it "includes the device in the response" do
send_request
parsed_device = JSON.parse(response.body)

expected_location = legacy_valid_attributes[:device][:location].merge(
rack_id: rack.id,
end_u: legacy_valid_attributes[:device][:location][:start_u] + device_template.height - 1,
type: "rack",
depth: device_template.depth,
).stringify_keys

expect(parsed_device["name"]).to eq legacy_valid_attributes[:device][:name]
expect(parsed_device["description"]).to eq legacy_valid_attributes[:device][:description]
expect(parsed_device["location"]).to eq expected_location
expect(parsed_device["metadata"]).to eq legacy_valid_attributes[:device][:metadata]
expect(parsed_device["status"]).to eq legacy_valid_attributes[:device][:status]
expect(parsed_device["template"]["id"]).to eq legacy_valid_attributes[:template_id]
expect(parsed_device["cost"]).to eq "#{'%.2f' % legacy_valid_attributes[:device][:cost]}"

expect(parsed_device["public_ips"]).to eq legacy_valid_attributes[:device][:public_ips]
expect(parsed_device["private_ips"]).to eq legacy_valid_attributes[:device][:private_ips]
expect(parsed_device["ssh_key"]).to eq legacy_valid_attributes[:device][:ssh_key]
expect(parsed_device["login_user"]).to eq legacy_valid_attributes[:device][:login_user]
expect(parsed_device["volume_details"]["id"]).to eq legacy_valid_attributes[:device][:volume_details][:id]
end
end

context "with invalid parameters" do
def send_request
post url_under_test,
Expand Down Expand Up @@ -256,7 +186,7 @@ def send_request
expect(parsed_device["template"]["id"]).to eq valid_attributes[:template_id]
expect(parsed_device["cost"]).to eq "#{'%.2f' % valid_attributes[:device][:cost]}"

parsed_details = parsed_device #["details"]
parsed_details = parsed_device["details"]
expect(parsed_details["mtu"]).to eq valid_attributes[:device][:details][:mtu]
expect(parsed_details["dns_domain"]).to eq valid_attributes[:device][:details][:dns_domain]
end
Expand Down Expand Up @@ -316,7 +246,7 @@ def send_request
expect(parsed_device["template"]["id"]).to eq valid_attributes[:template_id]
expect(parsed_device["cost"]).to eq "#{'%.2f' % valid_attributes[:device][:cost]}"

parsed_details = parsed_device #["details"]
parsed_details = parsed_device["details"]
expect(parsed_details["bootable"]).to eq valid_attributes[:device][:details][:bootable]
expect(parsed_details["size"]).to eq valid_attributes[:device][:details][:size]
end
Expand Down