diff --git a/matchbox/storage/filestore.go b/matchbox/storage/filestore.go index 6e9e5ce0..3caa633f 100644 --- a/matchbox/storage/filestore.go +++ b/matchbox/storage/filestore.go @@ -56,6 +56,11 @@ func (s *fileStore) GroupGet(id string) (*storagepb.Group, error) { return group, err } +// GroupDelete deletes a machine Group by id. +func (s *fileStore) GroupDelete(id string) error { + return Dir(s.root).deleteFile(filepath.Join("groups", id+".json")) +} + // GroupList lists all machine Groups. func (s *fileStore) GroupList() ([]*storagepb.Group, error) { files, err := Dir(s.root).readDir("groups") @@ -101,6 +106,11 @@ func (s *fileStore) ProfileGet(id string) (*storagepb.Profile, error) { return profile, err } +// ProfileDelete deletes a profile by id. +func (s *fileStore) ProfileDelete(id string) error { + return Dir(s.root).deleteFile(filepath.Join("profiles", id+".json")) +} + // ProfileList lists all profiles. func (s *fileStore) ProfileList() ([]*storagepb.Profile, error) { files, err := Dir(s.root).readDir("profiles") diff --git a/matchbox/storage/filestore_test.go b/matchbox/storage/filestore_test.go index c68aefdb..b6aa8919 100644 --- a/matchbox/storage/filestore_test.go +++ b/matchbox/storage/filestore_test.go @@ -13,7 +13,7 @@ import ( fake "github.com/coreos/matchbox/matchbox/storage/testfakes" ) -func TestGroupPut(t *testing.T) { +func TestGroupPutAndDelete(t *testing.T) { dir, err := setup(&fake.FixedStore{}) assert.Nil(t, err) defer os.RemoveAll(dir) @@ -22,11 +22,20 @@ func TestGroupPut(t *testing.T) { // assert that: // - Group creation was successful // - Group can be retrieved by id + // - Group can be deleted by id err = store.GroupPut(fake.Group) assert.Nil(t, err) + group, err := store.GroupGet(fake.Group.Id) assert.Nil(t, err) assert.Equal(t, fake.Group, group) + + err = store.GroupDelete(fake.Group.Id) + assert.Nil(t, err) + _, err = store.GroupGet(fake.Group.Id) + if assert.Error(t, err) { + assert.IsType(t, err, &os.PathError{}) + } } func TestGroupGet(t *testing.T) { @@ -82,7 +91,7 @@ func TestGroupList(t *testing.T) { } } -func TestProfilePut(t *testing.T) { +func TestProfilePutAndDelete(t *testing.T) { dir, err := setup(&fake.FixedStore{}) assert.Nil(t, err) defer os.RemoveAll(dir) @@ -91,11 +100,20 @@ func TestProfilePut(t *testing.T) { // assert that: // - Profile creation was successful // - Profile can be retrieved by id + // - Profile can be deleted by id err = store.ProfilePut(fake.Profile) assert.Nil(t, err) + profile, err := store.ProfileGet(fake.Profile.Id) assert.Nil(t, err) assert.Equal(t, fake.Profile, profile) + + err = store.ProfileDelete(fake.Profile.Id) + assert.Nil(t, err) + _, err = store.ProfileGet(fake.Profile.Id) + if assert.Error(t, err) { + assert.IsType(t, err, &os.PathError{}) + } } func TestProfileGet(t *testing.T) { diff --git a/matchbox/storage/fs.go b/matchbox/storage/fs.go index b812447a..7df558e9 100644 --- a/matchbox/storage/fs.go +++ b/matchbox/storage/fs.go @@ -55,6 +55,16 @@ func (d Dir) writeFile(path string, data []byte) error { return ioutil.WriteFile(path, data, defaultFileMode) } +// deleteFile removes the file at the given path, restricted to a specific +// directory tree. +func (d Dir) deleteFile(path string) error { + path, err := d.sanitize(path) + if err != nil { + return err + } + return os.Remove(path) +} + // Borrowed directly from net/http Dir.Open and FileServer. func (d Dir) sanitize(name string) (string, error) { if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) || diff --git a/matchbox/storage/fs_test.go b/matchbox/storage/fs_test.go index aeedb436..576ccb43 100644 --- a/matchbox/storage/fs_test.go +++ b/matchbox/storage/fs_test.go @@ -25,6 +25,7 @@ func TestDir(t *testing.T) { assert.Nil(t, err) defer os.RemoveAll(tdir) + // create a Dir, restricted to the temp dir dir := Dir(tdir) // write files rooted in the dir for _, c := range cases { @@ -41,6 +42,17 @@ func TestDir(t *testing.T) { assert.Nil(t, err) assert.Equal(t, []byte(c.expected), b) } + // delete the files that were written + for _, c := range cases { + err := dir.deleteFile(c.path) + assert.Nil(t, err) + } + // ensure the expected files were removed + for _, c := range cases { + rpath := filepath.Join(tdir, c.expected) + _, err := os.Stat(rpath) + assert.True(t, os.IsNotExist(err), "expected path %s would not exist", rpath) + } } func TestSanitizePath(t *testing.T) { diff --git a/matchbox/storage/storage.go b/matchbox/storage/storage.go index bcbf79fa..1343a95a 100644 --- a/matchbox/storage/storage.go +++ b/matchbox/storage/storage.go @@ -18,6 +18,8 @@ type Store interface { GroupPut(group *storagepb.Group) error // GroupGet returns a machine Group by id. GroupGet(id string) (*storagepb.Group, error) + // GroupDelete deletes a machine Group by id. + GroupDelete(id string) error // GroupList lists all machine Groups. GroupList() ([]*storagepb.Group, error) @@ -25,6 +27,8 @@ type Store interface { ProfilePut(profile *storagepb.Profile) error // ProfileGet gets a profile by id. ProfileGet(id string) (*storagepb.Profile, error) + // ProfileDelete deletes a profile by id. + ProfileDelete(id string) error // ProfileList lists all profiles. ProfileList() ([]*storagepb.Profile, error) diff --git a/matchbox/storage/testfakes/broken_store.go b/matchbox/storage/testfakes/broken_store.go index 8755950f..32cf7073 100644 --- a/matchbox/storage/testfakes/broken_store.go +++ b/matchbox/storage/testfakes/broken_store.go @@ -23,6 +23,11 @@ func (s *BrokenStore) GroupGet(id string) (*storagepb.Group, error) { return nil, errIntentional } +// GroupDelete returns an error. +func (s *BrokenStore) GroupDelete(id string) error { + return errIntentional +} + // GroupList returns an error. func (s *BrokenStore) GroupList() (groups []*storagepb.Group, err error) { return groups, errIntentional @@ -38,6 +43,11 @@ func (s *BrokenStore) ProfileGet(id string) (*storagepb.Profile, error) { return nil, errIntentional } +// ProfileDelete returns an error. +func (s *BrokenStore) ProfileDelete(id string) error { + return errIntentional +} + // ProfileList returns an error. func (s *BrokenStore) ProfileList() (profiles []*storagepb.Profile, err error) { return profiles, errIntentional diff --git a/matchbox/storage/testfakes/empty_store.go b/matchbox/storage/testfakes/empty_store.go index d5bc0d60..64daaba1 100644 --- a/matchbox/storage/testfakes/empty_store.go +++ b/matchbox/storage/testfakes/empty_store.go @@ -19,6 +19,11 @@ func (s *EmptyStore) GroupGet(id string) (*storagepb.Group, error) { return nil, fmt.Errorf("Group not found") } +// GroupDelete returns a nil error (successful deletion). +func (s *EmptyStore) GroupDelete(id string) error { + return nil +} + // GroupList returns an empty list of groups. func (s *EmptyStore) GroupList() (groups []*storagepb.Group, err error) { return groups, nil @@ -34,6 +39,11 @@ func (s *EmptyStore) ProfileGet(id string) (*storagepb.Profile, error) { return nil, fmt.Errorf("Profile not found") } +// ProfileDelete returns a nil error (successful deletion). +func (s *EmptyStore) ProfileDelete(id string) error { + return nil +} + // ProfileList returns an empty list of profiles. func (s *EmptyStore) ProfileList() (profiles []*storagepb.Profile, err error) { return profiles, nil diff --git a/matchbox/storage/testfakes/fixed_store.go b/matchbox/storage/testfakes/fixed_store.go index cf289da0..49800365 100644 --- a/matchbox/storage/testfakes/fixed_store.go +++ b/matchbox/storage/testfakes/fixed_store.go @@ -40,6 +40,12 @@ func (s *FixedStore) GroupGet(id string) (*storagepb.Group, error) { return nil, fmt.Errorf("Group not found") } +// GroupDelete deletes the Group from the Groups map with the given id. +func (s *FixedStore) GroupDelete(id string) error { + delete(s.Groups, id) + return nil +} + // GroupList returns the groups in the Groups map. func (s *FixedStore) GroupList() ([]*storagepb.Group, error) { groups := make([]*storagepb.Group, len(s.Groups)) @@ -65,6 +71,12 @@ func (s *FixedStore) ProfileGet(id string) (*storagepb.Profile, error) { return nil, fmt.Errorf("Profile not found") } +// ProfileDelete deletes the Profile from the Profiles map with the given id. +func (s *FixedStore) ProfileDelete(id string) error { + delete(s.Profiles, id) + return nil +} + // ProfileList returns the profiles in the Profiles map. func (s *FixedStore) ProfileList() ([]*storagepb.Profile, error) { profiles := make([]*storagepb.Profile, len(s.Profiles))