diff --git a/core/base-service/base-graphql.js b/core/base-service/base-graphql.js new file mode 100644 index 0000000000000..9be1ca6c7eaff --- /dev/null +++ b/core/base-service/base-graphql.js @@ -0,0 +1,52 @@ +'use strict' + +const { print } = require('graphql/language/printer') +const BaseService = require('./base') +const { InvalidResponse, ShieldsRuntimeError } = require('./errors') +const { parseJson } = require('./json') + +function defaultTransformErrors(errors) { + return new InvalidResponse({ prettyMessage: errors[0].message }) +} + +class BaseGraphqlService extends BaseService { + _parseJson(buffer) { + return parseJson(buffer) + } + + async _requestGraphql({ + schema, + url, + query, + variables = {}, + options = {}, + httpErrorMessages = {}, + transformErrors = defaultTransformErrors, + }) { + const mergedOptions = { + ...{ headers: { Accept: 'application/json' } }, + ...options, + } + mergedOptions.method = 'POST' + mergedOptions.body = JSON.stringify({ query: print(query), variables }) + const { buffer } = await this._request({ + url, + options: mergedOptions, + errorMessages: httpErrorMessages, + }) + const json = this._parseJson(buffer) + if (json.errors) { + const exception = transformErrors(json.errors) + if (exception instanceof ShieldsRuntimeError) { + throw exception + } else { + throw Error( + `transformErrors() must return a ShieldsRuntimeError; got ${exception}` + ) + } + } + return this.constructor._validate(json, schema) + } +} + +module.exports = BaseGraphqlService diff --git a/core/base-service/base-graphql.spec.js b/core/base-service/base-graphql.spec.js new file mode 100644 index 0000000000000..4f72887d2d68c --- /dev/null +++ b/core/base-service/base-graphql.spec.js @@ -0,0 +1,209 @@ +'use strict' + +const Joi = require('@hapi/joi') +const { expect } = require('chai') +const gql = require('graphql-tag') +const sinon = require('sinon') +const BaseGraphqlService = require('./base-graphql') +const { InvalidResponse } = require('./errors') + +const dummySchema = Joi.object({ + requiredString: Joi.string().required(), +}).required() + +class DummyGraphqlService extends BaseGraphqlService { + static get category() { + return 'cat' + } + + static get route() { + return { + base: 'foo', + } + } + + async handle() { + const { requiredString } = await this._requestGraphql({ + schema: dummySchema, + url: 'http://example.com/graphql', + query: gql` + query { + requiredString + } + `, + }) + return { message: requiredString } + } +} + +describe('BaseGraphqlService', function() { + describe('Making requests', function() { + let sendAndCacheRequest + beforeEach(function() { + sendAndCacheRequest = sinon.stub().returns( + Promise.resolve({ + buffer: '{"some": "json"}', + res: { statusCode: 200 }, + }) + ) + }) + + it('invokes _sendAndCacheRequest', async function() { + await DummyGraphqlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + + expect(sendAndCacheRequest).to.have.been.calledOnceWith( + 'http://example.com/graphql', + { + body: '{"query":"{\\n requiredString\\n}\\n","variables":{}}', + headers: { Accept: 'application/json' }, + method: 'POST', + } + ) + }) + + it('forwards options to _sendAndCacheRequest', async function() { + class WithOptions extends DummyGraphqlService { + async handle() { + const { value } = await this._requestGraphql({ + schema: dummySchema, + url: 'http://example.com/graphql', + query: gql` + query { + requiredString + } + `, + options: { qs: { queryParam: 123 } }, + }) + return { message: value } + } + } + + await WithOptions.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + + expect(sendAndCacheRequest).to.have.been.calledOnceWith( + 'http://example.com/graphql', + { + body: '{"query":"{\\n requiredString\\n}\\n","variables":{}}', + headers: { Accept: 'application/json' }, + method: 'POST', + qs: { queryParam: 123 }, + } + ) + }) + }) + + describe('Making badges', function() { + it('handles valid json responses', async function() { + const sendAndCacheRequest = async () => ({ + buffer: '{"requiredString": "some-string"}', + res: { statusCode: 200 }, + }) + expect( + await DummyGraphqlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + message: 'some-string', + }) + }) + + it('handles json responses which do not match the schema', async function() { + const sendAndCacheRequest = async () => ({ + buffer: '{"unexpectedKey": "some-string"}', + res: { statusCode: 200 }, + }) + expect( + await DummyGraphqlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + isError: true, + color: 'lightgray', + message: 'invalid response data', + }) + }) + + it('handles unparseable json responses', async function() { + const sendAndCacheRequest = async () => ({ + buffer: 'not json', + res: { statusCode: 200 }, + }) + expect( + await DummyGraphqlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + isError: true, + color: 'lightgray', + message: 'unparseable json response', + }) + }) + }) + + describe('Error handling', function() { + it('handles generic error', async function() { + const sendAndCacheRequest = async () => ({ + buffer: '{ "errors": [ { "message": "oh noes!!" } ] }', + res: { statusCode: 200 }, + }) + expect( + await DummyGraphqlService.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + isError: true, + color: 'lightgray', + message: 'oh noes!!', + }) + }) + + it('handles custom error', async function() { + class WithErrorHandler extends DummyGraphqlService { + async handle() { + const { requiredString } = await this._requestGraphql({ + schema: dummySchema, + url: 'http://example.com/graphql', + query: gql` + query { + requiredString + } + `, + transformErrors: function(errors) { + if (errors[0].message === 'oh noes!!') { + return new InvalidResponse({ + prettyMessage: 'a terrible thing has happened', + }) + } + }, + }) + return { message: requiredString } + } + } + + const sendAndCacheRequest = async () => ({ + buffer: '{ "errors": [ { "message": "oh noes!!" } ] }', + res: { statusCode: 200 }, + }) + expect( + await WithErrorHandler.invoke( + { sendAndCacheRequest }, + { handleInternalErrors: false } + ) + ).to.deep.equal({ + isError: true, + color: 'lightgray', + message: 'a terrible thing has happened', + }) + }) + }) +}) diff --git a/core/base-service/base-json.js b/core/base-service/base-json.js index bb61f9768bbb2..a76c78f43ceea 100644 --- a/core/base-service/base-json.js +++ b/core/base-service/base-json.js @@ -1,28 +1,11 @@ 'use strict' -// See available emoji at http://emoji.muan.co/ -const emojic = require('emojic') const BaseService = require('./base') -const trace = require('./trace') -const { InvalidResponse } = require('./errors') +const { parseJson } = require('./json') class BaseJsonService extends BaseService { _parseJson(buffer) { - const logTrace = (...args) => trace.logTrace('fetch', ...args) - let json - try { - json = JSON.parse(buffer) - } catch (err) { - logTrace(emojic.dart, 'Response JSON (unparseable)', buffer) - throw new InvalidResponse({ - prettyMessage: 'unparseable json response', - underlyingError: err, - }) - } - logTrace(emojic.dart, 'Response JSON (before validation)', json, { - deep: true, - }) - return json + return parseJson(buffer) } async _requestJson({ schema, url, options = {}, errorMessages = {} }) { diff --git a/core/base-service/graphql.js b/core/base-service/graphql.js new file mode 100644 index 0000000000000..3a78deb09a6d8 --- /dev/null +++ b/core/base-service/graphql.js @@ -0,0 +1,52 @@ +'use strict' +/** + * @module + */ + +/** + * Utility function to merge two graphql queries together + * This is basically copied from + * [graphql-query-merge](https://www.npmjs.com/package/graphql-query-merge) + * but can't use that due to incorrect packaging. + * + * @param {...object} queries queries to merge + * @returns {object} merged query + */ +function mergeQueries(...queries) { + const merged = { + kind: 'Document', + definitions: [ + { + directives: [], + operation: 'query', + variableDefinitions: [], + kind: 'OperationDefinition', + selectionSet: { kind: 'SelectionSet', selections: [] }, + }, + ], + } + + queries.forEach(query => { + const parsedQuery = query + parsedQuery.definitions.forEach(definition => { + merged.definitions[0].directives = [ + ...merged.definitions[0].directives, + ...definition.directives, + ] + + merged.definitions[0].variableDefinitions = [ + ...merged.definitions[0].variableDefinitions, + ...definition.variableDefinitions, + ] + + merged.definitions[0].selectionSet.selections = [ + ...merged.definitions[0].selectionSet.selections, + ...definition.selectionSet.selections, + ] + }) + }) + + return merged +} + +module.exports = { mergeQueries } diff --git a/core/base-service/graphql.spec.js b/core/base-service/graphql.spec.js new file mode 100644 index 0000000000000..a77e40f85d88e --- /dev/null +++ b/core/base-service/graphql.spec.js @@ -0,0 +1,94 @@ +'use strict' + +const { expect } = require('chai') +const gql = require('graphql-tag') +const { print } = require('graphql/language/printer') +const { mergeQueries } = require('./graphql') + +require('../register-chai-plugins.spec') + +describe('mergeQueries function', function() { + it('merges valid gql queries', function() { + expect( + print( + mergeQueries( + gql` + query($param: String!) { + foo(param: $param) { + bar + } + } + ` + ) + ) + ).to.equalIgnoreSpaces( + 'query ($param: String!) { foo(param: $param) { bar } }' + ) + + expect( + print( + mergeQueries( + gql` + query($param: String!) { + foo(param: $param) { + bar + } + } + `, + gql` + query { + baz + } + ` + ) + ) + ).to.equalIgnoreSpaces( + 'query ($param: String!) { foo(param: $param) { bar } baz }' + ) + + expect( + print( + mergeQueries( + gql` + query { + foo + } + `, + gql` + query { + bar + } + `, + gql` + query { + baz + } + ` + ) + ) + ).to.equalIgnoreSpaces('{ foo bar baz }') + + expect( + print( + mergeQueries( + gql` + { + foo + } + `, + gql` + { + bar + } + ` + ) + ) + ).to.equalIgnoreSpaces('{ foo bar }') + }) + + it('throws an error when passed invalid params', function() { + expect(() => mergeQueries('', '')).to.throw(Error) + expect(() => mergeQueries(undefined, 17, true)).to.throw(Error) + expect(() => mergeQueries(gql``, gql`foo`)).to.throw(Error) + }) +}) diff --git a/core/base-service/index.js b/core/base-service/index.js index 7e7dd8a51f16f..5764be1200b55 100644 --- a/core/base-service/index.js +++ b/core/base-service/index.js @@ -2,6 +2,7 @@ const BaseService = require('./base') const BaseJsonService = require('./base-json') +const BaseGraphqlService = require('./base-graphql') const NonMemoryCachingBaseService = require('./base-non-memory-caching') const BaseStaticService = require('./base-static') const BaseSvgScrapingService = require('./base-svg-scraping') @@ -20,6 +21,7 @@ const { module.exports = { BaseService, BaseJsonService, + BaseGraphqlService, NonMemoryCachingBaseService, BaseStaticService, BaseSvgScrapingService, diff --git a/core/base-service/json.js b/core/base-service/json.js new file mode 100644 index 0000000000000..6186b1e6b0b57 --- /dev/null +++ b/core/base-service/json.js @@ -0,0 +1,28 @@ +'use strict' + +// See available emoji at http://emoji.muan.co/ +const emojic = require('emojic') +const { InvalidResponse } = require('./errors') +const trace = require('./trace') + +function parseJson(buffer) { + const logTrace = (...args) => trace.logTrace('fetch', ...args) + let json + try { + json = JSON.parse(buffer) + } catch (err) { + logTrace(emojic.dart, 'Response JSON (unparseable)', buffer) + throw new InvalidResponse({ + prettyMessage: 'unparseable json response', + underlyingError: err, + }) + } + logTrace(emojic.dart, 'Response JSON (before validation)', json, { + deep: true, + }) + return json +} + +module.exports = { + parseJson, +} diff --git a/package-lock.json b/package-lock.json index d097e3ff1f22c..9fa9402ae0368 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12705,7 +12705,6 @@ "version": "14.4.2", "resolved": "https://registry.npmjs.org/graphql/-/graphql-14.4.2.tgz", "integrity": "sha512-6uQadiRgnpnSS56hdZUSvFrVcQ6OF9y6wkxJfKquFtHlnl7+KSuWwSJsdwiK1vybm1HgcdbpGkCpvhvsVQ0UZQ==", - "dev": true, "requires": { "iterall": "^1.2.2" } @@ -12767,6 +12766,11 @@ "cross-fetch": "2.2.2" } }, + "graphql-tag": { + "version": "2.10.1", + "resolved": "https://registry.npmjs.org/graphql-tag/-/graphql-tag-2.10.1.tgz", + "integrity": "sha512-jApXqWBzNXQ8jYa/HLkZJaVw9jgwNqZkywa2zfFn16Iv1Zb7ELNHkJaXHR7Quvd5SIGsy6Ny7SUKATgnu05uEg==" + }, "graphql-type-json": { "version": "0.2.4", "resolved": "https://registry.npmjs.org/graphql-type-json/-/graphql-type-json-0.2.4.tgz", @@ -14889,8 +14893,7 @@ "iterall": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/iterall/-/iterall-1.2.2.tgz", - "integrity": "sha512-yynBb1g+RFUPY64fTrFv7nsjRrENBQJaX2UL+2Szc9REFrSNm1rpSXHGzhmAy7a9uv3vlvgBlXnf9RqmPH1/DA==", - "dev": true + "integrity": "sha512-yynBb1g+RFUPY64fTrFv7nsjRrENBQJaX2UL+2Szc9REFrSNm1rpSXHGzhmAy7a9uv3vlvgBlXnf9RqmPH1/DA==" }, "iterate-object": { "version": "1.3.3", diff --git a/package.json b/package.json index 1b6ce75e2605f..e445515a1948a 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,8 @@ "fsos": "^1.1.6", "gh-badges": "file:gh-badges", "glob": "^7.1.4", + "graphql": "^14.4.2", + "graphql-tag": "^2.10.1", "ioredis": "^4.11.1", "joi-extension-semver": "3.0.0", "js-yaml": "^3.13.1", diff --git a/services/github/github-api-provider.js b/services/github/github-api-provider.js index 6def1f9fcb5cc..b4be59a634f1f 100644 --- a/services/github/github-api-provider.js +++ b/services/github/github-api-provider.js @@ -1,6 +1,7 @@ 'use strict' const Joi = require('@hapi/joi') +const log = require('../../core/server/log') const { TokenPool } = require('../../core/token-pooling/token-pool') const { nonNegativeInteger } = require('../validators') @@ -12,6 +13,22 @@ const headerSchema = Joi.object({ .required() .unknown(true) +const bodySchema = Joi.object({ + data: Joi.object({ + rateLimit: Joi.object({ + limit: nonNegativeInteger, + remaining: nonNegativeInteger, + resetAt: Joi.date().iso(), + }) + .required() + .unknown(true), + }) + .required() + .unknown(true), +}) + .required() + .unknown(true) + // Provides an interface to the Github API. Manages the base URL. class GithubApiProvider { // reserveFraction: The amount of much of a token's quota we avoid using, to @@ -34,6 +51,7 @@ class GithubApiProvider { if (this.withPooling) { this.standardTokens = new TokenPool({ batchSize: 25 }) this.searchTokens = new TokenPool({ batchSize: 5 }) + this.graphqlTokens = new TokenPool({ batchSize: 25 }) } } @@ -42,6 +60,7 @@ class GithubApiProvider { return { standardTokens: this.standardTokens.serializeDebugInfo({ sanitize }), searchTokens: this.searchTokens.serializeDebugInfo({ sanitize }), + graphqlTokens: this.graphqlTokens.serializeDebugInfo({ sanitize }), } } else { return {} @@ -52,33 +71,70 @@ class GithubApiProvider { if (this.withPooling) { this.standardTokens.add(tokenString) this.searchTokens.add(tokenString) + this.graphqlTokens.add(tokenString) } else { throw Error('When not using a token pool, do not provide tokens') } } - updateToken(token, headers) { + getV3RateLimitFromHeaders(headers) { + const h = Joi.attempt(headers, headerSchema) + return { + rateLimit: h['x-ratelimit-limit'], + totalUsesRemaining: h['x-ratelimit-remaining'], + nextReset: h['x-ratelimit-reset'], + } + } + + getV4RateLimitFromBody(body) { + const parsedBody = JSON.parse(body) + const b = Joi.attempt(parsedBody, bodySchema) + return { + rateLimit: b.data.rateLimit.limit, + totalUsesRemaining: b.data.rateLimit.remaining, + nextReset: Date.parse(b.data.rateLimit.resetAt) / 1000, + } + } + + updateToken({ token, url, res }) { let rateLimit, totalUsesRemaining, nextReset - try { - ;({ - 'x-ratelimit-limit': rateLimit, - 'x-ratelimit-remaining': totalUsesRemaining, - 'x-ratelimit-reset': nextReset, - } = Joi.attempt(headers, headerSchema)) - } catch (e) { - const logHeaders = { - 'x-ratelimit-limit': headers['x-ratelimit-limit'], - 'x-ratelimit-remaining': headers['x-ratelimit-remaining'], - 'x-ratelimit-reset': headers['x-ratelimit-reset'], + if (url.startsWith('/graphql')) { + try { + ;({ + rateLimit, + totalUsesRemaining, + nextReset, + } = this.getV4RateLimitFromBody(res.body)) + } catch (e) { + console.error( + `Could not extract rate limit info from response body ${res.body}` + ) + log.error(e) + return + } + } else { + try { + ;({ + rateLimit, + totalUsesRemaining, + nextReset, + } = this.getV3RateLimitFromHeaders(res.headers)) + } catch (e) { + const logHeaders = { + 'x-ratelimit-limit': res.headers['x-ratelimit-limit'], + 'x-ratelimit-remaining': res.headers['x-ratelimit-remaining'], + 'x-ratelimit-reset': res.headers['x-ratelimit-reset'], + } + console.error( + `Invalid GitHub rate limit headers ${JSON.stringify( + logHeaders, + undefined, + 2 + )}` + ) + log.error(e) + return } - console.log( - `Invalid GitHub rate limit headers ${JSON.stringify( - logHeaders, - undefined, - 2 - )}` - ) - return } const reserve = Math.ceil(this.reserveFraction * rateLimit) @@ -95,6 +151,8 @@ class GithubApiProvider { tokenForUrl(url) { if (url.startsWith('/search')) { return this.searchTokens.next() + } else if (url.startsWith('/graphql')) { + return this.graphqlTokens.next() } else { return this.standardTokens.next() } @@ -103,7 +161,7 @@ class GithubApiProvider { // Act like request(), but tweak headers and query to avoid hitting a rate // limit. Inject `request` so we can pass in `cachingRequest` from // `request-handler.js`. - request(request, url, query, callback) { + request(request, url, options = {}, callback) { const { baseUrl } = this let token @@ -120,24 +178,26 @@ class GithubApiProvider { tokenString = this.globalToken } - const options = { - url, - baseUrl, - qs: query, - headers: { - 'User-Agent': 'Shields.io', - Accept: 'application/vnd.github.v3+json', - Authorization: `token ${tokenString}`, + const mergedOptions = { + ...options, + ...{ + url, + baseUrl, + headers: { + 'User-Agent': 'Shields.io', + Accept: 'application/vnd.github.v3+json', + Authorization: `token ${tokenString}`, + }, }, } - request(options, (err, res, buffer) => { + request(mergedOptions, (err, res, buffer) => { if (err === null) { if (this.withPooling) { if (res.statusCode === 401) { this.invalidateToken(token) } else if (res.statusCode < 500) { - this.updateToken(token, res.headers) + this.updateToken({ token, url, res }) } } } @@ -145,9 +205,9 @@ class GithubApiProvider { }) } - requestAsPromise(request, url, query) { + requestAsPromise(request, url, options) { return new Promise((resolve, reject) => { - this.request(request, url, query, (err, res, buffer) => { + this.request(request, url, options, (err, res, buffer) => { if (err) { reject(err) } else { diff --git a/services/github/github-api-provider.spec.js b/services/github/github-api-provider.spec.js index 3c89f4f8551f3..f67e788424c7b 100644 --- a/services/github/github-api-provider.spec.js +++ b/services/github/github-api-provider.spec.js @@ -8,7 +8,7 @@ describe('Github API provider', function() { const baseUrl = 'https://github-api.example.com' const reserveFraction = 0.333 - let mockStandardToken, mockSearchToken, provider + let mockStandardToken, mockSearchToken, mockGraphqlToken, provider beforeEach(function() { provider = new GithubApiProvider({ baseUrl, reserveFraction }) @@ -17,6 +17,9 @@ describe('Github API provider', function() { mockSearchToken = { update: sinon.spy(), invalidate: sinon.spy() } sinon.stub(provider.searchTokens, 'next').returns(mockSearchToken) + + mockGraphqlToken = { update: sinon.spy(), invalidate: sinon.spy() } + sinon.stub(provider.graphqlTokens, 'next').returns(mockGraphqlToken) }) context('a search API request', function() { @@ -28,6 +31,22 @@ describe('Github API provider', function() { expect(err).to.be.undefined expect(provider.searchTokens.next).to.have.been.calledOnce expect(provider.standardTokens.next).not.to.have.been.called + expect(provider.graphqlTokens.next).not.to.have.been.called + done() + }) + }) + }) + + context('a graphql API request', function() { + const mockRequest = (options, callback) => { + callback() + } + it('should obtain an appropriate token', function(done) { + provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => { + expect(err).to.be.undefined + expect(provider.searchTokens.next).not.to.have.been.called + expect(provider.standardTokens.next).not.to.have.been.called + expect(provider.graphqlTokens.next).to.have.been.calledOnce done() }) }) @@ -42,12 +61,13 @@ describe('Github API provider', function() { expect(err).to.be.undefined expect(provider.searchTokens.next).not.to.have.been.called expect(provider.standardTokens.next).to.have.been.calledOnce + expect(provider.graphqlTokens.next).not.to.have.been.called done() }) }) }) - context('a valid response', function() { + context('a valid V3 API response', function() { const rateLimit = 12500 const remaining = 7955 const nextReset = 123456789 @@ -89,6 +109,54 @@ describe('Github API provider', function() { }) }) + context('a valid V4 API response', function() { + const rateLimit = 12500 + const remaining = 7955 + const nextReset = 123456789 + const mockResponse = { + statusCode: 200, + headers: {}, + body: `{ + "data": { + "rateLimit": { + "limit": 12500, + "cost": 1, + "remaining": 7955, + "resetAt": "1973-11-29T21:33:09Z" + } + } + }`, + } + const mockBuffer = Buffer.alloc(0) + const mockRequest = (...args) => { + const callback = args.pop() + callback(null, mockResponse, mockBuffer) + } + + it('should invoke the callback', function(done) { + provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => { + expect(err).to.equal(null) + expect(Object.is(res, mockResponse)).to.be.true + expect(Object.is(buffer, mockBuffer)).to.be.true + done() + }) + }) + + it('should update the token with the expected values', function(done) { + provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => { + expect(err).to.equal(null) + const expectedUsesRemaining = + remaining - Math.ceil(reserveFraction * rateLimit) + expect(mockGraphqlToken.update).to.have.been.calledWith( + expectedUsesRemaining, + nextReset + ) + expect(mockGraphqlToken.invalidate).not.to.have.been.called + done() + }) + }) + }) + context('an unauthorized response', function() { const mockResponse = { statusCode: 401 } const mockBuffer = Buffer.alloc(0) diff --git a/services/github/github-auth-service.js b/services/github/github-auth-service.js index c398ce6de0212..76e05f1c71820 100644 --- a/services/github/github-auth-service.js +++ b/services/github/github-auth-service.js @@ -1,20 +1,23 @@ 'use strict' +const gql = require('graphql-tag') +const { mergeQueries } = require('../../core/base-service/graphql') const { staticAuthConfigured } = require('./github-helpers') const { BaseJsonService } = require('..') +const { BaseGraphqlService } = require('..') function createRequestFetcher(context, config) { const { sendAndCacheRequestWithCallbacks, githubApiProvider } = context - return async (url, { qs }) => + return async (url, options) => githubApiProvider.requestAsPromise( sendAndCacheRequestWithCallbacks, url, - qs + options ) } -class GithubAuthService extends BaseJsonService { +class GithubAuthV3Service extends BaseJsonService { constructor(context, config) { super(context, config) this._requestFetcher = createRequestFetcher(context, config) @@ -23,11 +26,11 @@ class GithubAuthService extends BaseJsonService { } // Use Github auth, but only when static auth is configured. By using this -// class, in production it will behave like GithubAuthService, and in self- +// class, in production it will behave like GithubAuthV3Service, and in self- // hosting (i.e. with a configured token) like BaseJsonService. This is // useful when consuming GitHub endpoints which are not rate-limited: it // avoids wasting API quota on them in production. -class ConditionalGithubAuthService extends BaseJsonService { +class ConditionalGithubAuthV3Service extends BaseJsonService { constructor(context, config) { super(context, config) if (staticAuthConfigured()) { @@ -39,7 +42,57 @@ class ConditionalGithubAuthService extends BaseJsonService { } } +class GithubAuthV4Service extends BaseGraphqlService { + constructor(context, config) { + super(context, config) + this._requestFetcher = createRequestFetcher(context, config) + this.staticAuthConfigured = true + } + + async _requestGraphql(attrs) { + const url = `/graphql` + + /* + The Github v4 API requires us to query the rateLimit object to return + rate limit info in the query body instead of the headers: + https://developer.github.com/v4/guides/resource-limitations/#returning-a-calls-rate-limit-status + This appends the relevant rateLimit query clause to each + call to the GH v4 API so we can keep track of token usage. + */ + const query = mergeQueries( + attrs.query, + gql` + query { + rateLimit { + limit + cost + remaining + resetAt + } + } + ` + ) + + return super._requestGraphql({ ...attrs, ...{ url, query } }) + } +} + +/* +Choosing between the Github V3 and V4 APIs when creating a new badge: + +With the V3 API, one request = one point off the usage limit. +With the V4 API one request may be many points off the usage limit depending +on the query (but will be a minimum of one). +https://developer.github.com/v4/guides/resource-limitations/#calculating-nodes-in-a-call + +If we can save ourselves some usage limit it may be worth going with a +REST (V3) call over a graphql query. +All other things being equal, a graphql query will almost always be a smaller +number of bytes over the wire and a smaller/simpler object to parse. +*/ + module.exports = { - GithubAuthService, - ConditionalGithubAuthService, + GithubAuthV3Service, + ConditionalGithubAuthV3Service, + GithubAuthV4Service, } diff --git a/services/github/github-commit-activity.service.js b/services/github/github-commit-activity.service.js index e5701b2e98054..5e33bf84fc67a 100644 --- a/services/github/github-commit-activity.service.js +++ b/services/github/github-commit-activity.service.js @@ -3,7 +3,7 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { errorMessagesFor, documentation } = require('./github-helpers') const schema = Joi.array() @@ -14,7 +14,7 @@ const schema = Joi.array() ) .required() -module.exports = class GithubCommitActivity extends GithubAuthService { +module.exports = class GithubCommitActivity extends GithubAuthV3Service { static get category() { return 'activity' } diff --git a/services/github/github-commit-status.service.js b/services/github/github-commit-status.service.js index 8d7772e4d8e53..4677840121ab7 100644 --- a/services/github/github-commit-status.service.js +++ b/services/github/github-commit-status.service.js @@ -1,7 +1,7 @@ 'use strict' const Joi = require('@hapi/joi') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const { NotFound, InvalidParameter } = require('..') @@ -10,7 +10,7 @@ const schema = Joi.object({ status: Joi.equal('identical', 'ahead', 'behind', 'diverged'), }).required() -module.exports = class GithubCommitStatus extends GithubAuthService { +module.exports = class GithubCommitStatus extends GithubAuthV3Service { static get category() { return 'issue-tracking' } diff --git a/services/github/github-commits-since.service.js b/services/github/github-commits-since.service.js index 8794d77e2ff5d..31414e711773c 100644 --- a/services/github/github-commits-since.service.js +++ b/services/github/github-commits-since.service.js @@ -3,13 +3,13 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { fetchLatestRelease } = require('./github-common-fetch') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ ahead_by: nonNegativeInteger }).required() -module.exports = class GithubCommitsSince extends GithubAuthService { +module.exports = class GithubCommitsSince extends GithubAuthV3Service { static get category() { return 'activity' } diff --git a/services/github/github-contributors.service.js b/services/github/github-contributors.service.js index ee07898098eff..c6345eef6dedb 100644 --- a/services/github/github-contributors.service.js +++ b/services/github/github-contributors.service.js @@ -3,13 +3,13 @@ const Joi = require('@hapi/joi') const parseLinkHeader = require('parse-link-header') const { renderContributorBadge } = require('../contributor-count') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') // All we do is check its length. const schema = Joi.array().items(Joi.object()) -module.exports = class GithubContributors extends GithubAuthService { +module.exports = class GithubContributors extends GithubAuthV3Service { static get category() { return 'activity' } diff --git a/services/github/github-downloads.service.js b/services/github/github-downloads.service.js index 2e1e71cb71cf8..44e63a3b4db1d 100644 --- a/services/github/github-downloads.service.js +++ b/services/github/github-downloads.service.js @@ -4,7 +4,7 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') const { downloadCount: downloadCountColor } = require('../color-formatters') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const { NotFound } = require('..') @@ -24,7 +24,7 @@ const releaseArraySchema = Joi.alternatives().try( const keywords = ['github download'] -module.exports = class GithubDownloads extends GithubAuthService { +module.exports = class GithubDownloads extends GithubAuthV3Service { static get category() { return 'downloads' } diff --git a/services/github/github-followers.service.js b/services/github/github-followers.service.js index 6415837492bd8..f3c72055c2076 100644 --- a/services/github/github-followers.service.js +++ b/services/github/github-followers.service.js @@ -3,14 +3,14 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ followers: nonNegativeInteger, }).required() -module.exports = class GithubFollowers extends GithubAuthService { +module.exports = class GithubFollowers extends GithubAuthV3Service { static get category() { return 'social' } diff --git a/services/github/github-forks.service.js b/services/github/github-forks.service.js index 5a4db261ab608..65e84f717034d 100644 --- a/services/github/github-forks.service.js +++ b/services/github/github-forks.service.js @@ -1,16 +1,23 @@ 'use strict' +const gql = require('graphql-tag') const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') -const { documentation, errorMessagesFor } = require('./github-helpers') +const { GithubAuthV4Service } = require('./github-auth-service') +const { documentation, transformErrors } = require('./github-helpers') const schema = Joi.object({ - forks_count: nonNegativeInteger, + data: Joi.object({ + repository: Joi.object({ + forks: Joi.object({ + totalCount: nonNegativeInteger, + }).required(), + }).required(), + }).required(), }).required() -module.exports = class GithubForks extends GithubAuthService { +module.exports = class GithubForks extends GithubAuthV4Service { static get category() { return 'social' } @@ -67,11 +74,24 @@ module.exports = class GithubForks extends GithubAuthService { } async handle({ user, repo }) { - const { forks_count: forkCount } = await this._requestJson({ - url: `/repos/${user}/${repo}`, + const json = await this._requestGraphql({ + query: gql` + query($user: String!, $repo: String!) { + repository(owner: $user, name: $repo) { + forks { + totalCount + } + } + } + `, + variables: { user, repo }, schema, - errorMessages: errorMessagesFor(), + transformErrors, + }) + return this.constructor.render({ + user, + repo, + forkCount: json.data.repository.forks.totalCount, }) - return this.constructor.render({ user, repo, forkCount }) } } diff --git a/services/github/github-helpers.js b/services/github/github-helpers.js index 9851bf44049f9..18430fdd3a92d 100644 --- a/services/github/github-helpers.js +++ b/services/github/github-helpers.js @@ -2,6 +2,7 @@ const serverSecrets = require('../../lib/server-secrets') const { colorScale } = require('../color-formatters') +const { InvalidResponse, NotFound } = require('..') const documentation = `

