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] Support cancelling search / sync operations. #71

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

ChadSikorra
Copy link
Contributor

@ChadSikorra ChadSikorra commented Aug 2, 2023

This adds some special handling for cancelling search / sync operations. These are arguably the only requests it makes practical sense to support handling for cancellations. As in general, it makes little sense to try to cancel a request that will not have multiple responses / is not long running. The usage of cancellations is also somewhat questionable with paging being widely supported. However, it does make some sense in the context of a sync operation. As cancellation allows ending it in a more graceful manner.

#50

@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #71 (559d78b) into 1.0 (226934e) will decrease coverage by 0.43%.
The diff coverage is 66.94%.

❗ Current head 559d78b differs from pull request most recent head 70f73d3. Consider uploading reports for the commit 70f73d3 to get more accurate results

@@             Coverage Diff              @@
##                1.0      #71      +/-   ##
============================================
- Coverage     86.90%   86.48%   -0.43%     
- Complexity     2093     2116      +23     
============================================
  Files           151      155       +4     
  Lines          6432     6517      +85     
============================================
+ Hits           5590     5636      +46     
- Misses          842      881      +39     
Files Changed Coverage Δ
...FreeDSx/Ldap/Operation/Response/CancelResponse.php 0.00% <0.00%> (ø)
...tocol/ClientProtocolHandler/ClientBasicHandler.php 100.00% <ø> (ø)
...ocol/ClientProtocolHandler/ClientCancelHandler.php 0.00% <0.00%> (ø)
...c/FreeDSx/Ldap/Operation/Request/SearchRequest.php 90.00% <41.17%> (-6.78%) ⬇️
...rc/FreeDSx/Ldap/Protocol/ClientProtocolHandler.php 89.23% <50.00%> (-1.25%) ⬇️
...Protocol/ClientProtocolHandler/RequestCanceler.php 71.42% <71.42%> (ø)
.../FreeDSx/Ldap/Exception/CancelRequestException.php 100.00% <100.00%> (ø)
...FreeDSx/Ldap/Operation/Response/SearchResponse.php 100.00% <100.00%> (ø)
...otocol/ClientProtocolHandler/ClientSearchTrait.php 100.00% <100.00%> (ø)
...otocol/ClientProtocolHandler/ClientSyncHandler.php 80.35% <100.00%> (+1.07%) ⬆️
... and 2 more

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

@ChadSikorra ChadSikorra force-pushed the v1-cancel_operation branch 8 times, most recently from 5a15b6f to 18973dc Compare August 3, 2023 16:09
… operations that make sense to have special logic for handling a cancellation.

Add a cancellation strategy for search based requests. They can either choose to "continue" to process messages received until the server acknowledges the cancellation, or they can choose to "stop" (the default) and ignore any subsequent messages received from the point of cancellation to the point where the server acknowledges it.

I can see the "stop" strategy being useful in normal search contexts, while a "continue" would likely be desired in a sync operation search (as you would not want to ignore sync data).
@ChadSikorra ChadSikorra force-pushed the v1-cancel_operation branch from 18973dc to 70f73d3 Compare August 3, 2023 16:27
@ChadSikorra ChadSikorra marked this pull request as ready for review August 3, 2023 16:30
@ChadSikorra ChadSikorra merged commit b7f1f9f into FreeDSx:1.0 Aug 3, 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