From 31f79fbe9d4a73b5dd813a479011f0cc5e8185d7 Mon Sep 17 00:00:00 2001 From: "Robert St. John" Date: Wed, 7 Aug 2024 15:13:05 -0600 Subject: [PATCH] refactor(service): migrate read observations rest api to DI adapter layer --- .../adapters.observations.controllers.web.ts | 130 +++++++++++++++++- .../adapters.observations.db.mongoose.ts | 5 +- .../observations/app.api.observations.ts | 23 ++++ .../observations/entities.observations.ts | 19 ++- service/src/routes/observations.js | 102 -------------- ...pters.observations.controllers.web.test.ts | 39 ++++++ .../app/observations/app.observations.test.ts | 4 +- 7 files changed, 213 insertions(+), 109 deletions(-) diff --git a/service/src/adapters/observations/adapters.observations.controllers.web.ts b/service/src/adapters/observations/adapters.observations.controllers.web.ts index 016552375..2b0856164 100644 --- a/service/src/adapters/observations/adapters.observations.controllers.web.ts +++ b/service/src/adapters/observations/adapters.observations.controllers.web.ts @@ -1,11 +1,12 @@ import express from 'express' import { compatibilityMageAppErrorHandler } from '../adapters.controllers.web' -import { AllocateObservationId, ExoAttachment, ExoIncomingAttachmentContent, ExoObservation, ObservationRequest, ReadAttachmentContent, ReadAttachmentContentRequest, ReadObservation, SaveObservation, SaveObservationRequest, StoreAttachmentContent, StoreAttachmentContentRequest } from '../../app.api/observations/app.api.observations' -import { AttachmentStore, EventScopedObservationRepository, ObservationState } from '../../entities/observations/entities.observations' +import { AllocateObservationId, ExoAttachment, ExoIncomingAttachmentContent, ExoObservation, ObservationRequest, ReadAttachmentContent, ReadAttachmentContentRequest, ReadObservation, ReadObservations, ReadObservationsRequest, SaveObservation, SaveObservationRequest, StoreAttachmentContent, StoreAttachmentContentRequest } from '../../app.api/observations/app.api.observations' +import { AttachmentStore, EventScopedObservationRepository, ObservationState, ObservationStateName } from '../../entities/observations/entities.observations' import { MageEvent, MageEventId } from '../../entities/events/entities.events' -import busboy from 'busboy' import { invalidInput } from '../../app.api/app.api.errors' import { exoObservationModFromJson } from './adapters.observations.dto.ecma404-json' +import busboy from 'busboy' +import moment from 'moment' declare module 'express-serve-static-core' { interface Request { @@ -16,6 +17,7 @@ declare module 'express-serve-static-core' { export interface ObservationAppLayer { allocateObservationId: AllocateObservationId saveObservation: SaveObservation + readObservations: ReadObservations readObservation: ReadObservation storeAttachmentContent: StoreAttachmentContent readAttachmentContent: ReadAttachmentContent @@ -201,9 +203,131 @@ export function ObservationRoutes(app: ObservationAppLayer, attachmentStore: Att next(appRes.error) }) + routes.route('/') + .get(async (req, res, next) => { + const readSpec: Pick = parseObservationQueryParams(req) + const appReq = createAppRequest(req, readSpec) + const appRes = await app.readObservations(appReq) + if (appRes.success) { + return res.json(appRes.success.map(x => jsonForObservation(x, qualifiedBaseUrl(req)))) + } + next(appRes.error) + }) + return routes.use(compatibilityMageAppErrorHandler) } +/** + * Attempt to parse the given string to an array of numbers that represents a + * bounding box of the form [ xMin, yMin, xMax, yMax ]. This does not validate + * lat/lon bounds, only array length and number type. The string can be a + * JSON string number array (deprecated), e.g., `'[ 1, 2, 3, 4 ]'`, or a comma- + * separated list, e.g., `'1,2,3,4'`. + */ +function parseBBox(maybeBBoxString: any): number[] | null { + if (typeof maybeBBoxString !== 'string') { + return null + } + let parsed: number[] = [] + try { + // TODO: move this geometryFormat.parse() call down to mongodb repository + // filter.geometries = geometryFormat.parse('bbox', bbox) + // TODO: would be better not to embed json strings in query parameters; use csv instead + parsed = JSON.parse(maybeBBoxString) + if (!Array.isArray(parsed)) { + return null + } + return null + } + catch(err) { + console.debug('invalid json string from query parameter `bbox`', maybeBBoxString, err) + } + // try csv instead of json + // TODO: this should be the only supported format + if (!parsed) { + parsed = maybeBBoxString.split(',').map(parseFloat) + } + if (parsed.length !== 4 && parsed.some(x => typeof x !== 'number' || isNaN(x))) { + return null + } + return parsed +} + +/** + * Parse {@link ObservationStateName} strings from the given input string. This expects the input string to be + * comma-separated values with no spaces. Only parse the first N state names, where N is number of valid state names. + * Return null if the input is not a string or contains no valid state names. + */ +function parseStatesParam(maybeStatesString: any): ObservationStateName[] | null { + if (typeof maybeStatesString !== 'string') { + return null + } + const allStateNames = Object.values(ObservationStateName) + const states = maybeStatesString.split(',', allStateNames.length).reduce((states: Set, stateName: any) => { + if (allStateNames.includes(stateName) && !states.has(stateName)) { + return states.add(stateName) + } + return states + }, new Set()) + return states.size > 0 ? Array.from(states.values()) : null +} + +const allowedSortFields = { + lastModified: true, + timestamp: true, +} as Record + +/** + * Parse a sort field specification of the form `field+order`, where `field` is the name of an observation field, + * and `order` is `desc`, `-`, or `-1`, to indicate a descending sort. The default sort order is ascending. Only the + * first valid sort field is used + */ +function parseSortParam(maybeSortString: any): ReadObservationsRequest['sort'] | null { + if (typeof maybeSortString !== 'string') { + return null + } + const sort = maybeSortString.split(',').reduce<{ field: string, order: 1 | -1 }[]>((sort, sortFieldSpec) => { + const [ name, orderString ] = sortFieldSpec.split('+') + const order = orderString?.toLowerCase() === 'desc' || orderString === '-' || orderString === '-1' ? -1 : 1 + if (allowedSortFields[name] === true) { + return [ ...sort, { field: name, order } ] + } + return sort + }, [] as { field: string, order: 1 | -1 }[])[0] + return sort || null +} + +function parseObservationQueryParams(req: express.Request): Pick { + const filter: ReadObservationsRequest['filter'] = {} + const startDate = req.query.startDate + if (startDate) { + filter.lastModifiedAfter = moment(String(startDate)).utc().toDate() + } + const endDate = req.query.endDate + if (endDate) { + filter.lastModifiedBefore = moment(String(endDate)).utc().toDate() + } + const observationStartDate = req.query.observationStartDate + if (observationStartDate) { + filter.timestampAfter = moment(String(observationStartDate)).utc().toDate() + } + const observationEndDate = req.query.observationEndDate + if (observationEndDate) { + filter.timestampBefore = moment(String(observationEndDate)).utc().toDate() + } + const bboxParam = parseBBox(req.query.bbox) + if (!bboxParam) { + console.warn('invalid bbox query parameter', req.query.bbox) + } + const states = parseStatesParam(req.query.states) + if (states) { + filter.states = states + } + const sort = parseSortParam(req.query.sort) || void(0) + const populate = req.query.populate === 'true' + return { filter, sort, populate } +} + export type WebObservation = Omit & { url: string state?: WebObservationState diff --git a/service/src/adapters/observations/adapters.observations.db.mongoose.ts b/service/src/adapters/observations/adapters.observations.db.mongoose.ts index dbc484b92..5a6324a9e 100644 --- a/service/src/adapters/observations/adapters.observations.db.mongoose.ts +++ b/service/src/adapters/observations/adapters.observations.db.mongoose.ts @@ -183,7 +183,7 @@ ObservationIdSchema.set('toJSON', { transform: transformObservationModelInstance export const StateSchema = new Schema({ name: { type: String, required: true }, userId: { type: Schema.Types.ObjectId, ref: 'User' } -}); +}) export const ThumbnailSchema = new Schema( { @@ -252,6 +252,9 @@ ObservationSchema.index({ 'attachments.oriented': 1 }) ObservationSchema.index({ 'attachments.contentType': 1 }) ObservationSchema.index({ 'attachments.thumbnails.minDimension': 1 }) +/** + * TODO: add support for mongoose `lean()` queries + */ export class MongooseObservationRepository extends BaseMongooseRepository implements EventScopedObservationRepository { constructor(model: ObservationModel, readonly idModel: ObservationIdModel, readonly eventScope: MageEventId, readonly eventLookup: (eventId: MageEventId) => Promise, readonly domainEvents: EventEmitter) { diff --git a/service/src/app.api/observations/app.api.observations.ts b/service/src/app.api/observations/app.api.observations.ts index 8fb7691ff..bf6871627 100644 --- a/service/src/app.api/observations/app.api.observations.ts +++ b/service/src/app.api/observations/app.api.observations.ts @@ -43,6 +43,29 @@ export interface ReadObservation { (req: ReadObservationRequest): Promise> } +export interface ReadObservationsRequest extends ObservationRequest { + filter: { + lastModifiedAfter?: Date | undefined, + lastModifiedBefore?: Date | undefined, + timestampAfter?: Date | undefined, + timestampBefore?: Date | undefined, + bbox?: GeoJSON.BBox, + states?: ObservationState['name'][] | undefined, + }, + sort?: { + field: string, + order?: 1 | -1, + }, + /** + * If `true`, populate the user names for the observation {@link ObservationAttrs.userId creator} and + * {@link ObservationImportantFlag.userId important flag}. + */ + populate?: boolean | undefined +} +export interface ReadObservations { + (req: ReadObservationsRequest): Promise> +} + export interface StoreAttachmentContent { (req: StoreAttachmentContentRequest): Promise> } diff --git a/service/src/entities/observations/entities.observations.ts b/service/src/entities/observations/entities.observations.ts index dcadbba4e..00d5c5f86 100644 --- a/service/src/entities/observations/entities.observations.ts +++ b/service/src/entities/observations/entities.observations.ts @@ -16,6 +16,9 @@ export interface ObservationAttrs extends Feature | undefined @@ -69,9 +72,23 @@ export interface ObservationImportantFlag { description?: string } +export enum ObservationStateName { + Active = 'active', + /** + * This state essentially marks the observation as deleted. The mobile apps use this so the server still returns + * deleted observations in queries and the mobile apps can delete their local records, or at least mark them deleted + * and hide them from view. + * TODO: actually delete the observation data and return only deleted observation IDs to clients + */ + Archived = 'archive', +} + +/** + * TODO: State changes should have a timestamp if we are bothering to track them. + */ export interface ObservationState { id: string | PendingEntityId - name: 'active' | 'archive' + name: ObservationStateName userId?: UserId | undefined /** * @deprecated TODO: confine URLs to the web layer diff --git a/service/src/routes/observations.js b/service/src/routes/observations.js index a1d839b12..9091d772a 100644 --- a/service/src/routes/observations.js +++ b/service/src/routes/observations.js @@ -5,16 +5,12 @@ module.exports = function (app, security) { , archiver = require('archiver') , path = require('path') , environment = require('../environment/env') - , moment = require('moment') , access = require('../access') , { default: turfCentroid } = require('@turf/centroid') - , geometryFormat = require('../format/geoJsonFormat') , observationXform = require('../transformers/observation') , passport = security.authentication.passport , { defaultEventPermissionsService: eventPermissions } = require('../permissions/permissions.events'); - const sortColumnWhitelist = ["lastModified"]; - function transformOptions(req) { return { event: req.event, @@ -113,84 +109,6 @@ module.exports = function (app, security) { }); } - function parseQueryParams(req, res, next) { - // setup defaults - const parameters = { - filter: { - } - }; - - const fields = req.query.fields; - if (fields) { - parameters.fields = JSON.parse(fields); - } - - const startDate = req.query.startDate; - if (startDate) { - parameters.filter.startDate = moment(startDate).utc().toDate(); - } - - const endDate = req.query.endDate; - if (endDate) { - parameters.filter.endDate = moment(endDate).utc().toDate(); - } - - const observationStartDate = req.query.observationStartDate; - if (observationStartDate) { - parameters.filter.observationStartDate = moment(observationStartDate).utc().toDate(); - } - - const observationEndDate = req.query.observationEndDate; - if (observationEndDate) { - parameters.filter.observationEndDate = moment(observationEndDate).utc().toDate(); - } - - const bbox = req.query.bbox; - if (bbox) { - parameters.filter.geometries = geometryFormat.parse('bbox', bbox); - } - - const geometry = req.query.geometry; - if (geometry) { - parameters.filter.geometries = geometryFormat.parse('geometry', geometry); - } - - const states = req.query.states; - if (states) { - parameters.filter.states = states.split(','); - } - - const sort = req.query.sort; - if (sort) { - const columns = {}; - let err = null; - sort.split(',').every(function (column) { - const sortParams = column.split('+'); - // Check sort column is in whitelist - if (sortColumnWhitelist.indexOf(sortParams[0]) === -1) { - err = `Cannot sort on column '${sortParams[0]}'`; - return false; // break - } - // Order can be nothing (ASC by default) or ASC, DESC - let direction = 1; // ASC - if (sortParams.length > 1 && sortParams[1] === 'DESC') { - direction = -1; // DESC - } - columns[sortParams[0]] = direction; - }); - if (err) { - return res.status(400).send(err); - } - parameters.sort = columns; - } - - parameters.populate = req.query.populate === 'true'; - - req.parameters = parameters; - - next(); - } - app.get( '/api/events/:eventId/observations/(:observationId).zip', passport.authenticate('bearer'), @@ -239,26 +157,6 @@ module.exports = function (app, security) { } ); - app.get( - '/api/events/:eventId/observations', - passport.authenticate('bearer'), - validateObservationReadAccess, - parseQueryParams, - function (req, res, next) { - const options = { - filter: req.parameters.filter, - fields: req.parameters.fields, - sort: req.parameters.sort, - populate: req.parameters.populate - }; - - new api.Observation(req.event).getAll(options, function (err, observations) { - if (err) return next(err); - res.json(observationXform.transform(observations, transformOptions(req))); - }); - } - ); - app.put( '/api/events/:eventId/observations/:observationIdInPath/favorite', passport.authenticate('bearer'), diff --git a/service/test/adapters/observations/adapters.observations.controllers.web.test.ts b/service/test/adapters/observations/adapters.observations.controllers.web.test.ts index 11e43f0fc..45685686e 100644 --- a/service/test/adapters/observations/adapters.observations.controllers.web.test.ts +++ b/service/test/adapters/observations/adapters.observations.controllers.web.test.ts @@ -166,6 +166,45 @@ describe('observations web controller', function () { }) }) + describe('GET /observations', function() { + + describe('query string parsing', function() { + + it('parses bbox as a json string', async function() { + // TODO: this format should be deprecated + expect.fail('todo') + }) + + it('parses bbox as csv', async function() { + expect.fail('todo') + }) + + it('parses min last modified date', async function() { + expect.fail('todo') + }) + + it('parses max last modified date', async function() { + expect.fail('todo') + }) + + it('parses min timestamp', async function() { + expect.fail('todo') + }) + + it('parses max timestamp', async function() { + expect.fail('todo') + }) + + it('parses populate flag', async function() { + expect.fail('todo') + }) + + it('parses sort field', async function() { + expect.fail('todo') + }) + }) + }) + describe('GET /observations/{observationId}', function() { it('has tests', async function() { diff --git a/service/test/app/observations/app.observations.test.ts b/service/test/app/observations/app.observations.test.ts index 7a3ffe8a4..142fb381b 100644 --- a/service/test/app/observations/app.observations.test.ts +++ b/service/test/app/observations/app.observations.test.ts @@ -1545,13 +1545,13 @@ describe('observations use case interactions', function() { describe('reading observations', function() { - describe('reading by id', function() { + describe('finding one by id', function() { it('has tests', async function() { expect.fail('todo') }) }) - describe('searching', function() { + describe('reading many', function() { it('has tests', async function() { expect.fail('todo') })