Skip to content

Commit 795e09b

Browse files
committed
refactor: Handle no entities found gracefully in event cleanup
- Change deleteReadingsBySubQuery and deleteEvents to return KindEntityDoesNotExist instead of KindContractInvalid when no rows are found - Include SQL statement in error messages for debugging - Log KindEntityDoesNotExist error at debug level Signed-off-by: FelixTing <felix@iotechsys.com>
1 parent 5ff0a09 commit 795e09b

File tree

4 files changed

+252
-10
lines changed

4 files changed

+252
-10
lines changed

internal/pkg/infrastructure/postgres/event.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -236,14 +236,22 @@ func (c *Client) deleteEventsByConditions(cols []string, values pgx.NamedArgs) e
236236
// select the event-ids of the specified device name from event table as the sub-query of deleting readings
237237
subSqlStatement := sqlQueryEventIdFieldsByCol(cols...)
238238
if err = deleteReadingsBySubQuery(ctx, tx, subSqlStatement, values); err != nil {
239-
c.loggingClient.Errorf("failed delete readings with conditions '%v' '%v': %v", cols, values, err)
240-
return err
239+
if errors.Kind(err) == errors.KindEntityDoesNotExist {
240+
c.loggingClient.Debugf("no readings found for deletion: %s", err.Error())
241+
} else {
242+
c.loggingClient.Errorf("failed delete readings with conditions '%v' '%v': %v", cols, values, err)
243+
return err
244+
}
241245
}
242246

243247
err = deleteEvents(ctx, tx, sqlStatement, values)
244248
if err != nil {
245-
c.loggingClient.Errorf("failed delete event with conditions '%v' '%v': %v", cols, values, err)
246-
return err
249+
if errors.Kind(err) == errors.KindEntityDoesNotExist {
250+
c.loggingClient.Debugf("no events found for deletion: %s", err.Error())
251+
} else {
252+
c.loggingClient.Errorf("failed delete event with conditions '%v' '%v': %v", cols, values, err)
253+
return err
254+
}
247255
}
248256

