Allow requesting specific attributes (derived from #102)

[pond]

This is derived from #102 with:

• Some minor commentary fixes, including some drive-bys from when I was reviewing existing docs as part of understanding Allow requesting specific attributes #102 and wider context
• A change in Allow requesting specific attributes #102's blanket removal of nil values. There's a small chance that some existing Scimitar clients might be relying on "key is present, but value is nil" in returned data, so nil items are now only stripped out if an include-attributes list is included; otherwise, nil handling behaviour is unchanged.

Many thanks to @xjunior for this work.

[sierra-alpha]

Just in regards to your comment

A change in #102 blanket removal of nil values. There's a small chance that some existing Scimitar clients might be relying on "key is present, but value is nil" in returned data, so nil items are now only stripped out if an include-attributes list is included; otherwise, nil handling behaviour is unchanged.

I don't see anything asserting this in the new tests, I assume it's already covered in existing tests hence it doesn't show in the diff?

Otherwise happy.

[pond]

@sierra-alpha

I don't see anything asserting this in the new tests

Yes, "missing" diffs can be tricky things! If you look at https://github.com/RIPAGlobal/scimitar/pull/102/files#diff-f82acfe1ea74bff57d9a8330817ac4f7ed9b8a027d75a1ff36c9c0dd47cf5408R330 you'll see that in #102, tests which did check for nil entries were updated to no longer do so. Those changes were reverted, since the nil entries are still expected. As a result, there's no alteration to those tests anymore, so nothing to show in the diff - which I suppose in a way, you'd expect, given that the behaviour is supposed to be unchanged unless an attribute inclusion list is specified.

[xjunior]

Hey, @pond. It's great to see this moving, thank you!

For the removed attributes, you're right. Nevertheless, JSON clients could assume a difference between a present and a null value.

I'll look into removing these attributes from the output JSON when attributes are requested, so we get a cleaner JSON when needed in a future PR, this was the main goal of this blanket removal of nil values, but I agree it wasn't the best solution.

[pond]

@xjunior

I'll look into removing these attributes from the output JSON when attributes are requested, so we get a cleaner JSON when needed in a future PR, this was the main goal of this blanket removal of nil values, but I agree it wasn't the best solution.

That's what I've implemented. The nil removal code is still there, it just only runs if an attribute map is included.

https://github.com/RIPAGlobal/scimitar/pull/127/files#diff-e137e2371ae195d13d92508cf5e64cedd5c379c72fd9387a2b9c814d6199c2ebR604

That's really (aside from some minor formatting changes) the only difference between this and your PR (which is the starting basis in this feature branch of the code here).

[xjunior]

Perfect, thanks @pond ! Looking forward to incorporate this on our project!