Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

Commit

Permalink
Fix invalid db filter on transactions api
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdean-digicatapult committed Feb 13, 2024
1 parent ff42326 commit 043d0b6
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 31 deletions.
18 changes: 16 additions & 2 deletions src/controllers/v1/attachment/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Database from '../../../../lib/db/index.js'
import { octetExample, jsonExample } from './fixtures.js'
import Ipfs from '../../../../lib/ipfs.js'
import { BadRequest, InternalServerError, NotFound } from '../../../../lib/error-handler/index.js'
import { AttachmentRow } from '../../../../lib/db/types.js'
import { AttachmentRow, Where } from '../../../../lib/db/types.js'

describe('v1/attachment', () => {
let response: any
Expand Down Expand Up @@ -171,7 +171,8 @@ describe('v1/attachment', () => {

describe('getAll() - GET /', () => {
beforeEach(async () => {
stubs.get.callsFake(async () => [octetExample, jsonExample]), (response = await controller.getAll())
stubs.get.callsFake(async () => [octetExample, jsonExample])
response = await controller.getAll()
})

describe('if none found it', () => {
Expand Down Expand Up @@ -204,6 +205,19 @@ describe('v1/attachment', () => {
},
])
})

describe('passes created_after condition', () => {
beforeEach(async () => {
response = await controller.getAll('2024-01-01')
})

it('should fetch from db with conditions', () => {
const expectedCondition = [['created_at', '>=', new Date('2024-01-01')]] as Where<'attachment'>

expect(stubs.get.lastCall.args[0]).to.equal('attachment')
expect(stubs.get.lastCall.args[1]).to.deep.equal(expectedCondition)
})
})
})

describe('getById() - GET /{id}', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/v1/attachment/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ export class attachment extends Controller {

@Get('/')
@SuccessResponse(200, 'returns all attachment')
public async getAll(@Query() createdAt?: DATE): Promise<ListAttachmentsResponse> {
public async getAll(@Query() created_after?: DATE): Promise<ListAttachmentsResponse> {
this.log.debug('retrieving all attachments')

return await this.db.get('attachment', createdAt ? [['created_at', '>=', new Date(createdAt)]] : undefined)
return await this.db.get('attachment', created_after ? [['created_at', '>=', new Date(created_after)]] : undefined)
}

@Post('/')
Expand Down
18 changes: 17 additions & 1 deletion src/controllers/v1/certificate/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import ChainNode from '../../../../lib/chainNode.js'
import Commitment from '../../../../lib/services/commitment.js'
import EmissionsCalculator from '../../../../lib/services/emissionsCalculator.js'
import { BadRequest, InternalServerError, NotFound } from '../../../../lib/error-handler/index.js'
import { CertificateRow, TransactionRow } from '../../../../lib/db/types.js'
import { CertificateRow, TransactionRow, Where } from '../../../../lib/db/types.js'
import { certExamples, attachmentExample, transactionExample } from './fixtures.js'

describe('v1/certificate', () => {
Expand Down Expand Up @@ -170,6 +170,22 @@ describe('v1/certificate', () => {
hydrogen_owner: 'heidi',
})
})

describe('with params', () => {
beforeEach(async () => {
response = await controller.getAll('2024-01-01', '2024-01-02')
})

it('should fetch from db with conditions', () => {
const expectedCondition = [
['created_at', '>=', new Date('2024-01-01')],
['updated_at', '>=', new Date('2024-01-02')],
] as Where<'certificate'>

expect(stubs.get.lastCall.args[0]).to.equal('certificate')
expect(stubs.get.lastCall.args[1]).to.deep.equal(expectedCondition)
})
})
})

describe('getById() - GET /{id}', () => {
Expand Down
7 changes: 4 additions & 3 deletions src/controllers/v1/certificate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,10 @@ export class CertificateController extends Controller {
*/
@Get('/')
@Response<NotFound>(404, '<item> not found')
public async getAll(@Query() createdAt?: DATE): Promise<ListCertificatesResponse> {
const where: Where<'certificate'> = {}
if (createdAt) where.created_at = new Date(createdAt)
public async getAll(@Query() created_after?: DATE, @Query() updated_after?: DATE): Promise<ListCertificatesResponse> {
const where: Where<'certificate'> = []
if (created_after) where.push(['created_at', '>=', new Date(created_after)])
if (updated_after) where.push(['updated_at', '>=', new Date(updated_after)])

const found = await this.db.get('certificate', where)
const certificates = await this.mapIdentities(found)
Expand Down
16 changes: 14 additions & 2 deletions src/controllers/v1/transaction/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,19 @@ describe('v1/transaction', () => {
response = await controller.get(api_type)

expect(getStub.lastCall.args[0]).to.equal('transaction')
expect(getStub.lastCall.args[1]).to.to.deep.contain({ api_type })
expect(getStub.lastCall.args[1].length).to.equal(1)
expect(getStub.lastCall.args[1][0]).to.deep.contain({ api_type })
expect(response[1]).to.deep.contain(example)
})

it(` - [${api_type}] with args`, async () => {
response = await controller.get(api_type, 'finalised', '2024-01-01', '2024-01-02')

expect(getStub.lastCall.args[0]).to.equal('transaction')
expect(getStub.lastCall.args[1].length).to.equal(3)
expect(getStub.lastCall.args[1][0]).to.deep.equal({ api_type, state: 'finalised' })
expect(getStub.lastCall.args[1][1]).to.deep.equal(['created_at', '>=', new Date('2024-01-01')])
expect(getStub.lastCall.args[1][2]).to.deep.equal(['updated_at', '>=', new Date('2024-01-02')])
expect(response[1]).to.deep.contain(example)
})
})
Expand All @@ -60,7 +72,7 @@ describe('v1/transaction', () => {
})
})

it('returns transactions by updated_since query param', async () => {
it('returns transactions by updated_after query param', async () => {
response = await controller.get(undefined, undefined, '2024-01-10')

expect(getStub.lastCall.args[0]).to.equal('transaction')
Expand Down
17 changes: 11 additions & 6 deletions src/controllers/v1/transaction/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
ListTransactionResponse,
TransactionState,
} from '../../../models/transaction.js'
import { Where } from '../../../lib/db/types.js'

@injectable()
@Route('v1/transaction')
Expand All @@ -36,13 +37,17 @@ export class TransactionController extends Controller {
public async get(
@Query() api_type?: TransactionApiType,
@Query() state?: TransactionState,
@Query() updated_since?: DATE
@Query() created_after?: DATE,
@Query() updated_after?: DATE
): Promise<ListTransactionResponse> {
const where: { state?: TransactionState; api_type?: TransactionApiType; updated_since?: DATE } = {
state,
api_type,
updated_since,
}
const where: Where<'transaction'> = [
{
state,
api_type,
},
]
if (created_after) where.push(['created_at', '>=', new Date(created_after)])
if (updated_after) where.push(['updated_at', '>=', new Date(updated_after)])

return this.db.get('transaction', where)
}
Expand Down
13 changes: 3 additions & 10 deletions src/lib/db/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { singleton } from 'tsyringe'

import { pgConfig } from './knexfile.js'
import Zod, { tablesList, IDatabase, TABLE, Models, Where, Update, Order } from './types.js'
import { reduceWhere } from './util.js'

const clientSingleton = knex(pgConfig)

Expand Down Expand Up @@ -45,10 +46,7 @@ export default class Database {
...updates,
updated_at: this.client.fn.now(),
})
if (!Array.isArray(where)) {
where = [where]
}
query = where.reduce((acc, w) => (Array.isArray(w) ? acc.where(w[0], w[1], w[2]) : acc.where(w)), query)
query = reduceWhere(query, where)

return z.array(Zod[model].get).parse(await query.returning('*'))
}
Expand All @@ -60,12 +58,7 @@ export default class Database {
limit?: number
): Promise<Models[typeof model]['get'][]> => {
let query = this.db[model]()
if (where) {
if (!Array.isArray(where)) {
where = [where]
}
query = where.reduce((acc, w) => (Array.isArray(w) ? acc.where(w[0], w[1], w[2]) : acc.where(w)), query)
}
query = reduceWhere(query, where)
if (order && order.length !== 0) {
query = order.reduce((acc, [key, direction]) => acc.orderBy(key, direction), query)
}
Expand Down
29 changes: 29 additions & 0 deletions src/lib/db/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import knex from 'knex'
import { TABLE, Where } from './types.js'

// reduces the where condition on a knex query. Gracefully handles undefined values in WhereMatch objects
export const reduceWhere = <M extends TABLE>(
query: knex.Knex.QueryBuilder,
where?: Where<M>
): knex.Knex.QueryBuilder => {
if (where) {
if (!Array.isArray(where)) {
where = [where]
}
query = where.reduce((acc, w) => {
if (Array.isArray(w)) {
return acc.where(w[0], w[1], w[2])
}
return acc.where(
Object.entries(w).reduce(
(acc, [k, v]) => {
if (v !== undefined) acc[k] = v
return acc
},
{} as Record<string, unknown>
)
)
}, query)
}
return query
}
1 change: 0 additions & 1 deletion src/models/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@ export type HEX = `0x${string}`
* ISO 8601 date string
* @pattern (\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))|(\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d([+-][0-2]\d:[0-5]\d|Z))
* @format date
* @example "2023-05-04T09:47:32.393Z"
*/
export type DATE = string
8 changes: 4 additions & 4 deletions test/integration/offchain/attachment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ describe('attachment', () => {
expect(body).to.equal('attachment not found')
})

it('returns 422 with invalid updated_since date', async () => {
const { status, body } = await get(app, `/v1/attachment?createdAt=foo`)
it('returns 422 with invalid created_after date', async () => {
const { status, body } = await get(app, `/v1/attachment?created_after=foo`)
expect(status).to.equal(422)
expect(body).to.contain({
name: 'ValidateError',
Expand Down Expand Up @@ -75,7 +75,7 @@ describe('attachment', () => {
})

it('filters attachments based on created date', async () => {
const { status, body: attachments } = await get(app, `/v1/attachment?createdAt=2023-01-01T00:00:00.000Z`)
const { status, body: attachments } = await get(app, `/v1/attachment?created_after=2023-01-01T00:00:00.000Z`)
expect(status).to.equal(200)
expect(attachments).to.deep.equal([
{
Expand All @@ -86,7 +86,7 @@ describe('attachment', () => {
})

it('returns empty array if none found based on created date', async () => {
const { status, body } = await get(app, `/v1/attachment?createdAt=3010-01-01T00:00:00.000Z`)
const { status, body } = await get(app, `/v1/attachment?created_after=3010-01-01T00:00:00.000Z`)
expect(status).to.equal(200)
expect(body).to.deep.equal([])
})
Expand Down

0 comments on commit 043d0b6

Please sign in to comment.