Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
163 changes: 153 additions & 10 deletions openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,62 @@ components:
type: string
description: The primary image for a Recommendation.

LegacyFeedItem:
type: object
required:
- id
- title
- url
- excerpt
- domain
- image_src
- raw_image_src
properties:
id:
type: integer
title:
type: string
url:
type: string
excerpt:
type: string
domain:
type: string
image_src:
type: string
raw_image_src:
type: string

LegacySettings:
type: object
properties:
spocsPerNewTabs:
type: number
domainAffinityParameterSets:
type: object
timeSegments:
type: array
items:
type: object
required:
- id
- startTime
- endTime
- weightPosition
properties:
id:
type: string
startTime:
type: integer
endTime:
type: integer
weightPosition:
type: number
Comment on lines +201 to +206
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have little context around this but if we mean to represent these types as Typescript types then we'll have to use number for all of them 🤔

Unless this is just for the openapi schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is an openapi schema, which distinguishes between integers and floating-point numbers.

recsExpireTime:
type: integer
version:
type: string

# securitySchemes roughly map to authentication middleware
securitySchemes:
# The following schemes prefixed with "WS" all constitute a `WebSession` auth.
Expand Down Expand Up @@ -199,8 +255,8 @@ paths:
operationId: getRecommendations
# Intentionally blank security. No auth required.
security:
- WSConsumerKeyAuth: []
- WSConsumerKeyAuthAlias: []
- WSConsumerKeyAuth: [ ]
- WSConsumerKeyAuthAlias: [ ]
parameters:
- name: count
in: query
Expand Down Expand Up @@ -274,21 +330,108 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
/v3/firefox/global-recs:
get:
deprecated: true
summary: Used by older versions of Firefox to get a list of Recommendations for a Locale and Region. This operation is performed anonymously and requires no auth.
description: Supports Fx desktop version 115 and below.
operationId: getGlobalRecs
# Intentionally blank security. No auth required.
security:
- WSConsumerKeyAuth: [ ]
- WSConsumerKeyAuthAlias: [ ]
parameters:
- in: query
name: version
description: API version
required: true
schema:
type: integer
minimum: 3
maximum: 3
default: 3
- in: query
name: locale_lang
description: Firefox locale
required: true
schema:
type: string
default: en-US
- in: query
name: region
description: Firefox region
required: false
schema:
type: string
- in: query
name: count
description: Maximum number of items to return
required: false
schema:
type: integer
minimum: 1
maximum: 50
default: 20
responses:
'200':
description: OK
content:
application/json:
schema:
type: object
required:
- status
- spocs
- settings
- recommendations
properties:
status:
type: integer
enum:
- 1
spocs:
type: array
settings:
$ref: "#/components/schemas/LegacySettings"
recommendations:
type: array
items:
$ref: "#/components/schemas/LegacyFeedItem"
'400':
description: Invalid request parameters
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
'500':
description: This proxy service encountered an unexpected error.
'502':
description: Services downstream from this proxy encountered an unexpected error.
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
'504':
description: Requests to downstream services timed out.
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorResponse"
/desktop/v1/recent-saves:
get:
summary: Gets a list of the most recent saves for a specific user.
description: Supports Fx desktop version 113 and up.
operationId: getRecentSaves
security:
# require all WS (WebSession) security schemes together
- WSUserAuth: []
WSSessionAuth: []
WSLookupAuth: []
WSConsumerKeyAuth: []
- WSUserAuth: []
WSSessionAuth: []
WSLookupAuth: []
WSConsumerKeyAuthAlias: []
- WSUserAuth: [ ]
WSSessionAuth: [ ]
WSLookupAuth: [ ]
WSConsumerKeyAuth: [ ]
- WSUserAuth: [ ]
WSSessionAuth: [ ]
WSLookupAuth: [ ]
WSConsumerKeyAuthAlias: [ ]
parameters:
- name: count
in: query
Expand Down
2 changes: 1 addition & 1 deletion src/api/desktop/recent-saves/response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('response', () => {
// quick test to show ajv behavior, may not be familiar
it('ajv returns errors when bad objects', () => {
const valid = validate({});
expect(!valid).toBeTruthy();
expect(valid).toBeFalsy();
// uncomment if you want info about how errors look
// throw validate.errors
expect((validate.errors as DefinedError[]).length).toBeGreaterThan(0);
Expand Down
2 changes: 1 addition & 1 deletion src/api/desktop/recommendations/response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const validate = ajv.compile(schema);
describe('response', () => {
it('ajv returns errors when bad objects', () => {
const valid = validate({});
expect(!valid).toBeTruthy();
expect(valid).toBeFalsy();
// uncomment if you want info about how errors look
// throw validate.errors
expect((validate.errors as DefinedError[]).length).toBeGreaterThan(0);
Expand Down
13 changes: 13 additions & 0 deletions src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const router = express.Router();

import config from '../config';
import Desktop from './desktop';
import V3 from './v3';
import CacheControlHandler from './lib/cacheControlHandler';
import WebSessionAuthHandler from '../auth/web-session/webSessionAuthHandler';

Expand All @@ -18,4 +19,16 @@ router.use(
Desktop
);

// register all /desktop routes
router.use(
'/v3',
// include auth if available
WebSessionAuthHandler,
// set Cache-control headers on all routes
// this can be overwritten on downstream routes with another handler
CacheControlHandler('private, max-age=1800', config),
// register legacy v3 sub-router
V3
);

export default router;
43 changes: 43 additions & 0 deletions src/api/v3/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import express, { NextFunction, Request, Response } from 'express';
import config from '../../config';
import CacheControlHandler from '../lib/cacheControlHandler';
import ConsumerKeyHandler from '../../auth/consumerKeyHandler';
import { GraphQLErrorHandler } from '../error/graphQLErrorHandler';
import { handleQueryParameters } from './inputs';
import { BFFFxError } from '../../bfffxError';
import Recommendations from '../../graphql-proxy/recommendations/recommendations';
import { forwardHeadersMiddleware } from '../../graphql-proxy/lib/client';
import { RecommendationsQueryVariables } from '../../generated/graphql/types';
import { GlobalRecsResponse, responseTransformer } from './response';

const router = express.Router();

router.get(
'/firefox/global-recs',
// request must include a consumer
ConsumerKeyHandler,
CacheControlHandler('public, max-age=1800', config),
async (req: Request, res: Response, next: NextFunction) => {
try {
const variables = handleQueryParameters(req.query);

if (variables instanceof BFFFxError) {
return next(variables);
}

const graphRes = await Recommendations({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future enhancement it: Given all requests in this repo is probably forwarding a request to the graph it would be great to have either a middleware or some factory to generate a graph client for us with all of the authentication parameters already handled.

auth: req.auth,
consumer_key: req.consumer_key,
forwardHeadersMiddleware: forwardHeadersMiddleware(res),
variables: variables as RecommendationsQueryVariables,
});

res.json(responseTransformer(graphRes) as GlobalRecsResponse);
} catch (error) {
const responseError = GraphQLErrorHandler(error);
return next(responseError);
}
}
);

export default router;
75 changes: 75 additions & 0 deletions src/api/v3/inputs.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { faker } from '@faker-js/faker';
import assert from 'assert';

import { RecommendationsQueryVariables } from '../../generated/graphql/types';
import {
GlobalRecsQueryParameterStrings,
handleQueryParameters,
setDefaultsAndCoerceTypes,
} from './inputs';

import { APIError, APIErrorResponse, BFFFxError } from '../../bfffxError';

describe('input.ts recommendations query parameters', () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great example of tests and covering all the conditionals in your unit tests.

describe('setDefaultsAndCoerceTypes', () => {
it('converts count to an integer and passes through others', () => {
const res = setDefaultsAndCoerceTypes({
count: '3',
locale_lang: 'preValidatedLocale',
region: 'preValidatedRegion',
});
expect(res).toMatchObject({
count: 3,
locale: 'preValidatedLocale',
region: 'preValidatedRegion',
});
});

it('sets count to 20 if no default is provided, values without defaults are undefined', () => {
const res = setDefaultsAndCoerceTypes({});
// validation should return an error in this case, validating defaults though
expect(res).toMatchObject({
count: 20,
});
});
});

describe('handleQueryParameters', () => {
it('returns errors if invalid query parameters', () => {
const params: GlobalRecsQueryParameterStrings = {
count: '-1',
};

const error = handleQueryParameters(params);
assert(error instanceof BFFFxError);
const errors = JSON.parse(error.stringResponse);
expect(errors).toEqual(
expect.objectContaining<APIErrorResponse>({
errors: expect.arrayContaining<Array<APIError>>([
expect.objectContaining<APIError>({
status: '400',
title: 'Bad Request',
}),
]),
})
);
});
Comment on lines +46 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Wondering if we can use toMatchObject like above 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy-pasted these, so I'm not 100% sure, but I believe that APIError contains additional attributes that we didn't want to lock in tests? I don't know which one is better in this case.


it('returns GraphQL query variables on success', () => {
const params: GlobalRecsQueryParameterStrings = {
count: faker.datatype.number({ min: 1, max: 30 }).toString(),
locale_lang: 'fr',
region: 'FR',
};

const variables = handleQueryParameters(params);
expect(variables).toStrictEqual(
expect.objectContaining<RecommendationsQueryVariables>({
count: parseInt(params.count, 10),
locale: params.locale_lang,
region: params.region,
})
);
});
});
});
Loading