Remove org methods from sqlstore interface (#56358)

* Remove org methods from sqlstore interface

* Remove some mocks

* Fix some tests
This commit is contained in:
idafurjes 2022-10-05 15:47:56 +02:00 committed by GitHub
parent ad2a1dd680
commit bc7a383252
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 50 additions and 76 deletions

View File

@ -127,7 +127,7 @@ func (hs *HTTPServer) getOrgHelper(ctx context.Context, orgID int64) response.Re
// 409: conflictError
// 500: internalServerError
func (hs *HTTPServer) CreateOrg(c *models.ReqContext) response.Response {
cmd := models.CreateOrgCommand{}
cmd := org.CreateOrgCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
@ -136,8 +136,9 @@ func (hs *HTTPServer) CreateOrg(c *models.ReqContext) response.Response {
return response.Error(http.StatusForbidden, "Access denied", nil)
}
cmd.UserId = c.UserID
if err := hs.SQLStore.CreateOrg(c.Req.Context(), &cmd); err != nil {
cmd.UserID = c.UserID
result, err := hs.orgService.CreateWithMember(c.Req.Context(), &cmd)
if err != nil {
if errors.Is(err, models.ErrOrgNameTaken) {
return response.Error(http.StatusConflict, "Organization name taken", err)
}
@ -147,7 +148,7 @@ func (hs *HTTPServer) CreateOrg(c *models.ReqContext) response.Response {
metrics.MApiOrgCreate.Inc()
return response.JSON(http.StatusOK, &util.DynMap{
"orgId": cmd.Result.Id,
"orgId": result.ID,
"message": "Organization created",
})
}

View File

@ -194,7 +194,7 @@ func TestAPIEndpoint_PutCurrentOrgAddress_AccessControl(t *testing.T) {
// `/api/orgs/` endpoints test
// setupOrgsDBForAccessControlTests creates orgs up until orgID and fake user as member of org
func setupOrgsDBForAccessControlTests(t *testing.T, db sqlstore.Store, c accessControlScenarioContext, orgID int64) {
func setupOrgsDBForAccessControlTests(t *testing.T, db *sqlstore.SQLStore, c accessControlScenarioContext, orgID int64) {
t.Helper()
setInitCtxSignedInViewer(c.initCtx)
u := *c.initCtx.SignedInUser

View File

@ -122,8 +122,8 @@ func createUser(db sqlstore.Store, orgId int64, t *testing.T) int64 {
return user.ID
}
func setupTeamTestScenario(userCount int, db sqlstore.Store, t *testing.T) int64 {
teamService := teamimpl.ProvideService(db.(*sqlstore.SQLStore), setting.NewCfg()) // FIXME
func setupTeamTestScenario(userCount int, db *sqlstore.SQLStore, t *testing.T) int64 {
teamService := teamimpl.ProvideService(db, setting.NewCfg()) // FIXME
user, err := db.CreateUser(context.Background(), user.CreateUserCommand{SkipOrgSetup: true, Login: testUserLogin})
require.NoError(t, err)
testOrg, err := db.CreateOrgWithMember("TestOrg", user.ID)

View File

@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
@ -286,26 +287,28 @@ func (hs *HTTPServer) GetUserOrgList(c *models.ReqContext) response.Response {
}
func (hs *HTTPServer) getUserOrgList(ctx context.Context, userID int64) response.Response {
query := models.GetUserOrgListQuery{UserId: userID}
query := org.GetUserOrgListQuery{UserID: userID}
if err := hs.SQLStore.GetUserOrgList(ctx, &query); err != nil {
result, err := hs.orgService.GetUserOrgList(ctx, &query)
if err != nil {
return response.Error(500, "Failed to get user organizations", err)
}
return response.JSON(http.StatusOK, query.Result)
return response.JSON(http.StatusOK, result)
}
func (hs *HTTPServer) validateUsingOrg(ctx context.Context, userID int64, orgID int64) bool {
query := models.GetUserOrgListQuery{UserId: userID}
query := org.GetUserOrgListQuery{UserID: userID}
if err := hs.SQLStore.GetUserOrgList(ctx, &query); err != nil {
result, err := hs.orgService.GetUserOrgList(ctx, &query)
if err != nil {
return false
}
// validate that the org id in the list
valid := false
for _, other := range query.Result {
if other.OrgId == orgID {
for _, other := range result {
if other.OrgID == orgID {
valid = true
}
}

View File

@ -65,12 +65,10 @@ func TestMiddlewareAuth(t *testing.T) {
t *testing.T, sc *scenarioContext) {
sc.mockSQLStore.ExpectedOrg = &models.Org{Id: 1, Name: sc.cfg.AnonymousOrgName}
sc.orgService.ExpectedOrg = &org.Org{ID: 1, Name: sc.cfg.AnonymousOrgName}
org, err := sc.mockSQLStore.CreateOrgWithMember(sc.cfg.AnonymousOrgName, 1)
require.NoError(t, err)
sc.m.Get("/secure", reqSignIn, sc.defaultHandler)
sc.fakeReq("GET", fmt.Sprintf("/secure?orgId=%d", org.Id)).exec()
sc.fakeReq("GET", fmt.Sprintf("/secure?orgId=%d", 1)).exec()
assert.Equal(t, 200, sc.resp.Code)
}, configure)

View File

@ -342,12 +342,10 @@ func TestMiddlewareContext(t *testing.T) {
middlewareScenario(t, "When anonymous access is enabled", func(t *testing.T, sc *scenarioContext) {
sc.mockSQLStore.ExpectedOrg = &models.Org{Id: 1, Name: sc.cfg.AnonymousOrgName}
sc.orgService.ExpectedOrg = &org.Org{ID: 1, Name: sc.cfg.AnonymousOrgName}
orga, err := sc.mockSQLStore.CreateOrgWithMember(sc.cfg.AnonymousOrgName, 1)
require.NoError(t, err)
sc.fakeReq("GET", "/").exec()
assert.Equal(t, int64(0), sc.context.UserID)
assert.Equal(t, orga.Id, sc.context.OrgID)
assert.Equal(t, int64(1), sc.context.OrgID)
assert.Equal(t, org.RoleEditor, sc.context.OrgRole)
assert.False(t, sc.context.IsSignedIn)
}, func(cfg *setting.Cfg) {

View File

@ -271,8 +271,9 @@ func (ls *Implementation) syncOrgRoles(ctx context.Context, usr *user.User, extU
return nil
}
orgsQuery := &models.GetUserOrgListQuery{UserId: usr.ID}
if err := ls.SQLStore.GetUserOrgList(ctx, orgsQuery); err != nil {
orgsQuery := &org.GetUserOrgListQuery{UserID: usr.ID}
result, err := ls.orgService.GetUserOrgList(ctx, orgsQuery)
if err != nil {
return err
}
@ -280,15 +281,15 @@ func (ls *Implementation) syncOrgRoles(ctx context.Context, usr *user.User, extU
deleteOrgIds := []int64{}
// update existing org roles
for _, orga := range orgsQuery.Result {
handledOrgIds[orga.OrgId] = true
for _, orga := range result {
handledOrgIds[orga.OrgID] = true
extRole := extUser.OrgRoles[orga.OrgId]
extRole := extUser.OrgRoles[orga.OrgID]
if extRole == "" {
deleteOrgIds = append(deleteOrgIds, orga.OrgId)
deleteOrgIds = append(deleteOrgIds, orga.OrgID)
} else if extRole != orga.Role {
// update role
cmd := &org.UpdateOrgUserCommand{OrgID: orga.OrgId, UserID: usr.ID, Role: extRole}
cmd := &org.UpdateOrgUserCommand{OrgID: orga.OrgID, UserID: usr.ID, Role: extRole}
if err := ls.orgService.UpdateOrgUser(ctx, cmd); err != nil {
return err
}

View File

@ -14,7 +14,6 @@ import (
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/stretchr/testify/assert"
@ -26,15 +25,10 @@ func Test_syncOrgRoles_doesNotBreakWhenTryingToRemoveLastOrgAdmin(t *testing.T)
externalUser := createSimpleExternalUser()
authInfoMock := &logintest.AuthInfoServiceFake{}
store := &mockstore.SQLStoreMock{
ExpectedUserOrgList: createUserOrgDTO(),
ExpectedOrgListResponse: createResponseWithOneErrLastOrgAdminItem(),
}
login := Implementation{
QuotaService: &quotaimpl.Service{},
AuthInfoService: authInfoMock,
SQLStore: store,
SQLStore: nil,
userService: usertest.NewUserServiceFake(),
orgService: orgtest.NewOrgServiceFake(),
}
@ -52,18 +46,14 @@ func Test_syncOrgRoles_whenTryingToRemoveLastOrgLogsError(t *testing.T) {
authInfoMock := &logintest.AuthInfoServiceFake{}
store := &mockstore.SQLStoreMock{
ExpectedUserOrgList: createUserOrgDTO(),
ExpectedOrgListResponse: createResponseWithOneErrLastOrgAdminItem(),
}
orgService := orgtest.NewOrgServiceFake()
orgService.ExpectedError = models.ErrLastOrgAdmin
orgService.ExpectedUserOrgDTO = createUserOrgDTO()
orgService.ExpectedOrgListResponse = createResponseWithOneErrLastOrgAdminItem()
login := Implementation{
QuotaService: &quotaimpl.Service{},
AuthInfoService: authInfoMock,
SQLStore: store,
SQLStore: nil,
userService: usertest.NewUserServiceFake(),
orgService: orgService,
}
@ -132,20 +122,20 @@ func createSimpleUser() user.User {
return user
}
func createUserOrgDTO() []*models.UserOrgDTO {
users := []*models.UserOrgDTO{
func createUserOrgDTO() []*org.UserOrgDTO {
users := []*org.UserOrgDTO{
{
OrgId: 1,
OrgID: 1,
Name: "Bar",
Role: org.RoleViewer,
},
{
OrgId: 10,
OrgID: 10,
Name: "Foo",
Role: org.RoleAdmin,
},
{
OrgId: 11,
OrgID: 11,
Name: "Stuff",
Role: org.RoleViewer,
},
@ -164,14 +154,14 @@ func createSimpleExternalUser() models.ExternalUserInfo {
return externalUser
}
func createResponseWithOneErrLastOrgAdminItem() mockstore.OrgListResponse {
remResp := mockstore.OrgListResponse{
func createResponseWithOneErrLastOrgAdminItem() orgtest.OrgListResponse {
remResp := orgtest.OrgListResponse{
{
OrgId: 10,
OrgID: 10,
Response: models.ErrLastOrgAdmin,
},
{
OrgId: 11,
OrgID: 11,
Response: nil,
},
}

View File

@ -6,6 +6,11 @@ import (
"github.com/grafana/grafana/pkg/services/org"
)
type OrgListResponse []struct {
OrgID int64
Response error
}
type FakeOrgService struct {
ExpectedOrgUserID int64
ExpectedError error
@ -14,6 +19,7 @@ type FakeOrgService struct {
ExpectedOrg *org.Org
ExpectedOrgUsers []*org.OrgUserDTO
ExpectedSearchOrgUsersResult *org.SearchOrgUsersQueryResult
ExpectedOrgListResponse OrgListResponse
}
func NewOrgServiceFake() *FakeOrgService {
@ -85,7 +91,9 @@ func (f *FakeOrgService) GetOrgUsers(ctx context.Context, query *org.GetOrgUsers
}
func (f *FakeOrgService) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserCommand) error {
return f.ExpectedError
testData := f.ExpectedOrgListResponse[0]
f.ExpectedOrgListResponse = f.ExpectedOrgListResponse[1:]
return testData.Response
}
func (f *FakeOrgService) SearchOrgUsers(ctx context.Context, query *org.SearchOrgUsersQuery) (*org.SearchOrgUsersQueryResult, error) {

View File

@ -98,13 +98,6 @@ func (m *SQLStoreMock) GetOrgByNameHandler(ctx context.Context, query *models.Ge
return m.ExpectedError
}
func (m *SQLStoreMock) CreateOrgWithMember(name string, userID int64) (models.Org, error) {
return *m.ExpectedOrg, nil
}
func (m *SQLStoreMock) CreateOrg(ctx context.Context, cmd *models.CreateOrgCommand) error {
return m.ExpectedError
}
func (m *SQLStoreMock) UpdateOrgAddress(ctx context.Context, cmd *models.UpdateOrgAddressCommand) error {
return m.ExpectedError
}
@ -135,19 +128,10 @@ func (m *SQLStoreMock) CreateUser(ctx context.Context, cmd user.CreateUserComman
return nil, m.ExpectedError
}
func (m *SQLStoreMock) SetUsingOrg(ctx context.Context, cmd *models.SetUsingOrgCommand) error {
return m.ExpectedSetUsingOrgError
}
func (m *SQLStoreMock) GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error {
return m.ExpectedError
}
func (m *SQLStoreMock) GetUserOrgList(ctx context.Context, query *models.GetUserOrgListQuery) error {
query.Result = m.ExpectedUserOrgList
return m.ExpectedError
}
func (m *SQLStoreMock) GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error {
query.Result = m.ExpectedSignedInUser
return m.ExpectedError
@ -313,12 +297,6 @@ func (m *SQLStoreMock) SearchOrgUsers(ctx context.Context, query *models.SearchO
return m.ExpectedError
}
func (m *SQLStoreMock) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUserCommand) error {
testData := m.ExpectedOrgListResponse[0]
m.ExpectedOrgListResponse = m.ExpectedOrgListResponse[1:]
return testData.Response
}
func (m *SQLStoreMock) GetDashboardTags(ctx context.Context, query *models.GetDashboardTagsQuery) error {
return nil // TODO: Implement
}

View File

@ -19,13 +19,10 @@ type Store interface {
GetDBType() core.DbType
GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error
GetOrgByName(name string) (*models.Org, error)
CreateOrg(ctx context.Context, cmd *models.CreateOrgCommand) error
CreateOrgWithMember(name string, userID int64) (models.Org, error)
GetOrgById(context.Context, *models.GetOrgByIdQuery) error
GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error
CreateUser(ctx context.Context, cmd user.CreateUserCommand) (*user.User, error)
GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error
GetUserOrgList(ctx context.Context, query *models.GetUserOrgListQuery) error
GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error
WithDbSession(ctx context.Context, callback DBTransactionFunc) error
WithNewDbSession(ctx context.Context, callback DBTransactionFunc) error