On Thu, Apr 28, 2011 at 6:18 PM, Pierre-Arnaud Marcelot <[email protected]> wrote: > > On 28 avr. 2011, at 14:31, Kiran Ayyagari wrote: > >> 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) > > These methods would guaranty that the current object in the cursor is of the > requested type and avoid either getting a null reference or an exception when > getting the object via the getXXX() method. > > In the case you're requesting the next entry via nextEntry(), the cursor will > advance until it finds the next SearchResultEntry and discard any other > message type in between. > in the case of skipping the responses, yes, makes sense > This is only for convenience and to avoid having to test the current object's > type in the loop test like this, for a loop that's only interested in getting > entries: >> while ( cursor.next() && cursor.isCurrentEntry() ) >> { >> Entry entry = cursor.getEntry(); >> } > I like this approach rather than having a nextEntry() method, though not against it > >>> - 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) > > I don't know how it's done ATM and if it works, but if the previous() method > of the cursor work, it shouldn't be that difficult to implement as well. > we don't support the previous() methods atm >>> 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() > > +1, I was thinking the same thing. > It's much more convenient. > Just didn't want to influence by indicating it... ;) > > Regards, > Pierre-Arnaud
-- Kiran Ayyagari
