> 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? > > 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