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
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
@@ -1,6 +1,4 @@
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'>
>;
export type InsertTodo = Insert<Partial<Todo>>;
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<Partial<User>>;
Yavorss marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Up @@ -55,7 +55,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
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
2 changes: 1 addition & 1 deletion express-pg-auth0/src/utils/validation/attributes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { z } from 'zod';

export const id = z.number().int().positive().openapi({ example: 1 });
export const id = z.string().openapi({ example: 'tz4a98xxat96iws9zmbrgj3a' });

export const email = z
.string()
Expand Down
10 changes: 7 additions & 3 deletions express-pg-auth0/test/todos/delete.spec-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ describe('DELETE /v1/todos/:id', () => {
const jwtTokens = {
idToken: 'token',
};
const todoId = 1;
let dbClient: Knex;

beforeAll(() => {
Expand Down Expand Up @@ -60,8 +59,13 @@ describe('DELETE /v1/todos/:id', () => {
});

it('should return 204 when todo is deleted', async () => {
const todo = await dbClient('todos').first();
if (!todo) {
throw new Error('Todo not found');
}

await request(app)
.delete(`/v1/todos/${todoId}`)
.delete(`/v1/todos/${todo.id}`)
.set('Authorization', 'Bearer ' + jwtTokens.idToken)
.set('Content-Type', 'application/json')
.expect(204);
Expand All @@ -81,7 +85,7 @@ describe('DELETE /v1/todos/:id', () => {
});

await request(app)
.delete(`/v1/todos/${todoId}`)
.delete(`/v1/todos/tz4a98xxat96iws9zmbrgj3a`)
.set('Content-Type', 'application/json')
.expect(expectError(Unauthorized));
});
Expand Down
25 changes: 6 additions & 19 deletions express-pg-auth0/test/todos/get.spec-e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import {
expectError,
TodoNotFound,
Unauthorized,
UnprocessableEntity,
} from '../utils';
import { Todo } from '@api/todos';
import { Knex } from 'knex';
import { create } from '@database';
import createError from 'http-errors';
Expand All @@ -24,13 +22,6 @@ describe('GET /v1/todos/:id', () => {
let app: Express.Application;
let destroy: () => Promise<void>;
let dbClient: Knex;
const todo: Partial<Todo> = {
id: 1,
name: 'Laundry 1',
note: 'Buy detergent 1',
completed: false,
userId: 'tz4a98xxat96iws9zmbrgj3a',
};
const jwtTokens = {
idToken: 'token',
};
Expand Down Expand Up @@ -70,6 +61,11 @@ describe('GET /v1/todos/:id', () => {
describe('when user is authenticated', () => {
describe('given todo id in the query', () => {
it('should return the todo', async () => {
const todo = await dbClient('todos').first();
if (!todo) {
throw new Error('Todo not found');
}

const res = await request(app)
.get(`/v1/todos/${todo.id}`)
.set('Authorization', 'Bearer ' + jwtTokens.idToken)
Expand All @@ -90,15 +86,6 @@ describe('GET /v1/todos/:id', () => {
.expect(expectError(TodoNotFound));
});
});

describe('given a text id in the query', () => {
it('should return 422 error', async () => {
await request(app)
.get(`/v1/todos/test`)
.set('Authorization', 'Bearer ' + jwtTokens.idToken)
.expect(expectError(UnprocessableEntity));
});
});
});

describe('when user is not authenticated', () => {
Expand All @@ -108,7 +95,7 @@ describe('GET /v1/todos/:id', () => {
});

await request(app)
.get(`/v1/todos/${todo.id}`)
.get('/v1/todos/tz4a98xxat96iws9zmbrgj3a')
.expect(expectError(Unauthorized));
});
});
Expand Down
Loading
Loading