directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kiran Ayyagari <kayyag...@apache.org>
Subject Re: Search result in LDAP API
Date Thu, 28 Apr 2011 12:56:30 GMT
On Thu, Apr 28, 2011 at 6:18 PM, Pierre-Arnaud Marcelot <pa@marcelot.net> wrote:
>
> On 28 avr. 2011, at 14:31, Kiran Ayyagari wrote:
>
>> On Thu, Apr 28, 2011 at 5:50 PM, Pierre-Arnaud Marcelot <pa@marcelot.net> wrote:
>>>
>>> On 28 avr. 2011, at 13:39, Kiran Ayyagari wrote:
>>>
>>>> On Thu, Apr 28, 2011 at 4:38 PM, Emmanuel Lecharny <elecharny@gmail.com>
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

Mime
View raw message