From 7681c1b4a387543ab7b05dcdf07cbdfef2c86f5b Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Sun, 10 Nov 2024 23:50:20 +0100 Subject: [PATCH 01/10] feature: Query builder --- lib/src/repository/tag.dart | 25 ++++---- lib/src/services/db.dart | 7 ++- lib/src/util/query_builder.dart | 105 ++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 lib/src/util/query_builder.dart diff --git a/lib/src/repository/tag.dart b/lib/src/repository/tag.dart index 849d05b..8607717 100644 --- a/lib/src/repository/tag.dart +++ b/lib/src/repository/tag.dart @@ -3,6 +3,7 @@ import 'package:logging/logging.dart'; import 'package:postgres/postgres.dart'; import 'package:running_on_dart/running_on_dart.dart'; import 'package:running_on_dart/src/models/tag.dart'; +import 'package:running_on_dart/src/util/query_builder.dart'; class TagRepository { final _database = Injector.appInstance.get(); @@ -11,17 +12,19 @@ class TagRepository { /// Fetch all existing tags from the database. Future> fetchAllActiveTags() async { - final result = await _database.getConnection().execute(Sql.named(''' - SELECT * FROM tags WHERE enabled = TRUE; - ''')); + final query = SelectQuery.selectAll('tags')..andWhere('enabled = TRUE'); + + final result = await _database.executeQuery(query); return result.map((row) => row.toColumnMap()).map(Tag.fromRow); } Future> fetchActiveTagsByName(String nameQuery) async { - final result = await _database.getConnection().execute(Sql.named(''' - SELECT * FROM tags WHERE enabled = TRUE AND name LIKE @nameQuery; - '''), parameters: {'nameQuery': '%$nameQuery%'}); + final query = SelectQuery.selectAll("tags") + ..andWhere("enabled = TRUE") + ..andWhere("name LIKE @nameQuery"); + + final result = await _database.executeQuery(query, parameters: {'nameQuery': '%$nameQuery%'}); return result.map((row) => row.toColumnMap()).map(Tag.fromRow); } @@ -34,11 +37,11 @@ class TagRepository { return; } - await _database.getConnection().execute(Sql.named(''' - UPDATE tags SET enabled = FALSE WHERE id = @id; - '''), parameters: { - 'id': id, - }); + final query = UpdateQuery("tags") + ..addSet("enabled", "FALSE") + ..andWhere("id = @id"); + + await _database.executeQuery(query, parameters: {'id': id}); } /// Add a tag to the database. diff --git a/lib/src/services/db.dart b/lib/src/services/db.dart index 95d84d5..3428e85 100644 --- a/lib/src/services/db.dart +++ b/lib/src/services/db.dart @@ -6,6 +6,7 @@ import 'package:migent/migent.dart'; import 'package:postgres/postgres.dart'; import 'package:running_on_dart/src/settings.dart'; +import 'package:running_on_dart/src/util/query_builder.dart'; import 'package:running_on_dart/src/util/util.dart'; /// The user to use when connecting to the database. @@ -163,7 +164,7 @@ class DatabaseService implements RequiresInitialization { token VARCHAR NOT NULL, jellyfin_config_id INT NOT NULL, CONSTRAINT fk_jellyfin_configs - FOREIGN KEY(jellyfin_config_id) + FOREIGN KEY(jellyfin_config_id) REFERENCES jellyfin_configs(id) ); ''') @@ -178,4 +179,8 @@ class DatabaseService implements RequiresInitialization { } Connection getConnection() => _connection; + + Future executeQuery(Query query, {Map? parameters}) { + return getConnection().execute(query.build(), parameters: parameters); + } } diff --git a/lib/src/util/query_builder.dart b/lib/src/util/query_builder.dart new file mode 100644 index 0000000..f454f93 --- /dev/null +++ b/lib/src/util/query_builder.dart @@ -0,0 +1,105 @@ +import 'package:postgres/postgres.dart'; + +class QueryBuilderException implements Exception { + final String message; + + QueryBuilderException(this.message); + + @override + String toString() => "QueryBuilderException: $message"; +} + +String buildSelects(List selects) { + if (selects.isEmpty) { + throw QueryBuilderException("No select statements provided. Select query needs at least one select statement."); + } + + return selects.join(","); +} + +String buildWheres(List wheres, String type) { + return wheres.join(" $type "); +} + +String buildSets(Map sets) { + return sets.entries.map((entry) => '${entry.key} = ${entry.value}').join(","); +} + +abstract class Query { + final String from; + + Query(this.from); + + Sql build(); +} + +abstract class _WhereQuery extends Query { + final List _andWheres = []; + final List _orWheres = []; + + _WhereQuery(super.from); + + void andWhere(String expression) => _andWheres.add(expression); + void orWhere(String expression) => _orWheres.add(expression); + + void _buildWheres(StringBuffer buffer) { + if (_andWheres.isNotEmpty) { + buffer.write("WHERE "); + buffer.write(buildWheres(_andWheres, 'AND')); + } + + if (_orWheres.isNotEmpty) { + if (_andWheres.isEmpty) { + buffer.write("WHERE "); + } else { + buffer.write("OR "); + } + buffer.write(buildWheres(_orWheres, "OR")); + } + } +} + +class UpdateQuery extends _WhereQuery { + final Map _sets = {}; + + UpdateQuery(super.from); + + void addSet(String name, String value) => _sets[name] = value; + + @override + Sql build() { + if (_andWheres.isEmpty && _orWheres.isEmpty) { + throw QueryBuilderException("Update query requires where statement"); + } + + final buffer = StringBuffer("UPDATE $from SET "); + buffer.write(buildSets); + + _buildWheres(buffer); + + buffer.write(";"); + return Sql.named(buffer.toString()); + } +} + +class SelectQuery extends _WhereQuery { + final List _selects = []; + + SelectQuery(super.from); + factory SelectQuery.selectAll(String from) => SelectQuery(from)..select("*"); + + void select(String expression) => _selects.add(expression); + + @override + Sql build() { + final buffer = StringBuffer("SELECT "); + buffer.write(buildSelects(_selects)); + buffer.write(" FROM $from "); + + _buildWheres(buffer); + + buffer.write(";"); + + return Sql.named(buffer.toString()); + } +} From 4ee57cb583b7be5d52845d9bb70a32319f2925eb Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 02:50:03 +0100 Subject: [PATCH 02/10] Add test workflow; Implement InsertQuery; Update deps --- .github/workflows/test.yml | 105 +++++++++++++++++++++ Makefile | 11 ++- lib/src/util/query_builder.dart | 33 ++++++- lib/src/util/util.dart | 2 +- pubspec.lock | 160 +++++++++++++++++++++++++++++++- pubspec.yaml | 1 + test/query_builder_test.dart | 69 ++++++++++++++ 7 files changed, 372 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 test/query_builder_test.dart diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..33f2309 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,105 @@ +name: Test + +on: + push: + branches: + - rewrite + pull_request: + +jobs: + analyze: + name: dart analyze + runs-on: ubuntu-latest + steps: + - name: Setup Dart Action + uses: dart-lang/setup-dart@v1 + + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache + uses: actions/cache@v4 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pubspec-${{ hashFiles('**/pubspec.lock') }} + restore-keys: | + ${{ runner.os }}-pubspec- + + - name: Install dependencies + run: dart pub get + + - name: Analyze project source + run: dart analyze + + fix: + name: dart fix + runs-on: ubuntu-latest + steps: + - name: Setup Dart Action + uses: dart-lang/setup-dart@v1 + + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache + uses: actions/cache@v4 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pubspec-${{ hashFiles('**/pubspec.lock') }} + restore-keys: | + ${{ runner.os }}-pubspec- + + - name: Install dependencies + run: dart pub get + + - name: Analyze project source + run: dart fix --dry-run + + format: + name: dart format + runs-on: ubuntu-latest + steps: + - name: Setup Dart Action + uses: dart-lang/setup-dart@v1 + + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache + uses: actions/cache@v4 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pubspec-${{ hashFiles('**/pubspec.lock') }} + restore-keys: | + ${{ runner.os }}-pubspec- + + - name: Install dependencies + run: dart pub get + + - name: Format + run: dart format --set-exit-if-changed -l 160 ./lib + + tests: + needs: [ format, analyze, fix ] + name: Unit tests + runs-on: ubuntu-latest + steps: + - name: Setup Dart Action + uses: dart-lang/setup-dart@v1 + + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache + uses: actions/cache@v4 + with: + path: ~/.pub-cache + key: ${{ runner.os }}-pubspec-${{ hashFiles('**/pubspec.lock') }} + restore-keys: | + ${{ runner.os }}-pubspec- + + - name: Install dependencies + run: dart pub get + + - name: Unit tests + run: dart run test test/unit/** diff --git a/Makefile b/Makefile index 63493f4..5a0eec7 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,12 @@ format: ## Run dart format fix: ## Run dart fix dart fix --apply -fix-project: fix format ## Fix whole project +analyze: ## Run dart analyze + dart analyze -run: ## Run dev project - docker compose up --build \ No newline at end of file +tests: ## Run unit tests + dart run test + +fix-project: analyze fix format ## Fix whole project + +check-project: fix-project tests ## Run all checks diff --git a/lib/src/util/query_builder.dart b/lib/src/util/query_builder.dart index f454f93..13d11c4 100644 --- a/lib/src/util/query_builder.dart +++ b/lib/src/util/query_builder.dart @@ -25,6 +25,13 @@ String buildSets(Map sets) { return sets.entries.map((entry) => '${entry.key} = ${entry.value}').join(","); } +(String, String) buildInsert(Map inserts) { + return ( + inserts.entries.map((entry) => entry.key).join(","), + inserts.entries.map((entry) => entry.value).join(","), + ); +} + abstract class Query { final String from; @@ -59,6 +66,29 @@ abstract class _WhereQuery extends Query { } } +class InsertQuery extends Query { + final Map _inserts = {}; + + InsertQuery(super.from); + + void addInsert(String name, String value) => _inserts[name] = value; + void addNamedInsert(String name) => _inserts[name] = '@$name'; + + @override + Sql build() { + final buffer = StringBuffer("INSERT INTO $from ("); + + final (fields, values) = buildInsert(_inserts); + + buffer.write(fields); + buffer.write(") VALUES ("); + buffer.write(values); + buffer.write(");"); + + return Sql.named(buffer.toString()); + } +} + class UpdateQuery extends _WhereQuery { final Map _sets = {}; @@ -73,7 +103,8 @@ class UpdateQuery extends _WhereQuery { } final buffer = StringBuffer("UPDATE $from SET "); - buffer.write(buildSets); + buffer.write(buildSets(_sets)); + buffer.write(" "); _buildWheres(buffer); diff --git a/lib/src/util/util.dart b/lib/src/util/util.dart index c710fc7..899f10a 100644 --- a/lib/src/util/util.dart +++ b/lib/src/util/util.dart @@ -19,7 +19,7 @@ String getCurrentMemoryString() { return '$current/$rss MB'; } -String getDartPlatform() => Platform.version.split('(').first; +String getDartPlatform() => Platform.version.split('(').first.trim(); extension DurationFromTicks on Duration { String formatShort() => toString().split('.').first.padLeft(8, "0"); diff --git a/pubspec.lock b/pubspec.lock index b5ffddb..46cee48 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -105,6 +105,14 @@ packages: url: "https://pub.dev" source: hosted version: "3.1.2" + coverage: + dependency: transitive + description: + name: coverage + sha256: "4b03e11f6d5b8f6e5bb5e9f7889a56fe6c5cbe942da5378ea4d4d7f73ef9dfe5" + url: "https://pub.dev" + source: hosted + version: "1.11.0" crypto: dependency: transitive description: @@ -161,6 +169,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.1.1" + frontend_server_client: + dependency: transitive + description: + name: frontend_server_client + sha256: f64a0333a82f30b0cca061bc3d143813a486dc086b574bfb233b7c1372427694 + url: "https://pub.dev" + source: hosted + version: "4.0.0" fuzzy: dependency: "direct main" description: @@ -193,6 +209,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.2.2" + http_multi_server: + dependency: transitive + description: + name: http_multi_server + sha256: "97486f20f9c2f7be8f514851703d0119c3596d14ea63227af6f7a481ef2b2f8b" + url: "https://pub.dev" + source: hosted + version: "3.2.1" http_parser: dependency: transitive description: @@ -225,6 +249,22 @@ packages: url: "https://pub.dev" source: hosted version: "0.19.0" + io: + dependency: transitive + description: + name: io + sha256: "2ec25704aba361659e10e3e5f5d672068d332fc8ac516421d483a11e5cbd061e" + url: "https://pub.dev" + source: hosted + version: "1.0.4" + js: + dependency: transitive + description: + name: js + sha256: c1b2e9b5ea78c45e1a0788d29606ba27dc5f71f019f32ca5140f61ef071838cf + url: "https://pub.dev" + source: hosted + version: "0.7.1" lints: dependency: "direct dev" description: @@ -245,10 +285,10 @@ packages: dependency: transitive description: name: matcher - sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 + sha256: d2323aa2060500f906aa31a895b4030b6da3ebdcc5619d14ce1aada65cd161cb url: "https://pub.dev" source: hosted - version: "0.12.17" + version: "0.12.16+1" meta: dependency: transitive description: @@ -266,6 +306,22 @@ packages: url: "https://github.com/nyxx-discord/migent.git" source: git version: "1.1.0" + mime: + dependency: transitive + description: + name: mime + sha256: "41a20518f0cb1256669420fdba0cd90d21561e560ac240f26ef8322e45bb7ed6" + url: "https://pub.dev" + source: hosted + version: "2.0.0" + node_preamble: + dependency: transitive + description: + name: node_preamble + sha256: "6e7eac89047ab8a8d26cf16127b5ed26de65209847630400f9aefd7cd5c730db" + url: "https://pub.dev" + source: hosted + version: "2.0.2" nyxx: dependency: "direct main" description: @@ -342,10 +398,10 @@ packages: dependency: "direct main" description: name: postgres - sha256: c271fb05cf83f47ff8d6915ea7fc780381e581309f55846a21a3257ad6b05f6d + sha256: "9ce23b749d87aa0cc09b0328fe5193c86ad09d9940f920c887c6cbe85bea11cc" url: "https://pub.dev" source: hosted - version: "3.4.1" + version: "3.4.2" pub_semver: dependency: transitive description: @@ -394,6 +450,54 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.3" + shelf: + dependency: transitive + description: + name: shelf + sha256: e7dd780a7ffb623c57850b33f43309312fc863fb6aa3d276a754bb299839ef12 + url: "https://pub.dev" + source: hosted + version: "1.4.2" + shelf_packages_handler: + dependency: transitive + description: + name: shelf_packages_handler + sha256: "89f967eca29607c933ba9571d838be31d67f53f6e4ee15147d5dc2934fee1b1e" + url: "https://pub.dev" + source: hosted + version: "3.0.2" + shelf_static: + dependency: transitive + description: + name: shelf_static + sha256: c87c3875f91262785dade62d135760c2c69cb217ac759485334c5857ad89f6e3 + url: "https://pub.dev" + source: hosted + version: "1.1.3" + shelf_web_socket: + dependency: transitive + description: + name: shelf_web_socket + sha256: "073c147238594ecd0d193f3456a5fe91c4b0abbcc68bf5cd95b36c4e194ac611" + url: "https://pub.dev" + source: hosted + version: "2.0.0" + source_map_stack_trace: + dependency: transitive + description: + name: source_map_stack_trace + sha256: c0713a43e323c3302c2abe2a1cc89aa057a387101ebd280371d6a6c9fa68516b + url: "https://pub.dev" + source: hosted + version: "2.1.2" + source_maps: + dependency: transitive + description: + name: source_maps + sha256: "708b3f6b97248e5781f493b765c3337db11c5d2c81c3094f10904bfa8004c703" + url: "https://pub.dev" + source: hosted + version: "0.10.12" source_span: dependency: transitive description: @@ -451,6 +555,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.2.1" + test: + dependency: "direct dev" + description: + name: test + sha256: "713a8789d62f3233c46b4a90b174737b2c04cb6ae4500f2aa8b1be8f03f5e67f" + url: "https://pub.dev" + source: hosted + version: "1.25.8" test_api: dependency: transitive description: @@ -459,6 +571,14 @@ packages: url: "https://pub.dev" source: hosted version: "0.7.3" + test_core: + dependency: transitive + description: + name: test_core + sha256: "12391302411737c176b0b5d6491f466b0dd56d4763e347b6714efbaa74d7953d" + url: "https://pub.dev" + source: hosted + version: "0.6.5" typed_data: dependency: transitive description: @@ -475,6 +595,14 @@ packages: url: "https://pub.dev" source: hosted version: "0.3.0" + vm_service: + dependency: transitive + description: + name: vm_service + sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14" + url: "https://pub.dev" + source: hosted + version: "14.3.1" watcher: dependency: transitive description: @@ -491,6 +619,30 @@ packages: url: "https://pub.dev" source: hosted version: "1.1.0" + web_socket: + dependency: transitive + description: + name: web_socket + sha256: "3c12d96c0c9a4eec095246debcea7b86c0324f22df69893d538fcc6f1b8cce83" + url: "https://pub.dev" + source: hosted + version: "0.1.6" + web_socket_channel: + dependency: transitive + description: + name: web_socket_channel + sha256: "9f187088ed104edd8662ca07af4b124465893caf063ba29758f97af57e61da8f" + url: "https://pub.dev" + source: hosted + version: "3.0.1" + webkit_inspection_protocol: + dependency: transitive + description: + name: webkit_inspection_protocol + sha256: "87d3f2333bb240704cd3f1c6b5b7acd8a10e7f0bc28c28dcf14e782014f4a572" + url: "https://pub.dev" + source: hosted + version: "1.2.1" yaml: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index 46034f1..5bcdfe6 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -32,3 +32,4 @@ dependencies: dev_dependencies: lints: ^5.0.0 + test: ^1.25.8 diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart new file mode 100644 index 0000000..789bb9d --- /dev/null +++ b/test/query_builder_test.dart @@ -0,0 +1,69 @@ +import 'package:postgres/postgres.dart'; +import 'package:postgres/src/v3/query_description.dart' show SqlImpl; + +import 'package:running_on_dart/src/util/query_builder.dart'; +import 'package:test/expect.dart'; +import 'package:test/scaffolding.dart'; + +extension PostgresSqlStringExtension on Sql { + String asString() => (this as SqlImpl).sql; +} + +void main() { + group("Query builder tests", () { + group("Select tests", () { + test("Simple select", () { + final query = SelectQuery("test") + ..select("*") + ..andWhere("name = 'test'"); + + expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test';"); + }); + + test("Select without where", () { + final query = SelectQuery("test")..select("*"); + + expect(query.build().asString(), + "SELECT * FROM test ;"); // TODO: Somehow clean up unnecessary whitespaces after building query + }); + + test("Select multiple and statements", () { + final query = SelectQuery("test") + ..select("*") + ..andWhere("name = 'test'") + ..andWhere("model = 'xg'"); + + expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test' AND model = 'xg';"); + }); + + test("Select multiple or statements", () { + final query = SelectQuery("test") + ..select("*") + ..orWhere("name = 'test'") + ..orWhere("model = 'xg'"); + + expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test' OR model = 'xg';"); + }); + }); + + group("Update tests", () { + test("Simple update", () { + final query = UpdateQuery("test") + ..addSet("name", "moron") + ..andWhere("id = 1"); + + expect(query.build().asString(), "UPDATE test SET name = moron WHERE id = 1;"); + }); + }); + + group("Insert tests", () { + test("Simple insert", () { + final query = InsertQuery("test") + ..addInsert("name", "moron") + ..addNamedInsert("model"); + + expect(query.build().asString(), "INSERT INTO test (name,model) VALUES (moron,@model);"); + }); + }); + }); +} From 018cd323c2109c01b50d44049a122b18bac2375d Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 02:51:23 +0100 Subject: [PATCH 03/10] Fix test workflow format step --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 33f2309..3de124f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -77,7 +77,7 @@ jobs: run: dart pub get - name: Format - run: dart format --set-exit-if-changed -l 160 ./lib + run: dart format --set-exit-if-changed -l120 ./lib tests: needs: [ format, analyze, fix ] From 60b0f1f8840af45add9fa0fdeb5651f50d9e77ba Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 03:04:45 +0100 Subject: [PATCH 04/10] Fix test step --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3de124f..6c46e43 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -102,4 +102,4 @@ jobs: run: dart pub get - name: Unit tests - run: dart run test test/unit/** + run: dart run test From a0b557f38d3c1f3dd9f2468753436554f4f502bf Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 03:20:35 +0100 Subject: [PATCH 05/10] Add test for SelectQuery.selectAll --- test/query_builder_test.dart | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart index 789bb9d..9cae4fa 100644 --- a/test/query_builder_test.dart +++ b/test/query_builder_test.dart @@ -20,6 +20,13 @@ void main() { expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test';"); }); + test("Simple select all", () { + final query = SelectQuery.selectAll("test") + ..andWhere("name = 'test'"); + + expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test';"); + }); + test("Select without where", () { final query = SelectQuery("test")..select("*"); From ad94bf348a03d4f7f1ab70be8c4b63f8f09ac778 Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 10:09:12 +0100 Subject: [PATCH 06/10] Add join support --- lib/src/util/query_builder.dart | 74 ++++++++++++++++++++++++++------- test/query_builder_test.dart | 28 +++++++++++-- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/lib/src/util/query_builder.dart b/lib/src/util/query_builder.dart index 13d11c4..c48361b 100644 --- a/lib/src/util/query_builder.dart +++ b/lib/src/util/query_builder.dart @@ -32,20 +32,27 @@ String buildSets(Map sets) { ); } +String buildReturnings(List returnings) => returnings.join(","); + +extension ToStringCleanStringBufferExtension on StringBuffer { + String toStringClean() { + return toString().split(" ").where((substr) => substr.isNotEmpty).join(" ").replaceFirst(" ;", ";"); + } +} + abstract class Query { final String from; + final String? alias; - Query(this.from); + Query(this.from, {this.alias}); Sql build(); } -abstract class _WhereQuery extends Query { +mixin _WhereQuery implements Query { final List _andWheres = []; final List _orWheres = []; - _WhereQuery(super.from); - void andWhere(String expression) => _andWheres.add(expression); void orWhere(String expression) => _orWheres.add(expression); @@ -66,13 +73,43 @@ abstract class _WhereQuery extends Query { } } +class _Join { + final String target; + final String targetAlias; + final String joinType; // LEFT JOIN, RIGHT JOIN, JOIN, OUTER JOIN + final List conditions; + + _Join(this.target, this.targetAlias, this.joinType, this.conditions); + + String build() => "$joinType $target $targetAlias ON ${buildWheres(conditions, 'AND')}"; +} + +mixin _JoinQuery implements Query { + final List<_Join> _joins = []; + + void addJoin(String target, String alias, List conditions) => + _joins.add(_Join(target, alias, 'JOIN', conditions)); + void addLeftJoin(String target, String alias, List conditions) => + _joins.add(_Join(target, alias, 'LEFT JOIN', conditions)); + + void _buildJoins(StringBuffer buffer) { + if (_joins.isEmpty) { + return; + } + + buffer.write(_joins.map((join) => join.build()).join(",")); + } +} + class InsertQuery extends Query { final Map _inserts = {}; + final List _returnings = []; - InsertQuery(super.from); + InsertQuery(super.from, {super.alias}); void addInsert(String name, String value) => _inserts[name] = value; void addNamedInsert(String name) => _inserts[name] = '@$name'; + void addReturning(String name) => _returnings.add(name); @override Sql build() { @@ -83,16 +120,23 @@ class InsertQuery extends Query { buffer.write(fields); buffer.write(") VALUES ("); buffer.write(values); - buffer.write(");"); + buffer.write(")"); - return Sql.named(buffer.toString()); + if (_returnings.isNotEmpty) { + buffer.write(" RETURNING "); + buffer.write(buildReturnings(_returnings)); + } + + buffer.write(";"); + + return Sql.named(buffer.toStringClean()); } } -class UpdateQuery extends _WhereQuery { +class UpdateQuery extends Query with _WhereQuery { final Map _sets = {}; - UpdateQuery(super.from); + UpdateQuery(super.from, {super.alias}); void addSet(String name, String value) => _sets[name] = value; @@ -109,14 +153,14 @@ class UpdateQuery extends _WhereQuery { _buildWheres(buffer); buffer.write(";"); - return Sql.named(buffer.toString()); + return Sql.named(buffer.toStringClean()); } } -class SelectQuery extends _WhereQuery { +class SelectQuery extends Query with _WhereQuery, _JoinQuery { final List _selects = []; - SelectQuery(super.from); + SelectQuery(super.from, {super.alias}); factory SelectQuery.selectAll(String from) => SelectQuery(from)..select("*"); void select(String expression) => _selects.add(expression); @@ -125,12 +169,14 @@ class SelectQuery extends _WhereQuery { Sql build() { final buffer = StringBuffer("SELECT "); buffer.write(buildSelects(_selects)); - buffer.write(" FROM $from "); + buffer.write(" FROM $from ${alias ?? ""} "); + _buildJoins(buffer); + buffer.write(" "); _buildWheres(buffer); buffer.write(";"); - return Sql.named(buffer.toString()); + return Sql.named(buffer.toStringClean()); } } diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart index 9cae4fa..0501d5c 100644 --- a/test/query_builder_test.dart +++ b/test/query_builder_test.dart @@ -21,8 +21,7 @@ void main() { }); test("Simple select all", () { - final query = SelectQuery.selectAll("test") - ..andWhere("name = 'test'"); + final query = SelectQuery.selectAll("test")..andWhere("name = 'test'"); expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test';"); }); @@ -30,8 +29,7 @@ void main() { test("Select without where", () { final query = SelectQuery("test")..select("*"); - expect(query.build().asString(), - "SELECT * FROM test ;"); // TODO: Somehow clean up unnecessary whitespaces after building query + expect(query.build().asString(), "SELECT * FROM test;"); }); test("Select multiple and statements", () { @@ -51,6 +49,19 @@ void main() { expect(query.build().asString(), "SELECT * FROM test WHERE name = 'test' OR model = 'xg';"); }); + + test("Join another table", () { + final query = SelectQuery("test", alias: "t") + ..select("t.*") + ..select("ot.*") + ..orWhere("t.name = 'test'") + ..orWhere("t.model = 'xg'") + ..addJoin("other_table", "ot", ["ot.id = t.test_id"]) + ..addLeftJoin("another_table", "at", ["at.test_id = t.id"]); + + expect(query.build().asString(), + "SELECT t.*,ot.* FROM test t JOIN other_table ot ON ot.id = t.test_id,LEFT JOIN another_table at ON at.test_id = t.id WHERE t.name = 'test' OR t.model = 'xg';"); + }); }); group("Update tests", () { @@ -71,6 +82,15 @@ void main() { expect(query.build().asString(), "INSERT INTO test (name,model) VALUES (moron,@model);"); }); + + test("Insert with returning", () { + final query = InsertQuery("test") + ..addInsert("name", "moron") + ..addNamedInsert("model") + ..addReturning("id"); + + expect(query.build().asString(), "INSERT INTO test (name,model) VALUES (moron,@model) RETURNING id;"); + }); }); }); } From 13f05d0eedf73267477785a378bc428efcc2fa21 Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 10:46:04 +0100 Subject: [PATCH 07/10] Rewrite tag repository to use query builder --- lib/src/repository/tag.dart | 68 ++++++++++++++------------------- lib/src/util/query_builder.dart | 4 +- test/query_builder_test.dart | 23 +++++++++++ 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/lib/src/repository/tag.dart b/lib/src/repository/tag.dart index 8607717..36bbfd5 100644 --- a/lib/src/repository/tag.dart +++ b/lib/src/repository/tag.dart @@ -1,6 +1,5 @@ import 'package:injector/injector.dart'; import 'package:logging/logging.dart'; -import 'package:postgres/postgres.dart'; import 'package:running_on_dart/running_on_dart.dart'; import 'package:running_on_dart/src/models/tag.dart'; import 'package:running_on_dart/src/util/query_builder.dart'; @@ -51,21 +50,15 @@ class TagRepository { return; } - final result = await _database.getConnection().execute(Sql.named(''' - INSERT INTO tags ( - name, - content, - enabled, - guild_id, - author_id - ) VALUES ( - @name, - @content, - @enabled, - @guild_id, - @author_id - ) RETURNING id; - '''), parameters: { + final query = InsertQuery("tags") + ..addNamedInsert("name") + ..addNamedInsert("content") + ..addNamedInsert("enabled") + ..addNamedInsert("guild_id") + ..addNamedInsert("author_id") + ..addReturning("id"); + + final result = await _database.executeQuery(query, parameters: { 'name': tag.name, 'content': tag.content, 'enabled': tag.enabled, @@ -82,16 +75,15 @@ class TagRepository { return addTag(tag); } - await _database.getConnection().execute(Sql.named(''' - UPDATE tags SET - name = @name, - content = @content, - enabled = @enabled, - guild_id = @guild_id, - author_id = @author_id - WHERE - id = @id - '''), parameters: { + final query = UpdateQuery("tags") + ..addNamedSet('name') + ..addNamedSet('content') + ..addNamedSet('enabled') + ..addNamedSet('guild_id') + ..addNamedSet('author_id') + ..andWhere("id = @id"); + + await _database.executeQuery(query, parameters: { 'id': tag.id, 'name': tag.name, 'content': tag.content, @@ -102,25 +94,21 @@ class TagRepository { } Future> fetchTagUsage() async { - final result = await _database.getConnection().execute(Sql.named(''' - SELECT tu.* FROM tag_usage tu JOIN tags t ON t.id = tu.command_id AND t.enabled = TRUE; - ''')); + final query = SelectQuery.selectAll("tag_usage", alias: "tu") + ..addJoin('tags', 't', ['t.id = tu.command_id', 't.enabled = TRUE']); + + final result = await _database.executeQuery(query); return result.map((row) => row.toColumnMap()).map(TagUsedEvent.fromRow); } Future registerTagUsedEvent(TagUsedEvent event) async { - await _database.getConnection().execute(Sql.named(''' - INSERT INTO tag_usage ( - command_id, - use_date, - hidden - ) VALUES ( - @tag_id, - @use_date, - @hidden - ) - '''), parameters: { + final query = InsertQuery("tag_usage") + ..addNamedInsert("command_id") + ..addNamedInsert("use_date") + ..addNamedInsert("hidden"); + + await _database.executeQuery(query, parameters: { 'tag_id': event.tagId, 'use_date': event.usedAt, 'hidden': event.hidden, diff --git a/lib/src/util/query_builder.dart b/lib/src/util/query_builder.dart index c48361b..0d6fbc4 100644 --- a/lib/src/util/query_builder.dart +++ b/lib/src/util/query_builder.dart @@ -139,6 +139,7 @@ class UpdateQuery extends Query with _WhereQuery { UpdateQuery(super.from, {super.alias}); void addSet(String name, String value) => _sets[name] = value; + void addNamedSet(String name) => _sets[name] = "@$name"; @override Sql build() { @@ -161,7 +162,8 @@ class SelectQuery extends Query with _WhereQuery, _JoinQuery { final List _selects = []; SelectQuery(super.from, {super.alias}); - factory SelectQuery.selectAll(String from) => SelectQuery(from)..select("*"); + factory SelectQuery.selectAll(String from, {String? alias}) => + SelectQuery(from, alias: alias)..select("${alias != null ? '$alias.' : ''}*"); void select(String expression) => _selects.add(expression); diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart index 0501d5c..304411d 100644 --- a/test/query_builder_test.dart +++ b/test/query_builder_test.dart @@ -62,6 +62,20 @@ void main() { expect(query.build().asString(), "SELECT t.*,ot.* FROM test t JOIN other_table ot ON ot.id = t.test_id,LEFT JOIN another_table at ON at.test_id = t.id WHERE t.name = 'test' OR t.model = 'xg';"); }); + + test('Join multiple conditions', () { + final query = SelectQuery.selectAll("tag_usage", alias: "tu") + ..addJoin('tags', 't', ['t.id = tu.command_id', 't.enabled = TRUE']); + + expect(query.build().asString(), + "SELECT tu.* FROM tag_usage tu JOIN tags t ON t.id = tu.command_id AND t.enabled = TRUE;"); + }); + + test("Simple select all with alias", () { + final query = SelectQuery.selectAll("test", alias: 't')..andWhere("t.name = 'test'"); + + expect(query.build().asString(), "SELECT t.* FROM test t WHERE t.name = 'test';"); + }); }); group("Update tests", () { @@ -72,6 +86,15 @@ void main() { expect(query.build().asString(), "UPDATE test SET name = moron WHERE id = 1;"); }); + + test("Named sets", () { + final query = UpdateQuery("test") + ..addNamedSet("name") + ..addNamedSet("model") + ..andWhere("id = 1"); + + expect(query.build().asString(), "UPDATE test SET name = @name,model = @model WHERE id = 1;"); + }); }); group("Insert tests", () { From 034df1145c46f6e45c8a3778eaab95a83712016c Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 11:02:38 +0100 Subject: [PATCH 08/10] Add delete query --- lib/src/util/query_builder.dart | 20 ++++++++++++++++++++ test/query_builder_test.dart | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/src/util/query_builder.dart b/lib/src/util/query_builder.dart index 0d6fbc4..a300a26 100644 --- a/lib/src/util/query_builder.dart +++ b/lib/src/util/query_builder.dart @@ -44,6 +44,8 @@ abstract class Query { final String from; final String? alias; + String get aliasOrEmpty => alias ?? ''; + Query(this.from, {this.alias}); Sql build(); @@ -158,6 +160,24 @@ class UpdateQuery extends Query with _WhereQuery { } } +class DeleteQuery extends Query with _WhereQuery { + DeleteQuery(super.from); + + @override + Sql build() { + if (_andWheres.isEmpty && _orWheres.isEmpty) { + throw QueryBuilderException("Delete query requires where statement"); + } + + final buffer = StringBuffer("DELETE FROM $from $aliasOrEmpty "); + + _buildWheres(buffer); + buffer.write(";"); + + return Sql.named(buffer.toStringClean()); + } +} + class SelectQuery extends Query with _WhereQuery, _JoinQuery { final List _selects = []; diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart index 304411d..9844972 100644 --- a/test/query_builder_test.dart +++ b/test/query_builder_test.dart @@ -115,5 +115,13 @@ void main() { expect(query.build().asString(), "INSERT INTO test (name,model) VALUES (moron,@model) RETURNING id;"); }); }); + + group("Delete tests", () { + test("Simple delete", () { + final query = DeleteQuery("test")..andWhere("name = 'test'"); + + expect(query.build().asString(), "DELETE FROM test WHERE name = 'test';"); + }); + }); }); } From 349ac8edfae8dead5f6a08512963faba9023cfc5 Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 11:30:07 +0100 Subject: [PATCH 09/10] Fix tag usage quer --- Makefile | 3 +++ lib/src/repository/tag.dart | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5a0eec7..7541964 100644 --- a/Makefile +++ b/Makefile @@ -13,6 +13,9 @@ analyze: ## Run dart analyze tests: ## Run unit tests dart run test +run: ## Run dev project + docker compose up --build + fix-project: analyze fix format ## Fix whole project check-project: fix-project tests ## Run all checks diff --git a/lib/src/repository/tag.dart b/lib/src/repository/tag.dart index 36bbfd5..bb3736b 100644 --- a/lib/src/repository/tag.dart +++ b/lib/src/repository/tag.dart @@ -109,7 +109,7 @@ class TagRepository { ..addNamedInsert("hidden"); await _database.executeQuery(query, parameters: { - 'tag_id': event.tagId, + 'command_id': event.tagId, 'use_date': event.usedAt, 'hidden': event.hidden, }); From c35d736da5a12b3e9d339a5ddee3ae85bdac1398 Mon Sep 17 00:00:00 2001 From: Szymon Uglis Date: Mon, 11 Nov 2024 12:48:20 +0100 Subject: [PATCH 10/10] Add tests for other utils --- lib/src/util/util.dart | 4 +-- test/query_builder_test.dart | 3 +- test/utils_test.dart | 54 ++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 test/utils_test.dart diff --git a/lib/src/util/util.dart b/lib/src/util/util.dart index 899f10a..ee1c3d4 100644 --- a/lib/src/util/util.dart +++ b/lib/src/util/util.dart @@ -21,7 +21,7 @@ String getCurrentMemoryString() { String getDartPlatform() => Platform.version.split('(').first.trim(); -extension DurationFromTicks on Duration { +extension FormatShortDurationExtension on Duration { String formatShort() => toString().split('.').first.padLeft(8, "0"); } @@ -52,7 +52,7 @@ Iterable spliceEmbedsForMessageBuilders(Iterable e } } -Duration? getDurationFromStringOrDefault(String? durationString, Duration? defaultDuration) { +Duration? getDurationFromStringOrDefault(String? durationString, [Duration? defaultDuration]) { if (durationString == null) { return defaultDuration; } diff --git a/test/query_builder_test.dart b/test/query_builder_test.dart index 9844972..d9bdac6 100644 --- a/test/query_builder_test.dart +++ b/test/query_builder_test.dart @@ -2,8 +2,7 @@ import 'package:postgres/postgres.dart'; import 'package:postgres/src/v3/query_description.dart' show SqlImpl; import 'package:running_on_dart/src/util/query_builder.dart'; -import 'package:test/expect.dart'; -import 'package:test/scaffolding.dart'; +import 'package:test/test.dart'; extension PostgresSqlStringExtension on Sql { String asString() => (this as SqlImpl).sql; diff --git a/test/utils_test.dart b/test/utils_test.dart new file mode 100644 index 0000000..48f668e --- /dev/null +++ b/test/utils_test.dart @@ -0,0 +1,54 @@ +import 'package:running_on_dart/src/util/util.dart'; +import 'package:test/test.dart'; + +void main() { + test("random color test", () { + final color = getRandomColor(); + final secondColor = getRandomColor(); + + expect(color, isNot(secondColor)); + expect(color, color); + }); + + group("FormatShortDurationExtension", () { + test("Minutes and second", () { + final duration = Duration(minutes: 2, seconds: 56); + + expect(duration.formatShort(), '00:02:56'); + }); + + test("Hours, minutes and second", () { + final duration = Duration(hours: 10, minutes: 2, seconds: 56); + + expect(duration.formatShort(), '10:02:56'); + }); + }); + + group("valueOrNull", () { + test("value exists", () { + expect(valueOrNull('value'), 'value'); + }); + + test("whitespace", () { + expect(valueOrNull(' '), isNull); + }); + + test("null value", () { + expect(valueOrNull(null), isNull); + }); + }); + + group("getDurationFromStringOrDefault", () { + test("value exists", () { + expect(getDurationFromStringOrDefault("2 minutes"), Duration(minutes: 2)); + }); + + test("value null", () { + expect(getDurationFromStringOrDefault(null, Duration(seconds: 1)), Duration(seconds: 1)); + }); + + test("value null and default null", () { + expect(getDurationFromStringOrDefault(null, null), isNull); + }); + }); +}