Simplify the signature of the Delete() method

This change was finally made because the Delete function was the
only helper function that was not returning the ksql.ErrRecordNotFound
when no rows were found.

The other reason for this change is that we the most common use case is
by far for deleting a single element, and the philosophy of the library
is to optimize for the most common use-cases.

For making it easier to write queries for deleting many items
as well as many other less common use cases we
are already implementing the `kbuilder` package which is a
query builder.
pull/13/head
Vinícius Garcia 2021-11-22 19:42:31 -03:00
parent 40215d8099
commit a1403dc9d3
6 changed files with 59 additions and 101 deletions

View File

@ -87,10 +87,13 @@ it with as little functions as possible, so don't expect many additions:
```go
// Provider describes the ksql public behavior
//
// The Insert, Update, Delete and QueryOne functions return ksql.ErrRecordNotFound
// if no record was found or no rows were changed during the operation.
type Provider interface {
Insert(ctx context.Context, table Table, record interface{}) error
Update(ctx context.Context, table Table, record interface{}) error
Delete(ctx context.Context, table Table, idsOrRecords ...interface{}) error
Delete(ctx context.Context, table Table, idOrRecord interface{}) error
Query(ctx context.Context, records interface{}, query string, params ...interface{}) error
QueryOne(ctx context.Context, record interface{}, query string, params ...interface{}) error
@ -492,7 +495,7 @@ if err != nil {
In the example above, since we are only interested in a couple of columns it
is far simpler and more efficient for the database to only select the columns
that we actually care about, so it's better not to use composite kstructs.
that we actually care about, so it's better not to use composite structs.
### Testing Examples

View File

@ -14,11 +14,14 @@ var ErrRecordNotFound error = errors.Wrap(sql.ErrNoRows, "ksql: the query return
// ErrAbortIteration ...
var ErrAbortIteration error = fmt.Errorf("ksql: abort iteration, should only be used inside QueryChunks function")
// Provider describes the ksql public behavior
// Provider describes the ksql public behavior.
//
// The Insert, Update, Delete and QueryOne functions return ksql.ErrRecordNotFound
// if no record was found or no rows were changed during the operation.
type Provider interface {
Insert(ctx context.Context, table Table, record interface{}) error
Update(ctx context.Context, table Table, record interface{}) error
Delete(ctx context.Context, table Table, idsOrRecords ...interface{}) error
Delete(ctx context.Context, table Table, idOrRecord interface{}) error
Query(ctx context.Context, records interface{}, query string, params ...interface{}) error
QueryOne(ctx context.Context, record interface{}, query string, params ...interface{}) error

View File

@ -36,22 +36,17 @@ func (m *MockProvider) EXPECT() *MockProviderMockRecorder {
}
// Delete mocks base method.
func (m *MockProvider) Delete(ctx context.Context, table ksql.Table, idsOrRecords ...interface{}) error {
func (m *MockProvider) Delete(ctx context.Context, table ksql.Table, idOrRecord interface{}) error {
m.ctrl.T.Helper()
varargs := []interface{}{ctx, table}
for _, a := range idsOrRecords {
varargs = append(varargs, a)
}
ret := m.ctrl.Call(m, "Delete", varargs...)
ret := m.ctrl.Call(m, "Delete", ctx, table, idOrRecord)
ret0, _ := ret[0].(error)
return ret0
}
// Delete indicates an expected call of Delete.
func (mr *MockProviderMockRecorder) Delete(ctx, table interface{}, idsOrRecords ...interface{}) *gomock.Call {
func (mr *MockProviderMockRecorder) Delete(ctx, table, idOrRecord interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
varargs := append([]interface{}{ctx, table}, idsOrRecords...)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockProvider)(nil).Delete), varargs...)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockProvider)(nil).Delete), ctx, table, idOrRecord)
}
// Exec mocks base method.

46
ksql.go
View File

@ -555,17 +555,35 @@ func assertStructPtr(t reflect.Type) error {
return nil
}
// Delete deletes one or more instances from the database by id
// Delete deletes one record from the database using the ID or IDs
// defined on the ksql.Table passed as second argument.
//
// For tables with a single ID column you can pass the record
// to be deleted as a struct, as a map or just pass the ID itself.
//
// For tables with composite keys you must pass the record
// as a struct or a map so that ksql can read all the composite keys
// from it.
//
// The examples below should work for both types of tables:
//
// err := c.Delete(ctx, UsersTable, user)
//
// err := c.Delete(ctx, UserPostsTable, map[string]interface{}{
// "user_id": user.ID,
// "post_id": post.ID,
// })
//
// The example below is shorter but will only work for tables with a single primary key:
//
// err := c.Delete(ctx, UsersTable, user.ID)
//
func (c DB) Delete(
ctx context.Context,
table Table,
ids ...interface{},
idOrRecord interface{},
) error {
if len(ids) == 0 {
return nil
}
idMaps, err := normalizeIDsAsMaps(table.idColumns, ids)
idMaps, err := normalizeIDsAsMaps(table.idColumns, []interface{}{idOrRecord})
if err != nil {
return err
}
@ -578,7 +596,19 @@ func (c DB) Delete(
query, params = buildCompositeKeyDeleteQuery(c.dialect, table.name, table.idColumns, idMaps)
}
_, err = c.db.ExecContext(ctx, query, params...)
result, err := c.db.ExecContext(ctx, query, params...)
if err != nil {
return err
}
n, err := result.RowsAffected()
if err != nil {
return fmt.Errorf("unable to check if the record was succesfully deleted: %s", err)
}
if n == 0 {
return ErrRecordNotFound
}
return err
}

View File

@ -887,36 +887,6 @@ func TestDelete(t *testing.T) {
t.Fatal("could not create test table!, reason:", err.Error())
}
t.Run("should ignore empty lists of ids", func(t *testing.T) {
db, closer := connectDB(t, config)
defer closer.Close()
ctx := context.Background()
c := newTestDB(db, config.driver)
u := User{
Name: "Won't be deleted",
}
err := c.Insert(ctx, UsersTable, &u)
assert.Equal(t, nil, err)
assert.NotEqual(t, uint(0), u.ID)
result := User{}
err = getUserByID(c.db, c.dialect, &result, u.ID)
assert.Equal(t, nil, err)
assert.Equal(t, u.ID, result.ID)
err = c.Delete(ctx, UsersTable)
assert.Equal(t, nil, err)
result = User{}
err = getUserByID(c.db, c.dialect, &result, u.ID)
assert.Equal(t, nil, err)
assert.Equal(t, u.ID, result.ID)
})
t.Run("should delete one id correctly", func(t *testing.T) {
tests := []struct {
desc string
@ -993,58 +963,15 @@ func TestDelete(t *testing.T) {
}
})
t.Run("should delete multiple ids correctly", func(t *testing.T) {
t.Run("should return ErrRecordNotFound if no rows were deleted", func(t *testing.T) {
db, closer := connectDB(t, config)
defer closer.Close()
ctx := context.Background()
c := newTestDB(db, config.driver)
u1 := User{
Name: "Fernanda",
}
err := c.Insert(ctx, UsersTable, &u1)
assert.Equal(t, nil, err)
assert.NotEqual(t, uint(0), u1.ID)
u2 := User{
Name: "Juliano",
}
err = c.Insert(ctx, UsersTable, &u2)
assert.Equal(t, nil, err)
assert.NotEqual(t, uint(0), u2.ID)
u3 := User{
Name: "This won't be deleted",
}
err = c.Insert(ctx, UsersTable, &u3)
assert.Equal(t, nil, err)
assert.NotEqual(t, uint(0), u3.ID)
result := User{}
err = getUserByID(c.db, c.dialect, &result, u1.ID)
assert.Equal(t, nil, err)
assert.Equal(t, u1.ID, result.ID)
result = User{}
err = getUserByID(c.db, c.dialect, &result, u2.ID)
assert.Equal(t, nil, err)
assert.Equal(t, u2.ID, result.ID)
result = User{}
err = getUserByID(c.db, c.dialect, &result, u3.ID)
assert.Equal(t, nil, err)
assert.Equal(t, u3.ID, result.ID)
err = c.Delete(ctx, UsersTable, u1.ID, u2.ID)
assert.Equal(t, nil, err)
results := []User{}
err = getUsersByID(c.db, c.dialect, &results, u1.ID, u2.ID, u3.ID)
assert.Equal(t, nil, err)
assert.Equal(t, 1, len(results))
assert.Equal(t, "This won't be deleted", results[0].Name)
err = c.Delete(ctx, UsersTable, 4200)
assert.Equal(t, ErrRecordNotFound, err)
})
t.Run("should report error if it receives a nil pointer to a struct", func(t *testing.T) {

View File

@ -11,7 +11,7 @@ var _ Provider = Mock{}
type Mock struct {
InsertFn func(ctx context.Context, table Table, record interface{}) error
UpdateFn func(ctx context.Context, table Table, record interface{}) error
DeleteFn func(ctx context.Context, table Table, ids ...interface{}) error
DeleteFn func(ctx context.Context, table Table, idOrRecord interface{}) error
QueryFn func(ctx context.Context, records interface{}, query string, params ...interface{}) error
QueryOneFn func(ctx context.Context, record interface{}, query string, params ...interface{}) error
@ -95,11 +95,11 @@ func (m Mock) Update(ctx context.Context, table Table, record interface{}) error
}
// Delete ...
func (m Mock) Delete(ctx context.Context, table Table, ids ...interface{}) error {
func (m Mock) Delete(ctx context.Context, table Table, idOrRecord interface{}) error {
if m.DeleteFn == nil {
panic(fmt.Errorf("Mock.Delete(ctx, %v, %v) called but the ksql.Mock.DeleteFn() is not set", table, ids))
panic(fmt.Errorf("Mock.Delete(ctx, %v, %v) called but the ksql.Mock.DeleteFn() is not set", table, idOrRecord))
}
return m.DeleteFn(ctx, table, ids...)
return m.DeleteFn(ctx, table, idOrRecord)
}
// Query ...