Grouper Developers Forum

[Chris Hyzer <>]

I made these changes to the Subject API, though I uncovered a dilemma.

 

While taking the liberty of formatting the code, and removing eclipse warnings of the subject API per the discussion below, I added generic declarations to all of the collection types.  It turns out I believe that Subject.getAttributes() should return a map of string keys and a multivalued string value (currently it is only the vague Map return type).  It should be: Map<String, Set<String>>.  The JDBC and JNDI builtin implementations do this ok.  However, the handful of Grouper implementations only return a Map<String, String> assuming that there are no multivalued subject attributes, and RegistrySubject does not really implement another method correctly either.

 

So, first of all if this is not the correct return type, or if we don’t support multivalued subject attributes, let me know and maybe this is off base.

 

If so, then I propose an easy solution though it might require people to change their code if they have a custom subject implementations…

 

1.       I suggest we make a BaseSubject just like we have a BaseSourceAdapter (and use it for our built-ins as examples).  If everyone who implements they source adapter has subclassed the BaseSourceAdapter, then they have no work to do to upgrade to the new finders, since I made methods in the abstract class (though they are deprecated, so at some point you need to change your code).  Also, I think there is shared logic so implementers don’t have to do that much…  but we can still code to the interface Subject.

2.       I think instead of having all Subjects return a map of String to Set<String> (since that would mean the subject object would probably keep its representation like that internally, which means a LOT of Set objects which are not needed), we should get rid of that method (getAttributes()).  I think it isn’t really used anyways, and if so, only to get a list of attribute names.

3.       We should add two methods: Set<String> getAttributeNames(), and boolean isAttributeMultiValued(attributeName)

4.       We can document that we assume getAttributeValue(attributeName) and getAttributeValues(attributeName) are only called when the attribute is known to be single or multivalued (should check if so)…  this way everything is as expected without extraneous Set’s.

 

In general, returning a set for an attribute value is dangerous anyway, because if it is not Unmodifiable, then the caller can use it to edit the subject’s attributes.  And if it is unmodifiable, then it is another layer to extraneous objects (though I guess getAttributeValues(attributeName) should return Unmodifiable sets)

 

Btw. I think the API and UI and WS (the main consumers ofSubject.getAttributes() would not even be affected by this change… see, I said it wasn’t heavily used… J

 

Thoughts?

 

Thanks,

Chris

 

Ps. At some point I would like to continue a conversation we had last year with the core committers about the value of lack of eclipse warnings, since some of the warnings are useful, and if there are too many, the useful ones are diluted and not seen…

 

 

From: [mailto:] On Behalf Of Tom Zeller
Sent: Tuesday, March 17, 2009 12:47 PM
To: Chris Hyzer
Cc:
Subject: Re: [grouper-dev] exception handling and finders

 

One open issue is should we do the same thing to the subject API (make exceptions unchecked, and finders which return 1 result should take a param as to whether or not null is an exception)?  I would have already except I believe it would change the interface of custom sources, so people would need to tweak them and recompile before upgrading to 1.5… is this ok?

The subject API seems simple enough that we should opt for consistency, i.e. make the subject API like grouper.

 

Should we roll the subject API into grouper as we did ldappc ?

 

TomZ