Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6b69abf

Browse files
authored
fix(coderd): use stable sorting for insights and improve test coverage (#9250)
Fixes#9213
1 parent970072f commit6b69abf

19 files changed

+1974
-379
lines changed

‎Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ coderd/apidoc/swagger.json: $(shell find ./scripts/apidocgen $(FIND_EXCLUSIONS)
564564
./scripts/apidocgen/generate.sh
565565
pnpm run format:write:only ./docs/api ./docs/manifest.json ./coderd/apidoc/swagger.json
566566

567-
update-golden-files: cli/testdata/.gen-golden helm/coder/tests/testdata/.gen-golden helm/provisioner/tests/testdata/.gen-golden scripts/ci-report/testdata/.gen-golden enterprise/cli/testdata/.gen-golden
567+
update-golden-files: cli/testdata/.gen-golden helm/coder/tests/testdata/.gen-golden helm/provisioner/tests/testdata/.gen-golden scripts/ci-report/testdata/.gen-golden enterprise/cli/testdata/.gen-golden coderd/.gen-golden
568568
.PHONY: update-golden-files
569569

570570
cli/testdata/.gen-golden:$(wildcard cli/testdata/*.golden)$(wildcard cli/*.tpl)$(GO_SRC_FILES)$(wildcard cli/*_test.go)
@@ -583,6 +583,10 @@ helm/provisioner/tests/testdata/.gen-golden: $(wildcard helm/provisioner/tests/t
583583
gotest ./helm/provisioner/tests -run=TestUpdateGoldenFiles -update
584584
touch"$@"
585585

586+
coderd/.gen-golden:$(wildcard coderd/testdata/*/*.golden)$(GO_SRC_FILES)$(wildcard coderd/*_test.go)
587+
gotest ./coderd -run="Test.*Golden$$" -update
588+
touch"$@"
589+
586590
scripts/ci-report/testdata/.gen-golden:$(wildcard scripts/ci-report/testdata/*)$(wildcard scripts/ci-report/*.go)
587591
gotest ./scripts/ci-report -run=TestOutputMatchesGoldenFile -update
588592
touch"$@"

‎coderd/coderd_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderd_test
22

33
import (
44
"context"
5+
"flag"
56
"io"
67
"net/http"
78
"net/netip"
@@ -23,6 +24,9 @@ import (
2324
"github.com/coder/coder/v2/testutil"
2425
)
2526

27+
// updateGoldenFiles is a flag that can be set to update golden files.
28+
varupdateGoldenFiles=flag.Bool("update",false,"Update golden files")
29+
2630
funcTestMain(m*testing.M) {
2731
goleak.VerifyTestMain(m)
2832
}

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ package db2sdk
33

44
import (
55
"encoding/json"
6-
"sort"
6+
"strings"
77

88
"github.com/google/uuid"
9+
"golang.org/x/exp/slices"
910

1011
"github.com/coder/coder/v2/coderd/database"
1112
"github.com/coder/coder/v2/coderd/parameter"
@@ -125,9 +126,34 @@ func Role(role rbac.Role) codersdk.Role {
125126
}
126127

127128
funcTemplateInsightsParameters(parameterRows []database.GetTemplateParameterInsightsRow) ([]codersdk.TemplateParameterUsage,error) {
128-
parametersByNum:=make(map[int64]*codersdk.TemplateParameterUsage)
129+
// Use a stable sort, similarly to how we would sort in the query, note that
130+
// we don't sort in the query because order varies depending on the table
131+
// collation.
132+
//
133+
// ORDER BY utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value
134+
slices.SortFunc(parameterRows,func(a,b database.GetTemplateParameterInsightsRow)int {
135+
ifa.Name!=b.Name {
136+
returnstrings.Compare(a.Name,b.Name)
137+
}
138+
ifa.Type!=b.Type {
139+
returnstrings.Compare(a.Type,b.Type)
140+
}
141+
ifa.DisplayName!=b.DisplayName {
142+
returnstrings.Compare(a.DisplayName,b.DisplayName)
143+
}
144+
ifa.Description!=b.Description {
145+
returnstrings.Compare(a.Description,b.Description)
146+
}
147+
ifstring(a.Options)!=string(b.Options) {
148+
returnstrings.Compare(string(a.Options),string(b.Options))
149+
}
150+
returnstrings.Compare(a.Value,b.Value)
151+
})
152+
153+
parametersUsage:= []codersdk.TemplateParameterUsage{}
154+
indexByNum:=make(map[int64]int)
129155
for_,param:=rangeparameterRows {
130-
if_,ok:=parametersByNum[param.Num];!ok {
156+
if_,ok:=indexByNum[param.Num];!ok {
131157
varopts []codersdk.TemplateVersionParameterOption
132158
err:=json.Unmarshal(param.Options,&opts)
133159
iferr!=nil {
@@ -139,28 +165,24 @@ func TemplateInsightsParameters(parameterRows []database.GetTemplateParameterIns
139165
returnnil,err
140166
}
141167

142-
parametersByNum[param.Num]=&codersdk.TemplateParameterUsage{
168+
parametersUsage=append(parametersUsage,codersdk.TemplateParameterUsage{
143169
TemplateIDs:param.TemplateIDs,
144170
Name:param.Name,
145171
Type:param.Type,
146172
DisplayName:param.DisplayName,
147173
Description:plaintextDescription,
148174
Options:opts,
149-
}
175+
})
176+
indexByNum[param.Num]=len(parametersUsage)-1
150177
}
151-
parametersByNum[param.Num].Values=append(parametersByNum[param.Num].Values, codersdk.TemplateParameterValue{
178+
179+
i:=indexByNum[param.Num]
180+
parametersUsage[i].Values=append(parametersUsage[i].Values, codersdk.TemplateParameterValue{
152181
Value:param.Value,
153182
Count:param.Count,
154183
})
155184
}
156-
parametersUsage:= []codersdk.TemplateParameterUsage{}
157-
for_,param:=rangeparametersByNum {
158-
parametersUsage=append(parametersUsage,*param)
159-
}
160185

161-
sort.Slice(parametersUsage,func(i,jint)bool {
162-
returnparametersUsage[i].Name<parametersUsage[j].Name
163-
})
164186
returnparametersUsage,nil
165187
}
166188

‎coderd/database/dbfake/dbfake.go

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,6 +2018,10 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
20182018
returnnil,err
20192019
}
20202020

2021+
iflen(arg.TemplateIDs)>0&&!slices.Contains(arg.TemplateIDs,w.TemplateID) {
2022+
continue
2023+
}
2024+
20212025
app,_:=q.getWorkspaceAppByAgentIDAndSlugNoLock(ctx, database.GetWorkspaceAppByAgentIDAndSlugParams{
20222026
AgentID:s.AgentID,
20232027
Slug:s.SlugOrPort,
@@ -2095,6 +2099,8 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
20952099
})
20962100
}
20972101

2102+
// NOTE(mafredri): Add sorting if we decide on how to handle PostgreSQL collations.
2103+
// ORDER BY access_method, slug_or_port, display_name, icon, is_app
20982104
returnrows,nil
20992105
}
21002106

@@ -2264,7 +2270,6 @@ func (q *FakeQuerier) GetTemplateDailyInsights(ctx context.Context, arg database
22642270
}
22652271
ds.userSet[s.UserID]=struct{}{}
22662272
ds.templateIDSet[s.TemplateID]=struct{}{}
2267-
break
22682273
}
22692274
}
22702275

@@ -2278,24 +2283,27 @@ func (q *FakeQuerier) GetTemplateDailyInsights(ctx context.Context, arg database
22782283
continue
22792284
}
22802285

2286+
w,err:=q.getWorkspaceByIDNoLock(ctx,s.WorkspaceID)
2287+
iferr!=nil {
2288+
returnnil,err
2289+
}
2290+
2291+
iflen(arg.TemplateIDs)>0&&!slices.Contains(arg.TemplateIDs,w.TemplateID) {
2292+
continue
2293+
}
2294+
22812295
for_,ds:=rangedailyStats {
22822296
// (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_)
22832297
// OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_)
22842298
// OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_)
2285-
if!(((s.SessionStartedAt.After(arg.StartTime)||s.SessionStartedAt.Equal(arg.StartTime))&&s.SessionStartedAt.Before(arg.EndTime))||
2286-
(s.SessionEndedAt.After(arg.StartTime)&&s.SessionEndedAt.Before(arg.EndTime))||
2287-
(s.SessionStartedAt.Before(arg.StartTime)&& (s.SessionEndedAt.After(arg.EndTime)||s.SessionEndedAt.Equal(arg.EndTime)))) {
2299+
if!(((s.SessionStartedAt.After(ds.startTime)||s.SessionStartedAt.Equal(ds.startTime))&&s.SessionStartedAt.Before(ds.endTime))||
2300+
(s.SessionEndedAt.After(ds.startTime)&&s.SessionEndedAt.Before(ds.endTime))||
2301+
(s.SessionStartedAt.Before(ds.startTime)&& (s.SessionEndedAt.After(ds.endTime)||s.SessionEndedAt.Equal(ds.endTime)))) {
22882302
continue
22892303
}
22902304

2291-
w,err:=q.getWorkspaceByIDNoLock(ctx,s.WorkspaceID)
2292-
iferr!=nil {
2293-
returnnil,err
2294-
}
2295-
22962305
ds.userSet[s.UserID]=struct{}{}
22972306
ds.templateIDSet[w.TemplateID]=struct{}{}
2298-
break
22992307
}
23002308
}
23012309

@@ -2430,7 +2438,8 @@ func (q *FakeQuerier) GetTemplateParameterInsights(ctx context.Context, arg data
24302438
iftvp.TemplateVersionID!=tv.ID {
24312439
continue
24322440
}
2433-
key:=fmt.Sprintf("%s:%s:%s:%s",tvp.Name,tvp.DisplayName,tvp.Description,tvp.Options)
2441+
// GROUP BY tvp.name, tvp.type, tvp.display_name, tvp.description, tvp.options
2442+
key:=fmt.Sprintf("%s:%s:%s:%s:%s",tvp.Name,tvp.Type,tvp.DisplayName,tvp.Description,tvp.Options)
24342443
if_,ok:=uniqueTemplateParams[key];!ok {
24352444
num++
24362445
uniqueTemplateParams[key]=&database.GetTemplateParameterInsightsRow{
@@ -2480,6 +2489,8 @@ func (q *FakeQuerier) GetTemplateParameterInsights(ctx context.Context, arg data
24802489
}
24812490
}
24822491

2492+
// NOTE(mafredri): Add sorting if we decide on how to handle PostgreSQL collations.
2493+
// ORDER BY utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value
24832494
returnrows,nil
24842495
}
24852496

‎coderd/database/queries.sql.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/insights.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,13 +230,13 @@ WITH latest_workspace_builds AS (
230230
array_agg(DISTINCTwb.template_id)::uuid[]AS template_ids,
231231
array_agg(wb.id)::uuid[]AS workspace_build_ids,
232232
tvp.name,
233+
tvp.type,
233234
tvp.display_name,
234235
tvp.description,
235-
tvp.options,
236-
tvp.type
236+
tvp.options
237237
FROM latest_workspace_builds wb
238238
JOIN template_version_parameters tvpON (tvp.template_version_id=wb.template_version_id)
239-
GROUP BYtvp.name,tvp.display_name,tvp.description,tvp.options,tvp.type
239+
GROUP BYtvp.name,tvp.type,tvp.display_name,tvp.description,tvp.options
240240
)
241241

242242
SELECT
@@ -251,4 +251,4 @@ SELECT
251251
COUNT(wbp.value)AS count
252252
FROM unique_template_params utp
253253
JOIN workspace_build_parameters wbpON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id]ANDutp.name=wbp.name)
254-
GROUP BYutp.num,utp.name,utp.display_name,utp.description,utp.options,utp.template_ids,utp.type,wbp.value;
254+
GROUP BYutp.num,utp.template_ids,utp.name,utp.type,utp.display_name,utp.description,utp.options,wbp.value;

‎coderd/insights.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"net/http"
7+
"strings"
78
"time"
89

910
"github.com/google/uuid"
@@ -288,8 +289,10 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) {
288289
}
289290
for_,row:=rangedailyUsage {
290291
resp.IntervalReports=append(resp.IntervalReports, codersdk.TemplateInsightsIntervalReport{
291-
StartTime:row.StartTime,
292-
EndTime:row.EndTime,
292+
// NOTE(mafredri): This might not be accurate over DST since the
293+
// parsed location only contains the offset.
294+
StartTime:row.StartTime.In(startTime.Location()),
295+
EndTime:row.EndTime.In(startTime.Location()),
293296
Interval:interval,
294297
TemplateIDs:row.TemplateIDs,
295298
ActiveUsers:row.ActiveUsers,
@@ -377,6 +380,32 @@ func convertTemplateInsightsApps(usage database.GetTemplateInsightsRow, appUsage
377380
},
378381
}
379382

383+
// Use a stable sort, similarly to how we would sort in the query, note that
384+
// we don't sort in the query because order varies depending on the table
385+
// collation.
386+
//
387+
// ORDER BY access_method, slug_or_port, display_name, icon, is_app
388+
slices.SortFunc(appUsage,func(a,b database.GetTemplateAppInsightsRow)int {
389+
ifa.AccessMethod!=b.AccessMethod {
390+
returnstrings.Compare(a.AccessMethod,b.AccessMethod)
391+
}
392+
ifa.SlugOrPort!=b.SlugOrPort {
393+
returnstrings.Compare(a.SlugOrPort,b.SlugOrPort)
394+
}
395+
ifa.DisplayName.String!=b.DisplayName.String {
396+
returnstrings.Compare(a.DisplayName.String,b.DisplayName.String)
397+
}
398+
ifa.Icon.String!=b.Icon.String {
399+
returnstrings.Compare(a.Icon.String,b.Icon.String)
400+
}
401+
if!a.IsApp&&b.IsApp {
402+
return-1
403+
}elseifa.IsApp&&!b.IsApp {
404+
return1
405+
}
406+
return0
407+
})
408+
380409
// Template apps.
381410
for_,app:=rangeappUsage {
382411
if!app.IsApp {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp