Skip to content

Conversation

@the80srobot
Copy link

I changed the function extract-attribute to always return a vector of values.
Before this change, the function would check for the special case of exactly 1 value, in which
case it would just return the value itself. This cause inconsistency in attributes that
are normally arrays (e.g. a person's direct reports) but can sometimes have only a single member (e.g. a person with only one employee).

… value was returned.

Changed the function extract-attribute to always return a vector of values.
Before this change, the function would check for the special case of exactly 1 value, in which
case it would just return the value itself. This cause inconsistency in attributes that
are normally arrays (e.g. a person's direct reports) but can sometimes have only a single member
(e.g. a person with only one employee).
@bskinny
Copy link
Collaborator

bskinny commented Mar 20, 2016

@the80srobot: The majority of LDAP attributes are single-valued in the server schema. If we were to wrap everything in a vector, it would make the common case unwieldy. It's not ideal when a multi-valued attribute (in the schema) with a single value (in the db) for an attribute such as uniqueMember is returned as a String and then needs to be updated as a vector. The cn attribute is a counter example: It allows multiple values but most often contains just one, making the String value, as opposed to a vector of String, easier to work with. The LDAP SDKs don't take a side in the matter, simply letting the caller invoke getAttrValue or getAttrValues.

If we were to head down this path that you suggest, I would think we should look at the schema to determine when to return a vector of values. And to implement it, we would give the caller an option on search/get requests to request strict-schema: multi-valued attributes (defined in schema) are returned in vectors, integer syntax attributes (defined in schema) are converted to Number, etc.

aperiodic added a commit to aperiodic/clj-ldap that referenced this pull request Sep 21, 2016
bskinny added a commit that referenced this pull request Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants