mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): use stable sorting for insights and improve test coverage (#9250)
Fixes #9213
This commit is contained in:
@@ -3,9 +3,10 @@ package db2sdk
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"golang.org/x/exp/slices"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/parameter"
|
||||
@@ -125,9 +126,34 @@ func Role(role rbac.Role) codersdk.Role {
|
||||
}
|
||||
|
||||
func TemplateInsightsParameters(parameterRows []database.GetTemplateParameterInsightsRow) ([]codersdk.TemplateParameterUsage, error) {
|
||||
parametersByNum := make(map[int64]*codersdk.TemplateParameterUsage)
|
||||
// Use a stable sort, similarly to how we would sort in the query, note that
|
||||
// we don't sort in the query because order varies depending on the table
|
||||
// collation.
|
||||
//
|
||||
// ORDER BY utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value
|
||||
slices.SortFunc(parameterRows, func(a, b database.GetTemplateParameterInsightsRow) int {
|
||||
if a.Name != b.Name {
|
||||
return strings.Compare(a.Name, b.Name)
|
||||
}
|
||||
if a.Type != b.Type {
|
||||
return strings.Compare(a.Type, b.Type)
|
||||
}
|
||||
if a.DisplayName != b.DisplayName {
|
||||
return strings.Compare(a.DisplayName, b.DisplayName)
|
||||
}
|
||||
if a.Description != b.Description {
|
||||
return strings.Compare(a.Description, b.Description)
|
||||
}
|
||||
if string(a.Options) != string(b.Options) {
|
||||
return strings.Compare(string(a.Options), string(b.Options))
|
||||
}
|
||||
return strings.Compare(a.Value, b.Value)
|
||||
})
|
||||
|
||||
parametersUsage := []codersdk.TemplateParameterUsage{}
|
||||
indexByNum := make(map[int64]int)
|
||||
for _, param := range parameterRows {
|
||||
if _, ok := parametersByNum[param.Num]; !ok {
|
||||
if _, ok := indexByNum[param.Num]; !ok {
|
||||
var opts []codersdk.TemplateVersionParameterOption
|
||||
err := json.Unmarshal(param.Options, &opts)
|
||||
if err != nil {
|
||||
@@ -139,28 +165,24 @@ func TemplateInsightsParameters(parameterRows []database.GetTemplateParameterIns
|
||||
return nil, err
|
||||
}
|
||||
|
||||
parametersByNum[param.Num] = &codersdk.TemplateParameterUsage{
|
||||
parametersUsage = append(parametersUsage, codersdk.TemplateParameterUsage{
|
||||
TemplateIDs: param.TemplateIDs,
|
||||
Name: param.Name,
|
||||
Type: param.Type,
|
||||
DisplayName: param.DisplayName,
|
||||
Description: plaintextDescription,
|
||||
Options: opts,
|
||||
}
|
||||
})
|
||||
indexByNum[param.Num] = len(parametersUsage) - 1
|
||||
}
|
||||
parametersByNum[param.Num].Values = append(parametersByNum[param.Num].Values, codersdk.TemplateParameterValue{
|
||||
|
||||
i := indexByNum[param.Num]
|
||||
parametersUsage[i].Values = append(parametersUsage[i].Values, codersdk.TemplateParameterValue{
|
||||
Value: param.Value,
|
||||
Count: param.Count,
|
||||
})
|
||||
}
|
||||
parametersUsage := []codersdk.TemplateParameterUsage{}
|
||||
for _, param := range parametersByNum {
|
||||
parametersUsage = append(parametersUsage, *param)
|
||||
}
|
||||
|
||||
sort.Slice(parametersUsage, func(i, j int) bool {
|
||||
return parametersUsage[i].Name < parametersUsage[j].Name
|
||||
})
|
||||
return parametersUsage, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -2018,6 +2018,10 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if len(arg.TemplateIDs) > 0 && !slices.Contains(arg.TemplateIDs, w.TemplateID) {
|
||||
continue
|
||||
}
|
||||
|
||||
app, _ := q.getWorkspaceAppByAgentIDAndSlugNoLock(ctx, database.GetWorkspaceAppByAgentIDAndSlugParams{
|
||||
AgentID: s.AgentID,
|
||||
Slug: s.SlugOrPort,
|
||||
@@ -2095,6 +2099,8 @@ func (q *FakeQuerier) GetTemplateAppInsights(ctx context.Context, arg database.G
|
||||
})
|
||||
}
|
||||
|
||||
// NOTE(mafredri): Add sorting if we decide on how to handle PostgreSQL collations.
|
||||
// ORDER BY access_method, slug_or_port, display_name, icon, is_app
|
||||
return rows, nil
|
||||
}
|
||||
|
||||
@@ -2264,7 +2270,6 @@ func (q *FakeQuerier) GetTemplateDailyInsights(ctx context.Context, arg database
|
||||
}
|
||||
ds.userSet[s.UserID] = struct{}{}
|
||||
ds.templateIDSet[s.TemplateID] = struct{}{}
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2278,24 +2283,27 @@ func (q *FakeQuerier) GetTemplateDailyInsights(ctx context.Context, arg database
|
||||
continue
|
||||
}
|
||||
|
||||
w, err := q.getWorkspaceByIDNoLock(ctx, s.WorkspaceID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if len(arg.TemplateIDs) > 0 && !slices.Contains(arg.TemplateIDs, w.TemplateID) {
|
||||
continue
|
||||
}
|
||||
|
||||
for _, ds := range dailyStats {
|
||||
// (was.session_started_at >= ts.from_ AND was.session_started_at < ts.to_)
|
||||
// OR (was.session_ended_at > ts.from_ AND was.session_ended_at < ts.to_)
|
||||
// OR (was.session_started_at < ts.from_ AND was.session_ended_at >= ts.to_)
|
||||
if !(((s.SessionStartedAt.After(arg.StartTime) || s.SessionStartedAt.Equal(arg.StartTime)) && s.SessionStartedAt.Before(arg.EndTime)) ||
|
||||
(s.SessionEndedAt.After(arg.StartTime) && s.SessionEndedAt.Before(arg.EndTime)) ||
|
||||
(s.SessionStartedAt.Before(arg.StartTime) && (s.SessionEndedAt.After(arg.EndTime) || s.SessionEndedAt.Equal(arg.EndTime)))) {
|
||||
if !(((s.SessionStartedAt.After(ds.startTime) || s.SessionStartedAt.Equal(ds.startTime)) && s.SessionStartedAt.Before(ds.endTime)) ||
|
||||
(s.SessionEndedAt.After(ds.startTime) && s.SessionEndedAt.Before(ds.endTime)) ||
|
||||
(s.SessionStartedAt.Before(ds.startTime) && (s.SessionEndedAt.After(ds.endTime) || s.SessionEndedAt.Equal(ds.endTime)))) {
|
||||
continue
|
||||
}
|
||||
|
||||
w, err := q.getWorkspaceByIDNoLock(ctx, s.WorkspaceID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
ds.userSet[s.UserID] = struct{}{}
|
||||
ds.templateIDSet[w.TemplateID] = struct{}{}
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2430,7 +2438,8 @@ func (q *FakeQuerier) GetTemplateParameterInsights(ctx context.Context, arg data
|
||||
if tvp.TemplateVersionID != tv.ID {
|
||||
continue
|
||||
}
|
||||
key := fmt.Sprintf("%s:%s:%s:%s", tvp.Name, tvp.DisplayName, tvp.Description, tvp.Options)
|
||||
// GROUP BY tvp.name, tvp.type, tvp.display_name, tvp.description, tvp.options
|
||||
key := fmt.Sprintf("%s:%s:%s:%s:%s", tvp.Name, tvp.Type, tvp.DisplayName, tvp.Description, tvp.Options)
|
||||
if _, ok := uniqueTemplateParams[key]; !ok {
|
||||
num++
|
||||
uniqueTemplateParams[key] = &database.GetTemplateParameterInsightsRow{
|
||||
@@ -2480,6 +2489,8 @@ func (q *FakeQuerier) GetTemplateParameterInsights(ctx context.Context, arg data
|
||||
}
|
||||
}
|
||||
|
||||
// NOTE(mafredri): Add sorting if we decide on how to handle PostgreSQL collations.
|
||||
// ORDER BY utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value
|
||||
return rows, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -1788,13 +1788,13 @@ WITH latest_workspace_builds AS (
|
||||
array_agg(DISTINCT wb.template_id)::uuid[] AS template_ids,
|
||||
array_agg(wb.id)::uuid[] AS workspace_build_ids,
|
||||
tvp.name,
|
||||
tvp.type,
|
||||
tvp.display_name,
|
||||
tvp.description,
|
||||
tvp.options,
|
||||
tvp.type
|
||||
tvp.options
|
||||
FROM latest_workspace_builds wb
|
||||
JOIN template_version_parameters tvp ON (tvp.template_version_id = wb.template_version_id)
|
||||
GROUP BY tvp.name, tvp.display_name, tvp.description, tvp.options, tvp.type
|
||||
GROUP BY tvp.name, tvp.type, tvp.display_name, tvp.description, tvp.options
|
||||
)
|
||||
|
||||
SELECT
|
||||
@@ -1809,7 +1809,7 @@ SELECT
|
||||
COUNT(wbp.value) AS count
|
||||
FROM unique_template_params utp
|
||||
JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name)
|
||||
GROUP BY utp.num, utp.name, utp.display_name, utp.description, utp.options, utp.template_ids, utp.type, wbp.value
|
||||
GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value
|
||||
`
|
||||
|
||||
type GetTemplateParameterInsightsParams struct {
|
||||
|
||||
@@ -230,13 +230,13 @@ WITH latest_workspace_builds AS (
|
||||
array_agg(DISTINCT wb.template_id)::uuid[] AS template_ids,
|
||||
array_agg(wb.id)::uuid[] AS workspace_build_ids,
|
||||
tvp.name,
|
||||
tvp.type,
|
||||
tvp.display_name,
|
||||
tvp.description,
|
||||
tvp.options,
|
||||
tvp.type
|
||||
tvp.options
|
||||
FROM latest_workspace_builds wb
|
||||
JOIN template_version_parameters tvp ON (tvp.template_version_id = wb.template_version_id)
|
||||
GROUP BY tvp.name, tvp.display_name, tvp.description, tvp.options, tvp.type
|
||||
GROUP BY tvp.name, tvp.type, tvp.display_name, tvp.description, tvp.options
|
||||
)
|
||||
|
||||
SELECT
|
||||
@@ -251,4 +251,4 @@ SELECT
|
||||
COUNT(wbp.value) AS count
|
||||
FROM unique_template_params utp
|
||||
JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name)
|
||||
GROUP BY utp.num, utp.name, utp.display_name, utp.description, utp.options, utp.template_ids, utp.type, wbp.value;
|
||||
GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value;
|
||||
|
||||
Reference in New Issue
Block a user