From be00e2541c45bc09db32cd088da164cce1988ae6 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 2 Feb 2023 11:59:43 -0600 Subject: [PATCH] chore: remove unused `workspace_owner_count` field (#5958) This added unnecessary database load, because it's not used! --- coderd/apidoc/docs.go | 3 - coderd/apidoc/swagger.json | 3 - coderd/database/databasefake/databasefake.go | 35 -------- coderd/database/querier.go | 1 - coderd/database/queries.sql.go | 42 --------- coderd/database/queries/workspaces.sql | 13 --- coderd/templates.go | 94 ++------------------ coderd/templates_test.go | 35 -------- codersdk/templates.go | 17 ++-- docs/api/schemas.md | 4 +- docs/api/templates.md | 16 ++-- site/src/api/typesGenerated.ts | 1 - site/src/testHelpers/entities.ts | 1 - 13 files changed, 23 insertions(+), 242 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index dcd0f2f434..fb5236fe61 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -7345,9 +7345,6 @@ const docTemplate = `{ "updated_at": { "type": "string", "format": "date-time" - }, - "workspace_owner_count": { - "type": "integer" } } }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index fd7d5553ad..0d6c224dbc 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6602,9 +6602,6 @@ "updated_at": { "type": "string", "format": "date-time" - }, - "workspace_owner_count": { - "type": "integer" } } }, diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 8e40e30af9..800bf3d48b 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1272,41 +1272,6 @@ func (q *fakeQuerier) GetWorkspaceAppsByAgentIDs(_ context.Context, ids []uuid.U return apps, nil } -func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - counts := map[uuid.UUID]map[uuid.UUID]struct{}{} - for _, templateID := range templateIDs { - counts[templateID] = map[uuid.UUID]struct{}{} - for _, workspace := range q.workspaces { - if workspace.TemplateID != templateID { - continue - } - if workspace.Deleted { - continue - } - countByOwnerID, ok := counts[templateID] - if !ok { - countByOwnerID = map[uuid.UUID]struct{}{} - } - countByOwnerID[workspace.OwnerID] = struct{}{} - counts[templateID] = countByOwnerID - } - } - res := make([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, 0) - for key, value := range counts { - res = append(res, database.GetWorkspaceOwnerCountsByTemplateIDsRow{ - TemplateID: key, - Count: int64(len(value)), - }) - } - if len(res) == 0 { - return nil, sql.ErrNoRows - } - return res, nil -} - func (q *fakeQuerier) GetWorkspaceBuildByID(_ context.Context, id uuid.UUID) (database.WorkspaceBuild, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 7747371624..f930af0a94 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -121,7 +121,6 @@ type sqlcQuerier interface { GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUID) (Workspace, error) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWorkspaceByOwnerIDAndNameParams) (Workspace, error) - GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error) GetWorkspaceResourceByID(ctx context.Context, id uuid.UUID) (WorkspaceResource, error) GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceResourceMetadatum, error) GetWorkspaceResourceMetadataCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceResourceMetadatum, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 714ff68e53..0844e2dd48 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6477,48 +6477,6 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo return i, err } -const getWorkspaceOwnerCountsByTemplateIDs = `-- name: GetWorkspaceOwnerCountsByTemplateIDs :many -SELECT - template_id, - COUNT(DISTINCT owner_id) -FROM - workspaces -WHERE - template_id = ANY($1 :: uuid [ ]) - -- Ignore deleted workspaces - AND deleted != true -GROUP BY - template_id -` - -type GetWorkspaceOwnerCountsByTemplateIDsRow struct { - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - Count int64 `db:"count" json:"count"` -} - -func (q *sqlQuerier) GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceOwnerCountsByTemplateIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []GetWorkspaceOwnerCountsByTemplateIDsRow - for rows.Next() { - var i GetWorkspaceOwnerCountsByTemplateIDsRow - if err := rows.Scan(&i.TemplateID, &i.Count); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getWorkspaces = `-- name: GetWorkspaces :many SELECT workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, COUNT(*) OVER () as count diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 4221364ca0..e32e68a6fc 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -223,19 +223,6 @@ WHERE AND LOWER("name") = LOWER(@name) ORDER BY created_at DESC; --- name: GetWorkspaceOwnerCountsByTemplateIDs :many -SELECT - template_id, - COUNT(DISTINCT owner_id) -FROM - workspaces -WHERE - template_id = ANY(@ids :: uuid [ ]) - -- Ignore deleted workspaces - AND deleted != true -GROUP BY - template_id; - -- name: InsertWorkspace :one INSERT INTO workspaces ( diff --git a/coderd/templates.go b/coderd/templates.go index ad91c0602e..febe8ab84c 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -42,23 +42,6 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) { return } - workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID}) - if errors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace count.", - Detail: err.Error(), - }) - return - } - - count := uint32(0) - if len(workspaceCounts) > 0 { - count = uint32(workspaceCounts[0].Count) - } - createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, []database.Template{template}) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -68,7 +51,7 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()])) + httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, createdByNameMap[template.ID.String()])) } // @Summary Delete template by ID @@ -313,7 +296,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque return xerrors.Errorf("get creator name: %w", err) } - template = api.convertTemplate(dbTemplate, 0, createdByNameMap[dbTemplate.ID.String()]) + template = api.convertTemplate(dbTemplate, createdByNameMap[dbTemplate.ID.String()]) return nil }, nil) if err != nil { @@ -369,23 +352,6 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request) return } - templateIDs := make([]uuid.UUID, 0, len(templates)) - - for _, template := range templates { - templateIDs = append(templateIDs, template.ID) - } - workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, templateIDs) - if errors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace counts.", - Detail: err.Error(), - }) - return - } - createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, templates) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -395,7 +361,7 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request) return } - httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplates(templates, workspaceCounts, createdByNameMap)) + httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplates(templates, createdByNameMap)) } // @Summary Get templates by organization and template name @@ -433,23 +399,6 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re return } - workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID}) - if errors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching workspace counts.", - Detail: err.Error(), - }) - return - } - - count := uint32(0) - if len(workspaceCounts) > 0 { - count = uint32(workspaceCounts[0].Count) - } - createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, []database.Template{template}) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -459,7 +408,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re return } - httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()])) + httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, createdByNameMap[template.ID.String()])) } // @Summary Update template metadata by ID @@ -508,22 +457,8 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { return } - count := uint32(0) var updated database.Template err := api.Database.InTx(func(tx database.Store) error { - // Fetch workspace counts - workspaceCounts, err := tx.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID}) - if xerrors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - return err - } - - if len(workspaceCounts) > 0 { - count = uint32(workspaceCounts[0].Count) - } - if req.Name == template.Name && req.Description == template.Description && req.DisplayName == template.DisplayName && @@ -550,6 +485,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { desc = template.Description } + var err error updated, err = tx.UpdateTemplateMetaByID(ctx, database.UpdateTemplateMetaByIDParams{ ID: template.ID, UpdatedAt: database.Now(), @@ -587,7 +523,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(updated, count, createdByNameMap[updated.ID.String()])) + httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(updated, createdByNameMap[updated.ID.String()])) } // @Summary Get template DAUs by ID @@ -659,22 +595,11 @@ func getCreatedByNamesByTemplateIDs(ctx context.Context, db database.Store, temp return creators, nil } -func (api *API) convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow, createdByNameMap map[string]string) []codersdk.Template { +func (api *API) convertTemplates(templates []database.Template, createdByNameMap map[string]string) []codersdk.Template { apiTemplates := make([]codersdk.Template, 0, len(templates)) for _, template := range templates { - found := false - for _, workspaceCount := range workspaceCounts { - if workspaceCount.TemplateID.String() != template.ID.String() { - continue - } - apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(workspaceCount.Count), createdByNameMap[template.ID.String()])) - found = true - break - } - if !found { - apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(0), createdByNameMap[template.ID.String()])) - } + apiTemplates = append(apiTemplates, api.convertTemplate(template, createdByNameMap[template.ID.String()])) } // Sort templates by ActiveUserCount DESC @@ -686,7 +611,7 @@ func (api *API) convertTemplates(templates []database.Template, workspaceCounts } func (api *API) convertTemplate( - template database.Template, workspaceOwnerCount uint32, createdByName string, + template database.Template, createdByName string, ) codersdk.Template { activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) @@ -701,7 +626,6 @@ func (api *API) convertTemplate( DisplayName: template.DisplayName, Provisioner: codersdk.ProvisionerType(template.Provisioner), ActiveVersionID: template.ActiveVersionID, - WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, BuildTimeStats: buildTimeStats, Description: template.Description, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 73a846fe35..4a3b9aed7f 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -15,7 +15,6 @@ import ( "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" - "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/util/ptr" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" @@ -40,40 +39,6 @@ func TestTemplate(t *testing.T) { _, err := client.Template(ctx, template.ID) require.NoError(t, err) }) - - t.Run("WorkspaceCount", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - user := coderdtest.CreateFirstUser(t, client) - member := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner()) - memberWithDeleted := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner()) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - - // Create 3 workspaces with 3 users. 2 workspaces exist, 1 is deleted - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - memberWorkspace := coderdtest.CreateWorkspace(t, member, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, member, memberWorkspace.LatestBuild.ID) - - deletedWorkspace := coderdtest.CreateWorkspace(t, memberWithDeleted, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, deletedWorkspace.LatestBuild.ID) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - build, err := client.CreateWorkspaceBuild(ctx, deletedWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{ - Transition: codersdk.WorkspaceTransitionDelete, - }) - require.NoError(t, err) - coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) - - template, err = client.Template(ctx, template.ID) - require.NoError(t, err) - require.Equal(t, 2, int(template.WorkspaceOwnerCount), "workspace count") - }) } func TestPostTemplateByOrganization(t *testing.T) { diff --git a/codersdk/templates.go b/codersdk/templates.go index faa9615a78..3ee517a041 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -14,15 +14,14 @@ import ( // Template is the JSON representation of a Coder template. This type matches the // database object for now, but is abstracted for ease of change later on. type Template struct { - ID uuid.UUID `json:"id" format:"uuid"` - CreatedAt time.Time `json:"created_at" format:"date-time"` - UpdatedAt time.Time `json:"updated_at" format:"date-time"` - OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` - Name string `json:"name"` - DisplayName string `json:"display_name"` - Provisioner ProvisionerType `json:"provisioner" enums:"terraform"` - ActiveVersionID uuid.UUID `json:"active_version_id" format:"uuid"` - WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` + ID uuid.UUID `json:"id" format:"uuid"` + CreatedAt time.Time `json:"created_at" format:"date-time"` + UpdatedAt time.Time `json:"updated_at" format:"date-time"` + OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + Provisioner ProvisionerType `json:"provisioner" enums:"terraform"` + ActiveVersionID uuid.UUID `json:"active_version_id" format:"uuid"` // ActiveUserCount is set to -1 when loading. ActiveUserCount int `json:"active_user_count"` BuildTimeStats TemplateBuildTimeStats `json:"build_time_stats"` diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 7a84fd2900..da33460483 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4240,8 +4240,7 @@ Parameter represents a set value for the scope. "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ``` @@ -4265,7 +4264,6 @@ Parameter represents a set value for the scope. | `organization_id` | string | false | | | | `provisioner` | string | false | | | | `updated_at` | string | false | | | -| `workspace_owner_count` | integer | false | | | #### Enumerated Values diff --git a/docs/api/templates.md b/docs/api/templates.md index 24c23594d9..2d62e30f5c 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -121,8 +121,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ] ``` @@ -159,7 +158,6 @@ Status Code **200** | `» organization_id` | string(uuid) | false | | | | `» provisioner` | string | false | | | | `» updated_at` | string(date-time) | false | | | -| `» workspace_owner_count` | integer | false | | | #### Enumerated Values @@ -243,8 +241,7 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/templa "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ``` @@ -366,8 +363,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ``` @@ -677,8 +673,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template} \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ``` @@ -783,8 +778,7 @@ curl -X PATCH http://coder-server:8080/api/v2/templates/{template} \ "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "provisioner": "terraform", - "updated_at": "2019-08-24T14:15:22Z", - "workspace_owner_count": 0 + "updated_at": "2019-08-24T14:15:22Z" } ``` diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d5e18b83b4..6474f676e4 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -685,7 +685,6 @@ export interface Template { readonly display_name: string readonly provisioner: ProvisionerType readonly active_version_id: string - readonly workspace_owner_count: number readonly active_user_count: number readonly build_time_stats: TemplateBuildTimeStats readonly description: string diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index bfb7bc6a6d..89b684584b 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -247,7 +247,6 @@ export const MockTemplate: TypesGen.Template = { display_name: "Test Template", provisioner: MockProvisioner.provisioners[0], active_version_id: MockTemplateVersion.id, - workspace_owner_count: 2, active_user_count: 1, build_time_stats: { start: {