-
Notifications
You must be signed in to change notification settings - Fork 16
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
Database driver adaptations #32
base: master
Are you sure you want to change the base?
Conversation
Here's a patch for #33 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e950cfa..001c3d0 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -18,8 +18,18 @@ jobs:
env:
MYSQL_ALLOW_EMPTY_PASSWORD: "true"
MYSQL_ROOT_PASSWORD: ""
- options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3
+ options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3
+ postgres:
+ image: postgres:10.8
+ ports:
+ - 5432:5432
+ env:
+ POSTGRES_USER: "thankgodukachukwu"
+ POSTGRES_PASSWORD: "test_orm"
+ POSTGRES_DB: "test_orm"
+ options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
+
steps:
- uses: actions/checkout@v2
- name: Setup Deno
diff --git a/docker-compose.yml b/docker-compose.yml
index 682dddf..33420df 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -7,4 +7,12 @@ services:
- 3306:3306
environment:
MYSQL_ALLOW_EMPTY_PASSWORD: "true"
- MYSQL_ROOT_PASSWORD: ""
\ No newline at end of file
+ MYSQL_ROOT_PASSWORD: ""
+ postgres:
+ image: postgres:10.8
+ ports:
+ - 5432:5432
+ environment:
+ POSTGRES_USER: "thankgodukachukwu"
+ POSTGRES_PASSWORD: "test_orm"
+ POSTGRES_DB: "test_orm"
\ No newline at end of file
diff --git a/test.ts b/test.ts
index 8be1365..8c810d1 100644
--- a/test.ts
+++ b/test.ts
@@ -46,7 +46,7 @@ const config2 = {
user: "thankgodukachukwu",
database: "test_orm",
hostname: "127.0.0.1",
- password: "",
+ password: "test_orm",
port: 5432,
}; |
Thanks, two more things:
steps: with steps: |
export interface Base { | ||
type: string; // MySQl|Postgres|Sqlite | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the base file to define a unified abstraction of different database clients, similar to:
export enum ClientType {
MYSQL = "MYSQL",
POSTGRESS = "POSTGRESS",
SQLITE = "SQLITE",
}
export interface DsoConnection {
query<T = any>(sql: string, params?: any[]): Promise<T>;
execute<T = any>(sql: string, params?: any[]): Promise<T>;
}
/** Transaction processor */
export interface TransactionProcessor<T> {
(connection: DsoConnection): Promise<T>;
}
export interface PoolInfo {
size: number;
maxSize: number;
available: number;
}
/**
* DSO client
*/
export abstract class DsoClient {
/** get pool info */
abstract get pool(): PoolInfo;
/**
* connect to database
* @param config config for client
* @returns Clinet instance
*/
abstract connect<T>(config: T): Promise<DsoClient>;
/**
* excute query sql
* @param sql query sql string
* @param params query params
*/
abstract async query<T = any>(sql: string, params?: any[]): Promise<T>;
/**
* excute sql
* @param sql sql string
* @param params query params
*/
abstract async execute<T = any>(sql: string, params?: any[]): Promise<T>;
/**
* Execute a transaction process, and the transaction successfully
* returns the return value of the transaction process
* @param processor transation processor
*/
abstract async transaction<T = any>(
processor: TransactionProcessor<T>
): Promise<T>;
/**
* close connection
*/
abstract async close(): Promise<void>;
}
// import { Client } from "../../deps.ts";
// export class MysqlClient extends DsoClient {
// #client: Client = new Client();
// async close(): Promise<void> {
// this.#client.close();
// }
// async connect(config: MysqlConfig): Promise<DsoClient> {
// await this.#client.connect(config);
// return this;
// }
// ...
// }
This can avoid using too much template code when dso implements core functions, which is not conducive to maintenance and understanding
get configClientReturn(): _ClientType { | ||
return _configClientReturn; | ||
}, | ||
|
||
/** | ||
* all models |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we establish the correct client abstraction, the client used or exported by dso at this time will be DsoClient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of work. Thank you for your effort. I suggest to separate some of the changes and merge them first. For example, the abstraction of DsoClient and the implementation of MySQL.
async connect<T extends PostgresConfig | MysqlConfig | SqliteConfig>( | ||
config: T, | ||
): Promise<_ClientType | undefined> { | ||
if (config["type"].toUpperCase() === "POSTGRES") { | ||
_clientPostgres = new PostgresClient(config["clientConfig"]); | ||
await _clientPostgres.connect(); | ||
|
||
return _configClientReturn = { | ||
postgres: _clientPostgres, | ||
}; | ||
} else if (config["type"].toUpperCase() === "MYSQL") { | ||
if (config["client"] && config["client"] instanceof Client) { | ||
_client = config["client"]; | ||
} else { | ||
_client = new Client(); | ||
await _client.connect(config["clientConfig"]); | ||
} | ||
return _configClientReturn = { | ||
mysql: _client, | ||
}; | ||
} else if (config["type"].toUpperCase() === "SQLITE") { | ||
const configgy: any = config["clientConfig"]; | ||
|
||
_clientSqlite = new SqliteClient(configgy["database"]); | ||
|
||
return _configClientReturn = { | ||
sqlite: _clientSqlite, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since different databases will inherit and implement a unified DsoClient abstract class, I think such a large amount of template code will be avoided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update with the changes requested.
Following issue #4 , the Postgres referenced in #2 was adapted. Deno-sqlite was selected. DSO can now run on Postgres and Sqlite as well as MySQL.
Changes have been made to readme file. It seems there is need to bump up Deno version to 1.1.
Github workflow for automated testing needs to be updated to have Postgres and Sqlite Databases.