mirror of
https://github.com/zitadel/zitadel.git
synced 2025-12-24 03:36:46 +00:00
refactor: database interaction and error handling (#10762)
This pull request introduces a significant refactoring of the database interaction layer, focusing on improving explicitness, transactional control, and error handling. The core change is the removal of the stateful `QueryExecutor` from repository instances. Instead, it is now passed as an argument to each method that interacts with the database. This change makes transaction management more explicit and flexible, as the same repository instance can be used with a database pool or a specific transaction without needing to be re-instantiated. ### Key Changes - **Explicit `QueryExecutor` Passing:** - All repository methods (`Get`, `List`, `Create`, `Update`, `Delete`, etc.) in `InstanceRepository`, `OrganizationRepository`, `UserRepository`, and their sub-repositories now require a `database.QueryExecutor` (e.g., a `*pgxpool.Pool` or `pgx.Tx`) as the first argument. - Repository constructors no longer accept a `QueryExecutor`. For example, `repository.InstanceRepository(pool)` is now `repository.InstanceRepository()`. - **Enhanced Error Handling:** - A new `database.MissingConditionError` is introduced to enforce required query conditions, such as ensuring an `instance_id` is always present in `UPDATE` and `DELETE` operations. - The database error wrapper in the `postgres` package now correctly identifies and wraps `pgx.ErrTooManyRows` and similar errors from the `scany` library into a `database.MultipleRowsFoundError`. - **Improved Database Conditions:** - The `database.Condition` interface now includes a `ContainsColumn(Column) bool` method. This allows for runtime checks to ensure that critical filters (like `instance_id`) are included in a query, preventing accidental cross-tenant data modification. - A new `database.Exists()` condition has been added to support `EXISTS` subqueries, enabling more complex filtering logic, such as finding an organization that has a specific domain. - **Repository and Interface Refactoring:** - The method for loading related entities (e.g., domains for an organization) has been changed from a boolean flag (`Domains(true)`) to a more explicit, chainable method (`LoadDomains()`). This returns a new repository instance configured to load the sub-resource, promoting immutability. - The custom `OrgIdentifierCondition` has been removed in favor of using the standard `database.Condition` interface, simplifying the API. - **Code Cleanup and Test Updates:** - Unnecessary struct embeddings and metadata have been removed. - All integration and repository tests have been updated to reflect the new method signatures, passing the database pool or transaction object explicitly. - New tests have been added to cover the new `ExistsDomain` functionality and other enhancements. These changes make the data access layer more robust, predictable, and easier to work with, especially in the context of database transactions.
This commit is contained in:
Silvan
304
backend/v3/storage/database/errors_test.go
Normal file
304
backend/v3/storage/database/errors_test.go
Normal file
@@ -0,0 +1,304 @@
|
||||
package database
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestError(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
err error
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "missing condition without column",
|
||||
err: NewMissingConditionError(nil),
|
||||
want: "missing condition for column",
|
||||
},
|
||||
{
|
||||
name: "missing condition with column",
|
||||
err: NewMissingConditionError(NewColumn("table", "column")),
|
||||
want: "missing condition for column on table.column",
|
||||
},
|
||||
{
|
||||
name: "no row found without original error",
|
||||
err: NewNoRowFoundError(nil),
|
||||
want: "no row found",
|
||||
},
|
||||
{
|
||||
name: "no row found with original error",
|
||||
err: NewNoRowFoundError(errors.New("original error")),
|
||||
want: "no row found: original error",
|
||||
},
|
||||
{
|
||||
name: "multiple rows found without original error",
|
||||
err: NewMultipleRowsFoundError(nil),
|
||||
want: "multiple rows found",
|
||||
},
|
||||
{
|
||||
name: "multiple rows found with original error",
|
||||
err: NewMultipleRowsFoundError(errors.New("original error")),
|
||||
want: "multiple rows found: original error",
|
||||
},
|
||||
{
|
||||
name: "check violation without original error",
|
||||
err: NewCheckError("table", "constraint", nil),
|
||||
want: `integrity violation of type "check" on "table" (constraint: "constraint")`,
|
||||
},
|
||||
{
|
||||
name: "check violation with original error",
|
||||
err: NewCheckError("table", "constraint", errors.New("original error")),
|
||||
want: `integrity violation of type "check" on "table" (constraint: "constraint"): original error`,
|
||||
},
|
||||
{
|
||||
name: "unique violation without original error",
|
||||
err: NewUniqueError("table", "constraint", nil),
|
||||
want: `integrity violation of type "unique" on "table" (constraint: "constraint")`,
|
||||
},
|
||||
{
|
||||
name: "unique violation with original error",
|
||||
err: NewUniqueError("table", "constraint", errors.New("original error")),
|
||||
want: `integrity violation of type "unique" on "table" (constraint: "constraint"): original error`,
|
||||
},
|
||||
{
|
||||
name: "foreign key violation without original error",
|
||||
err: NewForeignKeyError("table", "constraint", nil),
|
||||
want: `integrity violation of type "foreign" on "table" (constraint: "constraint")`,
|
||||
},
|
||||
{
|
||||
name: "foreign key violation with original error",
|
||||
err: NewForeignKeyError("table", "constraint", errors.New("original error")),
|
||||
want: `integrity violation of type "foreign" on "table" (constraint: "constraint"): original error`,
|
||||
},
|
||||
{
|
||||
name: "not null violation without original error",
|
||||
err: NewNotNullError("table", "constraint", nil),
|
||||
want: `integrity violation of type "not null" on "table" (constraint: "constraint")`,
|
||||
},
|
||||
{
|
||||
name: "not null violation with original error",
|
||||
err: NewNotNullError("table", "constraint", errors.New("original error")),
|
||||
want: `integrity violation of type "not null" on "table" (constraint: "constraint"): original error`,
|
||||
},
|
||||
{
|
||||
name: "unknown error",
|
||||
err: NewUnknownError(errors.New("original error")),
|
||||
want: `unknown database error: original error`,
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
assert.Equal(t, test.want, test.err.Error())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnwrap(t *testing.T) {
|
||||
originalErr := errors.New("original error")
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
err error
|
||||
want error
|
||||
}{
|
||||
{
|
||||
name: "missing condition without column",
|
||||
err: NewMissingConditionError(nil),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "missing condition with column",
|
||||
err: NewMissingConditionError(NewColumn("table", "column")),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "no row found without original error",
|
||||
err: NewNoRowFoundError(nil),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "no row found with original error",
|
||||
err: NewNoRowFoundError(errors.New("original error")),
|
||||
want: originalErr,
|
||||
},
|
||||
{
|
||||
name: "multiple rows found without original error",
|
||||
err: NewMultipleRowsFoundError(nil),
|
||||
want: nil,
|
||||
},
|
||||
{
|
||||
name: "multiple rows found with original error",
|
||||
err: NewMultipleRowsFoundError(originalErr),
|
||||
want: originalErr,
|
||||
},
|
||||
{
|
||||
name: "check violation without original error",
|
||||
err: NewCheckError("table", "constraint", nil),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeCheck,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: nil,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "check violation with original error",
|
||||
err: NewCheckError("table", "constraint", originalErr),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeCheck,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: originalErr,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "unique violation without original error",
|
||||
err: NewUniqueError("table", "constraint", nil),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeUnique,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: nil,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "unique violation with original error",
|
||||
err: NewUniqueError("table", "constraint", originalErr),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeUnique,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: originalErr,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "foreign key violation without original error",
|
||||
err: NewForeignKeyError("table", "constraint", nil),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeForeign,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: nil,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "foreign key violation with original error",
|
||||
err: NewForeignKeyError("table", "constraint", originalErr),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeForeign,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: originalErr,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "not null violation without original error",
|
||||
err: NewNotNullError("table", "constraint", nil),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeNotNull,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: nil,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "not null violation with original error",
|
||||
err: NewNotNullError("table", "constraint", originalErr),
|
||||
want: &IntegrityViolationError{
|
||||
integrityType: IntegrityTypeNotNull,
|
||||
table: "table",
|
||||
constraint: "constraint",
|
||||
original: originalErr,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "unwrap integrity violation error",
|
||||
err: errors.Unwrap(NewNotNullError("table", "constraint", originalErr)),
|
||||
want: originalErr,
|
||||
},
|
||||
{
|
||||
name: "unknown error",
|
||||
err: NewUnknownError(originalErr),
|
||||
want: originalErr,
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
assert.Equal(t, test.want, errors.Unwrap(test.err))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIs(t *testing.T) {
|
||||
originalErr := errors.New("original error")
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
err error
|
||||
want error
|
||||
}{
|
||||
{
|
||||
name: "missing condition",
|
||||
err: NewMissingConditionError(NewColumn("table", "column")),
|
||||
want: new(MissingConditionError),
|
||||
},
|
||||
{
|
||||
name: "no row found",
|
||||
err: NewNoRowFoundError(errors.New("original error")),
|
||||
want: new(NoRowFoundError),
|
||||
},
|
||||
{
|
||||
name: "multiple rows found",
|
||||
err: NewMultipleRowsFoundError(originalErr),
|
||||
want: new(MultipleRowsFoundError),
|
||||
},
|
||||
{
|
||||
name: "check violation is for integrity",
|
||||
err: NewCheckError("table", "constraint", nil),
|
||||
want: new(IntegrityViolationError),
|
||||
},
|
||||
{
|
||||
name: "check violation is check violation",
|
||||
err: NewCheckError("table", "constraint", nil),
|
||||
want: new(CheckError),
|
||||
},
|
||||
{
|
||||
name: "unique violation is for integrity",
|
||||
err: NewUniqueError("table", "constraint", nil),
|
||||
want: new(IntegrityViolationError),
|
||||
},
|
||||
{
|
||||
name: "unique violation is unique violation",
|
||||
err: NewUniqueError("table", "constraint", nil),
|
||||
want: new(UniqueError),
|
||||
},
|
||||
{
|
||||
name: "foreign key violation is for integrity",
|
||||
err: NewForeignKeyError("table", "constraint", nil),
|
||||
want: new(IntegrityViolationError),
|
||||
},
|
||||
{
|
||||
name: "foreign key violation is foreign key violation",
|
||||
err: NewForeignKeyError("table", "constraint", nil),
|
||||
want: new(ForeignKeyError),
|
||||
},
|
||||
{
|
||||
name: "not null violation is for integrity",
|
||||
err: NewNotNullError("table", "constraint", nil),
|
||||
want: new(IntegrityViolationError),
|
||||
},
|
||||
{
|
||||
name: "not null violation is not null violation",
|
||||
err: NewNotNullError("table", "constraint", nil),
|
||||
want: new(NotNullError),
|
||||
},
|
||||
{
|
||||
name: "unknown error",
|
||||
err: NewUnknownError(originalErr),
|
||||
want: new(UnknownError),
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
assert.ErrorIs(t, test.err, test.want)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user