Drizzle Team•14mo ago

Help with types in Repository Pattern

Hello, I'm trying to set up a crud repository where it can be extended but I'm not sure how to get the typings correct. Here is what I have and I'm stuck. The issue that I have is I'm not sure how to actually get the primaryKey in a type safe way.

export interface BaseRepository<T, ID> {
  findAll(): Promise<T[]>;

  findAll(criteria: QueryCriteria): Promise<T[]>;

  findOne(id: ID): Promise<T | null>;

  update(id: ID, data: T): Promise<T>;

  create(data: T): Promise<T>;

  delete(id: ID): Promise<void>;
}


export abstract class CrudRepository<
  M extends MySqlTable,
  ID extends keyof S,
  S extends InferModel<M>,
  I extends InferModel<M, "insert">> implements BaseRepository<S, S[ID]> {


  protected constructor(protected readonly db: DbConnection,
                        protected readonly schema: M,
                        protected readonly primaryKey: ID) {
  }


  async create(data: S): Promise<S> {
    await this.db.insert(this.schema).values([{ ...data }]);

    const result = await this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], "LAST_INSERT_ID()"));
    return result[0];
  }

  async delete(id: S[ID]): Promise<void> {
    const result = await this.db.delete(this.schema).where(eq(this.schema[this.primaryKey], id));
  }

  async findAll(): Promise<S[]> {
    return this.db.select().from(this.schema);
  }

  findOne(id: S[ID]): Promise<S | null> {
    return this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], id));
  }

  async update(id: S[ID], data: I): Promise<S | null> {
    const entity = this.findOne(id);
    if (!entity) {
      throw new Error("Entity not found");
    }
    this.db.update(this.schema).set(data).where(eq(this.schema[this.primaryKey], id));
    return this.findOne(id);
  }

}

export interface BaseRepository<T, ID> {
  findAll(): Promise<T[]>;

  findAll(criteria: QueryCriteria): Promise<T[]>;

  findOne(id: ID): Promise<T | null>;

  update(id: ID, data: T): Promise<T>;

  create(data: T): Promise<T>;

  delete(id: ID): Promise<void>;
}


export abstract class CrudRepository<
  M extends MySqlTable,
  ID extends keyof S,
  S extends InferModel<M>,
  I extends InferModel<M, "insert">> implements BaseRepository<S, S[ID]> {


  protected constructor(protected readonly db: DbConnection,
                        protected readonly schema: M,
                        protected readonly primaryKey: ID) {
  }


  async create(data: S): Promise<S> {
    await this.db.insert(this.schema).values([{ ...data }]);

    const result = await this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], "LAST_INSERT_ID()"));
    return result[0];
  }

  async delete(id: S[ID]): Promise<void> {
    const result = await this.db.delete(this.schema).where(eq(this.schema[this.primaryKey], id));
  }

  async findAll(): Promise<S[]> {
    return this.db.select().from(this.schema);
  }

  findOne(id: S[ID]): Promise<S | null> {
    return this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], id));
  }

  async update(id: S[ID], data: I): Promise<S | null> {
    const entity = this.findOne(id);
    if (!entity) {
      throw new Error("Entity not found");
    }
    this.db.update(this.schema).set(data).where(eq(this.schema[this.primaryKey], id));
    return this.findOne(id);
  }

}

34 Replies

Ayaz•14mo ago

message.txt

Angelelz•14mo ago

Right now your Generic S and your Generic ID parameters don't have any relation The first thing you should do is in the first line:

export interface BaseRepository<T, ID extends keyof T> ...
...

export interface BaseRepository<T, ID extends keyof T> ...
...

Do your tables have a common name of the primary key, like id? That would make it a lot easier, with less generic parameters

Ayaz•14mo ago

Usually yes You can check in the file how I’m implementing it I was thinking of passing the primary key in the generic

Angelelz•14mo ago

If you do:

export abstract class CrudRepository<
  M extends MySqlTable & { id: string },
  ...

export abstract class CrudRepository<
  M extends MySqlTable & { id: string },
  ...

If your id is a string I might be wrong, but that would make not necessary to have a generic for the id Because you know in all your schemas have an id: string BTW, What errors are you seeing that made ask for help?

Ayaz•14mo ago

Ayaz•14mo ago

I ended up going this route

Ayaz•14mo ago

it is working but I'm on finding a way to add select criteria or I might just keep it as is and just make per repository queries @angelelz what do you think? 😄

Angelelz•14mo ago

Interesting, I didn't have time this weekend to test it.

Ayaz•14mo ago

this is how you extend it

Ayaz•14mo ago

not the best looking but works 😄

Angelelz•14mo ago

Looks great, mr. typescript wizard!

Jukka Tawast•14mo ago

Did you manage to do this in a way that would allow to pass columns as a parameter for select() and the types for the returned rows for find/findAll would be correct? I got it to a point where all the real db columns were right but it was missing the "virtual" ones. So

