Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/migrating to not auto incrementing ids #334

Merged
merged 10 commits into from
Feb 25, 2024
16 changes: 9 additions & 7 deletions .github/workflows/test-example-applications.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
image: postgres:latest
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_PASSWORD: password
POSTGRES_DB: test
ports: ["5432:5432"]

Expand All @@ -24,7 +24,7 @@ jobs:
ports: ["27017:27017"]
env:
MONGO_INITDB_ROOT_USERNAME: root
MONGO_INITDB_ROOT_PASSWORD: pass@ord
MONGO_INITDB_ROOT_PASSWORD: password

steps:
- name: Checkout code
Expand All @@ -33,10 +33,12 @@ jobs:
- name: Use Node.js
uses: actions/setup-node@v2
with:
node-version: 20
node-version: 18

- name: Install dependencies
run: npm ci
run: |
git init
npm install
working-directory: ${{ matrix.project }}

- name: Create .env.test file
Expand All @@ -50,17 +52,17 @@ jobs:
sed -i 's/REQUEST_LOGGING=.*/REQUEST_LOGGING=false/g' .env.test
sed -i 's/SWAGGER=.*/SWAGGER=false/g' .env.test

sed -i 's/PGHOST=.*/PGHOST=localhost/g' .env.test
sed -i 's/PGHOST=.*/PGHOST=0.0.0.0/g' .env.test
sed -i 's/PGPORT=.*/PGPORT=5432/g' .env.test
sed -i 's/PGUSER=.*/PGUSER=postgres/g' .env.test
sed -i 's/PGPASSWORD=.*/PGPASSWORD=postgres/g' .env.test
sed -i 's/PGPASSWORD=.*/PGPASSWORD=password/g' .env.test
sed -i 's/PGDATABASE=.*/PGDATABASE=test/g' .env.test

sed -i 's/MONGO_PROTOCOL=.*/MONGO_PROTOCOL=mongodb/g' .env.test
sed -i 's/MONGO_HOST=.*/MONGO_HOST=localhost/g' .env.test
sed -i 's/MONGO_PORT=.*/MONGO_PORT=27017/g' .env.test
sed -i 's/MONGO_USER=.*/MONGO_USER=root/g' .env.test
sed -i 's/MONGO_PASSWORD=.*/MONGO_PASSWORD=pass@ord/g' .env.test
sed -i 's/MONGO_PASSWORD=.*/MONGO_PASSWORD=password/g' .env.test
sed -i 's/MONGO_DATABASE_NAME=.*/MONGO_DATABASE_NAME=example-app/g' .env.test
working-directory: ${{ matrix.project }}

Expand Down
2 changes: 1 addition & 1 deletion express-pg-auth0/.secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,5 @@
}
],
"results": {},
"generated_at": "2024-01-20T21:29:38Z"
"generated_at": "2024-02-25T15:31:32Z"
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { createUpdatedAtTriggerSQL, dropUpdatedAtTriggerSQL } from './utils';

