Skip to content
This repository has been archived by the owner on Dec 15, 2020. It is now read-only.

Remove soft-deletion pattern #2327

Merged
merged 8 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/development/database-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ cd server/datastore/mysql/migrations/tables
goose create AddColumnFooToUsers
```

Find the file you created in the migrations directory and edit it. You can then update the database by running the following shell commands:
Find the file you created in the migrations directory and edit it:

* delete the import line for goose: `github.com/pressly/goose`
* change `goose.AddMigration(...)` to `MigrationClient.AddMigration(...)`
* add your migration code

You can then update the database by running the following shell commands:

``` bash
make build
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ const queryResult = {

const campaignWithNoQueryResults = {
created_at: '0001-01-01T00:00:00Z',
deleted: false,
deleted_at: null,
hosts_count: {
failed: 0,
successful: 0,
Expand Down
8 changes: 0 additions & 8 deletions frontend/test/stubs.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ export const flatConfigStub = {
export const hostStub = {
created_at: '2017-01-10T19:18:55Z',
updated_at: '2017-01-10T20:13:52Z',
deleted_at: null,
deleted: false,
id: 1,
detail_updated_at: '2017-01-10T20:01:48Z',
seen_time: '2017-01-10T20:13:54Z',
Expand Down Expand Up @@ -95,8 +93,6 @@ export const hostStub = {
export const labelStub = {
created_at: '2017-01-16T23:11:01Z',
updated_at: '2017-01-16T23:11:01Z',
deleted_at: null,
deleted: false,
id: 1,
name: 'All Hosts',
description: '',
Expand All @@ -116,8 +112,6 @@ export const labelStub = {
export const packStub = {
created_at: '0001-01-01T00:00:00Z',
updated_at: '0001-01-01T00:00:00Z',
deleted_at: null,
deleted: false,
id: 3,
name: 'Pack Name',
description: 'Pack Description',
Expand All @@ -130,8 +124,6 @@ export const packStub = {

export const queryStub = {
created_at: '2016-10-17T07:06:00Z',
deleted: false,
deleted_at: null,
description: '',
differential: false,
id: 1,
Expand Down
22 changes: 0 additions & 22 deletions server/datastore/datastore_hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,6 @@ func testDeleteHost(t *testing.T, ds kolide.Datastore) {
assert.NotNil(t, err)
}

func testIdempotentDeleteHost(t *testing.T, ds kolide.Datastore) {
host, err := ds.NewHost(&kolide.Host{
DetailUpdateTime: time.Now(),
LabelUpdateTime: time.Now(),
SeenTime: time.Now(),
NodeKey: "1",
UUID: "1",
HostName: "foo.local",
})
require.Nil(t, err)
require.NotNil(t, host)
id := host.ID
err = ds.DeleteHost(host.ID)
assert.Nil(t, err)

host, err = ds.Host(host.ID)
assert.NotNil(t, err)

err = ds.DeleteHost(id)
assert.Nil(t, err)
}

func testListHost(t *testing.T, ds kolide.Datastore) {
hosts := []*kolide.Host{}
for i := 0; i < 10; i++ {
Expand Down
1 change: 0 additions & 1 deletion server/datastore/datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ var testFunctions = [...]func(*testing.T, kolide.Datastore){
testMarkHostSeen,
testCleanupIncomingHosts,
testDuplicateNewQuery,
testIdempotentDeleteHost,
testChangeEmail,
testChangeLabelDetails,
testMigrationStatus,
Expand Down
4 changes: 2 additions & 2 deletions server/datastore/inmem/queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (d *Datastore) SaveQuery(query *kolide.Query) error {
return nil
}

// DeleteQueries (soft) deletes the existing query objects with the provided
// IDs. The number of deleted queries is returned along with any error.
// 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(ids []uint) (uint, error) {
d.mtx.Lock()
defer d.mtx.Unlock()
Expand Down
3 changes: 1 addition & 2 deletions server/datastore/mysql/campaigns.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (d *Datastore) NewDistributedQueryCampaign(camp *kolide.DistributedQueryCam

func (d *Datastore) DistributedQueryCampaign(id uint) (*kolide.DistributedQueryCampaign, error) {
sql := `
SELECT * FROM distributed_query_campaigns WHERE id = ? AND NOT deleted
SELECT * FROM distributed_query_campaigns WHERE id = ?
`
campaign := &kolide.DistributedQueryCampaign{}
if err := d.db.Get(campaign, sql, id); err != nil {
Expand All @@ -47,7 +47,6 @@ func (d *Datastore) SaveDistributedQueryCampaign(camp *kolide.DistributedQueryCa
status = ?,
user_id = ?
WHERE id = ?
AND NOT deleted
`
result, err := d.db.Exec(sqlStatement, camp.QueryID, camp.Status, camp.UserID, camp.ID)
if err != nil {
Expand Down
39 changes: 30 additions & 9 deletions server/datastore/mysql/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ package mysql
import (
"fmt"

"github.com/jmoiron/sqlx"
"github.com/pkg/errors"
)

// deleteEntity deletes an entity with the given id from the given DB table,
// returning a notFound error if appropriate.
func (d *Datastore) deleteEntity(dbTable string, id uint) error {
deleteStmt := fmt.Sprintf(
`
UPDATE %s SET deleted_at = ?, deleted = TRUE
WHERE id = ? AND NOT deleted
`, dbTable)
result, err := d.db.Exec(deleteStmt, d.clock.Now(), id)
deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id = ?`, dbTable)
result, err := d.db.Exec(deleteStmt, id)
if err != nil {
return errors.Wrapf(err, "delete %s", dbTable)
}
Expand All @@ -23,9 +22,8 @@ func (d *Datastore) deleteEntity(dbTable string, id uint) error {
return nil
}

// deleteEntityByName hard deletes an entity with the given name from the given
// DB table, returning a notFound error if appropriate.
// Note: deleteEntity uses soft deletion, but we are moving off this pattern.
// deleteEntityByName deletes an entity with the given name from the given DB
// table, returning a notFound error if appropriate.
func (d *Datastore) deleteEntityByName(dbTable string, name string) error {
deleteStmt := fmt.Sprintf("DELETE FROM %s WHERE name = ?", dbTable)
result, err := d.db.Exec(deleteStmt, name)
Expand All @@ -41,3 +39,26 @@ func (d *Datastore) deleteEntityByName(dbTable string, name string) error {
}
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(dbTable string, ids []uint) (uint, error) {
deleteStmt := fmt.Sprintf(`DELETE FROM %s WHERE id IN (?)`, dbTable)

query, args, err := sqlx.In(deleteStmt, ids)
if err != nil {
return 0, errors.Wrapf(err, "building delete entities query %s", dbTable)
}

result, err := d.db.Exec(query, args...)
if err != nil {
return 0, errors.Wrapf(err, "executing delete entities query %s", dbTable)
}

deleted, err := result.RowsAffected()
if err != nil {
return 0, errors.Wrapf(err, "fetching delete entities query rows affected %s", dbTable)
}

return uint(deleted), nil
}
19 changes: 6 additions & 13 deletions server/datastore/mysql/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (d *Datastore) SaveHost(host *kolide.Host) error {
}

func (d *Datastore) DeleteHost(hid uint) error {
_, err := d.db.Exec("DELETE FROM hosts WHERE id = ?", hid)
err := d.deleteEntity("hosts", hid)
if err != nil {
return errors.Wrapf(err, "deleting host with id %d", hid)
}
Expand All @@ -140,7 +140,7 @@ func (d *Datastore) DeleteHost(hid uint) error {
func (d *Datastore) Host(id uint) (*kolide.Host, error) {
sqlStatement := `
SELECT * FROM hosts
WHERE id = ? AND NOT deleted LIMIT 1
WHERE id = ? LIMIT 1
`
host := &kolide.Host{}
err := d.db.Get(host, sqlStatement, id)
Expand All @@ -154,7 +154,6 @@ func (d *Datastore) Host(id uint) (*kolide.Host, error) {
func (d *Datastore) ListHosts(opt kolide.HostListOptions) ([]*kolide.Host, error) {
sql := `
SELECT * FROM hosts
WHERE NOT deleted
`
var params []interface{}
switch opt.StatusFilter {
Expand Down Expand Up @@ -244,8 +243,7 @@ func (d *Datastore) EnrollHost(osqueryHostID, nodeKey, secretName string) (*koli
enroll_secret_name
) VALUES (?, ?, ?, ?, ?, ?)
ON DUPLICATE KEY UPDATE
node_key = VALUES(node_key),
deleted = FALSE
node_key = VALUES(node_key)
`

var result sql.Result
Expand Down Expand Up @@ -281,8 +279,6 @@ func (d *Datastore) AuthenticateHost(nodeKey string) (*kolide.Host, error) {
osquery_host_id,
created_at,
updated_at,
deleted_at,
deleted,
detail_update_time,
label_update_time,
node_key,
Expand Down Expand Up @@ -313,7 +309,7 @@ func (d *Datastore) AuthenticateHost(nodeKey string) (*kolide.Host, error) {
config_tls_refresh,
enroll_secret_name
FROM hosts
WHERE node_key = ? AND NOT deleted
WHERE node_key = ?
LIMIT 1
`

Expand Down Expand Up @@ -359,7 +355,6 @@ func (d *Datastore) searchHostsWithOmits(query string, omit ...uint) ([]*kolide.
MATCH (host_name, uuid) AGAINST (? IN BOOLEAN MODE)
OR MATCH (primary_ip, primary_mac) AGAINST (? IN BOOLEAN MODE)
)
AND NOT deleted
AND id NOT IN (?)
LIMIT 10
`
Expand All @@ -383,16 +378,15 @@ func (d *Datastore) searchHostsWithOmits(query string, omit ...uint) ([]*kolide.
func (d *Datastore) searchHostsDefault(omit ...uint) ([]*kolide.Host, error) {
sqlStatement := `
SELECT * FROM hosts
WHERE NOT deleted
AND id NOT IN (?)
WHERE id NOT in (?)
ORDER BY seen_time DESC
LIMIT 5
`

var in interface{}
{
// use -1 if there are no values to omit.
//Avoids empty args error for `sqlx.In`
// Avoids empty args error for `sqlx.In`
in = omit
if len(omit) == 0 {
in = -1
Expand Down Expand Up @@ -435,7 +429,6 @@ func (d *Datastore) SearchHosts(query string, omit ...uint) ([]*kolide.Host, err
MATCH (host_name, uuid) AGAINST (? IN BOOLEAN MODE)
OR MATCH (primary_ip, primary_mac) AGAINST (? IN BOOLEAN MODE)
)
AND NOT deleted
LIMIT 10
`
hosts := []*kolide.Host{}
Expand Down
39 changes: 11 additions & 28 deletions server/datastore/mysql/invites.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,15 @@ import (
"github.com/pkg/errors"
)

// NewInvite generates a new invitation
// NewInvite generates a new invitation.
func (d *Datastore) NewInvite(i *kolide.Invite) (*kolide.Invite, error) {
var (
deletedInvite kolide.Invite
sqlStmt string
)
err := d.db.Get(&deletedInvite, "SELECT * FROM invites WHERE email = ? AND deleted", i.Email)
switch err {
case nil:
sqlStmt = `
REPLACE INTO invites ( invited_by, email, admin, name, position, token, deleted, sso_enabled)
VALUES ( ?, ?, ?, ?, ?, ?, ?, ?)
`
case sql.ErrNoRows:
sqlStmt = `
INSERT INTO invites ( invited_by, email, admin, name, position, token, deleted, sso_enabled)
VALUES ( ?, ?, ?, ?, ?, ?, ?, ?)
`
default:
return nil, errors.Wrap(err, "check for existing invite")
}
sqlStmt := `
INSERT INTO invites ( invited_by, email, admin, name, position, token, sso_enabled)
VALUES ( ?, ?, ?, ?, ?, ?, ?)
`

deleted := false
result, err := d.db.Exec(sqlStmt, i.InvitedBy, i.Email, i.Admin,
i.Name, i.Position, i.Token, deleted, i.SSOEnabled)
i.Name, i.Position, i.Token, i.SSOEnabled)
if err != nil && isDuplicate(err) {
return nil, alreadyExists("Invite", 0)
} else if err != nil {
Expand All @@ -43,7 +27,6 @@ func (d *Datastore) NewInvite(i *kolide.Invite) (*kolide.Invite, error) {
i.ID = uint(id)

return i, nil

}

// ListInvites lists all invites in the Fleet database. Supply query options
Expand All @@ -52,7 +35,7 @@ func (d *Datastore) ListInvites(opt kolide.ListOptions) ([]*kolide.Invite, error

invites := []*kolide.Invite{}

query := appendListOptionsToSQL("SELECT * FROM invites WHERE NOT deleted", opt)
query := appendListOptionsToSQL("SELECT * FROM invites", opt)
err := d.db.Select(&invites, query)
if err == sql.ErrNoRows {
return nil, notFound("Invite")
Expand All @@ -65,7 +48,7 @@ func (d *Datastore) ListInvites(opt kolide.ListOptions) ([]*kolide.Invite, error
// Invite returns Invite identified by id.
func (d *Datastore) Invite(id uint) (*kolide.Invite, error) {
var invite kolide.Invite
err := d.db.Get(&invite, "SELECT * FROM invites WHERE id = ? AND NOT deleted", id)
err := d.db.Get(&invite, "SELECT * FROM invites WHERE id = ?", id)
if err == sql.ErrNoRows {
return nil, notFound("Invite").WithID(id)
} else if err != nil {
Expand All @@ -77,7 +60,7 @@ func (d *Datastore) Invite(id uint) (*kolide.Invite, error) {
// InviteByEmail finds an Invite with a particular email, if one exists.
func (d *Datastore) InviteByEmail(email string) (*kolide.Invite, error) {
var invite kolide.Invite
err := d.db.Get(&invite, "SELECT * FROM invites WHERE email = ? AND NOT deleted", email)
err := d.db.Get(&invite, "SELECT * FROM invites WHERE email = ?", email)
if err == sql.ErrNoRows {
return nil, notFound("Invite").
WithMessage(fmt.Sprintf("with email %s", email))
Expand All @@ -90,7 +73,7 @@ func (d *Datastore) InviteByEmail(email string) (*kolide.Invite, error) {
// InviteByToken finds an Invite with a particular token, if one exists.
func (d *Datastore) InviteByToken(token string) (*kolide.Invite, error) {
var invite kolide.Invite
err := d.db.Get(&invite, "SELECT * FROM invites WHERE token = ? AND NOT deleted", token)
err := d.db.Get(&invite, "SELECT * FROM invites WHERE token = ?", token)
if err == sql.ErrNoRows {
return nil, notFound("Invite").
WithMessage(fmt.Sprintf("with token %s", token))
Expand All @@ -105,7 +88,7 @@ func (d *Datastore) SaveInvite(i *kolide.Invite) error {
sql := `
UPDATE invites SET invited_by = ?, email = ?, admin = ?,
name = ?, position = ?, token = ?, sso_enabled = ?
WHERE id = ? AND NOT deleted
WHERE id = ?
`
results, err := d.db.Exec(sql, i.InvitedBy, i.Email,
i.Admin, i.Name, i.Position, i.Token, i.SSOEnabled, i.ID,
Expand Down
Loading