feat: handle missing article version, update Dockerfile for dev setup (…

…#840)

* feat: handle missing article version, update Dockerfile for dev setup

This commit updates the Dockerfile to include a setup for development
environment. It also modifies the `getArticleVersion` function in
`preprints-controller.ts`, `model.ts`, `mongodb-repository.ts`, and
`mongodb-repository.test.ts` to handle cases where no matching article
version is found. Instead of throwing an error, it now returns null, and
handles the null case appropriately.

* rename getArticleVersions to findArticleVersions for better semantics

* move log to controller level where it is more apropriate

* Alter the lines in docker-compose to match Dockerfile

* add integration tests for invalid identifiers

* add missing result parameter to test response

---------

Co-authored-by: Will Byrne <[email protected]>
Co-authored-by: aisha <[email protected]>

Dockerfile

@@ -27,3 +27,6 @@ COPY src/ src/
 
 EXPOSE 3000
 CMD [ "yarn", "start" ]
+
+FROM prod as dev
+CMD [ "yarn", "start:dev" ]

docker-compose.yaml

@@ -60,8 +60,7 @@ services:
   api:
     build:
       context: .
-      target: prod
-    command: yarn start:dev
+      target: dev
     healthcheck:
       test: ["CMD-SHELL", "sh -c 'apk add curl; curl http://api:3000/'"]
       interval: 5s

integration-tests/api.test.ts

@@ -969,6 +969,16 @@ describe('server tests', () => {
           },
         });
     });
+
+    it('returns a 404 when an invalid identifier is used', async () => {
+      const app = createApp(articleStore);
+      await request(app)
+        .get('/api/preprints/thisisnotanid')
+        .expect(404, {
+          result: false,
+          message: 'no result found for: (thisisnotanid)',
+        });
+    });
   });
 
   describe('/api/citations/:publisherId/:articleId/bibtex', () => {

src/controller/preprints-controller.ts

@@ -49,16 +49,24 @@ export const preprintsController = (repo: ArticleRepository) => {
 
   const getPreprintsByIdentifier = async (req: Request, res: Response, next: NextFunction) => {
     try {
-      const version = await repo.getArticleVersion(req.params.identifier);
-      const { msid, versionIdentifier } = version.article;
-      const pdfUrl = `https://github.com/elifesciences/enhanced-preprints-data/raw/master/data/${msid}/v${versionIdentifier}/${msid}-v${versionIdentifier}.pdf`;
-      try {
-        const { status } = await axios.get(pdfUrl);
-        if (status === 200) {
-          version.article.pdfUrl = pdfUrl;
+      const version = await repo.findArticleVersion(req.params.identifier);
+      if (!version) {
+        logger.info(`Cannot find a matching article version (${req.params.identifier})`);
+        res.status(404).send({
+          result: false,
+          message: `no result found for: (${req.params.identifier})`,
+        });
+      } else {
+        const { msid, versionIdentifier } = version.article;
+        const pdfUrl = `https://github.com/elifesciences/enhanced-preprints-data/raw/master/data/${msid}/v${versionIdentifier}/${msid}-v${versionIdentifier}.pdf`;
+        try {
+          const { status } = await axios.get(pdfUrl);
+          if (status === 200) {
+            version.article.pdfUrl = pdfUrl;
+          }
+        } finally {
+          res.send(version);
         }
-      } finally {
-        res.send(version);
       }
     } catch (err) {
       next(err);

src/model/model.ts

@@ -156,7 +156,7 @@ export interface ArticleRepository {
   getArticle(id: string): Promise<ProcessedArticle>;
   getArticleSummaries(): Promise<ArticleSummary[]>;
   storeEnhancedArticle(article: EnhancedArticle): Promise<boolean>;
-  getArticleVersion(identifier: string): Promise<EnhancedArticleWithVersions>;
+  findArticleVersion(identifier: string): Promise<EnhancedArticleWithVersions | null>;
   getEnhancedArticleSummaries(): Promise<ArticleSummary[]>;
   deleteArticleVersion(identifier: string): Promise<boolean>;
 }

src/model/mongodb/mongodb-repository.test.ts

@@ -235,8 +235,8 @@ describe('article-stores', () => {
     }]));
   });
 
-  it('throws an error with unknown identifier', async () => {
-    expect(async () => articleStore.getArticleVersion('not-an-id')).rejects.toThrow();
+  it('returns null with unknown identifier', async () => {
+    expect(await articleStore.findArticleVersion('not-an-id')).toBeNull();
   });
 
   it('stores and retrieves a Versioned Article by id with all fields', async () => {
@@ -261,7 +261,7 @@ describe('article-stores', () => {
       },
     };
     const result = await articleStore.storeEnhancedArticle(inputArticle);
-    const article = await articleStore.getArticleVersion('testid1');
+    const article = await articleStore.findArticleVersion('testid1');
 
     expect(result).toStrictEqual(true);
     expect(article).toMatchObject({
@@ -303,7 +303,7 @@ describe('article-stores', () => {
       },
     };
     const result = await articleStore.storeEnhancedArticle(inputArticle);
-    const article = await articleStore.getArticleVersion('testid2');
+    const article = await articleStore.findArticleVersion('testid2');
 
     expect(result).toStrictEqual(true);
     expect(article).toMatchObject({
@@ -362,7 +362,7 @@ describe('article-stores', () => {
     };
     const result1 = await articleStore.storeEnhancedArticle(inputArticle1);
     const result2 = await articleStore.storeEnhancedArticle(inputArticle2);
-    const article = await articleStore.getArticleVersion('testid3.1');
+    const article = await articleStore.findArticleVersion('testid3.1');
 
     expect(result1).toStrictEqual(true);
     expect(result2).toStrictEqual(true);
@@ -432,7 +432,7 @@ describe('article-stores', () => {
     };
     const result1 = await articleStore.storeEnhancedArticle(inputArticle1);
     const result2 = await articleStore.storeEnhancedArticle(inputArticle2);
-    const article = await articleStore.getArticleVersion('testid4');
+    const article = await articleStore.findArticleVersion('testid4');
 
     expect(result1).toStrictEqual(true);
     expect(result2).toStrictEqual(true);

src/model/mongodb/mongodb-repository.ts

@@ -128,7 +128,7 @@ class MongoDBArticleRepository implements ArticleRepository {
     }));
   }
 
-  async getArticleVersion(identifier: string): Promise<EnhancedArticleWithVersions> {
+  async findArticleVersion(identifier: string): Promise<EnhancedArticleWithVersions | null> {
     const version = await this.versionedCollection.findOne(
       { $or: [{ _id: identifier }, { msid: identifier }] },
       {
@@ -140,7 +140,7 @@ class MongoDBArticleRepository implements ArticleRepository {
     );
 
     if (!version) {
-      throw Error('Cannot find a matching article version');
+      return null;
     }
 
     const allVersions = await this.versionedCollection.find<VersionSummary>(