Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

unify count filters and introduce display name attribute detection #11837

Merged
merged 14 commits into from
Nov 24, 2014
Merged

unify count filters and introduce display name attribute detection #11837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cb3af1d
detect user display name attribute and return user count depending on…
blizzz Oct 24, 2014
f725cc6
consolidate user count filter in wizard and user back end
blizzz Oct 27, 2014
71944a5
detectors (email, displayname..) are now started in one place, trigge…
blizzz Oct 28, 2014
f9b4f5f
to reassure that selected attributes still work, do not count all mat…
blizzz Oct 29, 2014
4a3fe42
a corrected email attribute needs to be saved, not only returned
blizzz Oct 29, 2014
c9e8656
JS doc
blizzz Oct 29, 2014
d8bb8af
this happens already before counting, no need anymore
blizzz Oct 29, 2014
0e6d471
use underscore.js for undefined checks
blizzz Oct 29, 2014
031d6c1
better readbility, no effective changes
blizzz Oct 29, 2014
6b6147d
phpdoc and mixed up letters
blizzz Nov 20, 2014
b5e707b
trigger count on the correct filter
blizzz Nov 20, 2014
c07c338
fix counting when ldapPagingSize is 0
blizzz Nov 21, 2014
503de94
make updateCount work properly with new xp'd mode as well as without
blizzz Nov 21, 2014
9aef83b
make scrutinizer happier and always count users on assisted mode, eve…
blizzz Nov 21, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/user_ldap/ajax/wizard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
case 'guessPortAndTLS':
case 'guessBaseDN':
case 'detectEmailAttribute':
case 'detectUserDisplayNameAttribute':
case 'determineGroupMemberAssoc':
case 'determineUserObjectClasses':
case 'determineGroupObjectClasses':
Expand Down Expand Up @@ -115,4 +116,3 @@
//TODO: return 4xx error
break;
}

