We should implement OID 1.3.6.1.4.1.4203.1.5.1 as a supportedFeature in 389-ds as per rfc3673.
https://tools.ietf.org/html/rfc3673
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1290111
Note: Check GER as this implements the extension already.
attachment 0001-Ticket-48363-Support-for-rfc3673-to-return-operation.patch
attachment 0001-Ticket-48363-Support-for-rfc3673-to-return-operation.2.patch
Thanks for the review Noriko.
I didn't have that issue with building, but you did correctly notice I was missing that header. I have added it now. I've also fixed up the other points you raised. Here is a diff of what I changed, and the rebased patch is attached to the thread.
{{{
diff --git a/ldap/servers/slapd/features.c b/ldap/servers/slapd/features.c index aade6c3..077002e 100644 --- a/ldap/servers/slapd/features.c +++ b/ldap/servers/slapd/features.c @@ -1,6 +1,5 @@ /* BEGIN COPYRIGHT BLOCK - * Copyright (C) 2001 Sun Microsystems, Inc. Used by permission. - * Copyright (C) 2005 Red Hat, Inc. + * Copyright (C) 2016 Red Hat, Inc. * All rights reserved. * License: GPL (version 3 or any later version). @@ -15,6 +14,7 @@
#include <stdio.h> #include "slap.h" +#include "slapi-plugin.h"
static char *supported_features = NULL; static Slapi_RWLock supported_features_lock = NULL; @@ -28,7 +28,7 @@ init_features( void ) "init_features: failed to create lock.\n"); exit(1); } - slapi_register_supported_feature( LDAP_FEATURE_RFC3673_ALLOPERATIONAL ); + slapi_register_supported_feature( LDAP_FEATURE_ALL_OP_ATTRS ); }
int diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 5361973..f66ccd0 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -5619,7 +5619,9 @@ char *slapi_get_supported_extended_ops_copy( void ); * routines for dealing with supported features /
-#define LDAP_FEATURE_RFC3673_ALLOPERATIONAL "1.3.6.1.4.1.4203.1.5.1" +#ifndef USE_OPENLDAP +#define LDAP_FEATURE_ALL_OP_ATTRS "1.3.6.1.4.1.4203.1.5.1" +#endif
int slapi_get_supported_features_copy( char ***ftroidsp );
}}}
Looks good.
It's a very minor thing, but this is more straightforward? {{{ +#ifndef LDAP_FEATURE_ALL_OP_ATTRS +#define LDAP_FEATURE_ALL_OP_ATTRS "1.3.6.1.4.1.4203.1.5.1" +#endif }}} For instance, this header "slapi-plugin.h" has lots of similar macros... {{{ / * filter types * openldap defines these, but not mozldap /
commit f4b97595525e07ca22b1e361edabf207d0dbba48
To ssh://git.fedorahosted.org/git/389/ds.git 1d4fcea..08c40e8 master -> master
Thanks!
git patch file (master) -- one line fix; symptom: setup ds fails with server's crash. 0001-Ticket-48363-Support-for-rfc3673-to-return-operation.3.patch
One line fix; pushed to master: 4e39b5b..0cf249e master -> master commit 0cf249e
attachment 0001-Ticket-48363-CI-test-add-test-suite.patch
Yep, the test passes for me.
I think that it should be named something like "dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py"
I also liked the comments in my test that referenced the rfc. Maybe you could transplant these?
Otherwise, functionality wise, I'm happy to ack, and if you want to make these other changes, that would be awesome.
William, thank you for the review. :) Applied.
To ssh://git.fedorahosted.org/git/389/ds.git 17f30c6..03c1e86 master -> master commit 03c1e86 Author: Simon Pichugin spichugi@redhat.com Date: Fri May 20 10:54:51 2016 +0200
Metadata Update from @firstyear: - Issue assigned to firstyear - Issue set to the milestone: 1.3.5 backlog
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1694
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Log in to comment on this ticket.