Ack from me, is there a chance of a lib389 testcase with this too?

LGTM. ack from me too

I updated the CI schema test, and I also converted create_test.py to work in python3

Please review one more time...

Thanks! Looks really good!

One thing, the objectClass is still not fixed in the python-ldap yet. So we can't test it properly here.
But the change will work for both attributeTypes and objectClasses.
So you can test it for attributeType and then you can add the next check in the end of the last test case:

    assert "'USER_DEFINED'" in str(myschema.query_objectclass("testoc"))
    assert "( 'two' 'user defined' )" in str(myschema.query_objectclass("testoctwo"))

So this kind of turned into a can of worms...

python-ldap's Attribute does not handle X-ORIGIN correctly. It's actually a multivalued key, and that's how DS uses it:

X-ORIGIN ( 'ACME Attr Desc' 'user defined' )

But python-ldap parses this in a single value (the first item): ACME Attr Desc We lose user defined This will break the current UI schema design if a custom attribute already sets its own unique X-ORIGIN value and we lose "user defined".

We would have to basically manage X-ORIGIN for both objectclasses and attributes. This will require us to add some ugly code to get it working correctly - we'll need to construct the "string" representation of the attr/oc object: adding parenthesis and dollar signs, etc, etc.

So we should get this fixed in python-ldap first @spichugi this might impact your PR as well (https://github.com/python-ldap/python-ldap/pull/247). As X-ORIGIN is multivalued so your PR should reflect that.

Okay, I mentioned it in the PR there.

The code here looks good to me. Ack.

Okay, I mentioned it in the PR there.
The code here looks good to me. Ack.

Actually this code does not work correctly. I have a new fix, but its all a hack IMO. I'd rather fix python-ldap than hack up lib389 code to work around it, but maybe we have to use it temporarily. Hmm okay well I might have a new rebase to review soon...

Well I got it working to match how python-ldap works today, but it's only accurate when you return the json object. A non-json request will not return the objectclass's x-origin. To do that we would have to do a huge rewrite in the schema code to work around the python-ldap bugs. I'd rather leave our code as is, and fix python-ldap for the correct/clean solution.

Please review one last time...

Okay, looks good to me. Ack.

P.S. actually, I didn't see that X-ORIGIN with more than one values is used anywhere in our server... But yeah, if we allow it in the source code we should process it correctly. Thank you!

Okay, looks good to me. Ack.
P.S. actually, I didn't see that X-ORIGIN with more than one values is used anywhere in our server... But yeah, if we allow it in the source code we should process it correctly. Thank you!

If you add schema to user99.ldif with X-ORIGIN set to something, then "user defined" will get appended when you do an ldapsearch

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 pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3064

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.