Commit 8e14c14e authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'refactor-sentry-client-extract-projects' into 'master'

Extract projects related logic from Sentry

See merge request gitlab-org/gitlab!21499
parents ed9936bf dd0f6416
......@@ -86,7 +86,7 @@ module ErrorTracking
end
def list_sentry_projects
{ projects: sentry_client.list_projects }
{ projects: sentry_client.projects }
end
def issue_details(opts = {})
......
......@@ -2,6 +2,8 @@
module Sentry
class Client
include Sentry::Client::Projects
Error = Class.new(StandardError)
MissingKeysError = Class.new(StandardError)
ResponseInvalidSizeError = Class.new(StandardError)
......@@ -49,14 +51,6 @@ module Sentry
end
end
def list_projects
projects = get_projects
handle_mapping_exceptions do
map_to_projects(projects)
end
end
private
def validate_size(issues)
......@@ -121,10 +115,6 @@ module Sentry
http_get(issue_latest_event_api_url(issue_id))[:body]
end
def get_projects
http_get(projects_api_url)[:body]
end
def handle_request_exceptions
yield
rescue Gitlab::HTTP::Error => e
......@@ -155,13 +145,6 @@ module Sentry
raise Client::Error, message
end
def projects_api_url
projects_url = URI(@url)
projects_url.path = '/api/0/projects/'
projects_url
end
def issue_api_url(issue_id)
issue_url = URI(@url)
issue_url.path = "/api/0/issues/#{issue_id}/"
......@@ -187,10 +170,6 @@ module Sentry
issues.map(&method(:map_to_error))
end
def map_to_projects(projects)
projects.map(&method(:map_to_project))
end
def issue_url(id)
issues_url = @url + "/issues/#{id}"
......@@ -289,19 +268,5 @@ module Sentry
project_slug: issue.dig('project', 'slug')
)
end
def map_to_project(project)
organization = project.fetch('organization')
Gitlab::ErrorTracking::Project.new(
id: project.fetch('id', nil),
name: project.fetch('name'),
slug: project.fetch('slug'),
status: project.dig('status'),
organization_name: organization.fetch('name'),
organization_id: organization.fetch('id', nil),
organization_slug: organization.fetch('slug')
)
end
end
end
# frozen_string_literal: true
module Sentry
class Client
module Projects
def projects
projects = get_projects
handle_mapping_exceptions do
map_to_projects(projects)
end
end
private
def get_projects
http_get(projects_api_url)[:body]
end
def projects_api_url
projects_url = URI(url)
projects_url.path = '/api/0/projects/'
projects_url
end
def map_to_projects(projects)
projects.map(&method(:map_to_project))
end
def map_to_project(project)
organization = project.fetch('organization')
Gitlab::ErrorTracking::Project.new(
id: project.fetch('id', nil),
name: project.fetch('name'),
slug: project.fetch('slug'),
status: project.dig('status'),
organization_name: organization.fetch('name'),
organization_id: organization.fetch('id', nil),
organization_slug: organization.fetch('slug')
)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Sentry::Client::Projects do
include SentryClientHelpers
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
let(:token) { 'test-token' }
let(:client) { Sentry::Client.new(sentry_url, token) }
let(:projects_sample_response) do
Gitlab::Utils.deep_indifferent_access(
JSON.parse(fixture_file('sentry/list_projects_sample_response.json'))
)
end
shared_examples 'has correct return type' do |klass|
it "returns objects of type #{klass}" do
expect(subject).to all( be_a(klass) )
end
end
shared_examples 'has correct length' do |length|
it { expect(subject.length).to eq(length) }
end
describe '#projects' do
let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' }
let(:sentry_api_response) { projects_sample_response }
let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: sentry_api_response) }
subject { client.projects }
it_behaves_like 'calls sentry api'
it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
it_behaves_like 'has correct length', 2
context 'essential keys missing in API response' do
let(:sentry_api_response) do
projects_sample_response[0...1].map do |project|
project.except(:slug)
end
end
it 'raises exception' do
expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "slug"')
end
end
context 'optional keys missing in sentry response' do
let(:sentry_api_response) do
projects_sample_response[0...1].map do |project|
project[:organization].delete(:id)
project.delete(:id)
project.except(:status)
end
end
it_behaves_like 'calls sentry api'
it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
it_behaves_like 'has correct length', 1
end
context 'error object created from sentry response' do
using RSpec::Parameterized::TableSyntax
where(:sentry_project_object, :sentry_response) do
:id | :id
:name | :name
:status | :status
:slug | :slug
:organization_name | [:organization, :name]
:organization_id | [:organization, :id]
:organization_slug | [:organization, :slug]
end
with_them do
it do
expect(subject[0].public_send(sentry_project_object)).to(
eq(sentry_api_response[0].dig(*sentry_response))
)
end
end
end
context 'redirects' do
let(:sentry_api_url) { sentry_list_projects_url }
it_behaves_like 'no Sentry redirects'
end
# Sentry API returns 404 if there are extra slashes in the URL!
context 'extra slashes in URL' do
let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' }
let!(:valid_req_stub) do
stub_sentry_request(sentry_list_projects_url)
end
it 'removes extra slashes in api url' do
expect(Gitlab::HTTP).to receive(:get).with(
URI(sentry_list_projects_url),
anything
).and_call_original
subject
expect(valid_req_stub).to have_been_requested
end
end
context 'when exception is raised' do
let(:sentry_request_url) { sentry_list_projects_url }
it_behaves_like 'maps Sentry exceptions'
end
end
end
......@@ -3,6 +3,8 @@
require 'spec_helper'
describe Sentry::Client do
include SentryClientHelpers
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
let(:token) { 'test-token' }
let(:default_httparty_options) do
......@@ -18,89 +20,18 @@ describe Sentry::Client do
)
end
let(:projects_sample_response) do
Gitlab::Utils.deep_indifferent_access(
JSON.parse(fixture_file('sentry/list_projects_sample_response.json'))
)
end
subject(:client) { described_class.new(sentry_url, token) }
# Requires sentry_api_url and subject to be defined
shared_examples 'no redirects' do
let(:redirect_to) { 'https://redirected.example.com' }
let(:other_url) { 'https://other.example.org' }
let!(:redirected_req_stub) { stub_sentry_request(other_url) }
let!(:redirect_req_stub) do
stub_sentry_request(
sentry_api_url,
status: 302,
headers: { location: redirect_to }
)
end
it 'does not follow redirects' do
expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response status code: 302')
expect(redirect_req_stub).to have_been_requested
expect(redirected_req_stub).not_to have_been_requested
end
end
shared_examples 'has correct return type' do |klass|
it "returns objects of type #{klass}" do
expect(subject).to all( be_a(klass) )
end
end
shared_examples 'issues has correct return type' do |klass|
it "returns objects of type #{klass}" do
expect(subject[:issues]).to all( be_a(klass) )
end
end
shared_examples 'has correct length' do |length|
it { expect(subject.length).to eq(length) }
end
shared_examples 'issues has correct length' do |length|
it { expect(subject[:issues].length).to eq(length) }
end
# Requires sentry_api_request and subject to be defined
shared_examples 'calls sentry api' do
it 'calls sentry api' do
subject
expect(sentry_api_request).to have_been_requested
end
end
shared_examples 'maps exceptions' do
exceptions = {
Gitlab::HTTP::Error => 'Error when connecting to Sentry',
Net::OpenTimeout => 'Connection to Sentry timed out',
SocketError => 'Received SocketError when trying to connect to Sentry',
OpenSSL::SSL::SSLError => 'Sentry returned invalid SSL data',
Errno::ECONNREFUSED => 'Connection refused',
StandardError => 'Sentry request failed due to StandardError'
}
exceptions.each do |exception, message|
context "#{exception}" do
before do
stub_request(:get, sentry_request_url).to_raise(exception)
end
it do
expect { subject }
.to raise_exception(Sentry::Client::Error, message)
end
end
end
end
describe '#list_issues' do
let(:issue_status) { 'unresolved' }
let(:limit) { 20 }
......@@ -174,7 +105,7 @@ describe Sentry::Client do
context 'redirects' do
let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
it_behaves_like 'no redirects'
it_behaves_like 'no Sentry redirects'
end
# Sentry API returns 404 if there are extra slashes in the URL!
......@@ -265,7 +196,7 @@ describe Sentry::Client do
end
end
it_behaves_like 'maps exceptions'
it_behaves_like 'maps Sentry exceptions'
context 'when search term is present' do
let(:search_term) { 'NoMethodError' }
......@@ -287,112 +218,4 @@ describe Sentry::Client do
it_behaves_like 'issues has correct length', 1
end
end
describe '#list_projects' do
let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' }
let(:sentry_api_response) { projects_sample_response }
let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: sentry_api_response) }
subject { client.list_projects }
it_behaves_like 'calls sentry api'
it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
it_behaves_like 'has correct length', 2
context 'essential keys missing in API response' do
let(:sentry_api_response) do
projects_sample_response[0...1].map do |project|
project.except(:slug)
end
end
it 'raises exception' do
expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "slug"')
end
end
context 'optional keys missing in sentry response' do
let(:sentry_api_response) do
projects_sample_response[0...1].map do |project|
project[:organization].delete(:id)
project.delete(:id)
project.except(:status)
end
end
it_behaves_like 'calls sentry api'
it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
it_behaves_like 'has correct length', 1
end
context 'error object created from sentry response' do
using RSpec::Parameterized::TableSyntax
where(:sentry_project_object, :sentry_response) do
:id | :id
:name | :name
:status | :status
:slug | :slug
:organization_name | [:organization, :name]
:organization_id | [:organization, :id]
:organization_slug | [:organization, :slug]
end
with_them do
it do
expect(subject[0].public_send(sentry_project_object)).to(
eq(sentry_api_response[0].dig(*sentry_response))
)
end
end
end
context 'redirects' do
let(:sentry_api_url) { sentry_list_projects_url }
it_behaves_like 'no redirects'
end
# Sentry API returns 404 if there are extra slashes in the URL!
context 'extra slashes in URL' do
let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' }
let(:client) { described_class.new(sentry_url, token) }
let!(:valid_req_stub) do
stub_sentry_request(sentry_list_projects_url)
end
it 'removes extra slashes in api url' do
expect(Gitlab::HTTP).to receive(:get).with(
URI(sentry_list_projects_url),
anything
).and_call_original
subject
expect(valid_req_stub).to have_been_requested
end
end
context 'when exception is raised' do
let(:sentry_request_url) { sentry_list_projects_url }
it_behaves_like 'maps exceptions'
end
end
private
def stub_sentry_request(url, body: {}, status: 200, headers: {})
stub_request(:get, url)
.to_return(
status: status,
headers: { 'Content-Type' => 'application/json' }.merge(headers),
body: body.to_json
)
end
end
......@@ -193,7 +193,7 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
it 'calls sentry client' do
expect(subject).to receive(:sentry_client).and_return(sentry_client)
expect(sentry_client).to receive(:list_projects).and_return(projects)
expect(sentry_client).to receive(:projects).and_return(projects)
result = subject.list_sentry_projects
......
# frozen_string_literal: true
module SentryClientHelpers
private
def stub_sentry_request(url, body: {}, status: 200, headers: {})
stub_request(:get, url)
.to_return(
status: status,
headers: { 'Content-Type' => 'application/json' }.merge(headers),
body: body.to_json
)
end
end
# frozen_string_literal: true
# Requires sentry_api_request and subject to be defined
RSpec.shared_examples 'calls sentry api' do
it 'calls sentry api' do
subject
expect(sentry_api_request).to have_been_requested
end
end
# Requires sentry_api_url and subject to be defined
RSpec.shared_examples 'no Sentry redirects' do
let(:redirect_to) { 'https://redirected.example.com' }
let(:other_url) { 'https://other.example.org' }
let!(:redirected_req_stub) { stub_sentry_request(other_url) }
let!(:redirect_req_stub) do
stub_sentry_request(
sentry_api_url,
status: 302,
headers: { location: redirect_to }
)
end
it 'does not follow redirects' do
expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response status code: 302')
expect(redirect_req_stub).to have_been_requested
expect(redirected_req_stub).not_to have_been_requested
end
end
RSpec.shared_examples 'maps Sentry exceptions' do
exceptions = {
Gitlab::HTTP::Error => 'Error when connecting to Sentry',
Net::OpenTimeout => 'Connection to Sentry timed out',
SocketError => 'Received SocketError when trying to connect to Sentry',
OpenSSL::SSL::SSLError => 'Sentry returned invalid SSL data',
Errno::ECONNREFUSED => 'Connection refused',
StandardError => 'Sentry request failed due to StandardError'
}
exceptions.each do |exception, message|
context "#{exception}" do
before do
stub_request(:get, sentry_request_url).to_raise(exception)
end
it do
expect { subject }
.to raise_exception(Sentry::Client::Error, message)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment