Skip to content

refactor(service): remove unused POST support #47

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.

Sign up for GitHub

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

Open
joescottdave wants to merge 1 commit into from
+33 −61
Open

refactor(service): remove unused POST support #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 4 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand 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) | - | ✓ | -<sup>‡</sup> | ✓ | ✓ | ✓ |
| `query_string` | `String`, nilable<sup>§</sup> | - | ✓ | -<sup>‡</sup> | ✓ | ✓ | - |
| `method` | `String`, upcased | - | ✓ | -<sup>‡</sup> | - | - | ✓ |
| `method` | `String`, upcased | - | - | -<sup>‡</sup> | - | - | ✓ |
| `status` | `Integer`, nilable | - | - | -<sup>‡</sup> | always `503` | nilable<sup>¶</sup> | - |
| `duration` | `Integer`, **milliseconds** | - | - | ✓ | ✓ | ✓ | - |
| `will_retry_in` | `Float`, **seconds** | - | - | - | - | - | ✓ |
Expand All @@ -279,8 +279,7 @@ Hash - none of these field names apply; see above.<br>
<sup>‡</sup> derivable from the `response:`/`exception:` object already in
the payload rather than duplicated as a separate field - see the code
examples below.<br>
<sup>§</sup> `nil` for POST requests (which never have query params) and for
GET requests with no params.<br>
<sup>§</sup> `nil` for GET requests with no params.<br>
<sup>¶</sup> `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).
Expand Down
51 changes: 15 additions & 36 deletions lib/data_services_api/service.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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',
Expand Down
29 changes: 10 additions & 19 deletions sig/data_services_api/service.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,24 @@ 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
def get_json: (untyped http_url, untyped params, untyped options) -> json_value

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

Expand All @@ -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

Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion test/data_services_api/service_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down