mirror of
https://github.com/coder/coder.git
synced 2026-09-01 14:53:15 +08:00
Spike Curtis
f5d42a868e
This is the first of three PRs implementing lite `codersdk.Workspace` responses ([GRU-82](https://linear.app/codercom/issue/GRU-82), [RFC](https://app.notion.com/p/coderhq/Lite-Workspace-API-Requests-3aad579be5928060b5e7cc65c539717f)). A fully populated workspace fans out into a large tree of DB queries (template, latest build, provisioner job + queue position, resources, metadata, agents, apps, statuses, scripts, log sources, template version). Most callers do not need most of that data, and one of these queries, `GetProvisionerJobsByIDsWithQueuePosition`, is consistently the top resource consumer in scale tests. This PR adds the internal plumbing to load only a selected subset of that tree. It introduces a `workspaceRelated` selection type and threads it through `workspaceData` and `workspaceBuildsData` down to the individual query call sites, skipping any query whose data was not selected. The selection is a pointer tree that mirrors the parent/child relationships between the objects: branch nodes are pointers (non-nil when selected) and leaf nodes are bools. Modeling it this way makes selecting a child without its parent unrepresentable, which matches how loading works (a parent must be queried to learn its children's identifiers), so there is no normalization step to forget. No behavior change: every existing caller passes a fully populated selection (`allWorkspaceRelated` / `allLatestBuildRelated`), so responses are identical. Wiring the HTTP `include_related` query parameter (Phase 2) and narrowing individual callers (Phase 3) come in follow-up PRs. Two notes on the current mapping: - `queue_position` is modeled as its own node, but only one job query exists and it always computes the ranking, so selecting `job` runs the full query today. Splitting off a cheaper queue-position-free job query is deferred (it needs a new SQL query + `make gen`). - The workspace-level `LatestAppStatus` is gated on the `latest_build.resources.agents.apps.statuses` node. It is queried directly by workspace ID, but a caller selecting it also pulls in the build chain as ancestors. Happy to decouple this if preferred. <details> <summary>Implementation plan</summary> # Lite Workspace API — Phase 1 Plan Add an internal configuration structure to the Coderd method that fetches a workspace with its related data (`workspaceData` /`workspaceBuildsData`), thread it down to the SQL query call sites, and skip ("squash") queries that are not requested. Server-internal only. No HTTP query-string parsing (Phase 2) and no `wsrelated` struct tags or field nil-ing (Phase 3). ## Scope - In: config struct mapping the RFC related-data hierarchy; threading through `workspaceData` and `workspaceBuildsData`; gating every related-data query; comprehensive unit tests with a mocked `database.Store` (gomock/dbmock). - Out: HTTP `include_related` parsing, codersdk changes, `wsrelated` tags, docs autogen, and changing `convertWorkspaces` skip-on-missing-data behavior. All existing callers pass an "include all" config, so Phase 1 is a behavior- preserving refactor. ## Design New file `coderd/wsrelateddata.go` with a pointer tree that mirrors the RFC hierarchy. Branch nodes are pointers (present when non-nil); leaf nodes are bools. Because a child cannot be expressed without its parent, the RFC "includes all ancestors" rule is a structural invariant of the type: an illegal selection is unrepresentable, so no normalization pass exists or is needed. ```go type workspaceRelated struct { Template bool LatestBuild *latestBuildRelated } type latestBuildRelated struct { Job *jobRelated Resources *resourcesRelated TemplateVersion bool } type jobRelated struct{ QueuePosition bool } type resourcesRelated struct { Metadata bool Agents *agentsRelated } type agentsRelated struct { Apps *appsRelated Scripts bool LogSources bool } type appsRelated struct{ Statuses bool } ``` Helpers: - `allWorkspaceRelated()` / `allLatestBuildRelated()` construct a fully populated tree. Passed by all current callers so behavior is unchanged. - Zero value (nil subtree) includes nothing but the workspace itself. - `(*latestBuildRelated).appStatuses()` is a nil-safe accessor for the deep `apps.statuses` leaf so call sites descend the tree without a guard chain. Rationale for the pointer tree over a flat struct plus `withAncestors()`: the invariant holds by construction (illegal state is unrepresentable) instead of relying on a normalization step a caller must remember. `workspaceBuildsData` takes the `*latestBuildRelated` subtree directly. ### Query gating `workspaceData(ctx, workspaces, cfg)`: - `GetTemplatesWithFilter` gated on `Template`. - `GetLatestWorkspaceBuildsByWorkspaceIDs` gated on `LatestBuild`; when off, skip `workspaceBuildsData` entirely (empty builds). - `GetLatestWorkspaceAppStatusesByWorkspaceIDs` (workspace-level LatestAppStatus) gated on `AppStatuses`. `workspaceBuildsData(ctx, builds, cfg)` gates each stage: - jobs (`GetProvisionerJobsByIDsWithQueuePosition`) + eligible provisioner daemons gated on `Job`. Only one job query exists and it always computes the queue position, so `QueuePosition` does not gate a separate query yet; it is modeled for hierarchy/Phase 2 completeness. A cheaper job query without the window-function ranking is a worthwhile follow-up but is out of scope for a simple Phase 1 (needs new SQL + sqlc gen). - resources gated on `Resources`; metadata on `Metadata`; agents on `Agents`; apps on `Apps`; app statuses on `AppStatuses`; scripts on `Scripts`; log sources on `LogSources`; template versions on `TemplateVersion`. Downstream `convertWorkspaceBuilds` already tolerates empty slices, so squashing queries yields zero/nil fields without extra work. ## Testing (comprehensive, mocked database.Store) Internal tests (`coderd/wsrelateddata_internal_test.go`) build `&API{Options: &Options{Database: dbmock.NewMockStore(ctrl)}}` and call `workspaceData` directly. gomock is strict, so setting EXPECT only on permitted queries proves the rest are squashed. Cases: include-all (every query runs); zero value (no related queries run); each single node (template only, latest_build only, job, resources, metadata, agents, apps, statuses, scripts, log_sources, template_version, workspace-level app statuses); the empty-resources short-circuit; and the constructors plus the nil-safe `appStatuses()` accessor at every depth. ## Steps 1. Add `coderd/wsrelateddata.go` (selection tree, constructors, `appStatuses`). 2. Thread the selection through `workspaceData` and `workspaceBuildsData`; gate queries by descending the tree. 3. Update the callers to pass `allWorkspaceRelated()` / `allLatestBuildRelated()`. 4. Add unit tests; run `make fmt`, `make lint`, targeted `go test`. ## Resolved decisions 1. Pointer tree over a flat struct + normalizer, so the ancestor rule is a structural invariant. 2. Cheaper queue-position-free job query deferred to a follow-up. </details> --- *Generated by Coder Agents on behalf of @spikecurtis.*
88 lines
3.0 KiB
Go
88 lines
3.0 KiB
Go
package coderd
|
|
|
|
// workspaceRelated selects which workspace-related database objects to load when
|
|
// building a codersdk.Workspace. Loading a fully populated workspace is
|
|
// expensive, so callers use this to avoid querying data they will not use.
|
|
//
|
|
// The type is a tree that mirrors the parent/child relationships between those
|
|
// objects: a build has a job, resources, and a template version; a resource has
|
|
// agents; an agent has apps; and so on. Branch nodes are pointers that are
|
|
// non-nil when selected; leaf nodes are bools. Modeling it as a tree makes
|
|
// selecting a child without its parent unrepresentable, which is exactly the
|
|
// constraint loading requires: a parent must be queried to learn the
|
|
// identifiers of its children.
|
|
//
|
|
// A zero value (nil branches) selects nothing but the workspace itself.
|
|
// allWorkspaceRelated selects everything.
|
|
//
|
|
// The trailing comment on each field is that node's dotted path from the root
|
|
// of the tree, e.g. latest_build.resources.agents.
|
|
type workspaceRelated struct {
|
|
Template bool // template
|
|
LatestBuild *latestBuildRelated // latest_build
|
|
}
|
|
|
|
type latestBuildRelated struct {
|
|
Job *jobRelated // latest_build.job
|
|
Resources *resourcesRelated // latest_build.resources
|
|
TemplateVersion bool // latest_build.template_version
|
|
}
|
|
|
|
type jobRelated struct {
|
|
QueuePosition bool // latest_build.job.queue_position
|
|
}
|
|
|
|
type resourcesRelated struct {
|
|
Metadata bool // latest_build.resources.metadata
|
|
Agents *agentsRelated // latest_build.resources.agents
|
|
}
|
|
|
|
type agentsRelated struct {
|
|
Apps *appsRelated // latest_build.resources.agents.apps
|
|
Scripts bool // latest_build.resources.agents.scripts
|
|
LogSources bool // latest_build.resources.agents.log_sources
|
|
}
|
|
|
|
type appsRelated struct {
|
|
Statuses bool // latest_build.resources.agents.apps.statuses
|
|
}
|
|
|
|
// allWorkspaceRelated returns a selection that loads every related object. It
|
|
// reproduces the behavior of callers that have not been narrowed to a specific
|
|
// subset.
|
|
func allWorkspaceRelated() workspaceRelated {
|
|
latestBuild := allLatestBuildRelated()
|
|
return workspaceRelated{
|
|
Template: true,
|
|
LatestBuild: &latestBuild,
|
|
}
|
|
}
|
|
|
|
// allLatestBuildRelated returns the latest-build subtree with every node
|
|
// selected.
|
|
func allLatestBuildRelated() latestBuildRelated {
|
|
return latestBuildRelated{
|
|
Job: &jobRelated{QueuePosition: true},
|
|
Resources: &resourcesRelated{
|
|
Metadata: true,
|
|
Agents: &agentsRelated{
|
|
Apps: &appsRelated{Statuses: true},
|
|
Scripts: true,
|
|
LogSources: true,
|
|
},
|
|
},
|
|
TemplateVersion: true,
|
|
}
|
|
}
|
|
|
|
// appStatuses reports whether app statuses
|
|
// (latest_build.resources.agents.apps.statuses) are selected. It is nil-safe so
|
|
// callers holding a possibly-nil subtree can descend without a chain of guards.
|
|
func (c *latestBuildRelated) appStatuses() bool {
|
|
return c != nil &&
|
|
c.Resources != nil &&
|
|
c.Resources.Agents != nil &&
|
|
c.Resources.Agents.Apps != nil &&
|
|
c.Resources.Agents.Apps.Statuses
|
|
}
|