Skip to content

Commit

Permalink
Merge pull request #60 from PericlesTheo/remove-active-support-depend…
Browse files Browse the repository at this point in the history
…ency

Remove ActiveSupport dependency
  • Loading branch information
Nick Campbell authored Aug 29, 2019
2 parents 1c80e97 + fa3f718 commit 9287caf
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 63 deletions.
13 changes: 9 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,15 @@ client = CompaniesHouse::Client.new(config)
The client is configured by passing a hash to the constructor. The supported keys for this
hash are:

| Key | Description |
| ----------- | ----------- |
| `:api_key` | Required. The API key received after registration. |
| `:endpoint` | Optional. Specifies the base URI for the API (e.g. if using a self-hosted version) |
| Key | Description |
| ------------------ | ----------- |
| `:api_key` | Required. The API key received after registration. |
| `:endpoint` | Optional. Specifies the base URI for the API (e.g. if using a self-hosted version) |
| `:instrumentation` | Optional. Instruments the request/response (see Instrumentation for details) |

## Instrumentation
By default, no instrumentation is being applied.
If you are using Rails or the `ActiveSupport` gem, instrumentation will happen automatically via ![ActiveSupport::Notifications](https://api.rubyonrails.org/classes/ActiveSupport/Notifications.html)

## Requests

Expand Down
17 changes: 16 additions & 1 deletion lib/companies_house/client.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# frozen_string_literal: true

require "companies_house/request"
require "companies_house/instrumentation/null"
require "companies_house/instrumentation/active_support"
require "net/http"
require "securerandom"

module CompaniesHouse
# This class provides an interface to the Companies House API
Expand All @@ -10,7 +13,7 @@ module CompaniesHouse
class Client
ENDPOINT = "https://api.companieshouse.gov.uk"

attr_reader :api_key, :endpoint
attr_reader :api_key, :endpoint, :instrumentation

def initialize(config)
raise ArgumentError, "Missing API key" unless config[:api_key]
Expand All @@ -19,6 +22,7 @@ def initialize(config)
@endpoint = URI(config[:endpoint] || ENDPOINT)
@open_timeout = config[:open_timeout] || 60
@read_timeout = config[:read_timeout] || 60
@instrumentation = configure_instrumentation(config[:instrumentation])
raise ArgumentError, "HTTP is not supported" if @endpoint.scheme != "https"
end

Expand Down Expand Up @@ -99,7 +103,18 @@ def request(resource,
resource_type: resource,
resource_id: resource_id,
transaction_id: transaction_id,
instrumentation: instrumentation,
).execute
end

def configure_instrumentation(instrumentation)
return instrumentation unless instrumentation.nil?

if defined?(ActiveSupport::Notifications)
Instrumentation::ActiveSupport
else
Instrumentation::Null
end
end
end
end
9 changes: 9 additions & 0 deletions lib/companies_house/instrumentation/active_support.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

module Instrumentation
class ActiveSupport
def self.publish(*args)
::ActiveSupport::Notifications.publish(**args)
end
end
end
7 changes: 7 additions & 0 deletions lib/companies_house/instrumentation/null.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module Instrumentation
class Null
def self.publish(*args); end
end
end
10 changes: 5 additions & 5 deletions lib/companies_house/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
require "uri"
require "json"

require "active_support/notifications"

module CompaniesHouse
# This class manages individual requests.
# Users of the CompaniesHouse gem should not instantiate this class
Expand All @@ -33,6 +31,7 @@ class Request
attribute :resource_id, String

attribute :transaction_id, String, required: true
attribute :instrumentation

def initialize(args)
super(args)
Expand Down Expand Up @@ -71,11 +70,12 @@ def execute
private

def publish_notification
ActiveSupport::Notifications.publish(
instrumentation.publish(
"companies_house.#{resource_type}",
@started, Time.now.utc,
@started,
Time.now.utc,
transaction_id,
@notification_payload
@notification_payload,
)
end

Expand Down
66 changes: 40 additions & 26 deletions spec/companies_house/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@
client = described_class.new(api_key: "key", read_timeout: 2)
expect(client.connection.read_timeout).to eq(2)
end

it "sets the Null instrumenter when ActiveSupport is not present" do
client = described_class.new(api_key: "key")
expect(client.instrumentation).to eq(Instrumentation::Null)
end

it "can set the ActiveSupport instrumenter if constant is defined" do
class_double("ActiveSupport::Notifications").as_stubbed_const
client = described_class.new(api_key: "key")
expect(client.instrumentation).to eq(Instrumentation::ActiveSupport)
end
end

describe "#end_connection" do
Expand Down Expand Up @@ -160,36 +171,39 @@
expect(response).to eq(%w[item1 item2])
end

# rubocop:disable RSpec/MultipleExpectations
# rubocop:disable RSpec/ExampleLength
it "sends two notifications" do
notifications = notifications_of do
response
end

expect(notifications).to match(
[have_attributes(
name: "companies_house.officers",
payload: {
method: :get,
path: rest_path,
query: { start_index: 0 },
response: JSON[page1],
status: status.to_s,
},
), have_attributes(
name: "companies_house.officers",
payload: {
method: :get,
path: rest_path,
query: { start_index: 1 },
response: JSON[page2],
status: status.to_s,
},
# This should match the transaction ID of the first notification
transaction_id: notifications[0].transaction_id,
)],
expect(client.instrumentation).to receive(:publish).with(
"companies_house.officers",
kind_of(Time),
kind_of(Time),
kind_of(String),
hash_including(
method: :get,
path: rest_path,
query: { start_index: 0 },
response: JSON[page1],
status: status.to_s,
),
)
expect(client.instrumentation).to receive(:publish).with(
"companies_house.officers",
kind_of(Time),
kind_of(Time),
kind_of(String),
hash_including(
method: :get,
path: rest_path,
query: { start_index: 1 },
response: JSON[page2],
status: status.to_s,
),
)

response
end
# rubocop:enable RSpec/MultipleExpectations
# rubocop:enable RSpec/ExampleLength
end

Expand Down
44 changes: 17 additions & 27 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
# frozen_string_literal: true

require "webmock/rspec"

$LOAD_PATH.unshift File.expand_path("../lib", __dir__)
require "companies_house/client"
require "timecop"

def notifications_of
notifications = []
subscriber = ActiveSupport::Notifications.subscribe do |*args|
notifications << ActiveSupport::Notifications::Event.new(*args)
end

yield
ActiveSupport::Notifications.unsubscribe(subscriber)
notifications
end

shared_context "test credentials" do
let(:api_key) { "el-psy-congroo" }
let(:example_endpoint) { URI("https://api.example.com:8000") }
Expand Down Expand Up @@ -99,41 +89,41 @@ def notifications_of
let(:time) { Time.now.utc }

# rubocop:disable RSpec/ExampleLength
it "records to ActiveSupport" do
it "records an instrumentation" do
i = 0
allow(SecureRandom).to receive(:hex).with(10) do
i += 1
sprintf("RANDOM%04d", i)
end

recorded_notifications = notifications_of do
Timecop.freeze(time) do
response
rescue StandardError
""
end
end

expected_payload = {
method: :get,
path: rest_path,
query: rest_query,
status: status.to_s,
}

if error_class
expected_payload[:error] = be_a(error_class)
else
expected_payload[:response] = be_truthy
end

expected_event = have_attributes(
name: "companies_house.#{request_method}",
time: time,
end: time,
payload: expected_payload,
transaction_id: "RANDOM0001",
expect(client.instrumentation).to receive(:publish).with(
"companies_house.#{request_method}",
time,
time,
"RANDOM0001",
expected_payload,
)
expect(recorded_notifications).to match([expected_event])

begin
Timecop.freeze(time) do
response
end
rescue StandardError
""
end
end
# rubocop:enable RSpec/ExampleLength
end

0 comments on commit 9287caf

Please sign in to comment.