Skip to content

Commit

Permalink
Merge pull request #123 from o2Labs/ttl-fixes
Browse files Browse the repository at this point in the history
Fix TTL including milliseconds
  • Loading branch information
danielrbradley authored Dec 1, 2020
2 parents 147136f + 93804b1 commit 528ce63
Show file tree
Hide file tree
Showing 12 changed files with 184 additions and 30 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [2.1.0]

- Fix TTL bug resulting in the data retention policy not being followed.
- Add migration to fix TTL on existing database entries.
- Add a basic stats page showing all booking counts per office, per day.
- Improved usability and accessibility of login journey.
- Increase test coverage of critical path to booking.

## [2.0.2]

- Add check for user before attempting to retrieve office details.
Expand Down
3 changes: 2 additions & 1 deletion client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "office-booker",
"version": "0.1.0",
"version": "2.1.0",
"private": true,
"scripts": {
"start": "react-scripts start",
Expand All @@ -22,6 +22,7 @@
"@reach/router": "^1.3.3",
"@types/loadable__component": "^5.13.1",
"amazon-cognito-identity-js": "^4.2.2",
"array-fns": "^1.0.0",
"core-js": "^3.6.5",
"date-fns": "^2.12.0",
"lodash": "^4.17.19",
Expand Down
46 changes: 35 additions & 11 deletions client/src/components/App/Home.test.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
import React from 'react';
import { render, fireEvent, screen } from '@testing-library/react';
import { render, fireEvent, screen, within } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { server } from '../../../test/server.mock';
import { createFakeOfficeWithSlots, createFakeSystemAdminUser } from '../../../test/data';
import {
createFakeConfig,
createFakeOfficeWithSlots,
createFakeSystemAdminUser,
} from '../../../test/data';
import { TestContext } from '../../../test/TestContext';
import { mockGetBookings, mockGetOffice, mockGetOffices } from '../../../test/handlers';
import {
mockGetBookings,
mockGetConfig,
mockGetOffice,
mockGetOffices,
} from '../../../test/handlers';

import Home from './Home';

test('Selecting an office', async () => {
const office = createFakeOfficeWithSlots();
const user = createFakeSystemAdminUser([office]);
server.use(mockGetOffices([office]), mockGetBookings([]), mockGetOffice(office));
const config = createFakeConfig();
const office = createFakeOfficeWithSlots(config);
const user = createFakeSystemAdminUser([office], { quota: 2 });
server.use(
mockGetConfig(config),
mockGetOffices([office]),
mockGetBookings([]),
mockGetOffice(office)
);
render(
<TestContext user={user}>
<Home />
Expand All @@ -21,11 +36,20 @@ test('Selecting an office', async () => {

await screen.findByRole('heading', { level: 2, name: 'Select your office' });

fireEvent.click(screen.getByRole('button', { name: office.name }));
const main = within(screen.getByRole('main'));

fireEvent.click(main.getByRole('button', { name: office.name }));

await screen.findByRole('heading', { level: 2, name: office.name });
expect(screen.getByText(/You can make/i)).toHaveTextContent(`You can make 5 booking per week`);
expect(screen.getByText(/daily capacity/i)).toHaveTextContent(
`The Office has a daily capacity of 100 and car park capacity of 100.`
await main.findByRole('heading', { level: 2, name: office.name });
expect(main.getByText(/You can make/i)).toHaveTextContent(
`You can make ${user.quota} booking per week`
);
expect(main.getByText(/daily capacity/i)).toHaveTextContent(
`The Office has a daily capacity of ${office.quota} and car park capacity of ${office.parkingQuota}.`
);
expect(main.getByText(/bookings remaining/i)).toHaveTextContent(
`${user.quota} bookings remaining`
);

await main.findAllByRole('button', { name: 'Book' });
});
20 changes: 14 additions & 6 deletions client/test/data.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Config } from '../src/context/stores';
import { Booking, Office, OfficeWithSlots, User } from '../src/types/api';
import { format } from 'date-fns';
import { addDays, format } from 'date-fns';
import { init } from 'array-fns';

export const createFakeConfig = (prototype?: Partial<Config>): Config => ({
advancedBookingDays: 14,
Expand All @@ -18,12 +19,19 @@ export const createFakeOffice = (prototype?: Partial<Office>): Office => ({
});

export const createFakeOfficeWithSlots = (
config: Config,
prototype?: Partial<OfficeWithSlots>
): OfficeWithSlots => ({
...createFakeOffice(prototype),
slots: [],
...prototype,
});
): OfficeWithSlots => {
return {
...createFakeOffice(prototype),
slots: init(config.advancedBookingDays, (i) => ({
date: format(addDays(new Date(), i), 'yyyy-MM-dd'),
booked: 0,
bookedParking: 0,
})),
...prototype,
};
};

export const createFakeUser = (prototype?: Partial<User>): User => ({
email: 'mock.user@domain.test',
Expand Down
5 changes: 5 additions & 0 deletions client/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3002,6 +3002,11 @@ array-flatten@^2.1.0:
resolved "https://registry.yarnpkg.com/array-flatten/-/array-flatten-2.1.2.tgz#24ef80a28c1a893617e2149b0c6d0d788293b099"
integrity sha512-hNfzcOV8W4NdualtqBFPyVO+54DSJuZGY9qT4pRroB6S9e3iiido2ISIC5h9R2sPJ8H3FHCIiEnsv1lPXO3KtQ==

array-fns@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/array-fns/-/array-fns-1.0.0.tgz#bce7b5cc9b95c28187328a320555fd175cb545de"
integrity sha512-mPqxfuDBey5FYj+f3wy4Jlf1u74eNBOhBuJ26od/UovTLTwOhusNoNgByQN3JI7yEr//CD1XuDtWVQYYfpJRLw==

array-includes@^3.1.1:
version "3.1.1"
resolved "https://registry.yarnpkg.com/array-includes/-/array-includes-3.1.1.tgz#cdd67e6852bdf9c1215460786732255ed2459348"
Expand Down
2 changes: 1 addition & 1 deletion server/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const configureApp = (config: Config) => {
app.get('/api/config', (req, res, next) => {
try {
const clientConfig = {
version: '2.0.0',
version: '2.1.0',
showTestBanner: config.showTestBanner,
auth:
config.authConfig.type === 'cognito'
Expand Down
4 changes: 2 additions & 2 deletions server/db/bookings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Config } from '../app-config';
import DynamoDB from 'aws-sdk/clients/dynamodb';
import { attribute, table, hashKey, rangeKey } from '@aws/dynamodb-data-mapper-annotations';
import { DataMapper } from '@aws/dynamodb-data-mapper';
import { format, subDays, addDays } from 'date-fns';
import { format, subDays, addDays, getUnixTime } from 'date-fns';
import { AttributePath, FunctionExpression } from '@aws/dynamodb-expressions';

export interface CreateBookingModel {
Expand Down Expand Up @@ -100,7 +100,7 @@ export const createBooking = async (
try {
const created = await mapper.put(
Object.assign(new BookingsModel(), booking, {
ttl: addDays(new Date(booking.date), config.dataRetentionDays).getTime(),
ttl: getUnixTime(addDays(new Date(booking.date), config.dataRetentionDays)),
}),
{
condition: new FunctionExpression('attribute_not_exists', new AttributePath('id')),
Expand Down
4 changes: 2 additions & 2 deletions server/db/officeBookings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
UpdateExpression,
FunctionExpression,
} from '@aws/dynamodb-expressions';
import { addDays } from 'date-fns';
import { addDays, getUnixTime } from 'date-fns';

export interface OfficeBooking {
officeId: string;
Expand Down Expand Up @@ -57,7 +57,7 @@ export const incrementOfficeBookingCount = async (
date,
bookingCount: 0,
parkingCount: 0,
ttl: addDays(new Date(date), config.dataRetentionDays).getTime(),
ttl: getUnixTime(addDays(new Date(date), config.dataRetentionDays)),
}),
{
condition: {
Expand Down
4 changes: 2 additions & 2 deletions server/db/userBookings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
FunctionExpression,
} from '@aws/dynamodb-expressions';
import { Arrays } from 'collection-fns';
import { addDays } from 'date-fns';
import { addDays, getUnixTime } from 'date-fns';

export interface UserBookings {
email: string;
Expand Down Expand Up @@ -69,7 +69,7 @@ export const incrementUserBookingCount = async (
email: userEmail,
weekCommencing,
bookingCount: 0,
ttl: addDays(new Date(weekCommencing), config.dataRetentionDays + 7).getTime(),
ttl: getUnixTime(addDays(new Date(weekCommencing), config.dataRetentionDays + 7)),
}),
{
condition: {
Expand Down
41 changes: 41 additions & 0 deletions server/migrations/4-fix-ttl.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { DataMapper } from '@aws/dynamodb-data-mapper';
import { DynamoDB } from 'aws-sdk';
import { getUnixTime } from 'date-fns';
import { Config } from '../app-config';
import { BookingsModel } from '../db/bookings';
import { OfficeBookingModel } from '../db/officeBookings';
import { UserBookingsModel } from '../db/userBookings';

// Check if TTL is beyond the year 3000
const timestampTooHigh = (ttl: number) => ttl > 32503680000;

async function* getCorrectedTtls(mapper: DataMapper, model: { new (): any }) {
for await (const booking of mapper.scan(model)) {
if (timestampTooHigh(booking.ttl)) {
const ttl = new Date(booking.ttl);
booking.ttl = getUnixTime(ttl);
yield booking;
}
}
}

const scanAndFix = async (mapper: DataMapper, model: { new (): any }) => {
let updated = 0;
for await (const _item of mapper.batchPut(getCorrectedTtls(mapper, model))) {
updated++;
if (updated % 100 === 0) {
console.log('Updated ', updated);
}
}
console.log('Updated ', updated);
};

export const fixTtl = async (config: Config) => {
const mapper = new DataMapper({
client: new DynamoDB(config.dynamoDB),
tableNamePrefix: config.dynamoDBTablePrefix,
});
await scanAndFix(mapper, BookingsModel);
await scanAndFix(mapper, OfficeBookingModel);
await scanAndFix(mapper, UserBookingsModel);
};
4 changes: 4 additions & 0 deletions server/migrations/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { options } from 'yargs';
import { SSM } from 'aws-sdk';
import { parseConfigFromEnv, Config } from '../app-config';
import { fixTtl } from './4-fix-ttl';

/** Collection all migrations that should be applied to the system */
type Migrations = {
Expand Down Expand Up @@ -30,6 +31,9 @@ const migrations: Migrations = {
'3-move-bookings': {
reasonToFailPreCheck: 'You must deploy version 1.2.0 first.',
},
'4-fix-ttl': {
execute: fixTtl,
},
};

/**
Expand Down
73 changes: 68 additions & 5 deletions server/tests/migrations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,70 @@
// const { app, resetDb, config } = configureServer('migrations', {
// users: [{ Username: cognitoUsername, UserCreateDate: cognitoUserCreated }],
// });
import { configureServer } from './test-utils';
import { DataMapper } from '@aws/dynamodb-data-mapper';
import { DynamoDB } from 'aws-sdk';
import { UserBookingsModel } from '../db/userBookings';
import { fixTtl } from '../migrations/4-fix-ttl';
import { BookingsModel } from '../db/bookings';
import { OfficeBookingModel } from '../db/officeBookings';

// beforeEach(resetDb);
const { resetDb, config } = configureServer('migrations');

test('no-active-migration', () => {});
beforeEach(resetDb);

describe('4 - fix TTL', async () => {
it(`fixes bookings ttl`, async () => {
const expires = new Date('2020-03-01');
const mapper = new DataMapper({
client: new DynamoDB(config.dynamoDB),
tableNamePrefix: config.dynamoDBTablePrefix,
});

const userBookingKey = {
email: 'user.name@domain.test',
weekCommencing: '2020-01-01',
};
const officeBookingKey = {
name: 'the-office',
date: '2020-01-01',
};
const bookingKey = {
id: 'booking-id',
user: 'user.name@domain.test',
};
await mapper.put(
Object.assign(new UserBookingsModel(), userBookingKey, {
bookingCount: 1,
ttl: expires.getTime(),
})
);
await mapper.put(
Object.assign(new OfficeBookingModel(), officeBookingKey, {
bookingCount: 1,
parkingCount: 0,
ttl: expires.getTime(),
})
);
await mapper.put(
Object.assign(new BookingsModel(), bookingKey, {
date: '2020-01-01',
officeId: 'the-office',
parking: false,
ttl: expires.getTime(),
})
);

await fixTtl(config);

const updatedUserBooking = await mapper.get(
Object.assign(new UserBookingsModel(), userBookingKey)
);
expect(updatedUserBooking.ttl).toBe(Math.floor(expires.getTime() / 1000));

const updatedOfficeBooking = await mapper.get(
Object.assign(new OfficeBookingModel(), officeBookingKey)
);
expect(updatedOfficeBooking.ttl).toBe(Math.floor(expires.getTime() / 1000));

const updatedBooking = await mapper.get(Object.assign(new BookingsModel(), bookingKey));
expect(updatedBooking.ttl).toBe(Math.floor(expires.getTime() / 1000));
});
});

0 comments on commit 528ce63

Please # to comment.