From f6bd3d96bdad068ece6bb96e1c8f7e27d999937b Mon Sep 17 00:00:00 2001 From: Ryan Larkin Date: Sun, 2 Aug 2020 15:35:50 +0100 Subject: [PATCH 1/3] status code cleanup --- src/server/routes/v2/manifests.ts | 2 +- src/server/routes/v2/packages.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/routes/v2/manifests.ts b/src/server/routes/v2/manifests.ts index b5a4269..63965ed 100644 --- a/src/server/routes/v2/manifests.ts +++ b/src/server/routes/v2/manifests.ts @@ -28,7 +28,7 @@ export default async (fastify: FastifyInstance): Promise => { const manifest = await manifestService.findManifestVersion(id, version); if (manifest == null) { response.code(404); - return new Error("manifest not found"); + throw new Error("manifest not found"); } return { diff --git a/src/server/routes/v2/packages.ts b/src/server/routes/v2/packages.ts index 3a99ecf..03c14cc 100644 --- a/src/server/routes/v2/packages.ts +++ b/src/server/routes/v2/packages.ts @@ -177,7 +177,7 @@ export default async (fastify: FastifyInstance): Promise => { const pkg = await packageService.findSinglePackage(publisher, packageName); if (pkg == null) { response.code(404); - return new Error("package not found"); + throw new Error("package not found"); } statsService.incrementAccessCount(`${publisher}.${packageName}`); From 297de104d66d660b734a66cc3cfe1aab0e984839 Mon Sep 17 00:00:00 2001 From: Ryan Larkin Date: Mon, 24 Aug 2020 18:49:05 +0100 Subject: [PATCH 2/3] throw error on missing necessary fields --- src/server/routes/v2/manifests.ts | 5 +++++ src/server/routes/v2/packages.ts | 12 +++++++++++- src/server/routes/v2/stats.ts | 7 ++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/server/routes/v2/manifests.ts b/src/server/routes/v2/manifests.ts index 63965ed..72724e3 100644 --- a/src/server/routes/v2/manifests.ts +++ b/src/server/routes/v2/manifests.ts @@ -25,6 +25,11 @@ export default async (fastify: FastifyInstance): Promise => { fastify.get("/:id/:version", { schema: manifestSchema }, async (request, response) => { const { id, version } = request.params; + if (id == null || version == null) { + response.code(404); + throw new Error("id or version not specified, please do that"); + } + const manifest = await manifestService.findManifestVersion(id, version); if (manifest == null) { response.code(404); diff --git a/src/server/routes/v2/packages.ts b/src/server/routes/v2/packages.ts index 03c14cc..b616677 100644 --- a/src/server/routes/v2/packages.ts +++ b/src/server/routes/v2/packages.ts @@ -154,7 +154,7 @@ export default async (fastify: FastifyInstance): Promise => { }; }); - fastify.get("/:publisher", { schema: publisherPackageSchema }, async request => { + fastify.get("/:publisher", { schema: publisherPackageSchema }, async (request, response) => { const { publisher } = request.params; const { take = DEFAULT_PAGE_SIZE, @@ -163,6 +163,11 @@ export default async (fastify: FastifyInstance): Promise => { order = SortOrder.ASCENDING, } = request.query; + if (publisher == null) { + response.code(404); + throw new Error("publisher not specified, please do that"); + } + const [pkgs, total] = await packageService.findByPublisher(publisher, take, page, sort, order); return { @@ -174,6 +179,11 @@ export default async (fastify: FastifyInstance): Promise => { fastify.get("/:publisher/:packageName", { schema: singlePackageSchema }, async (request, response) => { const { publisher, packageName } = request.params; + if (publisher == null || packageName == null) { + response.code(404); + throw Error("publisher or package name not specified, please do that"); + } + const pkg = await packageService.findSinglePackage(publisher, packageName); if (pkg == null) { response.code(404); diff --git a/src/server/routes/v2/stats.ts b/src/server/routes/v2/stats.ts index 3cc3c0c..a172f24 100644 --- a/src/server/routes/v2/stats.ts +++ b/src/server/routes/v2/stats.ts @@ -27,7 +27,7 @@ const statsSchema = { export default async (fastify: FastifyInstance): Promise => { const statsService = new StatsService(); - fastify.get("/", { schema: statsSchema }, async request => { + fastify.get("/", { schema: statsSchema }, async (request, response) => { const { packageId, resolution, @@ -35,6 +35,11 @@ export default async (fastify: FastifyInstance): Promise => { before = (new Date()).toISOString(), } = request.query; + if (packageId == null) { + response.code(404); + throw new Error("package id not specified, please do that"); + } + const stats = await statsService.getPackageStats(packageId, resolution, new Date(after), new Date(before)); return { From 3c3c99a12092ecb01a10abb553bd7c1caa5b2a47 Mon Sep 17 00:00:00 2001 From: Ryan Larkin Date: Tue, 25 Aug 2020 17:23:22 +0100 Subject: [PATCH 3/3] update --- src/database/service/package.ts | 6 +++++- src/server/routes/v2/manifests.ts | 5 ----- src/server/routes/v2/packages.ts | 16 +++------------- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/database/service/package.ts b/src/database/service/package.ts index 4d1e9fe..bbde6a5 100644 --- a/src/database/service/package.ts +++ b/src/database/service/package.ts @@ -1,5 +1,7 @@ import { getMongoRepository } from "typeorm"; +import fastify, { FastifyReply } from "fastify"; +import { ServerResponse } from "http"; import BaseService from "./base"; import PackageModel from "../model/package"; import { @@ -81,18 +83,21 @@ class PackageService extends BaseService { sort: PackageSortFields | "SearchScore", order: SortOrder, searchOptions: IPackageSearchOptions, + response: fastify.FastifyReply, ): Promise<[Omit[], number]> { // const optionFields = Object.values(queryOptions).filter(e => e != null); // error if query AND another field is set (query only requirement) if (queryOptions.query != null && optionFields.length > 1) { + response.code(400); throw new Error("no other queryOptions should be set when 'query' is non-null"); } // error if non-query fields are set and ensureContains is false, in which case // all non-query fields behave like a qquery and may be misleading if (queryOptions.query == null && optionFields.length >= 1 && searchOptions.ensureContains === false) { + response.code(400); throw new Error("non-query search parameters are redundant if ensureContains is false"); } @@ -280,7 +285,6 @@ class PackageService extends BaseService { sort: PackageSortFields, order: SortOrder, ): Promise<[Omit[], number]> { - // const pkgs = await this.repository.aggregate([ { $match: { diff --git a/src/server/routes/v2/manifests.ts b/src/server/routes/v2/manifests.ts index 72724e3..63965ed 100644 --- a/src/server/routes/v2/manifests.ts +++ b/src/server/routes/v2/manifests.ts @@ -25,11 +25,6 @@ export default async (fastify: FastifyInstance): Promise => { fastify.get("/:id/:version", { schema: manifestSchema }, async (request, response) => { const { id, version } = request.params; - if (id == null || version == null) { - response.code(404); - throw new Error("id or version not specified, please do that"); - } - const manifest = await manifestService.findManifestVersion(id, version); if (manifest == null) { response.code(404); diff --git a/src/server/routes/v2/packages.ts b/src/server/routes/v2/packages.ts index b616677..35603fb 100644 --- a/src/server/routes/v2/packages.ts +++ b/src/server/routes/v2/packages.ts @@ -117,7 +117,7 @@ export default async (fastify: FastifyInstance): Promise => { // NOTE: query searches name > publisher > description // NOTE: tags are exact match, separated by ',' - fastify.get("/", { schema: packageSchema }, async request => { + fastify.get("/", { schema: packageSchema }, async (request, response) => { const { query, name, @@ -146,7 +146,7 @@ export default async (fastify: FastifyInstance): Promise => { publisher, description, ...(tags == null ? {} : { tags: tags.split(",") }), - }, take, page, sort, order, searchOptions); + }, take, page, sort, order, searchOptions, response); return { Packages: pkgs, @@ -154,7 +154,7 @@ export default async (fastify: FastifyInstance): Promise => { }; }); - fastify.get("/:publisher", { schema: publisherPackageSchema }, async (request, response) => { + fastify.get("/:publisher", { schema: publisherPackageSchema }, async (request) => { const { publisher } = request.params; const { take = DEFAULT_PAGE_SIZE, @@ -163,11 +163,6 @@ export default async (fastify: FastifyInstance): Promise => { order = SortOrder.ASCENDING, } = request.query; - if (publisher == null) { - response.code(404); - throw new Error("publisher not specified, please do that"); - } - const [pkgs, total] = await packageService.findByPublisher(publisher, take, page, sort, order); return { @@ -179,11 +174,6 @@ export default async (fastify: FastifyInstance): Promise => { fastify.get("/:publisher/:packageName", { schema: singlePackageSchema }, async (request, response) => { const { publisher, packageName } = request.params; - if (publisher == null || packageName == null) { - response.code(404); - throw Error("publisher or package name not specified, please do that"); - } - const pkg = await packageService.findSinglePackage(publisher, packageName); if (pkg == null) { response.code(404);