From c69937945a701580d9f4dbf5acfcb728a72233bc Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Mon, 20 Sep 2021 14:47:06 -0300 Subject: [PATCH] Introduce `entityName` type for mysql entity table names (#2139) * Introduce entity type to specify mysql table names for deleteEntit* functions * Remove changes entry for issue (non-user facing changes) --- server/datastore/mysql/delete.go | 18 +++++++++--------- server/datastore/mysql/delete_test.go | 6 +++--- server/datastore/mysql/hosts.go | 2 +- server/datastore/mysql/invites.go | 2 +- server/datastore/mysql/labels.go | 2 +- server/datastore/mysql/mysql.go | 16 ++++++++++++++++ server/datastore/mysql/packs.go | 2 +- server/datastore/mysql/queries.go | 4 ++-- server/datastore/mysql/scheduled_queries.go | 2 +- server/datastore/mysql/sessions.go | 2 +- server/datastore/mysql/teams.go | 2 +- server/datastore/mysql/users.go | 2 +- 12 files changed, 38 insertions(+), 22 deletions(-) diff --git a/server/datastore/mysql/delete.go b/server/datastore/mysql/delete.go index 384327b8b0..f310769469 100644 --- a/server/datastore/mysql/delete.go +++ b/server/datastore/mysql/delete.go @@ -10,41 +10,41 @@ import ( // deleteEntity deletes an entity with the given id from the given DB table, // returning a notFound error if appropriate. -func (d *Datastore) deleteEntity(ctx context.Context, dbTable string, id uint) error { - deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id = ?`, dbTable) +func (d *Datastore) deleteEntity(ctx context.Context, dbTable entity, id uint) error { + deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id = ?`, dbTable.name) result, err := d.writer.ExecContext(ctx, deleteStmt, id) if err != nil { return errors.Wrapf(err, "delete %s", dbTable) } rows, _ := result.RowsAffected() if rows != 1 { - return notFound(dbTable).WithID(id) + return notFound(dbTable.name).WithID(id) } return nil } // deleteEntityByName deletes an entity with the given name from the given DB // table, returning a notFound error if appropriate. -func (d *Datastore) deleteEntityByName(ctx context.Context, dbTable string, name string) error { - deleteStmt := fmt.Sprintf("DELETE FROM %s WHERE name = ?", dbTable) +func (d *Datastore) deleteEntityByName(ctx context.Context, dbTable entity, name string) error { + deleteStmt := fmt.Sprintf("DELETE FROM %s WHERE name = ?", dbTable.name) result, err := d.writer.ExecContext(ctx, deleteStmt, name) if err != nil { if isMySQLForeignKey(err) { - return foreignKey(dbTable, name) + return foreignKey(dbTable.name, name) } return errors.Wrapf(err, "delete %s", dbTable) } rows, _ := result.RowsAffected() if rows != 1 { - return notFound(dbTable).WithName(name) + return notFound(dbTable.name).WithName(name) } return nil } // deleteEntities deletes the existing entity objects with the provided IDs. // The number of deleted entities is returned along with any error. -func (d *Datastore) deleteEntities(ctx context.Context, dbTable string, ids []uint) (uint, error) { - deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id IN (?)`, dbTable) +func (d *Datastore) deleteEntities(ctx context.Context, dbTable entity, ids []uint) (uint, error) { + deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id IN (?)`, dbTable.name) query, args, err := sqlx.In(deleteStmt, ids) if err != nil { diff --git a/server/datastore/mysql/delete_test.go b/server/datastore/mysql/delete_test.go index abe0cc26fd..47cddd036a 100644 --- a/server/datastore/mysql/delete_test.go +++ b/server/datastore/mysql/delete_test.go @@ -29,7 +29,7 @@ func TestDeleteEntity(t *testing.T) { require.NoError(t, err) require.NotNil(t, host) - require.NoError(t, ds.deleteEntity(context.Background(), "hosts", host.ID)) + require.NoError(t, ds.deleteEntity(context.Background(), hostsTable, host.ID)) host, err = ds.Host(context.Background(), host.ID) require.Error(t, err) @@ -42,7 +42,7 @@ func TestDeleteEntityByName(t *testing.T) { query1 := test.NewQuery(t, ds, t.Name()+"time", "select * from time", 0, true) - require.NoError(t, ds.deleteEntityByName(context.Background(), "queries", query1.Name)) + require.NoError(t, ds.deleteEntityByName(context.Background(), queriesTable, query1.Name)) gotQ, err := ds.Query(context.Background(), query1.ID) require.Error(t, err) @@ -57,7 +57,7 @@ func TestDeleteEntities(t *testing.T) { query2 := test.NewQuery(t, ds, t.Name()+"time2", "select * from time", 0, true) query3 := test.NewQuery(t, ds, t.Name()+"time3", "select * from time", 0, true) - count, err := ds.deleteEntities(context.Background(), "queries", []uint{query1.ID, query2.ID}) + count, err := ds.deleteEntities(context.Background(), queriesTable, []uint{query1.ID, query2.ID}) require.NoError(t, err) assert.Equal(t, uint(2), count) diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index e23566b0d6..2af56b0e2d 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -292,7 +292,7 @@ func loadHostUsersDB(ctx context.Context, db sqlx.QueryerContext, host *fleet.Ho } func (d *Datastore) DeleteHost(ctx context.Context, hid uint) error { - err := d.deleteEntity(ctx, "hosts", hid) + err := d.deleteEntity(ctx, hostsTable, hid) if err != nil { return errors.Wrapf(err, "deleting host with id %d", hid) } diff --git a/server/datastore/mysql/invites.go b/server/datastore/mysql/invites.go index bd92bb8aaa..c2d9371c75 100644 --- a/server/datastore/mysql/invites.go +++ b/server/datastore/mysql/invites.go @@ -138,7 +138,7 @@ func (d *Datastore) InviteByToken(ctx context.Context, token string) (*fleet.Inv } func (d *Datastore) DeleteInvite(ctx context.Context, id uint) error { - return d.deleteEntity(ctx, "invites", id) + return d.deleteEntity(ctx, invitesTable, id) } func (d *Datastore) loadTeamsForInvites(ctx context.Context, invites []*fleet.Invite) error { diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 5da41d18da..a006ffe193 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -232,7 +232,7 @@ func (d *Datastore) SaveLabel(ctx context.Context, label *fleet.Label) (*fleet.L // DeleteLabel deletes a fleet.Label func (d *Datastore) DeleteLabel(ctx context.Context, name string) error { - return d.deleteEntityByName(ctx, "labels", name) + return d.deleteEntityByName(ctx, labelsTable, name) } // Label returns a fleet.Label identified by lid if one exists. diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go index 6d72fab858..9664df5111 100644 --- a/server/datastore/mysql/mysql.go +++ b/server/datastore/mysql/mysql.go @@ -62,6 +62,22 @@ type Datastore struct { type txFn func(sqlx.ExtContext) error +type entity struct { + name string +} + +var ( + hostsTable = entity{"hosts"} + invitesTable = entity{"invites"} + labelsTable = entity{"labels"} + packsTable = entity{"packs"} + queriesTable = entity{"queries"} + scheduledQueriesTable = entity{"scheduled_queries"} + sessionsTable = entity{"sessions"} + teamsTable = entity{"teams"} + usersTable = entity{"users"} +) + // retryableError determines whether a MySQL error can be retried. By default // errors are considered non-retryable. Only errors that we know have a // possibility of succeeding on a retry should return true in this function. diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index 20b2e89fed..3e03afc873 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -365,7 +365,7 @@ func (d *Datastore) SavePack(ctx context.Context, pack *fleet.Pack) error { // DeletePack deletes a fleet.Pack so that it won't show up in results. func (d *Datastore) DeletePack(ctx context.Context, name string) error { - return d.deleteEntityByName(ctx, "packs", name) + return d.deleteEntityByName(ctx, packsTable, name) } // Pack fetch fleet.Pack with matching ID diff --git a/server/datastore/mysql/queries.go b/server/datastore/mysql/queries.go index e15248ce9d..8a6eabeafd 100644 --- a/server/datastore/mysql/queries.go +++ b/server/datastore/mysql/queries.go @@ -138,13 +138,13 @@ func (d *Datastore) SaveQuery(ctx context.Context, q *fleet.Query) error { // DeleteQuery deletes Query identified by Query.ID. func (d *Datastore) DeleteQuery(ctx context.Context, name string) error { - return d.deleteEntityByName(ctx, "queries", name) + return d.deleteEntityByName(ctx, queriesTable, name) } // DeleteQueries deletes the existing query objects with the provided IDs. The // number of deleted queries is returned along with any error. func (d *Datastore) DeleteQueries(ctx context.Context, ids []uint) (uint, error) { - return d.deleteEntities(ctx, "queries", ids) + return d.deleteEntities(ctx, queriesTable, ids) } // Query returns a single Query identified by id, if such exists. diff --git a/server/datastore/mysql/scheduled_queries.go b/server/datastore/mysql/scheduled_queries.go index 8605f60b35..560ce5bec9 100644 --- a/server/datastore/mysql/scheduled_queries.go +++ b/server/datastore/mysql/scheduled_queries.go @@ -123,7 +123,7 @@ func saveScheduledQueryDB(ctx context.Context, exec sqlx.ExecerContext, sq *flee } func (d *Datastore) DeleteScheduledQuery(ctx context.Context, id uint) error { - return d.deleteEntity(ctx, "scheduled_queries", id) + return d.deleteEntity(ctx, scheduledQueriesTable, id) } func (d *Datastore) ScheduledQuery(ctx context.Context, id uint) (*fleet.ScheduledQuery, error) { diff --git a/server/datastore/mysql/sessions.go b/server/datastore/mysql/sessions.go index 16fb65c8fb..02e8273beb 100644 --- a/server/datastore/mysql/sessions.go +++ b/server/datastore/mysql/sessions.go @@ -70,7 +70,7 @@ func (d *Datastore) NewSession(ctx context.Context, session *fleet.Session) (*fl } func (d *Datastore) DestroySession(ctx context.Context, session *fleet.Session) error { - err := d.deleteEntity(ctx, "sessions", session.ID) + err := d.deleteEntity(ctx, sessionsTable, session.ID) if err != nil { return errors.Wrap(err, "deleting session") } diff --git a/server/datastore/mysql/teams.go b/server/datastore/mysql/teams.go index 122a734cc5..183f6c4dac 100644 --- a/server/datastore/mysql/teams.go +++ b/server/datastore/mysql/teams.go @@ -80,7 +80,7 @@ func saveTeamSecretsDB(ctx context.Context, exec sqlx.ExecerContext, team *fleet } func (d *Datastore) DeleteTeam(ctx context.Context, tid uint) error { - if err := d.deleteEntity(ctx, "teams", tid); err != nil { + if err := d.deleteEntity(ctx, teamsTable, tid); err != nil { return errors.Wrapf(err, "delete team id %d", tid) } return nil diff --git a/server/datastore/mysql/users.go b/server/datastore/mysql/users.go index 29399b1114..4f801295ca 100644 --- a/server/datastore/mysql/users.go +++ b/server/datastore/mysql/users.go @@ -267,5 +267,5 @@ func saveTeamsForUserDB(ctx context.Context, tx sqlx.ExtContext, user *fleet.Use // DeleteUser deletes the associated user func (d *Datastore) DeleteUser(ctx context.Context, id uint) error { - return d.deleteEntity(ctx, "users", id) + return d.deleteEntity(ctx, usersTable, id) }