Commit 9e5a576a authored by Alexandru Croitor's avatar Alexandru Croitor

Change projects to return all projects in a single request

Replace the paginated `projects` field implementation with one
that returns all Jira projects in one request. This is required
in order to support Jira Server API that does not yet support
getting the paginated list of projects.
parent 93bf7173
...@@ -13,11 +13,10 @@ module Resolvers ...@@ -13,11 +13,10 @@ module Resolvers
def resolve(name: nil, **args) def resolve(name: nil, **args)
authorize!(project) authorize!(project)
response, start_cursor, end_cursor = jira_projects(name: name, **compute_pagination_params(args)) response = jira_projects(name: name)
end_cursor = nil if !!response.payload[:is_last]
if response.success? if response.success?
Gitlab::Graphql::ExternallyPaginatedArray.new(start_cursor, end_cursor, *response.payload[:projects]) response.payload[:projects]
else else
raise Gitlab::Graphql::Errors::BaseError, response.message raise Gitlab::Graphql::Errors::BaseError, response.message
end end
...@@ -35,41 +34,10 @@ module Resolvers ...@@ -35,41 +34,10 @@ module Resolvers
jira_service&.project jira_service&.project
end end
def compute_pagination_params(params) def jira_projects(name:)
after_cursor = Base64.decode64(params[:after].to_s) args = { query: name }.compact
before_cursor = Base64.decode64(params[:before].to_s)
# differentiate between 0 cursor and nil or invalid cursor that decodes into zero. return Jira::Requests::Projects.new(project.jira_service, args).execute
after_index = after_cursor.to_i == 0 && after_cursor != "0" ? nil : after_cursor.to_i
before_index = before_cursor.to_i == 0 && before_cursor != "0" ? nil : before_cursor.to_i
if after_index.present? && before_index.present?
if after_index >= before_index
{ start_at: 0, limit: 0 }
else
{ start_at: after_index + 1, limit: before_index - after_index - 1 }
end
elsif after_index.present?
{ start_at: after_index + 1, limit: nil }
elsif before_index.present?
{ start_at: 0, limit: before_index - 1 }
else
{ start_at: 0, limit: nil }
end
end
def jira_projects(name:, start_at:, limit:)
args = { query: name, start_at: start_at, limit: limit }.compact
response = Jira::Requests::Projects.new(project.jira_service, args).execute
return [response, nil, nil] if response.error?
projects = response.payload[:projects]
start_cursor = start_at == 0 ? nil : Base64.encode64((start_at - 1).to_s)
end_cursor = Base64.encode64((start_at + projects.size - 1).to_s)
[response, start_cursor, end_cursor]
end end
end end
end end
......
...@@ -10,26 +10,13 @@ module Types ...@@ -10,26 +10,13 @@ module Types
authorize :admin_project authorize :admin_project
field :all_projects,
[Types::Projects::Services::JiraProjectType],
null: true,
connection: false,
description: 'List of all Jira projects fetched through Jira REST API. Latest Jira Server API version ([8.9.0](https://docs.atlassian.com/software/jira/docs/api/REST/8.9.0/)) compatible.'
field :projects, field :projects,
Types::Projects::Services::JiraProjectType.connection_type, Types::Projects::Services::JiraProjectType.connection_type,
null: true, null: true,
connection: false, connection: false,
extensions: [Gitlab::Graphql::Extensions::ExternallyPaginatedArrayExtension], extensions: [Gitlab::Graphql::Extensions::ExternallyPaginatedArrayExtension],
description: 'List of Jira projects fetched through Jira REST API', description: 'List of all Jira projects fetched through Jira REST API',
resolver: Resolvers::Projects::JiraProjectsResolver resolver: Resolvers::Projects::JiraProjectsResolver
def all_projects
raise Gitlab::Graphql::Errors::BaseError, _('Jira service not configured.') unless object&.active?
raise Gitlab::Graphql::Errors::BaseError, _('Unable to connect to the Jira instance. Please check your Jira integration configuration.') unless object.test(nil)[:success]
object.client.Project.all
end
end end
end end
end end
......
...@@ -5,22 +5,16 @@ module Jira ...@@ -5,22 +5,16 @@ module Jira
class Base class Base
include ProjectServicesLoggable include ProjectServicesLoggable
PER_PAGE = 50 attr_reader :jira_service, :project, :query
attr_reader :jira_service, :project, :limit, :start_at, :query def initialize(jira_service, query: nil)
def initialize(jira_service, limit: PER_PAGE, start_at: 0, query: nil)
@project = jira_service&.project @project = jira_service&.project
@jira_service = jira_service @jira_service = jira_service
@query = query
@limit = limit
@start_at = start_at
@query = query
end end
def execute def execute
return ServiceResponse.error(message: _('Jira service not configured.')) unless jira_service&.active? return ServiceResponse.error(message: _('Jira service not configured.')) unless jira_service&.active?
return ServiceResponse.success(payload: empty_payload) if limit.to_i <= 0
request request
end end
......
...@@ -9,19 +9,24 @@ module Jira ...@@ -9,19 +9,24 @@ module Jira
override :url override :url
def url def url
'/rest/api/2/project/search?query=%{query}&maxResults=%{limit}&startAt=%{start_at}' % '/rest/api/2/project'
{ query: CGI.escape(query.to_s), limit: limit.to_i, start_at: start_at.to_i }
end end
override :build_service_response override :build_service_response
def build_service_response(response) def build_service_response(response)
return ServiceResponse.success(payload: empty_payload) unless response['values'].present? return ServiceResponse.success(payload: empty_payload) unless response.present?
ServiceResponse.success(payload: { projects: map_projects(response), is_last: response['isLast'] }) ServiceResponse.success(payload: { projects: map_projects(response), is_last: true })
end end
def map_projects(response) def map_projects(response)
response['values'].map { |v| JIRA::Resource::Project.build(client, v) } response.map { |v| JIRA::Resource::Project.build(client, v) }.select(&method(:match_query?))
end
def match_query?(jira_project)
query = self.query.to_s.downcase
jira_project&.key&.downcase&.include?(query) || jira_project&.name&.downcase&.include?(query)
end end
def empty_payload def empty_payload
......
...@@ -6259,13 +6259,7 @@ type JiraService implements Service { ...@@ -6259,13 +6259,7 @@ type JiraService implements Service {
active: Boolean active: Boolean
""" """
List of all Jira projects fetched through Jira REST API. Latest Jira Server API version List of all Jira projects fetched through Jira REST API
([8.9.0](https://docs.atlassian.com/software/jira/docs/api/REST/8.9.0/)) compatible.
"""
allProjects: [JiraProject!]
"""
List of Jira projects fetched through Jira REST API
""" """
projects( projects(
""" """
......
...@@ -17302,31 +17302,9 @@ ...@@ -17302,31 +17302,9 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "allProjects",
"description": "List of all Jira projects fetched through Jira REST API. Latest Jira Server API version ([8.9.0](https://docs.atlassian.com/software/jira/docs/api/REST/8.9.0/)) compatible.",
"args": [
],
"type": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "OBJECT",
"name": "JiraProject",
"ofType": null
}
}
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "projects", "name": "projects",
"description": "List of Jira projects fetched through Jira REST API", "description": "List of all Jira projects fetched through Jira REST API",
"args": [ "args": [
{ {
"name": "name", "name": "name",
...@@ -925,8 +925,7 @@ Autogenerated return type of JiraImportUsers ...@@ -925,8 +925,7 @@ Autogenerated return type of JiraImportUsers
| Name | Type | Description | | Name | Type | Description |
| --- | ---- | ---------- | | --- | ---- | ---------- |
| `active` | Boolean | Indicates if the service is active | | `active` | Boolean | Indicates if the service is active |
| `allProjects` | JiraProject! => Array | List of all Jira projects fetched through Jira REST API. Latest Jira Server API version ([8.9.0](https://docs.atlassian.com/software/jira/docs/api/REST/8.9.0/)) compatible. | | `projects` | JiraProjectConnection | List of all Jira projects fetched through Jira REST API |
| `projects` | JiraProjectConnection | List of Jira projects fetched through Jira REST API |
| `type` | String | Class name of the service | | `type` | String | Class name of the service |
## JiraUser ## JiraUser
......
...@@ -63,7 +63,7 @@ describe Resolvers::Projects::JiraProjectsResolver do ...@@ -63,7 +63,7 @@ describe Resolvers::Projects::JiraProjectsResolver do
context 'when Jira connection is not valid' do context 'when Jira connection is not valid' do
before do before do
WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/project/search?maxResults=50&query=&startAt=0') WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/project')
.to_raise(JIRA::HTTPError.new(double(message: 'Some failure.'))) .to_raise(JIRA::HTTPError.new(double(message: 'Some failure.')))
end end
......
...@@ -6,7 +6,7 @@ describe GitlabSchema.types['JiraService'] do ...@@ -6,7 +6,7 @@ describe GitlabSchema.types['JiraService'] do
specify { expect(described_class.graphql_name).to eq('JiraService') } specify { expect(described_class.graphql_name).to eq('JiraService') }
it 'has basic expected fields' do it 'has basic expected fields' do
expect(described_class).to have_graphql_fields(:type, :active, :projects, :all_projects) expect(described_class).to have_graphql_fields(:type, :active, :projects)
end end
specify { expect(described_class).to require_graphql_authorizations(:admin_project) } specify { expect(described_class).to require_graphql_authorizations(:admin_project) }
......
# frozen_string_literal: true
require 'spec_helper'
describe 'query Jira projects' do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project) }
include_context 'jira projects request context'
let(:services) { graphql_data_at(:project, :services, :edges) }
let(:all_jira_projects) { services.first.dig('node', 'allProjects') }
let(:query) do
%(
query {
project(fullPath: "#{project.full_path}") {
services(type: JIRA_SERVICE) {
edges {
node {
... on JiraService {
allProjects {
key
name
projectId
}
}
}
}
}
}
}
)
end
context 'when user does not have access' do
it_behaves_like 'unauthorized users cannot read services'
end
context 'when user can access project services' do
before do
project.add_maintainer(current_user)
end
context 'when jira service enabled and working' do
before do
post_graphql(query, current_user: current_user)
end
it_behaves_like 'a working graphql query'
it 'returns list of all jira projects' do
project_keys = all_jira_projects.map { |jp| jp['key'] }
project_names = all_jira_projects.map { |jp| jp['name'] }
project_ids = all_jira_projects.map { |jp| jp['projectId'] }
expect(all_jira_projects.size).to eq(2)
expect(project_keys).to eq(%w(EX ABC))
expect(project_names).to eq(%w(Example Alphabetical))
expect(project_ids).to eq([10000, 10001])
end
end
context 'when connection to jira fails' do
before do
WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo').to_raise(Errno::ECONNREFUSED)
end
it 'returns error', :aggregate_failures do
post_graphql(query, current_user: current_user)
expect(all_jira_projects).to be_nil
expect(graphql_errors).to include(a_hash_including('message' => 'Unable to connect to the Jira instance. Please check your Jira integration configuration.'))
end
end
context 'when jira service is not active' do
before do
jira_service.update!(active: false)
end
it 'returns error', :aggregate_failures do
post_graphql(query, current_user: current_user)
expect(all_jira_projects).to be_nil
expect(graphql_errors).to include(a_hash_including('message' => 'Jira service not configured.'))
end
end
end
end
...@@ -80,34 +80,6 @@ describe 'query Jira projects' do ...@@ -80,34 +80,6 @@ describe 'query Jira projects' do
it_behaves_like 'fetches first project' it_behaves_like 'fetches first project'
end end
context 'with before cursor' do
let(:projects_query) { 'projects(before: "Mg==", first: 1)' }
it_behaves_like 'fetches first project'
end
context 'with after cursor' do
let(:projects_query) { 'projects(after: "MA==", first: 1)' }
it_behaves_like 'fetches first project'
end
end
context 'with valid but inexistent after cursor' do
let(:projects_query) { 'projects(after: "MTk==")' }
it 'retuns empty list of jira projects' do
expect(jira_projects.size).to eq(0)
end
end
context 'with invalid after cursor' do
let(:projects_query) { 'projects(after: "invalid==")' }
it 'treats the invalid cursor as no cursor and returns list of jira projects' do
expect(jira_projects.size).to eq(2)
end
end end
end end
end end
......
...@@ -32,14 +32,6 @@ describe Jira::Requests::Projects do ...@@ -32,14 +32,6 @@ describe Jira::Requests::Projects do
end end
context 'with jira_service' do context 'with jira_service' do
context 'when limit is invalid' do
let(:params) { { limit: 0 } }
it 'returns a paylod with no projects returned' do
expect(subject.payload[:projects]).to be_empty
end
end
context 'when validations and params are ok' do context 'when validations and params are ok' do
let(:client) { double(options: { site: 'https://jira.example.com' }) } let(:client) { double(options: { site: 'https://jira.example.com' }) }
...@@ -60,7 +52,7 @@ describe Jira::Requests::Projects do ...@@ -60,7 +52,7 @@ describe Jira::Requests::Projects do
context 'when the request does not return any values' do context 'when the request does not return any values' do
before do before do
expect(client).to receive(:get).and_return({ 'someKey' => 'value' }) expect(client).to receive(:get).and_return([])
end end
it 'returns a paylod with no projects returned' do it 'returns a paylod with no projects returned' do
...@@ -74,19 +66,15 @@ describe Jira::Requests::Projects do ...@@ -74,19 +66,15 @@ describe Jira::Requests::Projects do
context 'when the request returns values' do context 'when the request returns values' do
before do before do
expect(client).to receive(:get).and_return( expect(client).to receive(:get).and_return([{ "key" => 'project1' }, { "key" => 'project2' }])
{ 'values' => %w(project1 project2), 'isLast' => false }
)
expect(JIRA::Resource::Project).to receive(:build).with(client, 'project1').and_return('jira_project1')
expect(JIRA::Resource::Project).to receive(:build).with(client, 'project2').and_return('jira_project2')
end end
it 'returns a paylod with jira projets' do it 'returns a paylod with jira projets' do
payload = subject.payload payload = subject.payload
expect(subject.success?).to be_truthy expect(subject.success?).to be_truthy
expect(payload[:projects]).to eq(%w(jira_project1 jira_project2)) expect(payload[:projects].map(&:key)).to eq(%w(project1 project2))
expect(payload[:is_last]).to be_falsey expect(payload[:is_last]).to be_truthy
end end
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