249257
for _, deviceInfo := range deviceInfos {
@@ -297,14 +305,22 @@ func (c *Client) deleteEventsByAgeAndConditions(age int64, cols []string, values
297305
// select the event ids within the origin time range from event table as the sub-query of deleting readings
298306
subSqlStatement := sqlQueryEventIdFieldByTimeRangeAndConditions(originCol, cols...)
299307
if err := deleteReadingsBySubQuery(ctx, tx, subSqlStatement, values); err != nil {
300-
c.loggingClient.Errorf("failed delete readings by age '%d' nanoseconds: %v", age, err)
301-
return err
308+
if errors.Kind(err) == errors.KindEntityDoesNotExist {
309+
c.loggingClient.Debugf("no readings found for deletion by age '%d' nanoseconds: %s", age, err.Error())
310+
} else {
311+
c.loggingClient.Errorf("failed delete readings by age '%d' nanoseconds: %v", age, err)
312+
return err
313+
}
302314
}
303315

304316
err := deleteEvents(ctx, tx, sqlStatement, values)
305317
if err != nil {
306-
c.loggingClient.Errorf("failed delete event by age '%d' nanoseconds: %v", age, err)
307-
return err
318+
if errors.Kind(err) == errors.KindEntityDoesNotExist {
319+
c.loggingClient.Debugf("no events found for deletion by age '%d' nanoseconds: %s", age, err.Error())
320+
} else {
321+
c.loggingClient.Errorf("failed delete event by age '%d' nanoseconds: %v", age, err)
322+
return err
323+
}
308324
}
309325
return nil
310326
})
@@ -356,7 +372,7 @@ func deleteEvents(ctx context.Context, tx pgx.Tx, sqlStatement string, args pgx.
356372
return pgClient.WrapDBError("event(s) delete failed", err)
357373
}
358374
if commandTag.RowsAffected() == 0 {
359-
return errors.NewCommonEdgeX(errors.KindContractInvalid, "no event found", nil)
375+
return errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, fmt.Sprintf("no event found; SQL statement: %s", sqlStatement), nil)
360376
}
361377
return nil
362378
}
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
//
2+
// Copyright (C) 2025 IOTech Ltd
3+
//
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package postgres
7+
8+
import (
9+
"context"
10+
"fmt"
11+
"strings"
12+
"testing"
13+
14+
"github.com/edgexfoundry/edgex-go/internal/pkg/infrastructure/postgres/mocks"
15+
"github.com/edgexfoundry/go-mod-core-contracts/v4/errors"
16+
17+
"github.com/jackc/pgx/v5"
18+
"github.com/jackc/pgx/v5/pgconn"
19+
"github.com/stretchr/testify/assert"
20+
"github.com/stretchr/testify/mock"
21+
)
22+
23+
func TestDeleteEvents(t *testing.T) {
24+
ctx := context.Background()
25+
sqlStatement := "DELETE FROM event WHERE id = @id"
26+
args := pgx.NamedArgs{"id": "test-id"}
27+
28+
tests := []struct {
29+
name string
30+
rowsAffected int64
31+
execError error
32+
expectError bool
33+
expectedErrKind errors.ErrKind
34+
errorContains []string
35+
}{
36+
{
37+
name: "No rows affected - should return KindEntityDoesNotExist",
38+
rowsAffected: 0,
39+
execError: nil,
40+
expectError: true,
41+
expectedErrKind: errors.KindEntityDoesNotExist,
42+
errorContains: []string{"no event found", "SQL statement:", sqlStatement},
43+
},
44+
{
45+
name: "Rows affected - should succeed",
46+
rowsAffected: 1,
47+
execError: nil,
48+
expectError: false,
49+
expectedErrKind: "",
50+
errorContains: nil,
51+
},
52+
{
53+
name: "Exec returns error - should return KindDatabaseError",
54+
rowsAffected: 0,
55+
execError: fmt.Errorf("database connection error"),
56+
expectError: true,
57+
expectedErrKind: errors.KindDatabaseError,
58+
errorContains: []string{"event(s) delete failed"},
59+
},
60+
}
61+
62+
for _, tt := range tests {
63+
t.Run(tt.name, func(t *testing.T) {
64+
tx := new(mocks.Tx)
65+
// On successful completion, a DELETE command returns a command tag of the form "DELETE count"
66+
// https://www.postgresql.org/docs/16/sql-delete.html
67+
commandTag := pgconn.NewCommandTag(fmt.Sprintf("DELETE %d", tt.rowsAffected))
68+
tx.On("Exec", ctx, sqlStatement, args).Return(commandTag, tt.execError)
69+
70+
err := deleteEvents(ctx, tx, sqlStatement, args)
71+
72+
if tt.expectError {
73+
assert.Error(t, err)
74+
assert.Equal(t, tt.expectedErrKind, errors.Kind(err))
75+
for _, contains := range tt.errorContains {
76+
assert.Contains(t, err.Error(), contains)
77+
}
78+
} else {
79+
assert.NoError(t, err)
80+
}
81+
tx.AssertExpectations(t)
82+
})
83+
}
84+
}
85+
86+
func TestDeleteReadingsBySubQuery(t *testing.T) {
87+
ctx := context.Background()
88+
subQuerySql := "SELECT id FROM event WHERE devicename = @devicename"
89+
args := pgx.NamedArgs{"devicename": "test-device"}
90+
91+
tests := []struct {
92+
name string
93+
rowsAffected int64
94+
execError error
95+
expectError bool
96+
expectedErrKind errors.ErrKind
97+
errorContains []string
98+
}{
99+
{
100+
name: "No rows affected - should return KindEntityDoesNotExist",
101+
rowsAffected: 0,
102+
execError: nil,
103+
expectError: true,
104+
expectedErrKind: errors.KindEntityDoesNotExist,
105+
errorContains: []string{"no reading found", "SQL statement:"},
106+
},
107+
{
108+
name: "Rows affected - should succeed",
109+
rowsAffected: 1,
110+
execError: nil,
111+
expectError: false,
112+
expectedErrKind: "",
113+
errorContains: nil,
114+
},
115+
{
116+
name: "Exec returns error - should return KindDatabaseError",
117+
rowsAffected: 1, // Use non-zero to avoid RowsAffected check before error check
118+
execError: fmt.Errorf("database connection error"),
119+
expectError: true,
120+
expectedErrKind: errors.KindDatabaseError,
121+
errorContains: []string{"reading(s) delete failed"},
122+
},
123+
}
124+
125+
for _, tt := range tests {
126+
t.Run(tt.name, func(t *testing.T) {
127+
tx := new(mocks.Tx)
128+
// On successful completion, a DELETE command returns a command tag of the form "DELETE count"
129+
// https://www.postgresql.org/docs/16/sql-delete.html
130+
commandTag := pgconn.NewCommandTag(fmt.Sprintf("DELETE %d", tt.rowsAffected))
131+
tx.On("Exec", ctx, mock.MatchedBy(func(sql string) bool {
132+
return strings.Contains(sql, subQuerySql)
133+
}), args).Return(commandTag, tt.execError)
134+
135+
err := deleteReadingsBySubQuery(ctx, tx, subQuerySql, args)
136+
137+
if tt.expectError {
138+
assert.Error(t, err)
139+
assert.Equal(t, tt.expectedErrKind, errors.Kind(err))
140+
for _, contains := range tt.errorContains {
141+
assert.Contains(t, err.Error(), contains)
142+
}
143+
} else {
144+
assert.NoError(t, err)
145+
}
146+
tx.AssertExpectations(t)
147+
})
148+
}
149+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
//
2+
// Copyright (C) 2025 IOTech Ltd
3+
//
4+
// SPDX-License-Identifier: Apache-2.0
5+
6+
package mocks
7+
8+
import (
9+
"context"
10+
11+
"github.com/jackc/pgx/v5"
12+
"github.com/jackc/pgx/v5/pgconn"
13+
"github.com/stretchr/testify/mock"
14+
)
15+
16+
// Tx is a mock implementation of pgx.Tx for testing
17+
type Tx struct {
18+
mock.Mock
19+
}
20+
21+
func (t *Tx) Begin(ctx context.Context) (pgx.Tx, error) {
22+
args := t.Called(ctx)
23+
return args.Get(0).(pgx.Tx), args.Error(1)
24+
}
25+
26+
func (t *Tx) Commit(ctx context.Context) error {
27+
args := t.Called(ctx)
28+
return args.Error(0)
29+
}
30+
31+
func (t *Tx) Rollback(ctx context.Context) error {
32+
args := t.Called(ctx)
33+
return args.Error(0)
34+
}
35+
36+
func (t *Tx) CopyFrom(ctx context.Context, tableName pgx.Identifier, columnNames []string, rowSrc pgx.CopyFromSource) (int64, error) {
37+
args := t.Called(ctx, tableName, columnNames, rowSrc)
38+
return args.Get(0).(int64), args.Error(1)
39+
}
40+
41+
func (t *Tx) SendBatch(ctx context.Context, b *pgx.Batch) pgx.BatchResults {
42+
args := t.Called(ctx, b)
43+
return args.Get(0).(pgx.BatchResults)
44+
}
45+
46+
func (t *Tx) LargeObjects() pgx.LargeObjects {
47+
args := t.Called()
48+
return args.Get(0).(pgx.LargeObjects)
49+
}
50+
51+
func (t *Tx) Prepare(ctx context.Context, name, sql string) (*pgconn.StatementDescription, error) {
52+
args := t.Called(ctx, name, sql)
53+
return args.Get(0).(*pgconn.StatementDescription), args.Error(1)
54+
}
55+
56+
func (t *Tx) Exec(ctx context.Context, sql string, arguments ...any) (pgconn.CommandTag, error) {
57+
var callArgs []interface{}
58+
callArgs = []interface{}{ctx, sql}
59+
callArgs = append(callArgs, arguments...)
60+
mockArgs := t.Called(callArgs...)
61+
return mockArgs.Get(0).(pgconn.CommandTag), mockArgs.Error(1)
62+
}
63+
64+
func (t *Tx) Query(ctx context.Context, sql string, args ...any) (pgx.Rows, error) {
65+
mockArgs := t.Called(ctx, sql, args)
66+
return mockArgs.Get(0).(pgx.Rows), mockArgs.Error(1)
67+
}
68+
69+
func (t *Tx) QueryRow(ctx context.Context, sql string, args ...any) pgx.Row {
70+
mockArgs := t.Called(ctx, sql, args)
71+
return mockArgs.Get(0).(pgx.Row)
72+
}
73+
74+
func (t *Tx) Conn() *pgx.Conn {
75+
args := t.Called()
76+
return args.Get(0).(*pgx.Conn)
77+
}

internal/pkg/infrastructure/postgres/reading.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ func deleteReadingsBySubQuery(ctx context.Context, tx pgx.Tx, subQuerySql string
364364
args,
365365
)
366366
if commandTag.RowsAffected() == 0 {
367-
return errors.NewCommonEdgeX(errors.KindContractInvalid, "no reading found", nil)
367+
return errors.NewCommonEdgeX(errors.KindEntityDoesNotExist, fmt.Sprintf("no reading found; SQL statement: %s", sqlStatement), nil)
368368
}
369369
if err != nil {
370370
return pgClient.WrapDBError("reading(s) delete failed", err)

0 commit comments

Comments
 (0)