Since we are trying to use the correct int types, then shouldn't this be uint64_t?

Shouldn't we add the proper lib389 types here for filter rewriters as part of config, so that we can later expose them via the cli?

nitpick, you don't need to define a restart timeout anymore

You can define this in the for loop with for (size_t i ...) to prevent scope issues.

int types here thierry, :P

I'd like to see some other tests here, like invalid format sids, really long ones, too short, etc. We want to make sure that we test these edge cases :)

Otherwise it's looking good, thanks @tbordaz

The restart is to take into consideration the new rewriter (rewriters_init called by main). I think it would be possible to make it dynamic but I prefer to do it on a separate ticket if there is a need.

Anticipating this point ;). I had a doubt because 'i' is used in two loops. I can imagine the compiler will optimize this but I was not sure.
To make the code easier to read I will do that change, just stop me if it exists a concern to declare 'i' in two loops.

This is because 'rc' is the returned code of slapi-filter_apply that is a 'int'.

Where specifically would you implement rewriters in lib389 ?

That's not what I meant @tbordaz, you don't need the 60 in the restart, the timeout has a default value.

Also, you should never re-use an 'i' between loops incase you DONT reset it between them, so it's always better to have them seperate and per-loop.

We should comment about that int then I think ....

Where specifically would you implement rewriters in lib389 ?

Make a new rewriters.py similar to plugins I think?

I see that you allocate a memory here but I am not sure where you free it... Have you checked with ASAN?

Both test cases add cn=adfilter,cn=rewriters,cn=config and do some other stuff (like this line).
I checked and the second test case fails if we run the whole test suite as py.test dirsrvtests/tests/suites/rewriters/adfilter_test.py.

@spichugi you were right.. there was a leak ! Thanks for catching that. @firstyear I created a new lib389 rewriters library. Regarding running all the tests, the creation AD container/config entry is now in the lib389 rewriter and protected with try/except ALREADY_EXIST.

This doesn't seem right. Have a look at: https://pagure.io/389-ds-base/blob/master/f/src/lib389/lib389/idm/group.py#_26 for more. You should not need to call .add_s or Entry at all in a subclass of DSLdapObject. You should specify a "template" of what it looks like, and then calls to ".create" should work.

Similar here, you shouldn't need modify_s, you should be able to call self.replace I think.

Thank you sooo much @spichugi for your help !
Patch updated

Is this supposed to be commented out? If so, can it just be removed?

Oppsss I missed this one. Done

I think this is no longer needed as you can get the DN with get_schema_dn function (I think you need to fix def _create_user(inst, name, salt): dn = 'cn=%s,%s' % (name, schema_container) in the tests so schema_container is acquired using get_schema_dn.

The rest looks good to me! Thanks!

Thanks @spichugi , this is done

Great! You have my ack, at least:)

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/4120

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.