Skip to content

Commit

Permalink
prevent from removing function with subscriptions. Closes #208 (#370)
Browse files Browse the repository at this point in the history
  • Loading branch information
mthenw committed Feb 16, 2018
1 parent 3312967 commit 6bf544e
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 7 deletions.
7 changes: 7 additions & 0 deletions function/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,10 @@ type ErrFunctionError struct {
func (e ErrFunctionError) Error() string {
return fmt.Sprintf("Function call failed because of runtime error. Error: %q", e.Original)
}

// ErrFunctionHasSubscriptionsError occurs when function with subscription is being deleted.
type ErrFunctionHasSubscriptionsError struct{}

func (e ErrFunctionHasSubscriptionsError) Error() string {
return fmt.Sprintf("Function cannot be deleted because it's subscribed to a least one event.")
}
2 changes: 2 additions & 0 deletions httpapi/httpapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ func (h HTTPAPI) deleteFunction(w http.ResponseWriter, r *http.Request, params h
if err != nil {
if _, ok := err.(*function.ErrFunctionNotFound); ok {
w.WriteHeader(http.StatusNotFound)
} else if _, ok := err.(*function.ErrFunctionHasSubscriptionsError); ok {
w.WriteHeader(http.StatusBadRequest)
} else {
w.WriteHeader(http.StatusInternalServerError)
}
Expand Down
17 changes: 17 additions & 0 deletions httpapi/httpapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,23 @@ func TestRegisterFunction_InternalError(t *testing.T) {
assert.Equal(t, `processing error`, httpresp.Errors[0].Message)
}

func TestDeleteFunction_BadRequest(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
router, functions, _ := setup(ctrl)

functions.EXPECT().DeleteFunction("default", function.ID("func1")).Return(&function.ErrFunctionHasSubscriptionsError{})

resp := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodDelete, "/v1/spaces/default/functions/func1", nil)
router.ServeHTTP(resp, req)

httpresp := &httpapi.Response{}
json.Unmarshal(resp.Body.Bytes(), httpresp)
assert.Equal(t, http.StatusBadRequest, resp.Code)
assert.Equal(t, "Function cannot be deleted because it's subscribed to a least one event.", httpresp.Errors[0].Message)
}

func setup(ctrl *gomock.Controller) (
*httprouter.Router,
*mock.MockFunctionService,
Expand Down
12 changes: 11 additions & 1 deletion libkv/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,17 @@ func (service Service) GetFunctions(space string) (function.Functions, error) {

// DeleteFunction deletes function from the registry.
func (service Service) DeleteFunction(space string, id function.ID) error {
err := service.FunctionStore.Delete(FunctionKey{space, id}.String())
subs, err := service.GetSubscriptions(space)
if err != nil {
return err
}
for _, sub := range subs {
if id == sub.FunctionID {
return &function.ErrFunctionHasSubscriptionsError{}
}
}

err = service.FunctionStore.Delete(FunctionKey{space, id}.String())
if err != nil {
return &function.ErrFunctionNotFound{ID: id}
}
Expand Down
34 changes: 28 additions & 6 deletions libkv/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,12 @@ func TestDeleteFunction(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

db := mock.NewMockStore(ctrl)
db.EXPECT().Delete("default/testid").Return(nil)
service := &Service{FunctionStore: db, Log: zap.NewNop()}
kvs := []*store.KVPair{}
subscriptionsDB := mock.NewMockStore(ctrl)
subscriptionsDB.EXPECT().List("default/", &store.ReadOptions{Consistent: true}).Return(kvs, nil)
functionsDB := mock.NewMockStore(ctrl)
functionsDB.EXPECT().Delete("default/testid").Return(nil)
service := &Service{FunctionStore: functionsDB, SubscriptionStore: subscriptionsDB, Log: zap.NewNop()}

err := service.DeleteFunction("default", function.ID("testid"))

Expand All @@ -252,15 +255,34 @@ func TestDeleteFunction_NotFound(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

db := mock.NewMockStore(ctrl)
db.EXPECT().Delete("default/testid").Return(errors.New("KV func not found"))
service := &Service{FunctionStore: db, Log: zap.NewNop()}
kvs := []*store.KVPair{}
subscriptionsDB := mock.NewMockStore(ctrl)
subscriptionsDB.EXPECT().List("default/", &store.ReadOptions{Consistent: true}).Return(kvs, nil)
functionsDB := mock.NewMockStore(ctrl)
functionsDB.EXPECT().Delete("default/testid").Return(errors.New("KV func not found"))
service := &Service{FunctionStore: functionsDB, SubscriptionStore: subscriptionsDB, Log: zap.NewNop()}

err := service.DeleteFunction("default", function.ID("testid"))

assert.EqualError(t, err, `Function "testid" not found.`)
}

func TestDeleteFunction_SubscriptionExists(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

kvs := []*store.KVPair{
{Value: []byte(`{"subscriptionId":"s1","default":"default","event":"test","functionId":"testid"}`)}}
subscriptionsDB := mock.NewMockStore(ctrl)
subscriptionsDB.EXPECT().List("default/", &store.ReadOptions{Consistent: true}).Return(kvs, nil)
functionsDB := mock.NewMockStore(ctrl)
service := &Service{FunctionStore: functionsDB, SubscriptionStore: subscriptionsDB, Log: zap.NewNop()}

err := service.DeleteFunction("default", function.ID("testid"))

assert.Equal(t, err, &function.ErrFunctionHasSubscriptionsError{})
}

func TestValidateFunction_AWSLambdaMissingRegion(t *testing.T) {
service := &Service{Log: zap.NewNop()}

Expand Down

0 comments on commit 6bf544e

Please sign in to comment.