Skip to content

chore: refactor audit page to use window function for count #5133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Nov 21, 2022
Merged
44 changes: 9 additions & 35 deletions coderd/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,45 +57,19 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
httpapi.InternalServerError(rw, err)
return
}

httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
AuditLogs: convertAuditLogs(dblogs),
})
}

func (api *API) auditLogCount(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAuditLog) {
httpapi.Forbidden(rw)
return
}

queryStr := r.URL.Query().Get("q")
filter, errs := auditSearchQuery(queryStr)
if len(errs) > 0 {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid audit search query.",
Validations: errs,
// GetAuditLogsOffset does not return ErrNoRows because it uses a window function to get the count.
// So we need to check if the dblogs is empty and return an empty array if so.
if len(dblogs) == 0 {
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
AuditLogs: []codersdk.AuditLog{},
Count: 0,
})
return
}

count, err := api.Database.GetAuditLogCount(ctx, database.GetAuditLogCountParams{
ResourceType: filter.ResourceType,
ResourceID: filter.ResourceID,
Action: filter.Action,
Username: filter.Username,
Email: filter.Email,
DateFrom: filter.DateFrom,
DateTo: filter.DateTo,
})
if err != nil {
httpapi.InternalServerError(rw, err)
return
}

httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogCountResponse{
Count: count,
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
AuditLogs: convertAuditLogs(dblogs),
Count: dblogs[0].Count,
})
}

Expand Down
16 changes: 2 additions & 14 deletions coderd/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@ func TestAuditLogs(t *testing.T) {
err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{})
require.NoError(t, err)

count, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{})
require.NoError(t, err)

alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{
Pagination: codersdk.Pagination{
Limit: 1,
},
})
require.NoError(t, err)

require.Equal(t, int64(1), count.Count)
require.Equal(t, int64(1), alogs.Count)
require.Len(t, alogs.AuditLogs, 1)
})
}
Expand Down Expand Up @@ -161,16 +158,7 @@ func TestAuditLogsFilter(t *testing.T) {
})
require.NoError(t, err, "fetch audit logs")
require.Len(t, auditLogs.AuditLogs, testCase.ExpectedResult, "expected audit logs returned")
})

// Test count filtering
t.Run("GetCount"+testCase.Name, func(t *testing.T) {
t.Parallel()
response, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{
SearchQuery: testCase.SearchQuery,
})
require.NoError(t, err, "fetch audit logs count")
require.Equal(t, int(response.Count), testCase.ExpectedResult, "expected audit logs count returned")
require.Equal(t, testCase.ExpectedResult, int(auditLogs.Count), "expected audit log count returned")
})
}
})
Expand Down
1 change: 0 additions & 1 deletion coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@ func New(options *Options) *API {
)

r.Get("/", api.auditLogs)
r.Get("/count", api.auditLogCount)
r.Post("/testgenerate", api.generateFakeAuditLog)
})
r.Route("/files", func(r chi.Router) {
Expand Down
49 changes: 5 additions & 44 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -3054,59 +3054,20 @@ func (q *fakeQuerier) GetAuditLogsOffset(ctx context.Context, arg database.GetAu
UserCreatedAt: sql.NullTime{Time: user.CreatedAt, Valid: userValid},
UserStatus: user.Status,
UserRoles: user.RBACRoles,
Count: 0,
})

if len(logs) >= int(arg.Limit) {
break
}
}

return logs, nil
}

func (q *fakeQuerier) GetAuditLogCount(_ context.Context, arg database.GetAuditLogCountParams) (int64, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()

logs := make([]database.AuditLog, 0)

for _, alog := range q.auditLogs {
if arg.Action != "" && !strings.Contains(string(alog.Action), arg.Action) {
continue
}
if arg.ResourceType != "" && !strings.Contains(string(alog.ResourceType), arg.ResourceType) {
continue
}
if arg.ResourceID != uuid.Nil && alog.ResourceID != arg.ResourceID {
continue
}
if arg.Username != "" {
user, err := q.GetUserByID(context.Background(), alog.UserID)
if err == nil && !strings.EqualFold(arg.Username, user.Username) {
continue
}
}
if arg.Email != "" {
user, err := q.GetUserByID(context.Background(), alog.UserID)
if err == nil && !strings.EqualFold(arg.Email, user.Email) {
continue
}
}
if !arg.DateFrom.IsZero() {
if alog.Time.Before(arg.DateFrom) {
continue
}
}
if !arg.DateTo.IsZero() {
if alog.Time.After(arg.DateTo) {
continue
}
}

logs = append(logs, alog)
count := int64(len(logs))
for i := range logs {
logs[i].Count = count
}

return int64(len(logs)), nil
return logs, nil
}

func (q *fakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAuditLogParams) (database.AuditLog, error) {
Expand Down
1 change: 0 additions & 1 deletion coderd/database/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 4 additions & 84 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 2 additions & 56 deletions coderd/database/queries/auditlogs.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ SELECT
users.created_at AS user_created_at,
users.status AS user_status,
users.rbac_roles AS user_roles,
users.avatar_url AS user_avatar_url
users.avatar_url AS user_avatar_url,
COUNT(audit_logs.*) OVER() AS count
FROM
audit_logs
LEFT JOIN
Expand Down Expand Up @@ -69,61 +70,6 @@ LIMIT
OFFSET
$2;

-- name: GetAuditLogCount :one
SELECT
COUNT(*) as count
FROM
audit_logs
WHERE
-- Filter resource_type
CASE
WHEN @resource_type :: text != '' THEN
resource_type = @resource_type :: resource_type
ELSE true
END
-- Filter resource_id
AND CASE
WHEN @resource_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
resource_id = @resource_id
ELSE true
END
-- Filter by resource_target
AND CASE
WHEN @resource_target :: text != '' THEN
resource_target = @resource_target
ELSE true
END
-- Filter action
AND CASE
WHEN @action :: text != '' THEN
action = @action :: audit_action
ELSE true
END
-- Filter by username
AND CASE
WHEN @username :: text != '' THEN
user_id = (SELECT id from users WHERE users.username = @username )
ELSE true
END
-- Filter by user_email
AND CASE
WHEN @email :: text != '' THEN
user_id = (SELECT id from users WHERE users.email = @email )
ELSE true
END
-- Filter by date_from
AND CASE
WHEN @date_from :: timestamp with time zone != '0001-01-01 00:00:00' THEN
"time" >= @date_from
ELSE true
END
-- Filter by date_to
AND CASE
WHEN @date_to :: timestamp with time zone != '0001-01-01 00:00:00' THEN
"time" <= @date_to
ELSE true
END;

-- name: InsertAuditLog :one
INSERT INTO
audit_logs (
Expand Down
Loading