Skip to content

Commit

Permalink
Change storeEvents to bool (#124)
Browse files Browse the repository at this point in the history
Before it was int value, which was used as limit for events count and it was confusing
  • Loading branch information
nt0xa authored Mar 8, 2023
1 parent c9b3d00 commit 1e68e81
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 89 deletions.
22 changes: 12 additions & 10 deletions internal/actions/payloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Payload struct {
Subdomain string `json:"subdomain"`
Name string `json:"name"`
NotifyProtocols []string `json:"notifyProtocols"`
StoreEvents int `json:"storeEvents"`
StoreEvents bool `json:"storeEvents"`
CreatedAt time.Time `json:"createdAt"`
}

Expand All @@ -41,7 +41,7 @@ type Payload struct {
type PayloadsCreateParams struct {
Name string `err:"name" json:"name"`
NotifyProtocols []string `err:"notifyProtocols" json:"notifyProtocols"`
StoreEvents int `err:"storeEvents" json:"storeEvents"`
StoreEvents bool `err:"storeEvents" json:"storeEvents"`
}

func (p PayloadsCreateParams) Validate() error {
Expand All @@ -51,8 +51,6 @@ func (p PayloadsCreateParams) Validate() error {
models.ProtoCategoriesAll.Strings(),
true,
))),
// TODO: Get max from config
validation.Field(&p.StoreEvents, validation.Min(0), validation.Max(1000)),
)
}

Expand All @@ -68,7 +66,7 @@ func PayloadsCreateCommand(p *PayloadsCreateParams, local bool) (*cobra.Command,

cmd.Flags().StringSliceVarP(&p.NotifyProtocols, "protocols", "p",
models.ProtoCategoriesAll.Strings(), "Protocols to notify")
cmd.Flags().IntVarP(&p.StoreEvents, "events", "e", 0, "Store events in database")
cmd.Flags().BoolVarP(&p.StoreEvents, "events", "e", false, "Store events in database")

return cmd, func(cmd *cobra.Command, args []string) errors.Error {
p.Name = args[0]
Expand All @@ -84,7 +82,7 @@ type PayloadsUpdateParams struct {
Name string `err:"name" json:"-" path:"name"`
NewName string `err:"newName" json:"name"`
NotifyProtocols []string `err:"notifyProtocols" json:"notifyProtocols"`
StoreEvents int `err:"storeEvents" json:"storeEvents"`
StoreEvents *bool `err:"storeEvents" json:"storeEvents"`
}

func (p PayloadsUpdateParams) Validate() error {
Expand All @@ -94,8 +92,6 @@ func (p PayloadsUpdateParams) Validate() error {
models.ProtoCategoriesAll.Strings(),
true,
))),
// We need -1 here to find out if the value was changed
validation.Field(&p.StoreEvents, validation.Min(-1), validation.Max(1000)),
)
}

Expand All @@ -109,13 +105,19 @@ func PayloadsUpdateCommand(p *PayloadsUpdateParams, local bool) (*cobra.Command,
Args: oneArg("NAME"),
}

var storeEvents bool

cmd.Flags().StringVarP(&p.NewName, "name", "n", "", "Payload name")
cmd.Flags().StringSliceVarP(&p.NotifyProtocols, "protocols", "p", nil, "Protocols to notify")
// We need -1 here to find out if the value was changed
cmd.Flags().IntVarP(&p.StoreEvents, "events", "e", -1, "Store events in database")
cmd.Flags().BoolVarP(&storeEvents, "events", "e", false, "Store events in database")

return cmd, func(cmd *cobra.Command, args []string) errors.Error {
p.Name = args[0]

if cmd.Flags().Lookup("events").Changed {
p.StoreEvents = &storeEvents
}

return nil
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/actionsdb/payloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func (act *dbactions) PayloadsUpdate(ctx context.Context, p actions.PayloadsUpda
payload.NotifyProtocols = models.ProtoCategories(slice.StringsDedup(p.NotifyProtocols)...)
}

if p.StoreEvents >= 0 {
payload.StoreEvents = p.StoreEvents
if p.StoreEvents != nil {
payload.StoreEvents = *p.StoreEvents
}

err = act.db.PayloadsUpdate(payload)
Expand Down
36 changes: 5 additions & 31 deletions internal/actionsdb/payloads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/russtone/sonar/internal/actionsdb"
"github.com/russtone/sonar/internal/database/models"
"github.com/russtone/sonar/internal/utils/errors"
"github.com/russtone/sonar/internal/utils/pointer"
)

func TestCreatePayload_Success(t *testing.T) {
Expand Down Expand Up @@ -41,7 +42,7 @@ func TestCreatePayload_Success(t *testing.T) {
"store events",
actions.PayloadsCreateParams{
Name: "test-dns",
StoreEvents: 100,
StoreEvents: true,
NotifyProtocols: []string{},
},
},
Expand Down Expand Up @@ -99,24 +100,6 @@ func TestCreatePayload_Error(t *testing.T) {
},
&errors.ConflictError{},
},
{
"invalid store events",
ctx,
actions.PayloadsCreateParams{
Name: "test",
StoreEvents: 999999,
},
&errors.ValidationError{},
},
{
"invalid store events",
ctx,
actions.PayloadsCreateParams{
Name: "test",
StoreEvents: -1,
},
&errors.ValidationError{},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -309,7 +292,7 @@ func TestUpdatePayload_Success(t *testing.T) {
"update stored events",
actions.PayloadsUpdateParams{
Name: "payload1",
StoreEvents: 8,
StoreEvents: pointer.Bool(true),
},
},
}
Expand All @@ -331,8 +314,8 @@ func TestUpdatePayload_Success(t *testing.T) {
assert.Equal(t, tt.p.NotifyProtocols, []string(r.NotifyProtocols))
}

if tt.p.StoreEvents >= 0 {
assert.Equal(t, tt.p.StoreEvents, r.StoreEvents)
if tt.p.StoreEvents != nil {
assert.Equal(t, *tt.p.StoreEvents, r.StoreEvents)
}
})
}
Expand Down Expand Up @@ -366,15 +349,6 @@ func TestUpdatePayload_Error(t *testing.T) {
},
&errors.ValidationError{},
},
{
"invalid stored events",
ctx,
actions.PayloadsUpdateParams{
Name: "payload1",
StoreEvents: -10,
},
&errors.ValidationError{},
},
{
"not existing payload",
ctx,
Expand Down
9 changes: 5 additions & 4 deletions internal/cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
actions_mock "github.com/russtone/sonar/internal/actions/mock"
"github.com/russtone/sonar/internal/cmd"
"github.com/russtone/sonar/internal/database/models"
"github.com/russtone/sonar/internal/utils/pointer"
)

