Skip to content

Commit 3d5b1b4

Browse files
committed
fix(api): serialize empty read results as [] instead of null
The read layer declared its result slices with `var x []T`, which marshal to `null` when no rows match. The OpenAPI spec declares these fields as `type: array` and marks none of them nullable, so the API contradicted its own contract and callers compensated with `|| []`. Refs #588
1 parent 411c5a4 commit 3d5b1b4

11 files changed

Lines changed: 94 additions & 18 deletions

File tree

backend/pkg/api/activity_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ func TestGetActivity(t *testing.T) {
6565

6666
activityEntries, err = a.GetActivity(uuid.New().String(), ActivityQueryParams{})
6767
assert.NoError(t, err)
68-
assert.Nil(t, activityEntries, "Team with this id doesn't exist")
68+
assert.Empty(t, activityEntries, "Team with this id doesn't exist")
69+
assert.NotNil(t, activityEntries)
6970

7071
// We try counting with default Start==-3days, End==Now
7172
totalCount, err := a.GetActivityCount(tTeam.ID, ActivityQueryParams{})
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package api
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
"gopkg.in/guregu/null.v4"
11+
)
12+
13+
func assertMarshalsToEmptyArray(t *testing.T, name string, v interface{}) {
14+
t.Helper()
15+
b, err := json.Marshal(v)
16+
require.NoError(t, err)
17+
assert.Equal(t, "[]", string(b), "%s must serialize to [] and not null", name)
18+
}
19+
20+
func TestEmptyResultsSerializeToEmptyArray(t *testing.T) {
21+
a := newForTest(t)
22+
defer a.Close()
23+
24+
tTeam, err := a.AddTeam(&Team{Name: "test_team_empty_slice"})
25+
require.NoError(t, err)
26+
tApp, err := a.AddApp(&Application{Name: "test_app_empty_slice", TeamID: tTeam.ID})
27+
require.NoError(t, err)
28+
tChannel, err := a.AddChannel(&Channel{Name: "test_channel_empty_slice", Color: "blue", ApplicationID: tApp.ID, Arch: ArchAMD64})
29+
require.NoError(t, err)
30+
tGroup, err := a.AddGroup(&Group{
31+
Name: "test_group_empty_slice",
32+
ApplicationID: tApp.ID,
33+
ChannelID: null.StringFrom(tChannel.ID),
34+
PolicyUpdatesEnabled: true,
35+
PolicySafeMode: true,
36+
PolicyPeriodInterval: "15 minutes",
37+
PolicyMaxUpdatesPerPeriod: 2,
38+
PolicyUpdateTimeout: "60 minutes",
39+
})
40+
require.NoError(t, err)
41+
tPkg, err := a.AddPackage(&Package{
42+
Type: PkgTypeOther,
43+
URL: "http://sample.url/pkg",
44+
Version: "1.0.0",
45+
ApplicationID: tApp.ID,
46+
Arch: ArchAMD64,
47+
})
48+
require.NoError(t, err)
49+
50+
t.Run("group version breakdown", func(t *testing.T) {
51+
got, err := a.GetGroupVersionBreakdown(tGroup.ID)
52+
require.NoError(t, err)
53+
assertMarshalsToEmptyArray(t, "version breakdown", got)
54+
})
55+
56+
t.Run("instance status history", func(t *testing.T) {
57+
got, err := a.GetInstanceStatusHistory(uuid.New().String(), tApp.ID, tGroup.ID, 20)
58+
require.NoError(t, err)
59+
assertMarshalsToEmptyArray(t, "instance status history", got)
60+
})
61+
62+
t.Run("activity", func(t *testing.T) {
63+
got, err := a.GetActivity(uuid.New().String(), ActivityQueryParams{})
64+
require.NoError(t, err)
65+
assertMarshalsToEmptyArray(t, "activity", got)
66+
})
67+
68+
t.Run("package floor channels", func(t *testing.T) {
69+
got, err := a.GetPackageFloorChannels(tPkg.ID)
70+
require.NoError(t, err)
71+
assertMarshalsToEmptyArray(t, "package floor channels", got)
72+
})
73+
74+
t.Run("package extra files", func(t *testing.T) {
75+
got, err := a.GetPackage(tPkg.ID)
76+
require.NoError(t, err)
77+
assertMarshalsToEmptyArray(t, "package extra files", got.ExtraFiles)
78+
})
79+
}

