Additional comments

Fix methods which returned a pointer to a slice which is pointless.
Moved statement building functions in applications package to statement.go.
Fixed acknowledge handler if device UDID does not match any enrolled device (happens a lot in test cases).
Preliminary re-implementation of ackInstalledApplicationList
Added tests for the connect service.

applications/application.go

@@ -10,9 +10,10 @@ type Application struct {
 	Name         string         `json:"name,omitempty" db:"name"`
 	BundleSize   sql.NullInt64  `plist:",omitempty" json:"bundle_size,omitempty" db:"bundle_size"`
 
-	// The size of the app's document, library, and other folders, in bytes.
+	// The size of the app's document, library, and other folders, in bytes. Only applies to iOS
 	DynamicSize sql.NullInt64 `plist:",omitempty" json:"dynamic_size,omitempty" db:"dynamic_size"`
 
+	// iOS only.
 	IsValidated sql.NullBool `plist:",omitempty" json:"is_validated,omitempty" db:"is_validated"`
 }
 

applications/datastore.go

@@ -6,7 +6,6 @@ import (
 	"github.com/jmoiron/sqlx"
 	_ "github.com/lib/pq" // postgres driver
 	"github.com/pkg/errors"
-	"strings"
 	"time"
 )
 
@@ -14,7 +13,7 @@ import (
 type Datastore interface {
 	New(a *Application) (string, error)
 	Applications(params ...interface{}) ([]Application, error)
-	GetApplicationsByDeviceUUID(deviceUUID string) (*[]Application, error)
+	GetApplicationsByDeviceUUID(deviceUUID string) ([]Application, error)
 	SaveApplicationByDeviceUUID(deviceUUID string, app *Application) error
 }
 
@@ -53,31 +52,6 @@ func NewDB(driver, conn string, logger kitlog.Logger) (Datastore, error) {
 	}
 }
 
-// UUID is a filter that can be added as a parameter to narrow down the list of returned results
-type UUID struct {
-	UUID string
-}
-
-func (p UUID) where() string {
-	return fmt.Sprintf("application_uuid = '%s'", p.UUID)
-}
-
-type Name struct {
-	Name string
-}
-
-func (p Name) where() string {
-	return fmt.Sprintf("name = '%s'", p.Name)
-}
-
-type Version struct {
-	Version string
-}
-
-func (p Version) where() string {
-	return fmt.Sprintf("version = '%s'", p.Version)
-}
-
 // This function inserts a new application into the applications table.
 // Applications are uniquely identifier by both their name and their long form version because some do not have
 // identifiers, and some do not have short versions.
@@ -131,7 +105,7 @@ func (store pgStore) Applications(params ...interface{}) ([]Application, error)
 }
 
 // Retrieve only applications which are installed on the given device.
