From 061e06c226095a8279a53646451274276463121b Mon Sep 17 00:00:00 2001 From: moznion Date: Wed, 5 Dec 2018 22:19:40 +0900 Subject: [PATCH 1/3] Fix bug what updating org quota doesn't work https://github.com/grafana/grafana/blob/3c330c8e4c0b0f9fb258801ba8a7fe2586bbc819/pkg/services/sqlstore/quota.go#L106 In the real use case, `has` that is described by the above code is always `false` because it includes `Updated` in a query. So this commit fixes this issue. --- pkg/services/sqlstore/quota.go | 6 +++--- pkg/services/sqlstore/quota_test.go | 32 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/pkg/services/sqlstore/quota.go b/pkg/services/sqlstore/quota.go index 7005b341268..57c2c2476a0 100644 --- a/pkg/services/sqlstore/quota.go +++ b/pkg/services/sqlstore/quota.go @@ -99,14 +99,14 @@ func UpdateOrgQuota(cmd *m.UpdateOrgQuotaCmd) error { return inTransaction(func(sess *DBSession) error { //Check if quota is already defined in the DB quota := m.Quota{ - Target: cmd.Target, - OrgId: cmd.OrgId, - Updated: time.Now(), + Target: cmd.Target, + OrgId: cmd.OrgId, } has, err := sess.Get("a) if err != nil { return err } + quota.Updated = time.Now() quota.Limit = cmd.Limit if !has { quota.Created = time.Now() diff --git a/pkg/services/sqlstore/quota_test.go b/pkg/services/sqlstore/quota_test.go index 49e028e9cd3..f6ededb5b8b 100644 --- a/pkg/services/sqlstore/quota_test.go +++ b/pkg/services/sqlstore/quota_test.go @@ -2,6 +2,7 @@ package sqlstore import ( "testing" + "time" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" @@ -168,5 +169,36 @@ func TestQuotaCommandsAndQueries(t *testing.T) { So(query.Result.Limit, ShouldEqual, 5) So(query.Result.Used, ShouldEqual, 1) }) + + Convey("Should org quota updating is successful even if it called multiple time", func() { + orgCmd := m.UpdateOrgQuotaCmd{ + OrgId: orgId, + Target: "org_user", + Limit: 5, + } + err := UpdateOrgQuota(&orgCmd) + So(err, ShouldBeNil) + + query := m.GetOrgQuotaByTargetQuery{OrgId: orgId, Target: "org_user", Default: 1} + err = GetOrgQuotaByTarget(&query) + So(err, ShouldBeNil) + So(query.Result.Limit, ShouldEqual, 5) + + // XXX: resolution of `Updated` column is 1sec, so this makes delay + time.Sleep(1 * time.Second) + + orgCmd = m.UpdateOrgQuotaCmd{ + OrgId: orgId, + Target: "org_user", + Limit: 10, + } + err = UpdateOrgQuota(&orgCmd) + So(err, ShouldBeNil) + + query = m.GetOrgQuotaByTargetQuery{OrgId: orgId, Target: "org_user", Default: 1} + err = GetOrgQuotaByTarget(&query) + So(err, ShouldBeNil) + So(query.Result.Limit, ShouldEqual, 10) + }) }) } From d1e1cde00e5b918c514118f81d04fa7bd984b052 Mon Sep 17 00:00:00 2001 From: moznion Date: Wed, 5 Dec 2018 22:29:07 +0900 Subject: [PATCH 2/3] Fix bug what updating user quota doesn't work Reason is same as 061e06c226095a8279a53646451274276463121b --- pkg/services/sqlstore/quota.go | 6 +++--- pkg/services/sqlstore/quota_test.go | 31 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/pkg/services/sqlstore/quota.go b/pkg/services/sqlstore/quota.go index 57c2c2476a0..e90b7fec131 100644 --- a/pkg/services/sqlstore/quota.go +++ b/pkg/services/sqlstore/quota.go @@ -201,14 +201,14 @@ func UpdateUserQuota(cmd *m.UpdateUserQuotaCmd) error { return inTransaction(func(sess *DBSession) error { //Check if quota is already defined in the DB quota := m.Quota{ - Target: cmd.Target, - UserId: cmd.UserId, - Updated: time.Now(), + Target: cmd.Target, + UserId: cmd.UserId, } has, err := sess.Get("a) if err != nil { return err } + quota.Updated = time.Now() quota.Limit = cmd.Limit if !has { quota.Created = time.Now() diff --git a/pkg/services/sqlstore/quota_test.go b/pkg/services/sqlstore/quota_test.go index f6ededb5b8b..8ace14a6a75 100644 --- a/pkg/services/sqlstore/quota_test.go +++ b/pkg/services/sqlstore/quota_test.go @@ -200,5 +200,36 @@ func TestQuotaCommandsAndQueries(t *testing.T) { So(err, ShouldBeNil) So(query.Result.Limit, ShouldEqual, 10) }) + + Convey("Should user quota updating is successful even if it called multiple time", func() { + userQuotaCmd := m.UpdateUserQuotaCmd{ + UserId: userId, + Target: "org_user", + Limit: 5, + } + err := UpdateUserQuota(&userQuotaCmd) + So(err, ShouldBeNil) + + query := m.GetUserQuotaByTargetQuery{UserId: userId, Target: "org_user", Default: 1} + err = GetUserQuotaByTarget(&query) + So(err, ShouldBeNil) + So(query.Result.Limit, ShouldEqual, 5) + + // XXX: resolution of `Updated` column is 1sec, so this makes delay + time.Sleep(1 * time.Second) + + userQuotaCmd = m.UpdateUserQuotaCmd{ + UserId: userId, + Target: "org_user", + Limit: 10, + } + err = UpdateUserQuota(&userQuotaCmd) + So(err, ShouldBeNil) + + query = m.GetUserQuotaByTargetQuery{UserId: userId, Target: "org_user", Default: 1} + err = GetUserQuotaByTarget(&query) + So(err, ShouldBeNil) + So(query.Result.Limit, ShouldEqual, 10) + }) }) } From 4397ee61d09623bbd23e712196f6bca512c53aa6 Mon Sep 17 00:00:00 2001 From: moznion Date: Wed, 5 Dec 2018 22:47:41 +0900 Subject: [PATCH 3/3] Put issue number to test code --- pkg/services/sqlstore/quota_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/services/sqlstore/quota_test.go b/pkg/services/sqlstore/quota_test.go index 8ace14a6a75..976d54d10e2 100644 --- a/pkg/services/sqlstore/quota_test.go +++ b/pkg/services/sqlstore/quota_test.go @@ -170,6 +170,7 @@ func TestQuotaCommandsAndQueries(t *testing.T) { So(query.Result.Used, ShouldEqual, 1) }) + // related: https://github.com/grafana/grafana/issues/14342 Convey("Should org quota updating is successful even if it called multiple time", func() { orgCmd := m.UpdateOrgQuotaCmd{ OrgId: orgId, @@ -201,6 +202,7 @@ func TestQuotaCommandsAndQueries(t *testing.T) { So(query.Result.Limit, ShouldEqual, 10) }) + // related: https://github.com/grafana/grafana/issues/14342 Convey("Should user quota updating is successful even if it called multiple time", func() { userQuotaCmd := m.UpdateUserQuotaCmd{ UserId: userId,