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.

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

[1.0.0] Add support for RFC-4533 style directory synchronization. #67

Merged
merged 42 commits into from
Jun 29, 2023

Conversation

ChadSikorra
Copy link
Contributor

@ChadSikorra ChadSikorra commented Jun 7, 2023

This is very much a work-in-progress PR. I started this effort years ago but never finished it. This represents the work that was done and has been slightly altered for the updates to PHP and some other changes. I want to rethink my approach in general, so this will likely go through a couple of iterations before I merge it into the 1.0 branch. The original work was never in a completed state anyway, and only half worked.

For reference, this is the RFC:

https://datatracker.ietf.org/doc/html/rfc4533

This directory synchronization seems primarily used by OpenLDAP; I'm unsure if other LDAP implementations use it. Microsoft AD uses its own synchronization process that I already support via DirSync.

#50

@ChadSikorra ChadSikorra added this to the 1.0.0 milestone Jun 7, 2023
…tion properly. Each entry result / referral has associated controls related to the sync.
…ves more granular control over processing referral results / entry results and lets library users process results as they come in instead of waiting until the end of the result set.
…re. This still does not represent any finished product. However, it does represent what was being worked on before this effort was previously dropped.

There's a lot to be redesigned here and tested. But this provides a cleaner starting point.
@codecov
Copy link

codecov bot commented Jun 11, 2023

Codecov Report

Merging #67 (4f85f63) into 1.0 (52d4b1a) will increase coverage by 0.17%.
The diff coverage is 88.56%.

@@             Coverage Diff              @@
##                1.0      #67      +/-   ##
============================================
+ Coverage     86.89%   87.07%   +0.17%     
- Complexity     1873     2093     +220     
============================================
  Files           131      151      +20     
  Lines          5465     6507    +1042     
============================================
+ Hits           4749     5666     +917     
- Misses          716      841     +125     
Impacted Files Coverage Δ
src/FreeDSx/Ldap/ClientOptions.php 81.81% <ø> (ø)
src/FreeDSx/Ldap/Control/Control.php 80.28% <ø> (+0.28%) ⬆️
...ol/ClientProtocolHandler/ClientReferralHandler.php 74.25% <ø> (+0.57%) ⬆️
...rc/FreeDSx/Ldap/Protocol/ServerProtocolHandler.php 99.15% <ø> (+0.08%) ⬆️
...ol/ServerProtocolHandler/ServerAnonBindHandler.php 69.23% <ø> (+2.56%) ⬆️
...otocol/ServerProtocolHandler/ServerBindHandler.php 83.33% <ø> (+1.51%) ⬆️
...eDSx/Ldap/Server/RequestHandler/HandlerFactory.php 81.81% <ø> (ø)
...DSx/Ldap/Server/ServerRunner/PcntlServerRunner.php 0.00% <ø> (ø)
src/FreeDSx/Ldap/ServerOptions.php 71.79% <ø> (ø)
src/FreeDSx/Ldap/Control/Sync/SyncStateControl.php 74.13% <74.13%> (ø)
... and 29 more

... and 79 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…. It's less awkward than interfaces, as the result processing would not normally need a full class, even though the signature is more explicit than an anonymous function.
…r as an optional parameter. This contains potentially needed information, such as the current cookie or the current stage / phase of the sync process.
…ndled. It is either a polling sync that exists, or a listening sync that continues indefinitely.
@ChadSikorra ChadSikorra force-pushed the v1-rfc_4533 branch 2 times, most recently from 2169749 to a663564 Compare June 20, 2023 17:48
…ean for state results. Some of them, like "add", are not very straightforward. For instance, an "add" could just be modifications, initial content polling, or an entry being renamed / moved. These are details of how SyncRepl works and the library consumer will need to figure out the best course of action for the entry on the sync target.
…ng sync phases correctly for polling (the RFC is quite confusing).
@ChadSikorra ChadSikorra force-pushed the v1-rfc_4533 branch 3 times, most recently from 9827437 to 65b71c1 Compare June 29, 2023 14:18
@ChadSikorra
Copy link
Contributor Author

There is still work to be done on this, but I feel like it is in a good enough state to at least get into the 1.0 branch. The polling and listening methods both function and there is documentation. That said, there are some outstanding issues I need to clear-up before a release:

  • The logic for a poll sync might not be complete. The RFC is not clear to me how a second stage content update poll is supposed to work. This needs more research and integration tests.
  • There should be a way to cancel sync requests for the listen method. This requires supporting canceling requests more broadly within the client itself. I will implement this separately.
  • To support the sync I introduced the concept of handlers for searching. These handlers process entries / results as they are received instead of waiting until the end of the search operation. This should be the recommended way library users interact with search, as results will process faster and it exposes the full message / all controls. I need to update docs to make this more obvious that it should be used.

@ChadSikorra ChadSikorra merged commit 2984a4b into FreeDSx:1.0 Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant