Bug 1461188 - Creating an ActivationKey with "contentOverrides": null results in a 400 status
Creating an ActivationKey with "contentOverrides": null results in a 400 status
Status: NEW
Product: Candlepin
Classification: Community
Component: candlepin (Show other bugs)
2.0
Unspecified Unspecified
low Severity low
: ---
: ---
Assigned To: candlepin-bugs
Katello QA List
: Triaged
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-13 14:58 EDT by Shayne Riley
Modified: 2017-06-19 11:16 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Shayne Riley 2017-06-13 14:58:15 EDT
Description of problem:

If I create an ActivationKey with ..."contentOverrides": null,... specified in the JSON body, the activation key will fail to be created and a 400 status is returned.

But, if this is omitted from the body entirely, then the activation key creation succeeds.


Version-Release number of selected component (if applicable):
2.0.36


How reproducible:
Always


Steps to Reproduce:
1. Pick an owner.
2. Attempt to create an activation key with null contentOverrides:

curl -vku $USER:$PASS -H 'Content-Type: application/json' -X POST --data '{ "name": "test", "contentOverrides": null}' $HOST/candlepin/owners/$OWNERKEY/activation_keys


Actual results:

{"displayMessage":"Runtime Error com.fasterxml.jackson.databind.JsonMappingException: N/A (through reference chain: org.candlepin.model.activationkeys.ActivationKey[\"contentOverrides\"]) at org.candlepin.model.activationkeys.ActivationKey.addContentOverrides:294","requestUuid":"c67f3c8c-25e3-41fc-9124-98bf73aa8ebc"}


Expected results:

One or the other would be nice:
1. The null field is ignored, and the activation key is created regardless.
2. A more helpful error message is presented, e.g. "contentOverrides cannot be null." This is kinda misleading though, since it *CAN* be null (not specifying it is the same as being null). Another reason I don't like this second option is that it goes against convention established everywhere else in Candlepin, which allow their fields to be specified as null in the JSON body, but are ignored.



Additional info:

A couple of workarounds:
- Don't specify the field at all, if it is null. This can be tricky to disable for certain Java JSON+REST libraries like RestEasy as it will by default pass the field.
- Specify an empty array instead: "contentOverrides": []


The easiest fix would be in ActivationKey#addContentOverrides, line 294, just before the loop. Add a null check:


    public void addContentOverrides(Collection<ActivationKeyContentOverride> overrides) {
        if (overrides == null) {
            return;
        }
        for (ActivationKeyContentOverride newOverride : overrides) {
            this.addContentOverride(newOverride);
        }
    }
Comment 1 Shayne Riley 2017-06-13 15:12:08 EDT
Specifying...

"releaseVer": null

... Also generates the same 400 status, but for a different field. The fix (check for null) is nearly the same, but for a different method. In ActivationKey#setReleaseVer, line 329:

    public void setReleaseVer(Release releaseVer) {
        if (releaseVer != null) {
            this.releaseVer = releaseVer.getReleaseVer();
        } else {
            this.releaseVer = null;
        }
    }
Comment 2 Kevin Howell 2017-06-19 10:18:43 EDT
Is this a difference in behavior between CP 2.0 and previous versions? If we don't make any changes, is this going to break anything?
Comment 3 Shayne Riley 2017-06-19 11:16:26 EDT
In Candlepin 0.9.51.24 if you attempt to create an activation key and specify...

"contentOverrides": null

...within the JSON, a 500 status is generated instead of the 400 status.


So it seems that this bug existed since before Candlepin 2.0. I'd argue that, despite this, it should be fixed anyway.

Note You need to log in before you can comment on or make changes to this bug.