Search: Replace search implementation (#23855)

pkg/services/search/service.go

@@ -1,9 +1,10 @@
 package search
 
 import (
+	"sort"
+
 	"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
 	"github.com/grafana/grafana/pkg/setting"
-	"sort"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
@@ -48,8 +49,7 @@ type FindPersistedDashboardsQuery struct {
 	Page         int64
 	Permission   models.PermissionType
 
-	FeatureSearch2 bool
+	SortBy searchstore.FilterOrderBy
-	SortBy         searchstore.FilterOrderBy
 
 	Result HitList
 }
@@ -72,29 +72,21 @@ func (s *SearchService) Init() error {
 }
 
 func (s *SearchService) searchHandler(query *Query) error {
-	sortOpt, exists := s.sortOptions[query.Sort]
-	if !exists {
-		sortOpt = sortAlphaAsc
-	}
-
-	search2 := false
-	if s.Cfg != nil {
-		search2 = s.Cfg.FeatureToggles["search2"]
-	}
-
 	dashboardQuery := FindPersistedDashboardsQuery{
-		Title:          query.Title,
+		Title:        query.Title,
-		SignedInUser:   query.SignedInUser,
+		SignedInUser: query.SignedInUser,
-		IsStarred:      query.IsStarred,
+		IsStarred:    query.IsStarred,
-		DashboardIds:   query.DashboardIds,
+		DashboardIds: query.DashboardIds,
-		Type:           query.Type,
+		Type:         query.Type,
-		FolderIds:      query.FolderIds,
+		FolderIds:    query.FolderIds,
-		Tags:           query.Tags,
+		Tags:         query.Tags,
-		Limit:          query.Limit,
+		Limit:        query.Limit,
-		Page:           query.Page,
+		Page:         query.Page,
-		Permission:     query.Permission,
+		Permission:   query.Permission,
-		FeatureSearch2: search2,
+	}
-		SortBy:         sortOpt.Filter,
+
+	if sortOpt, exists := s.sortOptions[query.Sort]; exists {
+		dashboardQuery.SortBy = sortOpt.Filter
 	}
 
 	if err := bus.Dispatch(&dashboardQuery); err != nil {

pkg/services/search/sorting.go

@@ -1,20 +1,21 @@
 package search
 
 import (
-	"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
 	"sort"
+
+	"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
 )
 
 var (
 	sortAlphaAsc = SortOption{
 		Name:        "alpha-asc",
-		DisplayName: "A-Z",
+		DisplayName: "Alphabetically (A-Z)",
 		Description: "Sort results in an alphabetically ascending order",
 		Filter:      searchstore.TitleSorter{},
 	}
 	sortAlphaDesc = SortOption{
 		Name:        "alpha-desc",
-		DisplayName: "Z-A",
+		DisplayName: "Alphabetically (Z-A)",
 		Description: "Sort results in an alphabetically descending order",
 		Filter:      searchstore.TitleSorter{Descending: true},
 	}

pkg/services/sqlstore/dashboard.go

@@ -1,13 +1,12 @@
 package sqlstore
 
 import (
+	"strings"
+	"time"
+
 	"github.com/grafana/grafana/pkg/services/sqlstore/permissions"
 	"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
 	"github.com/prometheus/client_golang/prometheus"
-	"reflect"
-	"strconv"
-	"strings"
-	"time"
 
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/metrics"
@@ -217,11 +216,11 @@ type DashboardSearchProjection struct {
 }
 
 func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) {
-	sb := NewSearchBuilder(query.SignedInUser, query.Limit, query.Page, query.Permission).
+	if query.SortBy == nil {
-		WithTags(query.Tags).
+		query.SortBy = searchstore.TitleSorter{}
-		WithDashboardIdsIn(query.DashboardIds)
+	}
 
-	sb2filters := []interface{}{
+	filters := []interface{}{
 		query.SortBy,
 		permissions.DashboardPermissionFilter{
 			OrgRole:         query.SignedInUser.OrgRole,
@@ -232,72 +231,55 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear
 		},
 	}
 
+	if query.OrgId != 0 {
+		filters = append(filters, searchstore.OrgFilter{OrgId: query.OrgId})
+	} else if query.SignedInUser.OrgId != 0 {
+		filters = append(filters, searchstore.OrgFilter{OrgId: query.SignedInUser.OrgId})
+	}
+
 	if len(query.Tags) > 0 {
-		sb2filters = append(sb2filters, searchstore.TagsFilter{Tags: query.Tags})
+		filters = append(filters, searchstore.TagsFilter{Tags: query.Tags})
 	}
 
 	if len(query.DashboardIds) > 0 {
-		sb2filters = append(sb2filters, searchstore.DashboardFilter{IDs: query.DashboardIds})
+		filters = append(filters, searchstore.DashboardFilter{IDs: query.DashboardIds})
 	}
 
 	if query.IsStarred {
-		sb.IsStarred()
+		filters = append(filters, searchstore.StarredFilter{UserId: query.SignedInUser.UserId})
-		sb2filters = append(sb2filters, searchstore.StarredFilter{UserId: query.SignedInUser.UserId})
 	}
 
 	if len(query.Title) > 0 {
-		sb.WithTitle(query.Title)
+		filters = append(filters, searchstore.TitleFilter{Dialect: dialect, Title: query.Title})
-		sb2filters = append(sb2filters, searchstore.TitleFilter{Dialect: dialect, Title: query.Title})
 	}
 
 	if len(query.Type) > 0 {
-		sb.WithType(query.Type)
+		filters = append(filters, searchstore.TypeFilter{Dialect: dialect, Type: query.Type})
-		sb2filters = append(sb2filters, searchstore.TypeFilter{Dialect: dialect, Type: query.Type})
 	}
 
 	if len(query.FolderIds) > 0 {
-		sb.WithFolderIds(query.FolderIds)
+		filters = append(filters, searchstore.FolderFilter{IDs: query.FolderIds})
-		sb2filters = append(sb2filters, searchstore.FolderFilter{IDs: query.FolderIds})
 	}
 
 	var res []DashboardSearchProjection
+	sb := &searchstore.Builder{Dialect: dialect, Filters: filters}
 
-	sql, params := sb.ToSql()
+	limit := query.Limit
+	if limit < 1 {
+		limit = 1000
+	}
+
+	page := query.Page
+	if page < 1 {
+		page = 1
+	}
+
+	sql, params := sb.ToSql(limit, page)
 	err := x.SQL(sql, params...).Find(&res)
 	if err != nil {
 		return nil, err
 	}
 
-	if query.FeatureSearch2 {
-		var res2 []DashboardSearchProjection
-		sb := &searchstore.Builder{Dialect: dialect, Filters: sb2filters}
-		limit := query.Limit
-		if limit < 1 {
-			limit = 1000
-		}
-
-		page := query.Page
-		if page < 1 {
-			page = 1
-		}
-		shadowSql, params := sb.ToSql(limit, page)
-		err = x.SQL(shadowSql, params...).Find(&res2)
-
-		equal := reflect.DeepEqual(res2, res)
-		shadowSearchCounter.With(prometheus.Labels{
-			"equal": strconv.FormatBool(equal),
-			"error": strconv.FormatBool(err != nil),
-		}).Inc()
-		sqlog.Debug(
-			"shadow search query result",
-			"err", err,
-			"equal", equal,
-			"shadowQuery", strings.Replace(strings.Replace(shadowSql, "\n", " ", -1), "\t", " ", -1),
-			"query", strings.Replace(strings.Replace(sql, "\n", " ", -1), "\t", " ", -1),
-		)
-		return res2, nil
-	}
-
 	return res, nil
 }
 

pkg/services/sqlstore/migrations/dashboard_mig.go

@@ -215,4 +215,9 @@ func addDashboardMigration(mg *Migrator) {
 	mg.AddMigration("Add check_sum column", NewAddColumnMigration(dashboardExtrasTableV2, &Column{
 		Name: "check_sum", Type: DB_NVarchar, Length: 32, Nullable: true,
 	}))
+	mg.AddMigration("Add index for dashboard_title", NewAddIndexMigration(dashboardV2, &Index{
+		Cols: []string{"title"},
+		Type: IndexType,
+	}))
+
 }

pkg/services/sqlstore/search_builder.go

@@ -1,214 +0,0 @@
-package sqlstore
-
-import (
-	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
-	"strings"
-
-	"github.com/grafana/grafana/pkg/models"
-)
-
-// SearchBuilder is a builder/object mother that builds a dashboard search query
-type SearchBuilder struct {
-	SqlBuilder
-
-	dialect             migrator.Dialect
-	tags                []string
-	isStarred           bool
-	limit               int64
-	page                int64
-	signedInUser        *models.SignedInUser
-	whereDashboardIdsIn []int64
-	whereTitle          string
-	whereTypeFolder     bool
-	whereTypeDash       bool
-	whereFolderIds      []int64
-	permission          models.PermissionType
-}
-
-func NewSearchBuilder(signedInUser *models.SignedInUser, limit int64, page int64, permission models.PermissionType) *SearchBuilder {
-	// Default to page 1
-	if page < 1 {
-		page = 1
-	}
-
-	// default limit
-	if limit <= 0 {
-		limit = 1000
-	}
-
-	searchBuilder := &SearchBuilder{
-		signedInUser: signedInUser,
-		limit:        limit,
-		page:         page,
-		permission:   permission,
-		dialect:      dialect,
-	}
-
-	return searchBuilder
-}
-
-func (sb *SearchBuilder) WithDialect(dialect migrator.Dialect) *SearchBuilder {
-	sb.dialect = dialect
-	return sb
-}
-
-func (sb *SearchBuilder) WithTags(tags []string) *SearchBuilder {
-	if len(tags) > 0 {
-		sb.tags = tags
-	}
-
-	return sb
-}
-
-func (sb *SearchBuilder) IsStarred() *SearchBuilder {
-	sb.isStarred = true
-
-	return sb
-}
-
-func (sb *SearchBuilder) WithDashboardIdsIn(ids []int64) *SearchBuilder {
-	if len(ids) > 0 {
-		sb.whereDashboardIdsIn = ids
-	}
-
-	return sb
-}
-
-func (sb *SearchBuilder) WithTitle(title string) *SearchBuilder {
-	sb.whereTitle = title
-
-	return sb
-}
-
-func (sb *SearchBuilder) WithType(queryType string) *SearchBuilder {
-	if len(queryType) > 0 && queryType == "dash-folder" {
-		sb.whereTypeFolder = true
-	}
-
-	if len(queryType) > 0 && queryType == "dash-db" {
-		sb.whereTypeDash = true
-	}
-
-	return sb
-}
-
-func (sb *SearchBuilder) WithFolderIds(folderIds []int64) *SearchBuilder {
-	sb.whereFolderIds = folderIds
-	return sb
-}
-
-// ToSql builds the sql and returns it as a string, together with the params.
-func (sb *SearchBuilder) ToSql() (string, []interface{}) {
-	sb.params = make([]interface{}, 0)
-
-	sb.buildSelect()
-
-	if len(sb.tags) > 0 {
-		sb.buildTagQuery()
-	} else {
-		sb.buildMainQuery()
-	}
-
-	sb.sql.WriteString(`
-		ORDER BY dashboard.id ` + sb.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_tag on dashboard.id = dashboard_tag.dashboard_id`)
-
-	sb.sql.WriteString(" ORDER BY dashboard.title ASC")
-	return sb.sql.String(), sb.params
-}
-
-func (sb *SearchBuilder) buildSelect() {
-	sb.sql.WriteString(
-		`SELECT
-			dashboard.id,
-			dashboard.uid,
-			dashboard.title,
-			dashboard.slug,
-			dashboard_tag.term,
-			dashboard.is_folder,
-			dashboard.folder_id,
-			folder.uid as folder_uid,
-			folder.slug as folder_slug,
-			folder.title as folder_title
-		FROM `)
-}
-
-func (sb *SearchBuilder) buildTagQuery() {
-	sb.sql.WriteString(
-		`(
-	SELECT
-		dashboard.id FROM dashboard
-		LEFT OUTER JOIN dashboard_tag ON dashboard_tag.dashboard_id = dashboard.id
-	`)
-
-	if sb.isStarred {
-		sb.sql.WriteString(" INNER JOIN star on star.dashboard_id = dashboard.id")
-	}
-
-	sb.sql.WriteString(` WHERE dashboard_tag.term IN (?` + strings.Repeat(",?", len(sb.tags)-1) + `) AND `)
-	for _, tag := range sb.tags {
-		sb.params = append(sb.params, tag)
-	}
-
-	sb.buildSearchWhereClause()
-
-	// this ends the inner select (tag filtered part)
-	sb.sql.WriteString(` GROUP BY dashboard.id HAVING COUNT(dashboard.id) >= ? `)
-	sb.params = append(sb.params, len(sb.tags))
-}
-
-func (sb *SearchBuilder) buildMainQuery() {
-	sb.sql.WriteString(`( SELECT dashboard.id FROM dashboard `)
-
-	if sb.isStarred {
-		sb.sql.WriteString(" INNER JOIN star on star.dashboard_id = dashboard.id")
-	}
-
-	sb.sql.WriteString(` WHERE `)
-	sb.buildSearchWhereClause()
-
-}
-
-func (sb *SearchBuilder) buildSearchWhereClause() {
-	sb.sql.WriteString(` dashboard.org_id=?`)
-	sb.params = append(sb.params, sb.signedInUser.OrgId)
-
-	if sb.isStarred {
-		sb.sql.WriteString(` AND star.user_id=?`)
-		sb.params = append(sb.params, sb.signedInUser.UserId)
-	}
-
-	if len(sb.whereDashboardIdsIn) > 0 {
-		sb.sql.WriteString(` AND dashboard.id IN (?` + strings.Repeat(",?", len(sb.whereDashboardIdsIn)-1) + `)`)
-		for _, dashboardId := range sb.whereDashboardIdsIn {
-			sb.params = append(sb.params, dashboardId)
-		}
-	}
-
-	sb.writeDashboardPermissionFilter(sb.signedInUser, sb.permission)
-
-	if len(sb.whereTitle) > 0 {
-		sb.sql.WriteString(" AND dashboard.title " + sb.dialect.LikeStr() + " ?")
-		sb.params = append(sb.params, "%"+sb.whereTitle+"%")
-	}
-
-	if sb.whereTypeFolder {
-		sb.sql.WriteString(" AND dashboard.is_folder = " + sb.dialect.BooleanStr(true))
-	}
-
-	if sb.whereTypeDash {
-		sb.sql.WriteString(" AND dashboard.is_folder = " + sb.dialect.BooleanStr(false))
-	}
-
-	if len(sb.whereFolderIds) > 0 {
-		sb.sql.WriteString(` AND dashboard.folder_id IN (?` + strings.Repeat(",?", len(sb.whereFolderIds)-1) + `) `)
-		for _, id := range sb.whereFolderIds {
-			sb.params = append(sb.params, id)
-		}
-	}
-}

pkg/services/sqlstore/search_builder_test.go

@@ -1,39 +0,0 @@
-package sqlstore
-
-import (
-	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
-	"testing"
-
-	"github.com/grafana/grafana/pkg/models"
-	. "github.com/smartystreets/goconvey/convey"
-)
-
-func TestSearchBuilder(t *testing.T) {
-	Convey("Testing building a search", t, func() {
-		if dialect == nil {
-			dialect = &migrator.Sqlite3{}
-		}
-		signedInUser := &models.SignedInUser{
-			OrgId:  1,
-			UserId: 1,
-		}
-
-		sb := NewSearchBuilder(signedInUser, 1000, 0, models.PERMISSION_VIEW)
-
-		Convey("When building a normal search", func() {
-			sql, params := sb.IsStarred().WithTitle("test").ToSql()
-			So(sql, ShouldStartWith, "SELECT")
-			So(sql, ShouldContainSubstring, "INNER JOIN dashboard on ids.id = dashboard.id")
-			So(sql, ShouldContainSubstring, "ORDER BY dashboard.title ASC")
-			So(len(params), ShouldBeGreaterThan, 0)
-		})
-
-		Convey("When building a search with tag filter", func() {
-			sql, params := sb.WithTags([]string{"tag1", "tag2"}).ToSql()
-			So(sql, ShouldStartWith, "SELECT")
-			So(sql, ShouldContainSubstring, "LEFT OUTER JOIN dashboard_tag")
-			So(sql, ShouldContainSubstring, "ORDER BY dashboard.title ASC")
-			So(len(params), ShouldBeGreaterThan, 0)
-		})
-	})
-}

pkg/services/sqlstore/searchstore/builder.go

@@ -3,8 +3,9 @@ package searchstore
 import (
 	"bytes"
 	"fmt"
-	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 	"strings"
+
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 )
 
 // Builder defaults to returning a SQL query to get a list of all dashboards
@@ -27,15 +28,17 @@ func (b *Builder) ToSql(limit, page int64) (string, []interface{}) {
 	b.buildSelect()
 
 	b.sql.WriteString("( ")
-	b.applyFilters()
+	orderQuery := b.applyFilters()
 
 	b.sql.WriteString(b.Dialect.LimitOffset(limit, (page-1)*limit) + `) AS ids
-		INNER JOIN dashboard ON ids.id = dashboard.id
+		INNER JOIN dashboard ON ids.id = dashboard.id`)
-	`)
+	b.sql.WriteString("\n")
 
-	b.sql.WriteString(`
+	b.sql.WriteString(
-		LEFT OUTER JOIN dashboard AS folder ON folder.id = dashboard.folder_id
+		`LEFT OUTER JOIN dashboard AS folder ON folder.id = dashboard.folder_id
 		LEFT OUTER JOIN dashboard_tag ON dashboard.id = dashboard_tag.dashboard_id`)
+	b.sql.WriteString("\n")
+	b.sql.WriteString(orderQuery)
 
 	return b.sql.String(), b.params
 }
@@ -56,8 +59,9 @@ func (b *Builder) buildSelect() {
 		FROM `)
 }
 
-func (b *Builder) applyFilters() {
+func (b *Builder) applyFilters() (ordering string) {
 	joins := []string{}
+	orderJoins := []string{}
 
 	wheres := []string{}
 	whereParams := []interface{}{}
@@ -89,6 +93,9 @@ func (b *Builder) applyFilters() {
 		}
 
 		if f, ok := f.(FilterOrderBy); ok {
+			if f, ok := f.(FilterLeftJoin); ok {
+				orderJoins = append(orderJoins, fmt.Sprintf(" LEFT OUTER JOIN %s ", f.LeftJoin()))
+			}
 			orders = append(orders, f.OrderBy())
 		}
 	}
@@ -107,6 +114,12 @@ func (b *Builder) applyFilters() {
 	}
 
 	if len(orders) > 0 {
-		b.sql.WriteString(fmt.Sprintf(" ORDER BY %s", strings.Join(orders, ", ")))
+		orderBy := fmt.Sprintf(" ORDER BY %s", strings.Join(orders, ", "))
+		b.sql.WriteString(orderBy)
+
+		order := strings.Join(orderJoins, "")
+		order += orderBy
+		return order
 	}
+	return " ORDER BY dashboard.id"
 }

pkg/services/sqlstore/searchstore/search_test.go

@@ -4,6 +4,9 @@ package searchstore_test
 import (
 	"context"
 	"fmt"
+	"testing"
+	"time"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -12,8 +15,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"testing"
-	"time"
 )
 
 var dialect migrator.Dialect
@@ -47,25 +48,23 @@ func TestBuilder_EqualResults_Basic(t *testing.T) {
 		Dialect: dialect,
 	}
 
-	prevBuilder := sqlstore.NewSearchBuilder(user, limit, page, models.PERMISSION_EDIT)
+	res := []sqlstore.DashboardSearchProjection{}
-	prevBuilder.WithDialect(dialect)
-
-	newRes := []sqlstore.DashboardSearchProjection{}
 	err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
 		sql, params := builder.ToSql(limit, page)
-		return sess.SQL(sql, params...).Find(&newRes)
+		return sess.SQL(sql, params...).Find(&res)
 	})
 	require.NoError(t, err)
 
-	oldRes := []sqlstore.DashboardSearchProjection{}
+	assert.Len(t, res, 1)
-	err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
+	res[0].Uid = ""
-		sql, params := prevBuilder.ToSql()
+	assert.EqualValues(t, []sqlstore.DashboardSearchProjection{
-		return sess.SQL(sql, params...).Find(&oldRes)
+		{
-	})
+			Id:    1,
-	require.NoError(t, err)
+			Title: "A",
-
+			Slug:  "a",
-	assert.Len(t, newRes, 1)
+			Term:  "templated",
-	assert.EqualValues(t, oldRes, newRes)
+		},
+	}, res)
 }
 
 func TestBuilder_Pagination(t *testing.T) {
@@ -143,25 +142,14 @@ func TestBuilder_Permissions(t *testing.T) {
 		Dialect: dialect,
 	}
 
-	prevBuilder := sqlstore.NewSearchBuilder(user, limit, page, level)
+	res := []sqlstore.DashboardSearchProjection{}
-	prevBuilder.WithDialect(dialect)
-
-	newRes := []sqlstore.DashboardSearchProjection{}
 	err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
 		sql, params := builder.ToSql(limit, page)
-		return sess.SQL(sql, params...).Find(&newRes)
+		return sess.SQL(sql, params...).Find(&res)
 	})
 	require.NoError(t, err)
 
-	oldRes := []sqlstore.DashboardSearchProjection{}
+	assert.Len(t, res, 0)
-	err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
-		sql, params := prevBuilder.ToSql()
-		return sess.SQL(sql, params...).Find(&oldRes)
-	})
-	require.NoError(t, err)
-
-	assert.Len(t, newRes, 0)
-	assert.EqualValues(t, oldRes, newRes)
 }
 
 func setupTestEnvironment(t *testing.T) *sqlstore.SqlStore {