Allow requesting specific attributes

[xjunior]

• Remove nil attributes from response
• Allow to selectively serialize attributes with to_scim
• Request specific attributes through ActiveRecordBackedResourcesController

Closes #89

[xjunior]

I believe this method could be pushed up to ResourcesController, and let the specializations like ActiveRecordBackedResourcesController be responsible for supplying objects that respond to to_scim.

[xjunior]

Please, let me know if there's anything that I'm missing or could be better.

[xjunior]

Hello, @pond 😄 ! We're looking forward to having this feature in our SCIM endpoint. Do you think we could move forward with this PR?

[xjunior]

I'm really sorry to bother you with PR's, but we're looking to incorporate this feature in our SCIM server, and we might not be able to wait much longer. We're looking for feedback on this and perhaps move this work forward. Thank you

[xjunior]

Can I get a review on this PR?

[web-kat]

Hi @pond we would really love to use this feature in our SCIM implementation. How can we move forward with this PR?

[pond]

I've been too busy to get to this, but looking at it today.

Please do remember that anyone at any time can fork a gem with their own additions, to incorporate something that they have as a PR but is waiting on a maintainer. Your Gemfile just references the fork.

gem 'foo', git: 'https://github.com/myaccount/forked-gem-name.git', branch: 'something'

...so you don't need to wait on me to use a gem modification; you can in the short term use a fork or PR branch directly.

[xjunior]

@pond we'll come to a deadline next week, so I was wondering if you think this PR could make it until then? I believe you want to complete #109 first since they overlap.

[xjunior]

The other option is, of course, to fork meanwhile as you suggested. We were trying to avoid the overhead of doing it as much as we could hold.

[pond]

@xjunior

we'll come to a deadline next week, so I was wondering if you think this PR could make it until then

Well I can't be held to external third party unspecified deadlines, obviously. I misread this as being related to #109 originally - I'm juggling a great many things at the mo., sorry for the confusion if you saw the pre-edit version of this comment via a notification or similar.

I might be able to get to it - if we can clear #109, I can get this as next-cab-off-rank.

(That said I really don't see how getting a full JSON set back vs a small number of fields is particularly a deal-breaker, and always considered this a feature that's as likely to use enough CPU / RAM on the server side to be more expensive than bandwidth saved, assuming enough requests are coming in to make the bandwidth savings worthwhile).

[pond]

@xjunior OK, so this now has a lot of conflicts (as expected) after #109 went in.

I can't fix these as your new attributes parameter is undocumented. I don't know whether that's meant to be local data model attributes, SCIM attributes, if the latter whether that's described as dotted paths, arrays, or whatever, and whether it requires Symbols, Strings, etc...

...so, first thing, since you've added a new parameter to an already parameter-heavy back end method, please do take care to follow the existing commentary and indentation style to document your new parameter. Then we're in a position to assess how flexible that is, and how it should be applied to the heavily updated from-SCIM backend code.

(NB As is often said, the two hardest problems in computing are cache expiry, naming things and off-by-one errors. The parameter name attributes is very generic and confusing in the face of other very similar sounding attributes post-#109, so perhaps a bit of a rename is on the cards there, too?)

EDITED TO ADD: Yes, I see from reading tests and the reference to #89 that we're talking about https://datatracker.ietf.org/doc/html/rfc7644#section-3.10 representation here, but that needs documenting, String-vs-Symbol needs clarifying in the methods with the new parameter and I'm not sure if you handle schema prefixes in this patch. A good name by the way could be include_attributes since there's also not-done-in-this-PR excludedAttributes (https://datatracker.ietf.org/doc/html/rfc7644#section-3.9) and if we were to want to support that in future, include_attributes / exclude_attributes parameter naming in the lower levels would make life a lot easier.

[xjunior]

@pond I'll work on fixing these conflicts and following your suggestions, thank you!

[pond]

@xjunior Thanks. I don't mind this PR being a smaller implementation where e.g. it only handles non-schema prefix attributes etc., it's fine to take baby steps, but yeah defo please do the docs & have a think about parameter names with an eye to potential future changes.

I'll try to expedite a review, timezones permitting, once you give me a shout that it's ready for another look.

[xjunior]

I'm not sure if you handle schema prefixes in this patch

I did not. It seems by that prefixes are not required, but recommended:

clients MAY omit core schema attribute URN prefixes but SHOULD fully qualify extended attributes with the associated schema extension URN to avoid naming conflicts.
Section – 3.10

This implementation supports extension attributes, and the nature of the attribute mapping today doesn't allow a name conflict, but it does not support prefixing extension attributes with their schemas. Would that hold shipping this initial implementation?

[pond]

Merging this into a feature branch for further experimentation and hopefully inclusion in a release.