Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

SWAPI, Paging, DataLoader, Request ID #242

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 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
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,18 @@ In Windows:

- GraphQL
- [graphql-tools](https://github.com/apollographql/graphql-tools)
- [graphql-relay](https://github.com/graphql/graphql-relay-js)
- [graphql-relay-tools](https://github.com/excitement-engineer/graphql-relay-tools)
- [graphql-import](https://github.com/prisma/graphql-import)
- [graphql-tracing](https://github.com/apollographql/apollo-tracing)
- [apollo-server](https://github.com/apollographql/apollo-server)
- [graphql-voyager](https://apis.guru/graphql-voyager)
- [graphql-playground](https://github.com/graphcool/graphql-playground)
- [graphqlgen](https://github.com/prisma/graphqlgen)

- DataLoader
- [dataloader](https://github.com/facebook/dataloader)

- Jest
- [Documentation](https://facebook.github.io/jest/docs/en/getting-started.html)

Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@
"apollo-server-koa": "1.3.6",
"bluebird": "3.5.3",
"chalk": "2.4.2",
"dataloader": "1.4.0",
"envalid": "4.1.4",
"flashheart": "2.9.0",
"graphql": "14.0.2",
"graphql-cost-analysis": "1.0.2",
"graphql-import": "0.7.1",
"graphql-playground-middleware-koa": "1.6.8",
"graphql-relay": "0.5.5",
"graphql-relay-tools": "0.1.1",
"graphql-tools": "4.0.3",
"graphql-voyager": "1.0.0-rc.26",
"koa": "2.6.2",
Expand Down
52 changes: 52 additions & 0 deletions src/connectors/swapi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import DataLoader from 'dataloader';
import { createClient } from 'flashheart';
import logger from '../logger';

const http = createClient({ logger, timeout: 5000 });

async function getFromUrl(url) {
const response = await http.getAsync(url);
return response;
}

export const dataLoader = new DataLoader(urls =>
Promise.all(urls.map(getFromUrl)),
);

/**
* Given an object URL, fetch it, append the ID to it, and return it.
*/
export const getObjectFromUrl = async (url: string): Promise<any> => {
trioletas marked this conversation as resolved.
Show resolved Hide resolved
return await dataLoader.load(url);
};

/**
* Given a type, get the object with the ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

the function purpose can be easily derived from the name. does it make sense to rephrase the same in the comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, not at all. Comments left during code port. Removed.

*/
export const getObjectsFromType = async (type: string): Promise<any> => {
return await getObjectFromUrl(`${process.env.SWAPI_SERVICE_URL}/${type}/`);
};

/**
* Given a type and ID, get the object with the ID.
*/
export const getObjectFromTypeAndId = async (type: string, id: string): Promise<any> => {
const data = await getObjectFromUrl(`${process.env.SWAPI_SERVICE_URL}/${type}/${id}/`);
return objectWithId(data);
};

/**
* Given an objects URLs, fetch it, append the ID to it, sort it, and return it.
Copy link
Contributor

Choose a reason for hiding this comment

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

sort it

where?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed useless and incorrect comments (left from another project source code),

*/
export const getObjectsFromUrls = async (urls: string[]): Promise<any[]> => {
const array = await Promise.all(urls.map(getObjectFromUrl));
return array.map(objectWithId);
};

/**
* Objects returned from SWAPI don't have an ID field, so add one.
*/
export const objectWithId = (obj: {id: number, url: string}): Object => {
obj.id = parseInt(obj.url.split('/')[5], 10);
return obj;
};
1 change: 1 addition & 0 deletions src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const env = envalid.cleanEnv(process.env, {
SELF_URL: str({ devDefault: 'http://localhost:3001' }),
NODE_ENV: str({ devDefault: 'development' }),
JOKE_SERVICE_URL: url({ default: 'https://api.icndb.com' }),
SWAPI_SERVICE_URL: url({ default: 'https://swapi.co/api' }),
GRAPHQL_TRACING: bool({ default: true }),
GRAPHIQL: bool({ default: true }),
VOYAGER: bool({ default: true }),
Expand Down
56 changes: 56 additions & 0 deletions src/graphql/__tests__/__snapshots__/swapi.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`query.swapi should match snapshot with pagination 1`] = `
Object {
"data": Object {
"allFilms": Object {
"edges": Array [
Object {
"cursor": "YXJyYXljb25uZWN0aW9uOjA=",
"node": Object {
"episodeID": 4,
"title": "A New Hope",
},
},
Object {
"cursor": "YXJyYXljb25uZWN0aW9uOjE=",
"node": Object {
"episodeID": 2,
"title": "Attack of the Clones",
},
},
],
"films": Array [
Object {
"episodeID": 4,
"title": "A New Hope",
},
Object {
"episodeID": 2,
"title": "Attack of the Clones",
},
],
"pageInfo": Object {
"endCursor": "YXJyYXljb25uZWN0aW9uOjE=",
"hasNextPage": false,
"hasPreviousPage": false,
"startCursor": "YXJyYXljb25uZWN0aW9uOjA=",
},
"totalCount": 2,
},
},
"errors": undefined,
}
`;

exports[`query.swapi should match snapshot without pagination 1`] = `
Object {
"data": Object {
"film": Object {
"episodeID": 5,
"title": "The Empire Strikes Back",
},
},
"errors": undefined,
}
`;
93 changes: 93 additions & 0 deletions src/graphql/__tests__/swapi.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { graphql } from 'graphql';
import schema from '../schema';
import nock from 'nock';

const rootValue = {};
const context = {};

beforeAll(() => {

// mock service endpoint

const filmsResponse = {
count: 2,
next: null,
previous: null,
results: [
{
title: 'A New Hope',
episode_id: 4,
url: 'https://swapi.co/api/films/',
},
{
title: 'Attack of the Clones',
episode_id: 2,
url: 'https://swapi.co/api/films/',
},
],
};

nock(process.env.SWAPI_SERVICE_URL!)
.get('/films/')
.reply(200, filmsResponse);

const filmResponse = {
title: 'The Empire Strikes Back',
episode_id: 5,
url: 'https://swapi.co/api/films/2/',
};

nock(process.env.SWAPI_SERVICE_URL!)
.get('/films/2/')
.reply(200, filmResponse);
});

describe('query.swapi', () => {
it('should match snapshot with pagination', async () => {
const query = `
query Q {
allFilms {
edges {
node {
title
episodeID
}
cursor
}
pageInfo {
startCursor
endCursor
hasNextPage
hasPreviousPage
}
totalCount
films {
title
episodeID
}
}
}
`;

const result = await graphql(schema, query, rootValue, context);
const { data, errors } = result;

expect({ data, errors }).toMatchSnapshot();
});

it('should match snapshot without pagination', async () => {
const query = `
query Q {
film(filmID: 2) {
title
episodeID
}
}
`;

const result = await graphql(schema, query, rootValue, context);
const { data, errors } = result;

expect({ data, errors }).toMatchSnapshot();
});
});
19 changes: 19 additions & 0 deletions src/graphql/connection.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { connectionDefinitions } from 'graphql-relay-tools';

/**
* Constructs a GraphQL connection field config; it is assumed
* that the object has a property named `prop`, and that property
* contains a list of types.
*/
export function connectTypes(name: string, prop: string, type: string) {
const { connectionType } = connectionDefinitions({
name,
nodeType: type,
connectionFields: `
totalCount: Int
${prop}: [${type}]
`,
});

return connectionType;
}
76 changes: 76 additions & 0 deletions src/graphql/models/swapi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import {
getObjectFromUrl,
getObjectsFromUrls,
getObjectFromTypeAndId,
getObjectsFromType,
objectWithId,
} from '../../connectors/swapi';
Copy link
Contributor

Choose a reason for hiding this comment

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

To improve readability, I would suggest to import everything from the swapi connector and then just reference individual fns as such:

import * as swapi from '../../connectors/swapi';
....
const byTypeAndId = async (type: string, id: string): Promise<Object> =>
  // immediately clear that this model fn simply delegates to the connector module
  swapi.getObjectFromTypeAndId(type, id)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored.


type ObjectsByType = {
objects: Object[],
totalCount: number,
};

/**
* Given a type, fetch all of the pages, and join the objects together
*/
const byType = async (type: string): Promise<ObjectsByType> => {
const typeData = await getObjectsFromType(type);
let objects: Object[] = [];
let nextUrl = typeData.next;

objects = objects.concat(typeData.results.map(objectWithId));
while (nextUrl) {
// eslint-disable-next-line no-await-in-loop
Copy link
Contributor

Choose a reason for hiding this comment

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

eslint ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed (left from another project source code).

const pageData = await getObjectFromUrl(nextUrl);
objects = objects.concat(pageData.results.map(objectWithId));
nextUrl = pageData.next;
}

objects = sortObjectsById(objects);
return { objects, totalCount: objects.length };
};

/**
* Given a type and ID, get the object with the ID.
*/
const byTypeAndId = async (type: string, id: string): Promise<Object> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

await is kind of useless here, why not just

const byTypeAndId = async (type: string, id: string): Promise<Object> => getObjectFromTypeAndId(type, id)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored.

return await getObjectFromTypeAndId(type, id);
};

/**
* Given an object URL, fetch it, append the ID to it, and return it.
*/
const byUrl = async (url: string): Promise<any> => {
return await getObjectFromUrl(url);
};

/**
* Given an objects URLs, fetch it, append the ID to it, sort it, and return it.
*/
const byUrls = async (urls: string[]): Promise<any[]> => {
const array = await getObjectsFromUrls(urls);
return sortObjectsById(array);
};

const sortObjectsById = (array: any[]): Object[] => {
return array.sort((a, b) => a.id - b.id);
};

const convertToNumber = (value: string): number | null => {
if (['unknown', 'n/a'].indexOf(value) !== -1) {
return null;
}

// remove digit grouping
const numberString = value.replace(/,/, '');
return Number(numberString);
};

export {
byTypeAndId,
byType,
byUrl,
byUrls,
convertToNumber,
};
3 changes: 3 additions & 0 deletions src/graphql/resolvers/Node.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const node = {
__resolveType: () => null,
};
6 changes: 5 additions & 1 deletion src/graphql/resolvers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ import { Resolvers } from '../_generated/types';
import { query as Query } from './Query';
import { jokes as Jokes } from './Jokes';
import { joke as Joke } from './Joke';
import { node as Node } from './Node';

/** SWAPI resolvers */
Copy link
Contributor

Choose a reason for hiding this comment

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

redundant comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed.

import swapiResolvers from './swapi';

const resolvers: Resolvers = {
Query,
Jokes,
Joke,
};

export default merge(resolvers);
export default merge(resolvers, swapiResolvers, { Node });
Copy link
Contributor

Choose a reason for hiding this comment

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

why not spread over resolvers ?

const resolvers: Resolvers = {
 Query,
 Jokes,
 Joke,
...swapiResolvers,
{ Node }
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Generated TypeScript interface Resolvers does not contain SWAPI resolvers types.

31 changes: 31 additions & 0 deletions src/graphql/resolvers/swapi/Connection.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { connectionFromArray } from 'graphql-relay-tools';
import { byUrls, byType } from '../../models/swapi';

function connection(prop: string) {
return async (obj, args) => {
const array = await byUrls(obj[prop] || []);
const connObj = connectionFromArray(array, args);
return {
...connObj,
totalCount: array.length,
[prop]: _ => connObj.edges.map(edge => edge.node),
};
};
}

function rootConnection(swapiType) {
return async (_, args) => {
const { objects, totalCount } = await byType(swapiType);
const connObj = connectionFromArray(objects, args);
return {
...connObj,
totalCount,
[swapiType]: _ => connObj.edges.map(edge => edge.node),
};
};
}

export {
rootConnection,
connection,
};
Loading