From 0d43a9a3266b180f31fc88f16e17f8d4c267191c Mon Sep 17 00:00:00 2001 From: Chris Wilkinson Date: Thu, 23 Jan 2020 12:03:13 +0000 Subject: [PATCH] Try and compromise between the router and named nodes --- src/app.ts | 2 -- src/router.ts | 2 ++ src/routes/add-article.ts | 5 +++-- src/routes/article.ts | 13 +++---------- src/routes/index.ts | 1 + test/routes/article.test.ts | 39 +++++++++---------------------------- 6 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/app.ts b/src/app.ts index c3473e92..13b4e6f7 100644 --- a/src/app.ts +++ b/src/app.ts @@ -11,7 +11,6 @@ import errorHandler from './middleware/error-handler'; import jsonld from './middleware/jsonld'; import routing from './middleware/routing'; import namespaces from './namespaces'; -import article from './routes/article'; export type AppState = DefaultState; @@ -49,7 +48,6 @@ export default ( })); app.use(apiDocumentationLink(apiDocumentationPath)); app.use(errorHandler()); - app.use(article()); app.use(routing(router)); return app; diff --git a/src/router.ts b/src/router.ts index 585ed62b..9fd2b7fb 100644 --- a/src/router.ts +++ b/src/router.ts @@ -3,6 +3,7 @@ import { AppServiceContext, AppState } from './app'; import Routes from './routes'; import addArticle from './routes/add-article'; import apiDocumentation from './routes/api-documentation'; +import article from './routes/article'; import articleList from './routes/article-list'; import entryPoint from './routes/entry-point'; @@ -12,6 +13,7 @@ export default (): Router => { router.get(Routes.ApiDocumentation, '/doc', apiDocumentation()); router.get(Routes.ArticleList, '/articles', articleList()); router.post(Routes.AddArticle, '/articles', addArticle()); + router.get(Routes.Article, '/articles/:id', article()); router.get(Routes.EntryPoint, '/', entryPoint()); return router; diff --git a/src/routes/add-article.ts b/src/routes/add-article.ts index 328da076..18901032 100644 --- a/src/routes/add-article.ts +++ b/src/routes/add-article.ts @@ -8,10 +8,11 @@ import uniqueString from 'unique-string'; import url from 'url'; import { AppContext, AppMiddleware } from '../app'; import { rdf, schema } from '../namespaces'; +import Routes from './index'; export default (): AppMiddleware => ( async ({ - articles, dataFactory: { namedNode, quad }, request, response, + articles, dataFactory: { namedNode, quad }, request, response, router, }: AppContext, next: Next): Promise => { const id = clownface({ dataset: request.dataset }).has(rdf.type, schema.Article).term; @@ -27,7 +28,7 @@ export default (): AppMiddleware => ( throw new createHttpError.BadRequest(`Article must have at least one ${termToString(schema('name'))}`); } - const newId = namedNode(url.resolve(request.origin, 'articles/'.concat(uniqueString()))); + const newId = namedNode(url.resolve(request.origin, router.url(Routes.Article, uniqueString()))); [...request.dataset].forEach((originalQuad: Quad): void => { let newQuad: Quad; diff --git a/src/routes/article.ts b/src/routes/article.ts index d9a572c3..b39a467f 100644 --- a/src/routes/article.ts +++ b/src/routes/article.ts @@ -8,17 +8,8 @@ import ArticleNotFound from '../errors/article-not-found'; export default (): AppMiddleware => ( async ({ - path, articles, request, response, + articles, request, response, path, }: AppContext, next: Next): Promise => { - try { - await next(); - return; - } catch (error) { - if (!(error instanceof createHttpError.NotFound)) { - throw error; - } - } - try { response.dataset = await articles.get(namedNode(url.resolve(request.origin, path))); } catch (error) { @@ -30,5 +21,7 @@ export default (): AppMiddleware => ( } response.status = OK; + + await next(); } ); diff --git a/src/routes/index.ts b/src/routes/index.ts index 9f67ecaf..7089f0cb 100644 --- a/src/routes/index.ts +++ b/src/routes/index.ts @@ -1,6 +1,7 @@ enum Routes { 'AddArticle' = 'add-article', 'ApiDocumentation' = 'api-documentation', + 'Article' = 'article', 'ArticleList' = 'article-list', 'EntryPoint' = 'entry-point', } diff --git a/test/routes/article.test.ts b/test/routes/article.test.ts index 0c942970..be8bf6a8 100644 --- a/test/routes/article.test.ts +++ b/test/routes/article.test.ts @@ -1,21 +1,17 @@ -import { - namedNode, -} from '@rdfjs/data-model'; +import { namedNode } from '@rdfjs/data-model'; import createHttpError from 'http-errors'; import { OK } from 'http-status-codes'; import { Response } from 'koa'; import InMemoryArticles from '../../src/adaptors/in-memory-articles'; import Articles from '../../src/articles'; +import { WithDataset } from '../../src/middleware/dataset'; import article from '../../src/routes/article'; import createContext from '../context'; import createArticle from '../create-article'; -import runMiddleware, { NextMiddleware, throwingNext } from '../middleware'; -import { WithDataset } from '../../src/middleware/dataset'; +import runMiddleware, { NextMiddleware } from '../middleware'; const makeRequest = async ( - path: string, - articles?: Articles, - next: NextMiddleware = throwingNext(new createHttpError.NotFound()), + path: string, articles?: Articles, next?: NextMiddleware, ): Promise> => ( runMiddleware(article(), createContext({ articles, path }), next) ); @@ -31,21 +27,6 @@ describe('article', (): void => { expect(response.status).toBe(OK); }); - it('should not attempt article retrieval when next middleware throws not found http error', async (): Promise => { - const mockArticles: Articles = { - set: jest.fn(), - get: jest.fn(), - remove: jest.fn(), - contains: jest.fn(), - count: jest.fn(), - [Symbol.asyncIterator]: jest.fn(), - }; - - const next = jest.fn(); - await makeRequest('path-to/article/one', mockArticles, next); - expect(mockArticles.get).toHaveBeenCalledTimes(0); - }); - it('should throw an error if article is not found', async (): Promise => { const response = makeRequest('path-to/article/not-found'); @@ -53,15 +34,13 @@ describe('article', (): void => { await expect(response).rejects.toHaveProperty('message', 'Article http://example.com/path-to/article/not-found could not be found'); }); - it('should throw error raised in next middleware', async (): Promise => { - const response = makeRequest('path-to/article/not-found', undefined, throwingNext(new createHttpError.BadRequest())); - - await expect(response).rejects.toBeInstanceOf(createHttpError.BadRequest); - }); - it('should call the next middleware', async (): Promise => { + const id = namedNode('http://example.com/path-to/article/one'); + const articles = new InMemoryArticles(); + await articles.set(id, createArticle({ id })); const next = jest.fn(); - await makeRequest('path-to/article/one', undefined, next); + + await makeRequest('path-to/article/one', articles, next); expect(next).toHaveBeenCalledTimes(1); });