On Tue, Jan 9, 2018 at 6:11 PM, Craig Russell <apache....@gmail.com> wrote:
>
>> On Jan 8, 2018, at 11:55 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>
>> On Mon, Jan 8, 2018 at 11:34 PM, Craig Russell <apache....@gmail.com> wrote:
>>>
>>>> On Jan 8, 2018, at 7:32 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>>>
>>>> On Mon, Jan 8, 2018 at 7:07 PM, Craig Russell <apache....@gmail.com> wrote:
>>>>> /Users/clr/apache/git/whimsy/lib/whimsy/asf/ldap.rb:260:in `modify':
>>>>> Object class violation (LDAP::ResultError)
>>>>>
>>>>> And error reporting is not great. I guess more checking is needed but
>>>>> Object class violation is not very informative.
>>>>
>>>> Oh, and AGREED!
>>>>
>>>> LDAP sucks. You want to add zero members: Object class violation.
>>>> You want to add somebody who is already a member: Object class
>>>> violation. You want to remove somebody who is not a member: Object
>>>> class violation.
>>>>
>>>> That's why the action that caused the error is logged. In this case:
>>>>
>>>> LDAP_MOD_DELETE
>>>> {"member"=>[]}>
>>>> cn=pmc-chairs,ou=groups,ou=services,dc=apache,dc=org
>>>>
>>>> Here you are deleting nobody. That apparently is not allowed.
>>>
>>> Seems like there is some error checking that could be done in the ldap.rb
>>> code.
>>>
>>> Here is the remove code from lib/whimsy/asf/ldap.rb :
>>>
>>> # remove people from this service in LDAP
>>> def remove(people)
>>> @members = nil
>>> people = (Array(people) & members).map(&:dn)
>>> ASF::LDAP.modify(self.dn, [ASF::Base.mod_delete('member', people)])
>>> ensure
>>> @members = nil
>>> end
>>>
>>> Who wrote this code?
>>
>> That would be me.
>>
>>> Would it be possible for this code to check before calling ASF::LDAP.modify
>>> that the member actually exists already?
>>
>> The line above it takes the intersection between the list of people
>> passed to remove and the current set of members. So if a person who
>> is not a member of the service is passed in, they will be silently
>> removed from the set.
>
> Evidence suggests that the line and the corresponding line in Service.add are
> not working as intended.
>
> Where does the variable members come from? Is it different from @members that
> is set to nil just before trying to remove existing members?
members is a method call. Essentially, self.members().
https://whimsy.apache.org/docs/api/ASF/Service.html
@members is the cached value for this function. Setting it to nil
will force the members method to fetch new data from LDAP.
- Sam Ruby
>> I'm not sure how to refactor it, but there are add and remove methods
>> for a number of LDAP objects: Committee, Project, etc.
>
> I'm not sure that a refactoring is needed, just testing to make it work as
> intended.
>
> Craig
>>
>> - Sam Ruby
>>
>>> Craig
>>>
>>>> - Sam Ruby
>>>>
>>>> - Sam Ruby
>>>
>>> Craig L Russell
>>> Secretary, Apache Software Foundation
>>> c...@apache.org http://db.apache.org/jdo
>
> Craig L Russell
> Secretary, Apache Software Foundation
> c...@apache.org http://db.apache.org/jdo
>
