Return provider_class on provider requests

[jntullo]

Before:

GET /api/providers/:id?provider_class=provider
{
   "href": "/api/providers/:id".
   "actions": [
      { "href": "/api/providers/:id" }
  ]
}

After:

GET /api/providers/:id?provider_class=provider
{
   "href": "/api/providers/:id?provider_class=provider",
    "actions": [
      { "href": "/api/providers/:id?provider_class=provider" }
  ]
}

This PR resolves the issue where the provider_class is not being appended to the hrefs returned on both the object or actions. This could cause them to perform an action on a different object or get a different object in return.

@miq-bot add_label bug, api
@miq-bot assign @abellotti

Links

• BZ

[jntullo]

cc: @imtayadeway

[abellotti]

This might break some clients that expect the href to not include anything else (intent of our href). Is this a current issue reported ? what collection & collection_class specifically ?

clients calling our API would normally specify/add the provider_class in the URL in each request, this is how provider_class=provider for /api/providers is documented.

[miq-bot]

Checked commit jntullo@60e75da with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 🏆

[jntullo]

There is a BZ for this.

This will only add a suffix if ?provider_class=provider was a part of the request. If you run a get of a provider:

/api/providers/:id?provider_class=provider

The EDIT action has an href

/api/providers/:id

Which is a different object completely - or the object itself doesn't exist. Although the API client might add in the provider_class, I don't think we should send back misleading information or rely on them to do so. I can add in tests for other collections that there are concerns about breaking.

[abellotti]

hmm, ok. provider_class=provider would be the only concern, others are implemented for subclass refinement, so the original /api/:collection would hold true.

we need to play with the API client with this as it uses returned href's for accessing subsequent resources.

[abellotti]

Ok, I've tested this Fix with the API Client and it's a definite 👍

miq.vms.search_options(:provider_class => "provider").first.delete deleted the first Provider instance and not ExtMangementSystem. The href with the included ?provider_class was honored as is with the client while running the action.

Without this fix, the wrong provider would have been deleted.

Thanks @jntullo for fixing this 😍

[simaishi]

Fine backport details:

$ git log -1
commit f787f5f90c4208f52d8eb2913093e1480a400fcc
Author: Alberto Bellotti <[email protected]>
Date:   Fri Apr 7 16:21:26 2017 -0400

    Merge pull request #14657 from jntullo/bz/providers_href
    
    Return provider_class on provider requests
    (cherry picked from commit 58efc71e197464eb229fe3c425f9195cdbcb5502)

[simaishi]

@jntullo @abellotti Will it be a problem backporting this to Euwe and Darga as well (as per flags on the BZ)?

[abellotti]

@simaishi I don't think it's a straight backport, the PR touches files from a refactored API source base, so those releases will need separate PRs done for them.

[simaishi]

Backported to Euwe via #14866

[simaishi]

Backported to Darga via #14874