On Thu, Apr 28, 2011 at 5:50 PM, Pierre-Arnaud Marcelot <[email protected]> wrote: > > On 28 avr. 2011, at 13:39, Kiran Ayyagari wrote: > >> On Thu, Apr 28, 2011 at 4:38 PM, Emmanuel Lecharny <[email protected]> >> wrote: >>> Hi guys, >>> >>> yesterday, as I started to write the doc about the Search Operation, I faced >>> some issue. Let me explain. >>> >>> When you do a simple search, you get back a cursor : >>> >>> SearchCursor cursor = connection.search( "ou=system", >>> "(objectclass=*)", SearchScope.ONELEVEL ); >>> >>> The SearchCursor extends Cursor<Response>. >>> >>> That means you get some Response when you do a search, so you have to write >>> such code to get back the entries : >>> >>> while ( cursor.next() ) >>> { >>> Entry entry = ((SearchResultEntry)(cursor.get())).getEntry(); >>> >>> which is just horrible. > > I completely agree. > We are creating this API to make the user's life easier and not embarrass him > with such constructions. > I'm pretty sure 90% of the users of the API won't know there are multiple > types of responses to search. > Most of them only expect entries. > > On the other hand, LDAP experts know everything about the spec and the API > should also ease their work and allow them to access everything. > >>> The reason is that the search can return three kinds of responses : >>> - normal entries >>> - search result done >>> - and referrals >>> >> there is one more, the IntermediateResponse >> >>> We can deal with the searchResultDone (and we do, you can call a >>> cursor.getSearchResultDone() when you quit the loop), but the referral >>> handling is a bit more special. >>> >>> We have many options to clean up the API here : >>> - first, we can consider that a cursor.get() will always return an entry (or >>> a SearchResultEntry). In this case, if the next result is a referral, we >>> have two cases. The first one, if the users have asked the API to chase >>> referrals, he will get back an entry, and we are fine. The other case is >>> when we don't chase referrals, and then we can just throw a >>> ReferralException, up to the client to catch this exception >> IMHO a cursor, should never assume about the type of elements it is >> serving beyond a certain level of abstraction >> (sadly in LDAP, as we know, the result set contains different types >> of objects for the same operation, >> But we have already made some changes in this way, like getting the >> SearchResultDone) >>> - second, we can expect the user to check the result type before grabbing >>> it. If it's a SearchResultEntry, then do a cursor.getEntry(), otherwise do a >>> cursor.getReferral(). >> this is exactly the way we wanted it to work, not just for referrals >> but any sort of search response, >> i.e the cursor returns the ResponseS (the super type) and it is upto >> the user to deal based on the specific subtype >> infact the above shown code snippet casts directly to a >> SearchResultEntry cause we know first hand that this search >> (perhaps done in a test case) will *only* return search entries and >> nothing else. >> >>> - third, we can cumulate all the referrals and ask the user to look on the >>> cursor a second time to deal with referral. We would have to store those >>> referrals internally to the cursor, and add a nextReferral() method. >>> >> IMO, this should be avoided by all means, cause in the worst case it >> will lead to a OOM >> >>> I personally find the first solution the least painful for our users. In any >>> case, we have to do two things : >>> - change the current API >>> - implement the Referral chasing in the API >>> >>> So, now, wdyt ? >> my preference would be to keep the get() method as it is and add new >> method(s) like getEntry() , getReferral() >> and user can call the respective method based on his requirement, and >> these method can throw >> an exception when the expected type is not compatible >> e.x NotAnEntryException when getEntry() is called but the current >> result is a referral > > This would be my preference as well (with another method to access the > IntermediateResponse too). > > But I would add other convenient methods like: > - boolean nextEntry(); > - boolean nextReferral(); > - boolean nextIntermediate(); hmm, we can just use next() method alone, cause all that we need is to read the next response (OTOH nextEntry/Referral() might confuse users to think that the cursor will skip to next avalable entry in the responses) > - boolean previousEntry(); > - boolean previousReferral(); > - boolean previousIntermediate(); we cannot support these methods unless we store them in the cursor, but as we know, maintaining them in memory brings up other issues (GC, OOM) > This methods would allow the user to jump to the next/previous object of > their choice easily. > > For example, this snippet would allow the user to get all entries from a > search cursor in a much nicer way than today: >> while ( cursor.nextEntry() ) we can just use - while ( cursor.next() ) >> { >> Entry entry = cursor.getEntry(); >> } > > Other methods to know what is the type of the current object in the cursor > would also be useful like: > - boolean isCurrentEntry(); > - boolean isCurrentReferral(); > - boolean isCurrentIntermediate(); +1, makes it more convenient > > Lastly, I'm wondering if the getXXX() methods should return an Entry/Referral > or the associated SearchResult instead > (SearchResultEntry/SearchResultReference). > Thoughts? > just the plain Entry/Referral/IntermediateResponse objects directly, otherwise caller has to make another call like SearchResultEntry.getEntry()
-- Kiran Ayyagari
