Adding Chassis and Racks as PhysicalStorages subcollections

[EsdrasVP]

This PR is able to add:

• GET /api/physical_storages/:c_id/physical_chassis
• GET /api/physical_storages/:c_id/physical_chassis/:s_id
• GET /api/physical_storages/:c_id/physical_racks
• GET /api/physical_storages/:c_id/physical_racks/:s_id

Depends on:

• ManageIQ/manageiq-schema#224 [Merged]
• ManageIQ/manageiq#17616 [Merged]
• ManageIQ/manageiq-providers-lenovo#201 [Merged]

[EsdrasVP]

@abellotti Could you check this out?

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[miq-bot]

Checked commit EsdrasVP@dac0186 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 👍

[abellotti]

can't this be object.physical_rack instead of the PhysicalRack.where ... ?

[EsdrasVP]

Actually no, because PhysicalRack has many PhysicalStorages and not the inverse. So if we call object.physical_rack the code will break. That's why, instead, we use the reference physical_rack_id which denotes the PhysicalRack in which the PhysicalStorage belongs to.

[bdunne]

@EsdrasVP What will break? It looks like the relationships are defined in both directions...

app/models/physical_chassis.rb:9:  belongs_to :physical_rack, :foreign_key => :physical_rack_id, :inverse_of => :physical_chassis
app/models/physical_rack.rb:4:  belongs_to :ext_management_system, :foreign_key => :ems_id, :inverse_of => :physical_racks,
app/models/physical_rack.rb:6:  has_many :physical_chassis, :dependent => :nullify, :inverse_of => :physical_rack
app/models/physical_rack.rb:7:  has_many :physical_servers, :dependent => :nullify, :inverse_of => :physical_rack
app/models/physical_rack.rb:8:  has_many :physical_storages, :dependent => :nullify, :inverse_of => :physical_rack
app/models/physical_server.rb:22:  belongs_to :physical_rack, :foreign_key => :physical_rack_id, :inverse_of => :physical_servers
app/models/physical_storage.rb:4:  belongs_to :physical_rack, :foreign_key => :physical_rack_id, :inverse_of => :physical_storages

[EsdrasVP]

@bdunne They have, PhysicalRack and PhysicalChassis will have many PhysicalStorages, but the problem is that PhysicalStorage will only belong to one PhysicalRack/PhysicalChassis. So what happens when I use object.physical_rack is that a method all is called to return a list of PhysicalRacks/PhysicalChassis when this list doesn't exist and only a object is returned. So when I access /api/physical_storages/:id/physical_racks, for instance, the following error is presented:

{
    "error": {
        "kind": "internal_server_error",
        "message": "undefined method `all' for #<PhysicalRack:0x007f1bf0c52098>",
        "klass": "NoMethodError"
    }
}

[bdunne]

Sorry, I must have missed the notification... It sounds like it should be /api/physical_storages/:id/physical_racks should be /api/physical_storages/:id/physical_rack. Maybe this should be a sub-resource rather than a sub-collection. Is that right @abellotti?

[abellotti]

if there's one and only one, the subcollection does not make sense here, it can currently be accessed via GET /api/physical_storages/:id?attributes=physical_rack
as shown by the OPTIONS /api/physical_storages in the relationships section.

[EsdrasVP]

if there's one and only one, the subcollection does not make sense here, it can currently be accessed via GET /api/physical_storages/:id?attributes=physical_rack
as shown by the OPTIONS /api/physical_storages in the relationships section.

@abellotti @bdunne This makes sense, I spoke with @rodneyhbrown7 and we agreed to use the current method to access the physical storage relationships. So this PR can be closed.

[abellotti]

same question as below

[EsdrasVP]

@abellotti Is there any other reservation regarding this?