consolidate delete operations in mysql store (#746)

Adds a helper method which soft deletes entities from the database.

server/datastore/datastore_hosts_test.go

@@ -108,7 +108,7 @@ func testSaveHosts(t *testing.T, ds kolide.Datastore) {
 	require.NotNil(t, host)
 	assert.Nil(t, host.PrimaryNetworkInterfaceID)
 
-	err = ds.DeleteHost(host)
+	err = ds.DeleteHost(host.ID)
 	assert.Nil(t, err)
 
 	host, err = ds.Host(host.ID)
@@ -126,7 +126,7 @@ func testDeleteHost(t *testing.T, ds kolide.Datastore) {
 	require.Nil(t, err)
 	require.NotNil(t, host)
 
-	err = ds.DeleteHost(host)
+	err = ds.DeleteHost(host.ID)
 	assert.Nil(t, err)
 
 	host, err = ds.Host(host.ID)
@@ -183,7 +183,7 @@ func testListHost(t *testing.T, ds kolide.Datastore) {
 	assert.Equal(t, "en1", hosts2[1].NetworkInterfaces[1].Interface)
 	assert.Equal(t, "en2", hosts2[3].NetworkInterfaces[0].Interface)
 
-	err = ds.DeleteHost(hosts[0])
+	err = ds.DeleteHost(hosts[0].ID)
 	require.Nil(t, err)
 	hosts2, err = ds.ListHosts(kolide.ListOptions{})
 	require.Nil(t, err)

server/datastore/datastore_invites_test.go

@@ -96,7 +96,7 @@ func testDeleteInvite(t *testing.T, ds kolide.Datastore) {
 	assert.Nil(t, err)
 	assert.NotNil(t, invite)
 
-	err = ds.DeleteInvite(invite)
+	err = ds.DeleteInvite(invite.ID)
 	assert.Nil(t, err)
 
 	invite, err = ds.InviteByEmail("user0@foo.com")

server/datastore/datastore_queries_test.go

@@ -24,7 +24,7 @@ func testDeleteQuery(t *testing.T, ds kolide.Datastore) {
 	require.NotNil(t, query)
 	assert.NotEqual(t, query.ID, 0)
 
-	err = ds.DeleteQuery(query)
+	err = ds.DeleteQuery(query.ID)
 	require.Nil(t, err)
 
 	assert.NotEqual(t, query.ID, 0)

server/datastore/inmem/hosts.go

@@ -45,15 +45,15 @@ func (d *Datastore) SaveHost(host *kolide.Host) error {
 	return nil
 }
 
-func (d *Datastore) DeleteHost(host *kolide.Host) error {
+func (d *Datastore) DeleteHost(hid uint) error {
 	d.mtx.Lock()
 	defer d.mtx.Unlock()
 
-	if _, ok := d.hosts[host.ID]; !ok {
-		return notFound("Host").WithID(host.ID)
+	if _, ok := d.hosts[hid]; !ok {
+		return notFound("Host").WithID(hid)
 	}
 
-	delete(d.hosts, host.ID)
+	delete(d.hosts, hid)
 	return nil
 }
 

server/datastore/inmem/invites.go

@@ -121,13 +121,13 @@ func (d *Datastore) SaveInvite(invite *kolide.Invite) error {
 }
 
 // DeleteInvite deletes an invitation.
-func (d *Datastore) DeleteInvite(invite *kolide.Invite) error {
+func (d *Datastore) DeleteInvite(id uint) error {
 	d.mtx.Lock()
 	defer d.mtx.Unlock()
 
-	if _, ok := d.invites[invite.ID]; !ok {
-		return notFound("Invite").WithID(invite.ID)
+	if _, ok := d.invites[id]; !ok {
+		return notFound("Invite").WithID(id)
 	}
-	delete(d.invites, invite.ID)
+	delete(d.invites, id)
 	return nil
 }

server/datastore/inmem/queries.go

@@ -37,15 +37,15 @@ func (d *Datastore) SaveQuery(query *kolide.Query) error {
 	return nil
 }
 
-func (d *Datastore) DeleteQuery(query *kolide.Query) error {
+func (d *Datastore) DeleteQuery(qid uint) error {
 	d.mtx.Lock()
 	defer d.mtx.Unlock()
 
-	if _, ok := d.queries[query.ID]; !ok {
-		return notFound("Query").WithID(query.ID)
+	if _, ok := d.queries[qid]; !ok {
+		return notFound("Query").WithID(qid)
 	}
 
-	delete(d.queries, query.ID)
+	delete(d.queries, qid)
 	return nil
 }
 

server/datastore/mysql/delete.go

@@ -0,0 +1,24 @@
+package mysql
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+)
+
+func (d *Datastore) deleteEntity(dbTable string, id uint) error {
+	deleteStmt := fmt.Sprintf(
+		`
+		UPDATE %s SET deleted_at = ?, deleted = TRUE
+			WHERE id = ?
+	`, dbTable)
+	result, err := d.db.Exec(deleteStmt, d.clock.Now(), id)
+	if err != nil {
+		return errors.Wrap(err, fmt.Sprintf("delete %s", dbTable))
+	}
+	rows, _ := result.RowsAffected()
+	if rows != 1 {
+		return notFound(dbTable).WithID(id)
+	}
+	return nil
+}

server/datastore/mysql/hosts.go

@@ -239,19 +239,8 @@ func (d *Datastore) SaveHost(host *kolide.Host) error {
 	return nil
 }
 
-func (d *Datastore) DeleteHost(host *kolide.Host) error {
-	sqlStatement := `
-		UPDATE hosts SET
-			deleted = TRUE,
-			deleted_at = ?
-		WHERE id = ?
-	`
-	_, err := d.db.Exec(sqlStatement, d.clock.Now(), host.ID)
-	if err != nil {
-		return errors.DatabaseError(err)
-	}
-
-	return nil
+func (d *Datastore) DeleteHost(hid uint) error {
+	return d.deleteEntity("hosts", hid)
 }
 
 // TODO needs test

server/datastore/mysql/invites.go

@@ -101,15 +101,6 @@ func (d *Datastore) SaveInvite(i *kolide.Invite) error {
 
 }
 
-func (d *Datastore) DeleteInvite(i *kolide.Invite) error {
-	i.MarkDeleted(d.clock.Now())
-	sql := `
-	UPDATE invites SET deleted_at = ?, deleted = ?
-		WHERE id = ?
-	`
-	_, err := d.db.Exec(sql, i.DeletedAt, true, i.ID)
-	if err != nil {
-		return errors.Wrap(err, "delete invite")
-	}
-	return nil
+func (d *Datastore) DeleteInvite(id uint) error {
+	return d.deleteEntity("invites", id)
 }

server/datastore/mysql/labels.go

@@ -34,13 +34,7 @@ func (d *Datastore) NewLabel(label *kolide.Label) (*kolide.Label, error) {
 
 // DeleteLabel soft deletes a kolide.Label
 func (d *Datastore) DeleteLabel(lid uint) error {
-	sql := `
-		UPDATE labels
-		SET deleted_at = ?, deleted = TRUE
-		WHERE id = ?
-	`
-	_, err := d.db.Exec(sql, d.clock.Now(), lid)
-	return errors.DatabaseError(err)
+	return d.deleteEntity("labels", lid)
 }
 
 // Label returns a kolide.Label identified by  lid if one exists

server/datastore/mysql/packs.go

@@ -42,17 +42,7 @@ func (d *Datastore) SavePack(pack *kolide.Pack) error {
 
 // DeletePack soft deletes a kolide.Pack so that it won't show up in results
 func (d *Datastore) DeletePack(pid uint) error {
-	sql := `
-		UPDATE packs
-			SET deleted_at = ?, deleted = TRUE
-			WHERE id = ?
-	`
-	_, err := d.db.Exec(sql, d.clock.Now(), pid)
-	if err != nil {
-		return errors.DatabaseError(err)
-	}
-
-	return nil
+	return d.deleteEntity("packs", pid)
 }
 
 // Pack fetch kolide.Pack with matching ID

server/datastore/mysql/queries.go

@@ -44,19 +44,8 @@ func (d *Datastore) SaveQuery(q *kolide.Query) error {
 }
 
 // DeleteQuery soft deletes Query identified by Query.ID
-func (d *Datastore) DeleteQuery(query *kolide.Query) error {
-	query.MarkDeleted(d.clock.Now())
-	sql := `
-		UPDATE queries
-			SET deleted_at = ?, deleted = true
-			WHERE id = ?
-	`
-	_, err := d.db.Exec(sql, query.DeletedAt, query.ID)
-	if err != nil {
-		return errors.DatabaseError(err)
-	}
-
-	return nil
+func (d *Datastore) DeleteQuery(qid uint) error {
+	return d.deleteEntity("queries", qid)
 }
 
 // DeleteQueries (soft) deletes the existing query objects with the provided

server/datastore/mysql/scheduled_queries.go

@@ -66,17 +66,7 @@ func (d *Datastore) SaveScheduledQuery(sq *kolide.ScheduledQuery) (*kolide.Sched
 }
 
 func (d *Datastore) DeleteScheduledQuery(id uint) error {
-	query := `
-		UPDATE scheduled_queries
-			SET deleted_at = ?, deleted = ?
-			WHERE id = ?
-	`
-	_, err := d.db.Exec(query, d.clock.Now(), true, id)
-	if err != nil {
-		return errors.Wrap(err, "deleting a scheduled query")
-	}
-
-	return nil
+	return d.deleteEntity("scheduled_queries", id)
 }
 
 func (d *Datastore) ScheduledQuery(id uint) (*kolide.ScheduledQuery, error) {

server/kolide/hosts.go

@@ -11,7 +11,7 @@ import (
 type HostStore interface {
 	NewHost(host *Host) (*Host, error)
 	SaveHost(host *Host) error
-	DeleteHost(host *Host) error
+	DeleteHost(hid uint) error
 	Host(id uint) (*Host, error)
 	ListHosts(opt ListOptions) ([]*Host, error)
 	EnrollHost(osqueryHostId string, nodeKeySize int) (*Host, error)

server/kolide/invites.go

@@ -29,7 +29,7 @@ type InviteStore interface {
 	SaveInvite(i *Invite) error
 
 	// DeleteInvite deletes an invitation.
-	DeleteInvite(i *Invite) error
+	DeleteInvite(id uint) error
 }
 
 // InviteService contains methods for a service which deals with

server/kolide/queries.go

@@ -13,7 +13,7 @@ type QueryStore interface {
 	// SaveQuery saves changes to an existing query object.
 	SaveQuery(query *Query) error
 	// DeleteQuery (soft) deletes an existing query object.
-	DeleteQuery(query *Query) error
+	DeleteQuery(qid uint) error
 	// DeleteQueries (soft) deletes the existing query objects with the
 	// provided IDs. The number of deleted queries is returned along with
 	// any error.

server/kolide/traits.go

@@ -15,13 +15,6 @@ type DeleteFields struct {
 	Deleted   bool       `json:"deleted"`
 }
 
-// MarkDeleted indicates a record is deleted. It won't actually be removed from
-// the database, but won't be returned in result sets.
-func (d *DeleteFields) MarkDeleted(deleted time.Time) {
-	d.DeletedAt = &deleted
-	d.Deleted = true
-}
-
 // UpdateTimestamp contains a timestamp that is set whenever an entity is changed
 type UpdateTimestamp struct {
 	UpdatedAt time.Time `json:"updated_at" db:"updated_at"`

server/service/service_hosts.go

@@ -25,15 +25,5 @@ func (svc service) HostStatus(ctx context.Context, host kolide.Host) string {
 }
 
 func (svc service) DeleteHost(ctx context.Context, id uint) error {
-	host, err := svc.ds.Host(id)
-	if err != nil {
-		return err
-	}
-
-	err = svc.ds.DeleteHost(host)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return svc.ds.DeleteHost(id)
 }

server/service/service_invites.go

@@ -96,9 +96,5 @@ func (svc service) VerifyInvite(ctx context.Context, token string) (*kolide.Invi
 }
 
 func (svc service) DeleteInvite(ctx context.Context, id uint) error {
-	invite, err := svc.ds.Invite(id)
-	if err != nil {
-		return err
-	}
-	return svc.ds.DeleteInvite(invite)
+	return svc.ds.DeleteInvite(id)
 }

server/service/service_queries.go

@@ -68,17 +68,7 @@ func (svc service) ModifyQuery(ctx context.Context, id uint, p kolide.QueryPaylo
 }
 
 func (svc service) DeleteQuery(ctx context.Context, id uint) error {
-	query, err := svc.ds.Query(id)
-	if err != nil {
-		return err
-	}
-
-	err = svc.ds.DeleteQuery(query)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return svc.ds.DeleteQuery(id)
 }
 
 func (svc service) DeleteQueries(ctx context.Context, ids []uint) (uint, error) {

server/service/service_users.go

@@ -25,7 +25,8 @@ func (svc service) NewUser(ctx context.Context, p kolide.UserPayload) (*kolide.U
 	if err != nil {
 		return nil, err
 	}
-	err = svc.ds.DeleteInvite(invite)
+
+	err = svc.ds.DeleteInvite(invite.ID)
 	if err != nil {
 		return nil, err
 	}