Skip to content

Commit

Permalink
chore: return validated values from validators (#1259)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
1emu committed Sep 14, 2023
1 parent fef5c40 commit cfbd9fd
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 86 deletions.
9 changes: 3 additions & 6 deletions src/back/routes/badges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ export default routes((router) => {
})

async function getBadges(req: Request<{ address: string }>): Promise<UserBadges> {
const address = req.params.address
validateAddress(address)
const address = validateAddress(req.params.address)
return await BadgesService.getBadges(address)
}

Expand Down Expand Up @@ -85,7 +84,7 @@ async function uploadBadgeSpec(req: WithAuth): Promise<BadgeCreationResult> {

const { title, description, imgUrl, expiresAt } = req.body
validateRequiredStrings(['title', 'description', 'imgUrl'], req.body)
validateDate(expiresAt)
validateDate(expiresAt, 'optional')

try {
const result = await storeBadgeSpec({
Expand All @@ -104,9 +103,7 @@ async function createBadgeSpec(req: WithAuth): Promise<BadgeCreationResult> {
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) }
Expand Down
3 changes: 1 addition & 2 deletions src/back/routes/budget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ async function getCurrentContestedBudget(): Promise<BudgetWithContestants> {
}

async function getBudgetWithContestants(req: Request<{ proposal: string }>): Promise<BudgetWithContestants> {
const id = req.params.proposal
validateProposalId(id)
const id = validateProposalId(req.params.proposal)
return await BudgetService.getBudgetWithContestants(id)
}
3 changes: 1 addition & 2 deletions src/back/routes/coauthor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions src/back/routes/proposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -626,9 +625,8 @@ async function getGrants(): Promise<CategorizedGrants> {

// TODO: Still in use by user profile page.
async function getGrantsByUser(req: Request): ReturnType<typeof getGrants> {
const address = req.params.address
const address = validateAddress(req.params.address)
const isCoauthoring = req.query.coauthor === 'true'
validateAddress(address)

let coauthoringProposalIds = new Set<string>()

Expand Down
39 changes: 19 additions & 20 deletions src/back/routes/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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<SnapshotVote[]> {
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) {
Expand All @@ -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)
}
7 changes: 2 additions & 5 deletions src/back/routes/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<UserAttributes>({ address: address.toLowerCase() })

if (!user) {
Expand Down
11 changes: 4 additions & 7 deletions src/back/routes/votes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion src/back/services/vote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
40 changes: 10 additions & 30 deletions src/back/utils/validations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
21 changes: 12 additions & 9 deletions src/back/utils/validations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit cfbd9fd

Please sign in to comment.