libraryelements: Spanner compability (#103881)

* libraryelements: fix compatibility with Spanner

* Log errors leading to status code 500.

* Rename tests to make them integration tests.
pull/104476/head
Peter Štibraný 4 weeks ago committed by GitHub
parent e7b800f35a
commit 08205d64d1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 22
      pkg/services/libraryelements/api.go
  2. 2
      pkg/services/libraryelements/libraryelements_create_test.go
  3. 5
      pkg/services/libraryelements/libraryelements_delete_test.go
  4. 8
      pkg/services/libraryelements/libraryelements_get_all_test.go
  5. 5
      pkg/services/libraryelements/libraryelements_get_test.go
  6. 5
      pkg/services/libraryelements/libraryelements_patch_test.go
  7. 5
      pkg/services/libraryelements/libraryelements_test.go
  8. 9
      pkg/services/libraryelements/writers.go

@ -81,7 +81,7 @@ func (l *LibraryElementService) createHandler(c *contextmodel.ReqContext) respon
element, err := l.createLibraryElement(c.Req.Context(), c.SignedInUser, cmd)
if err != nil {
return toLibraryElementError(err, "Failed to create library element")
return l.toLibraryElementError(err, "Failed to create library element")
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc()
@ -118,7 +118,7 @@ func (l *LibraryElementService) createHandler(c *contextmodel.ReqContext) respon
func (l *LibraryElementService) deleteHandler(c *contextmodel.ReqContext) response.Response {
id, err := l.deleteLibraryElement(c.Req.Context(), c.SignedInUser, web.Params(c.Req)[":uid"])
if err != nil {
return toLibraryElementError(err, "Failed to delete library element")
return l.toLibraryElementError(err, "Failed to delete library element")
}
return response.JSON(http.StatusOK, model.DeleteLibraryElementResponse{
@ -148,7 +148,7 @@ func (l *LibraryElementService) getHandler(c *contextmodel.ReqContext) response.
},
)
if err != nil {
return toLibraryElementError(err, "Failed to get library element")
return l.toLibraryElementError(err, "Failed to get library element")
}
if l.features.IsEnabled(ctx, featuremgmt.FlagLibraryPanelRBAC) {
@ -189,13 +189,13 @@ func (l *LibraryElementService) getAllHandler(c *contextmodel.ReqContext) respon
}
elementsResult, err := l.getAllLibraryElements(c.Req.Context(), c.SignedInUser, query)
if err != nil {
return toLibraryElementError(err, "Failed to get library elements")
return l.toLibraryElementError(err, "Failed to get library elements")
}
if l.features.IsEnabled(c.Req.Context(), featuremgmt.FlagLibraryPanelRBAC) {
filteredPanels, err := l.filterLibraryPanelsByPermission(c, elementsResult.Elements)
if err != nil {
return toLibraryElementError(err, "Failed to evaluate permissions")
return l.toLibraryElementError(err, "Failed to evaluate permissions")
}
elementsResult.Elements = filteredPanels
}
@ -241,7 +241,7 @@ func (l *LibraryElementService) patchHandler(c *contextmodel.ReqContext) respons
element, err := l.patchLibraryElement(c.Req.Context(), c.SignedInUser, cmd, web.Params(c.Req)[":uid"])
if err != nil {
return toLibraryElementError(err, "Failed to update library element")
return l.toLibraryElementError(err, "Failed to update library element")
}
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.LibraryElements).Inc()
@ -276,7 +276,7 @@ func (l *LibraryElementService) patchHandler(c *contextmodel.ReqContext) respons
func (l *LibraryElementService) getConnectionsHandler(c *contextmodel.ReqContext) response.Response {
connections, err := l.getConnections(c.Req.Context(), c.SignedInUser, web.Params(c.Req)[":uid"])
if err != nil {
return toLibraryElementError(err, "Failed to get connections")
return l.toLibraryElementError(err, "Failed to get connections")
}
return response.JSON(http.StatusOK, model.LibraryElementConnectionsResponse{Result: connections})
@ -296,13 +296,13 @@ func (l *LibraryElementService) getConnectionsHandler(c *contextmodel.ReqContext
func (l *LibraryElementService) getByNameHandler(c *contextmodel.ReqContext) response.Response {
elements, err := l.getLibraryElementsByName(c.Req.Context(), c.SignedInUser, web.Params(c.Req)[":name"])
if err != nil {
return toLibraryElementError(err, "Failed to get library element")
return l.toLibraryElementError(err, "Failed to get library element")
}
if l.features.IsEnabled(c.Req.Context(), featuremgmt.FlagLibraryPanelRBAC) {
filteredElements, err := l.filterLibraryPanelsByPermission(c, elements)
if err != nil {
return toLibraryElementError(err, err.Error())
return l.toLibraryElementError(err, err.Error())
}
return response.JSON(http.StatusOK, model.LibraryElementArrayResponse{Result: filteredElements})
@ -326,7 +326,7 @@ func (l *LibraryElementService) filterLibraryPanelsByPermission(c *contextmodel.
return filteredPanels, nil
}
func toLibraryElementError(err error, message string) response.Response {
func (l *LibraryElementService) toLibraryElementError(err error, message string) response.Response {
if errors.Is(err, model.ErrLibraryElementAlreadyExists) {
return response.Error(http.StatusBadRequest, model.ErrLibraryElementAlreadyExists.Error(), err)
}
@ -354,6 +354,8 @@ func toLibraryElementError(err error, message string) response.Response {
if errors.Is(err, model.ErrLibraryElementUIDTooLong) {
return response.Error(http.StatusBadRequest, model.ErrLibraryElementUIDTooLong.Error(), err)
}
// Log errors that cause internal server error status code.
l.log.Error(message, "error", err)
return response.ErrOrFallback(http.StatusInternalServerError, message, err)
}

@ -11,7 +11,7 @@ import (
"github.com/grafana/grafana/pkg/util"
)
func TestCreateLibraryElement(t *testing.T) {
func TestIntegration_CreateLibraryElement(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to create a library panel that already exists, it should fail",
func(t *testing.T, sc scenarioContext) {
// nolint:staticcheck

@ -4,15 +4,16 @@ import (
"encoding/json"
"testing"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/libraryelements/model"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/web"
"github.com/stretchr/testify/require"
)
func TestDeleteLibraryElement(t *testing.T) {
func TestIntegration_DeleteLibraryElement(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to delete a library panel that does not exist, it should fail",
func(t *testing.T, sc scenarioContext) {
resp := sc.service.deleteHandler(sc.reqContext)

@ -14,7 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/search/sort"
)
func TestGetAllLibraryElements(t *testing.T) {
func TestIntegration_GetAllLibraryElements(t *testing.T) {
testScenario(t, "When an admin tries to get all library panels and none exists, it should return none",
func(t *testing.T, sc scenarioContext) {
resp := sc.service.getAllHandler(sc.reqContext)
@ -964,13 +964,14 @@ func TestGetAllLibraryElements(t *testing.T) {
require.NoError(t, err)
sc.reqContext.Req.Form.Add("perPage", "1")
sc.reqContext.Req.Form.Add("page", "1")
sc.reqContext.Req.Form.Add("searchString", "description")
sc.reqContext.Req.Form.Add("searchString", "DeScRiPtIoN") // mixed case to test case-insensitive search.
resp = sc.service.getAllHandler(sc.reqContext)
require.Equal(t, 200, resp.Status())
var result libraryElementsSearch
err = json.Unmarshal(resp.Body(), &result)
require.NoError(t, err)
require.Equal(t, int64(1), result.Result.TotalCount)
var expected = libraryElementsSearch{
Result: libraryElementsSearchResult{
TotalCount: 1,
@ -1039,13 +1040,14 @@ func TestGetAllLibraryElements(t *testing.T) {
err := sc.reqContext.Req.ParseForm()
require.NoError(t, err)
sc.reqContext.Req.Form.Add("searchString", "Library Panel")
sc.reqContext.Req.Form.Add("searchString", "library PANEL") // mixed-case to test case-insensitive search.
resp = sc.service.getAllHandler(sc.reqContext)
require.Equal(t, 200, resp.Status())
var result libraryElementsSearch
err = json.Unmarshal(resp.Body(), &result)
require.NoError(t, err)
require.Equal(t, int64(2), result.Result.TotalCount)
var expected = libraryElementsSearch{
Result: libraryElementsSearchResult{
TotalCount: 2,

@ -5,6 +5,8 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/components/simplejson"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/kinds/librarypanel"
@ -13,10 +15,9 @@ import (
"github.com/grafana/grafana/pkg/services/libraryelements/model"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/web"
"github.com/stretchr/testify/require"
)
func TestGetLibraryElement(t *testing.T) {
func TestIntegration_GetLibraryElement(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to get a library panel that does not exist, it should fail",
func(t *testing.T, sc scenarioContext) {
// by uid

@ -8,11 +8,12 @@ import (
"github.com/grafana/grafana/pkg/util"
"github.com/google/go-cmp/cmp"
"github.com/grafana/grafana/pkg/web"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/web"
)
func TestPatchLibraryElement(t *testing.T) {
func TestIntegration_PatchLibraryElement(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to patch a library panel that does not exist, it should fail",
func(t *testing.T, sc scenarioContext) {
cmd := model.PatchLibraryElementCommand{Kind: int64(model.PanelElement), Version: 1}

@ -62,7 +62,7 @@ func TestMain(m *testing.M) {
testsuite.Run(m)
}
func TestDeleteLibraryPanelsInFolder(t *testing.T) {
func TestIntegration_DeleteLibraryPanelsInFolder(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to delete a folder that contains connected library elements, it should fail",
func(t *testing.T, sc scenarioContext) {
dashJSON := map[string]any{
@ -137,7 +137,7 @@ func TestDeleteLibraryPanelsInFolder(t *testing.T) {
})
}
func TestGetLibraryPanelConnections(t *testing.T) {
func TestIntegration_GetLibraryPanelConnections(t *testing.T) {
scenarioWithPanel(t, "When an admin tries to get connections of library panel, it should succeed and return correct result",
func(t *testing.T, sc scenarioContext) {
dashJSON := map[string]any{
@ -549,6 +549,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
SQLStore: sqlStore,
folderService: folderSvc,
dashboardsService: dashService,
log: log.NewNopLogger(),
}
// deliberate difference between signed in user and user in db to make it crystal clear

@ -60,8 +60,11 @@ func writeTypeFilterSQL(typeFilter []string, builder *db.SQLBuilder) {
func writeSearchStringSQL(query model.SearchLibraryElementsQuery, sqlStore db.DB, builder *db.SQLBuilder) {
if len(strings.TrimSpace(query.SearchString)) > 0 {
builder.Write(" AND (le.name "+sqlStore.GetDialect().LikeStr()+" ?", "%"+query.SearchString+"%")
builder.Write(" OR le.description "+sqlStore.GetDialect().LikeStr()+" ?)", "%"+query.SearchString+"%")
sql, param := sqlStore.GetDialect().LikeOperator("le.name", true, query.SearchString, true)
builder.Write(" AND ("+sql, param)
sql, param = sqlStore.GetDialect().LikeOperator("le.description", true, query.SearchString, true)
builder.Write(" OR "+sql+")", param)
}
}
@ -147,7 +150,7 @@ func (f *FolderFilter) writeFolderFilterSQL(includeGeneral bool, builder *db.SQL
if !includeGeneral && isGeneralFolder(folderID) {
continue
}
params = append(params, filter)
params = append(params, folderID)
}
if len(params) > 0 {
sql.WriteString(` AND le.folder_id IN (?` + strings.Repeat(",?", len(params)-1) + ")")

Loading…
Cancel
Save