The Refactoring
I mentioned yesterday that I wanted to refactor the ternary operation in the ldap_con#search
call so today I used extract method to pull it out.
Before
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
# app/models/auth_source_ldap.rb class AuthSourceLdap self.base_dn, :filter => object_filter & login_filter, # only ask for the DN if on-the-fly registration is disabled :attributes=> (onthefly_register? ? ['dn', self.attr_firstname, self.attr_lastname, self.attr_mail] : ['dn'])) do |entry| dn = entry.dn attrs = get_user_attributes_from_ldap_entry(entry) if onthefly_register? logger.debug "DN found for #{login}: #{dn}" if logger && logger.debug? end if authenticate_dn(dn, password) logger.debug "Authentication successful for '#{login}'" if logger && logger.debug? return attrs end rescue Net::LDAP::LdapError => text raise "LdapError: " + text end end |
After
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 |
# app/models/auth_source_ldap.rb class AuthSourceLdap self.base_dn, :filter => object_filter & login_filter, :attributes=> search_attributes) do |entry| dn = entry.dn attrs = get_user_attributes_from_ldap_entry(entry) if onthefly_register? logger.debug "DN found for #{login}: #{dn}" if logger && logger.debug? end if authenticate_dn(dn, password) logger.debug "Authentication successful for '#{login}'" if logger && logger.debug? return attrs end rescue Net::LDAP::LdapError => text raise "LdapError: " + text end # Return the attributes needed for the LDAP search. It will only # include the user attributes if on-the-fly registration is enabled def search_attributes if onthefly_register? ['dn', self.attr_firstname, self.attr_lastname, self.attr_mail] else ['dn'] end end end |
Review
With this refactor, I’m ready to conclude working on AuthSourceLdap
for now. With it cleaned up and well factored, it is now easier to implement new features that I have in progress. Like in cooking, it’s easier to create good things when the kitchen is clean.