ZF-8233: New options in search function: sortDir and resultChangeCase

Issue Type:
Improvement
Created:
2009-11-05T09:27:41.000+0000
Last Updated:
2009-11-10T04:44:10.000+0000
Status:
Resolved
Fix version(s):
  • 1.10.0 (27/Jan/10)
Reporter:
Rodrigo Novelo Primolan (primolan)
Assignee:
Stefan Gehrig (sgehrig)
Tags:
  • Zend_Ldap
Related issues:
Attachments:

Description

I made two functions inside Zend_Ldap->search.


public function search($filter, $basedn = null, $scope = self::SEARCH_SCOPE_SUB,
        array $attributes = array(), $sort = null, $sortDir = null, $resultChangeCase = null, $collectionClass = null)

sortDir would be:


if (strtolower($sortDir) == 'desc') {
    array_reverse($result, true);
}

resultChangeCase would be:



$result = Zend_Ldap->search(...);

if ($resultChangeCase != null) {
    if (!in_array($resultChangeCase, array('strtolower' ,'strtoupper', 'ucfirst', 'ucwords'))) {
        throw new Exception('...');
    }
    array_walk_recursive($result, function(&$value, $key, $func) {
        $value = $func($value);
    }, $resultChangeCase);
}

I hope you guys enjoy it! :)

Comments

Posted by Rodrigo Novelo Primolan (primolan) on 2009-11-05T09:30:14.000+0000

Forgot to describe $result.

Posted by Stefan Gehrig (sgehrig) on 2009-11-05T10:04:40.000+0000

Changing the sort direction is a PHP-one-liner - I doubt it'll make sense to add another parameter to the already very long method signature of {{Zend_Ldap::search()}}. This functionality can easily be provided by user-land code.

Generall I like the idea of modifying the LDAP options properties' names but it won't be sufficient to provide this functionality in {{Zend_Ldap::search()}} - there are a lot more methods where a change would be necessary. I doubt that the work will be worth the effort as you won't gain anything new (LDAP attributes are case-insensitiv per se).

Posted by Rodrigo Novelo Primolan (primolan) on 2009-11-05T17:08:49.000+0000

As you said, it's a trivial case.

Still on the subject, i'd suggest that options should be in array. The reason is simple: if I need just some attributes, I'll need to fill every parameter till I get on attributes's parameter.

Considering that I'm not using any filters, default baseDn, default scope, I'll have to fill in three parameters till I get where I want.


public function search($filter, $basedn = null, $scope = self::SEARCH_SCOPE_SUB,
        array $attributes = array(), $sort = null, $sortDir = null, $resultChangeCase = null, $collectionClass = null)

And options being an array, you could consider my suggestions above. :)

Posted by Rodrigo Novelo Primolan (primolan) on 2009-11-05T17:14:05.000+0000

The new way would look like:


public function search(array(
    'attributes' => array('displayname', 'cn', 'dn', 'departament', 'description')
))

Much easier :)

Posted by Stefan Gehrig (sgehrig) on 2009-11-06T03:36:50.000+0000

Issue is reopened, but:

  • the option to change the attributes' names will not be added as the component favors lowercase attribute names in general
  • the option to reverse the sort direction cannot be added to {{Zend_Ldap::search()}} because the component relies solely on the LDAP server's capability to sort a result set. Results are retrieved sequentially from the server instead of instantiating the complete result set all at once. The option will be added {{to Zend_Ldap::searchEntries()}} as the whole result set is available here.

Both {{to Zend_Ldap::searchEntries()}} and {{Zend_Ldap::search()}} will be changed to accept an array parameter to remove the need to work through all the methods' parameters.

Posted by Stefan Gehrig (sgehrig) on 2009-11-06T04:06:24.000+0000

fixed in trunk (r18883): added {{$reverseSort}} argument to {{Zend_Ldap::searchEntries()}} added possibility to pass parameters to {{Zend_Ldap::searchEntries()}} and {{Zend_Ldap::search()}} in a single array argument

Posted by Rodrigo Novelo Primolan (primolan) on 2009-11-06T04:16:48.000+0000

Couldn't you add the 'resultChangeCase' function on {{Zend_Ldap::searchEntries()}} ?

Treating results' case in the source ({{Zend_Ldap::searchEntries()}}) would be better than treat one by one later. Considering that most of AD servers' entries aren't standardized, I think that it's better all come in one single case format (lower or upper for me) and then work upon results.

What do you think about it ? :)

Posted by Stefan Gehrig (sgehrig) on 2009-11-06T04:48:24.000+0000

Attribute names are already normalized to lower-case. I think there is no need to add this option just for the sake of cosmetics.