From 6d15d2b563525a73903306d4f89ad21bebfc8a36 Mon Sep 17 00:00:00 2001 From: joescottdave Date: Wed, 19 Aug 2026 15:25:13 +0100 Subject: [PATCH] refactor(service): remove unused POST support --- CHANGELOG.md | 4 ++ README.md | 9 ++--- lib/data_services_api/service.rb | 51 ++++++++------------------ sig/data_services_api/service.rbs | 29 +++++---------- test/data_services_api/service_test.rb | 1 - 5 files changed, 33 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba00439..7a6a626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 retry/timeout options - Extracted the duplicated request-timing/instrumentation/rescue logic in `get_from_api`/`post_to_api` into a shared `perform_request` helper +- **Breaking**: Removed POST support (`Service#api_post_json`/`post_json`/ + `post_to_api`). Confirmed unused by both consuming apps (`ppd-explorer`, + `ukhpi`); GET is the only HTTP method the gem now sends, so `method` is no + longer part of `request.data_services_api`'s payload ## 1.7.0 - 2026-07-13 diff --git a/README.md b/README.md index d61863f..63586e2 100644 --- a/README.md +++ b/README.md @@ -244,9 +244,9 @@ configured `instrumenter`: fields via its own accessors, e.g. `env.method`, `env.url`), so a subscriber to this event needs different handling than the rest. - **`request.data_services_api`** - fires immediately before a request is - sent (both GET and POST), regardless of how it later resolves. + sent, regardless of how it later resolves. - **`response.data_services_api`** - fires for every response that doesn't - raise (both GET and POST). + raise. - **`connection_failure.data_services_api`** - a network-level failure (timeout or refused connection), after retries are exhausted. The request never got a response at all. @@ -267,7 +267,7 @@ the event doesn't include that field): | `exception` | see note | - | - | - | `Faraday::TimeoutError`/`ConnectionFailed` | `ServiceException` | see note | | `path` | `String` (bare path, no scheme/host/query) | - | ✓ | -‡ | ✓ | ✓ | ✓ | | `query_string` | `String`, nilable§ | - | ✓ | -‡ | ✓ | ✓ | - | -| `method` | `String`, upcased | - | ✓ | -‡ | - | - | ✓ | +| `method` | `String`, upcased | - | - | -‡ | - | - | ✓ | | `status` | `Integer`, nilable | - | - | -‡ | always `503` | nilable | - | | `duration` | `Integer`, **milliseconds** | - | - | ✓ | ✓ | ✓ | - | | `will_retry_in` | `Float`, **seconds** | - | - | - | - | - | ✓ | @@ -279,8 +279,7 @@ Hash - none of these field names apply; see above.
‡ derivable from the `response:`/`exception:` object already in the payload rather than duplicated as a separate field - see the code examples below.
-§ `nil` for POST requests (which never have query params) and for -GET requests with no params.
+§ `nil` for GET requests with no params.
`nil` if Faraday never associated a response with the error (`Faraday::Error#response_status` returns `nil` in that case - can happen for some `Faraday::ParsingError`s). diff --git a/lib/data_services_api/service.rb b/lib/data_services_api/service.rb index 4f7c8a2..85d2171 100644 --- a/lib/data_services_api/service.rb +++ b/lib/data_services_api/service.rb @@ -64,10 +64,6 @@ def api_get_json(api, params, options = {}) get_json(as_http_api(api), params, options) end - def api_post_json(api, json) - post_json(as_http_api(api), json) - end - private # Get parsed JSON from the given URL @@ -78,7 +74,7 @@ def get_json(http_url, params, options) def get_from_api(http_url, accept_headers, params, options) query_params = params.merge(options) - perform_request(http_url, 'GET', query_params) do |conn| + perform_request(http_url, query_params) do |conn| conn.get do |req| req.headers['X-Request-Id'] = Thread.current[:request_id] if Thread.current[:request_id] req.headers['Accept'] = accept_headers @@ -88,28 +84,14 @@ def get_from_api(http_url, accept_headers, params, options) end end - def post_json(http_url, json) - post_to_api(http_url, json).body - end - - def post_to_api(http_url, json) - perform_request(http_url, 'POST') do |conn| - conn.post do |req| - req.headers['X-Request-Id'] = Thread.current[:request_id] if Thread.current[:request_id] - req.headers['Accept'] = 'application/json' - req.headers['Content-Type'] = 'application/json' - req.body = json - end - end - end - - # Perform an HTTP request against http_url, timing and instrumenting it consistently - # regardless of whether it succeeds, times out, fails to connect, or the remote API - # returns an error status or unparseable body. query_params, when given, is only used - # to report the query string on connection/service failures (a successful response - # reports its own resolved query string from the Faraday response itself) - def perform_request(http_url, method, query_params = nil) # rubocop:disable Metrics/MethodLength - instrument_request(http_url, method, query_params) + # Perform an HTTP GET request against http_url, timing and instrumenting it + # consistently regardless of whether it succeeds, times out, fails to connect, + # or the remote API returns an error status or unparseable body. query_params + # is only used to report the query string on connection/service failures (a + # successful response reports its own resolved query string from the Faraday + # response itself) + def perform_request(http_url, query_params) # rubocop:disable Metrics/MethodLength + instrument_request(http_url, query_params) start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :microsecond) conn = create_http_connection(http_url) @@ -211,14 +193,12 @@ def instrument_response(response, start_time) end # Fires 'request.data_services_api' immediately before a request is sent. - # Payload: path (String, no scheme/host/query), method (String, upcased), - # and query_string (String or nil - nil for POST requests, which never pass - # query_params, and for GET requests with no params). - def instrument_request(http_url, method, query_params) + # Payload: path (String, no scheme/host/query) and query_string (String or + # nil - nil for GET requests with no params). + def instrument_request(http_url, query_params) instrumenter&.instrument( 'request.data_services_api', path: URI.parse(http_url).path, - method:, query_string: query_params && URI.encode_www_form(query_params) ) end @@ -227,10 +207,9 @@ def instrument_request(http_url, method, query_params) # (after retries are exhausted): the request never got a response at all. # Payload: exception (Faraday::TimeoutError or ConnectionFailed), path # (String, no scheme/host/query), query_string (String or nil - nil for - # POST requests, which never pass query_params, and for GET requests with - # no params), duration (Integer milliseconds, see #instrument_response), - # and status (always the literal 503 - a fixed value, not derived from any - # actual response, since none was received). + # GET requests with no params), duration (Integer milliseconds, see + # #instrument_response), and status (always the literal 503 - a fixed + # value, not derived from any actual response, since none was received). def instrument_connection_failure(http_url, query_params, exception, start_time) instrumenter&.instrument( 'connection_failure.data_services_api', diff --git a/sig/data_services_api/service.rbs b/sig/data_services_api/service.rbs index 5f9e2f2..47b9d4d 100644 --- a/sig/data_services_api/service.rbs +++ b/sig/data_services_api/service.rbs @@ -29,14 +29,10 @@ module DataServicesApi def initialize: (?::Hash[untyped, untyped] config) -> void - def datasets: () -> Array[Dataset] - def dataset: (untyped name) -> Dataset def api_get_json: (untyped api, untyped params, ?::Hash[untyped, untyped] options) -> json_value - def api_post_json: (untyped api, untyped json) -> json_value - private # Get parsed JSON from the given URL @@ -44,16 +40,13 @@ module DataServicesApi def get_from_api: (untyped http_url, untyped accept_headers, untyped params, untyped options) -> untyped - def post_json: (untyped http_url, untyped json) -> untyped - - def post_to_api: (untyped http_url, untyped json) -> untyped - - # Perform an HTTP request against http_url, timing and instrumenting it consistently - # regardless of whether it succeeds, times out, fails to connect, or the remote API - # returns an error status or unparseable body. query_params, when given, is only used - # to report the query string on connection/service failures (a successful response - # reports its own resolved query string from the Faraday response itself) - def perform_request: (untyped http_url, untyped method, ?untyped? query_params) { (untyped) -> untyped } -> untyped + # Perform an HTTP GET request against http_url, timing and instrumenting it + # consistently regardless of whether it succeeds, times out, fails to connect, + # or the remote API returns an error status or unparseable body. query_params + # is only used to report the query string on connection/service failures (a + # successful response reports its own resolved query string from the Faraday + # response itself) + def perform_request: (untyped http_url, untyped query_params) { (untyped) -> untyped } -> untyped def create_http_connection: (untyped http_url) -> untyped @@ -72,10 +65,9 @@ module DataServicesApi def instrument_retry: (untyped env, untyped retry_count, untyped exception, untyped will_retry_in) -> untyped # Fires 'request.data_services_api' immediately before a request is sent, with - # path (String), method (String), and query_string (String or nil - nil for - # POST requests, which never pass query_params, and for GET requests with + # path (String) and query_string (String or nil - nil for GET requests with # no params). - def instrument_request: (untyped http_url, untyped method, untyped query_params) -> untyped + def instrument_request: (untyped http_url, untyped query_params) -> untyped def as_http_api: (untyped api) -> untyped @@ -90,8 +82,7 @@ module DataServicesApi # (after retries are exhausted): the request never got a response at all. # Payload: exception (Faraday::TimeoutError or ConnectionFailed), path # (String, no scheme/host/query), query_string (String or nil - nil for - # POST requests, which never pass query_params, and for GET requests with - # no params), duration (Integer milliseconds, see #instrument_response), + # GET requests with no params), duration (Integer milliseconds, see #instrument_response), # and status (always the literal 503 - a fixed value, not derived from any # actual response, since none was received). def instrument_connection_failure: (untyped http_url, untyped query_params, untyped exception, untyped start_time) -> untyped diff --git a/test/data_services_api/service_test.rb b/test/data_services_api/service_test.rb index 67289a5..b7855e3 100644 --- a/test/data_services_api/service_test.rb +++ b/test/data_services_api/service_test.rb @@ -131,7 +131,6 @@ def instrument(*args) _, payload = mock_notifier.instrumentations.find { |name, _| name == 'request.data_services_api' } _(payload).wont_be_nil - _(payload[:method]).must_equal 'GET' _(payload[:path]).must_equal '/landregistry/id/ukhpi' _(payload[:query_string]).must_equal '_limit=1' end