Commit 8b0dd424 by Torkel Ödegaard Committed by GitHub

Search: Fixes search limits and adds a page parameter (#16458)

* Search: Fixes search limits and adds a page parameter

This adds a page parameter to search api without adding
any major breaking change.

It does at an api validation error when trying to use
a limit beyond 5000. This is a breaking change. We could
remove this and have it only in the docs and describe that this
is a limit that grafana will apply silently.

Fixes #16049

* Fix: Corrected wrong array slice change

* Docs: minor docs fix

* Search: fixed folder tests

* Fixed: Moved limit to correct inner query

* Search: moving limit check and page check

* Search: limit in handler is no longer needed
parent 9cc67e49
...@@ -23,7 +23,8 @@ Query parameters: ...@@ -23,7 +23,8 @@ Query parameters:
- **dashboardIds** – List of dashboard id's to search for - **dashboardIds** – List of dashboard id's to search for
- **folderIds** – List of folder id's to search in for dashboards - **folderIds** – List of folder id's to search in for dashboards
- **starred** – Flag indicating if only starred Dashboards should be returned - **starred** – Flag indicating if only starred Dashboards should be returned
- **limit** – Limit the number of returned results - **limit** – Limit the number of returned results (max 5000)
- **page** – Use this parameter to access hits beyond limit. Numbering starts at 1. limit param acts as page size.
**Example request for retrieving folders and dashboards of the general folder**: **Example request for retrieving folders and dashboards of the general folder**:
...@@ -95,4 +96,4 @@ Content-Type: application/json ...@@ -95,4 +96,4 @@ Content-Type: application/json
"uri":"db/production-overview" // deprecated in Grafana v5.0 "uri":"db/production-overview" // deprecated in Grafana v5.0
} }
] ]
``` ```
\ No newline at end of file
...@@ -325,7 +325,7 @@ func (hs *HTTPServer) registerRoutes() { ...@@ -325,7 +325,7 @@ func (hs *HTTPServer) registerRoutes() {
}) })
// Search // Search
apiRoute.Get("/search/", Search) apiRoute.Get("/search/", Wrap(Search))
// metrics // metrics
apiRoute.Post("/tsdb/query", bind(dtos.MetricRequest{}), Wrap(hs.QueryMetrics)) apiRoute.Post("/tsdb/query", bind(dtos.MetricRequest{}), Wrap(hs.QueryMetrics))
......
...@@ -12,7 +12,7 @@ import ( ...@@ -12,7 +12,7 @@ import (
func GetFolders(c *m.ReqContext) Response { func GetFolders(c *m.ReqContext) Response {
s := dashboards.NewFolderService(c.OrgId, c.SignedInUser) s := dashboards.NewFolderService(c.OrgId, c.SignedInUser)
folders, err := s.GetFolders(c.QueryInt("limit")) folders, err := s.GetFolders(c.QueryInt64("limit"))
if err != nil { if err != nil {
return toFolderError(err) return toFolderError(err)
......
...@@ -213,7 +213,7 @@ type fakeFolderService struct { ...@@ -213,7 +213,7 @@ type fakeFolderService struct {
DeletedFolderUids []string DeletedFolderUids []string
} }
func (s *fakeFolderService) GetFolders(limit int) ([]*m.Folder, error) { func (s *fakeFolderService) GetFolders(limit int64) ([]*m.Folder, error) {
return s.GetFoldersResult, s.GetFoldersError return s.GetFoldersResult, s.GetFoldersError
} }
......
...@@ -9,16 +9,17 @@ import ( ...@@ -9,16 +9,17 @@ import (
"github.com/grafana/grafana/pkg/services/search" "github.com/grafana/grafana/pkg/services/search"
) )
func Search(c *m.ReqContext) { func Search(c *m.ReqContext) Response {
query := c.Query("query") query := c.Query("query")
tags := c.QueryStrings("tag") tags := c.QueryStrings("tag")
starred := c.Query("starred") starred := c.Query("starred")
limit := c.QueryInt("limit") limit := c.QueryInt64("limit")
page := c.QueryInt64("page")
dashboardType := c.Query("type") dashboardType := c.Query("type")
permission := m.PERMISSION_VIEW permission := m.PERMISSION_VIEW
if limit == 0 { if limit > 5000 {
limit = 1000 return Error(422, "Limit is above maximum allowed (5000), use page parameter to access hits beyond limit", nil)
} }
if c.Query("permission") == "Edit" { if c.Query("permission") == "Edit" {
...@@ -46,6 +47,7 @@ func Search(c *m.ReqContext) { ...@@ -46,6 +47,7 @@ func Search(c *m.ReqContext) {
Tags: tags, Tags: tags,
SignedInUser: c.SignedInUser, SignedInUser: c.SignedInUser,
Limit: limit, Limit: limit,
Page: page,
IsStarred: starred == "true", IsStarred: starred == "true",
OrgId: c.OrgId, OrgId: c.OrgId,
DashboardIds: dbIDs, DashboardIds: dbIDs,
...@@ -56,10 +58,9 @@ func Search(c *m.ReqContext) { ...@@ -56,10 +58,9 @@ func Search(c *m.ReqContext) {
err := bus.Dispatch(&searchQuery) err := bus.Dispatch(&searchQuery)
if err != nil { if err != nil {
c.JsonApiErr(500, "Search failed", err) return Error(500, "Search failed", err)
return
} }
c.TimeRequest(metrics.M_Api_Dashboard_Search) c.TimeRequest(metrics.M_Api_Dashboard_Search)
c.JSON(200, searchQuery.Result) return JSON(200, searchQuery.Result)
} }
...@@ -9,7 +9,7 @@ import ( ...@@ -9,7 +9,7 @@ import (
// FolderService service for operating on folders // FolderService service for operating on folders
type FolderService interface { type FolderService interface {
GetFolders(limit int) ([]*models.Folder, error) GetFolders(limit int64) ([]*models.Folder, error)
GetFolderByID(id int64) (*models.Folder, error) GetFolderByID(id int64) (*models.Folder, error)
GetFolderByUID(uid string) (*models.Folder, error) GetFolderByUID(uid string) (*models.Folder, error)
CreateFolder(cmd *models.CreateFolderCommand) error CreateFolder(cmd *models.CreateFolderCommand) error
...@@ -25,11 +25,7 @@ var NewFolderService = func(orgId int64, user *models.SignedInUser) FolderServic ...@@ -25,11 +25,7 @@ var NewFolderService = func(orgId int64, user *models.SignedInUser) FolderServic
} }
} }
func (dr *dashboardServiceImpl) GetFolders(limit int) ([]*models.Folder, error) { func (dr *dashboardServiceImpl) GetFolders(limit int64) ([]*models.Folder, error) {
if limit == 0 {
limit = 1000
}
searchQuery := search.Query{ searchQuery := search.Query{
SignedInUser: dr.user, SignedInUser: dr.user,
DashboardIds: make([]int64, 0), DashboardIds: make([]int64, 0),
......
...@@ -31,6 +31,7 @@ func (s *SearchService) searchHandler(query *Query) error { ...@@ -31,6 +31,7 @@ func (s *SearchService) searchHandler(query *Query) error {
FolderIds: query.FolderIds, FolderIds: query.FolderIds,
Tags: query.Tags, Tags: query.Tags,
Limit: query.Limit, Limit: query.Limit,
Page: query.Page,
Permission: query.Permission, Permission: query.Permission,
} }
...@@ -44,10 +45,6 @@ func (s *SearchService) searchHandler(query *Query) error { ...@@ -44,10 +45,6 @@ func (s *SearchService) searchHandler(query *Query) error {
// sort main result array // sort main result array
sort.Sort(hits) sort.Sort(hits)
if len(hits) > query.Limit {
hits = hits[0:query.Limit]
}
// sort tags // sort tags
for _, hit := range hits { for _, hit := range hits {
sort.Strings(hit.Tags) sort.Strings(hit.Tags)
......
...@@ -48,7 +48,8 @@ type Query struct { ...@@ -48,7 +48,8 @@ type Query struct {
Tags []string Tags []string
OrgId int64 OrgId int64
SignedInUser *models.SignedInUser SignedInUser *models.SignedInUser
Limit int Limit int64
Page int64
IsStarred bool IsStarred bool
Type string Type string
DashboardIds []int64 DashboardIds []int64
...@@ -67,7 +68,8 @@ type FindPersistedDashboardsQuery struct { ...@@ -67,7 +68,8 @@ type FindPersistedDashboardsQuery struct {
Type string Type string
FolderIds []int64 FolderIds []int64
Tags []string Tags []string
Limit int Limit int64
Page int64
Permission models.PermissionType Permission models.PermissionType
Result HitList Result HitList
......
...@@ -197,12 +197,7 @@ type DashboardSearchProjection struct { ...@@ -197,12 +197,7 @@ type DashboardSearchProjection struct {
} }
func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) { func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) {
limit := query.Limit sb := NewSearchBuilder(query.SignedInUser, query.Limit, query.Page, query.Permission).
if limit == 0 {
limit = 1000
}
sb := NewSearchBuilder(query.SignedInUser, limit, query.Permission).
WithTags(query.Tags). WithTags(query.Tags).
WithDashboardIdsIn(query.DashboardIds) WithDashboardIdsIn(query.DashboardIds)
......
...@@ -259,6 +259,35 @@ func TestDashboardDataAccess(t *testing.T) { ...@@ -259,6 +259,35 @@ func TestDashboardDataAccess(t *testing.T) {
So(hit.FolderTitle, ShouldEqual, "") So(hit.FolderTitle, ShouldEqual, "")
}) })
Convey("Should be able to limit search", func() {
query := search.FindPersistedDashboardsQuery{
OrgId: 1,
Limit: 1,
SignedInUser: &m.SignedInUser{OrgId: 1, OrgRole: m.ROLE_EDITOR},
}
err := SearchDashboards(&query)
So(err, ShouldBeNil)
So(len(query.Result), ShouldEqual, 1)
So(query.Result[0].Title, ShouldEqual, "1 test dash folder")
})
Convey("Should be able to search beyond limit using paging", func() {
query := search.FindPersistedDashboardsQuery{
OrgId: 1,
Limit: 1,
Page: 2,
SignedInUser: &m.SignedInUser{OrgId: 1, OrgRole: m.ROLE_EDITOR},
}
err := SearchDashboards(&query)
So(err, ShouldBeNil)
So(len(query.Result), ShouldEqual, 1)
So(query.Result[0].Title, ShouldEqual, "test dash 23")
})
Convey("Should be able to search for a dashboard folder's children", func() { Convey("Should be able to search for a dashboard folder's children", func() {
query := search.FindPersistedDashboardsQuery{ query := search.FindPersistedDashboardsQuery{
OrgId: 1, OrgId: 1,
......
...@@ -11,7 +11,8 @@ type SearchBuilder struct { ...@@ -11,7 +11,8 @@ type SearchBuilder struct {
SqlBuilder SqlBuilder
tags []string tags []string
isStarred bool isStarred bool
limit int limit int64
page int64
signedInUser *m.SignedInUser signedInUser *m.SignedInUser
whereDashboardIdsIn []int64 whereDashboardIdsIn []int64
whereTitle string whereTitle string
...@@ -21,10 +22,21 @@ type SearchBuilder struct { ...@@ -21,10 +22,21 @@ type SearchBuilder struct {
permission m.PermissionType permission m.PermissionType
} }
func NewSearchBuilder(signedInUser *m.SignedInUser, limit int, permission m.PermissionType) *SearchBuilder { func NewSearchBuilder(signedInUser *m.SignedInUser, limit int64, page int64, permission m.PermissionType) *SearchBuilder {
// Default to page 1
if page < 1 {
page = 1
}
// default limit
if limit <= 0 {
limit = 1000
}
searchBuilder := &SearchBuilder{ searchBuilder := &SearchBuilder{
signedInUser: signedInUser, signedInUser: signedInUser,
limit: limit, limit: limit,
page: page,
permission: permission, permission: permission,
} }
...@@ -89,11 +101,15 @@ func (sb *SearchBuilder) ToSql() (string, []interface{}) { ...@@ -89,11 +101,15 @@ func (sb *SearchBuilder) ToSql() (string, []interface{}) {
} }
sb.sql.WriteString(` sb.sql.WriteString(`
ORDER BY dashboard.id ` + dialect.LimitOffset(sb.limit, (sb.page-1)*sb.limit) + `) as ids
INNER JOIN dashboard on ids.id = dashboard.id
`)
sb.sql.WriteString(`
LEFT OUTER JOIN dashboard folder on folder.id = dashboard.folder_id LEFT OUTER JOIN dashboard folder on folder.id = dashboard.folder_id
LEFT OUTER JOIN dashboard_tag on dashboard.id = dashboard_tag.dashboard_id`) LEFT OUTER JOIN dashboard_tag on dashboard.id = dashboard_tag.dashboard_id`)
sb.sql.WriteString(" ORDER BY dashboard.title ASC" + dialect.Limit(5000)) sb.sql.WriteString(" ORDER BY dashboard.title ASC")
return sb.sql.String(), sb.params return sb.sql.String(), sb.params
} }
...@@ -133,12 +149,7 @@ func (sb *SearchBuilder) buildTagQuery() { ...@@ -133,12 +149,7 @@ func (sb *SearchBuilder) buildTagQuery() {
sb.buildSearchWhereClause() sb.buildSearchWhereClause()
// this ends the inner select (tag filtered part) // this ends the inner select (tag filtered part)
sb.sql.WriteString(` sb.sql.WriteString(`GROUP BY dashboard.id HAVING COUNT(dashboard.id) >= ? `)
GROUP BY dashboard.id HAVING COUNT(dashboard.id) >= ?
ORDER BY dashboard.id` + dialect.Limit(int64(sb.limit)) + `) as ids
INNER JOIN dashboard on ids.id = dashboard.id
`)
sb.params = append(sb.params, len(sb.tags)) sb.params = append(sb.params, len(sb.tags))
} }
...@@ -152,7 +163,6 @@ func (sb *SearchBuilder) buildMainQuery() { ...@@ -152,7 +163,6 @@ func (sb *SearchBuilder) buildMainQuery() {
sb.sql.WriteString(` WHERE `) sb.sql.WriteString(` WHERE `)
sb.buildSearchWhereClause() sb.buildSearchWhereClause()
sb.sql.WriteString(` ORDER BY dashboard.title` + dialect.Limit(int64(sb.limit)) + `) as ids INNER JOIN dashboard on ids.id = dashboard.id `)
} }
func (sb *SearchBuilder) buildSearchWhereClause() { func (sb *SearchBuilder) buildSearchWhereClause() {
......
...@@ -14,7 +14,7 @@ func TestSearchBuilder(t *testing.T) { ...@@ -14,7 +14,7 @@ func TestSearchBuilder(t *testing.T) {
UserId: 1, UserId: 1,
} }
sb := NewSearchBuilder(signedInUser, 1000, m.PERMISSION_VIEW) sb := NewSearchBuilder(signedInUser, 1000, 0, m.PERMISSION_VIEW)
Convey("When building a normal search", func() { Convey("When building a normal search", func() {
sql, params := sb.IsStarred().WithTitle("test").ToSql() sql, params := sb.IsStarred().WithTitle("test").ToSql()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment