fair-acc / opencmw-cpp

Open Common Middle-Ware library for accelerator equipment- and beam-based control systems at FAIR.
https://opencmw.io
GNU Lesser General Public License v3.0
10 stars 8 forks source link

Fix subscriptions via REST when using params #331

Closed frankosterfeld closed 11 months ago

frankosterfeld commented 1 year ago

Unify subscription topic handling and fix it for REST (#328, #331)

Do not lose params when subscribing to a topic via REST.

Bump cpp-httplib to newest version, as it fixes an encoding error with query parameters encoded in query parameters (SubscriptionContext) when used with redirects.

To unify subscription handling (different client/servers were making slightly different assumptions), the following is now implemented:

UPDATE: Rebased/merged #330 into this one:

Make sure that long-poll request handlers do not block forever when no corresponding event is received. Otherwise the clients will send one request after another once their request times out client-side, until the worker threads are exhausted and the server stops responding.

It would be great if we could also detect the client connection be dropped using Keep-Alive, but cpp-httplib doesn't seem to have API for that.

codecov[bot] commented 1 year ago

Codecov Report

Attention: 171 lines in your changes are missing coverage. Please review.

Comparison is base (99ae7d2) 55.84% compared to head (a03822f) 57.15%.

Files Patch % Lines
src/majordomo/include/majordomo/RestBackend.hpp 36.58% 18 Missing and 34 partials :warning:
src/majordomo/include/majordomo/Broker.hpp 35.29% 3 Missing and 30 partials :warning:
src/client/include/RestClientNative.hpp 15.15% 14 Missing and 14 partials :warning:
src/core/include/Topic.hpp 70.00% 1 Missing and 20 partials :warning:
src/client/include/Client.hpp 14.28% 2 Missing and 16 partials :warning:
src/majordomo/include/majordomo/Worker.hpp 39.13% 3 Missing and 11 partials :warning:
src/client/include/MockServer.hpp 40.00% 0 Missing and 3 partials :warning:
src/zmq/include/zmq/ZmqUtils.hpp 0.00% 0 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #331 +/- ## ========================================== + Coverage 55.84% 57.15% +1.30% ========================================== Files 69 69 Lines 7291 7310 +19 Branches 2691 2703 +12 ========================================== + Hits 4072 4178 +106 + Misses 1484 1322 -162 - Partials 1735 1810 +75 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 11 months ago

Quality Gate Failed Quality Gate failed

Failed conditions

35.3% Coverage on New Code (required ≥ 80%)
17.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud