From 9717e48f84bde5b679059e70486e2ba281582114 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Thu, 17 Oct 2019 17:15:05 +0200 Subject: [PATCH] Refactor application repo --- api/system/spec.json | 36 +++++++++- api/system/spec/application.json | 35 ++++++++- api/system/spec/reminder.json | 12 ++++ docs/system/README.md | 7 ++ system/provision-config.go | 2 +- system/repository/application.go | 91 ++++++++++++++++++------ system/rest/application.go | 110 +++++++++++++++++++++++------ system/rest/request/application.go | 27 +++++++ system/rest/request/reminder.go | 13 ++++ system/service/access_control.go | 4 ++ system/service/application.go | 44 +++++------- system/types/applications.go | 14 ++++ system/types/role.go | 9 +++ tests/system/application_test.go | 6 +- 14 files changed, 334 insertions(+), 76 deletions(-) diff --git a/api/system/spec.json b/api/system/spec.json index f16554774..0df10c8c2 100644 --- a/api/system/spec.json +++ b/api/system/spec.json @@ -1030,7 +1030,41 @@ "name": "list", "method": "GET", "title": "List applications", - "path": "/" + "path": "/", + "parameters": { + "get": [ + { + "name": "name", + "required": false, + "title": "Application name", + "type": "string" + }, + { + "name": "query", + "required": false, + "title": "Filter applications", + "type": "string" + }, + { + "name": "page", + "type": "uint", + "required": false, + "title": "Page number" + }, + { + "name": "perPage", + "type": "uint", + "required": false, + "title": "Returned items per page (default 50)" + }, + { + "name": "sort", + "required": false, + "title": "Sort", + "type": "string" + } + ] + } }, { "name": "create", diff --git a/api/system/spec/application.json b/api/system/spec/application.json index ed2ef64f8..5c1fe72ac 100644 --- a/api/system/spec/application.json +++ b/api/system/spec/application.json @@ -18,7 +18,40 @@ "Method": "GET", "Title": "List applications", "Path": "/", - "Parameters": null + "Parameters": { + "get": [ + { + "name": "name", + "required": false, + "title": "Application name", + "type": "string" + }, + { + "name": "query", + "required": false, + "title": "Filter applications", + "type": "string" + }, + { + "name": "page", + "required": false, + "title": "Page number", + "type": "uint" + }, + { + "name": "perPage", + "required": false, + "title": "Returned items per page (default 50)", + "type": "uint" + }, + { + "name": "sort", + "required": false, + "title": "Sort", + "type": "string" + } + ] + } }, { "Name": "create", diff --git a/api/system/spec/reminder.json b/api/system/spec/reminder.json index 7c95f8052..056336ea3 100644 --- a/api/system/spec/reminder.json +++ b/api/system/spec/reminder.json @@ -24,6 +24,12 @@ "Path": "/", "Parameters": { "get": [ + { + "name": "reminderID", + "required": false, + "title": "Filter by reminder ID", + "type": "[]string" + }, { "name": "resource", "required": false, @@ -71,6 +77,12 @@ "required": false, "title": "Returned items per page (default 50)", "type": "uint" + }, + { + "name": "sort", + "required": false, + "title": "Sort", + "type": "string" } ] } diff --git a/docs/system/README.md b/docs/system/README.md index 0631319a6..48522a2f2 100644 --- a/docs/system/README.md +++ b/docs/system/README.md @@ -20,6 +20,11 @@ | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | +| name | string | GET | Application name | N/A | NO | +| query | string | GET | Filter applications | N/A | NO | +| page | uint | GET | Page number | N/A | NO | +| perPage | uint | GET | Returned items per page (default 50) | N/A | NO | +| sort | string | GET | Sort | N/A | NO | ## Create application @@ -713,6 +718,7 @@ Organisations represent a top-level grouping entity. There may be many organisat | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | +| reminderID | []string | GET | Filter by reminder ID | N/A | NO | | resource | string | GET | Only reminders of a specific resource | N/A | NO | | assignedTo | uint64 | GET | Only reminders for a given user | N/A | NO | | scheduledFrom | *time.Time | GET | Only reminders from this time (included) | N/A | NO | @@ -721,6 +727,7 @@ Organisations represent a top-level grouping entity. There may be many organisat | excludeDismissed | bool | GET | Filter out dismissed reminders | N/A | NO | | page | uint | GET | Page number (0 based) | N/A | NO | | perPage | uint | GET | Returned items per page (default 50) | N/A | NO | +| sort | string | GET | Sort | N/A | NO | ## Add new reminder diff --git a/system/provision-config.go b/system/provision-config.go index 2a9ebbfbd..31f421d2d 100644 --- a/system/provision-config.go +++ b/system/provision-config.go @@ -57,7 +57,7 @@ func makeDefaultApplications(ctx context.Context, cmd *cobra.Command, c *cli.Con repo := repository.Application(ctx, db) - aa, err := repo.Find() + aa, _, err := repo.Find(types.ApplicationFilter{}) if err != nil { return err } diff --git a/system/repository/application.go b/system/repository/application.go index e94196e24..30da6f15f 100644 --- a/system/repository/application.go +++ b/system/repository/application.go @@ -5,7 +5,9 @@ import ( "time" "github.com/titpetric/factory" + "gopkg.in/Masterminds/squirrel.v1" + "github.com/cortezaproject/corteza-server/pkg/rh" "github.com/cortezaproject/corteza-server/system/types" ) @@ -14,7 +16,7 @@ type ( With(ctx context.Context, db *factory.DB) ApplicationRepository FindByID(id uint64) (*types.Application, error) - Find() (types.ApplicationSet, error) + Find(types.ApplicationFilter) (types.ApplicationSet, types.ApplicationFilter, error) Create(mod *types.Application) (*types.Application, error) Update(mod *types.Application) (*types.Application, error) @@ -24,16 +26,10 @@ type ( application struct { *repository - - // sql table reference - table string } ) const ( - sqlApplicationColumns = "id, rel_owner, name, enabled, unify, created_at, updated_at, deleted_at" - sqlApplicationScope = "deleted_at IS NULL" - ErrApplicationNotFound = repositoryError("ApplicationNotFound") ) @@ -45,41 +41,92 @@ func Application(ctx context.Context, db *factory.DB) ApplicationRepository { func (r *application) With(ctx context.Context, db *factory.DB) ApplicationRepository { return &application{ repository: r.repository.With(ctx, db), - table: "sys_application", } } -func (r *application) FindByID(id uint64) (*types.Application, error) { - sql := "SELECT " + sqlApplicationColumns + " FROM " + r.table + " WHERE id = ? AND " + sqlApplicationScope - mod := &types.Application{} - - return mod, isFound(r.db().Get(mod, sql, id), mod.ID > 0, ErrApplicationNotFound) +func (r application) table() string { + return "sys_application" } -func (r *application) Find() (types.ApplicationSet, error) { - rval := make([]*types.Application, 0) - params := make([]interface{}, 0) +func (r application) columns() []string { + return []string{ + "id", + "rel_owner", + "name", + "enabled", + "unify", + "created_at", + "updated_at", + "deleted_at", + } +} - sql := "SELECT " + sqlApplicationColumns + " FROM " + r.table + " WHERE " + sqlApplicationScope +func (r application) query() squirrel.SelectBuilder { + return squirrel. + Select(r.columns()...). + From(r.table()). + Where("deleted_at IS NULL") +} - sql += " ORDER BY id ASC" +func (r *application) FindByID(id uint64) (*types.Application, error) { + return r.findOneBy("id", id) +} - return rval, r.db().Select(&rval, sql, params...) +func (r application) findOneBy(field string, value interface{}) (*types.Application, error) { + var ( + app = &types.Application{} + + q = r.query(). + Where(squirrel.Eq{field: value}) + + err = rh.FetchOne(r.db(), q, app) + ) + + if err != nil { + return nil, err + } else if app.ID == 0 { + return nil, ErrApplicationNotFound + } + + return app, nil +} + +func (r *application) Find(filter types.ApplicationFilter) (set types.ApplicationSet, f types.ApplicationFilter, err error) { + f = filter + + if f.Sort == "" { + f.Sort = "id" + } + + query := r.query() + + var orderBy []string + if orderBy, err = rh.ParseOrder(f.Sort, r.columns()...); err != nil { + return + } else { + query = query.OrderBy(orderBy...) + } + + if f.Count, err = rh.Count(r.db(), query); err != nil || f.Count == 0 { + return + } + + return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) } func (r *application) Create(mod *types.Application) (*types.Application, error) { mod.ID = factory.Sonyflake.NextID() mod.CreatedAt = time.Now() - return mod, r.db().Insert(r.table, mod) + return mod, r.db().Insert(r.table(), mod) } func (r *application) Update(mod *types.Application) (*types.Application, error) { mod.UpdatedAt = timeNowPtr() - return mod, r.db().Replace(r.table, mod) + return mod, r.db().Replace(r.table(), mod) } func (r *application) DeleteByID(id uint64) error { - return r.updateColumnByID(r.table, "deleted_at", time.Now(), id) + return r.updateColumnByID(r.table(), "deleted_at", time.Now(), id) } diff --git a/system/rest/application.go b/system/rest/application.go index d4346b66d..ce49d1be4 100644 --- a/system/rest/application.go +++ b/system/rest/application.go @@ -5,6 +5,7 @@ import ( "github.com/titpetric/factory/resputil" + "github.com/cortezaproject/corteza-server/pkg/rh" "github.com/cortezaproject/corteza-server/system/rest/request" "github.com/cortezaproject/corteza-server/system/service" "github.com/cortezaproject/corteza-server/system/types" @@ -14,28 +15,60 @@ import ( var _ = errors.Wrap -type Application struct { - svc struct { +type ( + Application struct { application service.ApplicationService + ac applicationAccessController } -} + + applicationAccessController interface { + CanGrant(context.Context) bool + + CanUpdateApplication(context.Context, *types.Application) bool + CanDeleteApplication(context.Context, *types.Application) bool + } + + applicationPayload struct { + *types.Application + + CanGrant bool `json:"canGrant"` + CanUpdateApplication bool `json:"canUpdateApplication"` + CanDeleteApplication bool `json:"canDeleteApplication"` + } + + applicationSetPayload struct { + Filter types.ApplicationFilter `json:"filter"` + Set []*applicationPayload `json:"set"` + } +) func (Application) New() *Application { - ctrl := &Application{} - ctrl.svc.application = service.DefaultApplication - - return ctrl + return &Application{ + application: service.DefaultApplication, + ac: service.DefaultAccessControl, + } } func (ctrl *Application) List(ctx context.Context, r *request.ApplicationList) (interface{}, error) { - return ctrl.svc.application.With(ctx).Find() + f := types.ApplicationFilter{ + Name: r.Name, + Query: r.Query, + Sort: r.Sort, + PageFilter: rh.Paging(r.Page, r.PerPage), + } + + set, filter, err := ctrl.application.With(ctx).Find(f) + return ctrl.makeFilterPayload(ctx, set, filter, err) } func (ctrl *Application) Create(ctx context.Context, r *request.ApplicationCreate) (interface{}, error) { - app := &types.Application{ - Name: r.Name, - Enabled: r.Enabled, - } + var ( + err error + app = &types.Application{ + Name: r.Name, + Enabled: r.Enabled, + } + ) if r.Unify != nil { app.Unify = &types.ApplicationUnify{} @@ -44,15 +77,19 @@ func (ctrl *Application) Create(ctx context.Context, r *request.ApplicationCreat } } - return ctrl.svc.application.With(ctx).Create(app) + app, err = ctrl.application.With(ctx).Create(app) + return ctrl.makePayload(ctx, app, err) } func (ctrl *Application) Update(ctx context.Context, r *request.ApplicationUpdate) (interface{}, error) { - app := &types.Application{ - ID: r.ApplicationID, - Name: r.Name, - Enabled: r.Enabled, - } + var ( + err error + app = &types.Application{ + ID: r.ApplicationID, + Name: r.Name, + Enabled: r.Enabled, + } + ) if r.Unify != nil { app.Unify = &types.ApplicationUnify{} @@ -61,13 +98,44 @@ func (ctrl *Application) Update(ctx context.Context, r *request.ApplicationUpdat } } - return ctrl.svc.application.With(ctx).Update(app) + app, err = ctrl.application.With(ctx).Update(app) + return ctrl.makePayload(ctx, app, err) } func (ctrl *Application) Read(ctx context.Context, r *request.ApplicationRead) (interface{}, error) { - return ctrl.svc.application.With(ctx).FindByID(r.ApplicationID) + app, err := ctrl.application.With(ctx).FindByID(r.ApplicationID) + return ctrl.makePayload(ctx, app, err) } func (ctrl *Application) Delete(ctx context.Context, r *request.ApplicationDelete) (interface{}, error) { - return resputil.OK(), ctrl.svc.application.With(ctx).DeleteByID(r.ApplicationID) + return resputil.OK(), ctrl.application.With(ctx).DeleteByID(r.ApplicationID) +} + +func (ctrl Application) makePayload(ctx context.Context, m *types.Application, err error) (*applicationPayload, error) { + if err != nil || m == nil { + return nil, err + } + + return &applicationPayload{ + Application: m, + + CanGrant: ctrl.ac.CanGrant(ctx), + + CanUpdateApplication: ctrl.ac.CanUpdateApplication(ctx, m), + CanDeleteApplication: ctrl.ac.CanDeleteApplication(ctx, m), + }, nil +} + +func (ctrl Application) makeFilterPayload(ctx context.Context, nn types.ApplicationSet, f types.ApplicationFilter, err error) (*applicationSetPayload, error) { + if err != nil { + return nil, err + } + + msp := &applicationSetPayload{Filter: f, Set: make([]*applicationPayload, len(nn))} + + for i := range nn { + msp.Set[i], _ = ctrl.makePayload(ctx, nn[i], nil) + } + + return msp, nil } diff --git a/system/rest/request/application.go b/system/rest/request/application.go index dff68dace..f9343d1e4 100644 --- a/system/rest/request/application.go +++ b/system/rest/request/application.go @@ -34,6 +34,11 @@ var _ = multipart.FileHeader{} // Application list request parameters type ApplicationList struct { + Name string + Query string + Page uint + PerPage uint + Sort string } func NewApplicationList() *ApplicationList { @@ -43,6 +48,12 @@ func NewApplicationList() *ApplicationList { func (r ApplicationList) Auditable() map[string]interface{} { var out = map[string]interface{}{} + out["name"] = r.Name + out["query"] = r.Query + out["page"] = r.Page + out["perPage"] = r.PerPage + out["sort"] = r.Sort + return out } @@ -73,6 +84,22 @@ func (r *ApplicationList) Fill(req *http.Request) (err error) { post[name] = string(param[0]) } + if val, ok := get["name"]; ok { + r.Name = val + } + if val, ok := get["query"]; ok { + r.Query = val + } + if val, ok := get["page"]; ok { + r.Page = parseUint(val) + } + if val, ok := get["perPage"]; ok { + r.PerPage = parseUint(val) + } + if val, ok := get["sort"]; ok { + r.Sort = val + } + return err } diff --git a/system/rest/request/reminder.go b/system/rest/request/reminder.go index 8c4c1d16e..0a2e6f788 100644 --- a/system/rest/request/reminder.go +++ b/system/rest/request/reminder.go @@ -35,6 +35,7 @@ var _ = multipart.FileHeader{} // Reminder list request parameters type ReminderList struct { + ReminderID []string Resource string AssignedTo uint64 `json:",string"` ScheduledFrom *time.Time @@ -43,6 +44,7 @@ type ReminderList struct { ExcludeDismissed bool Page uint PerPage uint + Sort string } func NewReminderList() *ReminderList { @@ -52,6 +54,7 @@ func NewReminderList() *ReminderList { func (r ReminderList) Auditable() map[string]interface{} { var out = map[string]interface{}{} + out["reminderID"] = r.ReminderID out["resource"] = r.Resource out["assignedTo"] = r.AssignedTo out["scheduledFrom"] = r.ScheduledFrom @@ -60,6 +63,7 @@ func (r ReminderList) Auditable() map[string]interface{} { out["excludeDismissed"] = r.ExcludeDismissed out["page"] = r.Page out["perPage"] = r.PerPage + out["sort"] = r.Sort return out } @@ -91,6 +95,12 @@ func (r *ReminderList) Fill(req *http.Request) (err error) { post[name] = string(param[0]) } + if val, ok := urlQuery["reminderID[]"]; ok { + r.ReminderID = parseStrings(val) + } else if val, ok = urlQuery["reminderID"]; ok { + r.ReminderID = parseStrings(val) + } + if val, ok := get["resource"]; ok { r.Resource = val } @@ -121,6 +131,9 @@ func (r *ReminderList) Fill(req *http.Request) (err error) { if val, ok := get["perPage"]; ok { r.PerPage = parseUint(val) } + if val, ok := get["sort"]; ok { + r.Sort = val + } return err } diff --git a/system/service/access_control.go b/system/service/access_control.go index 077b9e6a4..44437e0bf 100644 --- a/system/service/access_control.go +++ b/system/service/access_control.go @@ -106,6 +106,10 @@ func (svc accessControl) CanReadApplication(ctx context.Context, app *types.Appl return svc.can(ctx, app, "read", permissions.Allowed) } +func (svc accessControl) FilterReadableApplications(ctx context.Context) *permissions.ResourceFilter { + return svc.permissions.ResourceFilter(ctx, types.ApplicationPermissionResource, "read", permissions.Deny) +} + func (svc accessControl) CanUpdateApplication(ctx context.Context, app *types.Application) bool { return svc.can(ctx, app, "update") } diff --git a/system/service/application.go b/system/service/application.go index 46b3be6c7..6d72ed00f 100644 --- a/system/service/application.go +++ b/system/service/application.go @@ -3,9 +3,9 @@ package service import ( "context" - "github.com/pkg/errors" "github.com/titpetric/factory" + "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/system/repository" "github.com/cortezaproject/corteza-server/system/types" ) @@ -25,13 +25,15 @@ type ( CanReadApplication(context.Context, *types.Application) bool CanUpdateApplication(context.Context, *types.Application) bool CanDeleteApplication(context.Context, *types.Application) bool + + FilterReadableApplications(ctx context.Context) *permissions.ResourceFilter } ApplicationService interface { With(ctx context.Context) ApplicationService FindByID(applicationID uint64) (*types.Application, error) - Find() (types.ApplicationSet, error) + Find(types.ApplicationFilter) (types.ApplicationSet, types.ApplicationFilter, error) Create(application *types.Application) (*types.Application, error) Update(application *types.Application) (*types.Application, error) @@ -56,48 +58,39 @@ func (svc *application) With(ctx context.Context) ApplicationService { } } -func (svc *application) FindByID(id uint64) (*types.Application, error) { - app, err := svc.application.FindByID(id) - if err != nil { +func (svc *application) FindByID(ID uint64) (app *types.Application, err error) { + if ID == 0 { + return nil, ErrInvalidID + } + + if app, err = svc.application.FindByID(ID); err != nil { return nil, err } if !svc.ac.CanReadApplication(svc.ctx, app) { - return nil, errors.New("Not allowed to access application") + return nil, ErrNoPermissions.withStack() } return app, nil } -func (svc *application) Find() (types.ApplicationSet, error) { - apps, err := svc.application.Find() - if err != nil { - return nil, err - } - - ret := []*types.Application{} - for _, app := range apps { - if svc.ac.CanReadApplication(svc.ctx, app) { - ret = append(ret, app) - } // - } - return ret, nil +func (svc *application) Find(f types.ApplicationFilter) (types.ApplicationSet, types.ApplicationFilter, error) { + f.IsReadable = svc.ac.FilterReadableApplications(svc.ctx) + return svc.application.Find(f) } func (svc *application) Create(mod *types.Application) (*types.Application, error) { if !svc.ac.CanCreateApplication(svc.ctx) { - return nil, errors.New("Not allowed to create application") + return nil, ErrNoPermissions.withStack() } return svc.application.Create(mod) } func (svc *application) Update(mod *types.Application) (t *types.Application, err error) { if !svc.ac.CanUpdateApplication(svc.ctx, mod) { - return nil, errors.New("Not allowed to update application") + return nil, ErrNoPermissions.withStack() } - // @todo: make sure archived & deleted entries can not be edited - return t, svc.db.Transaction(func() (err error) { if t, err = svc.application.FindByID(mod.ID); err != nil { return @@ -117,12 +110,9 @@ func (svc *application) Update(mod *types.Application) (t *types.Application, er } func (svc *application) DeleteByID(id uint64) error { - // @todo: make history unavailable - // @todo: notify users that application has been removed (remove from web UI) - app := &types.Application{ID: id} if !svc.ac.CanDeleteApplication(svc.ctx, app) { - return errors.New("Not allowed to delete application") + return ErrNoPermissions.withStack() } return svc.application.DeleteByID(id) } diff --git a/system/types/applications.go b/system/types/applications.go index 04c052430..14088c166 100644 --- a/system/types/applications.go +++ b/system/types/applications.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/cortezaproject/corteza-server/pkg/permissions" + "github.com/cortezaproject/corteza-server/pkg/rh" ) type ( @@ -33,6 +34,19 @@ type ( Config string `json:"config"` Order uint `json:"order"` } + + ApplicationFilter struct { + Name string `json:"name"` + Query string `json:"query"` + + Sort string `json:"sort"` + + // Standard paging fields & helpers + rh.PageFilter + + // Resource permission check filter + IsReadable *permissions.ResourceFilter `json:"-"` + } ) func (a *Application) Valid() bool { diff --git a/system/types/role.go b/system/types/role.go index 955f7e99f..6477441ab 100644 --- a/system/types/role.go +++ b/system/types/role.go @@ -4,6 +4,7 @@ import ( "time" "github.com/cortezaproject/corteza-server/pkg/permissions" + "github.com/cortezaproject/corteza-server/pkg/rh" ) type ( @@ -20,6 +21,14 @@ type ( RoleFilter struct { Query string + + Sort string `json:"sort"` + + // Standard paging fields & helpers + rh.PageFilter + + // Resource permission check filter + IsReadable *permissions.ResourceFilter `json:"-"` } ) diff --git a/tests/system/application_test.go b/tests/system/application_test.go index 9fa97c9d1..6594b8b53 100644 --- a/tests/system/application_test.go +++ b/tests/system/application_test.go @@ -63,7 +63,7 @@ func TestApplicationCreateForbidden(t *testing.T) { FormData("name", "my-app"). Expect(t). Status(http.StatusOK). - Assert(helpers.AssertError("Not allowed to create application")). + Assert(helpers.AssertError("system.service.NoPermissions")). End() } @@ -89,7 +89,7 @@ func TestApplicationUpdateForbidden(t *testing.T) { FormData("name", "changed-name"). Expect(t). Status(http.StatusOK). - Assert(helpers.AssertError("Not allowed to update application")). + Assert(helpers.AssertError("system.service.NoPermissions")). End() } @@ -120,7 +120,7 @@ func TestApplicationDeleteForbidden(t *testing.T) { Delete(fmt.Sprintf("/application/%d", a.ID)). Expect(t). Status(http.StatusOK). - Assert(helpers.AssertError("Not allowed to delete application")). + Assert(helpers.AssertError("system.service.NoPermissions")). End() }