a_adli/colouring-montreal
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve errors and data access for building update

Browse Source
This commit is contained in:
Maciej Ziarkowski 2020-03-22 19:03:05 +00:00
parent 786389a50b
commit aebf7c7135
8 changed files with 228 additions and 164 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
app/src/api/api.ts
View File

@ -103,19 +103,22 @@ server.use((err: any, req: express.Request, res: express.Response, next: express
}
if (err != undefined) {
console.log('Global error handler: ', err);
if (err instanceof ApiUserError) {
let errorMessage: string;
if(err instanceof ApiParamError) {
errorMessage = `Problem with parameter ${err.paramName}: ${err.message}`;
} else {
errorMessage = err.message;
}
res.status(400).send({ error: errorMessage });
} else if(err instanceof DatabaseError){
return res.status(400).send({ error: errorMessage });
}
// we need to log the error only if it's not an api user error
console.log('Global error handler: ', err);
if(err instanceof DatabaseError){
res.status(500).send({ error: 'Database error' });
} else {
res.status(500).send({ error: 'Server error' });

39
app/src/api/controllers/buildingController.ts
View File

@ -1,5 +1,7 @@
import express from 'express';
import { ApiUserError } from '../errors/api';
import { UserError } from '../errors/general';
import { parsePositiveIntParam, processParam } from '../parameters';
import asyncController from '../routes/asyncController';
import * as buildingService from '../services/building';
@ -67,19 +69,17 @@ async function updateBuilding(req: express.Request, res: express.Response, userI
const buildingUpdate = req.body;
let updatedBuilding: object;
try {
const building = await buildingService.saveBuilding(buildingId, buildingUpdate, userId);
if (typeof (building) === 'undefined') {
return res.send({ error: 'Database error' });
updatedBuilding = await buildingService.saveBuilding(buildingId, buildingUpdate, userId);
} catch(error) {
if(error instanceof UserError) {
throw new ApiUserError(error.message, error);
}
if (building.error) {
return res.send(building);
}
res.send(building);
} catch(err) {
res.send({ error: 'Database error' });
throw error;
}
res.send(updatedBuilding);
}
// GET building UPRNs
@ -137,21 +137,20 @@ const updateBuildingLikeById = asyncController(async (req: express.Request, res:
const buildingId = processParam(req.params, 'building_id', parsePositiveIntParam, true);
const { like } = req.body;
let updatedBuilding: object;
try {
const building = like ?
updatedBuilding = like ?
await buildingService.likeBuilding(buildingId, req.session.user_id) :
await buildingService.unlikeBuilding(buildingId, req.session.user_id);
if (building.error) {
return res.send(building);
}
if (typeof (building) === 'undefined') {
return res.send({ error: 'Database error' });
}
res.send(building);
} catch(error) {
res.send({ error: 'Database error' });
if(error instanceof UserError) {
throw new ApiUserError(error.message, error);
}
throw error;
}
res.send(updatedBuilding);
});
const getLatestRevisionId = asyncController(async (req: express.Request, res: express.Response) => {

3
app/src/api/controllers/editHistoryController.ts
View File

@ -22,8 +22,7 @@ const getGlobalEditHistory = asyncController(async (req: express.Request, res: e
res.send(result);
} catch(error) {
if(error instanceof ArgumentError && error.argumentName === 'count') {
const apiErr = new ApiParamError(error.message, 'count');
throw apiErr;
throw new ApiParamError(error.message, error, 'count');
}
throw error;

82
app/src/api/dataAccess/building.ts Normal file
View File

@ -0,0 +1,82 @@
import { errors, ITask } from 'pg-promise';
import db from '../../db';
import { ArgumentError, DatabaseError } from '../errors/general';
export async function getBuildingData(
buildingId: number,
lockForUpdate: boolean = false,
t?: ITask<any>
) {
let buildingData;
try {
buildingData = await (t || db).one(
`SELECT * FROM buildings WHERE building_id = $1${lockForUpdate ? ' FOR UPDATE' : ''};`,
[buildingId]
);
} catch(error) {
if(
error instanceof errors.QueryResultError &&
error.code === errors.queryResultErrorCode.noData
) {
throw new ArgumentError(`Building ID ${buildingId} does not exist`, 'buildingId');
}
throw new DatabaseError(error);
}
return buildingData;
}
export async function insertEditHistoryRevision(
buildingId: number,
userId: string,
forwardPatch: object,
reversePatch: object,
t?: ITask<any>
): Promise<string> {
try {
const { log_id } = await (t || db).one(
`INSERT INTO logs (
forward_patch, reverse_patch, building_id, user_id
) VALUES (
$1:json, $2:json, $3, $4
) RETURNING log_id
`,
[forwardPatch, reversePatch, buildingId, userId]
);
return log_id;
} catch(error) {
throw new DatabaseError(error);
}
}
export async function updateBuildingData(
buildingId: number,
forwardPatch: object,
revisionId: string,
t?: ITask<any>
): Promise<object> {
const sets = db.$config.pgp.helpers.sets(forwardPatch);
console.log('Setting', buildingId, sets);
try {
return await (t || db).one(
`UPDATE
buildings
SET
revision_id = $1,
$2:raw
WHERE
building_id = $3
RETURNING
*
`,
[revisionId, sets, buildingId]
);
} catch(error) {
throw new DatabaseError(error);
}
}

47
app/src/api/dataAccess/like.ts Normal file
View File

@ -0,0 +1,47 @@
import { errors, ITask } from 'pg-promise';
import db from '../../db';
import { DatabaseError, InvalidOperationError } from '../errors/general';
export async function getBuildingLikeCount(buildingId: number, t?: ITask<any>): Promise<number> {
try {
const result = await (t || db).one(
'SELECT count(*) as likes FROM building_user_likes WHERE building_id = $1;',
[buildingId]
);
return result.likes;
} catch(error) {
throw new DatabaseError(error);
}
}
export async function addBuildingUserLike(buildingId: number, userId: string, t?: ITask<any>): Promise<void> {
try {
return await (t || db).none(
'INSERT INTO building_user_likes ( building_id, user_id ) VALUES ($1, $2);',
[buildingId, userId]
);
} catch(error) {
if(error.detail?.includes('already exists')) {
throw new InvalidOperationError('User already likes this building');
}
throw new DatabaseError(error);
}
}
export async function removeBuildingUserLike(buildingId: number, userId: string, t?: ITask<any>): Promise<void> {
let result;
try {
result = await t.result(
'DELETE FROM building_user_likes WHERE building_id = $1 AND user_id = $2;',
[buildingId, userId]
);
} catch(error) {
throw new DatabaseError(error);
}
if (result.rowCount === 0) {
throw new InvalidOperationError("User doesn't like the building, cannot unlike");
}
}

23
app/src/api/errors/api.ts
View File

@ -6,39 +6,34 @@
*/
export class ApiUserError extends Error {
constructor(message?: string) {
public originalError: Error;
constructor(message?: string, originalError?: Error) {
super(message);
this.name = 'ApiUserError';
this.originalError = originalError;
}
}
export class ApiParamError extends ApiUserError {
public paramName: string;
constructor(message?: string, paramName?: string) {
super(message);
constructor(message?: string, originalError?: Error, paramName?: string) {
super(message, originalError);
this.name = 'ApiParamError';
this.paramName = paramName;
}
}
export class ApiParamRequiredError extends ApiParamError {
constructor(message?: string) {
super(message);
constructor(message?: string, originalError?: Error) {
super(message, originalError);
this.name = 'ApiParamRequiredError';
}
}
export class ApiParamOutOfBoundsError extends ApiParamError {
constructor(message?: string) {
super(message);
this.name = 'ApiParamOutOfBoundsError';
}
}
export class ApiParamInvalidFormatError extends ApiParamError {
constructor(message?: string) {
super(message);
constructor(message?: string, originalError?: Error) {
super(message, originalError);
this.name = 'ApiParamInvalidFormatError';
}
}

23
app/src/api/errors/general.ts
View File

@ -1,4 +1,11 @@
export class ArgumentError extends Error {
export class UserError extends Error {
constructor(message?: string) {
super(message);
this.name = 'UserError';
}
}
export class ArgumentError extends UserError {
public argumentName: string;
constructor(message?: string, argumentName?: string) {
super(message);
@ -7,6 +14,13 @@ export class ArgumentError extends Error {
}
}
export class InvalidOperationError extends UserError {
constructor(message?: string) {
super(message);
this.name = 'InvalidOperationError';
}
}
export class DatabaseError extends Error {
public detail: any;
constructor(detail?: string) {
@ -15,10 +29,3 @@ export class DatabaseError extends Error {
this.detail = detail;
}
}
export class DomainLogicError extends Error {
constructor(message?: string) {
super(message);
this.name = 'DomainLogicError';
}
}

160
app/src/api/services/building.ts
View File

@ -8,6 +8,9 @@ import { ITask } from 'pg-promise';
import db from '../../db';
import { tileCache } from '../../tiles/rendererDefinition';
import { BoundingBox } from '../../tiles/types';
import * as buildingDataAccess from '../dataAccess/building';
import * as likeDataAccess from '../dataAccess/like';
import { UserError } from '../errors/general';
import { processBuildingUpdate } from './domainLogic/processBuildingUpdate';
@ -155,83 +158,50 @@ async function getBuildingUPRNsById(id: number) {
}
}
async function saveBuilding(buildingId: number, building: any, userId: string) { // TODO add proper building type
try {
return await updateBuildingData(buildingId, userId, async () => {
const processedBuilding = await processBuildingUpdate(buildingId, building);
// remove read-only fields from consideration
delete processedBuilding.building_id;
delete processedBuilding.revision_id;
delete processedBuilding.geometry_id;
async function saveBuilding(buildingId: number, building: any, userId: string): Promise<object> { // TODO add proper building type
return await updateBuildingData(buildingId, userId, async () => {
const processedBuilding = await processBuildingUpdate(buildingId, building);
// remove read-only fields from consideration
delete processedBuilding.building_id;
delete processedBuilding.revision_id;
delete processedBuilding.geometry_id;
// return whitelisted fields to update
return pickAttributesToUpdate(processedBuilding, BUILDING_FIELD_WHITELIST);
});
} catch(error) {
console.error(error);
return { error: error };
}
// return whitelisted fields to update
return pickAttributesToUpdate(processedBuilding, BUILDING_FIELD_WHITELIST);
});
}
async function likeBuilding(buildingId: number, userId: string) {
try {
return await updateBuildingData(
buildingId,
userId,
async (t) => {
// return total like count after update
return getBuildingLikeCount(buildingId, t);
},
async (t) => {
// insert building-user like
await t.none(
'INSERT INTO building_user_likes ( building_id, user_id ) VALUES ($1, $2);',
[buildingId, userId]
);
},
);
} catch (error) {
console.error(error);
if (error.detail && error.detail.includes('already exists')) {
// 'already exists' is thrown if user already liked it
return { error: 'It looks like you already like that building!' };
} else {
return undefined;
}
}
return await updateBuildingData(
buildingId,
userId,
async (t) => {
// return total like count after update
return {
likes_total: await likeDataAccess.getBuildingLikeCount(buildingId, t)
};
},
(t) => {
return likeDataAccess.addBuildingUserLike(buildingId, userId, t);
},
);
}
async function unlikeBuilding(buildingId: number, userId: string) {
try {
return await updateBuildingData(
buildingId,
userId,
async (t) => {
// return total like count after update
return getBuildingLikeCount(buildingId, t);
},
async (t) => {
// remove building-user like
const result = await t.result(
'DELETE FROM building_user_likes WHERE building_id = $1 AND user_id = $2;',
[buildingId, userId]
);
if (result.rowCount === 0) {
throw new Error('No change');
}
},
);
} catch(error) {
console.error(error);
if (error.message === 'No change') {
// 'No change' is thrown if user doesn't like this building
return { error: 'It looks like you have already revoked your like for that building!' };
} else {
return undefined;
}
}
return await updateBuildingData(
buildingId,
userId,
async (t) => {
// return total like count after update
return {
likes_total: await likeDataAccess.getBuildingLikeCount(buildingId, t)
};
},
async (t) => {
return likeDataAccess.removeBuildingUserLike(buildingId, userId, t);
},
);
}
// === Utility functions ===
@ -246,18 +216,6 @@ function pickAttributesToUpdate(obj: any, fieldWhitelist: Set<string>) {
return subObject;
}
/**
*
* @param buildingId ID of the building to count likes for
* @param t The database context inside which the count should happen
*/
function getBuildingLikeCount(buildingId: number, t: ITask<unknown>) {
return t.one(
'SELECT count(*) as likes_total FROM building_user_likes WHERE building_id = $1;',
[buildingId]
);
}
/**
* Carry out an update of the buildings data. Allows for running any custom database operations before the main update.
* All db hooks get passed a transaction.
@ -271,7 +229,7 @@ async function updateBuildingData(
userId: string,
getUpdateValue: (t: ITask<any>) => Promise<object>,
preUpdateDbAction?: (t: ITask<any>) => Promise<void>,
) {
): Promise<object> {
return await db.tx({mode: serializable}, async t => {
if (preUpdateDbAction != undefined) {
await preUpdateDbAction(t);
@ -279,49 +237,23 @@ async function updateBuildingData(
const update = await getUpdateValue(t);
const oldBuilding = await t.one(
'SELECT * FROM buildings WHERE building_id = $1 FOR UPDATE;',
[buildingId]
);
const oldBuilding = await buildingDataAccess.getBuildingData(buildingId, true, t);
console.log(update);
const patches = compare(oldBuilding, update);
console.log('Patching', buildingId, patches);
const [forward, reverse] = patches;
if (Object.keys(forward).length === 0) {
throw 'No change provided';
throw new UserError('No change provided');
}
const revision = await t.one(
`INSERT INTO logs (
forward_patch, reverse_patch, building_id, user_id
) VALUES (
$1:json, $2:json, $3, $4
) RETURNING log_id
`,
[forward, reverse, buildingId, userId]
);
const revisionId = await buildingDataAccess.insertEditHistoryRevision(buildingId, userId, forward, reverse, t);
const sets = db.$config.pgp.helpers.sets(forward);
console.log('Setting', buildingId, sets);
const data = await t.one(
`UPDATE
buildings
SET
revision_id = $1,
$2:raw
WHERE
building_id = $3
RETURNING
*
`,
[revision.log_id, sets, buildingId]
);
const updatedData = await buildingDataAccess.updateBuildingData(buildingId, forward, revisionId, t);
expireBuildingTileCache(buildingId);
return data;
return updatedData;
});
}