From cfbd9fd2f46bbd674582d89bf185b0b7b0db89d7 Mon Sep 17 00:00:00 2001 From: lemu Date: Thu, 14 Sep 2023 15:52:50 -0300 Subject: [PATCH] chore: return validated values from validators (#1259) * refactor: return validated dates * refactor: return validated proposal id, remove optional validation * refactor: return validated address * refactor: return validated proposal snapshot id * refactor: return validated required string --- src/back/routes/badges.ts | 9 +++---- src/back/routes/budget.ts | 3 +-- src/back/routes/coauthor.ts | 3 +-- src/back/routes/proposal.ts | 6 ++--- src/back/routes/snapshot.ts | 39 ++++++++++++++--------------- src/back/routes/user.ts | 7 ++---- src/back/routes/votes.ts | 11 +++----- src/back/services/vote.ts | 2 +- src/back/utils/validations.test.ts | 40 ++++++++---------------------- src/back/utils/validations.ts | 21 +++++++++------- 10 files changed, 55 insertions(+), 86 deletions(-) diff --git a/src/back/routes/badges.ts b/src/back/routes/badges.ts index ca44e9d07..50ee2398c 100644 --- a/src/back/routes/badges.ts +++ b/src/back/routes/badges.ts @@ -33,8 +33,7 @@ export default routes((router) => { }) async function getBadges(req: Request<{ address: string }>): Promise { - const address = req.params.address - validateAddress(address) + const address = validateAddress(req.params.address) return await BadgesService.getBadges(address) } @@ -85,7 +84,7 @@ async function uploadBadgeSpec(req: WithAuth): Promise { const { title, description, imgUrl, expiresAt } = req.body validateRequiredStrings(['title', 'description', 'imgUrl'], req.body) - validateDate(expiresAt) + validateDate(expiresAt, 'optional') try { const result = await storeBadgeSpec({ @@ -104,9 +103,7 @@ async function createBadgeSpec(req: WithAuth): Promise { const user = req.auth validateDebugAddress(user) - const { badgeCid } = req.body - validateRequiredString('badgeCid', badgeCid) - + const badgeCid = validateRequiredString('badgeCid', req.body.badgeCid) try { const result = await createSpec(badgeCid) return { status: ActionStatus.Success, badgeCid: JSON.stringify(result) } diff --git a/src/back/routes/budget.ts b/src/back/routes/budget.ts index 3fb14f0f2..3916e27ce 100644 --- a/src/back/routes/budget.ts +++ b/src/back/routes/budget.ts @@ -43,7 +43,6 @@ async function getCurrentContestedBudget(): Promise { } async function getBudgetWithContestants(req: Request<{ proposal: string }>): Promise { - const id = req.params.proposal - validateProposalId(id) + const id = validateProposalId(req.params.proposal) return await BudgetService.getBudgetWithContestants(id) } diff --git a/src/back/routes/coauthor.ts b/src/back/routes/coauthor.ts index e6a927917..1c8159a6c 100644 --- a/src/back/routes/coauthor.ts +++ b/src/back/routes/coauthor.ts @@ -44,8 +44,7 @@ export async function filterCoauthorRequests(requests: CoauthorAttributes[]) { } export async function getProposals(req: Request) { - const address = req.params.address - validateAddress(address) + const address = validateAddress(req.params.address) const status = toCoauthorStatusType(req.params.status) const requests = await CoauthorModel.findProposals(address, status) return await filterCoauthorRequests(requests) diff --git a/src/back/routes/proposal.ts b/src/back/routes/proposal.ts index a30d8b79a..d2d2a7bd0 100644 --- a/src/back/routes/proposal.ts +++ b/src/back/routes/proposal.ts @@ -489,8 +489,7 @@ export async function createProposal(proposalInCreation: ProposalInCreation) { } export async function getProposal(req: Request<{ proposal: string }>) { - const id = req.params.proposal - validateProposalId(id) + const id = validateProposalId(req.params.proposal) try { return await ProposalService.getProposal(id) } catch (e) { @@ -626,9 +625,8 @@ async function getGrants(): Promise { // TODO: Still in use by user profile page. async function getGrantsByUser(req: Request): ReturnType { - const address = req.params.address + const address = validateAddress(req.params.address) const isCoauthoring = req.query.coauthor === 'true' - validateAddress(address) let coauthoringProposalIds = new Set() diff --git a/src/back/routes/snapshot.ts b/src/back/routes/snapshot.ts index c0c8410de..e8d31071a 100644 --- a/src/back/routes/snapshot.ts +++ b/src/back/routes/snapshot.ts @@ -5,7 +5,12 @@ import { Request } from 'express' import { SnapshotVote } from '../../clients/SnapshotGraphqlTypes' import { SnapshotService } from '../../services/SnapshotService' -import { validateAddress, validateDates, validateFields, validateProposalSnapshotId } from '../utils/validations' +import { + validateAddress, + validateDates, + validateProposalFields, + validateProposalSnapshotId, +} from '../utils/validations' export default routes((router) => { router.get('/snapshot/status-space/:spaceName', handleAPI(getStatusAndSpace)) @@ -30,40 +35,36 @@ async function getAddressesVotes(req: Request) { } async function getProposalVotes(req: Request<{ proposalSnapshotId?: string }>) { - const { proposalSnapshotId } = req.params - validateProposalSnapshotId(proposalSnapshotId) - - return await SnapshotService.getProposalVotes(proposalSnapshotId!) + const proposalSnapshotId = validateProposalSnapshotId(req.params.proposalSnapshotId) + return await SnapshotService.getProposalVotes(proposalSnapshotId) } async function getAllVotesBetweenDates(req: Request): Promise { const { start, end } = req.body - validateDates(start, end) + const { validatedStart, validatedEnd } = validateDates(start, end) - return await SnapshotService.getAllVotesBetweenDates(new Date(start), new Date(end)) + return await SnapshotService.getAllVotesBetweenDates(validatedStart, validatedEnd) } async function getProposals(req: Request) { const { start, end, fields } = req.body - validateDates(start, end) - validateFields(fields) + const { validatedStart, validatedEnd } = validateDates(start, end) + validateProposalFields(fields) - return await SnapshotService.getProposals(new Date(start), new Date(end), fields) + return await SnapshotService.getProposals(validatedStart, validatedEnd, fields) } async function getPendingProposals(req: Request) { const { start, end, fields, limit } = req.body - validateDates(start, end) - validateFields(fields) + const { validatedStart, validatedEnd } = validateDates(start, end) + validateProposalFields(fields) - return await SnapshotService.getPendingProposals(new Date(start), new Date(end), fields, limit) + return await SnapshotService.getPendingProposals(validatedStart, validatedEnd, fields, limit) } async function getVpDistribution(req: Request<{ address: string; proposalSnapshotId?: string }>) { const { address, proposalSnapshotId } = req.params - validateAddress(address) - - return await SnapshotService.getVpDistribution(address, proposalSnapshotId) + return await SnapshotService.getVpDistribution(validateAddress(address), proposalSnapshotId) } async function getScores(req: Request) { @@ -76,8 +77,6 @@ async function getScores(req: Request) { } async function getProposalScores(req: Request<{ proposalSnapshotId?: string }>) { - const { proposalSnapshotId } = req.params - validateProposalSnapshotId(proposalSnapshotId) - - return await SnapshotService.getProposalScores(proposalSnapshotId!) + const proposalSnapshotId = validateProposalSnapshotId(req.params.proposalSnapshotId) + return await SnapshotService.getProposalScores(proposalSnapshotId) } diff --git a/src/back/routes/user.ts b/src/back/routes/user.ts index 8f9720792..d7daffad8 100644 --- a/src/back/routes/user.ts +++ b/src/back/routes/user.ts @@ -81,8 +81,7 @@ async function checkValidationMessage(req: WithAuth) { } async function isValidated(req: Request) { - const address = req.params.address - validateAddress(address) + const address = validateAddress(req.params.address) try { return await UserModel.isForumValidated(address) } catch (error) { @@ -93,9 +92,7 @@ async function isValidated(req: Request) { } async function getProfile(req: Request) { - const address = req.params.address - validateAddress(address) - + const address = validateAddress(req.params.address) const user = await UserModel.findOne({ address: address.toLowerCase() }) if (!user) { diff --git a/src/back/routes/votes.ts b/src/back/routes/votes.ts index acf1c9546..c5e9e3550 100644 --- a/src/back/routes/votes.ts +++ b/src/back/routes/votes.ts @@ -25,9 +25,7 @@ export default routes((route) => { export async function getProposalVotes(req: Request<{ proposal: string }>) { const refresh = req.query.refresh === 'true' - const id = req.params.proposal - - validateProposalId(id) + const id = validateProposalId(req.params.proposal) const proposal = await ProposalService.getProposal(id) const latestVotes = await VoteService.getVotes(id) @@ -77,8 +75,7 @@ export async function getCachedVotes(req: Request) { } async function getAddressVotesWithProposals(req: Request) { - const address = req.params.address - validateAddress(address) + const address = validateAddress(req.params.address) const first = Number(req.query.first) || undefined const skip = Number(req.query.skip) || undefined @@ -117,8 +114,8 @@ async function getAddressVotesWithProposals(req: Request) { async function getTopVoters(req: Request) { const { start, end, limit } = req.body - validateDates(start, end) + const { validatedStart, validatedEnd } = validateDates(start, end) const validLimit = isNumber(limit) ? limit : undefined - return await VoteService.getTopVoters(start, end, validLimit) + return await VoteService.getTopVoters(validatedStart, validatedEnd, validLimit) } diff --git a/src/back/services/vote.ts b/src/back/services/vote.ts index b1ead6162..8b8b7ac57 100644 --- a/src/back/services/vote.ts +++ b/src/back/services/vote.ts @@ -13,7 +13,7 @@ export class VoteService { } static async getTopVoters(start: Date, end: Date, limit = DEFAULT_TOP_VOTERS_LIMIT) { - const votes = await SnapshotService.getAllVotesBetweenDates(new Date(start), new Date(end)) + const votes = await SnapshotService.getAllVotesBetweenDates(start, end) return this.getSortedVoteCountPerUser(votes).slice(0, limit) } diff --git a/src/back/utils/validations.test.ts b/src/back/utils/validations.test.ts index bd01057f2..5d3446a09 100644 --- a/src/back/utils/validations.test.ts +++ b/src/back/utils/validations.test.ts @@ -5,39 +5,19 @@ import { validateProposalId } from './validations' describe('validateProposalId', () => { const UUID = '00000000-0000-0000-0000-000000000000' - describe('when id is optional', () => { - it('should not throw an error when not provided', () => { - expect(() => validateProposalId(undefined, 'optional')).not.toThrow() - }) - - it('should not throw an error when it is an empty string', () => { - expect(() => validateProposalId('', 'optional')).not.toThrow(RequestError) - }) - - it('should throw an error for proposal id with spaces', () => { - expect(() => validateProposalId(' ', 'optional')).toThrow(RequestError) - }) - - it('should not throw an error for a valid proposal id', () => { - expect(() => validateProposalId(UUID, 'optional')).not.toThrow() - }) + it('should not throw an error for a valid proposal id', () => { + expect(() => validateProposalId(UUID)).not.toThrow() }) - describe('when it is required', () => { - it('should not throw an error for a valid proposal id', () => { - expect(() => validateProposalId(UUID)).not.toThrow() - }) - - it('should throw an error for a missing required proposal id', () => { - expect(() => validateProposalId(undefined)).toThrow(RequestError) - }) + it('should throw an error for a missing required proposal id', () => { + expect(() => validateProposalId(undefined)).toThrow(RequestError) + }) - it('should throw an error for an empty required proposal id', () => { - expect(() => validateProposalId('')).toThrow(RequestError) - }) + it('should throw an error for an empty required proposal id', () => { + expect(() => validateProposalId('')).toThrow(RequestError) + }) - it('should throw an error for proposal id with spaces', () => { - expect(() => validateProposalId(' ', 'optional')).toThrow(RequestError) - }) + it('should throw an error for proposal id with spaces', () => { + expect(() => validateProposalId(' ')).toThrow(RequestError) }) }) diff --git a/src/back/utils/validations.ts b/src/back/utils/validations.ts index fd019499e..4a86807e6 100644 --- a/src/back/utils/validations.ts +++ b/src/back/utils/validations.ts @@ -6,14 +6,16 @@ import { SnapshotProposal } from '../../clients/SnapshotGraphqlTypes' import isDebugAddress from '../../entities/Debug/isDebugAddress' export function validateDates(start?: string, end?: string) { - validateDate(start) - validateDate(end) + const validatedStart = new Date(validateDate(start)!) + const validatedEnd = new Date(validateDate(end)!) + return { validatedStart, validatedEnd } } export function validateDate(date?: string, required?: 'optional') { if ((required !== 'optional' && !(date && isValidDate(date))) || (date && !isValidDate(date))) { throw new RequestError('Invalid date', RequestError.BadRequest) } + return date } function isValidDate(date: string) { @@ -22,7 +24,7 @@ function isValidDate(date: string) { return !isNaN(parsedDate.getTime()) } -export function validateFields(fields: unknown) { +export function validateProposalFields(fields: unknown) { if (!fields || !Array.isArray(fields) || fields.length === 0) { throw new RequestError('Invalid fields', RequestError.BadRequest) } @@ -56,31 +58,32 @@ export function validateFields(fields: unknown) { } } -export function validateProposalId(id?: string, required?: 'optional') { - if (required !== 'optional' && (!id || !isUUID(id))) { - throw new RequestError('Invalid proposal id', RequestError.BadRequest) - } - if (id && !isUUID(id)) { +export function validateProposalId(id?: string) { + if (!(id && isUUID(id))) { throw new RequestError('Invalid proposal id', RequestError.BadRequest) } + return id } -export function validateAddress(address: string) { +export function validateAddress(address?: string) { if (!address || !isEthereumAddress(address)) { throw new RequestError(`Invalid address ${address}`, RequestError.BadRequest) } + return address } export function validateProposalSnapshotId(proposalSnapshotId?: string) { if (!proposalSnapshotId || proposalSnapshotId.length === 0) { throw new RequestError('Invalid snapshot id') } + return proposalSnapshotId } export function validateRequiredString(fieldName: string, value?: string) { if (!value || value.length === 0) { throw new RequestError(`Invalid ${fieldName}`, RequestError.BadRequest) } + return value } // eslint-disable-next-line @typescript-eslint/no-explicit-any