From 6b7accd3c6c542ab24c98136a849070a58e3f5e2 Mon Sep 17 00:00:00 2001 From: Denis Makogon Date: Mon, 11 Sep 2017 23:15:01 +0300 Subject: [PATCH] Simplifying app delete per review comments --- .../internal/datastoreutil/metrics.go | 18 ------------ .../internal/datastoreutil/validator.go | 12 -------- api/datastore/sql/sql.go | 2 ++ api/models/datastore.go | 3 -- api/models/logs.go | 2 -- api/server/apps_delete.go | 28 +------------------ 6 files changed, 3 insertions(+), 62 deletions(-) diff --git a/api/datastore/internal/datastoreutil/metrics.go b/api/datastore/internal/datastoreutil/metrics.go index 796e4dd02..cf1c8afbf 100644 --- a/api/datastore/internal/datastoreutil/metrics.go +++ b/api/datastore/internal/datastoreutil/metrics.go @@ -118,23 +118,5 @@ func (m *metricds) DeleteLog(ctx context.Context, appName, callID string) error return m.ds.DeleteLog(ctx, appName, callID) } -func (m *metricds) BatchDeleteLogs(ctx context.Context, appName string) error { - span, ctx := opentracing.StartSpanFromContext(ctx, "ds_batch_delete_logs") - defer span.Finish() - return m.ds.BatchDeleteLogs(ctx, appName) -} - -func (m *metricds) BatchDeleteCalls(ctx context.Context, appName string) error { - span, ctx := opentracing.StartSpanFromContext(ctx, "ds_batch_delete_calls") - defer span.Finish() - return m.ds.BatchDeleteCalls(ctx, appName) -} - -func (m *metricds) BatchDeleteRoutes(ctx context.Context, appName string) error { - span, ctx := opentracing.StartSpanFromContext(ctx, "ds_batch_delete_routes") - defer span.Finish() - return m.ds.BatchDeleteRoutes(ctx, appName) -} - // instant & no context ;) func (m *metricds) GetDatabase() *sqlx.DB { return m.ds.GetDatabase() } diff --git a/api/datastore/internal/datastoreutil/validator.go b/api/datastore/internal/datastoreutil/validator.go index a5bbec3e0..c08728820 100644 --- a/api/datastore/internal/datastoreutil/validator.go +++ b/api/datastore/internal/datastoreutil/validator.go @@ -142,18 +142,6 @@ func (v *validator) DeleteLog(ctx context.Context, appName, callID string) error return v.Datastore.DeleteLog(ctx, appName, callID) } -func (v *validator) BatchDeleteLogs(ctx context.Context, appName string) error { - return v.Datastore.BatchDeleteLogs(ctx, appName) -} - -func (v *validator) BatchDeleteCalls(ctx context.Context, appName string) error { - return v.Datastore.BatchDeleteCalls(ctx, appName) -} - -func (v *validator) BatchDeleteRoutes(ctx context.Context, appName string) error { - return v.Datastore.BatchDeleteRoutes(ctx, appName) -} - // GetDatabase returns the underlying sqlx database implementation func (v *validator) GetDatabase() *sqlx.DB { return v.Datastore.GetDatabase() diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index a7ba76ce8..77945fbbe 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -221,6 +221,8 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. } func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { + ds.db.ExecContext(ctx, ds.db.Rebind( + `DELETE FROM routes, calls, logs WHERE app_name=?`), appName) query := ds.db.Rebind(`DELETE FROM apps WHERE name = ?`) _, err := ds.db.ExecContext(ctx, query, appName) return err diff --git a/api/models/datastore.go b/api/models/datastore.go index 3268619b3..b77ba6b48 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -28,7 +28,6 @@ type Datastore interface { // RemoveApp removes the App named appName. Returns ErrDatastoreEmptyAppName if appName is empty. // Returns ErrAppsNotFound if an App is not found. - // TODO remove routes automatically? #528 RemoveApp(ctx context.Context, appName string) error // GetRoute looks up a matching Route for appName and the literal request route routePath. @@ -68,8 +67,6 @@ type Datastore interface { // calls exist, an empty list and a nil error are returned. GetCalls(ctx context.Context, filter *CallFilter) ([]*Call, error) - BatchDeleteCalls(ctx context.Context, appName string) error - BatchDeleteRoutes(ctx context.Context, appName string) error // Implement LogStore methods for convenience LogStore diff --git a/api/models/logs.go b/api/models/logs.go index 0ab0268db..a4cc7da88 100644 --- a/api/models/logs.go +++ b/api/models/logs.go @@ -20,6 +20,4 @@ type LogStore interface { // DeleteLog will remove the log at callID, it will not return an error if // the log does not exist before removal. DeleteLog(ctx context.Context, appName, callID string) error - - BatchDeleteLogs(ctx context.Context, appName string) error } diff --git a/api/server/apps_delete.go b/api/server/apps_delete.go index a572ee5fa..bdcbba751 100644 --- a/api/server/apps_delete.go +++ b/api/server/apps_delete.go @@ -7,7 +7,6 @@ import ( "github.com/fnproject/fn/api/common" "github.com/fnproject/fn/api/models" "github.com/gin-gonic/gin" - "strconv" ) func (s *Server) handleAppDelete(c *gin.Context) { @@ -16,32 +15,7 @@ func (s *Server) handleAppDelete(c *gin.Context) { app := &models.App{Name: c.MustGet(api.AppName).(string)} - routes, err := s.Datastore.GetRoutesByApp(ctx, app.Name, &models.RouteFilter{}) - if err != nil { - log.WithError(err).Error("error getting route in app delete") - handleErrorResponse(c, err) - return - } - forceDelete, _ := strconv.ParseBool(c.Query("force")) - if !forceDelete { - if len(routes) > 0 { - handleErrorResponse(c, models.ErrDeleteAppsWithRoutes) - return - } - } else { - s.Datastore.BatchDeleteLogs(ctx, app.Name) - s.Datastore.BatchDeleteCalls(ctx, app.Name) - s.Datastore.BatchDeleteRoutes(ctx, app.Name) - } - - err = s.FireBeforeAppDelete(ctx, app) - if err != nil { - log.WithError(err).Error("error firing before app delete") - handleErrorResponse(c, err) - return - } - - app, err = s.Datastore.GetApp(ctx, app.Name) + app, err := s.Datastore.GetApp(ctx, app.Name) if err != nil { handleErrorResponse(c, err) return