var (
Expand Down Expand Up @@ -43,15 +44,15 @@ func TestCmd(t *testing.T) {
// Create

{
"new test -p dns,http -e 50",
"new test -p dns,http -e",
"PayloadsCreate",
actions.PayloadsCreateParams{
Name: "test",
NotifyProtocols: []string{
models.ProtoCategoryDNS.String(),
models.ProtoCategoryHTTP.String(),
},
StoreEvents: 50,
StoreEvents: true,
},
(actions.PayloadsCreateResult)(nil),
},
Expand All @@ -70,13 +71,13 @@ func TestCmd(t *testing.T) {
// Update

{
"mod -n new -p dns old -e 10",
"mod -n new -p dns old -e=false",
"PayloadsUpdate",
actions.PayloadsUpdateParams{
Name: "old",
NewName: "new",
NotifyProtocols: []string{models.ProtoCategoryDNS.String()},
StoreEvents: 10,
StoreEvents: pointer.Bool(false),
},
(actions.PayloadsUpdateResult)(nil),
},
Expand Down
5 changes: 3 additions & 2 deletions internal/cmd/server/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func (h *EventsHandler) AddNotifier(name string, notifier Notifier) {
}

func (h *EventsHandler) Start() error {
// TODO: use goroutines pool
for e := range h.events {

seen := make(map[string]struct{})
Expand All @@ -61,13 +62,13 @@ func (h *EventsHandler) Start() error {
e.PayloadID = p.ID

// Store event in database
if p.StoreEvents > 0 {
if p.StoreEvents {
if err := h.db.EventsCreate(e); err != nil {
continue
}

// Delete out of limit events
if err := h.db.EventsDeleteOutOfLimit(p.ID, p.StoreEvents); err != nil && err != sql.ErrNoRows {
if err := h.db.EventsDeleteOutOfLimit(p.ID, 1000 /* TODO: from config */); err != nil && err != sql.ErrNoRows {
continue
}
}
Expand Down
8 changes: 4 additions & 4 deletions internal/database/fixtures/payloads.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,29 @@
subdomain: c1da9f3d
name: payload1
notify_protocols: '{dns, http, smtp}'
store_events: 50
store_events: true
created_at: RAW=NOW() AT TIME ZONE 'UTC'

- id: 2
user_id: 2
subdomain: 6564e0c7
name: payload2
notify_protocols: '{dns, http, smtp}'
store_events: 0
store_events: false
created_at: RAW=NOW() AT TIME ZONE 'UTC'

- id: 3
user_id: 2
subdomain: 730d6a4a
name: payload3
notify_protocols: '{dns, http, smtp}'
store_events: 100
store_events: true
created_at: RAW=NOW() AT TIME ZONE 'UTC'

- id: 4
user_id: 1
subdomain: eb0d48c6
name: payload4
notify_protocols: '{dns, http, smtp}'
store_events: 10
store_events: true
created_at: RAW=NOW() AT TIME ZONE 'UTC'
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE payloads
ALTER COLUMN store_events DROP DEFAULT,
ALTER COLUMN store_events TYPE INT USING (CASE WHEN store_events THEN 1000 ELSE 0 END),
ALTER COLUMN store_events SET DEFAULT 0;
5 changes: 5 additions & 0 deletions internal/database/migrations/000018_store_events_bool.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE payloads
ALTER COLUMN store_events DROP DEFAULT,
ALTER COLUMN store_events TYPE BOOL USING store_events > 0,
ALTER COLUMN store_events SET DEFAULT false;

2 changes: 1 addition & 1 deletion internal/database/models/payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ type Payload struct {
Subdomain string `db:"subdomain"`
Name string `db:"name"`
NotifyProtocols ProtoCategoryArray `db:"notify_protocols"`
StoreEvents int `db:"store_events"`
StoreEvents bool `db:"store_events"`
CreatedAt time.Time `db:"created_at"`
}
6 changes: 3 additions & 3 deletions internal/database/payloads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func TestPayloadsCreate_Success(t *testing.T) {
Subdomain: "8a8b58beaf",
Name: "test",
NotifyProtocols: models.ProtoCategoriesAll,
StoreEvents: 10,
StoreEvents: true,
}

err := db.PayloadsCreate(o)
assert.NoError(t, err)
assert.NotZero(t, o.ID)
assert.WithinDuration(t, time.Now(), o.CreatedAt, 5*time.Second)
assert.Equal(t, models.ProtoCategoriesAll, o.NotifyProtocols)
assert.Equal(t, 10, o.StoreEvents)
assert.Equal(t, true, o.StoreEvents)
}

func TestPayloadsCreate_Duplicate(t *testing.T) {
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestPayloadsUpdate_Success(t *testing.T) {

o.Name = "payload1_updated"
o.NotifyProtocols = models.ProtoCategories("dns")
o.StoreEvents = 100
o.StoreEvents = false

err = db.PayloadsUpdate(o)
require.NoError(t, err)
Expand Down
36 changes: 6 additions & 30 deletions internal/modules/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestAPI(t *testing.T) {
method: "POST",
path: "/payloads",
token: User1Token,
json: `{"name": "test", "notifyProtocols": ["dns", "smtp"], "storeEvents": 10}`,
json: `{"name": "test", "notifyProtocols": ["dns", "smtp"], "storeEvents": true}`,
schema: (actions.PayloadsCreateResult)(nil),
result: map[string]matcher{
"$.subdomain": regex(regexp.MustCompile("^[a-f0-9]{8}$")),
Expand All @@ -159,7 +159,7 @@ func TestAPI(t *testing.T) {
models.ProtoCategorySMTP.String(),
},
),
"$.storeEvents": equal(10),
"$.storeEvents": equal(true),
"$.createdAt": withinDuration(time.Second * 10),
},
status: 201,
Expand Down Expand Up @@ -188,18 +188,6 @@ func TestAPI(t *testing.T) {
},
status: 400,
},
{
method: "POST",
path: "/payloads",
token: User1Token,
json: `{"name": "test", "storeEvents": -1}`,
schema: &errors.ValidationError{},
result: map[string]matcher{
"$.message": contains("validation"),
"$.errors.storeEvents": notEmpty(),
},
status: 400,
},
{
method: "POST",
path: "/payloads",
Expand Down Expand Up @@ -243,25 +231,25 @@ func TestAPI(t *testing.T) {
method: "PUT",
path: "/payloads/payload1",
token: User1Token,
json: `{"name":"test", "notifyProtocols": ["smtp"], "storeEvents": 11}`,
json: `{"name":"test", "notifyProtocols": ["smtp"], "storeEvents": false}`,
schema: (actions.PayloadsUpdateResult)(nil),
result: map[string]matcher{
"$.name": equal("test"),
"$.notifyProtocols": equal([]interface{}{models.ProtoCategorySMTP.String()}),
"$.storeEvents": equal(11),
"$.storeEvents": equal(false),
},
status: 200,
},
{
method: "PUT",
path: "/payloads/payload1",
token: User1Token,
json: `{"name":"test", "notifyProtocols": ["smtp"], "storeEvents": -1}`,
json: `{"name":"test", "notifyProtocols": ["smtp"], "storeEvents": null}`,
schema: (actions.PayloadsUpdateResult)(nil),
result: map[string]matcher{
"$.name": equal("test"),
"$.notifyProtocols": equal([]interface{}{models.ProtoCategorySMTP.String()}),
"$.storeEvents": equal(50), // Must not be changed
"$.storeEvents": equal(true), // Must not be changed
},
status: 200,
},
Expand Down Expand Up @@ -289,18 +277,6 @@ func TestAPI(t *testing.T) {
},
status: 400,
},
{
method: "PUT",
path: "/payloads/payload1",
token: User1Token,
json: `{"storeEvents": 999999}`,
schema: &errors.ValidationError{},
result: map[string]matcher{
"$.message": contains("validation"),
"$.errors.storeEvents": notEmpty(),
},
status: 400,
},
{
method: "PUT",
path: "/payloads/invalid",
Expand Down
Loading

0 comments on commit 1e68e81

Please sign in to comment.