synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-06 15:41:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: refactor time.Duration -> int64 milliseconds for FE consumption (#1944)

Browse Source 
* Changes all public-facing codersdk types to use a plain int64 (milliseconds) instead of time.Duration.
* Makes autostart_schedule a *string as it may not be present.
* Adds a utils/ptr package with some useful methods.
This commit is contained in:
Cian Johnston
2022-06-02 11:23:34 +01:00
committed by GitHub
parent 51c420c90a
commit dcf03d8ba3
24 changed files with 287 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
coderd/autobuild/executor/lifecycle_executor_test.go
View File

@ -14,6 +14,7 @@ import (
"github.com/coder/coder/coderd/autobuild/schedule"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
"github.com/google/uuid"
@ -44,7 +45,7 @@ func TestExecutorAutostartOK(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))
// When: the autobuild executor ticks
@ -95,7 +96,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))
// When: the autobuild executor ticks
@ -138,7 +139,7 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))
// When: the autobuild executor ticks
@ -316,12 +317,12 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
})
// Given: we have a user with a workspace that has no TTL set
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TTL = nil
cwr.TTLMillis = nil
})
)
// Given: workspace has no TTL set
require.Nil(t, workspace.TTL)
require.Nil(t, workspace.TTLMillis)
// Given: workspace is running
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
@ -359,7 +360,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))
// Given: workspace is deleted
@ -402,7 +403,7 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) {
sched, err := schedule.Weekly(futureTimeCron)
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))
// When: the autobuild executor ticks
@ -461,7 +462,7 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
)
// Given: the user changes their mind and decides their workspace should not auto-stop
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTL: nil})
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
require.NoError(t, err)
// When: the autobuild executor ticks after the deadline

9
coderd/coderdtest/coderdtest.go
View File

