Skip to content

Commit

Permalink
Revert "[mds-agency] Removing version param from agency payload to co…
Browse files Browse the repository at this point in the history
…nform with general v… (#331)"

This reverts commit f607f81.
  • Loading branch information
janedotx authored May 18, 2020
1 parent f607f81 commit 0a56b27
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 16 deletions.
27 changes: 20 additions & 7 deletions packages/mds-agency/request-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ export const registerVehicle = async (req: AgencyApiRequest, res: AgencyRegister
} catch (err) {
logger.error('writeRegisterEvent failure', err)
}
res.status(201).send({})
const { version } = res.locals
res.status(201).send({ version })
} catch (err) {
if (String(err).includes('duplicate')) {
res.status(409).send({
Expand Down Expand Up @@ -126,7 +127,8 @@ export const getVehicleById = async (req: AgencyApiRequest, res: AgencyGetVehicl
return
}
const compositeData = computeCompositeVehicleData(payload)
res.status(200).send({ ...compositeData })
const { version } = res.locals
res.status(200).send({ version, ...compositeData })
}

export const getVehiclesByProvider = async (req: AgencyApiRequest, res: AgencyGetVehiclesByProviderResponse) => {
Expand All @@ -147,7 +149,8 @@ export const getVehiclesByProvider = async (req: AgencyApiRequest, res: AgencyGe

try {
const response = await getVehicles(skip, take, url, req.query, provider_id)
return res.status(200).send({ ...response })
const { version } = res.locals
return res.status(200).send({ version, ...response })
} catch (err) {
logger.error('getVehicles fail', err)
return res.status(500).send(agencyServerError)
Expand Down Expand Up @@ -194,12 +197,15 @@ export const updateVehicle = async (req: AgencyApiRequest, res: AgencyUpdateVehi
} else {
const device = await db.updateDevice(device_id, provider_id, update)
// TODO should we warn instead of fail if the cache/stream doesn't work?
const { version } = res.locals
try {
await Promise.all([cache.writeDevice(device), stream.writeDevice(device)])
} catch (error) {
logger.warn(`Error writing to cache/stream ${error}`)
}
return res.status(201).send({})
return res.status(201).send({
version
})
}
} catch (err) {
await updateVehicleFail(req, res, provider_id, device_id, 'not found')
Expand Down Expand Up @@ -238,8 +244,10 @@ export const submitVehicleEvent = async (req: AgencyApiRequest, res: AgencySubmi
}

async function success() {
const { version } = res.locals
function fin() {
res.status(201).send({
version,
device_id,
status: EVENT_STATUS_MAP[event.event_type]
})
Expand Down Expand Up @@ -403,7 +411,9 @@ export const submitVehicleTelemetry = async (req: AgencyApiRequest, res: AgencyS
)
}
if (recorded_telemetry.length) {
const { version } = res.locals
res.status(201).send({
version,
result: `telemetry success for ${valid.length} of ${data.length}`,
recorded: now(),
unique: recorded_telemetry.length,
Expand Down Expand Up @@ -442,7 +452,8 @@ export const registerStop = async (req: AgencyApiRequest, res: AgencyRegisterSto
try {
isValidStop(stop)
const recorded_stop = await db.writeStop(stop)
return res.status(201).send({ ...recorded_stop })
const { version } = res.locals
return res.status(201).send({ version, ...recorded_stop })
} catch (error) {
if (error instanceof ValidationError) {
return res.status(400).send({ error })
Expand All @@ -460,7 +471,8 @@ export const readStop = async (req: AgencyApiRequest, res: AgencyReadStopRespons
if (!recorded_stop) {
return res.status(404).send({ error: new NotFoundError('Stop not found') })
}
res.status(200).send({ ...recorded_stop })
const { version } = res.locals
res.status(200).send({ version, ...recorded_stop })
} catch (err) {
res.status(500).send({ error: new ServerError() })
}
Expand All @@ -473,7 +485,8 @@ export const readStops = async (req: AgencyApiRequest, res: AgencyReadStopsRespo
if (!stops) {
return res.status(404).send({ error: new NotFoundError('No stops were found') })
}
res.status(200).send({ stops })
const { version } = res.locals
res.status(200).send({ version, stops })
} catch (err) {
return res.status(500).send({ error: new ServerError() })
}
Expand Down
22 changes: 22 additions & 0 deletions packages/mds-agency/tests/integration-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import { ApiServer } from '@mds-core/mds-api-server'
import { TEST1_PROVIDER_ID, TEST2_PROVIDER_ID } from '@mds-core/mds-providers'

import { api } from '../api'
import { AGENCY_API_DEFAULT_VERSION } from '../types'

/* eslint-disable-next-line no-console */
const log = console.log.bind(console)
Expand Down Expand Up @@ -332,6 +333,7 @@ describe('Tests API', () => {
.expect(201)
.end((err, result) => {
log('err', err, 'body', result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.value(result).hasHeader('content-type', APP_JSON)
done(err)
})
Expand All @@ -343,6 +345,7 @@ describe('Tests API', () => {
.expect(200)
.end((err, result) => {
// log(result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.string(result.body.vehicles[0].vehicle_id).is('test-id-1')
test.string(result.body.vehicles[0].status).is('removed')
test.string(result.body.links.first).contains('http')
Expand All @@ -358,6 +361,7 @@ describe('Tests API', () => {
.expect(200)
.end((err, result) => {
// log('----------', result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.object(result.body).match((obj: Device) => obj.device_id === DEVICE_UUID)
test.object(result.body).match((obj: Device) => obj.provider_id === TEST1_PROVIDER_ID)
test.object(result.body).match((obj: Device) => obj.status === VEHICLE_STATUSES.removed)
Expand All @@ -372,6 +376,7 @@ describe('Tests API', () => {
.expect(200)
.end((err, result) => {
// log('----------', result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.object(result.body).match((obj: Device) => obj.device_id === DEVICE_UUID)
test.object(result.body).match((obj: Device) => obj.provider_id === TEST1_PROVIDER_ID)
test.object(result.body).match((obj: Device) => obj.status === VEHICLE_STATUSES.removed)
Expand Down Expand Up @@ -426,6 +431,7 @@ describe('Tests API', () => {
.end((err, result) => {
// log('----> err', err, 'body', result.body)
// test.string(result.body.error).contains('already_registered')
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.value(result).hasHeader('content-type', APP_JSON)
done(err)
})
Expand All @@ -450,6 +456,7 @@ describe('Tests API', () => {
.set('Authorization', AUTH)
.expect(200)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.object(result.body).match((obj: Device) => obj.vehicle_id === NEW_VEHICLE_ID)
test.value(result).hasHeader('content-type', APP_JSON)
done(err)
Expand All @@ -461,6 +468,7 @@ describe('Tests API', () => {
.set('Authorization', AUTH)
.expect(200)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.object(result.body).match((obj: Device) => obj.vehicle_id === NEW_VEHICLE_ID)
test.value(result).hasHeader('content-type', APP_JSON)
done(err)
Expand Down Expand Up @@ -548,6 +556,7 @@ describe('Tests API', () => {
.expect(201)
.end((err, result) => {
testTimestamp += 20000
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.string(result.body.status).is('available')
done(err)
})
Expand All @@ -560,6 +569,7 @@ describe('Tests API', () => {
.expect(200)
.end((err, result) => {
log(result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.string(result.body.vehicles[0].vehicle_id).is('new-vehicle-id')
test.string(result.body.vehicles[0].status).is('available')
test.string(result.body.links.first).contains('http')
Expand All @@ -579,6 +589,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
test.string(result.body.status).is('unavailable')
done(err)
})
Expand All @@ -593,6 +604,7 @@ describe('Tests API', () => {
.expect(201)
.end((err, result) => {
log('post deregister response:', JSON.stringify(result.body))
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand Down Expand Up @@ -693,6 +705,8 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
// log('post event', result.body)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand Down Expand Up @@ -761,6 +775,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand Down Expand Up @@ -792,6 +807,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand All @@ -807,6 +823,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand All @@ -822,6 +839,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand All @@ -844,6 +862,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand All @@ -859,6 +878,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand Down Expand Up @@ -1066,6 +1086,7 @@ describe('Tests API', () => {
})
.expect(201)
.end((err, result) => {
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
done(err)
})
})
Expand Down Expand Up @@ -1116,6 +1137,7 @@ describe('Tests API', () => {
log('telemetry err', err)
} else {
// log('telemetry result', result)
test.object(result.body).hasProperty('version', AGENCY_API_DEFAULT_VERSION)
}
done(err)
})
Expand Down
20 changes: 16 additions & 4 deletions packages/mds-agency/tests/request-handlers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ import { Device, VEHICLE_TYPES } from '@mds-core/mds-types'
import db from '@mds-core/mds-db'
import cache from '@mds-core/mds-agency-cache'
import stream from '@mds-core/mds-stream'
import { AgencyApiRequest, AgencyApiResponse, AgencyGetVehiclesByProviderResponse } from '../types'
import {
AgencyApiRequest,
AgencyApiResponse,
AGENCY_API_DEFAULT_VERSION,
AgencyGetVehiclesByProviderResponse
} from '../types'
import {
registerVehicle,
getVehicleById,
Expand Down Expand Up @@ -243,6 +248,7 @@ describe('Agency API request handlers', () => {
} as any)
res.status = statusHandler
res.locals = getLocals(provider_id) as any
res.locals.version = AGENCY_API_DEFAULT_VERSION

const stubbedResponse = { total: 0, links: { first: '0', last: '0', prev: null, next: null }, vehicles: [] }
Sinon.replace(utils, 'getVehicles', Sinon.fake.resolves(stubbedResponse))
Expand All @@ -255,7 +261,7 @@ describe('Agency API request handlers', () => {
res
)
assert.equal(statusHandler.calledWith(200), true)
assert.equal(sendHandler.calledWith({ ...stubbedResponse }), true)
assert.equal(sendHandler.calledWith({ ...stubbedResponse, version: AGENCY_API_DEFAULT_VERSION }), true)
Sinon.restore()
})
})
Expand Down Expand Up @@ -285,7 +291,10 @@ describe('Agency API request handlers', () => {
'not found'
)
assert.equal(statusHandler.calledWith(404), true)
assert.equal(sendHandler.called, true)
assert.equal(
sendHandler.called,
true
)
Sinon.restore()
})

Expand Down Expand Up @@ -345,7 +354,10 @@ describe('Agency API request handlers', () => {
'not found'
)
assert.equal(statusHandler.calledWith(404), true)
assert.equal(sendHandler.called, true)
assert.equal(
sendHandler.called,
true
)
Sinon.restore()
})

Expand Down
9 changes: 5 additions & 4 deletions packages/mds-agency/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { UUID, Device, VehicleEvent, Telemetry, Timestamp, Recorded, VEHICLE_STATUS, Stop } from '@mds-core/mds-types'
import { MultiPolygon } from 'geojson'
import { ApiRequest, ApiClaims, ApiResponse } from '@mds-core/mds-api-server'
import { ApiRequest, ApiVersionedResponse, ApiClaims } from '@mds-core/mds-api-server'
import { Params, ParamsDictionary } from 'express-serve-static-core'

export const AGENCY_API_SUPPORTED_VERSIONS = ['0.4.1'] as const
Expand All @@ -11,18 +11,19 @@ export type AgencyApiRequest<P extends Params = ParamsDictionary> = ApiRequest<P

export type AgencyApiAccessTokenScopes = 'admin:all' | 'vehicles:read'

export type AgencyApiResponse<TBody = {}> = ApiResponse<
export type AgencyApiResponse<TBody = any> = ApiVersionedResponse<
AGENCY_API_SUPPORTED_VERSION,
ApiClaims<AgencyApiAccessTokenScopes> & {
provider_id: UUID
},
TBody
>

export type AgencyRegisterVehicleResponse = AgencyApiResponse
export type AgencyRegisterVehicleResponse = AgencyApiResponse<{}>

export type AgencyGetVehicleByIdResponse = AgencyApiResponse<CompositeVehicle>
export type AgencyGetVehiclesByProviderResponse = AgencyApiResponse<PaginatedVehiclesList>
export type AgencyUpdateVehicleResponse = AgencyApiResponse
export type AgencyUpdateVehicleResponse = AgencyApiResponse<{}>
export type AgencySubmitVehicleEventResponse = AgencyApiResponse<{
device_id: UUID
status: VEHICLE_STATUS
Expand Down
2 changes: 1 addition & 1 deletion packages/mds-api-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export type ApiQuery<Q1 extends string, Q2 extends string[] = never> = {

export interface ApiResponse<L = unknown, B = unknown>
extends express.Response<
B | { error: unknown; error_description?: string; error_details?: string[] } | { errors: unknown[] }
B | { error: unknown; error_description?: string; error_details?: string[] } | { errors: unknown[] } | {}
> {
locals: L
}
Expand Down

0 comments on commit 0a56b27

Please sign in to comment.