backend/pkg/api/internal/dbreads/activity.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func (q *Queries) GetActivityCount(teamID string, p types.ActivityQueryParams) (
1919
// GetActivity returns a list of activity entries that match the specified
2020
// criteria in the query parameters.
2121
func (q *Queries) GetActivity(teamID string, p types.ActivityQueryParams) ([]*types.Activity, error) {
22-
var activityEntries []*types.Activity
22+
activityEntries := []*types.Activity{}
2323
query, _, err := q.activityQuery(teamID, p, false).ToSQL()
2424
if err != nil {
2525
return nil, err

backend/pkg/api/internal/dbreads/applications.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (q *Queries) GetAppsCount(teamID string) (int, error) {
6565
// GetApps returns all applications that belong to the team id provided.
6666
func (q *Queries) GetApps(teamID string, page, perPage uint64) ([]*types.Application, error) {
6767
page, perPage = validatePaginationParams(page, perPage)
68-
var apps []*types.Application
68+
apps := []*types.Application{}
6969
limit, offset := sqlPaginate(page, perPage)
7070
query, _, err := q.appsQuery().
7171
Where(goqu.C("team_id").Eq(teamID)).

backend/pkg/api/internal/dbreads/channels.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func (q *Queries) getChannels(appID string) ([]*types.Channel, error) {
6666
}
6767

6868
func (q *Queries) getChannelsFromQuery(query string) ([]*types.Channel, error) {
69-
var channels []*types.Channel
69+
channels := []*types.Channel{}
7070
rows, err := q.db.Queryx(query)
7171
if err != nil {
7272
return nil, err

backend/pkg/api/internal/dbreads/groups.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ func (q *Queries) getGroups(appID string) ([]*types.Group, error) {
200200
}
201201

202202
func (q *Queries) getGroupsFromQuery(query string) ([]*types.Group, error) {
203-
var groups []*types.Group
203+
groups := []*types.Group{}
204204
rows, err := q.db.Queryx(query)
205205
if err != nil {
206206
return nil, err
@@ -299,7 +299,7 @@ func (q *Queries) groupsQuery() *goqu.SelectDataset {
299299

300300
// GetGroupVersionBreakdown returns a version breakdown of all instances running on a given group.
301301
func (q *Queries) GetGroupVersionBreakdown(groupID string) ([]*types.VersionBreakdownEntry, error) {
302-
var entryList []*types.VersionBreakdownEntry
302+
entryList := []*types.VersionBreakdownEntry{}
303303

304304
semverExpr, err := semverToIntArray("version")
305305
if err != nil {

backend/pkg/api/internal/dbreads/instances.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (q *Queries) getInstanceApp(appID, instanceID string, duration postgresDura
104104
// GetInstanceStatusHistory returns the status history of an instance in the
105105
// context of the application/group provided.
106106
func (q *Queries) GetInstanceStatusHistory(instanceID, appID, groupID string, limit uint64) ([]*types.InstanceStatusHistoryEntry, error) {
107-
var instanceStatusHistory []*types.InstanceStatusHistoryEntry
107+
instanceStatusHistory := []*types.InstanceStatusHistoryEntry{}
108108
query, _, err := q.instanceStatusHistoryQuery(instanceID, appID, groupID, limit).ToSQL()
109109
if err != nil {
110110
return nil, err
@@ -166,7 +166,7 @@ func prepareSearchQuery(finalQuery *goqu.SelectDataset, p types.InstancesQueryPa
166166

167167
// GetInstances returns all instances that match with the provided criteria.
168168
func (q *Queries) GetInstances(p types.InstancesQueryParams, duration string) (types.InstancesWithTotal, error) {
169-
var instances []*types.Instance
169+
instances := []*types.Instance{}
170170
var err error
171171
totalCount, err := q.GetInstancesCount(p, duration)
172172
if err != nil {

backend/pkg/api/internal/dbreads/packages.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (q *Queries) GetPackages(appID string, page, perPage uint64, searchVersion
8181
}
8282

8383
func (q *Queries) getPackagesFromQuery(query string) ([]*types.Package, error) {
84-
var pkgs []*types.Package
84+
pkgs := []*types.Package{}
8585
rows, err := q.db.Queryx(query)
8686
if err != nil {
8787
return nil, err
@@ -163,7 +163,7 @@ func (q *Queries) loadPackageExtras(packages []*types.Package) ([]*types.Package
163163
if files, ok := filesByPkg[pkg.ID]; ok {
164164
pkg.ExtraFiles = files
165165
} else {
166-
pkg.ExtraFiles = nil
166+
pkg.ExtraFiles = []types.File{}
167167
}
168168
if action, ok := actionsByPkg[pkg.ID]; ok {
169169
pkg.FlatcarAction = action
@@ -218,7 +218,7 @@ func (q *Queries) getExtraFiles(packageID string) ([]types.File, error) {
218218
return nil, err
219219
}
220220

221-
var files []types.File
221+
files := []types.File{}
222222
rows, err := q.db.Queryx(query)
223223
if err != nil {
224224
return nil, err
@@ -263,7 +263,7 @@ func (q *Queries) getPackage(packageID null.String) (*types.Package, error) {
263263
case nil:
264264
packageEntity.ExtraFiles = extraFiles
265265
case sql.ErrNoRows:
266-
packageEntity.ExtraFiles = nil
266+
packageEntity.ExtraFiles = []types.File{}
267267
default:
268268
return nil, err
269269
}

backend/pkg/api/internal/dbreads/packages_floors.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func (q *Queries) GetPackageFloorChannels(packageID string) ([]types.ChannelFloo
217217
}
218218
defer rows.Close()
219219

220-
var result []types.ChannelFloorInfo
220+
result := []types.ChannelFloorInfo{}
221221
for rows.Next() {
222222
var chWithFloor channelWithFloor
223223
if err := rows.StructScan(&chWithFloor); err != nil {

backend/pkg/handler/groups.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,6 @@ func (h *Handler) GetGroupVersionBreakdown(ctx echo.Context, _ string, groupID s
224224
return ctx.NoContent(http.StatusInternalServerError)
225225
}
226226

227-
if len(versionBreakdown) == 0 {
228-
// WAT?: because otherwise it serializes to null not []
229-
return ctx.JSON(http.StatusOK, []string{})
230-
}
231227
return ctx.JSON(http.StatusOK, versionBreakdown)
232228
}
233229

0 commit comments

Comments
 (0)