db.select({
  id: table.id, 
  madeUpFoo: sql<string>`lower(${table.bar})`
})...

db.select({
  id: table.id, 
  madeUpFoo: sql<string>`lower(${table.bar})`
})...

was missing the madeUpFoo from the return type.

Angelelz•14mo ago

Just my humble opinion, my 2 cents so to speak. I don't really like this pattern, I prefer to have my data model behind an interface. The types are really easy to implement, as the interface wouldn't care about implementation details, just return types. Then create classes that implement that interface, and instantiate it with the db object. Abstract classes are good for library code, where you need several implementation classes to have an underlying shared API and methods. In application code, when you create a record, you might need to also create related records. When you need to get records, you'll also likely need to join them with related records, etc. That's where this type of Crud abstract classes fall apart, imo. Interfaces also make your code easy to test and replacing implementation classes becomes a breeze

Jukka Tawast•14mo ago

Yeah, I do see what you mean. Personally I wasn't even planning to use the repositories for the more difficult queries/stuff for the reasons you just wrote, which in turn would make them kinda unnecessary I guess

Angelelz•14mo ago

It's an interesting exercise, not gonna lie! And I wouldn't like my opinion to discourage anyone from attempting to implement it and probably discover some good benefits!

Jukka Tawast•14mo ago

Well exercise it was indeed. And I lost 😄 But indeed you kinda have to learn quite a lot of a library when you're trying to do generic stuff...

DiamondDragon•14mo ago

@Angelelz @Ayaz this was my attempt at implementing this pattern... any thoughts/suggestions? @Raphaël M (@rphlmr) ⚡ i also saw you commented on this somewhere else. edit: my transaction was setup wrong but this seems to work

    private getQueryExecutor(tx?: Db): Db {
        return tx || db
    }

    public constructor(
        protected table: TTable,
        protected pk: TKeys,
        protected tx?: TransactionsDb,
    ) {}
....
    async delete(
        condition: SQL,
        tx?: TransactionsDb,
    ): Promise<InferSelectModel<TTable>[]> {
        const queryExecutor = this.getQueryExecutor(tx)

        return await queryExecutor
            .delete(this.table)
            .where(condition)
            .returning()
    }

    async transaction<T>(
        transactionCallback: (tx: TransactionsDb) => Promise<T>,
        config?: PgTransactionConfig,
    ): Promise<T> {
        return db.transaction(async tx => {
            return transactionCallback(tx)
        }, config)
    }

    private getQueryExecutor(tx?: Db): Db {
        return tx || db
    }

    public constructor(
        protected table: TTable,
        protected pk: TKeys,
        protected tx?: TransactionsDb,
    ) {}
....
    async delete(
        condition: SQL,
        tx?: TransactionsDb,
    ): Promise<InferSelectModel<TTable>[]> {
        const queryExecutor = this.getQueryExecutor(tx)

        return await queryExecutor
            .delete(this.table)
            .where(condition)
            .returning()
    }

    async transaction<T>(
        transactionCallback: (tx: TransactionsDb) => Promise<T>,
        config?: PgTransactionConfig,
    ): Promise<T> {
        return db.transaction(async tx => {
            return transactionCallback(tx)
        }, config)
    }

export.ts

DiamondDragon•14mo ago

@Andrew Sherman i saw you also recommended this service-repo pattern somewhere else. any suggestions? I was trying to add joins but was getting errors i think on the return type... not sure how to handle joins. Maybe it's too complicated and i should just handle this in other repos that implement BaseRepo?

interface ISelectOptions<TSelection> {
    where?: SQL
    joins?: Join[]
}

// TODO: Add joins
    async read(
        options?: ISelectOptions<InferInsertModel<TTable>>,
        tx?: TransactionsDb,
    ): Promise<InferSelectModel<TTable>[]> {
        const queryExecutor = this.getQueryExecutor(tx)

        let query = queryExecutor.select().from(this.table)

        if (options?.where) {
            query = query.where(options.where)
        }

    ...
        if (options?.joins) {
            for (const join of options.joins) {
                switch (join.joinType) {
                    case 'inner':
                        query = query.innerJoin(join.table, join.on)
                        break
                    case 'left':
                        query = query.leftJoin(join.table, join.on)
                        break
                    case 'right':
                        query = query.rightJoin(join.table, join.on)
                        break
                    case 'full':
                        query = query.fullJoin(join.table, join.on)
                        break
                }
            }
        }

        return (await query) as InferSelectModel<TTable>[]
    }

interface ISelectOptions<TSelection> {
    where?: SQL
    joins?: Join[]
}