export async function up(knex: Knex): Promise<void> {
await knex.schema.createTable('users', (table) => {
table.bigIncrements('id');
table
.string('id')
.unique({ indexName: 'unq_users_id' })
.index()
.notNullable();
table
.string('userId')
.unique({ indexName: 'unq_users_user_id' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { createUpdatedAtTriggerSQL, dropUpdatedAtTriggerSQL } from './utils';

export async function up(knex: Knex): Promise<void> {
await knex.schema.createTable('todos', (table) => {
table.bigIncrements('id');
table
.string('id')
.unique({ indexName: 'unq_todos_id' })
.index()
.notNullable();
table.string('name').notNullable();
table.text('note');
table.boolean('completed').notNullable().defaultTo(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import { Insert } from '@database/operations';
import { Todo } from './todo.entity';

export type InsertTodo = Insert<
Yavorss marked this conversation as resolved.
Show resolved Hide resolved
Pick<Todo, 'userId' | 'name' | 'note' | 'completed'>
Pick<Todo, 'userId' | 'name' | 'note' | 'completed'> & { id?: string }
>;
2 changes: 1 addition & 1 deletion express-pg-auth0/src/api/todos/entities/todo.entity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export interface Todo {
id: number;
id: string;
userId: string;
name: string;
note: string | null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { Paginated, extractPagination } from '@utils/query';
import { InsertTodo, ListTodosQuery, Todo, UpdateTodo } from '../../entities';
import { createId } from '@paralleldrive/cuid2';

export class TodosRepository {
private lastId = 0;
private records: Todo[] = [];
private records: Partial<Todo>[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this change comes from the previous one. In the SQL table we shouldn't have partial records


private nextId() {
return ++this.lastId;
}

async insertOne(input: InsertTodo): Promise<Todo> {
async insertOne(input: InsertTodo): Promise<Partial<Todo>> {
const record = {
id: this.nextId(),
id: createId(),
...input,
note: input.note ?? null,
createdAt: new Date().toISOString(),
Expand All @@ -26,15 +22,15 @@ export class TodosRepository {
async findById(
id: Todo['id'],
userId: Todo['userId']
): Promise<Todo | undefined> {
): Promise<Partial<Todo> | undefined> {
Yavorss marked this conversation as resolved.
Show resolved Hide resolved
return this.records.find((r) => r.id === id && r.userId === userId);
}

async updateById(
id: Todo['id'],
userId: Todo['userId'],
input: UpdateTodo
): Promise<Todo | undefined> {
): Promise<Partial<Todo> | undefined> {
const record = await this.findById(id, userId);

if (!record) {
Expand All @@ -61,7 +57,7 @@ export class TodosRepository {
async list(
userId: Todo['userId'],
query: ListTodosQuery
): Promise<Paginated<Todo>> {
): Promise<Paginated<Partial<Todo>>> {
const { page, items } = extractPagination(query.pagination);

const records = this.records.filter((r) => r.userId === userId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@ describe('TodosRepository', () => {
jest.spyOn(todosQb, 'returning');
jest.spyOn(todosQb, 'then');
jest.spyOn(todosQb, 'catch').mockImplementationOnce(async () => ({
id: 1,
...todo,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}));

const result = await todos.insertOne(todo);

expect(todosQb.insert).toHaveBeenCalledWith(todo);
expect(todosQb.insert).toHaveBeenCalledWith({
id: expect.any(String),
...todo,
});
expect(todosQb.returning).toHaveBeenCalledWith('*');
expect(todosQb.then).toHaveBeenCalled();
expect(todosQb.catch).toHaveBeenCalled();
Expand All @@ -45,7 +47,7 @@ describe('TodosRepository', () => {
describe('findById', () => {
it('should retrun the first record found', async () => {
const todo: Todo = {
id: 1,
id: '1',
userId: '1',
name: 'Laundry',
note: 'Now!',
Expand Down Expand Up @@ -74,7 +76,7 @@ describe('TodosRepository', () => {
describe('updateById', () => {
it('should perform a findById with empty payload', async () => {
const todo: Todo = {
id: 1,
id: '1',
userId: '1',
name: 'Laundry',
note: 'Now!',
Expand All @@ -94,7 +96,7 @@ describe('TodosRepository', () => {

it('should update the record and return it', async () => {
const todo: Todo = {
id: 1,
id: '1',
userId: '1',
name: 'Laundry',
note: 'Now!',
Expand Down Expand Up @@ -156,9 +158,9 @@ describe('TodosRepository', () => {
.spyOn(todosQb, 'del')
.mockImplementationOnce(() => Promise.resolve(1) as never);

const result = await todos.deleteById(1, '1');
const result = await todos.deleteById('1', '1');

expect(todosQb.where).toHaveBeenCalledWith({ id: 1, userId: '1' });
expect(todosQb.where).toHaveBeenCalledWith({ id: '1', userId: '1' });
expect(todosQb.del).toHaveBeenCalled();
expect(result).toBe(1);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ import { InsertTodo, ListTodosQuery, Todo, UpdateTodo } from '../entities';
import { TodoUserNotFound } from '../error-mappings';
import { filterByCompleted, filterByName } from '../filters';
import { sortByCreatedAt, sortByName } from '../sorters';
import { createId } from '@paralleldrive/cuid2';

export class TodosRepository {
constructor(private readonly knex: Knex) {}

async insertOne(input: InsertTodo): Promise<Todo> {
return await this.knex('todos')
.insert(input)
.insert({
id: createId(),
...input,
})
.returning('*')
.then(([todo]) => todo)
.catch(rethrowError(TodoUserNotFound));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import z from 'zod';
import { todoSchema } from './todo.schema';

export const todoIdParamSchema = z.object({
id: todoSchema.shape.id.coerce(),
id: z.string(),
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('TodoService', () => {
describe('when the record does not exist', () => {
it('should throw an error', async () => {
await expect(
todosService.find(Date.now(), userId)
todosService.find(Date.now().toString(), userId)
).rejects.toThrow(RecordNotFoundError);
});
});
Expand All @@ -51,7 +51,7 @@ describe('TodoService', () => {
describe('when the record does not exist', () => {
it('should throw an error', async () => {
await expect(
todosService.update(Date.now(), userId, { completed: true })
todosService.update(Date.now().toString(), userId, { completed: true })
).rejects.toThrow(RecordNotFoundError);
});
});
Expand Down Expand Up @@ -79,7 +79,7 @@ describe('TodoService', () => {
describe('when the record does not exist', () => {
it('should throw an error', async () => {
await expect(
todosService.delete(Date.now(), userId)
todosService.delete(Date.now().toString(), userId)
).rejects.toThrow(RecordNotFoundError);
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Insert } from '@database/operations';
import { User } from './user.entity';

export type InsertUser = Insert<Pick<User, 'email' | 'password' | 'userId'>>;
export type InsertUser = Insert<Pick<User, 'email' | 'password' | 'userId'> & { id?: string }>;
2 changes: 1 addition & 1 deletion express-pg-auth0/src/api/users/entities/user.entity.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export interface User {
id: number;
id: string;
userId: string;
email: string;
password: string | null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
import { DuplicateRecordError } from '@database/errors';
import { InsertUser, User } from '../../entities';
import { createId } from '@paralleldrive/cuid2';

export class UsersRepository {
private lastId = 0;
private records: User[] = [];
private records: Partial<User>[] = [];

private nextId() {
return ++this.lastId;
}
async insertOne(input: InsertUser): Promise<Partial<User>> {
if (!input.email) {
throw new Error('Email is required');
}

async insertOne(input: InsertUser): Promise<User> {
const existingRecord = await this.findByEmail(input.email);

if (existingRecord) {
throw new DuplicateRecordError('User Email already taken');
}

const record = {
id: this.nextId(),
id: createId(),
...input,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
Expand All @@ -28,7 +28,7 @@ export class UsersRepository {
return record;
}

async findByEmail(email: string): Promise<User | undefined> {
async findByEmail(email: string): Promise<Partial<User> | undefined> {
return this.records.find((r) => r.email === email);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,17 @@ describe('UsersRepository', () => {
jest.spyOn(usersQb, 'returning');
jest.spyOn(usersQb, 'then');
jest.spyOn(usersQb, 'catch').mockImplementationOnce(async () => ({
id: 1,
...user,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}));

const result = await users.insertOne(user);

expect(usersQb.insert).toHaveBeenCalledWith(user);
expect(usersQb.insert).toHaveBeenCalledWith({
id: expect.any(String),
...user,
});
expect(usersQb.returning).toHaveBeenCalledWith('*');
expect(usersQb.then).toHaveBeenCalled();
expect(usersQb.catch).toHaveBeenCalled();
Expand Down Expand Up @@ -55,7 +57,7 @@ describe('UsersRepository', () => {
describe('findByEmail', () => {
it('should return the first record found', async () => {
const user: User = {
id: 1,
id: '1',
email: '[email protected]',
userId: '1',
password: '123',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ import { Knex } from 'knex';
import { rethrowError } from '@utils/error';
import { InsertUser, User } from '../entities';
import { UserEmailTaken } from '../error-mappings';
import { createId } from '@paralleldrive/cuid2';

export class UsersRepository {
constructor(private readonly knex: Knex) {}

async insertOne(input: InsertUser): Promise<User> {
return await this.knex('users')
.insert(input)
.insert({
id: createId(),
...input,
})
.returning('*')
.then(([user]) => user)
.catch(rethrowError(UserEmailTaken));
Expand Down
5 changes: 5 additions & 0 deletions express-pg-auth0/src/database/seeds/test/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Knex } from "knex";
import { createId } from '@paralleldrive/cuid2';

/**
* @param { import("knex").Knex } knex
Expand All @@ -9,6 +10,7 @@ exports.seed = async function (knex: Knex) {
await knex('users').insert([
// The original password for this hash is 'pass@ord'
{
id: createId(),
email: '[email protected]',
password: '$2b$10$Mxur7NOiTlm22yuldEMZgOCbIV7bxDCcUbBLFbzrJ1MrnIczZB.92', // pragma: allowlist secret
userId: 'tz4a98xxat96iws9zmbrgj3a',
Expand All @@ -17,18 +19,21 @@ exports.seed = async function (knex: Knex) {

await knex('todos').insert([
{
id: createId(),
name: 'Laundry 1',
note: 'Buy detergent 1',
completed: false,
userId: 'tz4a98xxat96iws9zmbrgj3a',
},
{
id: createId(),
name: 'Laundry 2',
note: 'Buy detergent 2',
completed: false,
userId: 'tz4a98xxat96iws9zmbrgj3a',
},
{
id: createId(),
name: 'Laundry 3',
note: 'Buy detergent 3',
completed: true,
Expand Down
Loading
Loading