@ -26,6 +26,7 @@ import (
"time"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/util/ptr"
"cloud.google.com/go/compute/metadata"
"github.com/fullsailor/pkcs7"
@ -399,8 +400,8 @@ func CreateWorkspace(t *testing.T, client *codersdk.Client, organization uuid.UU
req := codersdk.CreateWorkspaceRequest{
TemplateID: templateID,
Name: randomUsername(),
AutostartSchedule: ptr("CRON_TZ=US/Central * * * * *"),
TTL: ptr(8 * time.Hour),
AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"),
TTLMillis: ptr.Ref((8 * time.Hour).Milliseconds()),
}
for _, mutator := range mutators {
mutator(&req)
@ -602,7 +603,3 @@ type roundTripper func(req *http.Request) (*http.Response, error)
func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
return r(req)
}
func ptr[T any](x T) *T {
return &x
}

23
coderd/util/ptr/ptr.go Normal file
View File

@ -0,0 +1,23 @@
// Package ptr contains some utility methods related to pointers.
package ptr
import "golang.org/x/exp/constraints"
type number interface {
constraints.Integer | constraints.Float
}
// Ref returns a reference to v.
func Ref[T any](v T) *T {
return &v
}
// NilOrEmpty returns true if s is nil or the empty string.
func NilOrEmpty(s *string) bool {
return s == nil || *s == ""
}
// NilOrZero returns true if v is nil or 0.
func NilOrZero[T number](v *T) bool {
return v == nil || *v == 0
}

81
coderd/util/ptr/ptr_test.go Normal file
View File

@ -0,0 +1,81 @@
package ptr_test
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/coder/coder/coderd/util/ptr"
)
func Test_Ref_Deref(t *testing.T) {
t.Parallel()
t.Run("String", func(t *testing.T) {
t.Parallel()
val := "test"
p := ptr.Ref(val)
assert.Equal(t, &val, p)
})
t.Run("Bool", func(t *testing.T) {
t.Parallel()
val := true
p := ptr.Ref(val)
assert.Equal(t, &val, p)
})
t.Run("Int64", func(t *testing.T) {
t.Parallel()
val := int64(42)
p := ptr.Ref(val)
assert.Equal(t, &val, p)
})
t.Run("Float64", func(t *testing.T) {
t.Parallel()
val := float64(3.14159)
p := ptr.Ref(val)
assert.Equal(t, &val, p)
})
}
func Test_NilOrEmpty(t *testing.T) {
t.Parallel()
nilString := (*string)(nil)
emptyString := ""
nonEmptyString := "hi"
assert.True(t, ptr.NilOrEmpty(nilString))
assert.True(t, ptr.NilOrEmpty(&emptyString))
assert.False(t, ptr.NilOrEmpty(&nonEmptyString))
}
func Test_NilOrZero(t *testing.T) {
t.Parallel()
nilInt64 := (*int64)(nil)
nilFloat64 := (*float64)(nil)
nilDuration := (*time.Duration)(nil)
zeroInt64 := int64(0)
zeroFloat64 := float64(0.0)
zeroDuration := time.Duration(0)
nonZeroInt64 := int64(1)
nonZeroFloat64 := float64(3.14159)
nonZeroDuration := time.Hour
assert.True(t, ptr.NilOrZero(nilInt64))
assert.True(t, ptr.NilOrZero(nilFloat64))
assert.True(t, ptr.NilOrZero(nilDuration))
assert.True(t, ptr.NilOrZero(&zeroInt64))
assert.True(t, ptr.NilOrZero(&zeroFloat64))
assert.True(t, ptr.NilOrZero(&zeroDuration))
assert.False(t, ptr.NilOrZero(&nonZeroInt64))
assert.False(t, ptr.NilOrZero(&nonZeroFloat64))
assert.False(t, ptr.NilOrZero(&nonZeroDuration))
}

63
coderd/workspaces.go
View File

@ -25,6 +25,7 @@ import (
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)
@ -345,7 +346,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
dbAutostartSchedule.String = *createWorkspace.AutostartSchedule
}
dbTTL, err := validWorkspaceTTL(createWorkspace.TTL)
dbTTL, err := validWorkspaceTTLMillis(createWorkspace.TTLMillis)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "validate workspace ttl",
@ -527,20 +528,15 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) {
return
}
var dbSched sql.NullString
if req.Schedule != "" {
validSched, err := schedule.Weekly(req.Schedule)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("invalid autostart schedule: %s", err),
})
return
}
dbSched.String = validSched.String()
dbSched.Valid = true
dbSched, err := validWorkspaceSchedule(req.Schedule)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("invalid autostart schedule: %s", err),
})
return
}
err := api.Database.UpdateWorkspaceAutostart(r.Context(), database.UpdateWorkspaceAutostartParams{
err = api.Database.UpdateWorkspaceAutostart(r.Context(), database.UpdateWorkspaceAutostartParams{
ID: workspace.ID,
AutostartSchedule: dbSched,
})
@ -564,7 +560,7 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
return
}
dbTTL, err := validWorkspaceTTL(req.TTL)
dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "validate workspace ttl",
@ -830,13 +826,18 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data
}
return apiWorkspaces, nil
}
func convertWorkspace(
workspace database.Workspace,
workspaceBuild database.WorkspaceBuild,
job database.ProvisionerJob,
template database.Template,
owner database.User) codersdk.Workspace {
var autostartSchedule *string
if workspace.AutostartSchedule.Valid {
autostartSchedule = &workspace.AutostartSchedule.String
}
ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl)
return codersdk.Workspace{
ID: workspace.ID,
CreatedAt: workspace.CreatedAt,
@ -848,25 +849,27 @@ func convertWorkspace(
TemplateName: template.Name,
Outdated: workspaceBuild.TemplateVersionID.String() != template.ActiveVersionID.String(),
Name: workspace.Name,
AutostartSchedule: workspace.AutostartSchedule.String,
TTL: convertSQLNullInt64(workspace.Ttl),
AutostartSchedule: autostartSchedule,
TTLMillis: ttlMillis,
}
}
func convertSQLNullInt64(i sql.NullInt64) *time.Duration {
func convertWorkspaceTTLMillis(i sql.NullInt64) *int64 {
if !i.Valid {
return nil
}
return (*time.Duration)(&i.Int64)
millis := time.Duration(i.Int64).Milliseconds()
return &millis
}
func validWorkspaceTTL(ttl *time.Duration) (sql.NullInt64, error) {
if ttl == nil {
func validWorkspaceTTLMillis(millis *int64) (sql.NullInt64, error) {
if ptr.NilOrZero(millis) {
return sql.NullInt64{}, nil
}
truncated := ttl.Truncate(time.Minute)
dur := time.Duration(*millis) * time.Millisecond
truncated := dur.Truncate(time.Minute)
if truncated < time.Minute {
return sql.NullInt64{}, xerrors.New("ttl must be at least one minute")
}
@ -902,3 +905,19 @@ func validWorkspaceDeadline(old, new time.Time) error {
return nil
}
func validWorkspaceSchedule(s *string) (sql.NullString, error) {
if ptr.NilOrEmpty(s) {
return sql.NullString{}, nil
}
_, err := schedule.Weekly(*s)
if err != nil {
return sql.NullString{}, err
}
return sql.NullString{
Valid: true,
String: *s,
}, nil
}

71
coderd/workspaces_test.go
View File

@ -8,6 +8,7 @@ import (
"time"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/util/ptr"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
@ -177,8 +178,8 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
req := codersdk.CreateWorkspaceRequest{
TemplateID: template.ID,
Name: "testing",
AutostartSchedule: ptr("CRON_TZ=US/Central * * * * *"),
TTL: ptr(59 * time.Second),
AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"),
TTLMillis: ptr.Ref((59 * time.Second).Milliseconds()),
}
_, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req)
require.Error(t, err)
@ -197,8 +198,8 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
req := codersdk.CreateWorkspaceRequest{
TemplateID: template.ID,
Name: "testing",
AutostartSchedule: ptr("CRON_TZ=US/Central * * * * *"),
TTL: ptr(24*7*time.Hour + time.Minute),
AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"),
TTLMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()),
}
_, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req)
require.Error(t, err)
@ -451,7 +452,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
testCases := []struct {
name string
schedule string
schedule *string
expectedError string
at time.Time
expectedNext time.Time
@ -459,12 +460,12 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
}{
{
name: "disable autostart",
schedule: "",
schedule: ptr.Ref(""),
expectedError: "",
},
{
name: "friday to monday",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * 1-5",
schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 * * 1-5"),
expectedError: "",
at: time.Date(2022, 5, 6, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 9, 9, 30, 0, 0, dublinLoc),
@ -472,7 +473,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
},
{
name: "monday to tuesday",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * 1-5",
schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 * * 1-5"),
expectedError: "",
at: time.Date(2022, 5, 9, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 5, 10, 9, 30, 0, 0, dublinLoc),
@ -481,7 +482,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
{
// DST in Ireland began on Mar 27 in 2022 at 0100. Forward 1 hour.
name: "DST start",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * *",
schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 * * *"),
expectedError: "",
at: time.Date(2022, 3, 26, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 3, 27, 9, 30, 0, 0, dublinLoc),
@ -490,7 +491,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
{
// DST in Ireland ends on Oct 30 in 2022 at 0200. Back 1 hour.
name: "DST end",
schedule: "CRON_TZ=Europe/Dublin 30 9 * * *",
schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 * * *"),
expectedError: "",
at: time.Date(2022, 10, 29, 9, 31, 0, 0, dublinLoc),
expectedNext: time.Date(2022, 10, 30, 9, 30, 0, 0, dublinLoc),
@ -498,17 +499,17 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
},
{
name: "invalid location",
schedule: "CRON_TZ=Imaginary/Place 30 9 * * 1-5",
schedule: ptr.Ref("CRON_TZ=Imaginary/Place 30 9 * * 1-5"),
expectedError: "status code 500: invalid autostart schedule: parse schedule: provided bad location Imaginary/Place: unknown time zone Imaginary/Place",
},
{
name: "invalid schedule",
schedule: "asdf asdf asdf ",
schedule: ptr.Ref("asdf asdf asdf "),
expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
{
name: "only 3 values",
schedule: "CRON_TZ=Europe/Dublin 30 9 *",
schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 *"),
expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ=<timezone> prefix`,
},
}
@ -526,7 +527,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.AutostartSchedule = nil
cwr.TTL = nil
cwr.TTLMillis = nil
})
)
@ -547,12 +548,14 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, testCase.schedule, updated.AutostartSchedule, "expected autostart schedule to equal requested")
if testCase.schedule == "" {
if testCase.schedule == nil || *testCase.schedule == "" {
require.Nil(t, updated.AutostartSchedule)
return
}
sched, err := schedule.Weekly(updated.AutostartSchedule)
require.EqualValues(t, *testCase.schedule, *updated.AutostartSchedule, "expected autostart schedule to equal requested")
sched, err := schedule.Weekly(*updated.AutostartSchedule)
require.NoError(t, err, "parse returned schedule")
next := sched.Next(testCase.at)
@ -569,7 +572,7 @@ func TestWorkspaceUpdateAutostart(t *testing.T) {
_ = coderdtest.CreateFirstUser(t, client)
wsid = uuid.New()
req = codersdk.UpdateWorkspaceAutostartRequest{
Schedule: "9 30 1-5",
Schedule: ptr.Ref("9 30 1-5"),
}
)
@ -586,32 +589,32 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
testCases := []struct {
name string
ttl *time.Duration
ttlMillis *int64
expectedError string
}{
{
name: "disable ttl",
ttl: nil,
ttlMillis: nil,
expectedError: "",
},
{
name: "below minimum ttl",
ttl: ptr(30 * time.Second),
ttlMillis: ptr.Ref((30 * time.Second).Milliseconds()),
expectedError: "ttl must be at least one minute",
},
{
name: "minimum ttl",
ttl: ptr(time.Minute),
ttlMillis: ptr.Ref(time.Minute.Milliseconds()),
expectedError: "",
},
{
name: "maximum ttl",
ttl: ptr(24 * 7 * time.Hour),
ttlMillis: ptr.Ref((24 * 7 * time.Hour).Milliseconds()),
expectedError: "",
},
{
name: "above maximum ttl",
ttl: ptr(24*7*time.Hour + time.Minute),
ttlMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()),
expectedError: "ttl must be less than 7 days",
},
}
@ -629,15 +632,15 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.AutostartSchedule = nil
cwr.TTL = nil
cwr.TTLMillis = nil
})
)
// ensure test invariant: new workspaces have no autostop schedule.
require.Nil(t, workspace.TTL, "expected newly-minted workspace to have no TTL")
require.Nil(t, workspace.TTLMillis, "expected newly-minted workspace to have no TTL")
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: testCase.ttl,
TTLMillis: testCase.ttlMillis,
})
if testCase.expectedError != "" {
@ -650,7 +653,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, testCase.ttl, updated.TTL, "expected autostop ttl to equal requested")
require.Equal(t, testCase.ttlMillis, updated.TTLMillis, "expected autostop ttl to equal requested")
})
}
@ -661,7 +664,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
_ = coderdtest.CreateFirstUser(t, client)
wsid = uuid.New()
req = codersdk.UpdateWorkspaceTTLRequest{
TTL: ptr(time.Hour),
TTLMillis: ptr.Ref(time.Hour.Milliseconds()),
}
)
@ -685,8 +688,8 @@ func TestWorkspaceExtend(t *testing.T) {
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID)
extend = 90 * time.Minute
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
oldDeadline = time.Now().Add(*workspace.TTL).UTC()
newDeadline = time.Now().Add(*workspace.TTL + extend).UTC()
oldDeadline = time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond).UTC()
newDeadline = time.Now().Add(time.Duration(*workspace.TTLMillis)*time.Millisecond + extend).UTC()
)
workspace, err := client.Workspace(ctx, workspace.ID)
@ -761,7 +764,3 @@ func mustLocation(t *testing.T, location string) *time.Location {
return loc
}
func ptr[T any](x T) *T {
return &x
}