63 changes: 49 additions & 14 deletions apps/user_ldap/js/ldapFilter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ function LdapFilter(target, determineModeCallback) {
this.determineModeCallback = determineModeCallback;
this.foundFeatures = false;
this.activated = false;
this.countPending = false;

if( target === 'User' ||
target === 'Login' ||
Expand All @@ -25,9 +26,13 @@ LdapFilter.prototype.activate = function() {
this.determineMode();
};

LdapFilter.prototype.compose = function(callback) {
LdapFilter.prototype.compose = function(updateCount) {
var action;

if(updateCount === true) {
this.countPending = updateCount;
}

if(this.locked) {
this.lazyRunCompose = true;
return false;
Expand All @@ -54,22 +59,36 @@ LdapFilter.prototype.compose = function(callback) {

LdapWizard.ajax(param,
function(result) {
LdapWizard.applyChanges(result);
filter.updateCount();
if(filter.target === 'Group') {
LdapWizard.detectGroupMemberAssoc();
}
if(typeof callback !== 'undefined') {
callback();
}
filter.afterComposeSuccess(result);
},
function () {
filter.countPending = false;
console.log('LDAP Wizard: could not compose filter. '+
'Please check owncloud.log');
}
);
};

/**
* this function is triggered after attribute detectors have completed in
* LdapWizard
*/
LdapFilter.prototype.afterDetectorsRan = function() {
this.updateCount();
};

/**
* this function is triggered after LDAP filters have been composed successfully
* @param {object} result returned by the ajax call
*/
LdapFilter.prototype.afterComposeSuccess = function(result) {
LdapWizard.applyChanges(result);
if(this.countPending) {
this.countPending = false;
this.updateCount();
}
};

LdapFilter.prototype.determineMode = function() {
var param = 'action=get'+encodeURIComponent(this.target)+'FilterMode'+
'&ldap_serverconfig_chooser='+
Expand Down Expand Up @@ -145,10 +164,26 @@ LdapFilter.prototype.findFeatures = function() {
}
};

/**
* this function is triggered before user and group counts are executed
* resolving the passed status variable will fire up counting
* @param {object} status an instance of $.Deferred
*/
LdapFilter.prototype.beforeUpdateCount = function() {
var status = $.Deferred();
LdapWizard.runDetectors(this.target, function() {
status.resolve();
});
return status;
};

LdapFilter.prototype.updateCount = function(doneCallback) {
if(this.target === 'User') {
LdapWizard.countUsers(doneCallback);
} else if (this.target === 'Group') {
LdapWizard.countGroups(doneCallback);
}
var filter = this;
$.when(this.beforeUpdateCount()).done(function() {
if(filter.target === 'User') {
LdapWizard.countUsers(doneCallback);
} else if (filter.target === 'Group') {
LdapWizard.countGroups(doneCallback);
}
});
};
117 changes: 90 additions & 27 deletions apps/user_ldap/js/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,10 @@ var LdapWizard = {
ajaxRequests: {},

ajax: function(param, fnOnSuccess, fnOnError, reqID) {
if(reqID !== undefined) {
if(!_.isUndefined(reqID)) {
if(LdapWizard.ajaxRequests.hasOwnProperty(reqID)) {
console.log('aborting ' + reqID);
console.log(param);
LdapWizard.ajaxRequests[reqID].abort();
}
}
Expand All @@ -167,9 +169,10 @@ var LdapWizard = {
}
}
);
if(reqID !== undefined) {
if(!_.isUndefined(reqID)) {
LdapWizard.ajaxRequests[reqID] = request;
}
return request;
},

applyChanges: function (result) {
Expand Down Expand Up @@ -342,7 +345,7 @@ var LdapWizard = {
},

_countThings: function(method, spinnerID, doneCallback) {
param = 'action='+method+
var param = 'action='+method+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use OC.buildQueryString({action: method, ...}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but then i need to do it everywhere. Maybe not in a bugfix PR.

'&ldap_serverconfig_chooser='+
encodeURIComponent($('#ldap_serverconfig_chooser').val());

Expand All @@ -351,14 +354,14 @@ var LdapWizard = {
function(result) {
LdapWizard.applyChanges(result);
LdapWizard.hideSpinner(spinnerID);
if(doneCallback !== undefined) {
if(!_.isUndefined(doneCallback)) {
doneCallback(method);
}
},
function (result) {
OC.Notification.show('Counting the entries failed with, ' + result.message);
LdapWizard.hideSpinner(spinnerID);
if(doneCallback !== undefined) {
if(!_.isUndefined(doneCallback)) {
doneCallback(method);
}
},
Expand All @@ -374,20 +377,62 @@ var LdapWizard = {
LdapWizard._countThings('countUsers', '#ldap_user_count', doneCallback);
},

/**
* called after detectors have run
* @callback runDetectorsCallback
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems somewhat lonely and without context to me that comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how JS doc works :) This defines the runDetectorsCallback used in the following method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Good to know. Black magic that is. ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/**
* runs detectors to determine appropriate attributes, e.g. displayName
* @param {string} type either "User" or "Group"
* @param {runDetectorsCallback} triggered after all detectors have completed
*/
runDetectors: function(type, callback) {
if(type === 'Group') {
$.when(LdapWizard.detectGroupMemberAssoc())
.then(callback, callback);
if( LdapWizard.admin.isExperienced
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to gather all detectors in two arrays ?
One array for the XP detectors and one for the non-XP detectors ?
Then just run them in sequence/parallel ?
(my understanding of detectors is currently not high enough to be able to say whether it makes sense or not).

At least it would remove the need for all these ifs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detectors do not depend on experienced mode or not, they are the same. Only in the later case it is not guaranteed that they are triggered, because this is only on demand, but having the detectors running is important for a proper configuration.

&& !(LdapWizard.detectorsRunInXPMode & LdapWizard.groupDetectors)) {
LdapWizard.detectorsRunInXPMode += LdapWizard.groupDetectors;
}
} else if(type === 'User') {
var req1 = LdapWizard.detectUserDisplayNameAttribute();
var req2 = LdapWizard.detectEmailAttribute();
$.when(req1, req2)
.then(callback, callback);
if( LdapWizard.admin.isExperienced
&& !(LdapWizard.detectorsRunInXPMode & LdapWizard.userDetectors)) {
LdapWizard.detectorsRunInXPMode += LdapWizard.userDetectors;
}
}
},

/**
* runs detector to find out a fitting user display name attribute
*/
detectUserDisplayNameAttribute: function() {
var param = 'action=detectUserDisplayNameAttribute' +
'&ldap_serverconfig_chooser='+
encodeURIComponent($('#ldap_serverconfig_chooser').val());

//runs in the background, no callbacks necessary
return LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectUserDisplayNameAttribute');
},

detectEmailAttribute: function() {
param = 'action=detectEmailAttribute'+
var param = 'action=detectEmailAttribute'+
'&ldap_serverconfig_chooser='+
encodeURIComponent($('#ldap_serverconfig_chooser').val());
//runs in the background, no callbacks necessary
LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectEmailAttribute');
return LdapWizard.ajax(param, LdapWizard.applyChanges, function(){}, 'detectEmailAttribute');
},

detectGroupMemberAssoc: function() {
param = 'action=determineGroupMemberAssoc'+
'&ldap_serverconfig_chooser='+
encodeURIComponent($('#ldap_serverconfig_chooser').val());

LdapWizard.ajax(param,
return LdapWizard.ajax(param,
function(result) {
//pure background story
},
Expand All @@ -409,7 +454,7 @@ var LdapWizard = {
$('#ldap_loginfilter_attributes').find('option').remove();
for (var i in result.options['ldap_loginfilter_attributes']) {
//FIXME: move HTML into template
attr = result.options['ldap_loginfilter_attributes'][i];
var attr = result.options['ldap_loginfilter_attributes'][i];
$('#ldap_loginfilter_attributes').append(
"<option value='"+attr+"'>"+attr+"</option>");
}
Expand Down Expand Up @@ -566,8 +611,12 @@ var LdapWizard = {
},

isConfigurationActiveControlLocked: true,
detectorsRunInXPMode: 0,
userDetectors: 1,
groupDetectors: 2,

init: function() {
LdapWizard.detectorsRunInXPMode = 0;
LdapWizard.instantiateFilters();
LdapWizard.admin.setExperienced($('#ldap_experienced_admin').is(':checked'));
LdapWizard.basicStatusCheck();
Expand Down Expand Up @@ -620,8 +669,9 @@ var LdapWizard = {
instantiateFilters: function() {
delete LdapWizard.userFilter;
LdapWizard.userFilter = new LdapFilter('User', function(mode) {
if(mode === LdapWizard.filterModeAssisted) {
LdapWizard.groupFilter.updateCount();
if( !LdapWizard.admin.isExperienced()
|| mode === LdapWizard.filterModeAssisted) {
LdapWizard.userFilter.updateCount();
}
LdapWizard.userFilter.findFeatures();
});
Expand All @@ -630,7 +680,6 @@ var LdapWizard = {
$('#ldap_user_count').text('');
LdapWizard.showSpinner('#rawUserFilterContainer .ldapGetEntryCount');
LdapWizard.userFilter.updateCount(LdapWizard.hideTestSpinner);
LdapWizard.detectEmailAttribute();
$('#ldap_user_count').removeClass('hidden');
});

Expand All @@ -641,7 +690,8 @@ var LdapWizard = {

delete LdapWizard.groupFilter;
LdapWizard.groupFilter = new LdapFilter('Group', function(mode) {
if(mode === LdapWizard.filterModeAssisted) {
if( !LdapWizard.admin.isExperienced()
|| mode === LdapWizard.filterModeAssisted) {
LdapWizard.groupFilter.updateCount();
}
LdapWizard.groupFilter.findFeatures();
Expand All @@ -651,7 +701,6 @@ var LdapWizard = {
$('#ldap_group_count').text('');
LdapWizard.showSpinner('#rawGroupFilterContainer .ldapGetEntryCount');
LdapWizard.groupFilter.updateCount(LdapWizard.hideTestSpinner);
LdapWizard.detectGroupMemberAssoc();
$('#ldap_group_count').removeClass('hidden');
});
},
Expand All @@ -668,7 +717,7 @@ var LdapWizard = {
postInitUserFilter: function() {
if(LdapWizard.userFilterObjectClassesHasRun &&
LdapWizard.userFilterAvailableGroupsHasRun) {
LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
LdapWizard.userFilter.compose();
}
},

Expand All @@ -679,7 +728,7 @@ var LdapWizard = {
//do not allow to switch tabs as long as a save process is active
return false;
}
newTabIndex = 0;
var newTabIndex = 0;
if(ui.newTab[0].id === '#ldapWizard2') {
LdapWizard.initUserFilter();
newTabIndex = 1;
Expand All @@ -691,9 +740,23 @@ var LdapWizard = {
newTabIndex = 3;
}

curTabIndex = $('#ldapSettings').tabs('option', 'active');
var curTabIndex = $('#ldapSettings').tabs('option', 'active');
if(curTabIndex >= 0 && curTabIndex <= 3) {
LdapWizard.controlUpdate(newTabIndex);
//run detectors in XP mode, when "Test Filter" button has not been
//clicked in order to make sure that email, displayname, member-
//group association attributes are properly set.
if( curTabIndex === 1
&& LdapWizard.admin.isExperienced
&& !(LdapWizard.detecorsRunInXPMode & LdapWizard.userDetectors)
) {
LdapWizard.runDetectors('User', function(){});
} else if( curTabIndex === 3
&& LdapWizard.admin.isExperienced
&& !(LdapWizard.detecorsRunInXPMode & LdapWizard.groupDetectors)
) {
LdapWizard.runDetectors('Group', function(){});
}
}
},

Expand All @@ -711,15 +774,15 @@ var LdapWizard = {
}
}

if(triggerObj.id === 'ldap_userlist_filter' && !LdapWizard.admin.isExperienced()) {
LdapWizard.detectEmailAttribute();
} else if(triggerObj.id === 'ldap_group_filter' && !LdapWizard.admin.isExperienced()) {
LdapWizard.detectGroupMemberAssoc();
}

if(triggerObj.id === 'ldap_loginfilter_username'
|| triggerObj.id === 'ldap_loginfilter_email') {
LdapWizard.loginFilter.compose();
} else if (!LdapWizard.admin.isExperienced()) {
if(triggerObj.id === 'ldap_userlist_filter') {
LdapWizard.userFilter.updateCount();
} else if (triggerObj.id === 'ldap_group_filter') {
LdapWizard.groupFilter.updateCount();
}
}

if($('#ldapSettings').tabs('option', 'active') == 0) {
Expand Down Expand Up @@ -749,7 +812,7 @@ var LdapWizard = {
LdapWizard._save($('#'+originalObj)[0], $.trim(values));
if(originalObj === 'ldap_userfilter_objectclass'
|| originalObj === 'ldap_userfilter_groups') {
LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
LdapWizard.userFilter.compose(true);
//when user filter is changed afterwards, login filter needs to
//be adjusted, too
if(!LdapWizard.loginFilter) {
Expand All @@ -760,7 +823,7 @@ var LdapWizard = {
LdapWizard.loginFilter.compose();
} else if(originalObj === 'ldap_groupfilter_objectclass'
|| originalObj === 'ldap_groupfilter_groups') {
LdapWizard.groupFilter.compose();
LdapWizard.groupFilter.compose(true);
}
},

Expand Down Expand Up @@ -830,10 +893,10 @@ var LdapWizard = {
LdapWizard._save({ id: modeKey }, LdapWizard.filterModeAssisted);
if(isUser) {
LdapWizard.blacklistRemove('ldap_userlist_filter');
LdapWizard.userFilter.compose(LdapWizard.detectEmailAttribute);
LdapWizard.userFilter.compose(true);
} else {
LdapWizard.blacklistRemove('ldap_group_filter');
LdapWizard.groupFilter.compose();
LdapWizard.groupFilter.compose(true);
}
}
},
Expand Down