// TODO: Add joins
    async read(
        options?: ISelectOptions<InferInsertModel<TTable>>,
        tx?: TransactionsDb,
    ): Promise<InferSelectModel<TTable>[]> {
        const queryExecutor = this.getQueryExecutor(tx)

        let query = queryExecutor.select().from(this.table)

        if (options?.where) {
            query = query.where(options.where)
        }

    ...
        if (options?.joins) {
            for (const join of options.joins) {
                switch (join.joinType) {
                    case 'inner':
                        query = query.innerJoin(join.table, join.on)
                        break
                    case 'left':
                        query = query.leftJoin(join.table, join.on)
                        break
                    case 'right':
                        query = query.rightJoin(join.table, join.on)
                        break
                    case 'full':
                        query = query.fullJoin(join.table, join.on)
                        break
                }
            }
        }

        return (await query) as InferSelectModel<TTable>[]
    }

Andrii Sherman•14mo ago

Can you share a real-world use case where you need to add joins to a query based on specific external values passed from the client?

DiamondDragon•14mo ago

Trying to design my app with Airtable like query/filter capabilities across other relational data... so user could filter on a "Contacts" object by applying filters on related data like a "Tags" object, filtering for "Tags that have conditions A, D, F, and E". I could totally be approaching this completely wrong tho

Andrii Sherman•14mo ago

good, this scenario makes sense for me @alexblokh @bloberenober I guess this is a good example for conditional joins

DiamondDragon•14mo ago

Yeah, this is the kind of query capabilities I'd love to build on the front end. Love the power& use cases it enables on the user side

Kuba•9mo ago

Was something changed in the meantime how sql behaves since this time? I tried implementing this pattern, but I'm facing a strange problem: - when I write sql`id` a result is found without a problem, - when I try interpolating id string in this manner: sql`${this.primaryKey}` I get 0 results from the query. (The id string is passed to the class as an argument and is saved to a field.) I ensured that the primaryKey has "id" string and I cannot understand why it I get 0 results when interpolating it @Andrii Sherman So in the nutshell, this works:

.where(eq(sql`id`, id));

.where(eq(sql`id`, id));

but this doesn't:

.where(eq(sql`${"id"}`, id));

.where(eq(sql`${"id"}`, id));

Inspecting this further with a debugger results that these are indeed different. The difference is in the queryChunks attribute:

// 1 chunk: StringChunk
const sqlRegular = sql`id`;

// 3 chunks: StringChunk, "id", String chunk
const value = "id";
const sqlInterpolated = sql`${value}`;

// 1 chunk: StringChunk
const sqlRegular = sql`id`;

// 3 chunks: StringChunk, "id", String chunk
const value = "id";
const sqlInterpolated = sql`${value}`;

Kuba•9mo ago

Angelelz•9mo ago

This is correct behavior. The SQL operator will take any dynamic parameter you pass, and put it in a parameters array to avoid SQL injection

Kuba•9mo ago

Exactly. I've read up on this on docs, as I was just curious why it isn't working (I assumed it worked in the code sent by @Ayaz ). As sql operator produces parametrized queries, now I understand that this evaluates to WHERE ('id' = 'some-id') , so this is always false and returns no results. sql.raw() is what I needed, as it doesn't escape the string pased to it. https://orm.drizzle.team/docs/sql#sqlraw

Drizzle ORM - next gen TypeScript ORM

Drizzle ORM is a lightweight and performant TypeScript ORM with developer experience in mind.

minester16•9mo ago

@Ayaz , that's an awesome example. And really helped me. Do you have github address which I can browse? I really like how you code and wanna inspect your proejcts and learn from them.

Ayaz•9mo ago

Hi @minester16 , sadly, I don't really have open projects to highlight these stuff, most of them are on the job.

Kuba•7mo ago

If anyone would need to add custom ordering to this pattern, take a look at https://discord.com/channels/1043890932593987624/1217899465508257893 If anyone needs to extend the above with upserts, there you go:

async upsert(
  data: InferInsertModel<TTable> & PgUpdateSetSource<TTable>,
): Promise<InferSelectModel<TTable>> {
  const upsertRes = await this.database
    .insert(this.table)
    .values(data)
    .onConflictDoUpdate({
      target: { name: "id" } as unknown as IndexColumn,
      set: data,
    })
    .returning();

  return upsertRes[0] as InferSelectModel<TTable>;
}

async upsert(
  data: InferInsertModel<TTable> & PgUpdateSetSource<TTable>,
): Promise<InferSelectModel<TTable>> {
  const upsertRes = await this.database
    .insert(this.table)
    .values(data)
    .onConflictDoUpdate({
      target: { name: "id" } as unknown as IndexColumn,
      set: data,
    })
    .returning();

  return upsertRes[0] as InferSelectModel<TTable>;
}

Kuba•7mo ago

The name attribute comes from this line of code

GitHub

drizzle-orm/drizzle-orm/src/pg-core/query-builders/insert.ts at 470...

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅 - drizzle-team/drizzle-orm

Gaming

Programming

Help with types in Repository Pattern