From 691481424a1b2bf3a4b143d1f9ca96b7f1587abf Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 26 Jan 2022 15:50:21 +0100 Subject: [PATCH] Make compose page removal more flexible Support four delete strategies - abort: raise an error of page to be deleted contains subpages - force: delete the page regardles of any potential subpages - cascade: remove all subpages - rebase: remove page and move subpages one level lower (to the place where the parent page was) --- compose/rest.yaml | 9 +- compose/rest/page.go | 15 ++- compose/rest/request/page.go | 23 ++++ compose/service/page.go | 162 +++++++++++++++++++++------- compose/service/page_actions.gen.go | 68 ++++++++++++ compose/service/page_actions.yaml | 7 ++ compose/service/page_test.go | 145 +++++++++++++++++++++++++ compose/types/page.go | 32 ++++++ 8 files changed, 417 insertions(+), 44 deletions(-) create mode 100644 compose/service/page_test.go diff --git a/compose/rest.yaml b/compose/rest.yaml index 6a4e63f09..592cc860e 100644 --- a/compose/rest.yaml +++ b/compose/rest.yaml @@ -417,14 +417,13 @@ endpoints: title: Page ID order - name: delete path: "/{pageID}" - method: Delete + method: DELETE title: Delete page parameters: path: - - type: uint64 - name: pageID - required: true - title: Page ID + - { type: uint64, name: pageID, required: true, title: Page ID } + get: + - { type: string, name: strategy, title: "Page delete strategy (abort, force, rebase, cascade)" } - name: upload path: "/{pageID}/attachment" method: POST diff --git a/compose/rest/page.go b/compose/rest/page.go index addc51c7c..7fee4efdd 100644 --- a/compose/rest/page.go +++ b/compose/rest/page.go @@ -40,7 +40,7 @@ type ( Create(ctx context.Context, page *types.Page) (*types.Page, error) Update(ctx context.Context, page *types.Page) (*types.Page, error) - DeleteByID(ctx context.Context, namespaceID, pageID uint64) error + DeleteByID(ctx context.Context, namespaceID, pageID uint64, pds types.PageChildrenDeleteStrategy) error Reorder(ctx context.Context, namespaceID, selfID uint64, pageIDs []uint64) error } @@ -169,7 +169,18 @@ func (ctrl *Page) Update(ctx context.Context, r *request.PageUpdate) (interface{ } func (ctrl *Page) Delete(ctx context.Context, r *request.PageDelete) (interface{}, error) { - return api.OK(), ctrl.page.DeleteByID(ctx, r.NamespaceID, r.PageID) + var strategy types.PageChildrenDeleteStrategy + + switch aux := types.PageChildrenDeleteStrategy(r.Strategy); aux { + case types.PageChildrenOnDeleteForce, + types.PageChildrenOnDeleteRebase, + types.PageChildrenOnDeleteCascade: + strategy = aux + default: + strategy = types.PageChildrenOnDeleteAbort + } + + return api.OK(), ctrl.page.DeleteByID(ctx, r.NamespaceID, r.PageID, strategy) } func (ctrl *Page) Upload(ctx context.Context, r *request.PageUpload) (interface{}, error) { diff --git a/compose/rest/request/page.go b/compose/rest/request/page.go index 4d5fe95ec..56c31f10b 100644 --- a/compose/rest/request/page.go +++ b/compose/rest/request/page.go @@ -238,6 +238,11 @@ type ( // // Page ID PageID uint64 `json:",string"` + + // Strategy GET parameter + // + // Page delete strategy (abort, force, rebase, cascade) + Strategy string } PageUpload struct { @@ -1150,6 +1155,7 @@ func (r PageDelete) Auditable() map[string]interface{} { return map[string]interface{}{ "namespaceID": r.NamespaceID, "pageID": r.PageID, + "strategy": r.Strategy, } } @@ -1163,9 +1169,26 @@ func (r PageDelete) GetPageID() uint64 { return r.PageID } +// Auditable returns all auditable/loggable parameters +func (r PageDelete) GetStrategy() string { + return r.Strategy +} + // Fill processes request and fills internal variables func (r *PageDelete) Fill(req *http.Request) (err error) { + { + // GET params + tmp := req.URL.Query() + + if val, ok := tmp["strategy"]; ok && len(val) > 0 { + r.Strategy, err = val[0], nil + if err != nil { + return err + } + } + } + { var val string // path params diff --git a/compose/service/page.go b/compose/service/page.go index 481fc5c56..59f03a3a2 100644 --- a/compose/service/page.go +++ b/compose/service/page.go @@ -314,90 +314,178 @@ func (svc page) Create(ctx context.Context, new *types.Page) (*types.Page, error } func (svc page) Update(ctx context.Context, upd *types.Page) (c *types.Page, err error) { - return svc.updater(ctx, upd.NamespaceID, upd.ID, PageActionUpdate, svc.handleUpdate(ctx, upd)) -} - -func (svc page) DeleteByID(ctx context.Context, namespaceID, pageID uint64) error { - return trim1st(svc.updater(ctx, namespaceID, pageID, PageActionDelete, svc.handleDelete)) -} - -func (svc page) UndeleteByID(ctx context.Context, namespaceID, pageID uint64) error { - return trim1st(svc.updater(ctx, namespaceID, pageID, PageActionUndelete, svc.handleUndelete)) -} - -func (svc page) updater(ctx context.Context, namespaceID, pageID uint64, action func(...*pageActionProps) *pageAction, fn pageUpdateHandler) (*types.Page, error) { - var ( - changes pageChanges - - ns *types.Namespace - p, old *types.Page - aProps = &pageActionProps{page: &types.Page{ID: pageID, NamespaceID: namespaceID}} - err error - ) - err = store.Tx(ctx, svc.store, func(ctx context.Context, s store.Storer) (err error) { - ns, p, err = loadPage(ctx, s, namespaceID, pageID) + ns, res, err := loadPage(ctx, s, upd.NamespaceID, upd.ID) if err != nil { return } - if err = label.Load(ctx, svc.store, p); err != nil { + c, err = svc.updater(ctx, svc.store, ns, res, PageActionUpdate, svc.handleUpdate(ctx, upd)) + return + }) + + return +} + +func (svc page) DeleteByID(ctx context.Context, namespaceID, pageID uint64, strategy types.PageChildrenDeleteStrategy) error { + var ( + validChildren, pp types.PageSet + + ns *types.Namespace + res *types.Page + + skipUndeleted = func(p *types.Page) (bool, error) { + return p.DeletedAt == nil, nil + } + ) + + return store.Tx(ctx, svc.store, func(ctx context.Context, s store.Storer) (err error) { + if strategy == types.PageChildrenOnDeleteForce { + // simply delete the page and ignore the subpages + ns, res, err = loadPage(ctx, s, namespaceID, pageID) + if err != nil { + return + } + } else { + // Load all pages in the namespace and + // try to figure out the family tree + pp, _, err = store.SearchComposePages(ctx, s, types.PageFilter{ + NamespaceID: namespaceID, + }) + + if res = pp.FindByID(pageID); res == nil { + return PageErrNotFound() + } + + validChildren, _ = pp.FindByParent(res.ID).Filter(skipUndeleted) + + switch strategy { + case types.PageChildrenOnDeleteAbort: + // Abort if there are any valid (undeleted) children + if len(validChildren) > 0 { + return PageErrDeleteAbortedForPageWithSubpages() + } + + case types.PageChildrenOnDeleteRebase: + // update all our children to point to our parent + err = validChildren.Walk(func(child *types.Page) (err error) { + updChild := child.Clone() + updChild.SelfID = res.SelfID + _, err = svc.updater(ctx, s, ns, child, PageActionUpdate, svc.handleUpdate(ctx, updChild)) + return err + }) + + if err != nil { + return + } + + case types.PageChildrenOnDeleteCascade: + // update all our children to point to our parent + err = pp.RecursiveWalk(res, func(child *types.Page, _ *types.Page) (err error) { + if child.DeletedAt != nil { + // skip the ones that are already deleted + return nil + } + + _, err = svc.updater(ctx, s, ns, child, PageActionDelete, svc.handleDelete) + return err + }) + + if err != nil { + return + } + default: + return PageErrUnknownDeleteStrategy() + } + + if ns, err = loadNamespace(ctx, s, namespaceID); err != nil { + return + } + } + + _, err = svc.updater(ctx, svc.store, ns, res, PageActionDelete, svc.handleDelete) + return + }) +} + +func (svc page) UndeleteByID(ctx context.Context, namespaceID, pageID uint64) error { + return store.Tx(ctx, svc.store, func(ctx context.Context, s store.Storer) (err error) { + ns, res, err := loadPage(ctx, s, namespaceID, pageID) + if err != nil { + return + } + + _, err = svc.updater(ctx, svc.store, ns, res, PageActionUpdate, svc.handleUndelete) + return + }) +} + +func (svc page) updater(ctx context.Context, s store.Storer, ns *types.Namespace, res *types.Page, action func(...*pageActionProps) *pageAction, fn pageUpdateHandler) (*types.Page, error) { + var ( + changes pageChanges + old *types.Page + aProps = &pageActionProps{page: res} + err error + ) + + err = store.Tx(ctx, s, func(ctx context.Context, s store.Storer) (err error) { + if err = label.Load(ctx, svc.store, res); err != nil { return err } // Get max blockID for later use blockID := uint64(0) - for _, b := range p.Blocks { + for _, b := range res.Blocks { if b.BlockID > blockID { blockID = b.BlockID } } - old = p.Clone() + old = res.Clone() aProps.setNamespace(ns) - aProps.setChanged(p) + aProps.setChanged(res) - if p.DeletedAt == nil { - err = svc.eventbus.WaitFor(ctx, event.PageBeforeUpdate(p, old, ns, nil)) + if res.DeletedAt == nil { + err = svc.eventbus.WaitFor(ctx, event.PageBeforeUpdate(old, res, ns, nil)) } else { - err = svc.eventbus.WaitFor(ctx, event.PageBeforeDelete(p, old, ns, nil)) + err = svc.eventbus.WaitFor(ctx, event.PageBeforeDelete(old, res, ns, nil)) } if err != nil { return } - if changes, err = fn(ctx, ns, p); err != nil { + if changes, err = fn(ctx, ns, res); err != nil { return err } if changes&pageChanged > 0 { - if err = store.UpdateComposePage(ctx, svc.store, p); err != nil { + if err = store.UpdateComposePage(ctx, s, res); err != nil { return err } } - if err = updateTranslations(ctx, svc.ac, svc.locale, p.EncodeTranslations()...); err != nil { + if err = updateTranslations(ctx, svc.ac, svc.locale, res.EncodeTranslations()...); err != nil { return } if changes&pageLabelsChanged > 0 { - if err = label.Update(ctx, s, p); err != nil { + if err = label.Update(ctx, s, res); err != nil { return } } - if p.DeletedAt == nil { - err = svc.eventbus.WaitFor(ctx, event.PageAfterUpdate(p, old, ns, nil)) + if res.DeletedAt == nil { + err = svc.eventbus.WaitFor(ctx, event.PageAfterUpdate(res, res, ns, nil)) } else { - err = svc.eventbus.WaitFor(ctx, event.PageAfterDelete(nil, old, ns, nil)) + err = svc.eventbus.WaitFor(ctx, event.PageAfterDelete(nil, res, ns, nil)) } return err }) - return p, svc.recordAction(ctx, aProps, action, err) + return res, svc.recordAction(ctx, aProps, action, err) } // lookup fn() orchestrates page lookup, namespace preload and check diff --git a/compose/service/page_actions.gen.go b/compose/service/page_actions.gen.go index 53826637c..20b8faf8c 100644 --- a/compose/service/page_actions.gen.go +++ b/compose/service/page_actions.gen.go @@ -742,6 +742,74 @@ func PageErrInvalidNamespaceID(mm ...*pageActionProps) *errors.Error { return e } +// PageErrDeleteAbortedForPageWithSubpages returns "compose:page.deleteAbortedForPageWithSubpages" as *errors.Error +// +// +// This function is auto-generated. +// +func PageErrDeleteAbortedForPageWithSubpages(mm ...*pageActionProps) *errors.Error { + var p = &pageActionProps{} + if len(mm) > 0 { + p = mm[0] + } + + var e = errors.New( + errors.KindInternal, + + p.Format("removal of page with subpages aborted", nil), + + errors.Meta("type", "deleteAbortedForPageWithSubpages"), + errors.Meta("resource", "compose:page"), + + errors.Meta(pagePropsMetaKey{}, p), + + // translation namespace & key + errors.Meta(locale.ErrorMetaNamespace{}, "compose"), + errors.Meta(locale.ErrorMetaKey{}, "page.errors.deleteAbortedForPageWithSubpages"), + + errors.StackSkip(1), + ) + + if len(mm) > 0 { + } + + return e +} + +// PageErrUnknownDeleteStrategy returns "compose:page.unknownDeleteStrategy" as *errors.Error +// +// +// This function is auto-generated. +// +func PageErrUnknownDeleteStrategy(mm ...*pageActionProps) *errors.Error { + var p = &pageActionProps{} + if len(mm) > 0 { + p = mm[0] + } + + var e = errors.New( + errors.KindInternal, + + p.Format("unknown delete strategy", nil), + + errors.Meta("type", "unknownDeleteStrategy"), + errors.Meta("resource", "compose:page"), + + errors.Meta(pagePropsMetaKey{}, p), + + // translation namespace & key + errors.Meta(locale.ErrorMetaNamespace{}, "compose"), + errors.Meta(locale.ErrorMetaKey{}, "page.errors.unknownDeleteStrategy"), + + errors.StackSkip(1), + ) + + if len(mm) > 0 { + } + + return e +} + // PageErrNotAllowedToRead returns "compose:page.notAllowedToRead" as *errors.Error // // diff --git a/compose/service/page_actions.yaml b/compose/service/page_actions.yaml index bd08ae4ab..3429abb2c 100644 --- a/compose/service/page_actions.yaml +++ b/compose/service/page_actions.yaml @@ -84,6 +84,13 @@ errors: message: "invalid or missing namespace ID" severity: warning + - error: deleteAbortedForPageWithSubpages + message: "removal of page with subpages aborted" + severity: debug + + - error: unknownDeleteStrategy + message: "unknown delete strategy" + - error: notAllowedToRead message: "not allowed to read this page" log: "could not read {{page}}; insufficient permissions" diff --git a/compose/service/page_test.go b/compose/service/page_test.go new file mode 100644 index 000000000..dd7efcf8a --- /dev/null +++ b/compose/service/page_test.go @@ -0,0 +1,145 @@ +package service + +import ( + "context" + "testing" + + "github.com/cortezaproject/corteza-server/compose/types" + "github.com/cortezaproject/corteza-server/pkg/eventbus" + "github.com/cortezaproject/corteza-server/pkg/locale" + "github.com/cortezaproject/corteza-server/pkg/logger" + "github.com/cortezaproject/corteza-server/pkg/rbac" + "github.com/cortezaproject/corteza-server/store" + "github.com/cortezaproject/corteza-server/store/sqlite3" + "github.com/stretchr/testify/require" +) + +func TestPageDeleting(t *testing.T) { + var ( + //ctx = context.Background() + //s, err = sqlite3.ConnectInMemory(ctx) + + ctx = logger.ContextWithValue(context.Background(), logger.MakeDebugLogger()) + s, err = sqlite3.ConnectInMemoryWithDebug(ctx) + + namespaceID = nextID() + ns *types.Namespace + + pages = types.PageSet{ + // should be deleted w/o a problem + &types.Page{ID: 1}, + &types.Page{ID: 2}, + &types.Page{ID: 3, SelfID: 2}, + //&types.Page{ID: 4}, + //&types.Page{ID: 5, SelfID: 4}, + + // will be used for rebase delete + &types.Page{ID: 10}, + &types.Page{ID: 11, SelfID: 10}, + &types.Page{ID: 12, SelfID: 10}, + &types.Page{ID: 121, SelfID: 12}, + &types.Page{ID: 122, SelfID: 12}, + + // will be used for cascade delete + &types.Page{ID: 20}, + &types.Page{ID: 21, SelfID: 20}, + &types.Page{ID: 22, SelfID: 20}, + &types.Page{ID: 221, SelfID: 22}, + &types.Page{ID: 222, SelfID: 22}, + } + + svc = &page{ + store: s, + ac: &accessControl{rbac: &rbac.ServiceAllowAll{}}, + eventbus: eventbus.New(), + locale: ResourceTranslationsManager(locale.Static()), + } + + pageLookup = func(t *testing.T, pageID uint64) *types.Page { + p, err := store.LookupComposePageByID(ctx, s, pageID) + require.NoError(t, err) + require.NotNil(t, p) + return p + } + ) + + if err != nil { + t.Fatalf("failed to init sqlite in-memory db: %v", err) + } + + //if err = store.Upgrade(ctx, zap.NewNop(), s); err != nil { + if err = store.Upgrade(ctx, logger.MakeDebugLogger(), s); err != nil { + t.Fatalf("failed to upgrade store: %v", err) + } + + if err = s.TruncateComposeNamespaces(ctx); err != nil { + t.Fatalf("failed to truncate compose namespaces: %v", err) + } + + if err = s.TruncateComposeModules(ctx); err != nil { + t.Fatalf("failed to truncate compose modules: %v", err) + } + + // + ns = &types.Namespace{Name: "testing", ID: namespaceID, CreatedAt: *now()} + if err = store.CreateComposeNamespace(ctx, s, ns); err != nil { + t.Fatalf("failed to seed namespaces: %v", err) + } + + _ = pages.Walk(func(p *types.Page) error { + p.NamespaceID = ns.ID + return nil + }) + + if err = store.CreateComposePage(ctx, s, pages...); err != nil { + t.Fatalf("failed to seed pages: %v", err) + } + + t.Run("remove page without children", func(t *testing.T) { + require.NoError(t, svc.DeleteByID(ctx, ns.ID, 1, types.PageChildrenOnDeleteAbort)) + require.NotNil(t, pageLookup(t, 1).DeletedAt, "parent should be deleted") + }) + + t.Run("abort when children are present", func(t *testing.T) { + require.ErrorIs(t, svc.DeleteByID(ctx, ns.ID, 2, types.PageChildrenOnDeleteAbort), PageErrDeleteAbortedForPageWithSubpages()) + require.Nil(t, pageLookup(t, 2).DeletedAt, "parent should be deleted") + require.Nil(t, pageLookup(t, 3).DeletedAt, "child should be deleted") + }) + + t.Run("remove only parent when forced", func(t *testing.T) { + require.NoError(t, svc.DeleteByID(ctx, ns.ID, 2, types.PageChildrenOnDeleteForce)) + require.NotNil(t, pageLookup(t, 2).DeletedAt, "parent should be deleted") + require.Nil(t, pageLookup(t, 3).DeletedAt, "child should not be deleted") + }) + + t.Run("delete parent and rebase children", func(t *testing.T) { + require.NoError(t, svc.DeleteByID(ctx, ns.ID, 10, types.PageChildrenOnDeleteRebase)) + require.NotNil(t, pageLookup(t, 10).DeletedAt) + + for _, pageID := range []uint64{11, 12} { + require.Nil(t, pageLookup(t, pageID).DeletedAt, "child page should not be deleted") + require.Equal(t, uint64(0), pageLookup(t, pageID).SelfID, "child page should be moved one level lower") + } + + for _, pageID := range []uint64{121, 122} { + require.Nil(t, pageLookup(t, pageID).DeletedAt, "grandchild page should not be deleted") + require.Equal(t, uint64(12), pageLookup(t, pageID).SelfID, "grand child page should stay where it is") + } + }) + + t.Run("delete parent and all children", func(t *testing.T) { + require.NoError(t, svc.DeleteByID(ctx, ns.ID, 20, types.PageChildrenOnDeleteCascade)) + require.NotNil(t, pageLookup(t, 20).DeletedAt, "parent page should be deleted") + + for _, pageID := range []uint64{21, 22} { + require.NotNil(t, pageLookup(t, pageID).DeletedAt, "child page should not be deleted") + require.Equal(t, uint64(20), pageLookup(t, pageID).SelfID, "child page should be stay where it is") + } + + for _, pageID := range []uint64{221, 222} { + require.NotNil(t, pageLookup(t, pageID).DeletedAt, "grandchild page should not be deleted") + require.Equal(t, uint64(22), pageLookup(t, pageID).SelfID, "grandchild page should be stay where it is") + } + + }) +} diff --git a/compose/types/page.go b/compose/types/page.go index f4c1a9705..034effc1e 100644 --- a/compose/types/page.go +++ b/compose/types/page.go @@ -98,6 +98,15 @@ type ( filter.Sorting filter.Paging } + + PageChildrenDeleteStrategy string +) + +const ( + PageChildrenOnDeleteAbort PageChildrenDeleteStrategy = "abort" + PageChildrenOnDeleteForce PageChildrenDeleteStrategy = "force" + PageChildrenOnDeleteRebase PageChildrenDeleteStrategy = "rebase" + PageChildrenOnDeleteCascade PageChildrenDeleteStrategy = "cascade" ) func (m Page) Clone() *Page { @@ -271,3 +280,26 @@ func (set PageSet) FindByParent(parentID uint64) (out PageSet) { return } + +// RecursiveWalk through all child pages +func (set PageSet) RecursiveWalk(parent *Page, fn func(c *Page, parent *Page) error) (err error) { + if parent == nil { + return + } + + for _, page := range set { + if page.SelfID != parent.ID { + continue + } + + if err = fn(page, parent); err != nil { + return + } + + if err = set.RecursiveWalk(page, fn); err != nil { + return + } + } + + return +}