Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I'm working on a large project where we're making heavy use of the LDAP Integration project, especially the LDAP Sync component. I've been working against the CVS HEAD for the project and have made several enhancements and fixes along the way.
I started by posting a small patch against another patch that added support for the Content Profile module to LDAP Sync: http://drupal.org/node/646640#comment-3918612
At this point, I've made quite a number of enhancements / fixes and dissecting them will be rather difficult at this point. I've attached a patch containing the various fixes.
Issues
- create content profile object when synchronizing - behaves more like standard LDAP Data / Profile integration (LDAP Data / Content Profile integration)
- don't display content profile fields if content profile module not enabled (LDAP Data / Content Profile integration)
- fix markup issues in ldapdata.admin.inc, ~line 172 (LDAP Data)
- mis-spelling of modifications in ldapauth.admin.inc, line 38 (LDAP Auth)
- minor tweaks for Drupal coding standards compliance (via coder module)
New Features
- [because this was based on a previous patch, adds support for content profile]
- add support for all LDAPs to blocked users check - previously supported only Active Directory (LDAP Sync)
- add options for enabling specific synchronization features - allows user to synchronize only users or users + profile, etc.
- allow user to configure synchronization settings per LDAP server (LDAP Sync)
- remove 'warn' on blocked users and add 'report' only mode for all functionality (LDAP Sync)
- add option for creating new taxonomy terms if not existent (LDAP Data / Content Profile integration)
Performance Enhancements
- performance enhancement on removing unnecessary attributes from LDAP calls (LDAP Sync)
- group updates when updating blocked users (LDAP Sync)
- group content profile field changes when making updates (previous patch would call update once per field)
- only update content profile field if values changed (LDAP Sync / Content Profile integration)
This patch resolves several other open issues:
- http://drupal.org/node/1031426
- http://drupal.org/node/646640
- http://drupal.org/node/867356
- http://drupal.org/node/1012500
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | ldap_integration_performance_fixes.patch | 41.93 KB | pembertona |
| ldap_integration_performance_fixes.patch | 55.1 KB | pembertona |
Comments
Comment #1
pembertona CreditAttribution: pembertona commentedRemoved various whitespace changes to make the important changes more clear.
Comment #2
verta CreditAttribution: verta commentedSubscribing, will try patch when I have some time, thanks.
Comment #3
omaster CreditAttribution: omaster commentedSync doesn't work after patching. Nothing is found.
Comment #4
silid CreditAttribution: silid commentedI agree that sync doesn't work after applying this patch.
Also I think that when using content profile you should be able to map a field to the title of the node, not just the additional fields.
Thanks.
Comment #5
silid CreditAttribution: silid commentedI have debugged it.
Sync doesn't work when your name attribute has upper-case characters. So when using Active Directory and a name attribute of 'sAMAccountName' it fails.
I have changed one line in ldapsync.module and that seems to work.
From:
$name = drupal_strtolower($entry[$name_attr][0]);To:
$name = drupal_strtolower($entry[drupal_strtolower($name_attr)][0]);There are probably other places where the case is sensitive too.
Comment #6
drewish CreditAttribution: drewish commentedI'm not a maintainer on this module but I think it'd be better if you split this up into separate patches that focus on each of the issues you've found. Correcting the spelling of maintenance shouldn't be mixed in with adding support for content profile fields.
Comment #7
johnbarclay CreditAttribution: johnbarclay commentedgreat work. I'm trying to get a number of patches into 6.x-1.x-dev so we can get a beta3 out for some more serious testing. Since I just committed several longstanding patches, now is a good time to get these patches in.
Is anyone interested in breaking these down into patches by featureset against head?
Comment #8
pembertona CreditAttribution: pembertona commentedCompletely agree with @drewish that these should be broken down (as I hinted in the initial post). @johnbarclay and others: anyone step up to break them down?
Comment #9
johnbarclay CreditAttribution: johnbarclay commentedIf someone wants to step up and break them down that is fine. However, after applying alot of latent patches to head, my thinking is that given (1) the number of obvious bugs, performance issues, and straighforward code in this patch and that (2) Head has a lot of newly committed, but old patches. I think it may be wise to save some energy by
- rerolling this entire patch against head
- commit entire thing to head
There needs to be alot of testing between head and beta3, so I think its best to get the patches all in and get to the testing part. This is my thinking here also: #1237378: Version and Releases Status Updates
I applied this patch: http://drupal.org/node/646640#comment-3918612
Comment #10
cgmonroe CreditAttribution: cgmonroe commentedI'm marking this as postponed and a feature request because most of the fixes in the patch seemed to already be fixed via other ways/code.
However there are a few ideas here that someone might run want to create a patch for the latest / soon to be 6.x-1.0-RC1. The ldapsync per server and report features stand out for me.
For details of what's happened recently see: #1475272: 6.x-1.0 Release Candidate 1 Status
Comment #11
apadernoI am closing this issue, as Drupal 6 is no longer supported.