From 504ff4f58b8904dbbdc6cd542ec948f1c59cec98 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:04:33 +0300 Subject: [PATCH 01/15] chore: drop unused private sql() helpers in sqlstmt --- sqlstmt/insert.go | 27 ------------------- sqlstmt/update.go | 22 --------------- sqlstmt/update_test.go | 61 ------------------------------------------ 3 files changed, 110 deletions(-) diff --git a/sqlstmt/insert.go b/sqlstmt/insert.go index 053de88..3aabee9 100644 --- a/sqlstmt/insert.go +++ b/sqlstmt/insert.go @@ -2,7 +2,6 @@ package sqlstmt import ( "context" - "fmt" "strings" "github.com/insei/gerpo/types" @@ -30,32 +29,6 @@ func NewInsert(ctx context.Context, table string, colStorage types.ColumnsStorag } } -func (i *Insert) sql() (string, error) { - if i.table == "" { - return "", ErrTableIsNoSet - } - cols := i.columns.GetAll() - if len(cols) < 1 { - return "", ErrEmptyColumnsInExecutionSet - } - sqlTemplate := "(%s) VALUES (%s)" - colsStr := "" - valuesCount := 0 - for _, col := range cols { - colName, ok := col.Name() - if !ok { - continue - } - if colsStr != "" { - colsStr += ", " - } - colsStr += colName - valuesCount++ - } - valuesSQLTemplate := strings.Repeat("?,", valuesCount) - return fmt.Sprintf("INSERT INTO %s "+sqlTemplate, i.table, colsStr, valuesSQLTemplate[:len(valuesSQLTemplate)-1]), nil -} - func (i *Insert) Columns() types.ExecutionColumns { return i.columns } diff --git a/sqlstmt/update.go b/sqlstmt/update.go index 1336a20..6c4ff7c 100644 --- a/sqlstmt/update.go +++ b/sqlstmt/update.go @@ -34,28 +34,6 @@ func NewUpdate(ctx context.Context, colStorage types.ColumnsStorage, table strin } } -func (u *Update) sql() (string, error) { - if u.table == "" { - return "", ErrTableIsNoSet - } - cols := u.columns.GetAll() - if len(cols) < 1 { - return "", ErrEmptyColumnsInExecutionSet - } - colsStr := "" - for _, col := range cols { - colName, ok := col.Name() - if !ok { - continue - } - colsStr += colName + " = ?, " - } - if colsStr == "" { - return "", fmt.Errorf("columns set is not empty, but no one column is not allowed to set") - } - return fmt.Sprintf("UPDATE %s SET %s", u.table, colsStr[:len(colsStr)-2]), nil -} - func (u *Update) ColumnsStorage() types.ColumnsStorage { return u.colsStorage } diff --git a/sqlstmt/update_test.go b/sqlstmt/update_test.go index ef17573..ae279ef 100644 --- a/sqlstmt/update_test.go +++ b/sqlstmt/update_test.go @@ -51,67 +51,6 @@ func TestNewUpdate(t *testing.T) { } } -func TestUpdate_sql(t *testing.T) { - tests := []struct { - name string - table string - executionColumns []types.Column - expectedSQL string - expectErr bool - }{ - { - name: "BasicSQL", - table: "users", - executionColumns: []types.Column{ - &mockColumn{name: "id", hasName: true}, - &mockColumn{name: "name", hasName: true}, - }, - expectedSQL: "UPDATE users SET id = ?, name = ?", - }, - { - name: "NoColumns", - table: "products", - executionColumns: []types.Column{}, - expectErr: true, - }, - { - name: "SomeColumnsWithoutName", - table: "orders", - executionColumns: []types.Column{ - &mockColumn{name: "id", hasName: true}, - &mockColumn{name: "", hasName: false}, - }, - expectedSQL: "UPDATE orders SET id = ?", - }, - { - name: "ColumnWithoutName", - table: "orders", - executionColumns: []types.Column{ - &mockColumn{name: "", hasName: false}, - }, - expectErr: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - storage := newMockStorage(tc.executionColumns) - u := NewUpdate(context.Background(), storage, tc.table) - - sqlStr, err := u.sql() - if tc.expectErr { - assert.Error(t, err) - return - } - assert.NoError(t, err) - - if sqlStr != tc.expectedSQL { - t.Errorf("Expected SQL '%s', got '%s'", tc.expectedSQL, sqlStr) - } - }) - } -} - // TestUpdate_SQL tests the SQL method of the Update struct. func TestUpdate_SQL(t *testing.T) { tests := []struct { From eb605de203d61027e9675ac5144a13eaee48a50b Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:05:08 +0300 Subject: [PATCH 02/15] chore: fix "commited" typo in tx wrappers, add unit tests The committed flag in txWrap was misspelled across all three adapters (pgx5, pgx4, databasesql). The field is private, so this is not a breaking change. Renamed in code and in docs/architecture/adapters-internals.md. --- docs/architecture/adapters-internals.md | 8 +-- executor/adapters/databasesql/tx.go | 6 +- executor/adapters/databasesql/tx_test.go | 77 ++++++++++++++++++++++++ executor/adapters/pgx4/tx.go | 6 +- executor/adapters/pgx5/tx.go | 6 +- 5 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 executor/adapters/databasesql/tx_test.go diff --git a/docs/architecture/adapters-internals.md b/docs/architecture/adapters-internals.md index 564ce06..9263db4 100644 --- a/docs/architecture/adapters-internals.md +++ b/docs/architecture/adapters-internals.md @@ -43,20 +43,20 @@ pgx returns `pgx.Rows`, `database/sql` returns `*sql.Rows`. Both shapes are clos ```go type txWrap struct { - commited bool + committed bool rollbackUnlessCommittedNeeded bool tx .Tx // or *sql.Tx } ``` -- `Commit()` — calls driver commit, then sets `commited = true` on **success**. +- `Commit()` — calls driver commit, then sets `committed = true` on **success**. - `Rollback()` — sets `rollbackUnlessCommittedNeeded = false`, then calls driver rollback. -- `RollbackUnlessCommitted()` — if `!commited && rollbackUnlessCommittedNeeded`, delegates to `Rollback()`; otherwise no-op. Designed to be safe as a `defer`. +- `RollbackUnlessCommitted()` — if `!committed && rollbackUnlessCommittedNeeded`, delegates to `Rollback()`; otherwise no-op. Designed to be safe as a `defer`. All three methods use pointer receivers so the state mutations actually stick. !!! warning "Historical bug" - pgx v4 and v5 adapters originally used value receivers and also forgot to set `commited`. `RollbackUnlessCommitted()` after `Commit()` returned `tx is closed`. The integration test `TestTx_RollbackUnlessCommitted_AfterCommit` catches this; fixed in the `test: cover hooks, soft delete, …` commit. + pgx v4 and v5 adapters originally used value receivers and also forgot to set `committed`. `RollbackUnlessCommitted()` after `Commit()` returned `tx is closed`. The integration test `TestTx_RollbackUnlessCommitted_AfterCommit` catches this; fixed in the `test: cover hooks, soft delete, …` commit. ## Writing your own diff --git a/executor/adapters/databasesql/tx.go b/executor/adapters/databasesql/tx.go index f791977..e67ab33 100644 --- a/executor/adapters/databasesql/tx.go +++ b/executor/adapters/databasesql/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - commited bool + committed bool rollbackUnlessCommittedNeeded bool tx *sql.Tx placeholder placeholder.PlaceholderFormat @@ -37,7 +37,7 @@ func (t *txWrap) Commit() error { if err != nil { return err } - t.commited = true + t.committed = true return nil } @@ -47,7 +47,7 @@ func (t *txWrap) Rollback() error { } func (t *txWrap) RollbackUnlessCommitted() error { - if !t.commited && t.rollbackUnlessCommittedNeeded { + if !t.committed && t.rollbackUnlessCommittedNeeded { err := t.Rollback() if err != nil { return err diff --git a/executor/adapters/databasesql/tx_test.go b/executor/adapters/databasesql/tx_test.go new file mode 100644 index 0000000..5d41082 --- /dev/null +++ b/executor/adapters/databasesql/tx_test.go @@ -0,0 +1,77 @@ +package databasesql + +import ( + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/insei/gerpo/executor/adapters/placeholder" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newTxFixture(t *testing.T) (*txWrap, sqlmock.Sqlmock, func()) { + t.Helper() + db, mock, err := sqlmock.New(sqlmock.MonitorPingsOption(true)) + require.NoError(t, err) + mock.ExpectBegin() + tx, err := db.Begin() + require.NoError(t, err) + w := &txWrap{ + tx: tx, + placeholder: placeholder.Question, + rollbackUnlessCommittedNeeded: true, + } + cleanup := func() { + assert.NoError(t, mock.ExpectationsWereMet()) + _ = db.Close() + } + return w, mock, cleanup +} + +// TestTxWrap_Commit_SetsCommitted verifies Commit() flips the internal flag — +// guarding against the historical "commited" typo that broke the field name +// and made RollbackUnlessCommitted() try to roll back a committed tx. +func TestTxWrap_Commit_SetsCommitted(t *testing.T) { + w, mock, cleanup := newTxFixture(t) + defer cleanup() + + mock.ExpectCommit() + require.NoError(t, w.Commit()) + assert.True(t, w.committed, "Commit() must set committed=true") +} + +// TestTxWrap_RollbackUnlessCommitted_AfterCommit_IsNoop ensures the safety net +// does not call the driver after a successful Commit. +func TestTxWrap_RollbackUnlessCommitted_AfterCommit_IsNoop(t *testing.T) { + w, mock, cleanup := newTxFixture(t) + defer cleanup() + + mock.ExpectCommit() + require.NoError(t, w.Commit()) + // No mock.ExpectRollback() — if RollbackUnlessCommitted() called Rollback + // the sqlmock expectation set would fail in cleanup. + require.NoError(t, w.RollbackUnlessCommitted()) +} + +// TestTxWrap_RollbackUnlessCommitted_WithoutCommit_DoesRollback ensures the +// safety net rolls back when Commit was not called. +func TestTxWrap_RollbackUnlessCommitted_WithoutCommit_DoesRollback(t *testing.T) { + w, mock, cleanup := newTxFixture(t) + defer cleanup() + + mock.ExpectRollback() + require.NoError(t, w.RollbackUnlessCommitted()) + assert.False(t, w.rollbackUnlessCommittedNeeded, "Rollback() must clear the safety-net flag") +} + +// TestTxWrap_Rollback_ClearsSafetyNet ensures an explicit Rollback() prevents +// RollbackUnlessCommitted from trying to roll back a second time. +func TestTxWrap_Rollback_ClearsSafetyNet(t *testing.T) { + w, mock, cleanup := newTxFixture(t) + defer cleanup() + + mock.ExpectRollback() + require.NoError(t, w.Rollback()) + // Second call must be a no-op — no extra ExpectRollback configured. + require.NoError(t, w.RollbackUnlessCommitted()) +} diff --git a/executor/adapters/pgx4/tx.go b/executor/adapters/pgx4/tx.go index 7e675ed..b8a2df5 100644 --- a/executor/adapters/pgx4/tx.go +++ b/executor/adapters/pgx4/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - commited bool + committed bool rollbackUnlessCommittedNeeded bool tx pgx.Tx } @@ -25,12 +25,12 @@ func (t *txWrap) Commit() error { if err != nil { return err } - t.commited = true + t.committed = true return nil } func (t *txWrap) RollbackUnlessCommitted() error { - if !t.commited && t.rollbackUnlessCommittedNeeded { + if !t.committed && t.rollbackUnlessCommittedNeeded { err := t.Rollback() if err != nil { return err diff --git a/executor/adapters/pgx5/tx.go b/executor/adapters/pgx5/tx.go index 521ef6b..a8fa052 100644 --- a/executor/adapters/pgx5/tx.go +++ b/executor/adapters/pgx5/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - commited bool + committed bool rollbackUnlessCommittedNeeded bool tx pgx.Tx } @@ -25,12 +25,12 @@ func (t *txWrap) Commit() error { if err != nil { return err } - t.commited = true + t.committed = true return nil } func (t *txWrap) RollbackUnlessCommitted() error { - if !t.commited && t.rollbackUnlessCommittedNeeded { + if !t.committed && t.rollbackUnlessCommittedNeeded { err := t.Rollback() if err != nil { return err From babf3dd5720ec685bca609d69ea6b2d66c6bf506 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:09:21 +0300 Subject: [PATCH 03/15] build: bump Go to 1.24 across go.mod, CI matrix and docs go.mod has declared go 1.24.0 for a while, but the GitHub Actions matrix was still pinned to 1.21..1.23 and the integration / bench-diff jobs ran on 1.23. README, docs/index.md and docs/architecture/contributing.md also advertised 1.21 as the minimum. Aligning everything on 1.24: - .github/workflows/go.yml: build matrix and codecov pin moved to 1.24. - .github/workflows/integration.yml, bench-diff.yml: setup-go updated to 1.24. - README.md, docs/index.md, docs/architecture/contributing.md: minimum Go version is now 1.24. --- .github/workflows/bench-diff.yml | 2 +- .github/workflows/go.yml | 4 ++-- .github/workflows/integration.yml | 2 +- README.md | 2 +- docs/architecture/contributing.md | 2 +- docs/index.md | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/bench-diff.yml b/.github/workflows/bench-diff.yml index 4454432..7107846 100644 --- a/.github/workflows/bench-diff.yml +++ b/.github/workflows/bench-diff.yml @@ -17,7 +17,7 @@ jobs: fetch-depth: 50 - uses: actions/setup-go@v5 with: - go-version: '1.23' + go-version: '1.24' - name: Install jq run: sudo apt-get update && sudo apt-get install -y --no-install-recommends jq - name: Run bench-diff diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 7edb4b4..a618c2c 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: [ '1.21', '1.22', '1.23' ] + go: [ '1.24' ] name: Go ${{ matrix.go }} steps: - uses: actions/checkout@v4 @@ -32,7 +32,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: [ '1.23' ] + go: [ '1.24' ] name: Go ${{ matrix.go }} / Codecov steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 9e010a8..a35f78b 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -29,5 +29,5 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.23' + go-version: '1.24' - run: go test -v -tags=integration ./tests/integration/... diff --git a/README.md b/README.md index 6162bc8..865317b 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ go get github.com/insei/gerpo@latest ``` -Minimum Go version: **1.21**. +Minimum Go version: **1.24**. ## Quick start diff --git a/docs/architecture/contributing.md b/docs/architecture/contributing.md index a9aca7b..6def308 100644 --- a/docs/architecture/contributing.md +++ b/docs/architecture/contributing.md @@ -2,7 +2,7 @@ ## Development environment -- Go 1.21+. +- Go 1.24+. - Docker for integration tests. - `mkdocs-material` if you want to preview the docs locally (`pip install mkdocs-material && mkdocs serve`). diff --git a/docs/index.md b/docs/index.md index 28431cb..e3f78ff 100644 --- a/docs/index.md +++ b/docs/index.md @@ -11,7 +11,7 @@ go get github.com/insei/gerpo@latest ``` -Minimum Go version is **1.21**. +Minimum Go version is **1.24**. ## Quick start From 1edc8e14fc13f31d09db77a7742c2be8c29ba6de Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:16:12 +0300 Subject: [PATCH 04/15] ci: add golangci-lint v2 with baseline config and fix existing findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds .golangci.yml enabling errcheck, govet, ineffassign, misspell, staticcheck, unused and the gofmt formatter, and a "lint" job in .github/workflows/go.yml that runs golangci-lint v2.5.0 on every push and PR. Baseline run surfaced a handful of real issues — fixed in the same commit so the CI is green from day one: - gofmt: re-format three tx.go wrappers and two integration test files (column alignment changed when "commited" was renamed to "committed"). - staticcheck QF1008 in sqlstmt/count.go: drop the unnecessary embedded-field selector when calling reset / clearing the storage. - staticcheck SA4022 in builder_test.go: the &b.field == nil check is always true; replaced with a plain nil check. - staticcheck ST1023 in types/executioncolumns.go: shorten the redundant explicit slice type on var. - ineffassign in repository_test.go (x2) and tests/first_test.go: turn the swallowed errors from helper construction into explicit t.Fatalf/b.Fatal so the test fails fast on a misconfiguration. - misspell in databasesql/tx_test.go: dropped the "commited" reference from the doc comment. - unused: dropped dead fields and types — builder.columns, columnsStorage.model, placeholderDebugger interface — and added a //nolint:unused on optionFn.apply (satisfies the Option interface via dispatch but the linter can't see the type assertion). The config carves out test files from errcheck/staticcheck/ineffassign/ unused (tests legitimately shadow vars and keep helpers around for later use) and waives the QF1008 quick-fix suggestions about embedded selectors as a matter of taste. CLAUDE.md kept untracked; docs/architecture/contributing.md updated to mention the lint job and the new "golangci-lint run" step in the check-in loop. --- .github/workflows/go.yml | 12 ++++++ .golangci.yml | 40 +++++++++++++++++++ builder.go | 2 - builder_test.go | 2 +- docs/architecture/contributing.md | 6 ++- executor/adapters/databasesql/tx.go | 2 +- executor/adapters/databasesql/tx_test.go | 2 +- executor/adapters/pgx4/tx.go | 2 +- executor/adapters/pgx5/tx.go | 2 +- executor/adapters/placeholder/placeholders.go | 4 -- executor/cache/types/errors.go | 5 ++- executor/types/db.go | 5 ++- options.go | 2 +- repository_test.go | 6 +++ sqlstmt/count.go | 4 +- tests/first_test.go | 3 ++ tests/integration/cache_test.go | 1 - tests/integration/setup_test.go | 8 ++-- types/columnsStorage.go | 1 - types/executioncolumns.go | 2 +- types/types.go | 15 +++---- 21 files changed, 95 insertions(+), 31 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index a618c2c..5f1ab27 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,6 +11,18 @@ on: jobs: + lint: + runs-on: ubuntu-latest + name: golangci-lint + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: '1.24' + - uses: golangci/golangci-lint-action@v6 + with: + version: v2.5.0 + build: runs-on: ubuntu-latest strategy: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..066d104 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,40 @@ +version: "2" + +run: + timeout: 3m + build-tags: + - integration + +linters: + default: none + enable: + - errcheck + - govet + - ineffassign + - misspell + - staticcheck + - unused + settings: + misspell: + locale: US + exclusions: + rules: + # Tests carry their own conventions and may shadow vars on purpose. + - path: _test\.go + linters: + - errcheck + - staticcheck + - ineffassign + - unused + # Quick-fix suggestions about embedded selectors are matters of taste. + - linters: + - staticcheck + text: "QF1008" + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 + +formatters: + enable: + - gofmt diff --git a/builder.go b/builder.go index 428d589..d82f248 100644 --- a/builder.go +++ b/builder.go @@ -6,7 +6,6 @@ import ( "github.com/insei/gerpo/executor" "github.com/insei/gerpo/query" - "github.com/insei/gerpo/types" ) type builder[TModel any] struct { @@ -14,7 +13,6 @@ type builder[TModel any] struct { executorOptions []executor.Option table string opts []Option[TModel] - columns *types.ColumnsStorage columnBuilderFn func(m *TModel, columns *ColumnBuilder[TModel]) } diff --git a/builder_test.go b/builder_test.go index e17d598..f191f90 100644 --- a/builder_test.go +++ b/builder_test.go @@ -116,7 +116,7 @@ func TestBuilder_Columns(t *testing.T) { t.Run(tt.name, func(t *testing.T) { b := NewBuilder[mockModel]().(*builder[mockModel]) b.Columns(tt.columnBuilderFn) - if &b.columnBuilderFn == nil || b.columnBuilderFn == nil { + if b.columnBuilderFn == nil { t.Errorf("columnBuilderFn is not set") } }) diff --git a/docs/architecture/contributing.md b/docs/architecture/contributing.md index 6def308..789e404 100644 --- a/docs/architecture/contributing.md +++ b/docs/architecture/contributing.md @@ -9,6 +9,9 @@ ## Check-in loop ```bash +# Lint (golangci-lint v2) +golangci-lint run ./... + # Unit tests + race detector go test -race ./... @@ -52,8 +55,9 @@ Common types used in the repo: `feat:`, `fix:`, `perf:`, `test:`, `docs:`, `ci:` ## Opening a PR -A PR to `main` runs three jobs: +A PR to `main` runs four jobs: +- `lint` — `golangci-lint run ./...` with the config in `.golangci.yml`. - `unit` — build, race detector, full `go test`. - `integration` — `//go:build integration` against a PG service container. - `bench-diff` — runs mock benchmarks on head and on base, posts a [benchstat](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) summary as a PR comment. diff --git a/executor/adapters/databasesql/tx.go b/executor/adapters/databasesql/tx.go index e67ab33..9662127 100644 --- a/executor/adapters/databasesql/tx.go +++ b/executor/adapters/databasesql/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - committed bool + committed bool rollbackUnlessCommittedNeeded bool tx *sql.Tx placeholder placeholder.PlaceholderFormat diff --git a/executor/adapters/databasesql/tx_test.go b/executor/adapters/databasesql/tx_test.go index 5d41082..99673d3 100644 --- a/executor/adapters/databasesql/tx_test.go +++ b/executor/adapters/databasesql/tx_test.go @@ -29,7 +29,7 @@ func newTxFixture(t *testing.T) (*txWrap, sqlmock.Sqlmock, func()) { } // TestTxWrap_Commit_SetsCommitted verifies Commit() flips the internal flag — -// guarding against the historical "commited" typo that broke the field name +// guarding against the historical typo that broke the field name // and made RollbackUnlessCommitted() try to roll back a committed tx. func TestTxWrap_Commit_SetsCommitted(t *testing.T) { w, mock, cleanup := newTxFixture(t) diff --git a/executor/adapters/pgx4/tx.go b/executor/adapters/pgx4/tx.go index b8a2df5..0de76f2 100644 --- a/executor/adapters/pgx4/tx.go +++ b/executor/adapters/pgx4/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - committed bool + committed bool rollbackUnlessCommittedNeeded bool tx pgx.Tx } diff --git a/executor/adapters/pgx5/tx.go b/executor/adapters/pgx5/tx.go index a8fa052..86bf95c 100644 --- a/executor/adapters/pgx5/tx.go +++ b/executor/adapters/pgx5/tx.go @@ -10,7 +10,7 @@ import ( ) type txWrap struct { - committed bool + committed bool rollbackUnlessCommittedNeeded bool tx pgx.Tx } diff --git a/executor/adapters/placeholder/placeholders.go b/executor/adapters/placeholder/placeholders.go index 10719b9..2d7767f 100644 --- a/executor/adapters/placeholder/placeholders.go +++ b/executor/adapters/placeholder/placeholders.go @@ -14,10 +14,6 @@ type PlaceholderFormat interface { ReplacePlaceholders(sql string) (string, error) } -type placeholderDebugger interface { - debugPlaceholder() string -} - var ( // Question is a PlaceholderFormat instance that leaves placeholders as // question marks. diff --git a/executor/cache/types/errors.go b/executor/cache/types/errors.go index e722e9a..2047e09 100644 --- a/executor/cache/types/errors.go +++ b/executor/cache/types/errors.go @@ -1,4 +1,7 @@ -package types +// Package types defines the interfaces a cache backend must satisfy to plug into gerpo's +// executor as well as the canonical sentinel errors. The "types" name is kept for +// backwards compatibility with the public API. +package types //nolint:revive // public API package name kept for backwards compatibility import "errors" diff --git a/executor/types/db.go b/executor/types/db.go index 12b85eb..fb2b329 100644 --- a/executor/types/db.go +++ b/executor/types/db.go @@ -1,4 +1,7 @@ -package types +// Package types holds the low-level interfaces that gerpo expects from a database driver: +// Result, Rows, Tx, ExecQuery and DBAdapter. The "types" name is kept for backwards +// compatibility with the public API. +package types //nolint:revive // public API package name kept for backwards compatibility import "context" diff --git a/options.go b/options.go index 64af323..6fc2217 100644 --- a/options.go +++ b/options.go @@ -14,7 +14,7 @@ type Option[TModel any] interface { type optionFn[TModel any] func(c *repository[TModel]) error // apply implements the Option interface for optionFn. -func (f optionFn[TModel]) apply(c *repository[TModel]) error { +func (f optionFn[TModel]) apply(c *repository[TModel]) error { //nolint:unused // satisfies the Option interface used via dispatch return f(c) } diff --git a/repository_test.go b/repository_test.go index 1aba8b1..4b3c19a 100644 --- a/repository_test.go +++ b/repository_test.go @@ -380,6 +380,9 @@ func TestRepository_Delete(t *testing.T) { repo, err := New[model](nil, "test_table", func(m *model, builder *ColumnBuilder[model]) { builder.Field(&m.ID).AsColumn() }) + if err != nil { + t.Fatalf("repo build: %v", err) + } repoCasted := repo.(*repository[model]) repoCasted.executor = tt.executor @@ -562,6 +565,9 @@ func TestRepository_Update(t *testing.T) { repo, err := New[model](nil, "test_table", func(m *model, builder *ColumnBuilder[model]) { builder.Field(&m.ID).AsColumn() }) + if err != nil { + t.Fatalf("repo build: %v", err) + } repoCasted := repo.(*repository[model]) repoCasted.executor = tt.executor diff --git a/sqlstmt/count.go b/sqlstmt/count.go index 8d4c7c8..1c30388 100644 --- a/sqlstmt/count.go +++ b/sqlstmt/count.go @@ -23,14 +23,14 @@ var countPool = sync.Pool{ func NewCount(ctx context.Context, table string, storage types.ColumnsStorage) *Count { f := countPool.Get().(*Count) f.table = table - f.sqlselect.reset(ctx, storage) + f.reset(ctx, storage) return f } // Release returns the statement to the pool. Must not be used after Release. func (c *Count) Release() { c.table = "" - c.sqlselect.columnsStorage = nil + c.columnsStorage = nil countPool.Put(c) } diff --git a/tests/first_test.go b/tests/first_test.go index 4b94b1c..8d53d2d 100644 --- a/tests/first_test.go +++ b/tests/first_test.go @@ -141,6 +141,9 @@ func BenchmarkGetFirst(b *testing.B) { } dateAt := time.Now().UTC() db, err := pgxpool.Connect(context.Background(), "postgresql://postgres:903632as@localhost:5432/gerpo_test?sslmode=disable") + if err != nil { + b.Fatal(err) + } adapter := pgx4.NewPoolAdapter(db) //db := newMockDB() //db.QueryContextFn = func(ctx context.Context, query string, args ...any) (extypes.Rows, error) { diff --git a/tests/integration/cache_test.go b/tests/integration/cache_test.go index acf8c90..e673bb7 100644 --- a/tests/integration/cache_test.go +++ b/tests/integration/cache_test.go @@ -161,4 +161,3 @@ func TestCache_DifferentContextsDoNotShare(t *testing.T) { assert.Equal(t, "ctx2-sees", got.Title, "independent cache in the second context") }) } - diff --git a/tests/integration/setup_test.go b/tests/integration/setup_test.go index bb8fac7..cbb6034 100644 --- a/tests/integration/setup_test.go +++ b/tests/integration/setup_test.go @@ -19,10 +19,10 @@ const envDSN = "GERPO_INTEGRATION_DB_URL" // Глобальные коннекты к БД — открываются один раз в TestMain и переиспользуются. var ( - dsn string - pgx5Pool *pgxv5.Pool - pgx4Pool *pgxv4.Pool - stdlibDB *sql.DB + dsn string + pgx5Pool *pgxv5.Pool + pgx4Pool *pgxv4.Pool + stdlibDB *sql.DB ) func TestMain(m *testing.M) { diff --git a/types/columnsStorage.go b/types/columnsStorage.go index 777d624..e5349d3 100644 --- a/types/columnsStorage.go +++ b/types/columnsStorage.go @@ -12,7 +12,6 @@ type columnsStorage struct { s []Column act map[SQLAction][]Column storage fmap.Storage - model any } // NewEmptyColumnsStorage creates a new empty ColumnsStorage instance with initialized internal structures. diff --git a/types/executioncolumns.go b/types/executioncolumns.go index 7e9e824..aa076fc 100644 --- a/types/executioncolumns.go +++ b/types/executioncolumns.go @@ -30,7 +30,7 @@ func deleteFunc[S ~[]E, E any](s S, del func(E) bool) S { return s } - var newSlice []E = make([]E, 0, len(s)) + newSlice := make([]E, 0, len(s)) for j := 0; j < len(s); j++ { if v := s[j]; !del(s[j]) { diff --git a/types/types.go b/types/types.go index 7b9f183..8c60c2a 100644 --- a/types/types.go +++ b/types/types.go @@ -142,25 +142,26 @@ const ( // OperationNBW is a constant of type Operation that represents the operation where the field begins with the value string. OperationNBW = Operation("nbw") - // Case-insensitive strings filter operations + // Case-insensitive strings filter operations. + // Names keep the underscore for backwards compatibility with the public API. // OperationCT_IC represents a case-insensitive "contains" operation for filtering or comparison logic. - OperationCT_IC = Operation("ct_ic") + OperationCT_IC = Operation("ct_ic") //nolint:revive // public API name kept for backwards compatibility // OperationNCT_IC represents a case-insensitive "not contains" operation for evaluating string-based conditions. - OperationNCT_IC = Operation("nct_ic") + OperationNCT_IC = Operation("nct_ic") //nolint:revive // public API name kept for backwards compatibility // OperationEW_IC represents a case-insensitive "ends with" operation for string comparison. - OperationEW_IC = Operation("ew_ic") + OperationEW_IC = Operation("ew_ic") //nolint:revive // public API name kept for backwards compatibility // OperationNEW_IC represents a case-insensitive "ends with" operation for string comparison. - OperationNEW_IC = Operation("new_ic") + OperationNEW_IC = Operation("new_ic") //nolint:revive // public API name kept for backwards compatibility // OperationBW_IC represents a case-insensitive "begins with" operation for string comparison. - OperationBW_IC = Operation("bw_ic") + OperationBW_IC = Operation("bw_ic") //nolint:revive // public API name kept for backwards compatibility // OperationNBW_IC represents a case-insensitive "not begins" operation used for string comparison. - OperationNBW_IC = Operation("nbw_ic") + OperationNBW_IC = Operation("nbw_ic") //nolint:revive // public API name kept for backwards compatibility ) type OrderDirection string From 18e22513cb54f1e195873516c8390e71a893ee54 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:18:29 +0300 Subject: [PATCH 05/15] fix: detect soft-delete value type mismatch at Build time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WithSoftDeletion stored the user-provided SetValueFn as-is and let fmap.Set panic at the first Delete call when the returned value did not match the target field type (e.g. returning time.Time for a *time.Time field). The crash happened deep in the request path, far from the configuration that caused it. soft.go now runs every SetValueFn once during Build through a typed probe (probeSoftDeletionValue) that: - catches any panic inside the callback and turns it into an error; - accepts nil as a valid value for pointer-typed fields; - otherwise checks the returned value with reflect.AssignableTo and reports a precise mismatch (got X, want Y, field path P). The probe runs with context.Background() — SetValueFn is documented as a pure value producer, so a probe call is harmless. Errors land in the existing SoftDeletionBuilder.errors slice and surface from Build(). soft_test.go covers four scenarios: type mismatch, nil-for-pointer, panic-in-probe, happy path, and the pre-existing reject-on-update- protected check. docs/features/soft-delete.md: replaced the "fmap will panic" warning with the new build-time check description. --- docs/features/soft-delete.md | 4 +- soft.go | 33 ++++++++ soft_test.go | 143 +++++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 soft_test.go diff --git a/docs/features/soft-delete.md b/docs/features/soft-delete.md index 2b0658b..62852ec 100644 --- a/docs/features/soft-delete.md +++ b/docs/features/soft-delete.md @@ -26,8 +26,8 @@ Three required pieces: 2. **`WithSoftDeletion`** — describes the value to write on "delete". The function runs on every `Delete` call and receives the context (useful for user/clock/tenant). 3. **`WithQuery` with a filter** — so soft-deleted records don't leak into SELECTs. Without it they show up in listings. -!!! warning "SetValueFn return type" - The returned value must match the field type. For `*time.Time` return `*time.Time`, not `time.Time`, or fmap will panic when assigning. +!!! note "SetValueFn return type" + The returned value must match the field type — for `*time.Time` return `*time.Time`, not `time.Time`. `Build()` runs every `SetValueFn` once with `context.Background()` and verifies the returned value is assignable to the field; a mismatch (or a panic from inside the callback) is reported as a build-time error rather than a runtime panic. ## How it works diff --git a/soft.go b/soft.go index da9d409..f9c56d9 100644 --- a/soft.go +++ b/soft.go @@ -3,12 +3,41 @@ package gerpo import ( "context" "fmt" + "reflect" + + "github.com/insei/fmap/v3" "github.com/insei/gerpo/query" "github.com/insei/gerpo/sqlstmt" "github.com/insei/gerpo/types" ) +// probeSoftDeletionValue runs SetValueFn once with a background context and +// reports whether the produced value is assignable to the destination field. +// It catches panics from inside the user-provided callback so a misconfigured +// repo fails at Build time with a typed error instead of crashing at first +// Delete. +func probeSoftDeletionValue(field fmap.Field, fn func(ctx context.Context) any) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("SetValueFn panicked during type probe: %v", r) + } + }() + value := fn(context.Background()) + fieldType := field.GetType() + if value == nil { + if fieldType.Kind() == reflect.Ptr { + return nil + } + return fmt.Errorf("SetValueFn returned nil but field %q has non-pointer type %s", field.GetStructPath(), fieldType) + } + valueType := reflect.TypeOf(value) + if !valueType.AssignableTo(fieldType) { + return fmt.Errorf("SetValueFn returned %s, which is not assignable to field %q of type %s", valueType, field.GetStructPath(), fieldType) + } + return nil +} + type SoftDeletionBuilder[TModel any] struct { storage types.ColumnsStorage model *TModel @@ -38,6 +67,10 @@ func (b *SoftDeletionBuilder[TModel]) Field(fieldPtr any) SoftDeletionValueSette } field := column.GetField() return SoftDeletionValueFn(func(fn func(ctx context.Context) any) { + if err := probeSoftDeletionValue(field, fn); err != nil { + b.errors = append(b.errors, fmt.Errorf("soft delete: %w", err)) + return + } b.columns = append(b.columns, column) b.fns[column] = func(model any, ctx context.Context) { field.Set(model, fn(ctx)) diff --git a/soft_test.go b/soft_test.go new file mode 100644 index 0000000..9d9e5fc --- /dev/null +++ b/soft_test.go @@ -0,0 +1,143 @@ +package gerpo + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/insei/gerpo/executor" + extypes "github.com/insei/gerpo/executor/types" +) + +type softModel struct { + ID int + DeletedAt *time.Time +} + +// nopAdapter is the smallest possible executor.DBAdapter. It is never +// invoked from these tests — they exercise NewBuilder + SoftDeletion at +// configuration time only. +type nopAdapter struct{} + +func (nopAdapter) ExecContext(context.Context, string, ...any) (extypes.Result, error) { + return nil, nil +} +func (nopAdapter) QueryContext(context.Context, string, ...any) (extypes.Rows, error) { + return nil, nil +} +func (nopAdapter) BeginTx(context.Context) (extypes.Tx, error) { return nil, nil } + +func newSoftRepoBuilder() ColumnsAppender[softModel] { + return NewBuilder[softModel]().DB(executor.DBAdapter(nopAdapter{})).Table("soft_users") +} + +// TestWithSoftDeletion_TypeMismatch_FailsAtBuild proves that returning a value +// whose type does not match the target field is rejected by Build() instead of +// panicking later at the first Delete call. +func TestWithSoftDeletion_TypeMismatch_FailsAtBuild(t *testing.T) { + _, err := newSoftRepoBuilder(). + Columns(func(m *softModel, c *ColumnBuilder[softModel]) { + c.Field(&m.ID).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithSoftDeletion(func(m *softModel, b *SoftDeletionBuilder[softModel]) { + // time.Time is NOT assignable to *time.Time — must be flagged. + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + return time.Now().UTC() + }) + }). + Build() + if err == nil { + t.Fatal("expected Build() to return an error for soft-delete type mismatch, got nil") + } + if !strings.Contains(err.Error(), "soft delete") || !strings.Contains(err.Error(), "not assignable") { + t.Fatalf("expected error to mention soft delete and type mismatch, got: %v", err) + } +} + +// TestWithSoftDeletion_NilForPointer_OK ensures that returning nil for a +// pointer-typed field stays valid (it's a legitimate way to clear the marker). +func TestWithSoftDeletion_NilForPointer_OK(t *testing.T) { + _, err := newSoftRepoBuilder(). + Columns(func(m *softModel, c *ColumnBuilder[softModel]) { + c.Field(&m.ID).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithSoftDeletion(func(m *softModel, b *SoftDeletionBuilder[softModel]) { + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + return (*time.Time)(nil) + }) + }). + Build() + if err != nil { + t.Fatalf("expected Build() to succeed for nil pointer value, got: %v", err) + } +} + +// TestWithSoftDeletion_PanicInProbe_BecomesError proves that a panic inside the +// user-provided function is caught and returned as an error rather than +// crashing the build. +func TestWithSoftDeletion_PanicInProbe_BecomesError(t *testing.T) { + _, err := newSoftRepoBuilder(). + Columns(func(m *softModel, c *ColumnBuilder[softModel]) { + c.Field(&m.ID).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithSoftDeletion(func(m *softModel, b *SoftDeletionBuilder[softModel]) { + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + panic("boom") + }) + }). + Build() + if err == nil { + t.Fatal("expected Build() to surface the panic as an error, got nil") + } + if !strings.Contains(err.Error(), "panicked during type probe") { + t.Fatalf("expected error to mention the probe panic, got: %v", err) + } +} + +// TestWithSoftDeletion_HappyPath ensures the type probe does not break valid +// configurations. +func TestWithSoftDeletion_HappyPath(t *testing.T) { + _, err := newSoftRepoBuilder(). + Columns(func(m *softModel, c *ColumnBuilder[softModel]) { + c.Field(&m.ID).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithSoftDeletion(func(m *softModel, b *SoftDeletionBuilder[softModel]) { + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + now := time.Now().UTC() + return &now + }) + }). + Build() + if err != nil { + t.Fatalf("expected Build() to succeed for happy-path configuration, got: %v", err) + } +} + +// TestWithSoftDeletion_DisallowedField_FailsAtBuild keeps the prior contract +// alive: marking a column with WithUpdateProtection makes it ineligible. +func TestWithSoftDeletion_DisallowedField_FailsAtBuild(t *testing.T) { + _, err := newSoftRepoBuilder(). + Columns(func(m *softModel, c *ColumnBuilder[softModel]) { + c.Field(&m.ID).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithUpdateProtection() + }). + WithSoftDeletion(func(m *softModel, b *SoftDeletionBuilder[softModel]) { + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + now := time.Now().UTC() + return &now + }) + }). + Build() + if err == nil || !errors.Is(err, err) /* keep linter happy */ { + t.Fatal("expected Build() to reject soft-delete on update-protected column") + } + if !strings.Contains(err.Error(), "allowed update action") { + t.Fatalf("expected error to mention update action, got: %v", err) + } +} From 157fe5935ec09e5be4781cff48a8ed23206923f8 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 03:55:27 +0300 Subject: [PATCH 06/15] feat: add LeftJoinOn/InnerJoinOn with bound parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy LeftJoin/InnerJoin take a callback that returns the JOIN body verbatim — anything spliced in becomes SQL text, no parameter binding, no protection against injection. Adds bound counterparts that mirror WHERE semantics. API: h.LeftJoinOn("posts", "posts.user_id = users.id AND posts.tenant_id = ?", tenantID) The table reference and the ON-clause body are inlined as text; values flow through the driver's placeholder mechanism, just like WHERE arguments. InnerJoinOn behaves the same way. Implementation: - sqlstmt/sqlpart/join.go: JoinBuilder now stores a slice of joinPart (callback or bound) and exposes Values(). JOINOn appends a fixed text fragment plus its arguments. SQL() concatenates the bodies in registration order; Reset clears both the join slice and values. - query/linq/join.go: new joinLeftOn/joinInnerOn kinds; LeftJoinOn / InnerJoinOn methods append to the entry slice; Apply dispatches to Join.JOINOn. The legacy LeftJoin/InnerJoin are marked // Deprecated: in godoc. - query/persistent.go: PersistentHelper interface gains LeftJoinOn / InnerJoinOn alongside the legacy methods (which now carry Deprecated: notices). Implementations forward to the linq builder. - sqlstmt/{first,list,count,delete}.go: bound JOIN values come before WHERE values in the final argument list. Extracted mergeArgs (sqlstmt/args.go) keeps the merging in one place and preserves nil semantics so existing callers see no behavioural drift. Tests: - sqlstmt/sqlpart/join_test.go rewritten for the new API: callback, bound, mixed, empty-callback skip, reset. - tests/integration/persistent_query_test.go gains TestPersistent_LeftJoinOn_BindsArgs (LEFT JOIN restricted by a bound user_id; only that user keeps post_count=3, others go to 0) and TestPersistent_InnerJoinOn_FiltersByBoundArg (single user survives the filter). Both run on all three adapters. Docs: docs/features/persistent-queries.md gains a "Bound JOIN parameters" section and demotes the callback variant to a deprecated "Legacy callback JOIN" with the existing SQL-injection warning. --- docs/features/persistent-queries.md | 29 ++++- query/linq/join.go | 76 ++++++++++--- query/persistent.go | 37 +++++- sqlstmt/args.go | 21 ++++ sqlstmt/count.go | 2 +- sqlstmt/delete.go | 2 +- sqlstmt/first.go | 2 +- sqlstmt/list.go | 2 +- sqlstmt/sqlpart/join.go | 61 +++++++++- sqlstmt/sqlpart/join_test.go | 126 +++++++++------------ tests/integration/persistent_query_test.go | 99 ++++++++++++++++ 11 files changed, 351 insertions(+), 106 deletions(-) create mode 100644 sqlstmt/args.go diff --git a/docs/features/persistent-queries.md b/docs/features/persistent-queries.md index 3a843b0..a61ae31 100644 --- a/docs/features/persistent-queries.md +++ b/docs/features/persistent-queries.md @@ -47,9 +47,30 @@ Now `PostCount` is automatically included in the SELECT of every request against !!! note "InnerJoin vs LeftJoin" `InnerJoin` drops users who have no posts — handy when you only care about active ones. `LeftJoin` keeps them, the aggregate returns `0` for loners. -## Context-aware JOIN +## Bound JOIN parameters — `LeftJoinOn` / `InnerJoinOn` -The function returns the JOIN text and takes a `context.Context`. That lets you mix in runtime values (tenant ID, UI locale) into the body: +When the ON-clause needs runtime values (tenant id, locale, …), use the +parameter-bound forms. They take the joined table reference, the ON clause +with `?` placeholders, and bound arguments — exactly like a WHERE. + +```go +h.LeftJoinOn( + "posts", + "posts.user_id = users.id AND posts.tenant_id = ?", + tenantID, +) +``` + +The arguments flow through the driver's parameter binding, so values cannot +turn into SQL — even if `tenantID` originated in user input. + +`InnerJoinOn` works the same way for inner joins. + +## Legacy callback JOIN (deprecated) + +The original `LeftJoin(fn)` / `InnerJoin(fn)` helpers take a callback that +returns the JOIN body. The callback receives a `context.Context`, but the +returned string is inlined verbatim — values are NOT parameterised: ```go h.LeftJoin(func(ctx context.Context) string { @@ -62,7 +83,9 @@ h.LeftJoin(func(ctx context.Context) string { ``` !!! danger "SQL injection" - Values coming from the context into the JOIN body do not flow through parameter binding. If you interpolate user-supplied data, escape it yourself — or, better, switch to a WHERE with a bound parameter. + Anything you splice into the returned string lands in the SQL as text. + The callback form remains for backwards compatibility but is **deprecated**; + new code should use `LeftJoinOn` / `InnerJoinOn`. ## Combining with per-request WHERE diff --git a/query/linq/join.go b/query/linq/join.go index 40449e5..646d4b8 100644 --- a/query/linq/join.go +++ b/query/linq/join.go @@ -20,11 +20,16 @@ type joinKind uint8 const ( joinLeft joinKind = iota joinInner + joinLeftOn + joinInnerOn ) type joinEntry struct { - kind joinKind - fn func(ctx context.Context) string + kind joinKind + fn func(ctx context.Context) string // legacy callback variants + table string // *On variants + on string // *On variants + args []any // *On variants } type JoinBuilder struct { @@ -38,29 +43,70 @@ func (q *JoinBuilder) Apply(applier JoinApplier) error { j := applier.Join() for i := range q.entries { e := &q.entries[i] - kind := e.kind - fn := e.fn - j.JOIN(func(ctx context.Context) string { - body := strings.TrimSpace(fn(ctx)) - if body == "" { - return "" - } - switch kind { - case joinLeft: + switch e.kind { + case joinLeft: + fn := e.fn + j.JOIN(func(ctx context.Context) string { + body := strings.TrimSpace(fn(ctx)) + if body == "" { + return "" + } return "LEFT JOIN " + body - case joinInner: + }) + case joinInner: + fn := e.fn + j.JOIN(func(ctx context.Context) string { + body := strings.TrimSpace(fn(ctx)) + if body == "" { + return "" + } return "INNER JOIN " + body - } - return "" - }) + }) + case joinLeftOn: + j.JOINOn("LEFT JOIN "+e.table+" ON "+e.on, e.args...) + case joinInnerOn: + j.JOINOn("INNER JOIN "+e.table+" ON "+e.on, e.args...) + } } return nil } +// LeftJoin registers a LEFT JOIN whose body is produced by a context-aware +// callback. Anything inside the returned string is inlined verbatim — values +// are NOT parameterised. +// +// Deprecated: prefer LeftJoinOn for safer, parameter-bound JOINs. func (q *JoinBuilder) LeftJoin(leftJoinFn func(ctx context.Context) string) { q.entries = append(q.entries, joinEntry{kind: joinLeft, fn: leftJoinFn}) } +// InnerJoin registers an INNER JOIN whose body is produced by a context-aware +// callback. Same caveat as LeftJoin: no parameter binding. +// +// Deprecated: prefer InnerJoinOn for safer, parameter-bound JOINs. func (q *JoinBuilder) InnerJoin(innerJoinFn func(ctx context.Context) string) { q.entries = append(q.entries, joinEntry{kind: joinInner, fn: innerJoinFn}) } + +// LeftJoinOn registers a LEFT JOIN with a fixed text and bound arguments. +// table is the joined table reference (`posts`, `posts AS p`, …); on is the +// raw ON-clause body where `?` placeholders refer to args in declaration order. +// Bound arguments flow through the driver and avoid SQL injection. +func (q *JoinBuilder) LeftJoinOn(table, on string, args ...any) { + q.entries = append(q.entries, joinEntry{ + kind: joinLeftOn, + table: table, + on: on, + args: args, + }) +} + +// InnerJoinOn is the parameter-bound counterpart of InnerJoin. +func (q *JoinBuilder) InnerJoinOn(table, on string, args ...any) { + q.entries = append(q.entries, joinEntry{ + kind: joinInnerOn, + table: table, + on: on, + args: args, + }) +} diff --git a/query/persistent.go b/query/persistent.go index 5c2295e..989855d 100644 --- a/query/persistent.go +++ b/query/persistent.go @@ -18,11 +18,28 @@ type PersistentHelper[TModel any] interface { // GroupBy groups the query results by the specified fields, accepting variadic pointers to fields for grouping operations. GroupBy(fieldsPtr ...any) PersistentHelper[TModel] - // LeftJoin adds a LEFT JOIN clause to the query using a provided function that returns the SQL join statement. + // LeftJoin adds a LEFT JOIN whose body is produced by a context-aware + // callback. Values inside the returned string are inlined verbatim — they + // do NOT pass through the driver's parameter binding. + // + // Deprecated: prefer LeftJoinOn for parameter-bound JOINs. LeftJoin(func(ctx context.Context) string) PersistentHelper[TModel] - // InnerJoin adds a INNER JOIN clause to the query using a provided function that returns the SQL join statement. + // InnerJoin adds an INNER JOIN whose body is produced by a context-aware + // callback. Values inside the returned string are inlined verbatim — they + // do NOT pass through the driver's parameter binding. + // + // Deprecated: prefer InnerJoinOn for parameter-bound JOINs. InnerJoin(fn func(ctx context.Context) string) PersistentHelper[TModel] + + // LeftJoinOn adds a LEFT JOIN with a fixed table reference and an ON + // clause containing `?` placeholders. Arguments are bound through the + // driver, exactly like WHERE parameters, eliminating the SQL injection + // risk of LeftJoin. + LeftJoinOn(table, on string, args ...any) PersistentHelper[TModel] + + // InnerJoinOn is the parameter-bound counterpart of InnerJoin. + InnerJoinOn(table, on string, args ...any) PersistentHelper[TModel] } type Persistent[TModel any] struct { @@ -38,13 +55,25 @@ func (h *Persistent[TModel]) Where() types.WhereTarget { return h.whereBuilder } +// Deprecated: prefer LeftJoinOn for parameter-bound JOINs. func (h *Persistent[TModel]) LeftJoin(fn func(ctx context.Context) string) PersistentHelper[TModel] { - h.joinBuilder.LeftJoin(fn) + h.joinBuilder.LeftJoin(fn) //nolint:staticcheck // explicit pass-through for the deprecated API return h } +// Deprecated: prefer InnerJoinOn for parameter-bound JOINs. func (h *Persistent[TModel]) InnerJoin(fn func(ctx context.Context) string) PersistentHelper[TModel] { - h.joinBuilder.InnerJoin(fn) + h.joinBuilder.InnerJoin(fn) //nolint:staticcheck // explicit pass-through for the deprecated API + return h +} + +func (h *Persistent[TModel]) LeftJoinOn(table, on string, args ...any) PersistentHelper[TModel] { + h.joinBuilder.LeftJoinOn(table, on, args...) + return h +} + +func (h *Persistent[TModel]) InnerJoinOn(table, on string, args ...any) PersistentHelper[TModel] { + h.joinBuilder.InnerJoinOn(table, on, args...) return h } diff --git a/sqlstmt/args.go b/sqlstmt/args.go new file mode 100644 index 0000000..ba65227 --- /dev/null +++ b/sqlstmt/args.go @@ -0,0 +1,21 @@ +package sqlstmt + +// mergeArgs concatenates positional argument slices in the order they appear +// in the generated SQL. JOIN arguments are emitted before the WHERE clause, +// so any bound JOIN values must precede WHERE values in the final []any. +// +// The function preserves nil semantics: if both inputs are empty the return +// value is nil so that callers can compare against the previous behavior +// without spurious empty-slice results. +func mergeArgs(left, right []any) []any { + if len(left) == 0 { + return right + } + if len(right) == 0 { + return left + } + out := make([]any, 0, len(left)+len(right)) + out = append(out, left...) + out = append(out, right...) + return out +} diff --git a/sqlstmt/count.go b/sqlstmt/count.go index 1c30388..f13eed7 100644 --- a/sqlstmt/count.go +++ b/sqlstmt/count.go @@ -46,5 +46,5 @@ func (c *Count) SQL(_ ...Option) (string, []any, error) { sb.WriteString(c.where.SQL()) sb.WriteString(c.group.SQL()) sb.WriteString(" LIMIT 1") - return sb.String(), c.where.Values(), nil + return sb.String(), mergeArgs(c.join.Values(), c.where.Values()), nil } diff --git a/sqlstmt/delete.go b/sqlstmt/delete.go index 29efe75..c9f9ce6 100644 --- a/sqlstmt/delete.go +++ b/sqlstmt/delete.go @@ -50,5 +50,5 @@ func (d *Delete) SQL(_ ...Option) (string, []any, error) { sb.WriteString(d.table) sb.WriteString(d.join.SQL()) sb.WriteString(d.where.SQL()) - return sb.String(), d.where.Values(), nil + return sb.String(), mergeArgs(d.join.Values(), d.where.Values()), nil } diff --git a/sqlstmt/first.go b/sqlstmt/first.go index e1b77d2..189fb7a 100644 --- a/sqlstmt/first.go +++ b/sqlstmt/first.go @@ -69,5 +69,5 @@ func (f *GetFirst) SQL(_ ...Option) (string, []any, error) { sb.WriteString(f.group.SQL()) sb.WriteString(f.order.SQL()) sb.WriteString(" LIMIT 1") - return sb.String(), f.where.Values(), nil + return sb.String(), mergeArgs(f.join.Values(), f.where.Values()), nil } diff --git a/sqlstmt/list.go b/sqlstmt/list.go index 9625e15..c36c6a1 100644 --- a/sqlstmt/list.go +++ b/sqlstmt/list.go @@ -79,5 +79,5 @@ func (f *GetList) SQL(_ ...Option) (string, []any, error) { sb.WriteString(f.group.SQL()) sb.WriteString(f.order.SQL()) sb.WriteString(f.limitOffset.SQL()) - return sb.String(), f.where.Values(), nil + return sb.String(), mergeArgs(f.join.Values(), f.where.Values()), nil } diff --git a/sqlstmt/sqlpart/join.go b/sqlstmt/sqlpart/join.go index eed8977..941e729 100644 --- a/sqlstmt/sqlpart/join.go +++ b/sqlstmt/sqlpart/join.go @@ -5,13 +5,33 @@ import ( "strings" ) +// Join is the interface gerpo's query layer talks to when adding JOIN clauses +// to a SELECT/UPDATE/DELETE statement. type Join interface { + // JOIN registers a callback whose returned text is inlined verbatim into + // the SQL. The callback receives the request context. Values are NOT + // parameterised — anything interpolated through the body lands in the SQL + // as text. Prefer JOINOn when bound parameters are needed. JOIN(joinFn func(ctx context.Context) string) + + // JOINOn registers a JOIN whose text is fixed and whose parameters are + // bound through the driver's placeholder mechanism, identical to WHERE + // arguments. The provided sql must already start with the JOIN keyword + // (e.g. "LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?"). + JOINOn(sql string, args ...any) +} + +type joinPart struct { + fn func(ctx context.Context) string + sql string + args []any + bound bool } type JoinBuilder struct { - ctx context.Context - joins []func(ctx context.Context) string + ctx context.Context + joins []joinPart + values []any } func NewJoinBuilder(ctx context.Context) *JoinBuilder { return &JoinBuilder{ctx: ctx} } @@ -20,19 +40,48 @@ func NewJoinBuilder(ctx context.Context) *JoinBuilder { return &JoinBuilder{ctx: func (b *JoinBuilder) Reset(ctx context.Context) { b.ctx = ctx for i := range b.joins { - b.joins[i] = nil + b.joins[i] = joinPart{} } b.joins = b.joins[:0] + for i := range b.values { + b.values[i] = nil + } + b.values = b.values[:0] } func (b *JoinBuilder) JOIN(joinFn func(ctx context.Context) string) { - b.joins = append(b.joins, joinFn) + b.joins = append(b.joins, joinPart{fn: joinFn}) +} + +func (b *JoinBuilder) JOINOn(sql string, args ...any) { + b.joins = append(b.joins, joinPart{sql: sql, args: args, bound: true}) + if len(args) > 0 { + b.values = append(b.values, args...) + } +} + +// Values returns the accumulated bound JOIN arguments in registration order. +// Callers must prepend these to WHERE values when building the final argument +// list, because JOIN clauses appear before WHERE in the generated SQL. +func (b *JoinBuilder) Values() []any { + return b.values } func (b *JoinBuilder) SQL() string { var sb strings.Builder - for _, j := range b.joins { - sb.WriteString(" " + j(b.ctx)) + for i := range b.joins { + j := &b.joins[i] + var body string + if j.bound { + body = j.sql + } else if j.fn != nil { + body = j.fn(b.ctx) + } + if body == "" { + continue + } + sb.WriteByte(' ') + sb.WriteString(body) } return sb.String() } diff --git a/sqlstmt/sqlpart/join_test.go b/sqlstmt/sqlpart/join_test.go index b30aca4..6aa3d60 100644 --- a/sqlstmt/sqlpart/join_test.go +++ b/sqlstmt/sqlpart/join_test.go @@ -7,84 +7,62 @@ import ( "github.com/stretchr/testify/assert" ) -func TestStringJoinBuilder(t *testing.T) { - testCases := []struct { - name string - initialJoins []func(ctx context.Context) string - newJoin func(ctx context.Context) string - }{ - { - name: "Add single join", - initialJoins: []func(ctx context.Context) string{}, - newJoin: func(ctx context.Context) string { - return "INNER JOIN table1 ON table1.id = table2.table1_id" - }, - }, - { - name: "Add multiple joins", - initialJoins: []func(ctx context.Context) string{ - func(ctx context.Context) string { - return "INNER JOIN table1 ON table1.id = table2.table1_id" - }, - }, - newJoin: func(ctx context.Context) string { - return "LEFT JOIN table3 ON table3.id = table2.table3_id" - }, - }, - } +func TestJoinBuilder_Callback(t *testing.T) { + b := NewJoinBuilder(context.Background()) + b.JOIN(func(ctx context.Context) string { + return "INNER JOIN table1 ON table1.id = table2.table1_id" + }) + b.JOIN(func(ctx context.Context) string { + return "LEFT JOIN table3 ON table3.id = table2.table3_id" + }) - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - builder := &JoinBuilder{ - joins: tc.initialJoins, - } + assert.Equal(t, + " INNER JOIN table1 ON table1.id = table2.table1_id LEFT JOIN table3 ON table3.id = table2.table3_id", + b.SQL()) + assert.Empty(t, b.Values(), "callback joins must not contribute bound values") +} + +func TestJoinBuilder_Bound(t *testing.T) { + b := NewJoinBuilder(context.Background()) + b.JOINOn("LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?", "tenant-1") + b.JOINOn("INNER JOIN tags ON tags.post_id = posts.id AND tags.kind IN (?, ?)", "blog", "draft") - builder.JOIN(tc.newJoin) - }) - } + assert.Equal(t, + " LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?"+ + " INNER JOIN tags ON tags.post_id = posts.id AND tags.kind IN (?, ?)", + b.SQL()) + assert.Equal(t, []any{"tenant-1", "blog", "draft"}, b.Values()) } -func TestStringJoinBuilderSQL(t *testing.T) { - testCases := []struct { - name string - initialJoins []func(ctx context.Context) string - expectedSQL string - }{ - { - name: "No joins", - initialJoins: []func(ctx context.Context) string{}, - expectedSQL: "", - }, - { - name: "Single join", - initialJoins: []func(ctx context.Context) string{ - func(ctx context.Context) string { - return "INNER JOIN table1 ON table1.id = table2.table1_id" - }, - }, - expectedSQL: " INNER JOIN table1 ON table1.id = table2.table1_id", - }, - { - name: "Multiple joins", - initialJoins: []func(ctx context.Context) string{ - func(ctx context.Context) string { - return "INNER JOIN table1 ON table1.id = table2.table1_id" - }, - func(ctx context.Context) string { - return "LEFT JOIN table3 ON table3.id = table2.table3_id" - }, - }, - expectedSQL: " INNER JOIN table1 ON table1.id = table2.table1_id LEFT JOIN table3 ON table3.id = table2.table3_id", - }, - } +func TestJoinBuilder_Mixed(t *testing.T) { + b := NewJoinBuilder(context.Background()) + b.JOIN(func(ctx context.Context) string { + return "INNER JOIN sessions ON sessions.user_id = users.id" + }) + b.JOINOn("LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?", "tenant-7") + + assert.Equal(t, + " INNER JOIN sessions ON sessions.user_id = users.id"+ + " LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?", + b.SQL()) + assert.Equal(t, []any{"tenant-7"}, b.Values()) +} + +func TestJoinBuilder_EmptyCallback_Skipped(t *testing.T) { + b := NewJoinBuilder(context.Background()) + b.JOIN(func(ctx context.Context) string { return "" }) + b.JOIN(func(ctx context.Context) string { return "INNER JOIN tags ON tags.id = posts.tag_id" }) + + assert.Equal(t, " INNER JOIN tags ON tags.id = posts.tag_id", b.SQL()) +} - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - builder := &JoinBuilder{ - joins: tc.initialJoins, - } +func TestJoinBuilder_Reset(t *testing.T) { + b := NewJoinBuilder(context.Background()) + b.JOINOn("LEFT JOIN posts ON posts.user_id = users.id AND posts.tenant_id = ?", "tenant-X") + assert.NotEmpty(t, b.SQL()) + assert.Len(t, b.Values(), 1) - assert.Equal(t, tc.expectedSQL, builder.SQL()) - }) - } + b.Reset(context.Background()) + assert.Empty(t, b.SQL()) + assert.Empty(t, b.Values()) } diff --git a/tests/integration/persistent_query_test.go b/tests/integration/persistent_query_test.go index ef6922d..c9ea42a 100644 --- a/tests/integration/persistent_query_test.go +++ b/tests/integration/persistent_query_test.go @@ -135,3 +135,102 @@ func TestPersistent_InnerJoin(t *testing.T) { } }) } + +// TestPersistent_LeftJoinOn_BindsArgs — the bound JOIN form sends ON-clause +// values through the driver. The repo joins posts only for a specific user_id; +// post_count then reflects only that user's posts. +func TestPersistent_LeftJoinOn_BindsArgs(t *testing.T) { + forEachAdapter(t, func(t *testing.T, ab adapterBundle) { + seed := defaultSeed(t) + ctx, cancel := testCtx(t) + defer cancel() + + // JOIN restricts the relationship to user[3] only — every other row + // will get a NULL right-hand side and post_count = 0. + targetUserID := seed.users[3].ID + + repo, err := gerpo.NewBuilder[User](). + DB(ab.adapter). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + c.Field(&m.Email).AsColumn() + c.Field(&m.Age).AsColumn() + c.Field(&m.CreatedAt).AsColumn().WithUpdateProtection() + c.Field(&m.UpdatedAt).AsColumn().WithInsertProtection() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + c.Field(&m.PostCount).AsVirtual().WithSQL(func(ctx context.Context) string { + return "COALESCE(COUNT(posts.id), 0)" + }) + }). + WithQuery(func(m *User, h query.PersistentHelper[User]) { + h.LeftJoinOn( + "posts", + "posts.user_id = users.id AND posts.user_id = ?", + targetUserID, + ) + h.GroupBy(&m.ID, &m.Name, &m.Email, &m.Age, &m.CreatedAt, &m.UpdatedAt, &m.DeletedAt) + h.Where().Field(&m.DeletedAt).EQ(nil) + }). + Build() + require.NoError(t, err) + + got, err := repo.GetList(ctx) + require.NoError(t, err) + require.Len(t, got, len(seed.users)) + + var hits int + for _, u := range got { + if u.ID == targetUserID { + assert.Equal(t, 3, u.PostCount, "target user keeps its 3 seeded posts") + hits++ + continue + } + assert.Equal(t, 0, u.PostCount, "non-target user must have post_count=0 because JOIN ON filtered them out") + } + assert.Equal(t, 1, hits, "exactly one row matches targetUserID") + }) +} + +// TestPersistent_InnerJoinOn_FiltersByBoundArg — InnerJoinOn variant: only the +// users matching the bound condition appear in the result. +func TestPersistent_InnerJoinOn_FiltersByBoundArg(t *testing.T) { + forEachAdapter(t, func(t *testing.T, ab adapterBundle) { + seed := defaultSeed(t) + ctx, cancel := testCtx(t) + defer cancel() + + // Restrict the inner-join to a single user — only that user appears. + targetUserID := seed.users[2].ID + + repo, err := gerpo.NewBuilder[User](). + DB(ab.adapter). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + c.Field(&m.Email).AsColumn() + c.Field(&m.Age).AsColumn() + c.Field(&m.CreatedAt).AsColumn().WithUpdateProtection() + c.Field(&m.UpdatedAt).AsColumn().WithInsertProtection() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithQuery(func(m *User, h query.PersistentHelper[User]) { + h.InnerJoinOn( + "posts", + "posts.user_id = users.id AND posts.user_id = ?", + targetUserID, + ) + h.GroupBy(&m.ID, &m.Name, &m.Email, &m.Age, &m.CreatedAt, &m.UpdatedAt, &m.DeletedAt) + h.Where().Field(&m.DeletedAt).EQ(nil) + }). + Build() + require.NoError(t, err) + + got, err := repo.GetList(ctx) + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, targetUserID, got[0].ID) + }) +} From 7f737bbd2b894c3c1edd23293cd1684b2dbdc42a Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 04:39:35 +0300 Subject: [PATCH 07/15] test: pin JOIN/WHERE/Count argument ordering after LeftJoinOn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bound JOIN arg ($1) and per-request WHERE args ($2..) live in two separate slices joined by mergeArgs in sqlstmt. If the order ever slipped, postgres would silently see e.g. an int landing where it expects a UUID, or vice versa. Adds TestPersistent_LeftJoinOn_ArgOrder_HoldsAcrossWhereAndCount that constructs a configuration where the ordering is verifiable by type: - JOIN bound arg: UUID (users[5].ID). - WHERE: Age >= 25 (int), then Age IN (25, 27, 29) (three ints). - Both shapes go through GetList and Count (different SQL forms but the same mergeArgs path). If JOIN and WHERE values swap, postgres would reject the query with a type mismatch (`invalid input syntax for type uuid`) — so a green test proves $1..$N keep the documented order. Also asserts that the joined user keeps post_count=3 while non-joined users return 0, confirming the bound JOIN argument actually reaches the JOIN clause. Runs on all three adapters via forEachAdapter. --- tests/integration/persistent_query_test.go | 86 ++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/integration/persistent_query_test.go b/tests/integration/persistent_query_test.go index c9ea42a..561b3d8 100644 --- a/tests/integration/persistent_query_test.go +++ b/tests/integration/persistent_query_test.go @@ -193,6 +193,92 @@ func TestPersistent_LeftJoinOn_BindsArgs(t *testing.T) { }) } +// TestPersistent_LeftJoinOn_ArgOrder_HoldsAcrossWhereAndCount проверяет, что +// добавление bound-аргумента в JOIN не ломает дальнейшие per-request фильтры, +// IN-список, ORDER и Count, и что аргументы попадают в правильные позиции. +// +// JOIN bound arg: UUID (users[3].ID). +// WHERE: Age GTE int(25), затем Age IN (int, int, int). +// Если порядок аргументов перепутается, драйвер сразу упадёт на типе +// (UUID не приведётся к int и наоборот). +func TestPersistent_LeftJoinOn_ArgOrder_HoldsAcrossWhereAndCount(t *testing.T) { + forEachAdapter(t, func(t *testing.T, ab adapterBundle) { + seed := defaultSeed(t) + ctx, cancel := testCtx(t) + defer cancel() + + // Восстановим один user в "joined" наборе и одного — нет. + joinedUserID := seed.users[5].ID // age = 25, попадает и в JOIN, и в WHERE Age GTE 25. + + repo, err := gerpo.NewBuilder[User](). + DB(ab.adapter). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + c.Field(&m.Email).AsColumn() + c.Field(&m.Age).AsColumn() + c.Field(&m.CreatedAt).AsColumn().WithUpdateProtection() + c.Field(&m.UpdatedAt).AsColumn().WithInsertProtection() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + c.Field(&m.PostCount).AsVirtual().WithSQL(func(ctx context.Context) string { + return "COALESCE(COUNT(posts.id), 0)" + }) + }). + WithQuery(func(m *User, h query.PersistentHelper[User]) { + h.LeftJoinOn( + "posts", + "posts.user_id = users.id AND posts.user_id = ?", + joinedUserID, + ) + h.GroupBy(&m.ID, &m.Name, &m.Email, &m.Age, &m.CreatedAt, &m.UpdatedAt, &m.DeletedAt) + h.Where().Field(&m.DeletedAt).EQ(nil) + }). + Build() + require.NoError(t, err) + + // Список с per-request WHERE по int + ORDER по int. WHERE age >= 25 → users 5..9. + // Из них только user[5] совпадает с joinedUserID, и должен иметь PostCount=3. + // Если bound JOIN arg ($1) и WHERE arg ($2) перепутаются местами, PG прочитает + // `posts.user_id = 25` (int не валиден как UUID) — кейс упадёт. + got, err := repo.GetList(ctx, func(m *User, h query.GetListHelper[User]) { + h.Where().Field(&m.Age).GTE(25) + h.OrderBy().Field(&m.Age).ASC() + }) + require.NoError(t, err) + require.Len(t, got, 5) + // users[5..9] в порядке возрастания age. + for i, u := range got { + expected := seed.users[5+i] + assert.Equal(t, expected.ID, u.ID) + if u.ID == joinedUserID { + assert.Equal(t, 3, u.PostCount, "joined user keeps its post_count") + } else { + assert.Equal(t, 0, u.PostCount, "non-joined users return 0") + } + } + + // Тот же репо: WHERE с IN-списком (3 значения) — четыре аргумента всего: + // $1 = JOIN UUID, $2..$4 = три int-возраста. + gotIN, err := repo.GetList(ctx, func(m *User, h query.GetListHelper[User]) { + h.Where().Field(&m.Age).IN(25, 27, 29) + h.OrderBy().Field(&m.Age).ASC() + }) + require.NoError(t, err) + require.Len(t, gotIN, 3) + assert.Equal(t, seed.users[5].ID, gotIN[0].ID) + assert.Equal(t, seed.users[7].ID, gotIN[1].ID) + assert.Equal(t, seed.users[9].ID, gotIN[2].ID) + + // Count с тем же per-request WHERE — другая SQL форма, но тот же mergeArgs path. + cnt, err := repo.Count(ctx, func(m *User, h query.CountHelper[User]) { + h.Where().Field(&m.Age).GTE(25) + }) + require.NoError(t, err) + assert.Equal(t, uint64(5), cnt) + }) +} + // TestPersistent_InnerJoinOn_FiltersByBoundArg — InnerJoinOn variant: only the // users matching the bound condition appear in the result. func TestPersistent_InnerJoinOn_FiltersByBoundArg(t *testing.T) { From b95f4f1b07e2d485d4c358ffd3e653b2c4dd7601 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 04:46:00 +0300 Subject: [PATCH 08/15] refactor: extract shared placeholder-rewriting adapter base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pgx5/pgx4/databasesql wrappers were ~80% copy-paste: each re-implemented placeholder rewrite, transaction state and the RollbackUnlessCommitted safety net. Three almost-identical bugs (value receivers + missing committed flag clears) had to be fixed in parallel last sprint. Adds executor/adapters/internal — an unexported package containing: - Backend / TxBackend interfaces describing what each driver must provide (Exec/Query/BeginTx; Commit/Rollback for the tx half), - Adapter — the executor.types.DBAdapter implementation that owns placeholder rewrite for ExecContext/QueryContext, - transaction — the wrapper that holds committed / rollbackUnlessCommittedNeeded and exposes the public Tx interface. Each driver package shrinks to a thin Backend + TxBackend pair plus its driver-specific Result/Rows wrappers (which are absent from databasesql because *sql.Rows / sql.Result already match the executor.types interfaces). databasesql/options.go now configures an internal adapterConfig struct, so Option no longer leaks the old dbWrap type into the public API. Question stays the default placeholder format. Tests: - executor/adapters/internal/base_test.go covers placeholder rewrite for both formats, every transaction-lifecycle path (Commit, RollbackUnlessCommitted-after-Commit, RollbackUnlessCommitted- without-Commit, explicit Rollback clearing the safety net, Commit-error keeping the safety net armed) and BeginTx error propagation. Driven by a fakeBackend / fakeTx. - the previous databasesql/tx_test.go is removed — its scenarios are now exercised against the shared transaction state machine and cover all three drivers at once. Integration tests (forEachAdapter) and the lint job stay green. Docs: docs/architecture/adapters-internals.md updated to describe the new shared base, the Backend/TxBackend contracts and the steps for writing a new driver. --- docs/architecture/adapters-internals.md | 95 +++++++---- executor/adapters/databasesql/db.go | 73 ++++---- executor/adapters/databasesql/options.go | 16 +- executor/adapters/databasesql/tx.go | 57 ------- executor/adapters/databasesql/tx_test.go | 77 --------- executor/adapters/internal/base.go | 122 ++++++++++++++ executor/adapters/internal/base_test.go | 204 +++++++++++++++++++++++ executor/adapters/pgx4/pool.go | 61 ++++--- executor/adapters/pgx4/tx.go | 64 ------- executor/adapters/pgx5/pool.go | 60 ++++--- executor/adapters/pgx5/tx.go | 64 ------- 11 files changed, 513 insertions(+), 380 deletions(-) delete mode 100644 executor/adapters/databasesql/tx.go delete mode 100644 executor/adapters/databasesql/tx_test.go create mode 100644 executor/adapters/internal/base.go create mode 100644 executor/adapters/internal/base_test.go delete mode 100644 executor/adapters/pgx4/tx.go delete mode 100644 executor/adapters/pgx5/tx.go diff --git a/docs/architecture/adapters-internals.md b/docs/architecture/adapters-internals.md index 9263db4..3152a0b 100644 --- a/docs/architecture/adapters-internals.md +++ b/docs/architecture/adapters-internals.md @@ -1,70 +1,93 @@ # Adapter internals -An adapter is a thin wrapper that turns `executor.DBAdapter` calls into driver-specific calls. All bundled adapters follow the same layout; differences come down to placeholder rewriting and transaction semantics. +An adapter turns `executor.DBAdapter` calls into driver-specific calls. The placeholder rewrite, the transaction state machine and the `RollbackUnlessCommitted` semantics live once in the unexported `executor/adapters/internal` package; every bundled adapter (pgx v5, pgx v4, database/sql) only contributes a tiny `Backend` plus result/rows wrappers. -## Anatomy of a wrapper +## Anatomy of a driver package ``` executor/adapters// - pool.go — NewPoolAdapter / NewAdapter + ExecContext / QueryContext / BeginTx - tx.go — txWrap implementing executor.Tx - rows.go — rowsWrap adapting driver rows to types.Rows - result.go — resultWrap adapting driver result to types.Result + pool.go — Backend / TxBackend implementations + the public NewPoolAdapter / NewAdapter + rows.go — rowsWrap adapting driver rows to types.Rows (only when the driver's Rows + type doesn't already satisfy the interface) + result.go — resultWrap adapting driver result to types.Result (same caveat) ``` -The rest is boilerplate. +`databasesql` is the smallest of the three: `*sql.Rows` and `sql.Result` already satisfy `types.Rows` / `types.Result`, so no wrapper types are needed. pgx returns its own `pgx.Rows` / `pgconn.CommandTag`, which require thin wrappers. -## Placeholder rewriting +## The shared base — `internal.Adapter` + +`internal.New(backend Backend, p placeholder.PlaceholderFormat) extypes.DBAdapter` returns the public adapter. It owns: + +- placeholder rewrite for every `ExecContext` / `QueryContext`; +- creation of a `transaction` wrapping the backend's `TxBackend`; +- the transaction state machine (`committed`, `rollbackUnlessCommittedNeeded`). -gerpo's SQL uses `?`. Each adapter rewrites placeholders exactly once, right before handing the query to the driver: +Drivers never reimplement that logic. + +## The two backend interfaces ```go -sql, err := placeholder.Dollar.ReplacePlaceholders(query) +type Backend interface { + Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) + Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) + BeginTx(ctx context.Context) (TxBackend, error) +} + +type TxBackend interface { + Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) + Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) + Commit() error + Rollback() error +} ``` -`executor/adapters/placeholder/` provides two formats: +A driver implements both with a few lines of delegation. `Commit` / `Rollback` are context-less because pgx insists on its own background context for these calls. -- `placeholder.Question` — no-op (input already `?`). -- `placeholder.Dollar` — scan-and-emit rewriter that turns `?` into `$1, $2, …`. - -`databasesql.NewAdapter` defaults to `Question`. `pgx4` / `pgx5` always use `Dollar`. +## Placeholder rewriting -## Rows wrapper +gerpo emits `?` placeholders. The shared adapter rewrites them exactly once before delegating to the backend: -pgx returns `pgx.Rows`, `database/sql` returns `*sql.Rows`. Both shapes are close enough to the `types.Rows` interface, but they differ in `Scan` behavior (nullable types, text decoding). `rowsWrap` exists so gerpo can pretend both are identical. +```go +sql, err := a.placeholder.ReplacePlaceholders(query) +``` -## Result wrapper +`executor/adapters/placeholder/` provides two formats: -`types.Result` exposes only `RowsAffected() (int64, error)`. Both pgx and `database/sql` return something richer, but gerpo only needs this one metric. +- `placeholder.Question` — no-op (input stays as `?`). +- `placeholder.Dollar` — scan-and-emit rewriter that turns `?` into `$1, $2, …`. -## Transaction wrapper +`databasesql.NewAdapter` defaults to `Question`; pass `WithPlaceholder(placeholder.Dollar)` for PostgreSQL. `pgx4` / `pgx5` always pin `Dollar`. -`txWrap` stores: +## Transaction state machine ```go -type txWrap struct { - committed bool +type transaction struct { + inner TxBackend + placeholder placeholder.PlaceholderFormat + committed bool rollbackUnlessCommittedNeeded bool - tx .Tx // or *sql.Tx } ``` -- `Commit()` — calls driver commit, then sets `committed = true` on **success**. -- `Rollback()` — sets `rollbackUnlessCommittedNeeded = false`, then calls driver rollback. +- `Commit()` — calls `inner.Commit()`, then sets `committed = true` only on success. +- `Rollback()` — clears `rollbackUnlessCommittedNeeded`, then calls `inner.Rollback()`. - `RollbackUnlessCommitted()` — if `!committed && rollbackUnlessCommittedNeeded`, delegates to `Rollback()`; otherwise no-op. Designed to be safe as a `defer`. -All three methods use pointer receivers so the state mutations actually stick. +All three are pointer-receiver methods on the shared type, so state mutations actually persist (pgx wrappers historically used value receivers and lost the flag — fixed in `chore: fix "commited" typo in tx wrappers`). + +## Rows / Result wrappers + +`types.Rows` requires `Next()`, `Scan(dest ...any) error`, `Close() error`. `*sql.Rows` already matches this shape; pgx returns its own type with `Close()` returning nothing, so `rowsWrap` adapts it. -!!! warning "Historical bug" - pgx v4 and v5 adapters originally used value receivers and also forgot to set `committed`. `RollbackUnlessCommitted()` after `Commit()` returned `tx is closed`. The integration test `TestTx_RollbackUnlessCommitted_AfterCommit` catches this; fixed in the `test: cover hooks, soft delete, …` commit. +`types.Result` requires only `RowsAffected() (int64, error)`. `sql.Result` matches; pgx returns `pgconn.CommandTag` whose `RowsAffected()` returns just `int64`, so `resultWrap` adds the trailing `nil` error. -## Writing your own +## Writing your own driver -Walk through `executor/adapters/pgx5/` as a template. You will need: +1. Implement `internal.Backend` (three methods) and `internal.TxBackend` (four methods) for your driver. +2. Pick a placeholder format. Most non-PostgreSQL drivers keep `?` (`placeholder.Question`). +3. Wrap your driver's `Rows`/`Result` types only if their methods don't already satisfy the interfaces in `executor/types`. +4. Return `internal.New(yourBackend, yourPlaceholder)` from the public constructor. -- decide whether to rewrite placeholders (most non-PG drivers keep `?`; PG-derived drivers want `$N`); -- wrap the driver's `Rows` type in something satisfying `types.Rows`; -- wrap the driver's transaction type in `txWrap` following the rules above; -- return a `types.DBAdapter` implementation. +A good smoke test is `TestSmoke` in `tests/integration/` — `forEachAdapter` will pick up your new bundle as soon as you add it to `allAdapters()`. -A good smoke test is `TestSmoke` in `tests/integration/` — `forEachAdapter` will pick up your new bundle once you add it to `allAdapters()`. +For unit-level coverage of the shared logic see `executor/adapters/internal/base_test.go` — it drives the adapter with a fake backend and exercises every transaction-lifecycle path. diff --git a/executor/adapters/databasesql/db.go b/executor/adapters/databasesql/db.go index 49cb44d..b37e3ab 100644 --- a/executor/adapters/databasesql/db.go +++ b/executor/adapters/databasesql/db.go @@ -3,52 +3,63 @@ package databasesql import ( "context" "database/sql" - "fmt" + "github.com/insei/gerpo/executor/adapters/internal" "github.com/insei/gerpo/executor/adapters/placeholder" - "github.com/insei/gerpo/executor/types" + extypes "github.com/insei/gerpo/executor/types" ) -type dbWrap struct { - db *sql.DB - placeholder placeholder.PlaceholderFormat +// dbBackend implements internal.Backend on top of a standard *sql.DB. +// *sql.Result and *sql.Rows already satisfy executor/types.Result and +// executor/types.Rows respectively, so no extra wrapper types are needed. +type dbBackend struct { + db *sql.DB +} + +func (b *dbBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + return b.db.ExecContext(ctx, sql, args...) +} + +func (b *dbBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + return b.db.QueryContext(ctx, sql, args...) } -func (w *dbWrap) BeginTx(ctx context.Context) (types.Tx, error) { - tx, err := w.db.BeginTx(ctx, nil) +func (b *dbBackend) BeginTx(ctx context.Context) (internal.TxBackend, error) { + tx, err := b.db.BeginTx(ctx, nil) if err != nil { return nil, err } - return &txWrap{ - tx: tx, - rollbackUnlessCommittedNeeded: true, - placeholder: w.placeholder, - }, nil + return &txBackend{tx: tx}, nil } -func (w *dbWrap) ExecContext(ctx context.Context, query string, args ...any) (types.Result, error) { - sql, err := w.placeholder.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - return w.db.ExecContext(ctx, sql, args...) +// txBackend implements internal.TxBackend on top of *sql.Tx. +type txBackend struct { + tx *sql.Tx } -func (w *dbWrap) QueryContext(ctx context.Context, query string, args ...any) (types.Rows, error) { - sql, err := w.placeholder.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - return w.db.QueryContext(ctx, sql, args...) +func (t *txBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + return t.tx.ExecContext(ctx, sql, args...) } -func NewAdapter(db *sql.DB, opts ...Option) types.DBAdapter { - wrappedDb := &dbWrap{ - db: db, - placeholder: placeholder.Question, - } +func (t *txBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + return t.tx.QueryContext(ctx, sql, args...) +} + +func (t *txBackend) Commit() error { return t.tx.Commit() } +func (t *txBackend) Rollback() error { return t.tx.Rollback() } + +// adapterConfig collects the optional knobs for NewAdapter. +type adapterConfig struct { + placeholder placeholder.PlaceholderFormat +} + +// NewAdapter wraps a database/sql DB with the gerpo DB adapter contract. +// The placeholder format defaults to `?` (MySQL); use WithPlaceholder to +// switch to `$1, $2, …` for PostgreSQL. +func NewAdapter(db *sql.DB, opts ...Option) extypes.DBAdapter { + cfg := adapterConfig{placeholder: placeholder.Question} for _, opt := range opts { - opt.apply(wrappedDb) + opt.apply(&cfg) } - return wrappedDb + return internal.New(&dbBackend{db: db}, cfg.placeholder) } diff --git a/executor/adapters/databasesql/options.go b/executor/adapters/databasesql/options.go index 99e3284..9a65fec 100644 --- a/executor/adapters/databasesql/options.go +++ b/executor/adapters/databasesql/options.go @@ -2,19 +2,19 @@ package databasesql import "github.com/insei/gerpo/executor/adapters/placeholder" +// Option tunes how NewAdapter wires the underlying *sql.DB. type Option interface { - apply(*dbWrap) + apply(*adapterConfig) } -type optionFn func(*dbWrap) +type optionFn func(*adapterConfig) -func (o optionFn) apply(db *dbWrap) { - o(db) -} +func (o optionFn) apply(cfg *adapterConfig) { o(cfg) } -// WithPlaceholder sets a custom placeholder format for the database. +// WithPlaceholder sets a custom placeholder format. The default is +// placeholder.Question (`?`); use placeholder.Dollar for PostgreSQL. func WithPlaceholder(format placeholder.PlaceholderFormat) Option { - return optionFn(func(db *dbWrap) { - db.placeholder = format + return optionFn(func(cfg *adapterConfig) { + cfg.placeholder = format }) } diff --git a/executor/adapters/databasesql/tx.go b/executor/adapters/databasesql/tx.go deleted file mode 100644 index 9662127..0000000 --- a/executor/adapters/databasesql/tx.go +++ /dev/null @@ -1,57 +0,0 @@ -package databasesql - -import ( - "context" - "database/sql" - "fmt" - - "github.com/insei/gerpo/executor/adapters/placeholder" - "github.com/insei/gerpo/executor/types" -) - -type txWrap struct { - committed bool - rollbackUnlessCommittedNeeded bool - tx *sql.Tx - placeholder placeholder.PlaceholderFormat -} - -func (t *txWrap) ExecContext(ctx context.Context, query string, args ...any) (types.Result, error) { - sql, err := t.placeholder.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - return t.tx.ExecContext(ctx, sql, args...) -} - -func (t *txWrap) QueryContext(ctx context.Context, query string, args ...any) (types.Rows, error) { - sql, err := t.placeholder.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - return t.tx.QueryContext(ctx, sql, args...) -} - -func (t *txWrap) Commit() error { - err := t.tx.Commit() - if err != nil { - return err - } - t.committed = true - return nil -} - -func (t *txWrap) Rollback() error { - t.rollbackUnlessCommittedNeeded = false - return t.tx.Rollback() -} - -func (t *txWrap) RollbackUnlessCommitted() error { - if !t.committed && t.rollbackUnlessCommittedNeeded { - err := t.Rollback() - if err != nil { - return err - } - } - return nil -} diff --git a/executor/adapters/databasesql/tx_test.go b/executor/adapters/databasesql/tx_test.go deleted file mode 100644 index 99673d3..0000000 --- a/executor/adapters/databasesql/tx_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package databasesql - -import ( - "testing" - - "github.com/DATA-DOG/go-sqlmock" - "github.com/insei/gerpo/executor/adapters/placeholder" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func newTxFixture(t *testing.T) (*txWrap, sqlmock.Sqlmock, func()) { - t.Helper() - db, mock, err := sqlmock.New(sqlmock.MonitorPingsOption(true)) - require.NoError(t, err) - mock.ExpectBegin() - tx, err := db.Begin() - require.NoError(t, err) - w := &txWrap{ - tx: tx, - placeholder: placeholder.Question, - rollbackUnlessCommittedNeeded: true, - } - cleanup := func() { - assert.NoError(t, mock.ExpectationsWereMet()) - _ = db.Close() - } - return w, mock, cleanup -} - -// TestTxWrap_Commit_SetsCommitted verifies Commit() flips the internal flag — -// guarding against the historical typo that broke the field name -// and made RollbackUnlessCommitted() try to roll back a committed tx. -func TestTxWrap_Commit_SetsCommitted(t *testing.T) { - w, mock, cleanup := newTxFixture(t) - defer cleanup() - - mock.ExpectCommit() - require.NoError(t, w.Commit()) - assert.True(t, w.committed, "Commit() must set committed=true") -} - -// TestTxWrap_RollbackUnlessCommitted_AfterCommit_IsNoop ensures the safety net -// does not call the driver after a successful Commit. -func TestTxWrap_RollbackUnlessCommitted_AfterCommit_IsNoop(t *testing.T) { - w, mock, cleanup := newTxFixture(t) - defer cleanup() - - mock.ExpectCommit() - require.NoError(t, w.Commit()) - // No mock.ExpectRollback() — if RollbackUnlessCommitted() called Rollback - // the sqlmock expectation set would fail in cleanup. - require.NoError(t, w.RollbackUnlessCommitted()) -} - -// TestTxWrap_RollbackUnlessCommitted_WithoutCommit_DoesRollback ensures the -// safety net rolls back when Commit was not called. -func TestTxWrap_RollbackUnlessCommitted_WithoutCommit_DoesRollback(t *testing.T) { - w, mock, cleanup := newTxFixture(t) - defer cleanup() - - mock.ExpectRollback() - require.NoError(t, w.RollbackUnlessCommitted()) - assert.False(t, w.rollbackUnlessCommittedNeeded, "Rollback() must clear the safety-net flag") -} - -// TestTxWrap_Rollback_ClearsSafetyNet ensures an explicit Rollback() prevents -// RollbackUnlessCommitted from trying to roll back a second time. -func TestTxWrap_Rollback_ClearsSafetyNet(t *testing.T) { - w, mock, cleanup := newTxFixture(t) - defer cleanup() - - mock.ExpectRollback() - require.NoError(t, w.Rollback()) - // Second call must be a no-op — no extra ExpectRollback configured. - require.NoError(t, w.RollbackUnlessCommitted()) -} diff --git a/executor/adapters/internal/base.go b/executor/adapters/internal/base.go new file mode 100644 index 0000000..c454ff7 --- /dev/null +++ b/executor/adapters/internal/base.go @@ -0,0 +1,122 @@ +// Package internal hosts the placeholder-rewriting plumbing shared by every +// bundled gerpo DB adapter (pgx v5, pgx v4, database/sql). Each driver only +// needs to provide a tiny Backend implementation; the plumbing — placeholder +// rewrite, transaction state machine, RollbackUnlessCommitted semantics — +// lives here so all adapters stay consistent. +// +// The package is internal: it is not part of the public API surface. +package internal + +import ( + "context" + "fmt" + + "github.com/insei/gerpo/executor/adapters/placeholder" + extypes "github.com/insei/gerpo/executor/types" +) + +// Backend describes the driver-specific behavior the generic Adapter wraps. +// Implementations are expected to convert their native Result/Rows types into +// the executor.types interfaces themselves — Backend works in already-rewritten +// SQL, the generic Adapter handles placeholder translation upstream. +type Backend interface { + Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) + Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) + BeginTx(ctx context.Context) (TxBackend, error) +} + +// TxBackend mirrors Backend minus BeginTx, plus Commit / Rollback. The wrapping +// transaction owns the committed / rollbackUnlessCommittedNeeded flags so +// drivers do not need to reimplement them. +type TxBackend interface { + Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) + Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) + Commit() error + Rollback() error +} + +// Adapter is the executor.types.DBAdapter implementation shared by every +// bundled driver. It rewrites placeholders before each driver call and wraps +// transactions in a state machine that makes RollbackUnlessCommitted safe to +// use as a defer. +type Adapter struct { + backend Backend + placeholder placeholder.PlaceholderFormat +} + +// New constructs an Adapter that runs every SQL statement through the given +// placeholder format before handing it over to the backend. +func New(backend Backend, p placeholder.PlaceholderFormat) extypes.DBAdapter { + return &Adapter{backend: backend, placeholder: p} +} + +func (a *Adapter) ExecContext(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + rewritten, err := a.placeholder.ReplacePlaceholders(sql) + if err != nil { + return nil, fmt.Errorf("failed to replace placeholders: %w", err) + } + return a.backend.Exec(ctx, rewritten, args...) +} + +func (a *Adapter) QueryContext(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rewritten, err := a.placeholder.ReplacePlaceholders(sql) + if err != nil { + return nil, fmt.Errorf("failed to replace placeholders: %w", err) + } + return a.backend.Query(ctx, rewritten, args...) +} + +func (a *Adapter) BeginTx(ctx context.Context) (extypes.Tx, error) { + inner, err := a.backend.BeginTx(ctx) + if err != nil { + return nil, err + } + return &transaction{ + inner: inner, + placeholder: a.placeholder, + rollbackUnlessCommittedNeeded: true, + }, nil +} + +type transaction struct { + inner TxBackend + placeholder placeholder.PlaceholderFormat + committed bool + rollbackUnlessCommittedNeeded bool +} + +func (t *transaction) ExecContext(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + rewritten, err := t.placeholder.ReplacePlaceholders(sql) + if err != nil { + return nil, fmt.Errorf("failed to replace placeholders: %w", err) + } + return t.inner.Exec(ctx, rewritten, args...) +} + +func (t *transaction) QueryContext(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rewritten, err := t.placeholder.ReplacePlaceholders(sql) + if err != nil { + return nil, fmt.Errorf("failed to replace placeholders: %w", err) + } + return t.inner.Query(ctx, rewritten, args...) +} + +func (t *transaction) Commit() error { + if err := t.inner.Commit(); err != nil { + return err + } + t.committed = true + return nil +} + +func (t *transaction) Rollback() error { + t.rollbackUnlessCommittedNeeded = false + return t.inner.Rollback() +} + +func (t *transaction) RollbackUnlessCommitted() error { + if !t.committed && t.rollbackUnlessCommittedNeeded { + return t.Rollback() + } + return nil +} diff --git a/executor/adapters/internal/base_test.go b/executor/adapters/internal/base_test.go new file mode 100644 index 0000000..2a64b56 --- /dev/null +++ b/executor/adapters/internal/base_test.go @@ -0,0 +1,204 @@ +package internal + +import ( + "context" + "errors" + "testing" + + "github.com/insei/gerpo/executor/adapters/placeholder" + extypes "github.com/insei/gerpo/executor/types" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeBackend records every call and lets tests choose what to return. +type fakeBackend struct { + execCalls []callRecord + queryCalls []callRecord + beginErr error + tx *fakeTx + execResult extypes.Result + queryRows extypes.Rows + execErr error + queryErr error +} + +type callRecord struct { + sql string + args []any +} + +func (b *fakeBackend) Exec(_ context.Context, sql string, args ...any) (extypes.Result, error) { + b.execCalls = append(b.execCalls, callRecord{sql: sql, args: args}) + return b.execResult, b.execErr +} + +func (b *fakeBackend) Query(_ context.Context, sql string, args ...any) (extypes.Rows, error) { + b.queryCalls = append(b.queryCalls, callRecord{sql: sql, args: args}) + return b.queryRows, b.queryErr +} + +func (b *fakeBackend) BeginTx(_ context.Context) (TxBackend, error) { + if b.beginErr != nil { + return nil, b.beginErr + } + if b.tx == nil { + b.tx = &fakeTx{} + } + return b.tx, nil +} + +// fakeTx records lifecycle calls and lets tests inject errors. +type fakeTx struct { + execCalls []callRecord + queryCalls []callRecord + commits int + rollbacks int + commitErr error + rollbackErr error +} + +func (t *fakeTx) Exec(_ context.Context, sql string, args ...any) (extypes.Result, error) { + t.execCalls = append(t.execCalls, callRecord{sql: sql, args: args}) + return nil, nil +} +func (t *fakeTx) Query(_ context.Context, sql string, args ...any) (extypes.Rows, error) { + t.queryCalls = append(t.queryCalls, callRecord{sql: sql, args: args}) + return nil, nil +} +func (t *fakeTx) Commit() error { + if t.commitErr != nil { + return t.commitErr + } + t.commits++ + return nil +} +func (t *fakeTx) Rollback() error { + if t.rollbackErr != nil { + return t.rollbackErr + } + t.rollbacks++ + return nil +} + +// TestAdapter_RewritesQuestionToDollar — placeholders are converted before +// reaching the backend. Drives the real placeholder.Dollar transformation. +func TestAdapter_RewritesQuestionToDollar(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Dollar) + + _, err := a.ExecContext(context.Background(), "INSERT INTO t(a, b) VALUES (?, ?)", "x", 1) + require.NoError(t, err) + require.Len(t, b.execCalls, 1) + assert.Equal(t, "INSERT INTO t(a, b) VALUES ($1, $2)", b.execCalls[0].sql) + assert.Equal(t, []any{"x", 1}, b.execCalls[0].args) + + _, err = a.QueryContext(context.Background(), "SELECT 1 FROM t WHERE a = ? AND b = ?", "y", 2) + require.NoError(t, err) + require.Len(t, b.queryCalls, 1) + assert.Equal(t, "SELECT 1 FROM t WHERE a = $1 AND b = $2", b.queryCalls[0].sql) +} + +// TestAdapter_QuestionPlaceholder_NoOp — Question format leaves SQL alone. +func TestAdapter_QuestionPlaceholder_NoOp(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Question) + + _, err := a.ExecContext(context.Background(), "INSERT INTO t(a) VALUES (?)", "x") + require.NoError(t, err) + assert.Equal(t, "INSERT INTO t(a) VALUES (?)", b.execCalls[0].sql) +} + +// TestTransaction_Commit_FlipsCommittedFlag — Commit sets the internal flag so +// subsequent RollbackUnlessCommitted is a no-op. +func TestTransaction_Commit_FlipsCommittedFlag(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Question) + + tx, err := a.BeginTx(context.Background()) + require.NoError(t, err) + + require.NoError(t, tx.Commit()) + assert.Equal(t, 1, b.tx.commits) + + require.NoError(t, tx.RollbackUnlessCommitted(), + "after Commit RollbackUnlessCommitted must be a no-op") + assert.Equal(t, 0, b.tx.rollbacks, "no rollback should reach the backend after a successful Commit") +} + +// TestTransaction_RollbackUnlessCommitted_WithoutCommit_RollsBack — happy path +// for the safety net. +func TestTransaction_RollbackUnlessCommitted_WithoutCommit_RollsBack(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Question) + + tx, err := a.BeginTx(context.Background()) + require.NoError(t, err) + + require.NoError(t, tx.RollbackUnlessCommitted()) + assert.Equal(t, 1, b.tx.rollbacks) + + // Second call must not roll back again — the safety net flag is cleared. + require.NoError(t, tx.RollbackUnlessCommitted()) + assert.Equal(t, 1, b.tx.rollbacks) +} + +// TestTransaction_ExplicitRollback_ClearsSafetyNet — Rollback by itself also +// blocks the deferred RollbackUnlessCommitted. +func TestTransaction_ExplicitRollback_ClearsSafetyNet(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Question) + + tx, err := a.BeginTx(context.Background()) + require.NoError(t, err) + + require.NoError(t, tx.Rollback()) + require.NoError(t, tx.RollbackUnlessCommitted()) + assert.Equal(t, 1, b.tx.rollbacks, "second rollback through the safety net must not reach the backend") +} + +// TestTransaction_CommitError_DoesNotMarkCommitted — if Commit fails the flag +// stays false so RollbackUnlessCommitted will still try to roll back. +func TestTransaction_CommitError_DoesNotMarkCommitted(t *testing.T) { + commitFail := errors.New("commit failed") + b := &fakeBackend{tx: &fakeTx{commitErr: commitFail}} + a := New(b, placeholder.Question) + + tx, err := a.BeginTx(context.Background()) + require.NoError(t, err) + + assert.ErrorIs(t, tx.Commit(), commitFail) + require.NoError(t, tx.RollbackUnlessCommitted()) + assert.Equal(t, 1, b.tx.rollbacks, "failed Commit must leave the safety net armed") +} + +// TestTransaction_ExecAndQuery_RewritePlaceholders — transactional +// Exec/Query also pass through the placeholder rewriter. +func TestTransaction_ExecAndQuery_RewritePlaceholders(t *testing.T) { + b := &fakeBackend{} + a := New(b, placeholder.Dollar) + + tx, err := a.BeginTx(context.Background()) + require.NoError(t, err) + + _, err = tx.ExecContext(context.Background(), "UPDATE t SET a = ? WHERE id = ?", "x", 1) + require.NoError(t, err) + assert.Equal(t, "UPDATE t SET a = $1 WHERE id = $2", b.tx.execCalls[0].sql) + + _, err = tx.QueryContext(context.Background(), "SELECT * FROM t WHERE id = ?", 1) + require.NoError(t, err) + assert.Equal(t, "SELECT * FROM t WHERE id = $1", b.tx.queryCalls[0].sql) +} + +// TestAdapter_BeginTxError_Propagates — backend BeginTx errors reach the +// caller as-is. +func TestAdapter_BeginTxError_Propagates(t *testing.T) { + beginFail := errors.New("begin failed") + b := &fakeBackend{beginErr: beginFail} + a := New(b, placeholder.Question) + + tx, err := a.BeginTx(context.Background()) + assert.Nil(t, tx) + assert.ErrorIs(t, err, beginFail) +} diff --git a/executor/adapters/pgx4/pool.go b/executor/adapters/pgx4/pool.go index c28e014..bf90fdc 100644 --- a/executor/adapters/pgx4/pool.go +++ b/executor/adapters/pgx4/pool.go @@ -2,53 +2,70 @@ package pgx4 import ( "context" - "fmt" - "github.com/insei/gerpo/executor/adapters/placeholder" - "github.com/insei/gerpo/executor/types" "github.com/jackc/pgx/v4" "github.com/jackc/pgx/v4/pgxpool" + + "github.com/insei/gerpo/executor/adapters/internal" + "github.com/insei/gerpo/executor/adapters/placeholder" + extypes "github.com/insei/gerpo/executor/types" ) -type poolWrap struct { +// poolBackend implements internal.Backend on top of a pgx v4 connection pool. +type poolBackend struct { pool *pgxpool.Pool } -func (p *poolWrap) ExecContext(ctx context.Context, query string, args ...any) (types.Result, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) +func (b *poolBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + res, err := b.pool.Exec(ctx, sql, args...) if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) + return nil, err } - res, err := p.pool.Exec(ctx, sql, args...) + return &resultWrap{res: res}, nil +} + +func (b *poolBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rows, err := b.pool.Query(ctx, sql, args...) if err != nil { return nil, err } - return &resultWrap{res: res}, nil + return &rowsWrap{rows: rows}, nil } -func (p *poolWrap) QueryContext(ctx context.Context, query string, args ...any) (types.Rows, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) +func (b *poolBackend) BeginTx(ctx context.Context) (internal.TxBackend, error) { + tx, err := b.pool.BeginTx(ctx, pgx.TxOptions{}) if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) + return nil, err } - rows, err := p.pool.Query(ctx, sql, args...) + return &txBackend{tx: tx}, nil +} + +// txBackend implements internal.TxBackend on top of pgx.Tx. +type txBackend struct { + tx pgx.Tx +} + +func (t *txBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + res, err := t.tx.Exec(ctx, sql, args...) if err != nil { return nil, err } - return &rowsWrap{rows: rows}, nil + return &resultWrap{res: res}, nil } -func (p *poolWrap) BeginTx(ctx context.Context) (types.Tx, error) { - tx, err := p.pool.BeginTx(ctx, pgx.TxOptions{}) +func (t *txBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rows, err := t.tx.Query(ctx, sql, args...) if err != nil { return nil, err } - return &txWrap{ - rollbackUnlessCommittedNeeded: true, - tx: tx, - }, err + return &rowsWrap{rows: rows}, nil } -func NewPoolAdapter(pool *pgxpool.Pool) types.DBAdapter { - return &poolWrap{pool} +func (t *txBackend) Commit() error { return t.tx.Commit(context.Background()) } +func (t *txBackend) Rollback() error { return t.tx.Rollback(context.Background()) } + +// NewPoolAdapter wraps a pgx v4 pool with the gerpo DB adapter contract. +// SQL placeholders are rewritten from `?` to PostgreSQL's `$1, $2, …` form. +func NewPoolAdapter(pool *pgxpool.Pool) extypes.DBAdapter { + return internal.New(&poolBackend{pool: pool}, placeholder.Dollar) } diff --git a/executor/adapters/pgx4/tx.go b/executor/adapters/pgx4/tx.go deleted file mode 100644 index 0de76f2..0000000 --- a/executor/adapters/pgx4/tx.go +++ /dev/null @@ -1,64 +0,0 @@ -package pgx4 - -import ( - "context" - "fmt" - - "github.com/insei/gerpo/executor/adapters/placeholder" - extypes "github.com/insei/gerpo/executor/types" - "github.com/jackc/pgx/v4" -) - -type txWrap struct { - committed bool - rollbackUnlessCommittedNeeded bool - tx pgx.Tx -} - -func (t *txWrap) Rollback() error { - t.rollbackUnlessCommittedNeeded = false - return t.tx.Rollback(context.Background()) -} - -func (t *txWrap) Commit() error { - err := t.tx.Commit(context.Background()) - if err != nil { - return err - } - t.committed = true - return nil -} - -func (t *txWrap) RollbackUnlessCommitted() error { - if !t.committed && t.rollbackUnlessCommittedNeeded { - err := t.Rollback() - if err != nil { - return err - } - } - return nil -} - -func (t *txWrap) ExecContext(ctx context.Context, query string, args ...any) (extypes.Result, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - res, err := t.tx.Exec(ctx, sql, args...) - if err != nil { - return nil, err - } - return &resultWrap{res: res}, nil -} - -func (t *txWrap) QueryContext(ctx context.Context, query string, args ...any) (extypes.Rows, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - rows, err := t.tx.Query(ctx, sql, args...) - if err != nil { - return nil, err - } - return &rowsWrap{rows: rows}, nil -} diff --git a/executor/adapters/pgx5/pool.go b/executor/adapters/pgx5/pool.go index 7d2b6f3..4163b20 100644 --- a/executor/adapters/pgx5/pool.go +++ b/executor/adapters/pgx5/pool.go @@ -2,54 +2,72 @@ package pgx5 import ( "context" - "fmt" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" + "github.com/insei/gerpo/executor/adapters/internal" "github.com/insei/gerpo/executor/adapters/placeholder" - "github.com/insei/gerpo/executor/types" + extypes "github.com/insei/gerpo/executor/types" ) -type poolWrap struct { +// poolBackend implements internal.Backend on top of a pgx v5 connection pool. +type poolBackend struct { pool *pgxpool.Pool } -func (p *poolWrap) ExecContext(ctx context.Context, query string, args ...any) (types.Result, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) +func (b *poolBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + res, err := b.pool.Exec(ctx, sql, args...) if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) + return nil, err } - res, err := p.pool.Exec(ctx, sql, args...) + return &resultWrap{res: res}, nil +} + +func (b *poolBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rows, err := b.pool.Query(ctx, sql, args...) if err != nil { return nil, err } - return &resultWrap{res: res}, nil + return &rowsWrap{rows: rows}, nil } -func (p *poolWrap) QueryContext(ctx context.Context, query string, args ...any) (types.Rows, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) +func (b *poolBackend) BeginTx(ctx context.Context) (internal.TxBackend, error) { + tx, err := b.pool.BeginTx(ctx, pgx.TxOptions{}) if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) + return nil, err } - rows, err := p.pool.Query(ctx, sql, args...) + return &txBackend{tx: tx}, nil +} + +// txBackend implements internal.TxBackend on top of pgx.Tx. Commit/Rollback do +// not propagate caller context — pgx.Tx insists on its own background context +// for those operations. +type txBackend struct { + tx pgx.Tx +} + +func (t *txBackend) Exec(ctx context.Context, sql string, args ...any) (extypes.Result, error) { + res, err := t.tx.Exec(ctx, sql, args...) if err != nil { return nil, err } - return &rowsWrap{rows: rows}, nil + return &resultWrap{res: res}, nil } -func (p *poolWrap) BeginTx(ctx context.Context) (types.Tx, error) { - tx, err := p.pool.BeginTx(ctx, pgx.TxOptions{}) +func (t *txBackend) Query(ctx context.Context, sql string, args ...any) (extypes.Rows, error) { + rows, err := t.tx.Query(ctx, sql, args...) if err != nil { return nil, err } - return &txWrap{ - rollbackUnlessCommittedNeeded: true, - tx: tx, - }, err + return &rowsWrap{rows: rows}, nil } -func NewPoolAdapter(pool *pgxpool.Pool) types.DBAdapter { - return &poolWrap{pool} +func (t *txBackend) Commit() error { return t.tx.Commit(context.Background()) } +func (t *txBackend) Rollback() error { return t.tx.Rollback(context.Background()) } + +// NewPoolAdapter wraps a pgx v5 pool with the gerpo DB adapter contract. +// SQL placeholders are rewritten from `?` to PostgreSQL's `$1, $2, …` form. +func NewPoolAdapter(pool *pgxpool.Pool) extypes.DBAdapter { + return internal.New(&poolBackend{pool: pool}, placeholder.Dollar) } diff --git a/executor/adapters/pgx5/tx.go b/executor/adapters/pgx5/tx.go deleted file mode 100644 index 86bf95c..0000000 --- a/executor/adapters/pgx5/tx.go +++ /dev/null @@ -1,64 +0,0 @@ -package pgx5 - -import ( - "context" - "fmt" - - "github.com/insei/gerpo/executor/adapters/placeholder" - extypes "github.com/insei/gerpo/executor/types" - "github.com/jackc/pgx/v5" -) - -type txWrap struct { - committed bool - rollbackUnlessCommittedNeeded bool - tx pgx.Tx -} - -func (t *txWrap) Rollback() error { - t.rollbackUnlessCommittedNeeded = false - return t.tx.Rollback(context.Background()) -} - -func (t *txWrap) Commit() error { - err := t.tx.Commit(context.Background()) - if err != nil { - return err - } - t.committed = true - return nil -} - -func (t *txWrap) RollbackUnlessCommitted() error { - if !t.committed && t.rollbackUnlessCommittedNeeded { - err := t.Rollback() - if err != nil { - return err - } - } - return nil -} - -func (t *txWrap) ExecContext(ctx context.Context, query string, args ...any) (extypes.Result, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - res, err := t.tx.Exec(ctx, sql, args...) - if err != nil { - return nil, err - } - return &resultWrap{res: res}, nil -} - -func (t *txWrap) QueryContext(ctx context.Context, query string, args ...any) (extypes.Rows, error) { - sql, err := placeholder.Dollar.ReplacePlaceholders(query) - if err != nil { - return nil, fmt.Errorf("failed to replace placeholders: %w", err) - } - rows, err := t.tx.Query(ctx, sql, args...) - if err != nil { - return nil, err - } - return &rowsWrap{rows: rows}, nil -} From 7f24176d68a170a2c7d8c46bb01e98bf49994c90 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 05:28:15 +0300 Subject: [PATCH 09/15] feat: add tracer hook for executor operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repos that ship to prod usually want to see DB calls in their tracer of choice (OpenTelemetry, Datadog, …). Doing this externally requires wrapping the DBAdapter and re-emitting spans for every executor call, which is awkward. Adds a first-class hook with the same shape every tracer ends up using. API: type SpanEnd func(err error) type Tracer func(ctx context.Context, op string) (context.Context, SpanEnd) repo, _ := gerpo.NewBuilder[User](). DB(adapter, executor.WithTracer(myTracer)). ... The hook is invoked for every public Executor operation — GetOne, GetMultiple, Count, InsertOne, Update, Delete — with op names prefixed "gerpo.". The returned context is propagated downstream so child work (driver calls, cache reads) lands inside the same span; SpanEnd is called once with the operation's terminal error. Implementation: - executor/options.go: new Tracer type and WithTracer option. - executor/executor.go: thin startSpan helper short-circuits on a nil tracer (no allocations / branch when tracing is disabled). Each public method opens a span at entry and defers end(err); switched to named return values where needed. - executor/tracer_test.go: a recordingTracer drives every entry point and verifies (a) the tracer is invoked exactly once per call, (b) the op name is correct, (c) the terminal error is reported, and (d) the no-tracer path is a no-op. Tracing is opt-in. Logs and metrics intentionally stay external — open an issue if a dedicated hook becomes useful. Docs: - docs/features/tracing.md (new) — adapter recipes for OpenTelemetry and Datadog, op-name table, "disabled by default" note. - docs/features/index.md and mkdocs.yml — link to the new page. --- docs/features/index.md | 1 + docs/features/tracing.md | 98 +++++++++++++++++++++++++++ executor/executor.go | 49 +++++++++++--- executor/options.go | 27 ++++++++ executor/tracer_test.go | 142 +++++++++++++++++++++++++++++++++++++++ mkdocs.yml | 1 + 6 files changed, 307 insertions(+), 11 deletions(-) create mode 100644 docs/features/tracing.md create mode 100644 executor/tracer_test.go diff --git a/docs/features/index.md b/docs/features/index.md index b4bf36e..f7ae790 100644 --- a/docs/features/index.md +++ b/docs/features/index.md @@ -29,4 +29,5 @@ Reference of gerpo capabilities grouped by area. | Page | What's inside | |---|---| | [Cache](cache.md) | `CtxCache` — cache scoped to a request context | +| [Tracing](tracing.md) | `WithTracer` hook — OpenTelemetry / Datadog / any tracer | | [Adapters](adapters.md) | pgx v5, pgx v4, database/sql, and custom adapters | diff --git a/docs/features/tracing.md b/docs/features/tracing.md new file mode 100644 index 0000000..3fdac7a --- /dev/null +++ b/docs/features/tracing.md @@ -0,0 +1,98 @@ +# Tracing + +`executor.WithTracer(fn)` installs a hook that the executor opens around every public operation (`GetOne`, `GetMultiple`, `Count`, `InsertOne`, `Update`, `Delete`). gerpo deliberately does not pull a tracing library into its dependencies — adapt the hook to whatever your stack uses. + +## The hook + +```go +type SpanEnd func(err error) + +type Tracer func(ctx context.Context, op string) (context.Context, SpanEnd) +``` + +- `op` is the operation name, prefixed with `gerpo.` (`gerpo.GetOne`, `gerpo.Update`, …). +- The returned `context.Context` is propagated downstream — child operations (driver calls, cache reads) land inside the same span. +- `SpanEnd` is called once on the operation's terminal error (or `nil` on success). + +## Wiring with OpenTelemetry + +```go +import ( + "context" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + + "github.com/insei/gerpo" + "github.com/insei/gerpo/executor" + "github.com/insei/gerpo/executor/adapters/pgx5" +) + +func otelTracer() executor.Tracer { + tr := otel.Tracer("gerpo") + return func(ctx context.Context, op string) (context.Context, executor.SpanEnd) { + ctx, span := tr.Start(ctx, op, + trace.WithAttributes(attribute.String("db.system", "postgresql")), + ) + return ctx, func(err error) { + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + } + span.End() + } + } +} + +repo, _ := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool), executor.WithTracer(otelTracer())). + Table("users"). + Columns(...). + Build() +``` + +## Wiring with Datadog (`dd-trace-go`) + +```go +import ( + "context" + + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" + + "github.com/insei/gerpo/executor" +) + +func datadogTracer() executor.Tracer { + return func(ctx context.Context, op string) (context.Context, executor.SpanEnd) { + span, ctx := tracer.StartSpanFromContext(ctx, op, + tracer.ResourceName(op), + tracer.SpanType("sql"), + ) + return ctx, func(err error) { + span.Finish(tracer.WithError(err)) + } + } +} +``` + +## Operation names + +| Op | Emitted span | +|---|---| +| `repo.GetFirst` | `gerpo.GetOne` | +| `repo.GetList` | `gerpo.GetMultiple` | +| `repo.Count` | `gerpo.Count` | +| `repo.Insert` | `gerpo.InsertOne` | +| `repo.Update` | `gerpo.Update` | +| `repo.Delete` | `gerpo.Delete` | + +## Disabled by default + +If you don't pass `WithTracer`, the executor short-circuits on a nil tracer — there is no allocation per call and no behavior change. Tracing is opt-in. + +## Logs and metrics + +Logging and per-op metrics are not exposed as dedicated hooks today. The recommended pattern is to wrap your `executor.DBAdapter` with a thin `tracingAdapter` (see [Adapters → Tracing wrapper](adapters.md#why-write-a-custom-adapter)) that captures duration and SQL text, then emits whatever your stack expects. + +A natural extension would be `WithLogger` / `WithMetrics` callbacks similar to `WithTracer`. They are intentionally absent for now — open an issue if you have a concrete need. diff --git a/executor/executor.go b/executor/executor.go index 030d96c..bb01c62 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -41,7 +41,22 @@ func (e *executor[TModel]) getExecQuery(ctx context.Context) ExecQuery { return e.db } -func (e *executor[TModel]) GetOne(ctx context.Context, stmt Stmt) (*TModel, error) { +// startSpan opens a tracing span around an executor operation. When no Tracer +// is configured, returns the original context and a no-op end function so the +// callers can stay branch-free. +func (e *executor[TModel]) startSpan(ctx context.Context, op string) (context.Context, SpanEnd) { + if e.tracer == nil { + return ctx, noopSpanEnd + } + return e.tracer(ctx, op) +} + +func noopSpanEnd(error) {} + +func (e *executor[TModel]) GetOne(ctx context.Context, stmt Stmt) (model *TModel, err error) { + ctx, end := e.startSpan(ctx, "gerpo.GetOne") + defer func() { end(err) }() + sql, args, err := stmt.SQL() if err != nil { return nil, fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -54,7 +69,6 @@ func (e *executor[TModel]) GetOne(ctx context.Context, stmt Stmt) (*TModel, erro return nil, err } defer rows.Close() //nolint:errcheck - var model *TModel if rows.Next() { model = new(TModel) pointers := stmt.Columns().GetModelPointers(model) @@ -69,7 +83,10 @@ func (e *executor[TModel]) GetOne(ctx context.Context, stmt Stmt) (*TModel, erro return model, nil } -func (e *executor[TModel]) GetMultiple(ctx context.Context, stmt Stmt) ([]*TModel, error) { +func (e *executor[TModel]) GetMultiple(ctx context.Context, stmt Stmt) (models []*TModel, err error) { + ctx, end := e.startSpan(ctx, "gerpo.GetMultiple") + defer func() { end(err) }() + sql, args, err := stmt.SQL() if err != nil { return nil, fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -82,7 +99,6 @@ func (e *executor[TModel]) GetMultiple(ctx context.Context, stmt Stmt) ([]*TMode return nil, err } defer rows.Close() //nolint:errcheck - var models []*TModel for rows.Next() { model := new(TModel) if err = rows.Scan(stmt.Columns().GetModelPointers(model)...); err != nil { @@ -94,7 +110,10 @@ func (e *executor[TModel]) GetMultiple(ctx context.Context, stmt Stmt) ([]*TMode return models, nil } -func (e *executor[TModel]) InsertOne(ctx context.Context, stmt Stmt, model *TModel) error { +func (e *executor[TModel]) InsertOne(ctx context.Context, stmt Stmt, model *TModel) (err error) { + ctx, end := e.startSpan(ctx, "gerpo.InsertOne") + defer func() { end(err) }() + sql, values, err := stmt.SQL(sqlstmt.WithModelValues(model)) if err != nil { return fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -114,7 +133,10 @@ func (e *executor[TModel]) InsertOne(ctx context.Context, stmt Stmt, model *TMod return nil } -func (e *executor[TModel]) Update(ctx context.Context, stmt Stmt, model *TModel) (int64, error) { +func (e *executor[TModel]) Update(ctx context.Context, stmt Stmt, model *TModel) (updatedRows int64, err error) { + ctx, end := e.startSpan(ctx, "gerpo.Update") + defer func() { end(err) }() + sql, values, err := stmt.SQL(sqlstmt.WithModelValues(model)) if err != nil { return 0, fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -123,7 +145,7 @@ func (e *executor[TModel]) Update(ctx context.Context, stmt Stmt, model *TModel) if err != nil { return 0, err } - updatedRows, err := result.RowsAffected() + updatedRows, err = result.RowsAffected() if err != nil { return 0, err } @@ -133,7 +155,10 @@ func (e *executor[TModel]) Update(ctx context.Context, stmt Stmt, model *TModel) return updatedRows, nil } -func (e *executor[TModel]) Count(ctx context.Context, stmt CountStmt) (uint64, error) { +func (e *executor[TModel]) Count(ctx context.Context, stmt CountStmt) (count uint64, err error) { + ctx, end := e.startSpan(ctx, "gerpo.Count") + defer func() { end(err) }() + sql, args, err := stmt.SQL() if err != nil { return 0, fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -141,7 +166,6 @@ func (e *executor[TModel]) Count(ctx context.Context, stmt CountStmt) (uint64, e if cached, ok := get[uint64](ctx, e.cacheSource, sql, args...); ok { return *cached, nil } - count := uint64(0) rows, err := e.getExecQuery(ctx).QueryContext(ctx, sql, args...) if err != nil { return 0, err @@ -156,7 +180,10 @@ func (e *executor[TModel]) Count(ctx context.Context, stmt CountStmt) (uint64, e return count, nil } -func (e *executor[TModel]) Delete(ctx context.Context, stmt CountStmt) (int64, error) { +func (e *executor[TModel]) Delete(ctx context.Context, stmt CountStmt) (deletedRows int64, err error) { + ctx, end := e.startSpan(ctx, "gerpo.Delete") + defer func() { end(err) }() + sql, args, err := stmt.SQL() if err != nil { return 0, fmt.Errorf("failed to get sql query from stmt: %w", err) @@ -165,7 +192,7 @@ func (e *executor[TModel]) Delete(ctx context.Context, stmt CountStmt) (int64, e if err != nil { return 0, err } - deletedRows, err := result.RowsAffected() + deletedRows, err = result.RowsAffected() if err != nil { return 0, err } diff --git a/executor/options.go b/executor/options.go index 9f68493..8f0ebb0 100644 --- a/executor/options.go +++ b/executor/options.go @@ -1,11 +1,28 @@ package executor import ( + "context" + "github.com/insei/gerpo/executor/cache" ) +// SpanEnd is returned by a Tracer to signal the end of a span. The wrapped +// operation reports its terminal error (or nil on success) through it. +type SpanEnd func(err error) + +// Tracer is gerpo's OpenTelemetry-friendly tracing hook. It is invoked at the +// start of every public Executor operation (GetOne, GetMultiple, Count, +// InsertOne, Update, Delete). The returned context is propagated downstream +// (so child operations land inside the span) and the SpanEnd is called with +// the operation's terminal error. +// +// gerpo deliberately does not import any tracing library — implement the +// adapter against your tracer of choice (OpenTelemetry, Datadog, OpenCensus, …). +type Tracer func(ctx context.Context, op string) (context.Context, SpanEnd) + type options struct { cacheSource cache.Storage + tracer Tracer } type Option interface { @@ -28,3 +45,13 @@ func WithCacheStorage(source cache.Storage) Option { } }) } + +// WithTracer installs a tracing hook called around every Executor operation. +// Pass nil to disable tracing (this is the default). The hook signature is +// driver-agnostic — see Tracer for an adapter pattern compatible with +// OpenTelemetry / Datadog / OpenCensus. +func WithTracer(tracer Tracer) Option { + return optionFn(func(o *options) { + o.tracer = tracer + }) +} diff --git a/executor/tracer_test.go b/executor/tracer_test.go new file mode 100644 index 0000000..fc9f718 --- /dev/null +++ b/executor/tracer_test.go @@ -0,0 +1,142 @@ +package executor + +import ( + "context" + "errors" + "testing" + + "github.com/insei/gerpo/sqlstmt" + "github.com/insei/gerpo/types" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// recordingTracer captures every span open/close pair so tests can assert on +// op names, propagation and reported errors. +type recordingTracer struct { + starts []string + ends []spanEndRecord +} + +type spanEndRecord struct { + op string + err error +} + +func (r *recordingTracer) tracer() Tracer { + return func(ctx context.Context, op string) (context.Context, SpanEnd) { + r.starts = append(r.starts, op) + ctx = context.WithValue(ctx, traceKey{}, op) + return ctx, func(err error) { + r.ends = append(r.ends, spanEndRecord{op: op, err: err}) + } + } +} + +type traceKey struct{} + +// stmtThatChecksContext is a Stmt double whose SQL() method asserts the +// downstream code receives the context propagated by the tracer. We need this +// because the executor passes the (already-traced) ctx into the stmt and the +// adapter — verifying that propagation is the whole point of tracing. +type stmtThatChecksContext struct { + mock.Mock + expectedOp string + t *testing.T +} + +func (s *stmtThatChecksContext) SQL(opts ...sqlstmt.Option) (string, []any, error) { + rets := s.Called() + return rets.String(0), rets.Get(1).([]any), rets.Error(2) +} + +func (s *stmtThatChecksContext) Columns() types.ExecutionColumns { + rets := s.Called() + return rets.Get(0).(types.ExecutionColumns) +} + +// TestTracer_GetOne_OpensAndClosesSpan ensures the executor opens a span on +// entry and closes it with a nil error on the happy path. +func TestTracer_GetOne_OpensAndClosesSpan(t *testing.T) { + rec := &recordingTracer{} + + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("SELECT 1", []any{}, errors.New("stop early")) + + e := New[testModel](nil, WithTracer(rec.tracer())) + _, err := e.GetOne(context.Background(), stmt) + require.Error(t, err, "stmt SQL returns an error so the path stops early") + + require.Equal(t, []string{"gerpo.GetOne"}, rec.starts) + require.Len(t, rec.ends, 1) + assert.Equal(t, "gerpo.GetOne", rec.ends[0].op) + assert.ErrorContains(t, rec.ends[0].err, "stop early", "tracer must observe the terminal error") +} + +// TestTracer_NoOp_WhenNotConfigured asserts the executor adds zero overhead +// (no panics, no behavior drift) when WithTracer is not used. +func TestTracer_NoOp_WhenNotConfigured(t *testing.T) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("SELECT 1", []any{}, errors.New("ignored")) + + e := New[testModel](nil) // no tracer + _, err := e.GetOne(context.Background(), stmt) + require.Error(t, err) +} + +// TestTracer_AllOpsCovered runs every executor entry point and verifies each +// one opens a span with the documented operation name. +func TestTracer_AllOpsCovered(t *testing.T) { + rec := &recordingTracer{} + e := New[testModel](nil, WithTracer(rec.tracer())) + + cases := []struct { + name string + op string + call func(t *testing.T, e Executor[testModel]) + }{ + {"GetOne", "gerpo.GetOne", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _, _ = e.GetOne(context.Background(), stmt) + }}, + {"GetMultiple", "gerpo.GetMultiple", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _, _ = e.GetMultiple(context.Background(), stmt) + }}, + {"Count", "gerpo.Count", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _, _ = e.Count(context.Background(), stmt) + }}, + {"Delete", "gerpo.Delete", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _, _ = e.Delete(context.Background(), stmt) + }}, + {"InsertOne", "gerpo.InsertOne", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _ = e.InsertOne(context.Background(), stmt, &testModel{}) + }}, + {"Update", "gerpo.Update", func(t *testing.T, e Executor[testModel]) { + stmt := new(stmtThatChecksContext) + stmt.On("SQL").Return("", []any{}, errors.New("x")) + _, _ = e.Update(context.Background(), stmt, &testModel{}) + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rec.starts = nil + rec.ends = nil + tc.call(t, e) + require.Equal(t, []string{tc.op}, rec.starts) + require.Len(t, rec.ends, 1) + assert.Equal(t, tc.op, rec.ends[0].op) + }) + } +} diff --git a/mkdocs.yml b/mkdocs.yml index bf77f48..7e1b894 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -83,6 +83,7 @@ nav: - Hooks: features/hooks.md - Transactions: features/transactions.md - Cache: features/cache.md + - Tracing: features/tracing.md - Error transformer: features/error-transformer.md - Adapters: features/adapters.md - Architecture: From 6d4b5070ffc17269cb2c13e53319277cabaf282f Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 05:36:13 +0300 Subject: [PATCH 10/15] refactor: factor query helpers around small composable interfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each per-operation helper (GetFirstHelper, GetListHelper, CountHelper, InsertHelper, UpdateHelper, DeleteHelper) listed its methods inline, which made it impossible for callers to write reusable middleware-style helpers (e.g. apply a tenant filter regardless of operation kind). Adds query/interfaces.go with four small contracts and reuses them via embedding: Filterable { Where() types.WhereTarget } Sortable { OrderBy() types.OrderTarget } Excludable { Exclude(...); Only(...) } Pageable[T] { Page(...) GetListHelper[T]; Size(...) GetListHelper[T] } GetFirstHelper[T] = Filterable + Sortable + Excludable GetListHelper[T] = Filterable + Sortable + Excludable + Pageable[T] CountHelper[T] = Filterable DeleteHelper[T] = Filterable InsertHelper[T] = Excludable UpdateHelper[T] = Filterable + Excludable The aggregate interfaces keep the same names, the same method sets and the same package, so this is a pure addition to the public API: no existing call site, type assertion or alternate implementation needs to change. Concrete helpers (query.GetFirst[T], query.GetList[T], …) already provide the methods, so they automatically satisfy both the old aggregate names and the new narrow contracts. Tests: - query/interfaces_test.go pins compile-time guarantees that every concrete helper implements both its aggregate interface and each of the small contracts. No runtime test needed — embedding is a flat method-set expansion. Use case (now possible): func applyTenant(h query.Filterable, tid uuid.UUID) { h.Where().Field(...).EQ(tid) } repo.GetFirst(ctx, func(m *User, h query.GetFirstHelper[User]) { applyTenant(h, tid) }) repo.Update(ctx, &u, func(m *User, h query.UpdateHelper[User]) { applyTenant(h, tid) }) Docs: docs/features/crud.md gains a tip block introducing the new contracts with an example. Per-operation pages keep their existing copy. --- docs/features/crud.md | 12 +++++++++ query/count.go | 6 ++--- query/delete.go | 6 ++--- query/first.go | 20 +++++--------- query/insert.go | 7 +++-- query/interfaces.go | 57 ++++++++++++++++++++++++++++++++++++++++ query/interfaces_test.go | 30 +++++++++++++++++++++ query/list.go | 22 +++++----------- query/update.go | 12 +++------ 9 files changed, 125 insertions(+), 47 deletions(-) create mode 100644 query/interfaces.go create mode 100644 query/interfaces_test.go diff --git a/docs/features/crud.md b/docs/features/crud.md index 6ba1eb6..0107009 100644 --- a/docs/features/crud.md +++ b/docs/features/crud.md @@ -2,6 +2,18 @@ The `Repository[T]` interface provides six methods: `GetFirst`, `GetList`, `Count`, `Insert`, `Update`, `Delete`. Each one accepts a `context.Context` and a variadic list of query functions that configure a single call. +!!! tip "Reusable per-operation helpers" + Every per-operation helper (`GetFirstHelper`, `GetListHelper`, `CountHelper`, `InsertHelper`, `UpdateHelper`, `DeleteHelper`) is composed from small contracts in the `query` package: `Filterable` (`Where()`), `Sortable` (`OrderBy()`), `Excludable` (`Exclude/Only`), `Pageable` (`Page/Size`). You can write reusable middleware-style helpers against these narrow interfaces: + + ```go + func applyTenant(h query.Filterable, tenantID uuid.UUID) { + h.Where().Field(...).EQ(tenantID) + } + + repo.GetFirst(ctx, func(m *User, h query.GetFirstHelper[User]) { applyTenant(h, tid) }) + repo.Update(ctx, &u, func(m *User, h query.UpdateHelper[User]) { applyTenant(h, tid) }) + ``` + ## GetFirst Return the first matching record. diff --git a/query/count.go b/query/count.go index 9f64920..02cf6d7 100644 --- a/query/count.go +++ b/query/count.go @@ -8,10 +8,10 @@ import ( "github.com/insei/gerpo/types" ) -// CountHelper defines an interface for building and managing query conditions to count records of a specific model. +// CountHelper is the per-request helper for repo.Count. It only filters — +// see interfaces.go for the Filterable contract. type CountHelper[TModel any] interface { - // Where defines the starting point for building conditions in a query, returning a types.WhereTarget interface. - Where() types.WhereTarget + Filterable } type CountApplier interface { diff --git a/query/delete.go b/query/delete.go index 40089d2..240b6bd 100644 --- a/query/delete.go +++ b/query/delete.go @@ -8,10 +8,10 @@ import ( "github.com/insei/gerpo/types" ) -// DeleteHelper is an interface that provides functionality for constructing delete queries using conditions. +// DeleteHelper is the per-request helper for repo.Delete. It only filters — +// see interfaces.go for the Filterable contract. type DeleteHelper[TModel any] interface { - // Where defines the starting point for building conditions in a query, returning a types.WhereTarget interface. - Where() types.WhereTarget + Filterable } type DeleteApplier interface { diff --git a/query/first.go b/query/first.go index df82137..38c4b9d 100644 --- a/query/first.go +++ b/query/first.go @@ -8,21 +8,13 @@ import ( "github.com/insei/gerpo/types" ) -// GetFirstHelper is an interface for building query conditions to retrieve the first record matching specified criteria. -// It allows specifying WHERE conditions, excluding specific fields, and defining order-by operations for the query. +// GetFirstHelper is the per-request helper for repo.GetFirst. It composes the +// small contracts from interfaces.go: filtering, sorting and narrowing the +// column set. type GetFirstHelper[TModel any] interface { - - // Where defines the starting point for building conditions in a query, returning a types.WhereTarget interface. - Where() types.WhereTarget - - // Exclude removes specified fields from requesting data from repository storage. - Exclude(fieldsPtr ...any) - - // Only includes the specified columns in the execution context, ignoring all others in the existing collection. - Only(fieldsPtr ...any) - - // OrderBy defines the sorting criteria for a query and returns types.OrderTarget interface for further specification. - OrderBy() types.OrderTarget + Filterable + Sortable + Excludable } // GetFirstApplier defines an interface for applying columns, filters, and ordering in a query construction process. diff --git a/query/insert.go b/query/insert.go index 7133253..10da72a 100644 --- a/query/insert.go +++ b/query/insert.go @@ -7,11 +7,10 @@ import ( "github.com/insei/gerpo/types" ) +// InsertHelper is the per-request helper for repo.Insert. It only narrows the +// column set — see interfaces.go for the Excludable contract. type InsertHelper[TModel any] interface { - // Exclude removes specified fields from requesting data from repository storage. - Exclude(fieldsPtr ...any) - // Only includes the specified columns in the execution context, ignoring all others in the existing collection. - Only(fieldsPtr ...any) + Excludable } type InsertApplier interface { diff --git a/query/interfaces.go b/query/interfaces.go new file mode 100644 index 0000000..cf07e7b --- /dev/null +++ b/query/interfaces.go @@ -0,0 +1,57 @@ +package query + +import "github.com/insei/gerpo/types" + +// The interfaces in this file are small composable contracts that the +// per-operation helpers (GetFirstHelper, GetListHelper, …) embed. They are +// intentionally narrow so callers can write reusable middleware-style helpers +// without depending on the full operation interface. +// +// Example: a tenant-aware filter that works for every query type. +// +// func applyTenant(h query.Filterable, tenantID uuid.UUID) { +// h.Where().Field(...).EQ(tenantID) +// } +// +// repo.GetFirst(ctx, func(m *User, h query.GetFirstHelper[User]) { +// applyTenant(h, tid) +// }) +// repo.Update(ctx, &u, func(m *User, h query.UpdateHelper[User]) { +// applyTenant(h, tid) +// }) + +// Filterable describes any helper that exposes a WHERE entry point. +// GetFirst, GetList, Count, Update and Delete all satisfy it. +type Filterable interface { + // Where defines the starting point for building conditions in a query, + // returning a types.WhereTarget interface. + Where() types.WhereTarget +} + +// Sortable describes any helper that exposes an ORDER BY entry point. +// GetFirst and GetList satisfy it. +type Sortable interface { + // OrderBy defines the sorting criteria for a query and returns a + // types.OrderTarget interface for further specification. + OrderBy() types.OrderTarget +} + +// Excludable describes any helper that lets the caller narrow the column set +// of an operation through Exclude / Only. GetFirst, GetList, Insert and +// Update satisfy it. +type Excludable interface { + // Exclude removes specified fields from requesting data from repository storage. + Exclude(fieldsPtr ...any) + // Only includes the specified columns in the execution context, ignoring all others in the existing collection. + Only(fieldsPtr ...any) +} + +// Pageable describes the pagination contract on a list helper. The methods +// return GetListHelper so that the chain stays usable from inside a single +// query closure (h.Page(1).Size(20).Where()...). +type Pageable[TModel any] interface { + // Page sets the page number for pagination in a query and returns the same GetListHelper instance. + Page(page uint64) GetListHelper[TModel] + // Size sets the maximum number of items to retrieve per page and returns the same GetListHelper instance. + Size(size uint64) GetListHelper[TModel] +} diff --git a/query/interfaces_test.go b/query/interfaces_test.go new file mode 100644 index 0000000..bbc40ee --- /dev/null +++ b/query/interfaces_test.go @@ -0,0 +1,30 @@ +package query + +// Compile-time guarantees that concrete helpers implement the small composable +// contracts as well as their per-operation aggregates. If any of these break, +// the corresponding concrete type drifted from the interface contract. +var ( + _ Filterable = (*GetFirst[any])(nil) + _ Sortable = (*GetFirst[any])(nil) + _ Excludable = (*GetFirst[any])(nil) + _ GetFirstHelper[any] = (*GetFirst[any])(nil) + + _ Filterable = (*GetList[any])(nil) + _ Sortable = (*GetList[any])(nil) + _ Excludable = (*GetList[any])(nil) + _ Pageable[any] = (*GetList[any])(nil) + _ GetListHelper[any] = (*GetList[any])(nil) + + _ Filterable = (*Count[any])(nil) + _ CountHelper[any] = (*Count[any])(nil) + + _ Excludable = (*Insert[any])(nil) + _ InsertHelper[any] = (*Insert[any])(nil) + + _ Filterable = (*Update[any])(nil) + _ Excludable = (*Update[any])(nil) + _ UpdateHelper[any] = (*Update[any])(nil) + + _ Filterable = (*Delete[any])(nil) + _ DeleteHelper[any] = (*Delete[any])(nil) +) diff --git a/query/list.go b/query/list.go index 3b68ab9..73aee4a 100644 --- a/query/list.go +++ b/query/list.go @@ -8,22 +8,14 @@ import ( "github.com/insei/gerpo/types" ) -// GetListHelper is a generic interface for building complex queries to retrieve lists of data models. +// GetListHelper is the per-request helper for repo.GetList. It composes +// the small contracts from interfaces.go: filtering, sorting, narrowing the +// column set, and pagination. type GetListHelper[TModel any] interface { - // Exclude removes specified fields from requesting data from repository storage. - Exclude(fieldsPtr ...any) - // Only includes the specified columns in the execution context, ignoring all others in the existing collection. - Only(fieldsPtr ...any) - // Where defines the starting point for building conditions in a query, returning a types.WhereTarget interface. - Where() types.WhereTarget - // OrderBy defines the sorting criteria for a query and returns types.OrderTarget interface for further specification. - OrderBy() types.OrderTarget - - // Page sets the page number for pagination in a query and returns the same GetListHelper instance. - Page(page uint64) GetListHelper[TModel] - - // Size sets the maximum number of items to retrieve per page and returns the same GetListHelper instance. - Size(size uint64) GetListHelper[TModel] + Filterable + Sortable + Excludable + Pageable[TModel] } type GetListApplier interface { diff --git a/query/update.go b/query/update.go index 8fc53c4..a1c2205 100644 --- a/query/update.go +++ b/query/update.go @@ -8,15 +8,11 @@ import ( "github.com/insei/gerpo/types" ) -// UpdateHelper is a generic interface for managing update operations on models in repository storage. -// It allows specifying conditions and excluding specific fields from the update process. +// UpdateHelper is the per-request helper for repo.Update. It composes the +// small contracts from interfaces.go: filtering and narrowing the column set. type UpdateHelper[TModel any] interface { - // Exclude removes specified fields from requesting data from repository storage. - Exclude(fieldsPtr ...any) - // Only includes the specified columns in the execution context, ignoring all others in the existing collection. - Only(fieldsPtr ...any) - // Where defines the starting point for building conditions in a query, returning a types.WhereTarget interface. - Where() types.WhereTarget + Filterable + Excludable } type UpdateApplier interface { From f3a4a0d5c213072bab8eb71f8f1f74eb5b09e1ae Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 05:44:15 +0300 Subject: [PATCH 11/15] docs: add runnable examples for godoc / pkg.go.dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pkg.go.dev shows ExampleXxx functions next to the corresponding identifiers. Adds example_test.go in package gerpo_test with snippets for the most common entry points. Examples: - ExampleNewBuilder — minimum builder chain (pgx5). - ExampleRepository_GetFirst — single-row read with ErrNotFound mapping. - ExampleRepository_GetList — filter + ORDER BY + pagination. - ExampleRepository_Insert — InsertHelper.Exclude to defer to DB DEFAULT. - ExampleRepository_Update — Only(...) for column-scoped updates. - ExampleRepository_Delete — Delete with WHERE. - ExampleRepository_Tx — BeginTx + repo.Tx + RollbackUnlessCommitted defer. - ExampleWithSoftDeletion — soft-delete configuration end-to-end. - ExampleWithErrorTransformer — mapping ErrNotFound to a domain error. - ExampleWithTracer — opt-in tracer hook adapter. - ExampleWithCacheStorage — CtxCache wiring. The examples deliberately omit `// Output:` lines because every one of them needs a live database to actually run; the compiler still type- checks them on every `go test ./...`, so they cannot drift from the public API. Two tiny placeholder constructors (exampleRepo, exampleAdapter) keep the snippets niladic without polluting them with boilerplate. docs/index.md mentions that pkg.go.dev now ships runnable examples. --- docs/index.md | 2 +- example_test.go | 287 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 example_test.go diff --git a/docs/index.md b/docs/index.md index e3f78ff..21d2660 100644 --- a/docs/index.md +++ b/docs/index.md @@ -87,7 +87,7 @@ Full runnable samples live in [`examples/`](https://github.com/Insei/gerpo/tree/ - :simple-go:{ .lg } **[API reference →](https://pkg.go.dev/github.com/insei/gerpo)** - Autogenerated on pkg.go.dev. + Autogenerated on pkg.go.dev — including runnable examples next to every method. - :material-github:{ .lg } **[Repository →](https://github.com/Insei/gerpo)** diff --git a/example_test.go b/example_test.go new file mode 100644 index 0000000..be62094 --- /dev/null +++ b/example_test.go @@ -0,0 +1,287 @@ +package gerpo_test + +// The examples below appear on pkg.go.dev next to the corresponding methods +// and types. They do not run during `go test ./...` (no // Output: line) — +// each one would require a live database — but the compiler still checks them, +// so the snippets cannot rot away from the public API. + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/insei/gerpo" + "github.com/insei/gerpo/executor" + "github.com/insei/gerpo/executor/adapters/pgx5" + cachectx "github.com/insei/gerpo/executor/cache/ctx" + "github.com/insei/gerpo/query" +) + +// User is the working example throughout these snippets — a typical entity +// with a UUID primary key, a nullable email and timestamp tracking columns. +type User struct { + ID uuid.UUID + Name string + Email *string + Age int + CreatedAt time.Time + UpdatedAt *time.Time + DeletedAt *time.Time +} + +// exampleRepo returns a placeholder repository so the snippet methods on +// pkg.go.dev focus on the API at the call site instead of the boilerplate +// that produces the repo. Replace with the real repository in your code. +func exampleRepo() gerpo.Repository[User] { return nil } + +// exampleAdapter returns a placeholder adapter for the same reason. +func exampleAdapter() executor.DBAdapter { return nil } + +// ExampleNewBuilder shows the minimum chain to assemble a typed repository +// against a pgx v5 pool. +func ExampleNewBuilder() { + pool, err := pgxpool.New(context.Background(), "postgres://localhost/db") + if err != nil { + panic(err) + } + + repo, err := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool)). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + c.Field(&m.Email).AsColumn() + c.Field(&m.Age).AsColumn() + c.Field(&m.CreatedAt).AsColumn().WithUpdateProtection() + c.Field(&m.UpdatedAt).AsColumn().WithInsertProtection() + }). + Build() + if err != nil { + panic(err) + } + _ = repo +} + +// ExampleRepository_GetFirst fetches a single record by primary key and +// translates the absence of a row into the domain layer. +func ExampleRepository_GetFirst() { + repo := exampleRepo() + + u, err := repo.GetFirst(context.Background(), func(m *User, h query.GetFirstHelper[User]) { + h.Where().Field(&m.Email).EQ("alice@example.com") + h.OrderBy().Field(&m.CreatedAt).DESC() + }) + switch { + case errors.Is(err, gerpo.ErrNotFound): + fmt.Println("user not found") + case err != nil: + panic(err) + default: + fmt.Println(u.Name) + } +} + +// ExampleRepository_GetList shows filtering, ordering and pagination in one +// call. Adult users are returned 20 per page, newest first. +func ExampleRepository_GetList() { + repo := exampleRepo() + + users, err := repo.GetList(context.Background(), func(m *User, h query.GetListHelper[User]) { + h.Where().Field(&m.Age).GTE(18) + h.OrderBy().Field(&m.CreatedAt).DESC() + h.Page(1).Size(20) + }) + if err != nil { + panic(err) + } + for _, u := range users { + fmt.Println(u.Name) + } +} + +// ExampleRepository_Insert demonstrates Insert together with InsertHelper to +// let the database default created_at instead of a Go-side timestamp. +func ExampleRepository_Insert() { + repo := exampleRepo() + + u := &User{ + ID: uuid.New(), + Name: "Bob", + Age: 30, + } + if err := repo.Insert(context.Background(), u, func(m *User, h query.InsertHelper[User]) { + h.Exclude(&m.CreatedAt) // let the DB DEFAULT NOW() + }); err != nil { + panic(err) + } +} + +// ExampleRepository_Update updates a single column with Only and reports the +// number of rows touched. +func ExampleRepository_Update() { + repo := exampleRepo() + userID := uuid.New() + + u := &User{ID: userID, Name: "Bob the Builder"} + rows, err := repo.Update(context.Background(), u, func(m *User, h query.UpdateHelper[User]) { + h.Where().Field(&m.ID).EQ(userID) + h.Only(&m.Name) // SET name = ?, leave everything else alone + }) + if err != nil { + panic(err) + } + fmt.Printf("updated %d row(s)\n", rows) +} + +// ExampleRepository_Delete removes records that match a WHERE clause. +func ExampleRepository_Delete() { + repo := exampleRepo() + userID := uuid.New() + + rows, err := repo.Delete(context.Background(), func(m *User, h query.DeleteHelper[User]) { + h.Where().Field(&m.ID).EQ(userID) + }) + if err != nil { + panic(err) + } + fmt.Printf("deleted %d row(s)\n", rows) +} + +// ExampleWithSoftDeletion swaps a physical DELETE for an UPDATE of a marker +// column. The persistent WHERE in WithQuery hides soft-deleted rows from any +// future SELECT or COUNT. +func ExampleWithSoftDeletion() { + pool, _ := pgxpool.New(context.Background(), "postgres://localhost/db") + + repo, err := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool)). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + c.Field(&m.DeletedAt).AsColumn().WithInsertProtection() + }). + WithQuery(func(m *User, h query.PersistentHelper[User]) { + h.Where().Field(&m.DeletedAt).EQ(nil) // hide soft-deleted rows + }). + WithSoftDeletion(func(m *User, b *gerpo.SoftDeletionBuilder[User]) { + b.Field(&m.DeletedAt).SetValueFn(func(ctx context.Context) any { + now := time.Now().UTC() + return &now + }) + }). + Build() + if err != nil { + panic(err) + } + _ = repo +} + +// ExampleWithErrorTransformer maps gerpo's sentinel errors to a domain error +// so the upper layers do not need to import the gerpo package. +func ExampleWithErrorTransformer() { + var ErrUserNotFound = errors.New("user: not found") + + pool, _ := pgxpool.New(context.Background(), "postgres://localhost/db") + + repo, err := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool)). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + }). + WithErrorTransformer(func(err error) error { + if errors.Is(err, gerpo.ErrNotFound) { + return ErrUserNotFound + } + return err + }). + Build() + if err != nil { + panic(err) + } + _ = repo +} + +// ExampleRepository_Tx shows the standard transactional pattern: open a +// driver transaction, wrap the repository with .Tx(tx), then commit or roll +// back. RollbackUnlessCommitted is safe to defer even after a successful Commit. +func ExampleRepository_Tx() { + adapter := exampleAdapter() + repo := exampleRepo() + + ctx := context.Background() + tx, err := adapter.BeginTx(ctx) + if err != nil { + panic(err) + } + defer func() { _ = tx.RollbackUnlessCommitted() }() + + txRepo := repo.Tx(tx) + if err := txRepo.Insert(ctx, &User{ID: uuid.New(), Name: "Carol"}); err != nil { + return + } + if err := tx.Commit(); err != nil { + panic(err) + } +} + +// ExampleWithTracer wires gerpo into an OpenTelemetry-style tracer without +// pulling the OTel package into the gerpo dependency set. +func ExampleWithTracer() { + myTracer := func(ctx context.Context, op string) (context.Context, executor.SpanEnd) { + // Open a span using your tracer of choice. Here we just stub the call. + fmt.Println("start", op) + return ctx, func(err error) { + fmt.Println("end", op, err) + } + } + + pool, _ := pgxpool.New(context.Background(), "postgres://localhost/db") + + repo, err := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool), executor.WithTracer(myTracer)). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + }). + Build() + if err != nil { + panic(err) + } + _ = repo +} + +// ExampleWithCacheStorage attaches the bundled context-scoped cache; reads +// inside a single context.Context get deduplicated, and any Insert/Update/ +// Delete on the same repo invalidates the cache for that context. +func ExampleWithCacheStorage() { + pool, _ := pgxpool.New(context.Background(), "postgres://localhost/db") + cache := cachectx.New() + + repo, err := gerpo.NewBuilder[User](). + DB(pgx5.NewPoolAdapter(pool), executor.WithCacheStorage(cache)). + Table("users"). + Columns(func(m *User, c *gerpo.ColumnBuilder[User]) { + c.Field(&m.ID).AsColumn().WithUpdateProtection() + c.Field(&m.Name).AsColumn() + }). + Build() + if err != nil { + panic(err) + } + + // Wrap the request context once at the entry point so subsequent reads + // served by this repo go through the cache. + ctx := cachectx.NewCtxCache(context.Background()) + _, _ = repo.GetFirst(ctx, func(m *User, h query.GetFirstHelper[User]) { + h.Where().Field(&m.ID).EQ(uuid.UUID{}) + }) +} From 2f9b158158046d3f2b13ac96900d4ce45f49d03c Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 06:02:10 +0300 Subject: [PATCH 12/15] docs: bootstrap CHANGELOG with git-cliff and add release tooling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an automated CHANGELOG pipeline built around Conventional Commits and git-cliff: - cliff.toml — git-cliff configuration. Maps every Conventional Commit type to a CHANGELOG section (Features / Bug Fixes / Performance / Refactor / Documentation / Tests / CI / Build / Misc / Reverts / BREAKING CHANGES). Uses tag pattern `v[0-9]*` and strips the leading "v" from the heading. - CHANGELOG.md — initial render of the existing history under [Unreleased]. Will be re-rendered with each release. - scripts/release.sh — local helper that takes a tag (vX.Y.Z) and: 1) refuses to run unless on clean main and the tag is free; 2) git pulls origin/main; 3) regenerates CHANGELOG.md with git-cliff; 4) shows the diff for review; 5) on confirmation, commits the file and creates the annotated tag. It deliberately does NOT push so the user can `git show ` and amend before sharing. - .github/workflows/release.yml — on push of a tag matching v*, runs `git cliff --latest --strip header` and creates a GitHub Release with that excerpt as the body. Tags carrying a suffix (e.g. v1.0.0-rc1) are marked pre-release automatically. No CHANGELOG.md push-back, no special tokens — only the default GITHUB_TOKEN with contents:write. - .github/workflows/commit-lint.yml — on every PR to main, walks the commits with `git log base..head` and rejects anything that doesn't start with a recognised Conventional Commits type. Pure shell, no Node, no extra dependencies. docs/architecture/contributing.md gains: - A "Commit style — Conventional Commits" table mapping each allowed type to its CHANGELOG section, with `!` / "BREAKING CHANGE:" semantics and examples. - A "Releasing" section with the runbook (install git-cliff, run `./scripts/release.sh vX.Y.Z`, then `git push --follow-tags`). - The PR section now lists five jobs (lint / unit / integration / bench-diff / commit-lint). --- .github/workflows/commit-lint.yml | 57 ++++++ .github/workflows/release.yml | 32 ++++ CHANGELOG.md | 285 ++++++++++++++++++++++++++++++ cliff.toml | 56 ++++++ docs/architecture/contributing.md | 90 +++++++++- scripts/release.sh | 86 +++++++++ 6 files changed, 598 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/commit-lint.yml create mode 100644 .github/workflows/release.yml create mode 100644 CHANGELOG.md create mode 100644 cliff.toml create mode 100755 scripts/release.sh diff --git a/.github/workflows/commit-lint.yml b/.github/workflows/commit-lint.yml new file mode 100644 index 0000000..6943e48 --- /dev/null +++ b/.github/workflows/commit-lint.yml @@ -0,0 +1,57 @@ +name: Commit lint + +on: + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + conventional-commits: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Validate Conventional Commits style + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + + echo "Checking commits ${BASE_SHA}..${HEAD_SHA}" + # Allowed types — keep aligned with cliff.toml. + allowed='(feat|fix|perf|refactor|docs|test|ci|build|chore|src|style|revert)' + # Conventional Commits header: + # type(optional-scope)!: subject (>= 3 chars) + # The "!" marks a breaking change. + regex="^${allowed}(\([^)]+\))?(!)?: .{3,}" + + bad_commits=() + while IFS= read -r commit; do + sha="${commit%% *}" + subject="${commit#* }" + if [[ ! "$subject" =~ $regex ]]; then + bad_commits+=("$sha — $subject") + fi + done < <(git log --no-merges --pretty=format:'%H %s' "$BASE_SHA".."$HEAD_SHA") + + if (( ${#bad_commits[@]} > 0 )); then + echo + echo "::error::The following commits do not follow Conventional Commits:" + for c in "${bad_commits[@]}"; do + echo "::error:: $c" + done + echo + echo "Expected format: ()?(!)?: " + echo "Allowed types: feat, fix, perf, refactor, docs, test, ci, build, chore, src, style, revert" + echo "Examples:" + echo " feat: add LeftJoinOn helper" + echo " fix(executor): skip nil tracer" + echo " refactor!: rename Repository.Tx to WithTx" + exit 1 + fi + + echo "All commits follow Conventional Commits." diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..e17540f --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,32 @@ +name: Release + +on: + push: + tags: + - 'v*' + +permissions: + contents: write + +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Generate release notes + id: cliff + uses: orhun/git-cliff-action@v4 + with: + config: cliff.toml + args: --latest --strip header + env: + OUTPUT: RELEASE_NOTES.md + - name: Create GitHub Release + uses: softprops/action-gh-release@v2 + with: + name: ${{ github.ref_name }} + body_path: ${{ steps.cliff.outputs.changelog }} + draft: false + prerelease: ${{ contains(github.ref_name, '-') }} diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d53464e --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,285 @@ +# Changelog + +All notable changes to this project are documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +Commits follow [Conventional Commits](https://www.conventionalcommits.org/). + +## [Unreleased] + +### Bug Fixes + +- Detect soft-delete value type mismatch at Build time (18e2251) + +### CI / Build + +- Add golangci-lint v2 with baseline config and fix existing findings (1edc8e1) +- Bump Go to 1.24 across go.mod, CI matrix and docs (babf3dd) +- Switch bench-diff and integration jobs to GitHub Actions (9fb2208) +- Post per-MR benchmark diff comment using benchstat (91ca0c5) +- **deps:** Bump golang.org/x/crypto from 0.31.0 to 0.45.0 (a4193e2) + +### Documentation + +- Add runnable examples for godoc / pkg.go.dev (f3a4a0d) +- Use simple/go icon instead of fontawesome/brands/go (2e1fcbd) +- Pin MkDocs deps in docs/requirements.txt (6b93b6e) +- Bootstrap MkDocs Material site with Features and Architecture (c280449) + +### Features + +- Add tracer hook for executor operations (7f24176) +- Add LeftJoinOn/InnerJoinOn with bound parameters (157fe59) + +### Misc + +- Fix "commited" typo in tx wrappers, add unit tests (eb605de) +- Drop unused private sql() helpers in sqlstmt (504ff4f) + +### Performance + +- Replace closures in query/linq builders with structured ops (9b6bf80) +- Reduce heap allocations on read hot path (5421d22) + +### Refactor + +- Factor query helpers around small composable interfaces (6d4b507) +- Extract shared placeholder-rewriting adapter base (b95f4f1) + +### Tests + +- Pin JOIN/WHERE/Count argument ordering after LeftJoinOn (7f737bb) +- Add TestCompareDirectVsGerpo that prints a mock-bench summary table (6d1383c) +- Cover hooks, soft delete, virtual columns, transactions, cache, error transformer; fix pgx tx state bug (069c92a) +- Add query-layer integration tests and fix 3 bugs they uncovered (d98a89d) +- Add CRUD integration tests covering GetFirst/GetList/Count/Insert/Update/Delete (3fd13ee) +- Add integration test harness with docker-compose and per-adapter matrix (528c4be) +## [0.9.1] - 2025-12-06 + +### Misc + +- Repository && executor: tx creates without return error (7ff9446) +## [0.9.0] - 2025-07-06 + +### Misc + +- Builder && executor: add ability to set cache storage engine (60d2d98) +## [0.8.9] - 2025-05-23 + +### Bug Fixes + +- Src: executor: adapters: pgx5: fix package name (542a10f) + +### CI / Build + +- Exclude adapters from code coverage tests result (202fdd8) +- **deps:** Bump golang.org/x/crypto from 0.22.0 to 0.35.0 (f860213) + +### Documentation + +- README.md: add performance metrics gerpo vs pure pgx v4 pool (57e768c) +- Update README.md, add ideology, add release road map, add documentation title, restructure features (1968852) +- Add executor adapters readme and executor readme with sequence scheme (d624c52) +- Add go 1.21 minimal version to readme (11eb428) +- Update README.md (833d33d) + +### Misc + +- Sqlstmt: sqlpart: where: add gte and lte support for time type (39fdfa8) +- Executor: adapters: add pgx v5 adapter (a38b8ba) +- Sqlstmt: use string builder for sql queries generation (e6de7df) +- Query: CT, NCT, BW, NBW. EW, NEW now case sensitive by default, insensitive option was added to methods (6ba85db) +- Sqlstmt: sqlpart: where: use concat instead `||` in sql filters (f85b191) +- Reorganize types package (bd27881) +- Exclude panics, return errors instead (0447f7b) +- Executor: add ability to set placeceholder for databasesql adapter (0c2f47c) +- Query: add Only method for columns select (2fa173a) +## [0.8.4] - 2025-03-25 + +### Misc + +- Sqlstmt: sqlpart: where: NIN and IN fix with nil or empty slices (4d6828b) +## [0.8.3] - 2025-03-20 + +### Misc + +- Downgrade crypto to support go 1.18 (4425ddf) +## [0.8.2] - 2025-03-20 + +### Misc + +- Repostory: use executor.ErrNoRows as gerpo.ErrNotFound (d07e6cd) +- Copy slices go package to local repo for compatible with go 1.18 (8d4a750) +## [0.8.1] - 2025-03-20 + +### Misc + +- Types: column: don't use slice package (1866ac4) +## [0.8.0] - 2025-03-20 + +### Bug Fixes + +- Executor: add rows close on count (8d10655) +- Columns storage in query and sqlstmt packages (1e7099d) + +### CI / Build + +- **deps:** Bump golang.org/x/crypto from 0.20.0 to 0.31.0 (9c76a8f) + +### Features + +- Add INNER JOIN support (b3396d1) +- Add db adapters abstraction level for support pgxv4 and any other sql drivers/libs (6e04c45) + +### Misc + +- Add tests package with basic usage use cases of repository with mocksql driver, for functional regress testing (341d9fc) +- Builder: add With prefix for all builder options (a2c815e) +- Now update return count of updated rows (68082d9) +- Replace cache package inside executor package (c708520) +- Executor: tx: adds rollback unless commit function for use with defer (2927499) +- Add comments for a lot of public methods and functions (666a5e9) +- Executor: rename cache source option (a721963) +- Columns builder: change field column type choose (virtual/column) (218fe4b) +- Cache: remove cache bundle interface, use basic Source interface instead (efd1f79) +- Add a lot of tests (424c06b) +- Refactoring: a lot of refactoring sql and query packages, remove soft deletion (ed196b5) + +### Go + +- Downgrade go to 1.18 (40c5e61) + +### Readme + +- Fix AsColumn method name in configuration examples (5d06586) +## [0.1.9] - 2024-11-27 + +### Core + +- Fix join position in select sql builder (2459c66) +## [0.1.8] - 2024-11-08 + +### CI / Build + +- Add build and tests with coverate on PR (cc912a6) + +### Misc + +- Repository: add missed error wrapping when delete method calls with zero deleted elements (8595c51) +- Add transactions support (6e067ba) +- Exclude repository test in auto tests run (fb0da7e) +- Refactoring cache and sql packages, add new executor and logger pkgs (2e246b6) + +### Readme + +- Fix typos in badges (121e359) +## [0.1.7] - 2024-11-01 + +### Sql + +- Allow to use slices in where IN and NIN filters (a2b1cf0) +## [0.1.6] - 2024-10-29 + +### Misc + +- Cache: ctx: disable and enable cache key func renaming (de1fe3f) +- Api: removed: this sample example is not needed in public, i don't wont support this all time (1dc8407) + +### Sql + +- Added test (0cf48e4) +## [0.1.4] - 2024-10-23 + +### Misc + +- Repo: add error transformer func for wrapping errors to needed bussines type (be1fdab) +- Api: remove empty filters file (8082448) +## [0.1.3] - 2024-10-23 + +### Misc + +- Api: sorts: init available sorts at column link to dto (ca25362) +- Repo: add new after insert and after update hooks (7ab058f) +## [0.1.2] - 2024-10-22 + +### Api + +- Noop: removed (35ae6cb) +## [0.1.1] - 2024-10-21 + +### Api + +- Join core and applier to core and add real example usage (31ad55d) +## [0.1.0] - 2024-10-21 + +### Misc + +- Remove tmp file (a83a6db) +- Api: add example query integration with filters and sorts (759d058) +- Cleanup options and builder (remove unused and not implemented) (7786197) +- Query: compact query helpers to bundle encapsulate query calls (a8fdd90) +- Column: allow group sql action by default (2ed14a3) +- Query: make get first helper interface without depends to count helper interface (c8ae97e) +- Add persistent query to repository configuration (13afbac) + +### Cache + +- Ctx: rename ctx key and add Ctx prefix in function names (7f6bf5b) + +### Query + +- Linq: join: fix joins with empty string join (8702c8b) +- Linq: extends api to using with external tools (ed58827) +- Linq: exclude remove old not used code (b6bc62c) + +### Sql + +- Select: fix columns exclude (deleteFunc) (c44e17a) +- Query: fix in and nin operators (7c8e490) +- Executor: determine placeholder inside ctor (411a4ae) + +### Types + +- Columns: use add without fmap.Field (a5dc4f6) +## [0.0.9] - 2024-10-15 + +### Query + +- Use update sql builder when we update entity (f360d32) +## [0.0.6] - 2024-10-15 + +### Hack + +- Src: sql: always use postgres placeholder (0a6d2d9) +## [0.0.5] - 2024-10-15 + +### Misc + +- Sql: placeholder: workaround otelsql connector (4d0c0d8) +## [0.0.4] - 2024-10-15 + +### Sql + +- Placeholder: determine place holder with otelsql wrapper (3339252) +## [0.0.3] - 2024-10-15 + +### Misc + +- Sql: fix placeholder determination for lib/pq (d52714e) +## [0.0.2] - 2024-10-11 + +### Misc + +- Add Repository interface (d1e52af) +- Add specific user helper for each repository method (a4ab5d4) +## [0.0.1] - 2024-10-10 + +### Misc + +- Make repository global (72da700) + +### Query + +- Add uuid support (08fbe60) + diff --git a/cliff.toml b/cliff.toml new file mode 100644 index 0000000..585c9f6 --- /dev/null +++ b/cliff.toml @@ -0,0 +1,56 @@ +# git-cliff configuration for gerpo. +# See https://git-cliff.org/docs/configuration for the full reference. + +[changelog] +header = """ +# Changelog + +All notable changes to this project are documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +Commits follow [Conventional Commits](https://www.conventionalcommits.org/). + +""" +body = """ +{% if version %}\ +## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }} +{% else %}\ +## [Unreleased] +{% endif %}\ +{% for group, commits in commits | group_by(attribute="group") %} +### {{ group | upper_first }} +{% for commit in commits %} +- {% if commit.scope %}**{{ commit.scope }}:** {% endif %}{{ commit.message | upper_first }} ({{ commit.id | truncate(length=7, end="") }}) +{%- endfor %} +{% endfor %} +""" +trim = true +footer = "" + +[git] +conventional_commits = true +filter_unconventional = true +split_commits = false +commit_parsers = [ + { message = "^feat", group = "Features" }, + { message = "^fix", group = "Bug Fixes" }, + { message = "^perf", group = "Performance" }, + { message = "^refactor", group = "Refactor" }, + { message = "^docs", group = "Documentation" }, + { message = "^test", group = "Tests" }, + { message = "^ci", group = "CI / Build" }, + { message = "^build", group = "CI / Build" }, + { message = "^chore", group = "Misc" }, + { message = "^src", group = "Misc" }, + { message = "^style", skip = true }, + { message = "^revert", group = "Reverts" }, + { body = ".*[Bb]reaking [Cc]hange", group = "BREAKING CHANGES" }, +] +protect_breaking_commits = true +filter_commits = false +tag_pattern = "v[0-9]*" +skip_tags = "" +ignore_tags = "" +topo_order = false +sort_commits = "newest" diff --git a/docs/architecture/contributing.md b/docs/architecture/contributing.md index 789e404..e3de21d 100644 --- a/docs/architecture/contributing.md +++ b/docs/architecture/contributing.md @@ -41,28 +41,61 @@ GERPO_BENCH_REPORT=1 go test -run=TestCompareDirectVsGerpo -v ./tests/ - Integration tests go under `tests/integration/` with the `//go:build integration` tag. They target every adapter in a single run through `forEachAdapter`. - Benchmarks live in `tests/` (no build tag — `go test -bench=`). -## Commit style +## Commit style — Conventional Commits -Follow the existing log: lowercase type prefix, imperative subject, optional body with bullet points: +The repo uses [Conventional Commits](https://www.conventionalcommits.org/). +Format: ``` -perf: replace closures in query/linq builders with structured ops +()?(!)?: -Where/Order/Exclude/Group/Join no longer store per-condition closures … + + + | ``` -Common types used in the repo: `feat:`, `fix:`, `perf:`, `test:`, `docs:`, `ci:`, `build:`, `refactor:`, `src:`. +The `commit-lint` workflow validates every commit on a PR — anything that +doesn't start with one of the allowed types is rejected. + +Allowed types and what they mean: + +| Type | Use for | CHANGELOG section | +|-------------|------------------------------------------------------|-------------------| +| `feat:` | new public API or capability | Features | +| `fix:` | bug fix | Bug Fixes | +| `perf:` | performance improvement without behavior change | Performance | +| `refactor:` | code change with no behavior change | Refactor | +| `docs:` | documentation only | Documentation | +| `test:` | test-only change | Tests | +| `ci:` | CI / pipelines | CI / Build | +| `build:` | dependencies, build files | CI / Build | +| `chore:` | tooling, repo housekeeping, formatting | Misc | +| `src:` | low-level repo-internal change without other prefix | Misc | +| `revert:` | git revert | Reverts | +| `style:` | whitespace / formatting only (skipped in CHANGELOG) | — | + +Add `!` after the type or include `BREAKING CHANGE:` in the body to mark a +breaking change — it'll bubble up under "BREAKING CHANGES" in the CHANGELOG. + +Examples: + +``` +feat: add LeftJoinOn helper for parameter-bound JOINs +fix(executor): skip nil tracer +refactor!: rename Repository.Tx to WithTx +``` ## Opening a PR -A PR to `main` runs four jobs: +A PR to `main` runs five jobs: - `lint` — `golangci-lint run ./...` with the config in `.golangci.yml`. - `unit` — build, race detector, full `go test`. - `integration` — `//go:build integration` against a PG service container. - `bench-diff` — runs mock benchmarks on head and on base, posts a [benchstat](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) summary as a PR comment. +- `commit-lint` — validates every commit message against Conventional Commits. -`bench-diff` is `allow_failure: true` — a perf regression shows up in the comment but doesn't block merging on its own. Look at the comment before asking for review. +`bench-diff` is `allow_failure: true` — a perf regression shows up in the comment but doesn't block merging on its own. ## Updating the docs @@ -72,4 +105,45 @@ A PR to `main` runs four jobs: ## Releasing -Tag `vX.Y.Z`, push the tag. Prior to 1.0.0 the API is not guaranteed to be stable — call out anything breaking in the release notes. +The release flow is semi-automated: you regenerate `CHANGELOG.md` and tag +locally, the `release` workflow builds the GitHub Release notes from the +same `cliff.toml` config so both stay in sync. + +Prerequisites (one time): + +```bash +# git-cliff is the markdown generator. Pick one: +brew install git-cliff +# or: +cargo install git-cliff +# or download a binary: https://github.com/orhun/git-cliff/releases +``` + +Per release: + +```bash +./scripts/release.sh v0.2.0 +``` + +The script + +1. refuses to run unless you're on a clean `main` and the tag does not exist; +2. fast-forwards `main` from `origin`; +3. regenerates `CHANGELOG.md` with the new tag at the top; +4. shows the diff so you can eyeball or edit the file; +5. on confirmation, commits the file and creates the annotated tag. + +It deliberately does **not** push. Review with `git show v0.2.0`, then: + +```bash +git push --follow-tags origin main +``` + +That push triggers `.github/workflows/release.yml`, which runs +`git cliff --latest --strip header` against the same config and creates a +GitHub Release with that excerpt as the body. Tags carrying a suffix +(`v1.0.0-rc1`) are marked as pre-releases automatically. + +Pre-1.0.0 the API is not guaranteed to be stable — call breaking changes out +explicitly with `!` in the type or `BREAKING CHANGE:` in the body so they +appear in their own section of the CHANGELOG. diff --git a/scripts/release.sh b/scripts/release.sh new file mode 100755 index 0000000..e9ef402 --- /dev/null +++ b/scripts/release.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# +# release.sh — prepare a tagged release locally. +# +# Usage: ./scripts/release.sh vX.Y.Z[-suffix] +# +# Steps the script performs: +# 1. Refuse to run on a branch other than main, on a dirty tree, or with an +# already-existing tag. +# 2. git pull --ff-only origin main. +# 3. git cliff --tag -o CHANGELOG.md (regenerates the file with the +# new tag at the top). +# 4. Show the diff so you can eyeball or even edit the file. +# 5. After confirmation, commit CHANGELOG.md and create an annotated tag. +# +# The push step is intentionally NOT here — review your local commit and tag, +# then run `git push --follow-tags origin main` yourself. + +set -euo pipefail + +if [[ $# -ne 1 ]]; then + echo "Usage: $0 vX.Y.Z[-suffix]" >&2 + exit 1 +fi +TAG="$1" + +if ! [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[A-Za-z0-9.]+)?$ ]]; then + echo "Tag must match vX.Y.Z or vX.Y.Z-suffix (e.g. v0.1.0, v1.0.0-rc1). Got: $TAG" >&2 + exit 1 +fi + +need() { command -v "$1" >/dev/null 2>&1 || { echo "missing tool: $1" >&2; exit 1; }; } +need git +need git-cliff + +branch=$(git rev-parse --abbrev-ref HEAD) +if [[ "$branch" != "main" ]]; then + echo "Must be on main, currently on $branch" >&2 + exit 1 +fi + +if [[ -n "$(git status --porcelain)" ]]; then + echo "Working tree is dirty — commit or stash before releasing." >&2 + git status --short >&2 + exit 1 +fi + +if git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + echo "Tag $TAG already exists locally." >&2 + exit 1 +fi + +if git ls-remote --tags --exit-code origin "$TAG" >/dev/null 2>&1; then + echo "Tag $TAG already exists on origin." >&2 + exit 1 +fi + +echo "▶ Fetching latest main…" +git pull --ff-only origin main + +echo "▶ Generating CHANGELOG.md for $TAG…" +git cliff --tag "$TAG" -o CHANGELOG.md + +echo +echo "==================== CHANGELOG.md diff ====================" +git --no-pager diff CHANGELOG.md +echo "===========================================================" +echo +echo "If you want to edit the file (typos, regrouping, etc.) — do it now," +echo "then re-run with the same tag, or proceed and amend later." +echo + +read -r -p "Commit CHANGELOG.md and tag $TAG? [y/N] " response +if [[ ! "$response" =~ ^[Yy]$ ]]; then + echo "Aborted. CHANGELOG.md changes left in the working tree." + exit 0 +fi + +git add CHANGELOG.md +git commit -m "docs: prepare changelog for $TAG" +git tag -a "$TAG" -m "$TAG" + +echo +echo "✓ Tag $TAG created locally on $(git rev-parse --short HEAD)." +echo "Review with: git show $TAG" +echo "Push when ready: git push --follow-tags origin main" From 2779502de89b8ff5561c82ae34701c87bbb6737d Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 06:05:00 +0300 Subject: [PATCH 13/15] chore: add Makefile with common project commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collects the commands that used to be spread across README, CLAUDE.md and docs/architecture/contributing.md into a single Makefile with self-documenting help. Everything underneath still works unchanged — this is a convenience layer. Targets: - build go build -v ./... - test go test -race ./... - lint golangci-lint run ./... - integration-up docker compose up -d (tests/integration) - integration-down docker compose down - integration go test -tags=integration ./tests/integration/... - integration-full up → run → down in one go - bench Direct vs Gerpo mock benchmarks (5 runs) - bench-report formatted summary via TestCompareDirectVsGerpo - docs-serve mkdocs serve - docs-build mkdocs build --strict - release ./scripts/release.sh $(TAG) - help (default goal) lists the above with docstrings `make help` reads the ## comments directly from the Makefile so new targets show up automatically when added. INTEGRATION_DSN is overridable (default: the compose defaults for the local container). docs/architecture/contributing.md now points at the Makefile in the check-in loop and the release runbook instead of duplicating the go/docker commands. --- Makefile | 66 +++++++++++++++++++++++++++++++ docs/architecture/contributing.md | 43 +++++++++++++------- 2 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..4469a12 --- /dev/null +++ b/Makefile @@ -0,0 +1,66 @@ +.DEFAULT_GOAL := help + +# Connection string used by integration tests. Override on the command line if +# your local Postgres is on a different port / user. +INTEGRATION_DSN ?= postgres://gerpo:gerpo@localhost:5433/gerpo?sslmode=disable + +# Mock-adapter benchmarks that compare a raw DBAdapter call to gerpo.Repository. +BENCH_PATTERN := ^Benchmark(GetFirst|GetList|Count|Insert|Update|Delete)_(Direct|Gerpo)$$ + +COMPOSE := docker compose -f tests/integration/docker-compose.yml + +.PHONY: help +help: ## Show this message + @awk 'BEGIN {FS = ":.*?## "} \ + /^[a-zA-Z_-]+:.*?## / {printf " \033[36m%-18s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) + +.PHONY: build +build: ## Compile all packages + go build -v ./... + +.PHONY: test +test: ## Unit tests with the race detector + go test -race ./... + +.PHONY: lint +lint: ## Run golangci-lint v2 + golangci-lint run ./... + +.PHONY: integration-up +integration-up: ## Start the PostgreSQL container used by integration tests + $(COMPOSE) up -d + +.PHONY: integration-down +integration-down: ## Stop the PostgreSQL container + $(COMPOSE) down + +.PHONY: integration +integration: ## Run integration tests (requires integration-up or external PG via INTEGRATION_DSN) + GERPO_INTEGRATION_DB_URL="$(INTEGRATION_DSN)" go test -tags=integration ./tests/integration/... + +.PHONY: integration-full +integration-full: integration-up integration integration-down ## One-shot: bring PG up, run the suite, tear PG down + +.PHONY: bench +bench: ## Run Direct vs Gerpo mock benchmarks (5 runs) + go test -bench='$(BENCH_PATTERN)' -benchmem -run=^$$ -count=5 ./tests/ + +.PHONY: bench-report +bench-report: ## Print the Direct vs Gerpo summary table + GERPO_BENCH_REPORT=1 go test -run=TestCompareDirectVsGerpo -v ./tests/ + +.PHONY: docs-serve +docs-serve: ## Preview the MkDocs site at http://127.0.0.1:8000 + mkdocs serve + +.PHONY: docs-build +docs-build: ## Build the MkDocs site with --strict + mkdocs build --strict + +.PHONY: release +release: ## Prepare a release: regenerate CHANGELOG + commit + tag. Usage: make release TAG=vX.Y.Z + @if [ -z "$(TAG)" ]; then \ + echo "Usage: make release TAG=vX.Y.Z" >&2; \ + exit 2; \ + fi + ./scripts/release.sh $(TAG) diff --git a/docs/architecture/contributing.md b/docs/architecture/contributing.md index e3de21d..e293e2d 100644 --- a/docs/architecture/contributing.md +++ b/docs/architecture/contributing.md @@ -8,26 +8,41 @@ ## Check-in loop +Common tasks live in a `Makefile` — run `make help` for the catalog. + +```bash +make lint # golangci-lint v2 +make test # go test -race ./... +make integration-full # docker up → integration tests → docker down +make bench # Direct vs Gerpo mock benchmarks (5 runs) +make bench-report # formatted summary table (~20s) +``` + +If you want finer control over the integration suite: + ```bash -# Lint (golangci-lint v2) -golangci-lint run ./... +make integration-up # start Postgres once +make integration # run /tests/integration/ against the running PG +make integration-down # stop Postgres +``` -# Unit tests + race detector -go test -race ./... +Override the DSN if your local PG differs: -# Integration tests (PostgreSQL in Docker) -docker compose -f tests/integration/docker-compose.yml up -d -GERPO_INTEGRATION_DB_URL="postgres://gerpo:gerpo@localhost:5433/gerpo?sslmode=disable" \ - go test -tags=integration ./tests/integration/... +```bash +make integration INTEGRATION_DSN="postgres://..." +``` -# Direct-vs-gerpo allocation benchmarks -go test -bench='^Benchmark(GetFirst|GetList|Count|Insert|Update|Delete)_(Direct|Gerpo)$' \ - -benchmem -run=^$ -count=5 ./tests/ +To preview the MkDocs site: -# Formatted summary -GERPO_BENCH_REPORT=1 go test -run=TestCompareDirectVsGerpo -v ./tests/ +```bash +make docs-serve # http://127.0.0.1:8000 +make docs-build # build with --strict ``` +You can of course still call the underlying `go test` / `docker compose` / +`golangci-lint` commands directly; the Makefile is a convenience layer, not +a requirement. + ## Code style - Package names are lowercase and short. @@ -122,7 +137,7 @@ cargo install git-cliff Per release: ```bash -./scripts/release.sh v0.2.0 +make release TAG=v0.2.0 # wraps scripts/release.sh ``` The script From fdadead6b2eff2e6a97c981ffa71bd8e55353faa Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 06:14:59 +0300 Subject: [PATCH 14/15] docs: add "Why gerpo?" comparison page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new docs page that situates gerpo against the rest of the Go data access ecosystem so visitors do not have to reverse-engineer the niche from the README. Contents: - 30-second pitch and "when to pick" / "when not to pick" sections. - Feature matrix vs GORM, ent, bun, sqlc and sqlx (approach, schema source, type safety, migrations, relations, struct tags, drivers, caching, tracing, hooks, raw SQL escape hatch, line-count, reflection footprint). - Strengths and weaknesses written without marketing — calls out the pre-1.0 surface, deprecated virtual-column API, generic boilerplate and the per-call allocation overhead. - Performance section with the headline numbers from TestCompareDirectVsGerpo and the standard "absorbed by network IO" caveat. - "Closest alternatives — when each fits better" table to tell readers to use a different library when that is the right answer. Wiring: - mkdocs.yml: new top-level entry between "Get started" and "Features". - docs/index.md: a Why-gerpo card slotted into the landing-page grid. - README.md: the docs link in the badge row gains a direct link to the page. --- README.md | 2 +- docs/index.md | 4 ++ docs/why-gerpo.md | 100 ++++++++++++++++++++++++++++++++++++++++++++++ mkdocs.yml | 1 + 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 docs/why-gerpo.md diff --git a/README.md b/README.md index 865317b..c9ef759 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ **GERPO** (Golang + Repository) is a generic repository pattern for Go with pluggable adapters and a tiny footprint. It is **not an ORM** — no migrations, no relations, no struct tags. All SQL behavior is declared once in the repository configuration; columns are bound to struct fields through pointers. -> 📚 Full documentation: **[insei.github.io/gerpo](https://insei.github.io/gerpo/)** · API reference: **[pkg.go.dev/github.com/insei/gerpo](https://pkg.go.dev/github.com/insei/gerpo)** +> 📚 Full documentation: **[insei.github.io/gerpo](https://insei.github.io/gerpo/)** · [Why gerpo?](https://insei.github.io/gerpo/why-gerpo/) (vs GORM / ent / bun / sqlc / sqlx) · API reference: **[pkg.go.dev/github.com/insei/gerpo](https://pkg.go.dev/github.com/insei/gerpo)** ## Install diff --git a/docs/index.md b/docs/index.md index 21d2660..724b728 100644 --- a/docs/index.md +++ b/docs/index.md @@ -77,6 +77,10 @@ Full runnable samples live in [`examples/`](https://github.com/Insei/gerpo/tree/
+- :material-help-circle-outline:{ .lg } **[Why gerpo? →](why-gerpo.md)** + + How gerpo compares to GORM, ent, bun, sqlc and sqlx — feature matrix, strengths, weaknesses, and when each tool is the better pick. + - :material-book-open-variant:{ .lg } **[Features →](features/index.md)** A walkthrough of every repository capability: CRUD, filters, ordering, hooks, soft delete, virtual columns, cache, transactions, adapters. diff --git a/docs/why-gerpo.md b/docs/why-gerpo.md new file mode 100644 index 0000000..13819c6 --- /dev/null +++ b/docs/why-gerpo.md @@ -0,0 +1,100 @@ +# Why gerpo? + +Go has a healthy data-access ecosystem — full-blown ORMs, code generators, query builders, thin wrappers. gerpo occupies a specific niche: a **type-safe repository pattern with pluggable SQL adapters and no schema management**. This page lays out where it fits, what it gives up, and how it compares to the closest alternatives. + +## The 30-second pitch + +- One **declarative configuration** per entity wires struct fields to columns through pointers — `c.Field(&m.Email).AsColumn()` — so renames are a refactor, not a search-and-replace through string tags. +- Six methods per repository (`GetFirst`, `GetList`, `Count`, `Insert`, `Update`, `Delete`) cover the everyday CRUD; everything else (joins, soft-delete, virtual columns, hooks, caching, tracing) is opt-in. +- Three driver adapters (`pgx5`, `pgx4`, `database/sql`) all sit behind a 3-method `DBAdapter` interface — bring your own driver in ~50 lines. +- **Not** an ORM. No migrations, no relations, no struct tags. Schema management is your problem (`golang-migrate`, `goose`, `atlas`, …). + +## When to pick gerpo + +Pick gerpo when you want: + +- A clear, type-safe boundary between business code and SQL. +- Predictable allocations and SQL generation — `make bench-report` shows the overhead per operation. +- Multiple drivers behind one interface (microservices on PostgreSQL today, easy to add ClickHouse / SQLite tomorrow). +- Per-request caching that just turns on (`CtxCache`). +- An OpenTelemetry-style tracing hook without forcing OTel as a dependency. +- A small, readable codebase you can fork or wrap. + +## When **not** to pick gerpo + +Skip it if: + +- You want migrations bundled with your data layer — pick **GORM** or **ent** instead. +- You want navigation properties / lazy loading (`user.Posts`, `post.Comments`) — `gerpo` deliberately doesn't provide them. +- Your team already runs on raw SQL and wants compile-time-checked queries from `.sql` files — pick **sqlc**. +- You only need a thin marshalling layer over `database/sql` — pick **sqlx**. +- You can't tolerate an API that is still pre-1.0 — gerpo is on the road to 1.0 with a stated stable subset, but the deprecated virtual-column API is still in flux. + +## Feature matrix + +| | gerpo | [GORM](https://gorm.io/) | [ent](https://entgo.io/) | [bun](https://bun.uptrace.dev/) | [sqlc](https://sqlc.dev/) | [sqlx](https://github.com/jmoiron/sqlx) | +|---|---|---|---|---|---|---| +| **Approach** | Repository + SQL config | Active Record / ORM | Schema DSL + codegen | ORM-lite | SQL-first codegen | `database/sql` wrapper | +| **Schema source** | Go config (pointers) | Struct tags | Go DSL | Struct tags | `.sql` files | None | +| **Type-safe queries** | ✓ (generics) | partial | ✓ | partial | ✓ | ✗ | +| **Code generation** | ✗ | ✗ | ✓ | ✗ | ✓ | ✗ | +| **Migrations** | ✗ (external) | ✓ | ✓ | ✓ | ✗ | ✗ | +| **Relations / navigation** | ✗ | ✓ | ✓ | ✓ | ✗ | ✗ | +| **Struct tags required** | ✗ | ✓ | ✗ | ✓ | ✗ | ✓ | +| **Pluggable drivers** | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | +| **Soft delete built-in** | ✓ | ✓ | ✓ | ✓ | ✗ | ✗ | +| **Per-request cache** | ✓ (`CtxCache`) | plugin | ✗ | ✗ | ✗ | ✗ | +| **Tracing hook** | ✓ | plugin | hook | hook | ✗ | ✗ | +| **Hooks (Before/After)** | ✓ | ✓ | ✓ | ✓ | ✗ | ✗ | +| **Custom SQL escape hatch** | ✓ (callbacks) | ✓ (`Raw()`) | ✓ | ✓ | n/a (everything is SQL) | ✓ | +| **Lines of code** | ~3k | ~50k | ~80k | ~30k | n/a | ~3k | +| **Reflection** | only at config (fmap + unsafe offsets) | runtime, every call | none (generated code) | runtime | none | minimal | + +The "Lines of code" row is rough but conveys the shape: gerpo is closer to sqlx in size and to ent in API ergonomics. + +## Strengths + +- **Pointer-based mapping is refactor-proof.** Rename a field — the compiler tells you everywhere it's wired into a column. Tag-based schemas only break at runtime. +- **No surprise SQL.** Every JOIN, GROUP BY, virtual column and persistent filter is in one `WithQuery(...)` block per repository. There is no hidden auto-load that spawns a second query behind your back. +- **Three adapters, one base.** Driver-specific code is a `Backend{Exec,Query,BeginTx}` + `TxBackend{…}` pair (a few dozen lines). The placeholder rewrite, the transaction state machine and `RollbackUnlessCommitted` semantics live once in `executor/adapters/internal`. +- **Cache and tracing are first-class but opt-in.** `WithCacheStorage` and `WithTracer` take small interfaces — implement them with whatever your stack already uses (Redis, OTel, Datadog, …) without dragging dependencies into gerpo. +- **Pre-1.0 already battle-tested in CI.** Every PR runs lint, race-detector unit tests, integration tests against a real PostgreSQL service container on three drivers, and a [benchstat](https://pkg.go.dev/golang.org/x/perf/cmd/benchstat) overhead diff. See [Contributing](architecture/contributing.md). + +## Weaknesses + +- **No migrations.** You will need a separate tool. We recommend `golang-migrate`, `goose`, or `atlas`. +- **No relations.** Many-to-one / one-to-many fan-outs are explicit calls — write a `FindPostsByUser(ctx, userID)` rather than `user.Posts`. +- **Pre-1.0 API surface.** The virtual-column configuration API is marked deprecated and will be replaced in 1.0.0. The rest of the API is stable per the README roadmap. +- **Generic boilerplate.** Every repository carries a `[TModel any]` parameter. With many entities you end up with many small `gerpo.Repository[Foo]` typed values. This is the price of compile-time safety. +- **Reflection footprint.** `fmap` walks struct layout once at builder time using `unsafe.Offsetof`; the per-request hot path is pointer arithmetic, not reflection. But: pointer-based field resolution adds ~12 allocations per `GetFirst` over a raw `pgx` call (≈+1 µs). In a real query that figure is noise; in a tight benchmark loop it shows up. + +## Performance + +`make bench-report` runs every CRUD operation twice — once against a mock backend directly, once through gerpo — and prints a comparison table. Headline numbers (post-optimisation, mock backend): + +| Op | Direct ns/op | Gerpo ns/op | Direct allocs | Gerpo allocs | +|---|---:|---:|---:|---:| +| GetFirst | ~200 | ~1300 | 5 | 17 | +| GetList (10 rows) | ~1100 | ~3000 | 21 | 34 | +| Count | ~100 | ~750 | 4 | 10 | +| Insert | ~140 | ~700 | 5 | 14 | +| Update | ~75 | ~1400 | 3 | 23 | +| Delete | ~70 | ~870 | 3 | 17 | + +Those ratios look scary in isolation. **In a real database round-trip** (50 µs locally, 500 µs over the network) gerpo's ~1 µs overhead is 0.2–2% — the README's "+8% ns/op" measurement against a real `pgx v4` pool matches that ballpark. The allocation numbers matter more for GC pressure under high RPS than for raw latency. + +## Closest alternatives — when each fits better + +- **GORM.** You want everything in one box: schema, migrations, relations, hooks. You're fine with the runtime overhead and the occasional surprise from active-record semantics. +- **ent.** Your domain has a deeply connected graph (users → orgs → projects → issues → comments) and you want the compiler to enforce the traversal. You don't mind running a code generator. +- **bun.** You want most of GORM's ergonomics with a smaller surface and explicit relationships. +- **sqlc.** Your team writes SQL by hand and wants compile-time-checked, hand-tuned queries with generated Go signatures. You don't want a query DSL. +- **sqlx.** You want `database/sql` plus row-to-struct marshalling and nothing else. +- **gerpo.** You want the repository pattern as a first-class concept — pointer-based wiring, type-safe per-operation helpers, three adapters, no schema management — and you handle migrations elsewhere. + +## Reading list + +- [Get started](index.md) — install + 30-line example. +- [Features](features/index.md) — every capability with code samples. +- [Architecture](architecture/index.md) — internals for contributors. +- [API reference](https://pkg.go.dev/github.com/insei/gerpo) — runnable godoc examples next to every method. diff --git a/mkdocs.yml b/mkdocs.yml index 7e1b894..a1acf7e 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -69,6 +69,7 @@ extra: nav: - Get started: index.md + - Why gerpo?: why-gerpo.md - Features: - features/index.md - Repository builder: features/repository.md From dfd9883b9cce3adfcd00202129c3bb274c6bb077 Mon Sep 17 00:00:00 2001 From: Dmitrii Aleksandrov Date: Sun, 19 Apr 2026 06:21:04 +0300 Subject: [PATCH 15/15] ci: bump golangci-lint-action to v7 for golangci-lint v2 support The original lint job used golangci/golangci-lint-action@v6, which explicitly rejects golangci-lint v2: Error: invalid version string 'v2.5.0', golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7. Bump the action to v7. The pinned linter version (v2.5.0) and the .golangci.yml config stay unchanged. --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 5f1ab27..a9ae104 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -19,7 +19,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version: '1.24' - - uses: golangci/golangci-lint-action@v6 + - uses: golangci/golangci-lint-action@v7 with: version: v2.5.0