@@ -24,6 +25,14 @@ function errorMessagesFor(notFoundMessage = 'repo not found') { } } +function transformErrors(errors) { + if (errors[0].type === 'NOT_FOUND') { + return new NotFound({ prettyMessage: 'repo not found' }) + } else { + return new InvalidResponse({ prettyMessage: errors[0].message }) + } +} + const commentsColor = colorScale([1, 3, 10, 25], undefined, true) function staticAuthConfigured() { @@ -35,5 +44,6 @@ module.exports = { stateColor, commentsColor, errorMessagesFor, + transformErrors, staticAuthConfigured, } diff --git a/services/github/github-issue-detail.service.js b/services/github/github-issue-detail.service.js index 9dff561447533..011c2f9af4cf7 100644 --- a/services/github/github-issue-detail.service.js +++ b/services/github/github-issue-detail.service.js @@ -4,7 +4,7 @@ const Joi = require('@hapi/joi') const { nonNegativeInteger } = require('../validators') const { formatDate, metric } = require('../text-formatters') const { age } = require('../color-formatters') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor, @@ -154,7 +154,7 @@ const propertyMap = { 'last-update': ageUpdateMap, } -module.exports = class GithubIssueDetail extends GithubAuthService { +module.exports = class GithubIssueDetail extends GithubAuthV3Service { static get category() { return 'issue-tracking' } diff --git a/services/github/github-issues.service.js b/services/github/github-issues.service.js index a8bce133b5e56..ef5b0064cd937 100644 --- a/services/github/github-issues.service.js +++ b/services/github/github-issues.service.js @@ -3,7 +3,7 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const isPRVariant = { @@ -20,7 +20,7 @@ const schema = Joi.object({ total_count: nonNegativeInteger, }).required() -module.exports = class GithubIssues extends GithubAuthService { +module.exports = class GithubIssues extends GithubAuthV3Service { static get category() { return 'issue-tracking' } diff --git a/services/github/github-languages-base.js b/services/github/github-languages-base.js index e6662f4e35018..61c9b4470c05d 100644 --- a/services/github/github-languages-base.js +++ b/services/github/github-languages-base.js @@ -2,7 +2,7 @@ const Joi = require('@hapi/joi') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { errorMessagesFor } = require('./github-helpers') /* @@ -11,7 +11,7 @@ The keys could be anything and {} is a valid response (e.g: for an empty repo) */ const schema = Joi.object().pattern(/./, nonNegativeInteger) -class BaseGithubLanguage extends GithubAuthService { +class BaseGithubLanguage extends GithubAuthV3Service { async fetch({ user, repo }) { return this._requestJson({ url: `/repos/${user}/${repo}/languages`, diff --git a/services/github/github-last-commit.service.js b/services/github/github-last-commit.service.js index f8658ec53e315..e7a28983c62ef 100644 --- a/services/github/github-last-commit.service.js +++ b/services/github/github-last-commit.service.js @@ -3,7 +3,7 @@ const Joi = require('@hapi/joi') const { formatDate } = require('../text-formatters') const { age: ageColor } = require('../color-formatters') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const commonExampleAttrs = { keywords: ['activity', 'latest'], @@ -22,7 +22,7 @@ const schema = Joi.array() ) .required() -module.exports = class GithubLastCommit extends GithubAuthService { +module.exports = class GithubLastCommit extends GithubAuthV3Service { static get category() { return 'activity' } diff --git a/services/github/github-license.service.js b/services/github/github-license.service.js index 5970caa89b7b3..bd33d1ab0b3be 100644 --- a/services/github/github-license.service.js +++ b/services/github/github-license.service.js @@ -2,7 +2,7 @@ const Joi = require('@hapi/joi') const { renderLicenseBadge } = require('../licenses') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ @@ -10,7 +10,7 @@ const schema = Joi.object({ license: Joi.object({ spdx_id: Joi.string().required() }).allow(null), }).required() -module.exports = class GithubLicense extends GithubAuthService { +module.exports = class GithubLicense extends GithubAuthV3Service { static get category() { return 'license' } diff --git a/services/github/github-manifest.service.js b/services/github/github-manifest.service.js index d930a82d28adf..53de9b0c45412 100644 --- a/services/github/github-manifest.service.js +++ b/services/github/github-manifest.service.js @@ -7,7 +7,7 @@ const { transformAndValidate, renderDynamicBadge, } = require('../dynamic-common') -const { ConditionalGithubAuthService } = require('./github-auth-service') +const { ConditionalGithubAuthV3Service } = require('./github-auth-service') const { fetchJsonFromRepo } = require('./github-common-fetch') const { documentation } = require('./github-helpers') @@ -17,7 +17,7 @@ const schema = Joi.object({ const flexibleSchema = Joi.object().required() -class GithubManifestVersion extends ConditionalGithubAuthService { +class GithubManifestVersion extends ConditionalGithubAuthV3Service { static get category() { return 'version' } @@ -75,7 +75,7 @@ class GithubManifestVersion extends ConditionalGithubAuthService { } } -class DynamicGithubManifest extends ConditionalGithubAuthService { +class DynamicGithubManifest extends ConditionalGithubAuthV3Service { static get category() { return 'other' } diff --git a/services/github/github-package-json.service.js b/services/github/github-package-json.service.js index f0588af02a035..a4d81e663dbc4 100644 --- a/services/github/github-package-json.service.js +++ b/services/github/github-package-json.service.js @@ -11,7 +11,7 @@ const { getDependencyVersion, } = require('../package-json-helpers') const { semver } = require('../validators') -const { ConditionalGithubAuthService } = require('./github-auth-service') +const { ConditionalGithubAuthV3Service } = require('./github-auth-service') const { fetchJsonFromRepo } = require('./github-common-fetch') const { documentation } = require('./github-helpers') @@ -21,7 +21,7 @@ const versionSchema = Joi.object({ version: semver, }).required() -class GithubPackageJsonVersion extends ConditionalGithubAuthService { +class GithubPackageJsonVersion extends ConditionalGithubAuthV3Service { static get category() { return 'version' } @@ -78,7 +78,7 @@ class GithubPackageJsonVersion extends ConditionalGithubAuthService { } } -class GithubPackageJsonDependencyVersion extends ConditionalGithubAuthService { +class GithubPackageJsonDependencyVersion extends ConditionalGithubAuthV3Service { static get category() { return 'platform-support' } @@ -173,7 +173,7 @@ class GithubPackageJsonDependencyVersion extends ConditionalGithubAuthService { // This must be exported after GithubPackageJsonVersion in order for the // former to work correctly. -class DynamicGithubPackageJson extends ConditionalGithubAuthService { +class DynamicGithubPackageJson extends ConditionalGithubAuthV3Service { static get category() { return 'other' } diff --git a/services/github/github-pull-request-check-state.service.js b/services/github/github-pull-request-check-state.service.js index 3c764dc2d405f..81ab826fbe1cd 100644 --- a/services/github/github-pull-request-check-state.service.js +++ b/services/github/github-pull-request-check-state.service.js @@ -2,7 +2,7 @@ const Joi = require('@hapi/joi') const countBy = require('lodash.countby') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { fetchIssue } = require('./github-common-fetch') const { documentation, errorMessagesFor } = require('./github-helpers') @@ -19,7 +19,7 @@ const schema = Joi.object({ const keywords = ['pullrequest', 'detail'] -module.exports = class GithubPullRequestCheckState extends GithubAuthService { +module.exports = class GithubPullRequestCheckState extends GithubAuthV3Service { static get category() { return 'build' } diff --git a/services/github/github-release-date.service.js b/services/github/github-release-date.service.js index 7bbd8585b90aa..4685b86afd252 100644 --- a/services/github/github-release-date.service.js +++ b/services/github/github-release-date.service.js @@ -4,7 +4,7 @@ const moment = require('moment') const Joi = require('@hapi/joi') const { age } = require('../color-formatters') const { formatDate } = require('../text-formatters') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.alternatives( @@ -20,7 +20,7 @@ const schema = Joi.alternatives( .min(1) ) -module.exports = class GithubReleaseDate extends GithubAuthService { +module.exports = class GithubReleaseDate extends GithubAuthV3Service { static get category() { return 'activity' } diff --git a/services/github/github-release.service.js b/services/github/github-release.service.js index dec0aa48d3f97..743c3004834b8 100644 --- a/services/github/github-release.service.js +++ b/services/github/github-release.service.js @@ -2,10 +2,10 @@ const { addv } = require('../text-formatters') const { fetchLatestRelease } = require('./github-common-fetch') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation } = require('./github-helpers') -module.exports = class GithubRelease extends GithubAuthService { +module.exports = class GithubRelease extends GithubAuthV3Service { static get category() { return 'version' } diff --git a/services/github/github-repo-size.service.js b/services/github/github-repo-size.service.js index c802028fad73b..a16a3fe5ee24d 100644 --- a/services/github/github-repo-size.service.js +++ b/services/github/github-repo-size.service.js @@ -3,14 +3,14 @@ const Joi = require('@hapi/joi') const prettyBytes = require('pretty-bytes') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ size: nonNegativeInteger, }).required() -module.exports = class GithubRepoSize extends GithubAuthService { +module.exports = class GithubRepoSize extends GithubAuthV3Service { static get category() { return 'size' } diff --git a/services/github/github-search.service.js b/services/github/github-search.service.js index c2433ecf94b90..4393feff170c4 100644 --- a/services/github/github-search.service.js +++ b/services/github/github-search.service.js @@ -3,12 +3,12 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { errorMessagesFor, documentation } = require('./github-helpers') const schema = Joi.object({ total_count: nonNegativeInteger }).required() -module.exports = class GithubSearch extends GithubAuthService { +module.exports = class GithubSearch extends GithubAuthV3Service { static get category() { return 'analysis' } diff --git a/services/github/github-size.service.js b/services/github/github-size.service.js index 46b9a6cd2cd29..23a964c39a3b9 100644 --- a/services/github/github-size.service.js +++ b/services/github/github-size.service.js @@ -3,7 +3,7 @@ const Joi = require('@hapi/joi') const prettyBytes = require('pretty-bytes') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const { NotFound } = require('..') @@ -14,7 +14,7 @@ const schema = Joi.alternatives( Joi.array().required() ) -module.exports = class GithubSize extends GithubAuthService { +module.exports = class GithubSize extends GithubAuthV3Service { static get category() { return 'size' } diff --git a/services/github/github-stars.service.js b/services/github/github-stars.service.js index 832f635795ea9..c4592a6d7e885 100644 --- a/services/github/github-stars.service.js +++ b/services/github/github-stars.service.js @@ -3,14 +3,14 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ stargazers_count: nonNegativeInteger, }).required() -module.exports = class GithubStars extends GithubAuthService { +module.exports = class GithubStars extends GithubAuthV3Service { static get category() { return 'social' } diff --git a/services/github/github-tag.service.js b/services/github/github-tag.service.js index 40c9dc2c3e002..462aa17e84162 100644 --- a/services/github/github-tag.service.js +++ b/services/github/github-tag.service.js @@ -4,7 +4,7 @@ const Joi = require('@hapi/joi') const { addv } = require('../text-formatters') const { version: versionColor } = require('../color-formatters') const { latest } = require('../version') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const { NotFound } = require('..') @@ -21,7 +21,7 @@ const schema = Joi.alternatives() ) .required() -module.exports = class GithubTag extends GithubAuthService { +module.exports = class GithubTag extends GithubAuthV3Service { static get category() { return 'version' } diff --git a/services/github/github-watchers.service.js b/services/github/github-watchers.service.js index 35ea30f093352..813f62e226799 100644 --- a/services/github/github-watchers.service.js +++ b/services/github/github-watchers.service.js @@ -3,14 +3,14 @@ const Joi = require('@hapi/joi') const { metric } = require('../text-formatters') const { nonNegativeInteger } = require('../validators') -const { GithubAuthService } = require('./github-auth-service') +const { GithubAuthV3Service } = require('./github-auth-service') const { documentation, errorMessagesFor } = require('./github-helpers') const schema = Joi.object({ subscribers_count: nonNegativeInteger, }).required() -module.exports = class GithubWatchers extends GithubAuthService { +module.exports = class GithubWatchers extends GithubAuthV3Service { static get category() { return 'social' }