Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: return validated values from validators #1259

Merged
merged 5 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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