-func (store pgStore) GetApplicationsByDeviceUUID(deviceUUID string) (*[]Application, error) {
+func (store pgStore) GetApplicationsByDeviceUUID(deviceUUID string) ([]Application, error) {
 	var apps []Application
 	query := `SELECT * FROM applications
 	RIGHT JOIN devices_applications ON applications.application_uuid = devices_applications.application_uuid
@@ -143,7 +117,7 @@ func (store pgStore) GetApplicationsByDeviceUUID(deviceUUID string) (*[]Applicat
 		return nil, err
 	}
 
-	return &apps, nil
+	return apps, nil
 }
 
 // Associate the given applications with the given device uuid by inserting into `device_applications`.
@@ -155,88 +129,3 @@ func (store pgStore) SaveApplicationByDeviceUUID(deviceUUID string, app *Applica
 	_, err := store.Exec(stmt, deviceUUID, app.UUID)
 	return err
 }
-
-// whereer is for building args passed into a method which finds resources
-type whereer interface {
-	where() string
-}
-
-// add WHERE clause from params
-func addWhereFilters(stmt string, separator string, params ...interface{}) string {
-	var where []string
-	for _, param := range params {
-		if f, ok := param.(whereer); ok {
-			where = append(where, f.where())
-		}
-	}
-
-	if len(where) != 0 {
-		whereFilter := strings.Join(where, " "+separator+" ")
-		stmt = fmt.Sprintf("%s WHERE %s", stmt, whereFilter)
-	}
-	return stmt
-}
-
-// boolean operators are applied to where conditions which are part of a whereClauseGroup
-type booleanOperator string
-
-const (
-	OR  = "OR"
-	AND = "AND"
-)
-
-type whereClauseGroup struct {
-	Operator booleanOperator
-	Clauses  []whereClause
-}
-
-// Get a string representing the where clause
-// Second return value is an array of arguments to give to db.Exec etc.
-func (cg whereClauseGroup) String() (string, []string) {
-	var clauses []string
-	var values []string = make([]string, len(cg.Clauses))
-
-	for i, c := range cg.Clauses {
-		c.Placeholder = fmt.Sprintf("$%d", i)
-		clauses = append(clauses, c.String())
-		values = append(values, c.Value)
-	}
-
-	return strings.Join(clauses, string(cg.Operator)), values
-}
-
-// Struct representation of a where clause. Does not deal with field name escaping or any inference of the value.
-// I.E Do your own quoting.
-type whereClause struct {
-	Operator    string
-	Field       string
-	Value       string
-	Placeholder string
-}
-
-func (c whereClause) String() string {
-	return fmt.Sprintf(`%s %s %s`, c.Field, c.Operator, c.Value)
-}
-
-func Where(field string, operator string, value string) whereClause {
-	return whereClause{
-		Operator:    operator,
-		Field:       field,
-		Value:       value,
-		Placeholder: "$1",
-	}
-}
-
-func WhereAnd(clauses ...whereClause) whereClauseGroup {
-	return whereClauseGroup{
-		Operator: "AND",
-		Clauses:  clauses,
-	}
-}
-
-func WhereOr(clauses ...whereClause) whereClauseGroup {
-	return whereClauseGroup{
-		Operator: "OR",
-		Clauses:  clauses,
-	}
-}

applications/statement.go

@@ -0,0 +1,116 @@
+package applications
+
+import (
+	"fmt"
+	"strings"
+)
+
+// UUID is a filter that can be added as a parameter to narrow down the list of returned results
+type UUID struct {
+	UUID string
+}
+
+func (p UUID) where() string {
+	return fmt.Sprintf("application_uuid = '%s'", p.UUID)
+}
+
+type Name struct {
+	Name string
+}
+
+func (p Name) where() string {
+	return fmt.Sprintf("name = '%s'", p.Name)
+}
+
+type Version struct {
+	Version string
+}
+
+func (p Version) where() string {
+	return fmt.Sprintf("version = '%s'", p.Version)
+}
+
+// whereer is for building args passed into a method which finds resources
+type whereer interface {
+	where() string
+}
+
+// add WHERE clause from params
+func addWhereFilters(stmt string, separator string, params ...interface{}) string {
+	var where []string
+	for _, param := range params {
+		if f, ok := param.(whereer); ok {
+			where = append(where, f.where())
+		}
+	}
+
+	if len(where) != 0 {
+		whereFilter := strings.Join(where, " "+separator+" ")
+		stmt = fmt.Sprintf("%s WHERE %s", stmt, whereFilter)
+	}
+	return stmt
+}
+
+// boolean operators are applied to where conditions which are part of a whereClauseGroup
+type booleanOperator string
+
+const (
+	OR  = "OR"
+	AND = "AND"
+)
+
+type whereClauseGroup struct {
+	Operator booleanOperator
+	Clauses  []whereClause
+}
+
+// Get a string representing the where clause
+// Second return value is an array of arguments to give to db.Exec etc.
+func (cg whereClauseGroup) String() (string, []string) {
+	var clauses []string
+	var values []string = make([]string, len(cg.Clauses))
+
+	for i, c := range cg.Clauses {
+		c.Placeholder = fmt.Sprintf("$%d", i)
+		clauses = append(clauses, c.String())
+		values = append(values, c.Value)
+	}
+
+	return strings.Join(clauses, string(cg.Operator)), values
+}
+
+// Struct representation of a where clause. Does not deal with field name escaping or any inference of the value.
+// I.E Do your own quoting.
+type whereClause struct {
+	Operator    string
+	Field       string
+	Value       string
+	Placeholder string
+}
+
+func (c whereClause) String() string {
+	return fmt.Sprintf(`%s %s %s`, c.Field, c.Operator, c.Value)
+}
+
+func Where(field string, operator string, value string) whereClause {
+	return whereClause{
+		Operator:    operator,
+		Field:       field,
+		Value:       value,
+		Placeholder: "$1",
+	}
+}
+
+func WhereAnd(clauses ...whereClause) whereClauseGroup {
+	return whereClauseGroup{
+		Operator: "AND",
+		Clauses:  clauses,
+	}
+}
+
+func WhereOr(clauses ...whereClause) whereClauseGroup {
+	return whereClauseGroup{
+		Operator: "OR",
+		Clauses:  clauses,
+	}
+}

connect/service.go

@@ -1,10 +1,11 @@
 package connect
 
 import (
+	"database/sql"
 	"encoding/json"
 	"fmt"
 	"github.com/micromdm/mdm"
-	"github.com/micromdm/micromdm/applications"
+	apps "github.com/micromdm/micromdm/applications"
 	"github.com/micromdm/micromdm/command"
 	"github.com/micromdm/micromdm/device"
 	"github.com/pkg/errors"
@@ -20,7 +21,7 @@ type Service interface {
 }
 
 // NewService creates a mdm service
-func NewService(devices device.Datastore, apps applications.Datastore, cs command.Service) Service {
+func NewService(devices device.Datastore, apps apps.Datastore, cs command.Service) Service {
 	return &service{
 		commands: cs,
 		devices:  devices,
@@ -30,7 +31,7 @@ func NewService(devices device.Datastore, apps applications.Datastore, cs comman
 
 type service struct {
 	devices  device.Datastore
-	apps     applications.Datastore
+	apps     apps.Datastore
 	commands command.Service
 }
 
@@ -98,11 +99,15 @@ func (svc service) ackQueryResponses(req mdm.Response) error {
 	)
 
 	if err != nil {
-		return err
+		return errors.Wrap(err, "ackQueryResponses fetching device")
+	}
+
+	if len(devices) == 0 {
+		return errors.New("no enrolled device matches the one responding")
 	}
 
 	if len(devices) > 1 {
-		return errors.New("expected a single query result for device, got more than one.")
+		return fmt.Errorf("expected a single device for udid: %s, serial number: %s, but got more than one.", req.UDID, req.QueryResponses.SerialNumber)
 	}
 
 	existing := devices[0]
@@ -142,48 +147,52 @@ func (svc service) ackInstalledApplicationList(req mdm.Response) error {
 		return err
 	}
 
-	// Any installed applications that are already represented in the applications datastore should be skipped.
-	var updated []applications.Application = make([]applications.Application, len(req.InstalledApplicationList))
+	var removed []apps.Application = make([]apps.Application, len(req.InstalledApplicationList))
+	var deviceAppsRemaining []apps.Application = make([]apps.Application, len(req.InstalledApplicationList))
+
+	// Check to see whether installed applications exist in the latest response
+	// If they do not, they are added to the removed slice.
+	// TODO: This is a pretty horrible algorithm and I should re-design it at some point. m.
+removedouter:
+	for _, deviceApp := range deviceApps {
+		for _, app := range req.InstalledApplicationList {
+			if deviceApp.Version.Valid && deviceApp.Version.String == app.Version && deviceApp.Name == app.Name {
+				deviceAppsRemaining = append(deviceAppsRemaining, deviceApp)
+				continue removedouter
+			}
+		}
+
+		removed = append(removed, deviceApp)
+	}
+
+	// Any installed applications that are already represented in the `applications` table AND
+	// allocated to the device in `devices_applications` should be skipped.
+	var updated []apps.Application = make([]apps.Application, len(req.InstalledApplicationList))
 
 skip:
 	for _, ackApp := range req.InstalledApplicationList {
-		for _, app := range *deviceApps {
-			if app.Name == ackApp.Name && app.Version == ackApp.Version {
+		for _, app := range deviceAppsRemaining {
+			if app.Name == ackApp.Name && app.Version.Valid && app.Version.String == ackApp.Version {
 				continue skip
 			}
 		}
 
-		updated = append(updated, ackApp)
+		identifier := sql.NullString{ackApp.Identifier, ackApp.Identifier != ""}
+
+		appUpdated := apps.Application{
+			Name:       ackApp.Name,
+			Identifier: identifier,
+			//ShortVersion: sql.NullString{}.Scan(ackApp.ShortVersion),
+			//Version: sql.NullString{}.Scan(ackApp.Version),
+			//BundleSize: sql.NullInt64{}.Scan(ackApp.BundleSize),
+			//DynamicSize: sql.NullInt64{}.Scan(ackApp.DynamicSize),
+			//IsValidated: sql.NullBool{}.Scan(ackApp.IsValidated),
+		}
+		updated = append(updated, appUpdated)
 	}
 
-	if len(updated) == 0 {
-		return nil
-	}
-
-	// Determine applications which we have no record of at all, then insert them (find or create).
-	for _, newApp := range updated {
-		existing, err := svc.apps.Applications(applications.Name{newApp.Name}, applications.Version{newApp.Version})
-		if err != nil {
-			return err
-		}
-
-		switch {
-		case len(existing) > 1:
-			return fmt.Errorf("expected a single application match for application name: %s, got %d results", newApp.Name, len(existing))
-		case len(existing) == 0: // No record exists and therefore both the application row and device association must be created.
-			appUuid, err := svc.apps.New(newApp)
-			if err != nil {
-				return err
-			}
-
-			newApp.UUID = appUuid
-		}
-
-		// For both len(existing) == 0 and len(existing) == 1, the row must be inserted for devices_applications.
-		if err := svc.apps.SaveApplicationByDeviceUUID(device.UUID, newApp.UUID); err != nil {
-			return err
-		}
-	}
+	fmt.Printf("removed %#v\n", removed)
+	fmt.Printf("updated %#v\n", updated)
 
 	return nil
 }

connect/service_test.go

@@ -0,0 +1,88 @@
+package connect
+
+import (
+	"github.com/micromdm/mdm"
+	"github.com/micromdm/micromdm/applications"
+	"github.com/micromdm/micromdm/device"
+	"testing"
+	"time"
+)
+
+type MockDevices struct{}
+
+func (md MockDevices) New(src string, d *device.Device) (string, error) {
+	return "", nil
+}
+func (md MockDevices) GetDeviceByUDID(udid string, fields ...string) (*device.Device, error) {
+	return &device.Device{}, nil
+}
+func (md MockDevices) GetDeviceByUUID(uuid string, fields ...string) (*device.Device, error) {
+	return &device.Device{}, nil
+}
+func (md MockDevices) Devices(params ...interface{}) ([]device.Device, error) {
+	return []device.Device{}, nil
+}
+func (md MockDevices) Save(msg string, dev *device.Device) error {
+	return nil
+}
+
+type MockApps struct{}
+
+func (ma MockApps) New(a *applications.Application) (string, error) {
+	return "", nil
+}
+func (ma MockApps) Applications(params ...interface{}) ([]applications.Application, error) {
+	return []applications.Application{}, nil
+}
+func (ma MockApps) GetApplicationsByDeviceUUID(deviceUUID string) ([]applications.Application, error) {
+	return []applications.Application{}, nil
+}
+func (ma MockApps) SaveApplicationByDeviceUUID(deviceUUID string, app *applications.Application) error {
+	return nil
+}
+
+type MockCmd struct{}
+
+func (mc MockCmd) NewCommand(*mdm.CommandRequest) (*mdm.Payload, error) {
+	return &mdm.Payload{}, nil
+}
+func (mc MockCmd) NextCommand(udid string) ([]byte, int, error) {
+	return []byte{}, 0, nil
+}
+func (mc MockCmd) DeleteCommand(deviceUDID, commandUUID string) (int, error) {
+	return 0, nil
+}
+
+type MockContext struct{}
+
+func (mc MockContext) Done() <-chan struct{} {
+	ch := make(chan struct{})
+
+	return ch
+}
+func (mc MockContext) Err() error {
+	return nil
+}
+func (mc MockContext) Deadline() (deadline time.Time, ok bool) {
+	return time.Now(), true
+}
+func (mc MockContext) Value(key interface{}) interface{} {
+	return nil
+}
+
+func TestAckQueryResponses(t *testing.T) {
+	response := mdm.Response{
+		UDID:           "00000000-1111-2222-3333-444455556666",
+		Status:         "Acknowledged",
+		CommandUUID:    "10000000-1111-2222-3333-444455556666",
+		RequestType:    "DeviceInformation",
+		QueryResponses: mdm.QueryResponses{},
+	}
+
+	mockDevices := MockDevices{}
+	mockApps := MockApps{}
+	mockCmd := MockCmd{}
+
+	svc := NewService(mockDevices, mockApps, mockCmd)
+	svc.Acknowledge(MockContext{}, response)
+}

management/endpoint_installedapps.go

@@ -22,6 +22,6 @@ func makeInstalledAppsEndpoint(svc Service) endpoint.Endpoint {
 		if err != nil {
 			return installedAppsResponse{Err: err}, nil
 		}
-		return installedAppsResponse{applications: *apps}, nil
+		return installedAppsResponse{applications: apps}, nil
 	}
 }

management/service.go

@@ -29,7 +29,7 @@ type Service interface {
 	Device(uuid string) (*device.Device, error)
 
 	// Installed Applications
-	InstalledApps(deviceUUID string) (*[]applications.Application, error)
+	InstalledApps(deviceUUID string) ([]applications.Application, error)
 
 	// AssignWorkflow assigns a workflow to a device
 	AssignWorkflow(deviceUUID, workflowUUID string) error
@@ -165,7 +165,7 @@ func (svc service) AssignWorkflow(deviceUUID, workflowUUID string) error {
 	return svc.devices.Save("assignWorkflow", dev)
 }
 
-func (svc service) InstalledApps(deviceUUID string) (*[]applications.Application, error) {
+func (svc service) InstalledApps(deviceUUID string) ([]applications.Application, error) {
 	apps, err := svc.applications.GetApplicationsByDeviceUUID(deviceUUID)
 	if err != nil {
 		return nil, errors.Wrap(err, "management: installed apps")